Skip to content

unify(logic): Merge game loading related variables and functions from Zero Hour#2444

Merged
xezon merged 1 commit intoTheSuperHackers:mainfrom
stephanmeesters:unify/gameloading-variables
Mar 13, 2026
Merged

unify(logic): Merge game loading related variables and functions from Zero Hour#2444
xezon merged 1 commit intoTheSuperHackers:mainfrom
stephanmeesters:unify/gameloading-variables

Conversation

@stephanmeesters
Copy link

@stephanmeesters stephanmeesters commented Mar 12, 2026

This unifies some variables/functions used for loading :

Removed (commented out): isLoadingGame()

Added: isLoadingMap() , isLoadingSave(), isClearingGameData()

This was done for the replicate to Generals in #2440.

Tested skirmish/campaign in Generals.

Replicated by hand using WinMerge

@greptile-apps
Copy link

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR refactors game-loading state tracking in the Generals engine by replacing the single m_loadingScene / isLoadingGame() / setGameLoading() trio with three more descriptive booleans — m_loadingMap, m_loadingSave, and m_clearingGameData — and their corresponding getters/setters. It is a hand-replicated port of the same change already applied in GeneralsMD (#2440).

Key changes:

  • GameLogic.h: Removes m_loadingScene and isLoadingGame(); adds m_loadingMap, m_loadingSave, m_clearingGameData with inline getters/setters. Old declarations are left as commented-out code.
  • GameLogic.cpp: Moves the loading-start signal from prepareNewGame() (old: setGameLoading(TRUE)) to the top of startNewGame() (setLoadingMap(TRUE)), with a small gap between prepareNewGame() returning and startNewGame() being called where no loading flag is set. This gap is mitigated by the blank background window that is up during that window, and the author acknowledges it.
  • GameLogicDispatch.cpp: clearGameData() is now bracketed with setClearingGameData(TRUE/FALSE), and setGameLoading(TRUE) in prepareNewGame() is commented out.
  • Call-site updates in Diplomacy.cpp and QuitMenu.cpp replace isLoadingGame() with isLoadingMap(), which is functionally equivalent since setLoadingMap(TRUE) is called in startNewGame() for all game types including save games.

Incomplete replication: The GeneralsMD version of GameStateMap::xfer() calls setLoadingSave(TRUE/FALSE) when xfer mode is XFER_LOAD, enabling callers to distinguish save-game loading via isLoadingSave(). The Generals GameStateMap.cpp was not updated and never calls setLoadingSave(), meaning isLoadingSave() will permanently return FALSE in Generals even during an active save-game load.

Confidence Score: 3/5

  • Mostly safe to merge, but isLoadingSave() is non-functional in Generals due to a missing setter call that exists in the GeneralsMD equivalent.
  • The call-site changes (Diplomacy, QuitMenu) and the new m_loadingMap/m_clearingGameData flags are correctly implemented and don't regress existing behaviour. However, the replication is incomplete: setLoadingSave() is never called in the Generals codebase, leaving isLoadingSave() as a permanently-false getter despite its documented contract. No current Generals code queries isLoadingSave(), so there is no immediate runtime regression, but the gap is real and could silently bite future callers.
  • Generals/Code/GameEngine/Include/GameLogic/GameLogic.h (isLoadingSave() contract) and the unmodified Generals/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp (missing setLoadingSave calls).

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Include/GameLogic/GameLogic.h Replaces m_loadingScene/isLoadingGame()/setGameLoading() with three new booleans (m_loadingMap, m_loadingSave, m_clearingGameData) and matching getters/setters. isLoadingSave() is declared and documented but setLoadingSave() is never called in the Generals codebase, so it will always return FALSE.
Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp Moves setLoadingMap(TRUE) to the top of startNewGame() (previously setGameLoading(TRUE) was in prepareNewGame()), initializes the three new booleans to FALSE in the constructor, and removes the old setGameLoading() implementation. Logic is otherwise preserved.
Generals/Code/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp Wraps clearGameData() body with setClearingGameData(TRUE/FALSE) and comments out setGameLoading(TRUE) in prepareNewGame() with an explanatory note, deferring to setLoadingMap() in startNewGame().
Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Diplomacy.cpp Simple call-site update: isLoadingGame() → isLoadingMap(). Functionally equivalent since startNewGame() sets m_loadingMap=TRUE for all game types including save games.
Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/QuitMenu.cpp Simple call-site update: isLoadingGame() → isLoadingMap(). Functionally equivalent for the same reason as Diplomacy.cpp.

Sequence Diagram

sequenceDiagram
    participant Dispatch as GameLogicDispatch
    participant Logic as GameLogic
    participant UI as UI (Diplomacy/QuitMenu)
    participant GSM as GameStateMap

    Note over Dispatch,Logic: Non-save-game path
    Dispatch->>Logic: prepareNewGame()
    Note right of Logic: m_loadingMap still FALSE here<br/>(old setGameLoading(TRUE) removed)
    Dispatch->>Logic: startNewGame(FALSE)
    Logic->>Logic: setLoadingMap(TRUE)
    Logic->>Logic: m_startNewGame=TRUE, return early
    UI->>Logic: isLoadingMap() → TRUE ✓
    Logic->>Logic: update() → startNewGame(FALSE) again
    Logic->>Logic: [full map load]
    Logic->>Logic: setLoadingMap(FALSE)

    Note over GSM,Logic: Save-game path
    GSM->>Logic: startNewGame(TRUE)
    Logic->>Logic: setLoadingMap(TRUE)
    Note right of Logic: setLoadingSave(TRUE) NOT called<br/>in Generals (missing vs GeneralsMD)
    Logic->>Logic: [full save load]
    Logic->>Logic: setLoadingMap(FALSE)
    Note right of Logic: m_loadingSave always FALSE in Generals

    Note over Dispatch,Logic: clearGameData path
    Dispatch->>Logic: clearGameData()
    Logic->>Logic: setClearingGameData(TRUE)
    Logic->>Logic: [teardown]
    Logic->>Logic: setClearingGameData(FALSE)
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: Generals/Code/GameEngine/Include/GameLogic/GameLogic.h
Line: 185

Comment:
**`isLoadingSave()` always returns FALSE in Generals**

`setLoadingSave()` is never called anywhere in the `Generals/` codebase, so `isLoadingSave()` will always return `FALSE` here — making this API misleading and effectively a dead getter.

The GeneralsMD counterpart correctly calls the setter in `GameStateMap::xfer()`:
```cpp
// GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp : 247
if( xfer->getXferMode() == XFER_LOAD )
{
    TheGameLogic->setLoadingSave( TRUE );
}
// ... later at line 441:
TheGameLogic->setLoadingSave( FALSE );
```

The equivalent `Generals/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp::xfer()` has no such calls. Since `startNewGame(TRUE)` is called from that same function, the save-game load path exists and `setLoadingSave()` should be called there too — otherwise any future caller of `isLoadingSave()` in Generals will silently get `FALSE` even during an active save-game load.

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: a0df0cc

//Kris: Cut isLoadingGame() and replaced with isLoadingMap() and isLoadingSave() -- reason: nomenclature
//Bool isLoadingGame() const { return m_loadingScene; } // This is the old function that isn't very clear on it's definition.
Bool isLoadingMap() const { return m_loadingMap; } // Whenever a map is in the process of loading.
Bool isLoadingSave() const { return m_loadingSave; } // Whenever a saved game is in the process of loading.
Copy link

Choose a reason for hiding this comment

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

isLoadingSave() always returns FALSE in Generals

setLoadingSave() is never called anywhere in the Generals/ codebase, so isLoadingSave() will always return FALSE here — making this API misleading and effectively a dead getter.

The GeneralsMD counterpart correctly calls the setter in GameStateMap::xfer():

// GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp : 247
if( xfer->getXferMode() == XFER_LOAD )
{
    TheGameLogic->setLoadingSave( TRUE );
}
// ... later at line 441:
TheGameLogic->setLoadingSave( FALSE );

The equivalent Generals/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp::xfer() has no such calls. Since startNewGame(TRUE) is called from that same function, the save-game load path exists and setLoadingSave() should be called there too — otherwise any future caller of isLoadingSave() in Generals will silently get FALSE even during an active save-game load.

Prompt To Fix With AI
This is a comment left during a code review.
Path: Generals/Code/GameEngine/Include/GameLogic/GameLogic.h
Line: 185

Comment:
**`isLoadingSave()` always returns FALSE in Generals**

`setLoadingSave()` is never called anywhere in the `Generals/` codebase, so `isLoadingSave()` will always return `FALSE` here — making this API misleading and effectively a dead getter.

The GeneralsMD counterpart correctly calls the setter in `GameStateMap::xfer()`:
```cpp
// GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp : 247
if( xfer->getXferMode() == XFER_LOAD )
{
    TheGameLogic->setLoadingSave( TRUE );
}
// ... later at line 441:
TheGameLogic->setLoadingSave( FALSE );
```

The equivalent `Generals/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp::xfer()` has no such calls. Since `startNewGame(TRUE)` is called from that same function, the save-game load path exists and `setLoadingSave()` should be called there too — otherwise any future caller of `isLoadingSave()` in Generals will silently get `FALSE` even during an active save-game load.

How can I resolve this? If you propose a fix, please make it concise.

@xezon xezon added Gen Relates to Generals Unify Unifies code between Generals and Zero Hour labels Mar 12, 2026
@xezon xezon added this to the Code foundation build up milestone Mar 12, 2026
Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

@xezon xezon changed the title unify(logic): Unify game loading variables/functions unify(logic): Merge game loading related variables and functions Mar 13, 2026
@xezon xezon changed the title unify(logic): Merge game loading related variables and functions unify(logic): Merge game loading related variables and functions from Zero Hour Mar 13, 2026
@xezon xezon merged commit c52fb6a into TheSuperHackers:main Mar 13, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gen Relates to Generals Unify Unifies code between Generals and Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants