CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.27k stars 4.12k forks source link

Keybindings menu - UI audit #58358

Open ZeroInternalReflection opened 2 years ago

ZeroInternalReflection commented 2 years ago

Describe the bug

The keybindings menu has several graphical and navigational issues. These include:

Display issues: 1] Actions with several keybindings associated will print over the border/wrap around to the next line: Keybinding (?) -- Border overprinting 2] The keybindings menu doesn't seem to want to use the full width/height available, leaving a border of the previous screen: Keybindings menu -- 80x24 -- Tiles 3] The menu says "Keybinding is active only on this screen", but there is no indication what "this screen" is or what it means (e.g. setting a keybinding for the in-game MAIN MENU (Escape) sets that keybinding for all uilist screens, rather than just that menu. 4] White scrollbar line on a dark gray border 5] When using -, some actions will reset to the default value, other keybindings will leave the action unbound. I'm not entirely clear when/why 6] Some translations draw attention to -/+/= keybindings, while others do not: Keybindings -- 80x24 -- Russian 7] Some action descriptions are cut off by the keybinding (pictured: 'Examine nearby terrain, furniture, and items:' in Russian. Similar issues have been found in Polish and Spanish) Keybindings - Cut off text -- Medium size -- Russian Accessibility issues: 8] The cursor is not set for screen readers The cursor is set within string_input_popup Navigation issues: 9] No mouse support is available (Should be added to #58106) 10] If a player hits ? twice in rapid succession, they end up in the keybindings menu for the keybindings menu, with no indication that they've done something wrong 11] Page up/Page down fast scroll will loop around to the other side, while Up arrow/Down arrow will stop at the beginning/end of the list. This seems counterintuitive 12] Many Ctrl+ key combinations work as keybindings, but attempting to bind a key to Ctrl+i binds it to Tab as well. Other overlaps might exist This is apparently due to terminal limitations. See discussion below

Steps to reproduce

  1. While in-game, open the keybindings menu with ?, then follow individual steps

Expected behavior

Screenshots

No response

Versions and configuration

Tiles - OS: Linux - OS Version: LSB Version: n/a; Distributor ID: Arch; Description: Arch Linux; Release: rolling; Codename: n/a; - Game Version: fdf38c4 [64-bit] - Graphics Version: Tiles - Game Language: English [en] - Mods loaded: [ Dark Days Ahead [dda], Disable NPC Needs [no_npc_food], No Fungal Growth [no_fungal_growth], Bionic Professions [package_bionic_professions] ]
Curses - OS: Linux - OS Version: LSB Version: n/a; Distributor ID: Arch; Description: Arch Linux; Release: rolling; Codename: n/a; - Game Version: 0.F-9048-g7cd1fb57676 [64-bit] - Graphics Version: Curses - Game Language: English [en] - Mods loaded: [ Dark Days Ahead [dda], Disable NPC Needs [no_npc_food], No Fungal Growth [no_fungal_growth], Bionic Professions [package_bionic_professions] ]

Additional context

For clarification of some of my UI terminology (e.g. scroll vs. fast scroll, column vs. pane), refer to my general notes here

Qrox commented 2 years ago

5] When using -, some actions will reset to the default value, other keybindings will leave the action unbound. I'm not entirely clear when/why

I think that's because some keybindings have no global counterparts so removing the local keybindings makes them unbound.

8] The cursor is not set for screen readers

I think the cursor is currently set to the text input field.

hexagonrecursion commented 2 years ago

/confirmed

This issue lumps together a number of related bugs without a common cause. @ZeroInternalReflection, please split it into several issues to make keeping track of the troubleshooting progress easier

1] Actions with several keybindings associated will print over the border/wrap around to the next line:

confirmed

2] The keybindings menu doesn't seem to want to use the full width/height available, leaving a border of the previous screen:

why?

3] The menu says "Keybinding is active only on this screen", but there is no indication what "this screen" is or what it means (e.g. setting a keybinding for the in-game MAIN MENU (Escape) sets that keybinding for all uilist screens, rather than just that menu.

confirmed: setting a keybinding for the [ESC] MAIN MENU also assigns it for the item details menu

4] The scrollbar line colour and border colour should match

Why? This makes the scroll bar more obvious.

5] When using -, some actions will reset to the default value, other keybindings will leave the action unbound. I'm not entirely clear when/why

