embark-anyone.lua bugfix: Test the current viewscreen #1485
Conversation
the choose_start_site viewscreen, before trying to use it. This was found while diagnosing Issue #5509, but is not related. Minimal changes to the script.
embark-anyone.lua
Outdated
| @@ -3,6 +3,9 @@ local utils = require('utils') | |||
|
|
|||
| function addCivToEmbarkList(info) | |||
| local viewscreen = dfhack.gui.getDFViewscreen(true) | |||
| if viewscreen._type ~= df.viewscreen_choose_start_sitest then | |||
There was a problem hiding this comment.
this code is duplicated, which means it should be pulled out as its own function that is called in both places that it appears
There was a problem hiding this comment.
My intent was to keep the changes minimal. I'm willing to refactor if you think it's important.
There was a problem hiding this comment.
I have a strong objection to duplication of code and any sort of cut-and-paste programming. Toady does this a lot and DF's code suffers for it.
Also, a change that doesn't duplicate code would not be less minimal than the change you're proposing, at least by my standards.
There was a problem hiding this comment.
Okay, found a minimal-change that doesn't duplicate code.
Edit: what is the local policy on comments? I was shocked to see how few comments there are in most scripts.
There was a problem hiding this comment.
We don't have a lot of solid editorial policy in general, and I'm especially poorly suited to comment as I am not much of a Lua programmer and thus don't have a sense of what good form for Lua would be
The bug: if the embark-anyone script is executed on any viewscreen other than the embark viewscreen, it aborts with a stack trace. This bugfix makes it cleanly abort with a reasonably-descriptive error message. This was found while diagnosing Issue #5509, but is not related. Minimal changes to the script. Note on the code change: by moving the function addCivToEmbarkList() inside the function embarkAnyone(), addCivToEmbarkList() cannot execute unless embarkAnyone() has at least passed its safety check.
Test the current viewscreen to ensure that it is the choose_start_site viewscreen, before trying to use it.
The bug: if the embark-anyone script is executed on any viewscreen other than the embark viewscreen, it aborts with a stack trace.
This bugfix makes it cleanly abort with a reasonably-descriptive error message.
This was found while diagnosing Issue #5509, but is not related.
Minimal changes to the script.