bugfix(lan): Fix crash when changing settings in the LAN lobby

[Caball009]

It's currently possible to crash the game with a well-timed option change in the LAN lobby / options menu. The game calls DeinitLanGameGadgets, which sets a few button pointers to nullptr, when the countdown is somewhere between 1 and game start. If the options are changed after this (but before the game the actually starts), the game crashes because it attempts to dereference the pointers.

This PR adds a few checks in LanGameOptionsMenuSystem to ignore such changes. This PR disables the LAN buttons early, before they're deinitialized, to prevent the host from using them at the last moment.

Callstack for DeinitLanGameGadgets:

generalszh.exe!DeinitLanGameGadgets()
generalszh.exe!shutdownComplete(WindowLayout * layout)
generalszh.exe!LanGameOptionsMenuShutdown(WindowLayout * layout, void * userData)
generalszh.exe!WindowLayout::runShutdown(void * userData)
generalszh.exe!Shell::hideShell()
generalszh.exe!GameLogic::prepareNewGame(GameMode gameMode, GameDifficulty diff, int rankPoints)
generalszh.exe!GameLogic::logicMessageDispatcher(GameMessage * msg, void * userData)
generalszh.exe!GameLogic::processCommandList(CommandList * list)
generalszh.exe!GameLogic::update()
generalszh.exe!SubsystemInterface::UPDATE()
generalszh.exe!GameEngine::update()
generalszh.exe!Win32GameEngine::update()
generalszh.exe!GameEngine::execute()
generalszh.exe!GameMain()
generalszh.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow)

Callstack for crash (in this case changing the super weapon limit):

generalszh.exe!GadgetCheckBoxIsChecked(GameWindow * g)
generalszh.exe!handleLimitSuperweaponsClick()
generalszh.exe!LanGameOptionsMenuSystem(GameWindow * window, unsigned int msg, unsigned int mData1, unsigned int mData2)
generalszh.exe!GameWindowManager::winSendSystemMsg(GameWindow * window, unsigned int msg, unsigned int mData1, unsigned int mData2)
generalszh.exe!PassMessagesToParentSystem(GameWindow * window, unsigned int msg, unsigned int mData1, unsigned int mData2)
generalszh.exe!GameWindowManager::winSendSystemMsg(GameWindow * window, unsigned int msg, unsigned int mData1, unsigned int mData2)
generalszh.exe!GadgetCheckBoxInput(GameWindow * window, unsigned int msg, unsigned int mData1, unsigned int mData2)
generalszh.exe!GameWindowManager::winSendInputMsg(GameWindow * window, unsigned int msg, unsigned int mData1, unsigned int mData2)
generalszh.exe!GameWindowManager::winProcessMouseEvent(GameWindowMessage msg, ICoord2D * mousePos, void * data)
generalszh.exe!WindowTranslator::translateGameMessage(const GameMessage * msg)
generalszh.exe!MessageStream::propagateMessages()
generalszh.exe!GameEngine::update()
generalszh.exe!Win32GameEngine::update()
generalszh.exe!GameEngine::execute()
generalszh.exe!GameMain()
generalszh.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow)

TODO:

• Replicate in Generals.

[greptile-apps]

Greptile Summary

This PR fixes a rare crash in the LAN lobby by disabling all UI controls (buttons, combo boxes, checkboxes) exactly one second before the game starts, preventing any player input from reaching callbacks that dereference pointers that are about to be nulled by DeinitLanGameGadgets.

• Adds LANDisableButtons() in LanGameOptionsMenu.cpp that disables every interactive control across all MAX_SLOTS entries, and declares it in LANGameInfo.h.
• Hooks the call into LANAPI::update() at the m_gameStartSeconds == 1 threshold — one full countdown tick before RequestGameStart() is dispatched and the shutdown sequence that calls DeinitLanGameGadgets is triggered.

Confidence Score: 4/5

Safe to merge with one small hardening change recommended — the new function works correctly for the crash it targets but is fragile against unexpected teardown orderings.

The timing fix is logically sound: disabling controls one second before RequestGameStart is dispatched ensures the countdown gap is covered. The one concern is that LANDisableButtons dereferences all GUI pointers unconditionally, and those same pointers are nulled by DeinitLanGameGadgets. The function is now called from the network update loop, which does not control GUI lifetime. If any non-standard teardown path calls DeinitLanGameGadgets while the countdown is still at 1 second, the fix itself crashes. Adding a single early-return null guard would eliminate that risk entirely.