Confirmed: the default keybinding config is a mess: some keybindings are local, some are global, some are unbound locally, some are unbound globally, some are disabled

Screenshot from 2022-06-12 09-01-57

unbinding move north and pause has different results:

Screenshot from 2022-06-12 09-05-36

I think the keybinding system might just be too complicated: a given command can be in too many states. This both makes it hard for the user to understand what is going on and makes it hard for the UI to represent and control the states

6] Some translations draw attention to -/+/= keybindings, while others do not:

What do you mean by "draw attention"?

7] Some action descriptions are cut off by the keybinding (pictured: 'Examine nearby terrain, furniture, and items:' in Russian. Similar issues have been found in Polish and Spanish)

Confirmed

8] The cursor is not set for screen readers

How to test this?

9] No mouse support is available

Confirmed

10] If a player hits ? twice in rapid succession, they end up in the keybindings menu for the keybindings menu, with no indication that they've done something wrong

Confirmed. A subset of 3?

11] Page up/Page down fast scroll will loop around to the other side, while Up arrow/Down arrow will stop at the beginning/end of the list. This seems counterintuitive

Confirmed.

12] Many Ctrl+ key combinations work as keybindings, but attempting to bind a key to Ctrl+i binds it to Tab as well. Other overlaps might exist

Confirmed.

Night-Pryanik commented 2 years ago

Some translations draw attention to -/+/= keybindings, while others do not:

I don't think it's an UI issue per se, it's more language-specific translation issue, as translators make their translations as they please and sometimes may ignore some original texts.

ZeroInternalReflection commented 2 years ago

Qrox, 5] When using -, some actions will reset to the default value, other keybindings will leave the action unbound. I'm not entirely clear when/why

I think that's because some keybindings have no global counterparts so removing the local keybindings makes them unbound.

I think it would be beneficial to, instead of indicating global vs. local with a colour, to list both the global and the local keybinding. Perhaps a structure like this: Action | Local keybinding | Global keybinding | Default keybinding So that you could indicate that removing local causes it to use the global, and removing the global leaves it unbound, but can be reset to the default. This would require a fair bit more room, though.

8] The cursor is not set for screen readers

I think the cursor is currently set to the text input field.

You are correct. I did not spot that it was set inside string_input_popup. I have tested with orca, and it seems to work. Whether it read the list of keybindings below the text input was a bit dodgy, but I think that's an orca issue. I'm willing to cross that one out unless someone who actually uses a screen reader raises any shortcomings.

hexagonrecursion:

This issue lumps together a number of related bugs without a common cause. @ZeroInternalReflection, please split it into several issues to make keeping track of the troubleshooting progress easier

I raised whether to group issues or split them up on the Discord prior to starting this, and the preference came down on single issue per menu to limit boilerplate. However, now that I've done a bunch of them and this one has some discussion, it's probably time to have that conversation again to see if the plan still makes sense. I can do multiple issues per menu going forward, but it will definitely slow things down. Only 350 more UI screens to go.

2] The keybindings menu doesn't seem to want to use the full width/height available, leaving a border of the previous screen:

why?

Why is it occurring? Or why do I think that this looks a bit sloppy? Keybindings - Visible portion of underlying screen -- 80x24 If it tried to leave the header of the underlying screen visible, I would understand. However, whether Randal Culver is no longer afraid is of no relevance to the keybinding screen.

4] The scrollbar line colour and border colour should match

Why? This makes the scroll bar more obvious.

Making the scrollbar stand out like that is a valid design choice., It's not what I favour, but it's an option. However, if that is the design choice, then a variety of places where the scrollbar has been specifically set to match the border should be changed accordingly: Martial arts style with scrollbar -- 80x24 Credits with scrollbar -- 80x24

6] Some translations draw attention to -/+/= keybindings, while others do not:

What do you mean by "draw attention"?

In the English text above, the -/+/= keybindings are simply written into the sentence. In the Russian translation, they're <<->>/<<+>>/<<=>> (I'm not actually sure how to replicate the double <<) I don't consider this a significant issue on its own, but I noted it because there's very little consistency in how keybindings are shown on screen. Sometimes they're left as part of the word but in a different colour. Sometimes they're left as part of a word but in bold, sometimes they're in brackets. Then there's all the different colours that are used to flag keybindings... I'd just like a little consistency is all.

leemuar commented 2 years ago

