Skip to content

feat(cli): Load save files from absolute paths - #3226

Open
bobtista wants to merge 1 commit into
TheSuperHackers:mainfrom
bobtista:bobtista/feature/load-save-from-absolute-path
Open

feat(cli): Load save files from absolute paths#3226
bobtista wants to merge 1 commit into
TheSuperHackers:mainfrom
bobtista:bobtista/feature/load-save-from-absolute-path

Conversation

@bobtista

Copy link
Copy Markdown

-loadsave resolves its argument inside the managed user Save directory, so a file anywhere else has to be copied in before it can be opened. That rules out launching the game directly for a .sav supplied by an operating-system file handler.

Now an absolute path is opened in place while a relative name still resolves from the Save directory, so both forms keep working. FileSystem::isAbsolutePath classifies the argument - a drive root (C:\, C:/), a current-drive or UNC root (leading separator), or a POSIX root - and GameState::getSaveGamePathForRead returns the path untouched when it is absolute. Save writes and the save menu are unaffected; they continue to use the managed directory.

parseLoadSave also gains the argument validation and return value that parseReplay already uses in the same file: reject a name that does not end in .sav, return 2 only when an argument was consumed, and 1 otherwise. Previously it returned 2 unconditionally, so -loadsave with no argument consumed the following token.

Paths containing spaces work when quoted, which is the form an operating-system file handler passes. nextParam is quote-aware: a token beginning with " terminates on the next " rather than on whitespace.

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
Save from an absolute path outside the user directory loads
Save from an absolute path containing spaces (quoted) loads
Relative Save filename loads from the managed directory
Save from a UNC path (\\localhost\C$\...) loads
Save from a POSIX absolute path containing spaces loads

Todo:

  • Both games build (z_generals and g_generals)
  • Absolute path classification covers drive roots, UNC and current-drive roots, and POSIX roots
  • Save paths outside the user data directory
  • Paths containing spaces
  • Windows drive paths and UNC paths
  • Relative Save filenames still resolve from the managed directory
  • Replicate to Generals

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

Copy link
Copy Markdown

PR Summary by Qodo

Load CLI save games from absolute paths

✨ Enhancement 🐞 Bug fix 🕐 20-40 Minutes

Grey Divider

AI Description

• Opens absolute -loadsave paths in place across both game variants.
• Preserves managed Save-directory resolution for relative filenames and menu loads.
• Validates .sav arguments and avoids consuming tokens when no filename is supplied.
Diagram

graph TD
  A["-loadsave CLI"] --> B{"Valid .sav?"}
  B -->|yes| C["Queued save"] --> D{"Absolute path?"}
  D -->|yes| E["Selected file"] --> G["Save loader"]
  D -->|no| F["Save directory"] --> G
  B -->|no| H["Exit error"]
Loading
High-Level Assessment

The centralized path classifier plus read-only GameState resolver is the appropriate approach: it preserves existing write and menu behavior while consistently handling all save read entry points. Using std::filesystem::path was considered, but would introduce broader runtime/toolchain coupling without improving this narrowly scoped integration.

Files changed (7) +79 / -14

Enhancement (6) +68 / -12
FileSystem.hExpose cross-platform absolute-path classification +1/-0

Expose cross-platform absolute-path classification

• Declares a shared helper for distinguishing absolute paths from names relative to managed directories.

Core/GameEngine/Include/Common/FileSystem.h

FileSystem.cppClassify Windows and POSIX absolute paths +25/-0

Classify Windows and POSIX absolute paths

• Implements platform-specific absolute-path detection for rooted Windows drive paths, leading-separator Windows paths including UNC forms, and POSIX root paths.

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

GameState.hDeclare save-read path resolver for Generals +1/-0

Declare save-read path resolver for Generals

• Adds the GameState API that resolves absolute save paths differently from relative save filenames.

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

GameState.cppLoad Generals saves from absolute or managed paths +20/-6

Load Generals saves from absolute or managed paths

• Routes existence checks, metadata reads, queued startup loads, and full loads through a read-path resolver. Absolute paths are preserved while relative names continue resolving under the user Save directory.

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

GameState.hDeclare save-read path resolver for Zero Hour +1/-0

Declare save-read path resolver for Zero Hour

• Adds the mirrored GameState API for resolving absolute and relative save inputs.

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

GameState.cppLoad Zero Hour saves from absolute or managed paths +20/-6

Load Zero Hour saves from absolute or managed paths

• Mirrors the Generals read-path handling across existence checks, metadata inspection, queued startup loading, and full save loading. Existing relative menu filenames remain scoped to the user Save directory.

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

Bug fix (1) +11 / -2
CommandLine.cppValidate and correctly consume -loadsave arguments +11/-2

Validate and correctly consume -loadsave arguments

• Requires supplied save names to end in '.sav', reports invalid names, and returns the correct consumed-argument count. Missing filenames no longer cause the parser to consume a following token.

Core/GameEngine/Source/Common/CommandLine.cpp

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

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

@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown

Greptile Summary

This PR allows -loadsave to open absolute save paths directly while preserving managed Save-directory resolution for relative filenames.

  • Adds platform-specific absolute-path classification to the shared filesystem API.
  • Routes save existence checks, metadata reads, and full loads through a common read-path resolver in both game variants.
  • Validates the command-line filename extension and corrects argument-consumption behavior when no filename is supplied.

Confidence Score: 5/5

The PR appears safe to merge with no concrete correctness or security defects identified.

Save paths are resolved consistently at each read boundary, existing relative menu filenames retain managed-directory behavior, absolute command-line paths remain intact, and command-line argument consumption now matches the parser contract.

Important Files Changed

Filename Overview
Core/GameEngine/Source/Common/CommandLine.cpp Validates .sav arguments and consumes the filename only when one is present.
Core/GameEngine/Source/Common/System/FileSystem.cpp Adds host-platform absolute-path classification for POSIX roots and Windows drive, rooted, and UNC forms.
Core/GameEngine/Include/Common/FileSystem.h Exposes the new absolute-path classifier through the shared filesystem interface.
Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp Resolves absolute save paths in place and relative names beneath the managed Save directory across metadata and load operations.
GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp Replicates the corrected save read-path behavior for Zero Hour.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CLI["-loadsave argument"] --> Validate{"Ends with .sav?"}
    Validate -- No --> Exit["Exit with invalid name"]
    Validate -- Yes --> Absolute{"Absolute path?"}
    Absolute -- Yes --> Direct["Use supplied path unchanged"]
    Absolute -- No --> Managed["Prepend managed Save directory"]
    Direct --> Inspect["Read save metadata"]
    Managed --> Inspect
    Inspect --> Load["Load save game"]
Loading

Reviews (1): Last reviewed commit: "feat(cli): Load save files from absolute..." | Re-trigger Greptile

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