Skip to content

feat(cli): Play a replay file from the command line - #3227

Open
bobtista wants to merge 2 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/feature/loadreplay-cli
Open

feat(cli): Play a replay file from the command line#3227
bobtista wants to merge 2 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/feature/loadreplay-cli

Conversation

@bobtista

Copy link
Copy Markdown

There is no way to play a replay visually from the command line. -replay simulates headlessly, so an externally supplied .rep cannot be launched from an operating-system file handler and watched.

Now -loadreplay <file> plays a replay through the normal client, and an absolute path is opened in place while a relative name still resolves from the Replay directory. RecorderClass::readReplayHeader uses FileSystem::isAbsolutePath from #3226 for that, and loadQueuedReplay runs at the point -loadsave already uses, once the client has initialized the shell, so the menus the playback returns to are on the stack.

A replay whose map is unavailable starts a game that cannot load, so loadQueuedReplay rejects it up front and quits rather than failing deep in map loading.

Verified with a bogus path as a control so a pass is distinguishable from "the game started anyway":

case result
control: bogus absolute path exits
Replay from an absolute path containing spaces (quoted) loads and plays
Relative Replay filename loads from the managed directory
Replay from a UNC path (\\localhost\C$\...) loads
Restarting a Replay loaded from an absolute path restarts and replays from the beginning
Normal Replay synchronization reporting reports and pauses (InGame:D9C721A5 Replay:D8A198C0 Frame:110)

Todo:

  • Both games build (z_generals and g_generals)
  • Replay paths outside the user data directory
  • Paths containing spaces
  • Windows drive paths and UNC paths
  • Relative Replay filenames still resolve from the managed directory
  • Restarting an externally loaded Replay
  • Normal Replay synchronization reporting is unchanged
  • Replicate to Generals

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add visual replay playback and absolute file loading to CLI

✨ Enhancement 🕐 40+ Minutes

Grey Divider

AI Description

• Adds -loadreplay to launch visual replay playback after client shell initialization.
• Supports absolute replay and save paths while preserving managed-directory resolution for relative
 names.
• Rejects unreadable, malformed, or map-missing replays before entering gameplay.
Diagram

graph TD
  CLI["CLI parser"] --> Queue["Startup request"] --> Client["Client update"] --> Loader["Replay loader"] --> Resolver["Path resolver"] --> Check{"Replay valid?"}
  Check -->|Yes| Playback["Visual playback"]
  Check -->|No| Quit["Quit game"]
Loading
High-Level Assessment

The queued startup approach is appropriate because it reuses the established -loadsave lifecycle point, ensuring the client shell exists before playback and remains available afterward. Starting playback directly during command-line parsing was considered but would run before required client and filesystem state is initialized; centralizing absolute-versus-relative path resolution also preserves existing menu behavior.

Files changed (15) +214 / -18

Enhancement (15) +214 / -18
FileSystem.hExpose platform-aware absolute path detection +1/-0

Expose platform-aware absolute path detection

• Declares a shared helper for distinguishing explicit absolute paths from names resolved within managed directories.

Core/GameEngine/Include/Common/FileSystem.h

CommandLine.cppAdd and validate startup file-loading options +34/-2

Add and validate startup file-loading options

• Adds the '-loadreplay' parser and registration, validates replay and save extensions, and queues startup playback while suppressing intro and shell-map startup. Missing arguments now consume only the option itself.

Core/GameEngine/Source/Common/CommandLine.cpp

FileSystem.cppImplement cross-platform absolute path recognition +25/-0

Implement cross-platform absolute path recognition

• Recognizes Windows drive-rooted, current-drive-rooted, and UNC-style paths, plus POSIX root paths.

Core/GameEngine/Source/Common/System/FileSystem.cpp

GameState.hDeclare save read-path resolution helper +1/-0

Declare save read-path resolution helper

• Adds the Generals API for resolving absolute save paths or managed-directory filenames.

Generals/Code/GameEngine/Include/Common/GameState.h

GlobalData.hStore queued replay startup requests +1/-0

Store queued replay startup requests

• Adds global startup state for the replay requested through '-loadreplay'.

Generals/Code/GameEngine/Include/Common/GlobalData.h

Recorder.hExpose queued replay loading +1/-0

Expose queued replay loading

• Declares the recorder entry point that validates and starts a command-line replay request.

Generals/Code/GameEngine/Include/Common/Recorder.h

Recorder.cppResolve and preflight queued replays +49/-2

Resolve and preflight queued replays

• Reads absolute replay paths in place while retaining Replay-directory lookup for relative names. Validates replay headers, game options, and map availability before playback, quitting cleanly on failure.

Generals/Code/GameEngine/Source/Common/Recorder.cpp

GameState.cppSupport absolute save paths in Generals +20/-6

Support absolute save paths in Generals

• Centralizes save read-path resolution so command-line absolute paths open in place and relative menu names remain under the Save directory. Applies the helper to metadata, existence, and full-load paths.

Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp

GameClient.cppStart queued replays after client initialization +5/-0