Added mouse support for this window to my to-do list

chrispikula commented 2 years ago

Was the issue with unbinding something permanently fixed? It used to be, (circa 6 months ago), get auto-populated with the default next game restart, even if you've re-keyed that key to something else.

Example, remove the change view position keys so that you can use HJKL for something more useful.

Qrox commented 2 years ago

auto-populated with the default next game restart

Sounds like #58402 which seems to be caused by game launchers.

chrispikula commented 2 years ago

Sounds like #58402 which seems to be caused by game launchers.

I'll look into it some more, but I don't use a launcher. Weird.

l29ah commented 1 year ago

Would be nice if there was a reset keybinding action in addition to -remove keybinding action.

Klokinator commented 1 year ago

Added mouse support for this window to my to-do list

I'm currently playing on Stable. Was this added at any point to an experimental release? Rebinding your buttons is agony as of this moment. Being able to simply click and rebind something is necessary QOL in 2023.

See: Literally any game made after 2010.

Zireael07 commented 1 year ago

@Klokinator I know there's been a lot of movement around mouse support and UI recently. Do you mean 0.G when you say "stable"?

leemuar commented 1 year ago

Was this added at any point to an experimental release?

it was not yet added

leemuar commented 1 year ago

I already have experience of implementing mouse controls to cata windows - I'll do scrolling and "click-to-set-the-key" for this window

Klokinator commented 1 year ago

Yeah, 0.G is the current Stable release. I was asking if any of the newest Experimental builds had better mouse support, specifically for keybinding. I ended up slowly rebinding all my keys the current way, but it was indeed... quite painful.

I already have experience of implementing mouse controls to cata windows - I'll do scrolling and "click-to-set-the-key" for this window

Would be awesome of you!

leemuar commented 1 year ago

Yeah, 0.G is the current Stable release. I was asking if any of the newest Experimental builds had better mouse support, specifically for keybinding. I ended up slowly rebinding all my keys the current way, but it was indeed... quite painful.

I already have experience of implementing mouse controls to cata windows - I'll do scrolling and "click-to-set-the-key" for this window

Would be awesome of you!

it was merged in recent experimental, any build after April 9 have it now

Brambor commented 8 months ago

These are solved by #64632

katemonster33 commented 8 months ago

@Brambor are you meaning to link #65709 ?

Brambor commented 8 months ago

@Brambor are you meaning to link #65709 ?

I might have quoted wrong PR, but I ment something merged. I found out in game and then blamed the file.

db48x commented 8 months ago

12] Many Ctrl+ key combinations work as keybindings, but attempting to bind a key to Ctrl+i binds it to Tab as well. Other overlaps might exist

It must be noted that inside of a terminal, Control+i and Tab are the exact same thing. Programs running inside of a terminal cannot distinguish between them. Specifically, the first 32 characters (numbers 0 through 31) in the ASCII standard are typed by holding down control and pressing another key. Whatever other key you press is bitwise ANDed with 0x1F to mask off the top three bits. If you type an i, then it computes 0x69 && 0x1F to get 0x09, which is a tab character. Similarly, a Backspace is Control+h, a Line Feed is Control+j, Control+l is a Form–feed character (which is why it clears the screen of a glass teletype), and so on.

Some terminal emulators, such as XTerm, have a setting that causes all of these key combinations to send a long escape sequence describing exactly what modifier keys were held at the time. Many terminal emulators do whatever XTerm does, so some of them have a similar setting. Others habitually invent their own thing and have a different set of escape codes. However, none of them do that by default because most programs aren’t expecting these types of escape codes.

GUI applications do not have this limitation; they receive separate keydown and keyup events for every key. Each event includes both the scancode and the corresponding character as looked up in the user’s chosen keyboard layout, along with state bits that remind the application which modifier keys are being held down (saving you from always having to keep track of that in every application you write).

Brambor commented 6 months ago

9b] It should be possible to select a keybinding for modification with the mouse

Is actually solved. In the legacy KB screen, you can click anywhere on the line. In the new imgui KB screen, you have to click the text directly (which is a bit confusing).

Note: You first need to select what shall be done (add +, remove -, ...), before you can select the action (row) to modify it.

Brambor commented 6 months ago

5] The results of using - to remove a keybinding should be clear and predictable

Is bit better after #71968 & #72045, but probably still unresolved.