Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix usage of stale state in legends racefilter #1346

Merged
merged 3 commits into from
Dec 7, 2024
Merged

Fix usage of stale state in legends racefilter #1346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d288642
Reset racefilter on world and map load
NicksWorld Dec 5, 2024
e8bb513
Add entry to changelog
NicksWorld Dec 6, 2024
b6372bf
Only reset state on world load
NicksWorld Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Template for new versions:
- `geld`, `ungeld`: save-and-reload no longer loses changes done by `geld` and `ungeld` for units who are historical figures
- `rejuvenate`: fix error when specifying ``--age`` parameter
- `gui/notify`: don't classify (peacefully) visiting night creatures as hostile
- `exportlegends`: fix race filter not refreshing on world load, leading to incorrect results

## Misc Improvements
- `idle-crafting`: also support making shell crafts for workshops with linked input stockpiles
Expand Down
5 changes: 5 additions & 0 deletions exportlegends.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,11 @@ dfhack.onStateChange[GLOBAL_KEY] = function(sc)
if sc == SC_VIEWSCREEN_CHANGED and df.viewscreen_choose_game_typest:is_instance(dfhack.gui.getDFViewscreen(true)) then
asyncexport.reset_state()
end

-- Reset state when a world or map is loaded to ensure data remains current
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why map and not just world? isn't race specific to world?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes can occur between loading the map and opening legends thanks to the open-legends tool. As fort mode could easily modify historical data, I thought it safest to ensure that path starts with a reset state.

Copy link
Member

@myk002 myk002 Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change won't catch that specific case -- if a map is loaded, then the race of a histfig is changed (dangerous, btw!), then open-legends is run, this check won't catch it. I'm not certain that it's a case we really need to address, though. resetting on world load, however, is very important and should absolutely be done here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only reason I'm being picky about this is that maps are loaded and unloaded all the time in adventure mode, and I don't think we should run more than we really have to

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally worried about werebeasts changing the race of histfigs, but after some research that doesn't appear to occur. As we aren't particularly worried about the case of open-legends, removing the reset on map load seems safe to me.

if sc == SC_WORLD_LOADED then
racefilter.reset_state()
end
end

if dfhack_flags.module then
Expand Down
Loading