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

Main menu overhaul (Part 1): Mouse controls + Vertical submenus #57764

Merged
merged 10 commits into from
Jun 2, 2022

Conversation

dseguin
Copy link
Member

@dseguin dseguin commented May 17, 2022

Summary

None

Purpose of change

Describe the solution

This is just a first pass introducing some QoL improvements to the main menu:

  • Allow mouse controls for menu and submenu buttons (SDL only)
  • Standardize on vertical secondary menus
  • Use uilists for tertiary menus

Mouse controls

Primary, secondary and tertiary menu options are mouse accessible on SDL builds (non-curses). Moving the mouse also highlights primary and secondary menu options:

2022-05-17.04-24-41.mp4

Left-clicking on a menu item selects it and confirms the option (where applicable). Mouse movement does nothing on tertiary menus since they rely on uilist for input handling.

The mouse wheel scrolls through vertical submenu options, as well as scrollable text like the MOTD/Credits windows.


Vertical submenus

Secondary menus use a common vertical layout:

vert_submenu

A couple of extra minor changes:

  • Submenus (where applicable) are automatically displayed when switching main menu options
    • Menu items are navigated with LEFT/BACKTAB and RIGHT/TAB
    • Submenu items are navigated with UP/PAGEUP and DOWN/PAGEDOWN
    • Confirming a submenu option is done with ENTER
  • Hints for "New Character" options are listed on the line below the main menu options (in yellow)
  • Submenu hotkeys were revised to avoid conflicts with the main menu (since both are accessible at the same time)

Describe alternatives you've considered

This is just a first pass addressing some of the items from #57098 (mostly limited by the size of these changes). Future changes will address the problems with the Settings and World Options menus to round off the issue.

Testing

See clips above. Manually tested each menu and submenu item to check that they behave the same as before.

Also tested with the minimal resolution, just to make sure each submenu is accessible:

win_small

Also tested the curses build:

win_curses

Additional context

I took some liberties with some of the design elements, but I think it works pretty well.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels May 17, 2022
@github-actions
Copy link
Contributor

Spell checker encountered unrecognized words in the in-game text added in this pull request. See below for details.

Click to expand
  • %s has no loadable characters!
  • A<u|U>topickup
  • Copy World Sett<i|I>ngs
  • Sa<f|F>emode

This alert is automatically generated. You can simply disregard if this is inaccurate, or (optionally) you can also add the new words to tools/spell_checker/dictionary.txt so they will not trigger an alert next time.

@Night-Pryanik
Copy link
Contributor

Awesome!

Is it possible to highlight element that you mouse-clicked on first, and then start its function, not the way around, as it is now?

@dseguin
Copy link
Member Author

dseguin commented May 17, 2022

It's possible, for sure. I'm not sure if that's the way Erk would want it, since it would add extra clicks (and on touchscreen controls you would have to tap the option twice, but tapping twice also acts as ESC)

C.C.: @I-am-Erk

@Night-Pryanik
Copy link
Contributor

No, you misunderstood me. No need to double click: just highlight clicked option and then start its function right after that - all with one click.

@dseguin
Copy link
Member Author

dseguin commented May 17, 2022

Oh I see, my mistake :P

The short answer is "yes", but it would add a lot of complexity.

Nevermind, I'm totally wrong! It's literally a 2 line change:

diff --git a/src/main_menu.cpp b/src/main_menu.cpp
index 76090b507c..014b6ff48e 100644
--- a/src/main_menu.cpp
+++ b/src/main_menu.cpp
@@ -684,6 +684,7 @@ bool main_menu::opening_screen()
                     if( sel1 == getopt( main_menu_opts::HELP ) || sel1 == getopt( main_menu_opts::QUIT ) ) {
                         action = "CONFIRM";
                     }
+                    ui_manager::redraw();
                     break;
                 }
             }
@@ -692,6 +693,7 @@ bool main_menu::opening_screen()
                     sel1 = it.second.first;
                     sel2 = it.second.second;
                     action = "CONFIRM";
+                    ui_manager::redraw();
                     break;
                 }
             }

src/main_menu.cpp Outdated Show resolved Hide resolved
src/main_menu.cpp Outdated Show resolved Hide resolved
src/main_menu.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot added Code: Tooling Tooling that is not part of the main game but is part of the repo. BasicBuildPassed This PR builds correctly, label assigned by github actions labels May 17, 2022
Co-authored-by: Anton Burmistrov <Night_Pryanik@mail.ru>
@leemuar
Copy link
Contributor

leemuar commented May 17, 2022

No, you misunderstood me. No need to double click: just highlight clicked option and then start its function right after that - all with one click.

I think it might be better to highlight on "mouse over" and to activate on click. I have no idea how to implement Mouse Over events on menu items though...

@dseguin
Copy link
Member Author

dseguin commented May 17, 2022

I think it might be better to highlight on "mouse over" and to activate on click.

Turns out this was pretty easy:

2022-05-17.04-24-41.mp4

@dseguin dseguin added Info / User Interface Game - player communication, menus, etc. SDL: Tiles / Sound Tiles visual interface and sounds. Controls / Input Keyboard, mouse, keybindings, input UI, etc. labels May 17, 2022
@leemuar
Copy link
Contributor

