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:

• Replicate in Generals.

[greptile-apps]

Greptile Summary

This PR improves determinism for restarted skirmish games by ensuring that randomly-assigned color, start position, and faction values are reset to their original (first-start) values before populateRandomSideAndColor and populateRandomStartPosition are called again. Without this fix, the logical seed consumed during the first random assignment is absent from subsequent restarts, producing divergent seed states.

The implementation introduces a one-shot flag (m_saveOffOriginalInfo) on GameSlot that allows saveOffOriginalInfo() to distinguish a first start (saves values, returns TRUE) from a restart (skips the save, returns FALSE). The GameLogic::startNewGame caller uses the return value to restore original slot values on restart before randomization runs again. The overall approach is clean and well-scoped to the skirmish use case.

• GameSlot::saveOffOriginalInfo() changed from void to Bool to signal first-start vs restart
• New Bool m_saveOffOriginalInfo flag initialized to TRUE in GameSlot::reset() and set to FALSE after the first save
• On restart (saveOffOriginalInfo() returns FALSE), original color/startPos/playerTemplate are restored before the random population calls
• DEBUG_ASSERTCRASH added to flag unexpected non-skirmish restarts hitting this path
• The DEBUG_ASSERTCRASH and restore logic execute for all MAX_SLOTS slots in the loop, including unoccupied ones; guarding with isOccupied() would reduce noise in debug sessions

Confidence Score: 4/5

• Safe to merge; logic is correct and the only concern is minor debug noise from the per-slot assert on unoccupied slots.
• The determinism fix is logically sound: the flag correctly distinguishes first-start from restart, original values are restored before randomization, and the ordering of setStartPos/setPlayerTemplate is safe given the PLAYERTEMPLATE_MIN boundary. The one open item (TODO: replicate in Generals) is acknowledged by the author. The single style note about the assert firing on all MAX_SLOTS — including closed slots — doesn't affect correctness.
• No files require special attention; GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp has a minor debug-noise issue worth considering.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/GameNetwork/GameInfo.h	Adds Bool m_saveOffOriginalInfo member to GameSlot and changes saveOffOriginalInfo() return type from void to Bool; changes are minimal and correct.
Core/GameEngine/Source/GameNetwork/GameInfo.cpp	Implements the gate flag: initializes m_saveOffOriginalInfo = TRUE in reset(), saves original values only on the first call, returns TRUE/FALSE to distinguish first-start vs restart; logic is sound.
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Uses the new return value to restore original (pre-randomization) slot values on game restart, with a debug assert to flag unexpected non-skirmish restarts; see note about assert firing per-slot in a loop.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[startNewGame called] --> B{loadingSaveGame?}
    B -- Yes --> G[Skip slot info logic]
    B -- No --> C[Loop over all slots]
    C --> D{slot->saveOffOriginalInfo\nreturn value?}
    D -- TRUE\nfirst start\nm_saveOffOriginalInfo was TRUE --> E[Original values saved\ncolor/startPos/playerTemplate\nm_saveOffOriginalInfo = FALSE]
    D -- FALSE\nrestart\nm_saveOffOriginalInfo was FALSE --> F[DEBUG_ASSERTCRASH if not GAME_SKIRMISH\nRestore slot to original values\nsetColor / setStartPos / setPlayerTemplate]
    E --> H[populateRandomSideAndColor]
    F --> H
    G --> H
    H --> I[populateRandomStartPosition]
    I --> J[Continue game start...]

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
Line: 1246-1253

Comment:
**Assert fires for every slot, including unoccupied ones**

The outer loop iterates all `MAX_SLOTS` slots unconditionally, so on a skirmish restart the `DEBUG_ASSERTCRASH` (and the restore logic below it) will execute for *every* slot — including `SLOT_CLOSED` / `SLOT_OPEN` slots — rather than only for occupied ones. For empty slots the restore is harmless (values are already `-1`), but the repeated assert is noisy in debug sessions. Consider guarding this block with an occupancy check:

```suggestion
				{
					DEBUG_ASSERTCRASH(m_gameMode == GAME_SKIRMISH, ("Expected skirmish game mode but got %d", m_gameMode));

					if (slot->isOccupied())
					{
						// TheSuperHackers @fix Caball009 19/03/2026 Random color, position and faction are based on the logical seed. For improved determinism, 
						// restarted games should set the original values so that the games start with the exact same logical seed values as the first time.
						slot->setColor(slot->getOriginalColor());
						slot->setStartPos(slot->getOriginalStartPos());
						slot->setPlayerTemplate(slot->getOriginalPlayerTemplate());
					}
```

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

_{Last reviewed commit: "Updated comment date..."}

[xezon]

OK

Needs to be replicated in Generals

[xezon]

Consider adding or using a function to grab strings for game mode.

[xezon]

What does this mean? saveOffOriginalInfo?

[xezon]

should = now