[Caball009]

• Follow up for fix(skirmish): Improve determinism for restarted games by resetting slot values #2373

#2373 broke replays of restarted skirmish games again, making them mismatch 100% of the time. This PR fixes that.

The issue is that restoring the original slot data (color, position and faction) in GameLogic::tryStartNewGame happens too late because the RecorderClass update runs first. So for restarted games, a new replay is created with incorrect data. That's why I refactored the code, extracted it to a separate function and call it from both places.

Calling handleOriginalSetups from GameLogic::tryStartNewGame is strictly unnecessary now, but I kept the call there because it shouldn't need to rely on the code in RecorderClass to handle the player setups.

TODO:

• Replicate to Generals.

[coderabbitai]

Walkthrough

SkirmishGameInfo now restores saved original slot setups or saves current setups as originals. New skirmish game startup and replay recording call this shared handler.

Changes

Skirmish setup handling

Layer / File(s)	Summary
Define and implement setup handling Core/GameEngine/Include/GameNetwork/GameInfo.h, Core/GameEngine/Source/GameNetwork/GameInfo.cpp	SkirmishGameInfo declares and implements handleOriginalSetups(). It restores saved color, start position, and player template values, or saves the current values when no original setup exists.
Call setup handling GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp, GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp	New skirmish game startup and replay recording call handleOriginalSetups(). Startup calls it before scanning slots for AI players. Recording calls it before setting the CRC interval and serializing the slot list.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested labels: Fix, Minor, Refactor

Suggested reviewers: bobtista

Merge Risk: Low · up to 70b8e

Saved skirmish setups with duplicate colors can produce mismatching replays. Fix the ordering or accept this limited risk before merging.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly identifies the replay mismatch bug and the fix that restores slot setup before replay creation.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description states that PR #2373 broke replays of restarted skirmish games, and that restoring original slot data (color, position, faction) in GameLogic::tryStartNewGame runs too late because Rec…

Full details: Description check

Explanation

The description states that PR #2373 broke replays of restarted skirmish games, and that restoring original slot data (color, position, faction) in GameLogic::tryStartNewGame runs too late because RecorderClass updates first. It says the fix extracts this logic into a separate function called from both places. This matches the changeset, which adds SkirmishGameInfo::handleOriginalSetups() and calls it from Recorder.cpp and GameLogic.cpp. The description is specific and related to the changes.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6f7d5887-40b1-4ab3-8bac-0cf610230070

Commits

Reviewing files that changed from the base of the PR and between 68d1668 and 70b8ea6.

Files selected for processing: 4

• Core/GameEngine/Include/GameNetwork/GameInfo.h
• Core/GameEngine/Source/GameNetwork/GameInfo.cpp
• GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[coderabbitai]

Minor · Data Integrity & Integration · Quick win

Restore setups before checking duplicate colors.

If a saved Skirmish.ini slot list contains duplicate colors, the parser accepts it and the skirmish menu loads it. (raw.githubusercontent.com) The recorder saves those colors as originals. This call then restores them after checkForDuplicateColors clears one. Playback clears the duplicate without restoring it, so the game and replay use different colors and random draws.

Move this call before checkForDuplicateColors so both paths normalize the same setup.

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The changes affect skirmish setup and replay playback.

The replay fix appears safe to merge, with a non-blocking display regression for network games and replay playback.

Findings

1.  Opponent choices appear random ▶

Summary

Moves skirmish slot restoration ahead of replay creation so restarted games record their original choices.

• Skirmish replays record the original slot setup before saving replay data.

Diagram

sequenceDiagram
  participant Recorder
  participant Slots as Skirmish slots
  participant Logic as Game logic
  Recorder->>Slots: Restore or save original choices
  Recorder->>Recorder: Write replay slot data
  Logic->>Slots: Restore original choices
  Logic->>Slots: Fill random choices

Loading

_{Reviews (1) · Last reviewed commit: "bugfix(recorder): Fixed mismatch by hand..." · Reviewed by Greptile}

[greptile-apps]

Opponent choices appear random

The new GAME_SKIRMISH guard also removes saveOriginalSetup() for network games and replay playback. Those slots keep their original fields at -1, so the loading screen and diplomacy panel show opponents as random even when they picked a fixed color and faction. Keep saving the original setup for every active slot; restrict only restoration to skirmish games.