Skip to content

[GEN][ZH] Refactor implemention for Shell recreation on Display Resolution change#1031

Merged
xezon merged 11 commits intoTheSuperHackers:mainfrom
xezon:xezon/fix-optionsmenu-close-on-resolution-change
Jul 1, 2025
Merged

[GEN][ZH] Refactor implemention for Shell recreation on Display Resolution change#1031
xezon merged 11 commits intoTheSuperHackers:mainfrom
xezon:xezon/fix-optionsmenu-close-on-resolution-change

Conversation

@xezon
Copy link
Copy Markdown

@xezon xezon commented Jun 8, 2025

With this change the Options Menu no longer closes when the Display Resolution is changed.

This change refactors the way the Shell recreation is implemented. It now recreates the window stack with the previous state, instead of recreating it manually by pushing the "Menus/MainMenu.wnd".

The initial motivation here was to try make the Display Resolution change also work in-game, but that will require a lot more work on top of this. So this change only makes improvements towards changing the resolution in the Main Menu as per the original restrictions.

User facing nothing changes.

TODO

  • Replicate in Generals

@xezon xezon added Bug Something is not working right, typically is user facing GUI For graphical user interface Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour labels Jun 8, 2025
@Polypheides
Copy link
Copy Markdown

Polypheides commented Jun 8, 2025

I'll try to ask this question more clearly.

By default, pressing the ACCEPT button closes the options menu. Example video:
Accept closes options

With this change, the ACCEPT button now keeps the options menu open. Example video:
Accept leave options open

Given this new behavior, would it make sense to rename the ACCEPT and CANCEL buttons to APPLY and CLOSE?

@xezon
Copy link
Copy Markdown
Author

xezon commented Jun 8, 2025

ACCEPT does not close the Options Menu.

Comment thread GeneralsMD/Code/GameEngine/Include/GameClient/GameWindowTransitions.h Outdated
Copy link
Copy Markdown

@Caball009 Caball009 Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this was also in the original code, but is there a way to reset TheShell instead of delete and new?

Shell has a reset function, does that do the equivalent? It seems like it does:

Reset the shell system to a clean state just as though init had just been called and ready to re-use

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested that. It works.

I think this then also means we can move most of the code of shell::recreateShell into a new function in Shell.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wrong. When I tested I did not update generalszh.exe. The Shell::reset function does not do what we need and using it breaks the menus on resolution change. I think it was meant to be used for non resolution change reset.

I did a refactor anyway and moved some of the code into the Shell class. The new function will now reconstruct the Shell from within. I also improved the conditioning for the main menu button transition, to only trigger it if the Main Menu is actually in the Screen stack and not hidden (which is always the case in the Menu, but would be relevant if we wanted to recreate the UI in-game, where the Main Menu is hidden).

@xezon xezon force-pushed the xezon/fix-optionsmenu-close-on-resolution-change branch from 2b8b0bb to e9f2ede Compare June 10, 2025 18:07
@xezon xezon changed the title [GEN][ZH] No longer close the Options Menu when changing Display Resolution [GEN][ZH] Refactor implemention for Shell recreation on Display Resolution change Jun 11, 2025
@xezon xezon added Refactor Edits the code with insignificant behavior changes, is never user facing and removed Bug Something is not working right, typically is user facing labels Jun 11, 2025
@xezon
Copy link
Copy Markdown
Author

xezon commented Jun 11, 2025

By default, pressing the ACCEPT button closes the options menu.

Right. I was totally confused here with what I was doing. This change was bogus.

I have now thrown out all the junk that changed the Options Menu ACCEPT button behaviour. It now functions the same way as it did originally. It now is a Refactor towards making the Resolution change work in-game in a future change.

This change should be correct now.

Copy link
Copy Markdown

@Skyaero42 Skyaero42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this PR wants to be.

  • It fixes spelling errors in comments, removes tabs and add/removes blank lines in some locations, but not in others.
  • It adds the const keyword to some functions but not others.
  • It refactors the construction and deconstruction
  • It add new functionality through recreateWindowLayouts.

Comment thread GeneralsMD/Code/GameEngine/Include/GameClient/GameWindowManager.h Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many other missing or multiple line breaks and random tabs. Why not fix those?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I reverted all now unnecessary changes to this file.

@xezon xezon requested a review from a team June 13, 2025 10:19
@xezon
Copy link
Copy Markdown
Author

xezon commented Jun 15, 2025

  • It fixes spelling errors in comments, removes tabs and add/removes blank lines in some locations, but not in others.

