Skip to content

feat(replay): Check CRC messages from all players in replays#2649

Open
Caball009 wants to merge 6 commits into
TheSuperHackers:mainfrom
Caball009:feat_replay_check_all_crc
Open

feat(replay): Check CRC messages from all players in replays#2649
Caball009 wants to merge 6 commits into
TheSuperHackers:mainfrom
Caball009:feat_replay_check_all_crc

Conversation

@Caball009

@Caball009 Caball009 commented Apr 23, 2026

Copy link
Copy Markdown

The original behavior wrt replays is that only the CRC messages from the player that recorded the replay are checked. This means that players can experience a 'live' mismatch, but the game won't show the mismatch for their replay if they didn't cause it.

That's not a big deal for regular players but still unexpected behavior. For developers it can be very useful to get the exact frame the mismatch happens, regardless of which player recorded the replay or which player caused the mismatch.

This PR changes the default behavior so that the CRC messages from all players are checked. I also added an opt-out command line -replayLocalPlayerCRC because developers with large replay collections may want to rely on the original behavior. As a small bonus the name of the mismatching player is now displayed on screen if it's possible to determine which player is responsible.

See commits for cleaner diff.

Before:

replay_crc_before.mp4

After:

replay_crc_after.mp4

TODO:

  • Replicate in Generals.
  • Do another large replay test before merging.
  • Clean up commits.
  • Update this post with the updated command line options.
  • Create separate commit for the deprecation of the CRC playback argument.
  • Move changes from last commit to other commits.
  • Split into smaller commits.
  • Add comments

@Caball009 Caball009 added Enhancement Is new feature or request Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour labels Apr 23, 2026
@Caball009 Caball009 force-pushed the feat_replay_check_all_crc branch 4 times, most recently from 6b0ab7d to b2e84cd Compare April 24, 2026 19:28
@Caball009 Caball009 force-pushed the feat_replay_check_all_crc branch 4 times, most recently from ea2f91d to 2bf436a Compare June 4, 2026 14:22
@Caball009 Caball009 force-pushed the feat_replay_check_all_crc branch 4 times, most recently from beaa007 to 3a28eae Compare June 21, 2026 16:50
@Caball009 Caball009 marked this pull request as ready for review June 21, 2026 17:02
@greptile-apps

greptile-apps Bot commented Jun 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR changes replay CRC verification to check messages from all recorded players (not just the one who recorded the replay), so a desync shows up regardless of which player caused it or saved the file. The mismatch dialog now includes the responsible player's name where attribution is possible, and a new -replayCRCMode command-line option lets developers revert to the original per-recorder behavior.

  • RecorderClass::CRCInfo is overhauled: a single std::list is replaced with a separate playback queue plus per-player std::vector arrays, and handleCRCMessage() is split into handlePlaybackCRCMessage() / handlePlayerCRCMessage() / checkForMismatch().
  • GameLogic::m_shouldValidateCRCs (bool) is promoted to m_validationModeCRC (enum), and the mismatch-check block in processCommandList() is extracted into a dedicated checkForMismatch() method.
  • The RETAIL_COMPATIBLE_CRC preprocessor guard now gates the boolean "isPlayback" argument on CRC messages, making the argument optional for non-retail builds.

Confidence Score: 5/5

Safe to merge; the new all-players CRC path is logically equivalent to the old single-player path for existing replays, and the mode flag defaults to the new behaviour without breaking backward compatibility.

The refactored CRCInfo correctly uses signed Byte sentinel values (-1, -2) that rely on Byte being typedef'd to char (confirmed in BaseTypeCore.h). Queue synchronisation between m_playbackData and m_playerData is maintained by the same REPLAY_CRC_INTERVAL cadence. The sawCRCMismatch() early-exit guard is preserved via the return-false in onLogicCrc(), and the getFrame()>0 guard from the old code is effectively covered by the existing m_skippedOne logic. The extracted GameLogic::checkForMismatch() is functionally identical to the old inline block.

No files require special attention; Recorder.cpp carries the most new complexity but the generateMismatchData() logic and queue-drain path have been traced through all edge cases.

Important Files Changed

Filename Overview
GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp Core of the change: CRCInfo refactored into separate playback and per-player queues; generateMismatchData() implements multi-player mismatch attribution with correct queue-drain and frame calculation. Logic is sound, including the signed-Byte sentinel values (-1, -2) which work correctly because Byte is typedef'd to char (signed).
Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp onLogicCrc() now routes messages to handlePlaybackCRCMessage or handlePlayerCRCMessage based on isLocalPlayer(); early-exit guards for sawCRCMismatch() preserve the previous "stop after first mismatch" invariant.
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp processCommandList() now dispatches to GameLogic::checkForMismatch() (network) or TheRecorder->checkForMismatch() (replay) via the new CRCValidationMode enum; extracted checkForMismatch() is equivalent to the old inline block.
GeneralsMD/Code/GameEngine/Include/Common/Recorder.h CRCInfo API redesigned with MismatchData struct and mode enum; clean separation of playback vs player CRC operations.
GeneralsMD/Code/GameEngine/Source/Common/CommandLine.cpp Adds -replayCRCMode option with numeric argument (0/1/2 → all-single/all-multi/local); out-of-range values fall through to mode 0 gracefully.
GeneralsMD/Code/GameEngine/Include/Common/Player.h getPlayerDisplayName() gains const qualifier, required for calls on const Player* in the new mismatch reporting path.
GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h m_replayCRCCheckMode field added (UnsignedByte); initialized to 0 in GlobalData.cpp constructor.
GeneralsMD/Code/GameEngine/Include/GameLogic/GameLogic.h m_shouldValidateCRCs (Bool) replaced by m_validationModeCRC (CRCValidationMode enum); checkForMismatch() declared private.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant GL as GameLogic::update()
    participant CL as TheCommandList
    participant GLD as onLogicCrc()
    participant REC as RecorderClass
    participant CI as CRCInfo

    Note over GL: Every REPLAY_CRC_INTERVAL frames
    GL->>CL: appendMessage(MSG_LOGIC_CRC, crc, isPlayback)
    Note over CL: Replay file CRCs also in list

    GL->>GLD: processCommandList() → logicMessageDispatcher()

    alt isLocalPlayer (observer/playback)
        GLD->>REC: handlePlaybackCRCMessage(crc)
        REC->>CI: pushPlaybackCRC(crc)
        Note over CI: Skips first in multiplayer (m_skippedOne)
    else non-local player (recorded)
        GLD->>REC: handlePlayerCRCMessage(playerIndex, crc)
        REC->>CI: pushPlayerCRC(playerIndex, crc)
        GLD->>GL: "m_validationModeCRC = CRCMODE_REPLAY"
    end

    GL->>REC: checkForMismatch() [if CRCMODE_REPLAY]
    REC->>CI: generateMismatchData()

    loop for each accumulated CRC interval j
        CI->>CI: popPlaybackCRC()
        loop for each player i
            CI->>CI: compare m_playerData[i][j] vs playbackCRC
        end
        alt single mismatch and MULTIPLEMISMATCH mode
            CI-->>CI: ignore, continue
        else mismatch confirmed
            CI-->>REC: MismatchData(playerIndex, frame, crcs)
        end
    end

    alt mmData.mismatched
        REC->>REC: "TheInGameUI->message(GUI:CRCMismatch)"
        REC->>REC: "TheInGameUI->message(details with player name)"
        REC->>REC: setGamePaused() + setSawCRCMismatch()
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant GL as GameLogic::update()
    participant CL as TheCommandList
    participant GLD as onLogicCrc()
    participant REC as RecorderClass
    participant CI as CRCInfo

    Note over GL: Every REPLAY_CRC_INTERVAL frames
    GL->>CL: appendMessage(MSG_LOGIC_CRC, crc, isPlayback)
    Note over CL: Replay file CRCs also in list

    GL->>GLD: processCommandList() → logicMessageDispatcher()

    alt isLocalPlayer (observer/playback)
        GLD->>REC: handlePlaybackCRCMessage(crc)
        REC->>CI: pushPlaybackCRC(crc)
        Note over CI: Skips first in multiplayer (m_skippedOne)
    else non-local player (recorded)
        GLD->>REC: handlePlayerCRCMessage(playerIndex, crc)
        REC->>CI: pushPlayerCRC(playerIndex, crc)
        GLD->>GL: "m_validationModeCRC = CRCMODE_REPLAY"
    end

    GL->>REC: checkForMismatch() [if CRCMODE_REPLAY]
    REC->>CI: generateMismatchData()

    loop for each accumulated CRC interval j
        CI->>CI: popPlaybackCRC()
        loop for each player i
            CI->>CI: compare m_playerData[i][j] vs playbackCRC
        end
        alt single mismatch and MULTIPLEMISMATCH mode
            CI-->>CI: ignore, continue
        else mismatch confirmed
            CI-->>REC: MismatchData(playerIndex, frame, crcs)
        end
    end

    alt mmData.mismatched
        REC->>REC: "TheInGameUI->message(GUI:CRCMismatch)"
        REC->>REC: "TheInGameUI->message(details with player name)"
        REC->>REC: setGamePaused() + setSawCRCMismatch()
    end
Loading

Reviews (10): Last reviewed commit: "Expanded CRC check modes." | Re-trigger Greptile

Comment thread Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp
Comment thread GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp Outdated
Comment thread GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp Outdated
@Caball009 Caball009 marked this pull request as draft June 21, 2026 17:38
@Caball009 Caball009 marked this pull request as ready for review June 21, 2026 17:41
@Caball009

Copy link
Copy Markdown
Author

@greptileai re-review this pull request now that the previous review concerns are addressed.

@Caball009 Caball009 force-pushed the feat_replay_check_all_crc branch from 3a28eae to 8907b92 Compare June 21, 2026 18:57
Comment on lines +3779 to +3784
#if RETAIL_COMPATIBLE_CRC
// TheSuperHackers @tweak Caball009 21/06/2026 Playback argument serves no purpose anymore
// other than to be able play replays from newer retail compatible builds on older builds or retail.
const bool isPlayback = (TheRecorder && TheRecorder->isPlaybackMode());
msg->appendBooleanArgument(isPlayback);
#endif

@Caball009 Caball009 Jun 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Perhaps this deserves special attention during reviewing due to potential issues wrt backward compatibility of the replay format.

@Caball009 Caball009 force-pushed the feat_replay_check_all_crc branch 4 times, most recently from f521203 to eec0f3d Compare June 23, 2026 00:31
@Caball009 Caball009 force-pushed the feat_replay_check_all_crc branch from eec0f3d to df84dba Compare June 25, 2026 22:34
@Caball009 Caball009 force-pushed the feat_replay_check_all_crc branch from e0738c5 to 5461f5b Compare June 26, 2026 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Is new feature or request Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant