tweak: Allow exit during match outcome#2074
Conversation
Stubbjax
added
Enhancement
|
I think it was always possible to move the cursor (even if it doesn't show) to the button above the idle worker button and use that to bring up the quit menu. Much better to have ESC working at all times, though. |
I swear that has worked for me before too, but I haven't been able to reproduce it. |
| { | ||
|
|
||
| TheMouse->setCursor( Mouse::ARROW ); | ||
| TheMouse->setVisibility(true); |
There was a problem hiding this comment.
Instead of doing that, perhaps we should not hide mouse in victory screen?
Because now when toggling Quit Menu, then Mouse will be visible anyway, right?
There was a problem hiding this comment.
The cursor is hidden as part of the ScriptActions::doDisableInput method. It's preferable not to touch this as it may be used in other pathways (such as via script commands or during cinematic playback).
While we could simply re-enable the mouse after doDisableInput is called, it does not feel as clean. It makes sense to enforce cursor visibility when showing the quit menu to remove the responsibility from other call sites.
Additionally, as the control bar is also disabled on match outcome, it makes sense to leave the cursor hidden to further communicate to the user that they cannot access it. Pressing ESC will of course reveal the cursor, but by then the user will no longer have any reason to access the control bar.
We can create a follow-up task if we want to try and figure a way to have only the options button on the control bar available during the match outcome and leave the cursor visible. I expect such a task will be a lot more involved.
There was a problem hiding this comment.
If I cancel the QuitMenu (using Esc key), the mouse stays visible. Shouldn't it be hidden again?
There was a problem hiding this comment.
Either that or never hidden at all I would say.
If mouse visibility is difficult to control with just a single boolean flag, then perhaps it requires flags similar to how we do it for the cursor capture (see CursorCaptureBlockReason).
The core principle of using the flags approach is that multiple systems can ask for hiding a cursor but do not conflict with each other.
|
If this also affects online play, I would double check that this does not affect the match outcome handling. |
|
What is the plan on this one? |
|
I think we need to check with GeneralsOnline to see if they can adjust their match outcome handling as Mauller said. If that is too difficult, we may want to hold off on this PR. |
Stubbjax
force-pushed
the
allow-exit-during-match-outcome
branch
from
b9af56a to
40bd7fc
Compare
I've been procrastinating on it because the suggested refactor will likely require a far more intricate solution. If we go with a flags approach, do we apply it just for cursor visibility, or input too? Should such flag behaviour be made into a standalone/generic module? Do we also replace the existing For now I've adjusted it so that the cursor will hide itself again when closing the quit menu during the match outcome. Is this acceptable?
I've already looked into this and have a pending solution! |
Greptile Overview
|
| Filename | Overview |
|---|---|
| Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/QuitMenu.cpp | Allows quit menu during game-ending flow; adds mouse visibility toggles and brings quit menu forward when shown. |
| GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/QuitMenu.cpp | Same quit-menu visibility changes as Generals variant for Zero Hour; removes game-ending guard and adjusts mouse visibility when toggling. |
Sequence Diagram
sequenceDiagram
participant User
participant Input as Input/ESC handler
participant QuitMenu as ToggleQuitMenu()
participant UI as WindowLayout/TransitionHandler
participant Mouse as TheMouse
participant Script as TheScriptEngine
User->>Input: Press ESC
Input->>QuitMenu: ToggleQuitMenu()
alt Intro movie or loading
QuitMenu-->>Input: return
else Quit menu not visible
QuitMenu->>UI: Create/select quitMenuLayout
QuitMenu->>UI: setGroup("QuitFull"/"QuitNoSave")
QuitMenu->>UI: bringForward()
QuitMenu->>Mouse: setVisibility(true)
QuitMenu-->>Input: isVisible = TRUE
else Quit menu visible
QuitMenu->>UI: reverse("QuitFullBack"/"QuitNoSaveBack")
QuitMenu->>Script: isGameEnding()
alt game ending
QuitMenu->>Mouse: setVisibility(false)
end
QuitMenu-->>Input: isVisible = FALSE
end
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| HideDiplomacy(); | ||
| HideInGameChat(); | ||
| TheControlBar->hidePurchaseScience(); | ||
| quitMenuLayout->bringForward(); | ||
| TheMouse->setVisibility(true); | ||
| isVisible = TRUE; |
There was a problem hiding this comment.
Mouse cursor forced visible
ToggleQuitMenu() now unconditionally calls TheMouse->setVisibility(true) when opening the quit menu. If this path is used during the match outcome/game-ending flow (the stated goal), it overrides whatever cursor state the outcome screen expects and may leave the cursor visible when it shouldn’t be until the later close path runs. Consider scoping cursor visibility changes to the game-ending case only (or restoring previous visibility state) so opening the quit menu doesn’t permanently change cursor visibility for non-ending gameplay states.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/QuitMenu.cpp
Line: 434:439
Comment:
**Mouse cursor forced visible**
`ToggleQuitMenu()` now unconditionally calls `TheMouse->setVisibility(true)` when opening the quit menu. If this path is used during the match outcome/game-ending flow (the stated goal), it overrides whatever cursor state the outcome screen expects and may leave the cursor visible when it shouldn’t be until the later close path runs. Consider scoping cursor visibility changes to the game-ending case only (or restoring previous visibility state) so opening the quit menu doesn’t permanently change cursor visibility for non-ending gameplay states.
How can I resolve this? If you propose a fix, please make it concise.| HideDiplomacy(); | ||
| HideInGameChat(); | ||
| TheControlBar->hidePurchaseScience(); | ||
| quitMenuLayout->bringForward(); | ||
| TheMouse->setVisibility(true); | ||
| isVisible = TRUE; |
There was a problem hiding this comment.
Mouse cursor forced visible
ToggleQuitMenu() now unconditionally calls TheMouse->setVisibility(true) when opening the quit menu. If this path is used during the match outcome/game-ending flow, it overrides whatever cursor state the outcome screen expects and may leave the cursor visible when it shouldn’t be until the later close path runs. Consider scoping cursor visibility changes to the game-ending case only (or restoring previous visibility state) so opening the quit menu doesn’t permanently change cursor visibility for non-ending gameplay states.
Prompt To Fix With AI
This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/QuitMenu.cpp
Line: 434:439
Comment:
**Mouse cursor forced visible**
`ToggleQuitMenu()` now unconditionally calls `TheMouse->setVisibility(true)` when opening the quit menu. If this path is used during the match outcome/game-ending flow, it overrides whatever cursor state the outcome screen expects and may leave the cursor visible when it shouldn’t be until the later close path runs. Consider scoping cursor visibility changes to the game-ending case only (or restoring previous visibility state) so opening the quit menu doesn’t permanently change cursor visibility for non-ending gameplay states.
How can I resolve this? If you propose a fix, please make it concise.5 participants
This change allows players to bring up the quit menu and exit the game during the match outcome screen via the ESC key. Players are no longer forced to wait for the duration of the match outcome before they can exit the game.
QUIT_MENU.mp4