bugfix(recorder): Validate next frame value range in RecorderClass::readNextFrame()

[Caball009]

This PR adds a sanity check to RecorderClass::readNextFrame to prevent replays with corrupt data from taking forever to complete. I came across a handful of GenTool replays that had this issue.

Non-headless mode has a fail-safe that partially mitigates the issue there. The script engine triggers a call to GameLogic::exitGame on the final victory / defeat screen, and this adds a GameMessage::MSG_CLEAR_GAME_DATA message to the message stream. This ends replay playback. The message stream is ignored in headless mode, so this doesn't apply there. It's also not a complete fix because the corrupted data may occur before a victory or defeat takes place. I have not encountered replays for which that was the case.

The maximum next frame value is set one hour away, which seems more than reasonable to me. The only way this generates a false positive is if no game action is taken for an hour or more and the CRC interval is set to an hour or more (it's a couple of seconds by default).

---

Here are four Gentool replays that are corrupted somehow and cause the game to keep running without end in headless mode: replays.zip.

TODO:

• Replicate to Generals.

---

@Skyaero42 Tagging you so you're aware of this PR.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: 4cbacfa
• Latest commit, not reviewed: e5551c0

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• ✅ Review completed - (🔄 Check again to review again)

Walkthrough

Recorder playback validates the first and subsequent next-frame values, except during analysis. A value earlier than the current frame or at least 3,600 logic-seconds ahead stops playback and sets the next frame to -1.

Changes

Playback frame validation

Layer / File(s)	Summary
Validate the next playback frame GeneralsMD/Code/GameEngine/Include/Common/Recorder.h, GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp	RecorderClass declares validateNextFrameValue(). playbackFile validates the first frame, and updatePlayback validates each subsequent next-frame value. During analysis, validation is skipped. Otherwise, an earlier-than-current frame or a frame at least 3,600 logic-seconds ahead sets the next frame to -1 and stops playback.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: xezon

Merge Risk: 🔵 Low · up to 4cbac

Normal replay completion produces a misleading invalid-frame log and a duplicate clear-game-data message. Playback still stops, so this is a bounded cleanup and diagnostic issue rather than a replay-crash risk.

### Architecture Summary

Architecture risk: 🟡 Medium · up to 1489f

The change affects 1 system.

Changed systems: GeneralsMD

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — GeneralsMD (service) was modified; 2 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in GeneralsMD/Code/GameEngine/Include/Common/Recorder.h: Adds the protected validateNextFrame() declaration to RecorderClass.
• observed — Modified behavior in GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp: updatePlayback now validates the next frame after reading it.
• observed — Modified behavior in GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp: Added validateNextFrame, which accepts frame numbers from the current game frame up to, but not including, the current frame plus 3,600 seconds of logic frames. An out-of-range value triggers a debug crash, sets m_nextFrame to -1, and stops playback.

Reliability and maintainability

• inferred — Risk-relevant change factors for GeneralsMD: blast_radius_1; direct_dependents_1

Pre-merge checks: 4

Check name	Status	Explanation
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.
Title check		The title clearly identifies the bug fix: validating the next-frame value in RecorderClass::readNextFrame().
Description check		The description explains how the frame-value check prevents corrupted replays from running indefinitely, including the headless-mode context.

• Autopilot · 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.}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Adds replay frame checks to Generals and Zero Hour.

• Replay playback stops when recorded frames fall outside a one-hour range.

_{Reviews (9) · Last reviewed commit: "bugfix(replay): Validate next frame valu..." · Reviewed by Greptile}

[xezon]

The bots are not too happy about this 😄

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 00f01869-6682-4fb3-b5c4-2c58097628f4

📥 Commits

Reviewing files that changed from the base of the PR and between f49dc4c and 1489f3d.

📒 Files selected for processing (2)

• GeneralsMD/Code/GameEngine/Include/Common/Recorder.h
• GeneralsMD/Code/GameEngine/Source/Common/Recorder.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]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: fbdf2d4c-2fd1-451c-8729-ec3faf00f5d8

📥 Commits

Reviewing files that changed from the base of the PR and between 1489f3d and e336693.

📒 Files selected for processing (2)

• GeneralsMD/Code/GameEngine/Include/Common/Recorder.h
• GeneralsMD/Code/GameEngine/Source/Common/Recorder.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]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: adcc53f8-8715-48da-a761-2449570ddc24

📥 Commits

Reviewing files that changed from the base of the PR and between 4dc9a49 and 4cbacfa.

📒 Files selected for processing (2)

• GeneralsMD/Code/GameEngine/Include/Common/Recorder.h
• GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp

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

[Caball009]

The bots are not too happy about this 😄

They're much happier now :)

Adding the m_doingAnalysis check did help with that. Perhaps we can remove m_doingAnalysis entirely at some point, though.

[xezon]

Logic flow looks complicated again. Can we simplify this?

[xezon]

Needs replicate.