search

DFHack / dfhack

Memory hacking library for Dwarf Fortress and a set of tools that use it
Other
1.84k stars 463 forks source link

Fix thread-safety issues around armok mode logic #4678

Closed lethosor closed 3 weeks ago

lethosor commented 3 weeks ago

Followup to feedback in #4674.

I verified that dfhack.internal.setArmokTools() and dfhack.setMortalMode() can still be called from the lua interpreter without deadlocking. I haven't seen meaningful changes occur from calling them, though, so my testing may not be exhaustive.

myk002 commented 3 weeks ago

quick way to test:

  1. disable mortal mode
  2. go to the main dwarfmode map and hit Ctrl-Shift-T to bring up gui/teleport (which should come up)
  3. enable mortal mode
  4. go back to the map and hit Ctrl-Shift-T again. nothing should happen

you can see more info about the change in processing by running this before doing the testing:

debugfilter set debug core keybinding
myk002 commented 3 weeks ago

also, to test setArmokTools:

  1. enable mortal mode
  2. on main dwarfmode map, hit Ctrl-Shift-T and fail to bring up gui/teleport
  3. exit back to title screen
  4. edit hack/docs/docs/tools/gui/teleport.txt and take armok out of the tags
  5. load up a world, Ctrl-Shift-T should now launch gui/teleport even with mortal mode enabled
lethosor commented 3 weeks ago

I was trying to filter out teleport in gui/launcher, with no luck. Didn't try loading a fort.

myk002 commented 3 weeks ago

that shouldn't be affected by this change, and should continue to work.

However, I think I see the issue that you might have been running into -- the tag filters in gui/launcher are not reset when you change the mortal mode config setting in gui/control-panel. I'll see if I can make that experience a little more smooth.

https://github.com/DFHack/dfhack/assets/977482/b174cbbb-bf78-4b56-b227-9685c482cfaf

myk002 commented 3 weeks ago

fixed in https://github.com/DFHack/dfhack/pull/4680 and https://github.com/DFHack/scripts/pull/1153

lethosor commented 3 weeks ago

That would explain what I was seeing, thanks.