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

GUI: restore the Pause Menu after closing the Settings Menu #15891

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SmallJoker
Copy link
Member

Because it is more intuitive this way.

When closing the in-game Settings menu, the Pause menu will be shown again.
The GUIModalMenu code is responsible for handling overlaid formspec "layers". C++ GUI implementations (e.g. GUIPasswordChange) do support this well. The Lua side of the Main Menu has its own layer code, which works the same way. However, when entering the Settings menu (Lua formspec) though the Pause menu (C++), there is no way back.
This PR fixes that by allowing to overlay any kind of showPauseMenuFormSpec formspecs (until they get closed explicitly).

To do

This PR is Ready for Review.

How to test

  1. Open the in-game Settings menu
  2. Open the key change menu
  3. Close the key change menu
  4. You must see the Settings menu (already works in master because the C++ GUIKeyChangeMenu does not depend on m_formspec)
  5. Close the Settings menu
  6. You must see the Pause menu (new since this PR)

Also open and close some random formspecs to ensure it's not causing side-effects.

@grorp grorp self-requested a review March 13, 2025 17:17
@grorp
Copy link
Member

grorp commented Mar 22, 2025

When I saw this PR, I was initially going to comment something along the lines of

I don't think closing the settings menu should get you back to the "main" pause menu. This would make the settings menu inconsistent with the other sub-menus implemented in C++.

Also, it would introduce an extra click/tap/keypress/swipe for users who want to go back to their game. There is rarely a need to go back to the "main" pause menu after adjusting settings (you'd have to happen to want e.g. to change your server password and your settings at exactly the same time).

However, I now found out that, as implied in the PR description...

The GUIModalMenu code is responsible for handling overlaid formspec "layers". C++ GUI implementations (e.g. GUIPasswordChange) do support this well.

... closing C++ sub-menus actually gets you back to the "main" pause menu on 5.12.0-dev. This is actually a recent regression caused by 2c50066:

The button_exit[]s were replaced by regular button[]s, to avoid a very short unpause when you click the btn_settings (probably because it uses ClientEvent stuff).

This can easily be fixed by only applying that workaround to the settings button, where it is actually necessary, and reverting it for the other buttons.

@SmallJoker
Copy link
Member Author

This can easily be fixed by only applying that workaround to the settings button, where it is actually necessary, and reverting it for the other buttons

It seems this is a subjective matter because I would prefer to always return to the pause menu before explicitly closing it. This is also done similarly in other games - difference being that these layers of menus (/formspecs) are yet not visually represented in Luanti.

@grorp
Copy link
Member

grorp commented Mar 27, 2025

Yeah, this seems like a matter of preference. I suppose either behavior is objectively fine as long as it's consistent between all the sub-menus (C++ and Lua).
On 5.11.0 and before it was still consistent. In 5.12.0-dev it's inconsistent. This PR would make it consistent again, but with the opposite behavior.

If you want to continue with this, you should also update the buttons that are currently labeled "Exit" to say "Back" instead:

("button[0,%f;%f,0.8;back;%s]"):format(
tabsize.height + 0.2, back_w,
INIT == "pause_menu" and fgettext("Exit") or fgettext("Back")),

GUIButton::addButton(Environment, rect, m_tsrc, this, ID_soundExitButton,
wstrgettext("Exit").c_str());

@grorp grorp removed their request for review March 27, 2025 00:41
@SmallJoker SmallJoker force-pushed the pr_15891_settings_exit branch from d117cca to 263024e Compare March 29, 2025 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants