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 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.

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 guards four message-handling cases (GCM_SELECTED, GBM_SELECTED, GBM_SELECTED_RIGHT, GEM_EDIT_DONE) in LanGameOptionsMenuSystem against the race where DeinitLanGameGadgets nullifies gadget pointers before the game actually starts, by checking isGameInProgress() and returning MSG_IGNORED early.

• The new guards call myGame->isGameInProgress() immediately after TheLAN->GetMyGame() without a null check. The existing code in the same file at line 825 uses TheLAN->GetMyGame() && TheLAN->GetMyGame()->isGameInProgress(), showing GetMyGame() can return nullptr. A null myGame would cause the fix itself to crash.

Confidence Score: 3/5

Needs a fix before merging — the guard added to prevent a crash can itself crash if GetMyGame() returns nullptr.

A single P1 finding that mirrors the exact class of bug being fixed (null dereference) pulls the score below the P1 ceiling. The rest of the refactoring (reusing the local myGame variable) is clean and correct.

LanGameOptionsMenu.cpp — all four new myGame->isGameInProgress() call sites.

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/LanGameOptionsMenu.cpp	Adds early-return guards in four message-handler cases to ignore option changes after DeinitLanGameGadgets nulls the gadget pointers; however, the new guards dereference TheLAN->GetMyGame() without a null check, inconsistent with the existing null-guarded pattern at line 825.

Sequence Diagram

sequenceDiagram
    participant UI as GUI / Input
    participant Sys as LanGameOptionsMenuSystem
    participant LAN as TheLAN
    participant Gadgets as DeinitLanGameGadgets

    Note over Gadgets: Called during countdown (1→0)
    Gadgets->>Gadgets: Sets comboBox*, button*, textEntry* → nullptr

    UI->>Sys: GCM_SELECTED / GBM_SELECTED / GBM_SELECTED_RIGHT / GEM_EDIT_DONE
    Sys->>LAN: GetMyGame()
    alt myGame == nullptr
        LAN-->>Sys: nullptr
        Sys->>Sys: *** CRASH: myGame->isGameInProgress() ***
    else myGame != nullptr
        LAN-->>Sys: LANGameInfo*
        Sys->>Sys: isGameInProgress() → true
        Sys-->>UI: MSG_IGNORED  ✓ crash prevented
    end

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:1156-1158
**Missing null check before `isGameInProgress()` call**

`TheLAN->GetMyGame()` can return `nullptr` — the existing code at line 825 of this same file already accounts for this with an explicit null guard: `if (TheLAN->GetMyGame() && TheLAN->GetMyGame()->isGameInProgress())`. All four new `myGame->isGameInProgress()` checks in this PR skip that guard, so if `GetMyGame()` ever returns `nullptr` in a message-handler context (e.g., after the LAN session is partially torn down), the fix itself will crash at the same dereference it is trying to prevent.

```suggestion
				LANGameInfo* myGame = TheLAN->GetMyGame();
				if (!myGame || myGame->isGameInProgress())
					return MSG_IGNORED;
```

_{Reviews (1): Last reviewed commit: "bugfix(lan): Fix crash when changing set..." | 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.