DFHack / dfhack

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

buildingplan: Auto-allocation toggles while navigating with numpad #1544

Open Nilsolm opened 4 years ago

Nilsolm commented 4 years ago

It seems that navigating with the Numpad while having a designated furniture selected toggles auto-allocation if the assigned key is pressed. Pressing '1' in this case allocates the room to the relevant noble position instead of moving the cursor.

This is admittedly a rather minor thing, but it can lead to confusion if players accidentally auto-allocate rooms to certain nobles. It happened to me until I've found out what caused it.

How to reproduce:

  1. Select a relevant furniture in a designated room (bed, table, chair).
  2. Use the Numpad to navigate while pressing one of the assigned keys.

DFHack version:

Platform & OS:

I haven't tried other platforms. It's possible that this is specific to Linux.

Possible solutions:

lethosor commented 4 years ago

This might not be that hard to fix, actually. The feed() method gets a list of all interfacekeys that are used, so it could do nothing in this case if a CURSOR* key is present. However, those might also trigger with the default keybindings if the regular number keys are used... seems like a problem we've dealt with before, but I don't remember how off the top of my head.

Nilsolm commented 4 years ago

Alright, I'll look into it sometime next week and see if I can figure it out.

lethosor commented 4 years ago

tweak shift-8-scroll is one I thought of, but that's doing something else (resolving a conflict between a SECONDSCROLL and CURSOR key by picking the former).

I don't have a numpad, so testing this is a bit tricky. One thing you could try: using this script, check what keys are produced when you press regular 1 vs numpad 1 (with as close to the default keybindings as possible). Hopefully at least something is different. If not, we may have to rework the interface to not conflict with numpad scrolling (e.g. by hiding these options in a submenu).

Nilsolm commented 4 years ago

It seems that both "1" and "Numpad 1" are bound to the same set of commands by default: A_CONV_PERSUADE, CURSOR_DOWNLEFT, A_MOVE_SW, STRING_A049, so no easy solution there. It seems putting the option in a submenu that inhibits navigation is the better option.

Also, while testing this, I have stumbled upon another bug: pressing "Numpad 0" while a viewscreen with auto-allocation is active causes the game to crash. GDB says: https://gist.github.com/Nilsolm/9355304f69134de2862f99e0a95d5484

It seems this part is the culprit: https://github.com/DFHack/dfhack/blob/8427f518c941f8e5734e98f2ff56648522f1281b/plugins/buildingplan.cpp#L196-L210

Numpad 0 corresponds to STRING_A048 according to your script which I think is what's causing it. Not sure why the other plug-ins are throwing errors though.

I'll see what I can do about this during the weekend.

lethosor commented 4 years ago

Yeah, STRING_A048 shouldn't be allowed if the keys start at 1, and 0 would never be greater than np.size() so that check before return false would never fire. The other plugins are showing up in the backtrace because those also interpose viewscreen_dwarfmodest::feed(). Multiple plugins can do this, so a call to INTERPOSE_NEXT will end up calling the next function in the chain, which could be either the native DF function or another plugin's function. In your case, the zone plugin called buildingplan code from here, but the exact order depends on runtime factors like what order plugins were enabled in. (It usually shouldn't make a difference.)