Start queued replays after client initialization

• Invokes replay loading from the established queued-load lifecycle point after shell setup, while retaining save-load priority.

Generals/Code/GameEngine/Source/GameClient/GameClient.cpp

GameState.hDeclare Zero Hour save path resolution +1/-0

Declare Zero Hour save path resolution

• Adds the Zero Hour API for resolving absolute save paths or managed-directory filenames.

GeneralsMD/Code/GameEngine/Include/Common/GameState.h

GlobalData.hStore Zero Hour replay startup requests +1/-0

Store Zero Hour replay startup requests

• Adds global startup state for the replay requested through '-loadreplay'.

GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h

Recorder.hExpose Zero Hour queued replay loading +1/-0

Expose Zero Hour queued replay loading

• Declares the recorder entry point that validates and starts a command-line replay request.

GeneralsMD/Code/GameEngine/Include/Common/Recorder.h

Recorder.cppResolve and preflight Zero Hour replays +49/-2

Resolve and preflight Zero Hour replays

• Mirrors absolute and relative replay path handling for Zero Hour. Validates replay headers, game options, and map availability before playback, quitting cleanly on failure.

GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp

GameState.cppSupport absolute save paths in Zero Hour +20/-6

Support absolute save paths in Zero Hour

• Mirrors centralized save read-path resolution so absolute command-line paths open in place and relative names remain under the Save directory.

GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp

GameClient.cppStart Zero Hour replays after initialization +5/-0

Start Zero Hour replays after initialization

• Invokes queued replay loading after shell setup in Zero Hour while retaining queued save-load priority.

GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp

@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds -loadreplay startup playback, including absolute-path support, replay validation, and coordinated implementations for Generals and Zero Hour. It also extends the related -loadsave path handling from the dependent change.

  • Adds platform-aware absolute-path detection to the shared filesystem interface.
  • Queues command-line replay playback until the client shell is initialized.
  • Validates replay headers and map availability before starting playback.
  • Allows absolute replay and save paths while preserving managed-directory resolution for relative names.

Confidence Score: 5/5

The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issues identified.

Absolute and relative path handling remains consistent with the documented command-line contracts, queued playback runs after required initialization, and the replay preflight cleans up temporary state before beginning playback.

Important Files Changed

Filename Overview
Core/GameEngine/Source/Common/CommandLine.cpp Adds -loadreplay parsing and validates replay/save extensions before queuing startup work.
Core/GameEngine/Source/Common/System/FileSystem.cpp Adds platform-aware recognition of rooted Windows and POSIX paths.
Generals/Code/GameEngine/Source/Common/Recorder.cpp Supports absolute replay paths and validates queued replay headers and map availability before playback.
GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp Mirrors the queued replay and absolute-path behavior for Zero Hour.
Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp Resolves absolute save paths in place while retaining managed Save-directory lookup for relative names.
GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp Mirrors the command-line save-path resolution behavior for Zero Hour.
Generals/Code/GameEngine/Source/GameClient/GameClient.cpp Starts queued replay playback after shell initialization, following the existing queued-save lifecycle point.
GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp Mirrors the post-shell queued replay startup hook for Zero Hour.

Sequence Diagram

sequenceDiagram
    participant CLI as Command line
    participant Client as GameClient
    participant Recorder as RecorderClass
    participant FS as FileSystem
    participant Maps as MapCache
    CLI->>Client: Queue -loadreplay path
    Client->>Client: Initialize and show shell
    Client->>Recorder: loadQueuedReplay()
    Recorder->>FS: Resolve absolute path or Replay directory
    FS-->>Recorder: Replay header
    Recorder->>Maps: Validate replay map
    Maps-->>Recorder: Map available
    Recorder->>Recorder: playbackFile(path)
    Recorder-->>Client: Start visual replay playback
Loading

Reviews (1): Last reviewed commit: "feat(cli): Play a replay file from the c..." | Re-trigger Greptile

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (1) 📜 Skill insights (0)

Grey Divider


Action required

1. -ignoreReplaySyncErrors is unregistered 📎 Requirement gap ≡ Correctness
Description
The PR registers -loadreplay but does not register the required -ignoreReplaySyncErrors option,
so invoking the mandated suppression flag cannot set TheDebugIgnoreSyncErrors. Only the
differently named legacy -ignoresync option reaches parseSync.
Code

Core/GameEngine/Source/Common/CommandLine.cpp[1208]

+	{ "-loadreplay", parseLoadReplay },
Evidence
PR Compliance ID 6 explicitly requires suppression when -ignoreReplaySyncErrors is supplied. The
PR extends the startup command table with -loadreplay at line 1208, while the branch contains no
registration for -ignoreReplaySyncErrors; parseSync at lines 810-814 provides the required
behavior but is registered only under -ignoresync at line 1322.

Honor explicit replay synchronization error suppression
Core/GameEngine/Source/Common/CommandLine.cpp[810-814]
Core/GameEngine/Source/Common/CommandLine.cpp[1205-1208]
Core/GameEngine/Source/Common/CommandLine.cpp[1322-1322]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`-loadreplay` must support the explicit `-ignoreReplaySyncErrors` command-line option, but that name is not registered.

