control-panel repeats persistence #1170
Conversation
Because internal\control-panel\common.lua:persist_enabled_repeats only ever persists true values, values indexed out of enabled_repeats will only ever be true (loaded from persisted data) or nil (not present in persisted data), leaving this condition always true.
To let user-changed "enabled" settings for control-panel repeat commands take precedence over those command's autostart settings and default-enabled definitions, always convert the enabled_repeats value to a boolean and pass it as the "enabled" argument of apply_command when restoring repeat commands in apply_fort_loaded_config. Because of a limitation in the persistence data (only the enabled repeat commands are recorded), we can not readily tell the difference between a "new" default-enabled repeat command (e.g. introduced in a newly updated DFHack installation) and a previously existing repeat command that the user intentionally disabled. This means that we can not automatically enable such a "new" default-enabled repeat command when loading a DF save under an updated DFHack installation. The user can still manually enable such a "new" repeat command, and that setting will be persisted and restored properly (new embarks are not affected since they are handled in apply_autostart_config where the "enabled" argument is left off to get the defaulting behavior).
This will let us distinguish "new" repeat commands (in new DFHack versions) from those that were simply disabled.
Concentrate the occurrences of the 'control-panel/' repeat prefix into just the (un)munging helper functions.
Since the persistence data was updated to include disabled repeat commands, we can now distinguish newly added repeat commands from those that were disabled. Remove the boolean conversion in control-panel.lua:apply_fort_loaded_config so that "new" commands (nil in enabled_repeats) will be applied with their default enabled value.
|
I think this fixes a legitimate issue with the changes that migrated
The easiest way is to subscribe to the You can also build DFHack from source, but that is a more involved process, and totally not necessary if you're just modifying scripts. |
|
Thanks for the review! Okay, the changelog now has a commit, too. I noticed that there is another nearby change (under Misc. Improvements), but GitHub says "no conflicts" here in this PR (my local, trial merge and rebase also resolved without any conflict). |
|
Looks good! Please feel free to add your name to Authors.rst! |
DF v50.13
DFHack 50.13-r2.1
Scenario
In one of my games, I used the DFHack GUI control-panel to enable the
automilkandautoshearrepeating commands. Later, after having saved and resumed, I noticed that milk was not being gathered. Checking the control-panel showed both those repeats disabled. I am new to DF and DFHack (and Lua), so I don't know if this was the expected behavior, but I thought I would investigate.I do not have anything set to "autostart" (which is for new embarks?). It looks like it also acts as a kind of override to the default-enabled definition when the "default" is requested (nil/missing "enabled" parameter in apply_command calls). An autostart configuration changes the behavior with respect to default-disabled commands: if the user enabled a default-disabled command and configured it for autostart, then it would be enabled after loading, but only due to the autostart configuration—even though that is not an "autostart" situation, as I understand it.
Poking through the save data revealed references to some of the repeat commands in one of the files. Making a new save right after enabling
automilkandautoshearshowed them in the save data. But they were always disabled when I loaded that save. Later experimentation showed thatfix/dead-units(an example default-enabled repeat) was affected in the opposite way: it was always enabled after loading even if it was disabled when the save was made.It looks like the intention is to save and restore the enabled/disabled status of these repeat commands, so I started making changes to get that working for me. Each commit is a fairly small change to keep them more focused and hopefully easier to review and discuss. If any/all of them should be squashed instead, I can reroll them that way.
In the following descriptions, a "future" repeat command is one that was not known by the DFHack that made a save, but is known by the DFHack that is loading the save.
Minimal Change (no persistence changes)
The first two commits (through 5e7fe93) accomplish the saving and loading. The code will now treat any repeat command that is not present in the save data as if it were disabled. This means future default-enabled repeat commands would be initially disabled. The user could still enable these future commands and that setting would be subsequently loaded as enabled.
Larger Change (persistence change)
The rest of the commits make it so that future repeat commands will use the default mechanism (autostart configuration and default definition) when loading a save that does not mention them. The persisted data is changed to include both enabled and disabled repeats (true and false values).
Possible Extension (prevent some spurious defaulting)
There is a small(?) drawback to the final implementation: since "old" saves did not record disabled repeats, they will be treated as "new/future" repeats and will get their default (or autostart) settings (i.e. a default-enabled repeat that was disabled by the user in an "old" save will be loaded as enabled, but any subsequent changes will be properly handled across new saves and loads). This could be partially worked around by adding another bit of persisted data that indicates how the missing repeat commands should be treated (missing means disabled if flag is not set, otherwise missing means new and use its default). It would only be a partial workaround because loading from an old save (prior to the new persistence changes) would still initially disable any "future" default-enabled repeat commands instead of defaulting them to enabled.
This drawback might be acceptable since it is a subset of how the code already handles things (a disabled default-enabled repeat command is loaded as enabled).
Testing
I tested these changes by extracting a patch against master and applying it to the base DFHack 50.13-r2.1
hack/scripts. I tried replacingscriptsas a whole, but there are other changes on master that expect new surrounding bits (e.g. the mortal/armok change).Is there a recommened way to test unreleased DFHack? Use recent CI build and replace some bits?