Cataclysm-dda: Prompt/query hotkeys (e.g. when repairing) are "masked" by global binds (regression)

[keyspace]

PR #20041 made some keys (most importantly, 2 and q, but also others) unavailable in queries, e.g. when repairing an item with a tool.

Savefile demonstrates this. Character has a soldering iron and a pocket knife. Activating the iron and choosing the knife for repair brings up a repair/practice window, where 2 and q are shown as hotkeys for "repair as long as you can" and "cancel" respectively. Pressing those keys results in the selection moving down and no-action respectively.

This issue is only partially addressed by follow-up PR #20072 ATM - q does quit, but 2/8, for example, are still mapped to scrolling.

---

This issue has been split for easier tracking. Original many-edit braindump after the double break.

-----

Several issues conflated.

System: Arch Linux, curses, self-built, current git master 0.C-20721-g82a1273ad9.

Cheat menu actions skip mappings

See #20091.

Works as intended? Missing keys are 2/8 (up/down on numpad), f (find), j/k (vi up/down), and q (quit).

Guess PR #20041, commit 18d78a0 ("Prevent hotkeys in uimenu that are already mapped to actions.") - confirmed by bisect.

Can provide save if necessary, but easier to test through cheat menu (that's what I did). Previously, all actions in it were mapped to keys (seemingly sequentially). Now, several actions don't have a key associated.

Commit can be easily reverted (will submit PR), but perhaps it was needed for fixing the bug?..

Mapping f to "find" breaks cheat-wishing contained liquid

Incorrect! See #20147.

Now, both / and f are mapped to "find".

Could be easily remedied by mapping something else to "contained" (c?). Won't work well with wishing for a friendly monster, though.

Crafting prompts don't map q

See top of this report.

Repairing an item with a soldering iron no longer maps "quit" to q key. The q is highlighted in the "Repairing/Practicing" prompt, but pressing the key does nothing.

For testing save, see below.

Pressing ESC does nothing either (pre-existing).

---

Ping @BevapDin.

EDIT: Removed EDIT notes for readability, re-structured.

[keyspace]

Correction: reverting that commit 18d78a0 fixes the cheat menu, but not quitting from item repair menu.

[keyspace]

Tested on f136e3d688 (merge commit previous to #20041), works OK.

Attaching save file used: test-repair-hotkeys.zip - character has soldering iron and items to repair/practice.

[keyspace]

Repeated bisect, 73bc2a4f914332d824ba45d5ba792f42d4c6c1e6 culprit for repair menu/query/prompt failure.

Anecdotal, similar issue reported on IRC for "Use which component?" prompt - didn't ask for build/system info.

[Coolthulhu]

That "skipping" feature is pretty seriously wrong: menu autoassignment SHOULD go 1-9-0-a-z, with no breaks in between.
This would only really make sense if it avoided only menu-specific keys, explicitly bound to functions. For example, 'q' for quit.

[keyspace]

There's worse: mapping f to "find" results in both / and f mapped to "find" in cheat-wish item menu, making it impossible to cheat-wish a contained liquid.

[kevingranade]

I'm very much leaning toward just reverting 18d78a0. Theres no apparent
upside, and it broke a number if menus.

[keyspace]

As I've mentioned in PR #20072, reverting that one commit is not enough. It'll make things even worse.

18d78a0 is part of PR #20041.

20065 (this issue) was introduced by PR #20041, which fixed issue #20037, which was introduced in PR #19961, which doesn't seem to reference any then-open issue, but outlines as issue by itself, namely different input handling - "getch in (*nix?) curses, native functions in SDL (and/or?) Windows".

20065 (this issue) is conflated - three separate ones by now, described "as examples" rather than underlying broken functionality. PR #20072 fixes one of three (EDIT: only partially).

I guess I'll go split them into separate ones (retaining the original mess). EDIT: reworked OP, commented in already-open #20091, and opened #20147.

[keyspace]

Almost forgot: PR #19961 introduced issue #20047, which was fixed in #20063.

The graph looks like this (rewrote actual names with descriptions):

• PR #19961 - replace getch with input manager
  
  
  
  • #20037 - menu disappears after mouse moved
  
  
  • PR #20041 - allow scrolling, use input_event
    
    
    
    
    
    • #20065 - prompt/query hotkeys unavailable (this issue)
    
    
    
    • PR #20072 - q hotkey becomes available, others not (not yet merged)
    
    
    
    • #20091 - debug menu skips mappings
    
    
    
    • #20147 - special input unavailable in cheat sub-menus
    
    
    
  
  
  • #20047 - CTRL+C bypassable, breaks screen refreshing
  
  
  • PR #20063 - fix that
  
  

As always, sorry for the noise. :/

[kevingranade]

The problem I'm seeing is that a series of PRs starting at #19961 has
introduced a number of different bugs with menu handling, and the fixes for
these bugs each target a different place in the code rather than
systemically adressing the root cause. In this kind of situation what I
want to do is roll back the changes to the last known good code and then
reapply the fixed version of the original code all at once.

A shorter version is that the menu bugs we have now are worse than the fix
that caused them, so the most direct fix is to revert and try again, more
carefully this time.

On Jan 28, 2017 10:15 AM, "Keyspace-1" [e-mail] wrote:

As I've mentioned in PR #20072
https://github.com/CleverRaven/Cataclysm-DDA/pull/20072#issuecomment-275862438,
reverting that one commit is not enough. It'll make things even worse.

18d78a0
https://github.com/CleverRaven/Cataclysm-DDA/commit/18d78a077fd745ad07033f1ecec4e2f0d14cdad0
is part of PR #20041
https://github.com/CleverRaven/Cataclysm-DDA/pull/20041.

20065 https://github.com/CleverRaven/Cataclysm-DDA/issues/20065 (this

issue) was introduced by PR #20041
https://github.com/CleverRaven/Cataclysm-DDA/pull/20041, which fixed
issue #20037 https://github.com/CleverRaven/Cataclysm-DDA/issues/20037,
which was introduced in PR #19961
https://github.com/CleverRaven/Cataclysm-DDA/pull/19961, which doesn't
seem to reference any then-open issue, but outlines as issue by itself,
namely different input handling - "getch in (*nix?) curses, native
functions in SDL (and/or?) Windows".

20065 https://github.com/CleverRaven/Cataclysm-DDA/issues/20065 (this

issue) is conflated - three separate ones by now, described "as examples"
rather than underlying functionality. PR #20072
https://github.com/CleverRaven/Cataclysm-DDA/pull/20072 fixes one of
three.

I guess I'll go split them into separate ones (retaining the original mess).

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/issues/20065#issuecomment-275864550,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA0gdOO6SFhadS8dCOXwH0pDno1aFeiqks5rW4W8gaJpZM4LqM4h
.

[kevingranade]

To be clear though, if there is a systemic solution to all the menu
breakage, we can go that way, I just don't want to be playing whack-a-mole
with this for the next month.

[BrettDong]

Is there anybody working on fixing this? Those problems on UI input are really annoying.

[AdonaiJr]

I used to apply hotkeys to actions on "action-menu", by hitting "?" (It was a "hidden" option). Doesn't seems to work anymore. Maybe it's also related to this.

[keyspace]

There is now also issue #20234, which was caused by PR #20033 - not strictly related to OP, but similar due to ESC having unexpected behaviour.

[keyspace]

Looked at #20033 more, above seems unrelated.

[kolsurma]

~Are the 2/8 and J/K keys required for normal scrolling in item menus?~

~2/8 on the number row and 2/8 on the numpad are received as the same keypress, if scrolling with the keypad is expected or required then they can't be assigned to options in those menus (debug spawning, action menu, martial arts, etc.)~

~If, instead, we can rely on the directional arrow keys (non-keypad) or J/K for scrolling the 2/8 keys can be freed and allowed to be used in the affected menus~

~Edit: The numpad is used for character movement, but the ASCII key code for the number keys are the same as the keypad.~

~Edit2: From what I've read, ncurses doesn't support a different code for keypad characters. The solution then should be un-mapping 2/8 from scrolling, maybe.~

All of that is actually fixed with #20174