The body of LANDisableButtons() in LanGameOptionsMenu.cpp — specifically the missing null guard at the top of the function before the first pointer dereference.

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/LanGameOptionsMenu.cpp	Adds LANDisableButtons() which disables all GUI controls; no null guards on any pointer dereference, called cross-layer from the network update loop
Core/GameEngine/Source/GameNetwork/LANAPI.cpp	Calls LANDisableButtons() when m_gameStartSeconds == 1; timing is correct — one full tick before RequestGameStart() is sent and DeinitLanGameGadgets is triggered
Core/GameEngine/Include/GameNetwork/LANGameInfo.h	Adds the LANDisableButtons() forward declaration alongside the existing LANEnableStartButton declaration; straightforward and correct

Sequence Diagram

sequenceDiagram
    participant U as User Input
    participant LAN as LANAPI::update()
    participant GUI as LanGameOptionsMenu
    participant GL as GameLogic

    Note over LAN: m_gameStartSeconds = 3..2
    LAN->>LAN: RequestGameStartTimer(N)
    Note over LAN: m_gameStartSeconds = 1
    LAN->>GUI: LANDisableButtons()
    GUI-->>U: All controls disabled
    LAN->>LAN: RequestGameStartTimer(1)
    Note over LAN: m_gameStartSeconds = 0
    LAN->>LAN: ResetGameStartTimer()
    LAN->>GL: RequestGameStart()
    GL->>GL: prepareNewGame()
    GL->>GL: Shell::hideShell()
    GL->>GUI: DeinitLanGameGadgets() — sets all ptrs to nullptr
    U--xGUI: Input blocked (buttons disabled)

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/LanGameOptionsMenu.cpp:392-409
`LANDisableButtons()` dereferences every GUI pointer without a null check, yet it is now called from the network-layer update loop (`LANAPI::update`) rather than from within the GUI layer like the rest of this file. `DeinitLanGameGadgets` nulls all of these same pointers. If any code path causes `DeinitLanGameGadgets` to run while `m_gameStartSeconds` is still 1 (e.g., an early teardown or a cancelled game that doesn't also call `ResetGameStartTimer`), the next `LANAPI::update` tick will crash on `buttonStart->winEnable(false)`. Adding a simple guard matching the pattern used throughout this file makes the function safe across all teardown orderings.

```suggestion
void LANDisableButtons()
{
	if (!buttonStart)
		return;

	buttonStart->winEnable(false);
	buttonBack->winEnable(false);
	buttonSelectMap->winEnable(false);
	checkboxLimitSuperweapons->winEnable(false);
	comboBoxStartingCash->winEnable(false);

	for (Int i = 0; i < MAX_SLOTS; ++i)
	{
		comboBoxPlayer[i]->winEnable(false);
		comboBoxColor[i]->winEnable(false);
		comboBoxPlayerTemplate[i]->winEnable(false);
		comboBoxTeam[i]->winEnable(false);
		buttonAccept[i]->winEnable(false);
		buttonMapStartPosition[i]->winEnable(false);
	}
}
```

_{Reviews (3): Last reviewed commit: "Tweaked comment." | Re-trigger Greptile}

[xezon]

It is very suspicious that the menu can still be interacted with after it was shutdown. Why is this possible? Is there maybe a general issue with menus?

[Caball009]

I don't think this is because of a general issues with menus. The game just nulls the button pointers before it changes the window / takes away control from the user.

Perhaps from user perspective the cleanest solution would be to disable (grey out) the buttons when the timer has zero seconds remaining. I don't know how to access the buttons in the code that has access to the timer, though.

[xezon]

It would be good if the window is changed or control is taken before the button pointers are nulled. That way no special safe guards need to be placed in multiple handlers.

Is the issue maybe that the handling of a button press is delayed by 1 or multiple frames because of input system updates, and so it can arrive after the menu screen was destroyed? If so, maybe the solution here is to add a condition into every menu input handler and check that the owning menu is still active, and if not, return early and do not handle the input.

[Caball009]

I changed the implementation to disable the buttons when the countdown is at one second remaining.