## Issue Context
The existing `parseSync` handler already enables `TheDebugIgnoreSyncErrors`, and the legacy `-ignoresync` registration should remain compatible. Register the required option name as an alias to the same handler.

## Fix Focus Areas
- Core/GameEngine/Source/Common/CommandLine.cpp[1205-1208]
- Core/GameEngine/Source/Common/CommandLine.cpp[1319-1323]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Playerless replay crashes startup 🐞 Bug ≡ Correctness
Description
loadQueuedReplay() passes headers with localPlayerIndex == -1 into playbackFile(), which
dereferences getSlot(-1) and crashes instead of playing or cleanly rejecting the replay. The
recorder itself can write -1 for non-network single-player recordings, so -loadreplay can hit
this with a generated replay file.
Code

Generals/Code/GameEngine/Source/Common/Recorder.cpp[1117]

+	if (!playbackFile(filename))
Evidence
The replay writer initializes the recorded local index to -1 and leaves it unchanged for
non-network, non-skirmish single-player recording. The reader considers -1 valid, while the newly
invoked playback path passes it to GameInfo::getSlot, which returns null for negative indexes
before the caller dereferences it; GeneralsMD mirrors the same path.

Generals/Code/GameEngine/Source/Common/Recorder.cpp[590-640]
Generals/Code/GameEngine/Source/Common/Recorder.cpp[923-938]
Generals/Code/GameEngine/Source/Common/Recorder.cpp[1117-1120]
Generals/Code/GameEngine/Source/Common/Recorder.cpp[1191-1200]
Core/GameEngine/Source/GameNetwork/GameInfo.cpp[445-452]
GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp[1120-1123]
GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp[1194-1203]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Queued playback can receive a valid replay header with `localPlayerIndex == -1`, but `playbackFile()` unconditionally dereferences that slot and crashes. Handle the no-local-player case without calling `getSlot(-1)`, and apply the equivalent fix to both game variants.

## Issue Context
`readReplayHeader()` explicitly accepts `-1`, and `startRecording()` can serialize `-1` for non-network single-player recordings. The multiplayer flag should only inspect a slot when the index is nonnegative; otherwise use the appropriate non-multiplayer default.

## Fix Focus Areas
- Generals/Code/GameEngine/Source/Common/Recorder.cpp[1117-1120]
- Generals/Code/GameEngine/Source/Common/Recorder.cpp[1191-1200]
- GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp[1120-1123]
- GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp[1194-1203]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources

Grey Divider

Tip of the day
💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo


// TheSuperHackers @feature bobtista 22/07/2026 Load a save game file from the command line.
{ "-loadsave", parseLoadSave },
{ "-loadreplay", parseLoadReplay },

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. -ignorereplaysyncerrors is unregistered 📎 Requirement gap ≡ Correctness

The PR registers -loadreplay but does not register the required -ignoreReplaySyncErrors option,
so invoking the mandated suppression flag cannot set TheDebugIgnoreSyncErrors. Only the
differently named legacy -ignoresync option reaches parseSync.
Agent Prompt
## Issue description
`-loadreplay` must support the explicit `-ignoreReplaySyncErrors` command-line option, but that name is not registered.

## Issue Context
The existing `parseSync` handler already enables `TheDebugIgnoreSyncErrors`, and the legacy `-ignoresync` registration should remain compatible. Register the required option name as an alias to the same handler.

## Fix Focus Areas
- Core/GameEngine/Source/Common/CommandLine.cpp[1205-1208]
- Core/GameEngine/Source/Common/CommandLine.cpp[1319-1323]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

return;
}

if (!playbackFile(filename))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Playerless replay crashes startup 🐞 Bug ≡ Correctness

loadQueuedReplay() passes headers with localPlayerIndex == -1 into playbackFile(), which
dereferences getSlot(-1) and crashes instead of playing or cleanly rejecting the replay. The
recorder itself can write -1 for non-network single-player recordings, so -loadreplay can hit
this with a generated replay file.
Agent Prompt
## Issue description
Queued playback can receive a valid replay header with `localPlayerIndex == -1`, but `playbackFile()` unconditionally dereferences that slot and crashes. Handle the no-local-player case without calling `getSlot(-1)`, and apply the equivalent fix to both game variants.

## Issue Context
`readReplayHeader()` explicitly accepts `-1`, and `startRecording()` can serialize `-1` for non-network single-player recordings. The multiplayer flag should only inspect a slot when the index is nonnegative; otherwise use the appropriate non-multiplayer default.

## Fix Focus Areas
- Generals/Code/GameEngine/Source/Common/Recorder.cpp[1117-1120]
- Generals/Code/GameEngine/Source/Common/Recorder.cpp[1191-1200]
- GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp[1120-1123]
- GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp[1194-1203]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow -loadsave and -loadreplay to load files from any directory

1 participant