Escape menu cleanup #2576
Escape menu cleanup #2576
Conversation
Used the params from Gwen.Control.Titlebar, which is the default for MainWindow : Window so it would look similar.
…instead of a func<>.
| private SettingsWindow SettingsWindow | ||
| { | ||
| _settingsWindow ??= new SettingsWindow(GameCanvas) | ||
| get | ||
| { | ||
| IsVisible = false, | ||
| }; | ||
|
|
||
| return _settingsWindow; | ||
| return _settingsWindow ??= new SettingsWindow(GameCanvas) | ||
| { | ||
| IsVisible = false, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| public GameInterface(Canvas canvas) : base(canvas) | ||
| { | ||
| GameCanvas = canvas; | ||
| EscapeMenu = new EscapeMenu(GameCanvas, GetOrCreateSettingsWindow) {IsHidden = true}; | ||
| SimplifiedEscapeMenu = new SimplifiedEscapeMenu(GameCanvas, GetOrCreateSettingsWindow) {IsHidden = true}; | ||
| EscapeMenu = new EscapeMenu(GameCanvas, SettingsWindow) {IsHidden = true}; | ||
| SimplifiedEscapeMenu = new SimplifiedEscapeMenu(GameCanvas, SettingsWindow) {IsHidden = true}; |
There was a problem hiding this comment.
tl;dr: Revert the changes in this file
SettingsWindow is a bit heavy and I intentionally wanted to defer creation until it's needed. To that end, passing a delegate was necessary and the getter, while on the surface is doing the same thing, isn't actually having the same effect.
In this case it's creating only one, but it's creating it immediately in the constructor of GameInterface. With the methods, it was only ever created when _settingsWindowProvider() was invoked.
There was a problem hiding this comment.
Also revert, since the only changes are SettingsWindow related
| private readonly SettingsWindow _settingsWindow; | ||
| private readonly Button _buttonCharacterSelect; | ||
| private readonly Panel _versionPanel; | ||
|
|
||
| public EscapeMenu(Canvas gameCanvas, Func<SettingsWindow> settingsWindowProvider) : base(gameCanvas, nameof(EscapeMenu)) | ||
| private readonly Framework.Graphics.GameFont _font; | ||
|
|
||
| public EscapeMenu(Canvas gameCanvas, SettingsWindow settingsWindow) : base(gameCanvas, nameof(EscapeMenu)) | ||
| { | ||
| _settingsWindowProvider = settingsWindowProvider; | ||
| _settingsWindow = settingsWindow; |
There was a problem hiding this comment.
We definitely want to continue using the delegate, revert the SettingsWindow changes here (but keep the font one)
There was a problem hiding this comment.
That makes sense. I hadn't considered the lazy implementation. I guess I assumed settingsmenu would be called long before the EscapeMenu would be constructed. But you're correct. If someone plays the game and exits without ever hitting settings, the delegate version saves having to load the settings menu.
…ference instead of a func<>." This reverts commit 2abebfa.
|
This is no longer required. New PR obsoletes these changes. |
This should fix the layout so it looks like this now:
Might need to make an update to the Asset repo to reflect these changes. The label edits are overridden by the EscapeMenu.json unless you completely comment out the label, compile and run, then uncomment.
Maybe I'll take a look at how the load and save json works later. Save that headache.
Fixes Issue #2546