These were remnants of earlier changes. I reverted this now.

  • It adds the const keyword to some functions but not others.

This is sort of required as part of this refactor.

void Shell::recreateWindowLayouts( void )
{
...
  const WindowLayout* layout = getScreenLayout(screenIndex);
  ScreenInfo& screenInfo = screenStackInfos[screenIndex];
  screenInfo.filename = layout->getFilename(); // this would fail to compile if the function was not const
  screenInfo.isHidden = layout->isHidden(); // this would fail to compile if the function was not const
  • It refactors the construction and deconstruction

This is required as part of this refactor. These are called in constructor + destructor, and also in recreateWindowLayouts.

  • It add new functionality through recreateWindowLayouts.

This is required as part of this refactor.

@xezon xezon requested a review from Skyaero42 June 15, 2025 10:59
@xezon xezon force-pushed the xezon/fix-optionsmenu-close-on-resolution-change branch from c527755 to 373a9f3 Compare June 19, 2025 09:13
@xezon
Copy link
Copy Markdown
Author

xezon commented Jun 19, 2025

Rebased.

@xezon
Copy link
Copy Markdown
Author

xezon commented Jun 25, 2025

Needs review.

@xezon xezon requested a review from Caball009 June 28, 2025 18:16
Copy link
Copy Markdown

@Skyaero42 Skyaero42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@xezon xezon force-pushed the xezon/fix-optionsmenu-close-on-resolution-change branch from 373a9f3 to 00377f6 Compare July 1, 2025 17:46
@xezon
Copy link
Copy Markdown
Author

xezon commented Jul 1, 2025

Rebased without conflicts.

Replicated in Generals with 1 mini conflict.

$ git diff 1ebce079804c879126b540baffccd6ae910ec106..08800291a3f426723d1020982d3bfda0d951ceca > changes.patch

$ git apply -p2 --directory=Generals --reject --whitespace=fix changes.patch
changes.patch:6: trailing whitespace.
        virtual void update( void );
Checking patch Generals/Code/GameEngine/Include/GameClient/Shell.h...
Checking patch Generals/Code/GameEngine/Include/GameClient/WindowLayout.h...
Checking patch Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/MainMenu.cpp...
Hunk #1 succeeded at 724 (offset -37 lines).
Checking patch Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp...
Hunk #1 succeeded at 1241 (offset -60 lines).
Checking patch Generals/Code/GameEngine/Source/GameClient/GUI/Shell/Shell.cpp...
Hunk #1 succeeded at 55 (offset -2 lines).
Hunk #2 succeeded at 93 (offset -2 lines).
Hunk #3 succeeded at 112 (offset -2 lines).
Hunk #4 succeeded at 144 (offset -2 lines).
Hunk #5 succeeded at 173 (offset -2 lines).
Hunk #6 succeeded at 225 (offset -2 lines).
Hunk #7 succeeded at 297 (offset -2 lines).
error: while searching for:
    Profile::StopRange("init");
#endif
        //else
                TheShell->push( AsciiString("Menus/MainMenu.wnd") );
  }
        m_isShellActive = TRUE;
}  // end showShell

error: patch failed: Generals/Code/GameEngine/Source/GameClient/GUI/Shell/Shell.cpp:464
Applied patch Generals/Code/GameEngine/Include/GameClient/Shell.h cleanly.
Applied patch Generals/Code/GameEngine/Include/GameClient/WindowLayout.h cleanly.
Applied patch Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/MainMenu.cpp cleanly.
Applied patch Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp cleanly.
Applying patch Generals/Code/GameEngine/Source/GameClient/GUI/Shell/Shell.cpp with 1 reject...
Hunk #1 applied cleanly.
Hunk #2 applied cleanly.
Hunk #3 applied cleanly.
Hunk #4 applied cleanly.
Hunk #5 applied cleanly.
Hunk #6 applied cleanly.
Hunk #7 applied cleanly.
Rejected hunk #8.

@xezon xezon merged commit 3bda27a into TheSuperHackers:main Jul 1, 2025
14 checks passed
@xezon xezon deleted the xezon/fix-optionsmenu-close-on-resolution-change branch July 1, 2025 18:20
fbraz3 pushed a commit to fbraz3/GeneralsX that referenced this pull request Nov 10, 2025
fbraz3 pushed a commit to fbraz3/GeneralsX that referenced this pull request Feb 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gen Relates to Generals GUI For graphical user interface Minor Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants