fix(skirmish): Improve determinism for restarted games by resetting slot values

[Caball009]

Restarted skirmish games may not start with the same logical seed value as the first start. This depends on whether there are players with a random color / position / faction. Those random values are determined by using and updating the logical seed on the first start, after which the random values are stored. That means that for restarted games the logical seed isn't used or updated for those purposes. This PR resets those values to improve determinism for restarted games.

You can put a breakpoint after GameLogic::startNewGame and compare the values of theGameLogicSeed for the first start and following starts and see how the values for the first start deviate from restarts.

TODO:

• Address feedback.
• Replicate in Generals.

[greptile-apps]

Greptile Summary

This PR improves determinism for restarted skirmish games by preventing the logical seed from drifting between the first start and subsequent restarts. It does this by tracking (via m_saveOffOriginalInfo) whether a slot's values have already been snapshotted, and on a restart, restoring the pre-random original values before populateRandomSideAndColor / populateRandomStartPosition run — ensuring those functions consume the same seed values as the first start.

Key changes:

• GameSlot::saveOffOriginalInfo() now returns Bool and is guarded by a new m_saveOffOriginalInfo flag; it only saves once until the slot state is explicitly set again via setState() or reset().
• GameSlot::reset() and the non-player branch of GameSlot::setState() both initialize m_saveOffOriginalInfo = TRUE, so a fresh lobby visit correctly re-enables snapshotting.
• GameLogic::startNewGame() uses the Bool return value to detect a restart and restores the slot's original color, start position, and player template before the random-population passes run.
• A toString(GameMode) helper is added and used in a DEBUG_ASSERTCRASH to surface any unexpected non-skirmish restart paths during development.
• Note: The fix is currently only applied to Zero Hour (GeneralsMD/). The Core/GameEngine changes are shared, but the corresponding GameLogic.cpp in Generals/ still needs the same call-site update (tracked in the PR's TODO).

Confidence Score: 4/5

• Safe to merge for Zero Hour; one outstanding TODO (Generals replication) and the open feedback checklist keep this at 4.
• The fix is logically sound and well-scoped. The flag lifecycle (set in reset() and the non-player setState() branch, cleared after first snapshot) correctly covers all slot-state transitions. No new crashes or data-loss risk are introduced. The only blockers are the acknowledged TODO to replicate the GameLogic.cpp call-site change in the vanilla Generals codebase, and the author's own "Address feedback" checklist item.
• No files require special attention; reviewers may want to verify the Generals-side GameLogic.cpp also gets the matching call-site update.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameNetwork/GameInfo.h	Adds m_saveOffOriginalInfo Bool member and changes saveOffOriginalInfo() return type from void to Bool; clean, well-placed change.
Core/GameEngine/Source/GameNetwork/GameInfo.cpp	Implements the Bool guard in saveOffOriginalInfo() and resets the flag in setState() (non-player branch) and reset(); one subtle concern that SLOT_PLAYER path already resets via reset() but the non-player branch now also resets, covering AI/open/closed slots between games correctly.
GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h	Adds free-function declaration const char* toString(GameMode mode) used for the new debug assert message.
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Adds toString(GameMode) helper, calls saveOffOriginalInfo() and uses its Bool return to conditionally restore original slot values on restart; DEBUG_ASSERTCRASH guards against unexpected non-skirmish restarts but the restore logic executes unconditionally in release builds.

Sequence Diagram

sequenceDiagram
    participant UI as Game Lobby
    participant GS as GameSlot
    participant GL as GameLogic

    Note over UI,GL: First game start
    UI->>GS: setState(SLOT_EASY_AI / SLOT_PLAYER)
    GS->>GS: reset() → m_saveOffOriginalInfo = TRUE
    GL->>GS: startNewGame() → saveOffOriginalInfo()
    GS->>GS: save color/startPos/playerTemplate → orig*
    GS->>GS: m_saveOffOriginalInfo = FALSE
    GS-->>GL: returns TRUE
    GL->>GL: populateRandomSideAndColor() (consumes logical seed)

    Note over UI,GL: Restarted game (no lobby, m_saveOffOriginalInfo stays FALSE)
    GL->>GS: startNewGame() → saveOffOriginalInfo()
    GS-->>GL: returns FALSE (already saved)
    GL->>GS: setColor(getOriginalColor())
    GL->>GS: setStartPos(getOriginalStartPos())
    GL->>GS: setPlayerTemplate(getOriginalPlayerTemplate())
    GL->>GL: populateRandomSideAndColor() (same seed → same values)

    Note over UI,GL: If lobby is visited between games
    UI->>GS: setState(SLOT_EASY_AI / SLOT_PLAYER)
    GS->>GS: m_saveOffOriginalInfo = TRUE (reset)
    GL->>GS: startNewGame() → saveOffOriginalInfo()
    GS-->>GL: returns TRUE (fresh save)

Loading

_{Reviews (3): Last reviewed commit: "Set 'm_saveOffOriginalInfo' back to true..." | Re-trigger Greptile}

[xezon]

OK

Needs to be replicated in Generals

[xezon]

Greptile is still complaining about something?

[Caball009]

Greptile is still complaining about something?

Yes, that was a good point. I think it could have affected the randomization of AI players if a game was loaded from inside another game or replay.

GeneralsGameCode/Core/GameEngine/Source/GameNetwork/GameInfo.cpp

Lines 1594 to 1600 in 637dbba

	m_slot[slot]->setState((SlotState)state, name);
	if (isAccepted) m_slot[slot]->setAccept();
	m_slot[slot]->setPlayerTemplate(origPlayerTemplate);
	m_slot[slot]->setStartPos(origStartPos);
	m_slot[slot]->setColor(origColor);
	m_slot[slot]->saveOffOriginalInfo();

It should work correctly now because GameSlot::setState sets m_saveOffOriginalInfo back to true before GameSlot::saveOffOriginalInfo is called.

---

Replication in Generals still needs doing.

[xezon]

The reason for this is unintuitive.

Perhaps it would be better to have 2 versions of saveOffOriginalInfo, one used in the xfer and the other on new game start? Or just test the boolean outside of saveOffOriginalInfo.

That may also be a good opportunity to fix the name of saveOffOriginalInfo, for example saveOriginalSetup.