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.

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

Prompt/query hotkeys (e.g. when repairing) are "masked" by global binds (regression) #20065

Closed
keyspace opened this issue Jan 21, 2017 · 15 comments

Comments

7 participants
@keyspace
Copy link
Contributor

commented Jan 21, 2017

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

This comment has been minimized.

Copy link
Contributor Author

commented Jan 21, 2017

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

@keyspace

This comment has been minimized.

Copy link
Contributor Author

commented Jan 21, 2017

Tested on f136e3d (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

This comment has been minimized.

Copy link
Contributor Author

commented Jan 21, 2017

Repeated bisect, 73bc2a4 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

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2017

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

This comment has been minimized.

Copy link
Contributor Author

commented Jan 27, 2017

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

This comment has been minimized.

Copy link
Member

commented Jan 28, 2017

@keyspace

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2017

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 keyspace changed the title Some hotkeys no longer do anything Prompt/query hotkeys (e.g. when repairing) are "masked" by global binds (regression) Jan 28, 2017

@keyspace

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2017

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

As always, sorry for the noise. :/

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jan 28, 2017

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jan 28, 2017

@BrettDong

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2017

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

@AdonaiJr

This comment has been minimized.

Copy link

commented Feb 7, 2017

action menu

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

This comment has been minimized.

Copy link
Contributor Author

commented Feb 10, 2017

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

This comment has been minimized.

Copy link
Contributor Author

commented Feb 10, 2017

Looked at #20033 more, above seems unrelated.

@kolsurma

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2017

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

@kevingranade kevingranade added this to the 0.D milestone Mar 8, 2017

@kevingranade kevingranade added this to Closed Issues in 0.D Release Aug 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.