leemuar commented May 17, 2022

Turns out this was pretty easy:

This is absolutely awesome addition, thanks!

@ZeroInternalReflection
Copy link
Contributor

This definitely fixes some of the quirks of the main menu. One note on the mouseover behaviour, however, is that the credits/MOTD displays get a bit tricky on small screens. You can get kicked out of the screen by moving your mouse such that it lands on a menu that you can't see (The video is in Ukrainian since I also wanted to test whether everything works when the menu options get bumped to a second line, and it appears to):
https://user-images.githubusercontent.com/89038572/168846661-63a935d8-86b6-4c1c-81dd-46d540c32dac.mp4

@Inglonias
Copy link
Contributor

Inglonias commented May 17, 2022

what. this is amazing.

@dseguin
Copy link
Member Author

dseguin commented May 17, 2022

You can get kicked out of the screen by moving your mouse such that it lands on a menu that you can't see

I'm not sure how to address this. I see 2 solutions, both involve adding a special case for TERMX size:

  • Prevent mouse functions when MOTD/Credits are selected
  • Require confirming the MOTD/Credits option to view the text in a new window

@ZeroInternalReflection
Copy link
Contributor

I'm not sure how to address this. I see 2 solutions, both involve adding a special case for TERMX size:

* Prevent mouse functions when MOTD/Credits are selected

* Require confirming the MOTD/Credits option to view the text in a new window

Is there a particular reason that display_text() is currently fixed at FULL_SCREEN_HEIGHT/WIDTH? Rather than having a slightly different mouse interface for the credits/MOTD, I'd favour just capping the window height in display_text(). I feel consistency in interface behaviour is more important than maximizing the size of the credits/MOTD panes. This test video uses a fixed allowance of 4 rows for tips/menu/etc. This should ensure the menu you're mousing over is always visible, no matter what screen size/language you're using.

80x24_modified_credits_height.mp4

@dseguin
Copy link
Member Author

dseguin commented May 18, 2022

Cheers, that's a much better solution. Not sure why I didn't consider it, haha.

Edit: I've also added mousewheel scrolling for submenus and MOTD/Credits windows. It just scrolls one element at a time regardless of mouse sensitivity (I don't think it's possible to get the number of scroll "ticks" without querying SDL directly).

@leemuar
Copy link
Contributor

leemuar commented May 18, 2022

I think it would be better to not activate "main" menus (MOTD, New Game, Load, World, Special, Settings, Help, Credits, Quit) by mouse hovering, it should require a click. It should be highlighted, but not activated.
Submenus however should act just like it is now

P.S. again, thanks for this change, it really made the game much more "reactive" and responsive to the player

@dseguin
Copy link
Member Author

dseguin commented May 19, 2022

About this:

not activate "main" menus [...] by mouse hovering, it should require a click

I tested hiding the submenus when mousing over:

2022-05-18 23-01-24

In my opinion, this creates a bit more complexity in the controls, since cycling the options using keys behaves differently than mousing over. It also goes against this point from #57098:

Submenu options (such as the new world/select a world dialogue above) automatically become visible when you highlight the main option ([World] in this case). You don't press enter or space to open a submenu. This reduces a lot of keypresses and menu navigation confusion at no cost, since because of item 3 below, if this isn't the menu you want, you just keep on scrolling.

@leemuar
Copy link
Contributor

leemuar commented May 19, 2022

In my opinion, this creates a bit more complexity in the controls, since cycling the options using keys behaves differently than mousing over. It also goes against this point from #57098:

These are good arguments, I haven't thought about that. Thanks for making it clear for me!

@I-am-Erk
Copy link
Member

This looks great to me. How does it respond to a touch screen?

@dseguin
Copy link
Member Author

dseguin commented May 19, 2022

How does it respond to a touch screen?

I don't have the hardware to test this, but from looking around it looks like touch gestures are mapped to keys. So tapping once is just a . input, which means "confirm". So it looks like no tapping on menu options, unfortunately.

I think checking for touchscreen tap coordinates would require some voodoo magic (or implementing a native Android UI like in #53132). Brett also pointed out that the buttons might be too small to press accurately on a touch screen, so it might be a whole other project

Edit: I enabled release actions on my fork so there are Android builds if anyone wants to give it a try: https://github.com/dseguin/Cataclysm-DDA/releases/tag/cdda-experimental-2022-05-19-1658

@Stone94
Copy link
Contributor

Stone94 commented May 24, 2022

I've been waiting years for this 😮

src/main_menu.cpp Outdated Show resolved Hide resolved
Co-authored-by: leemuar <leemuar@gmail.com>
@Night-Pryanik
Copy link
Contributor

Should this resolve #53699?

@dseguin
Copy link
Member Author

dseguin commented Jun 1, 2022

Should this resolve #53699?

Nope, this PR doesn't affect the character creation menu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Tooling Tooling that is not part of the main game but is part of the repo. Controls / Input Keyboard, mouse, keybindings, input UI, etc. Info / User Interface Game - player communication, menus, etc. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions SDL: Tiles / Sound Tiles visual interface and sounds.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants