[bobtista]

• Closes Map transfer does not place the received files on non-Windows builds #3102

Map transfer built the preview and companion file paths with a hard coded backslash, so a map path using / produced mixed separators. The save game map path search also only looked for backslashes.

Now the companion files are built in the map's own directory with its separator, and the save game search accepts either separator.

Todo:

• Transfer a custom map between clients and compare checksums
• Confirm Windows map transfers still work
• Launch both games to the main menu, with -quickstart, and into a campaign mission

Note

Handle both '/' and '' as path separators in map path lookups

• Map path helpers in FileTransfer (e.g. GetBasePathFromPath, GetFileFromPath, GetINIFromMap) previously searched only for backslashes; they now use new PathUtil helpers (getLastPathSeparator, getFileName, isPathSeparator) that accept both separators
• Adds PathUtil.isNativePathSeparator and getNativePathSeparator, and a new FileSystem.appendPathSeparator helper in FileSystem.cpp
• Sibling-path construction in FileTransfer.GetFileInMapDirectory reuses the input path's own separator instead of hardcoding a backslash
• GameState.getMapLeafName and GameState.getMapLeafAndDirName in GameState.cpp and GameState.cpp now extract map leaf names after either separator
• Behavioral Change: PathUtil.isPathSeparator now accepts both separators on every platform, and map sibling paths (.ini, .str, .tga preview, etc.) keep the input path's separator form rather than a hardcoded backslash; several FileTransfer getters now take path parameters by const reference

^{Macroscope summarized 3e76237. (Automatic summaries will resume when PR exits draft mode or review begins).}

[coderabbitai]

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

Walkthrough

Path utilities now provide shared separator and filename handling. Map-transfer helpers use these utilities to build companion paths, and save-game map-path parsing recognizes both slash types.

Changes

Map path handling

Layer / File(s)	Summary
Path separator utilities Core/Libraries/Include/Lib/PathUtil.h, Core/GameEngine/Include/Common/FileSystem.h, Core/GameEngine/Source/Common/System/FileSystem.cpp	Path utilities distinguish native separators from separators accepted on all platforms. They add last-separator and filename helpers, update absolute-path and extension handling, and add FileSystem::appendPathSeparator.
Save-game map-path parsing Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp, GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp	Both GameState implementations use shared separator helpers to find map leaf names and directories. Their range searches use exclusive end pointers.
Map-transfer path construction Core/GameEngine/Include/GameNetwork/FileTransfer.h, Core/GameEngine/Source/GameNetwork/FileTransfer.cpp	Map-path helper parameters change to const references. Map-transfer parsing and companion-path construction use shared path utilities.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: Low · previous review 3e762

Saves with an empty description and a forward-slash map path can show the full path instead of the map filename. This is a limited save-list display issue; the PR is otherwise mergeable with this correction noted.

Architecture risk: Medium · up to 3e762

The change affects 3 systems.

Changed systems: Generals, GeneralsMD, Core

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — Generals (service) was modified; 1 changed file maps to changed impact.
• observed — GeneralsMD (service) was modified; 1 changed file maps to changed impact.
• observed — Core (service) was modified; 5 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in Core/GameEngine/Include/Common/FileSystem.h: FileSystem adds the public static appendPathSeparator declaration, documented to append the platform separator unless the path is empty or already ends with either separator.
• observed — Modified behavior in Core/GameEngine/Include/GameNetwork/FileTransfer.h: The five map-path helper declarations now accept a const AsciiString reference instead of taking the argument by value.
• observed — Modified behavior in Core/GameEngine/Source/Common/System/FileSystem.cpp: Added FileSystem::appendPathSeparator; it leaves empty paths and paths already ending in a path separator unchanged, and otherwise appends the native path separator.
• observed — Modified behavior in Core/GameEngine/Source/GameNetwork/FileTransfer.cpp: Added the filesystem and path utility headers used by the updated path construction and parsing.

Reliability and maintainability

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

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Out of Scope Changes check		The current pull request adds unrelated map parsing changes in Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp and the matching GeneralsMD file. These changes alter save-game …	Remove the save-game changes from this pull request, or move them to a separate pull request. Keep only changes that support map-transfer path construction and normalization for #3102.

Passed checks: 3 passed

Check name	Status	Explanation
Linked Issues check		The implementation satisfies #3102. getLastPathSeparator and getFileName recognize both / and \\\\. GetFileInMapDirectory preserves the map directory when it builds companion-file paths. `GetP…
Description check		The description directly explains the map path separator bug, the implemented fixes, and the validation performed.
Title check		The title clearly and concisely identifies the main change: handling both map path separators in network transfers.

Full details: Linked Issues check

Explanation

The implementation satisfies #3102. getLastPathSeparator and getFileName recognize both / and \\. GetFileInMapDirectory preserves the map directory when it builds companion-file paths. GetPreviewFromMap uses shared extension handling. The resulting paths support non-Windows receivers and retain Windows support. Issue #3102 has no separate automated-test requirement.

Full details: Out of Scope Changes check

Explanation

The current pull request adds unrelated map parsing changes in Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp and the matching GeneralsMD file. These changes alter save-game map leaf parsing, but they do not affect network file-transfer path construction or receiver writes. The previous reviewed head already contained the transfer implementation.

Resolution

Remove the save-game changes from this pull request, or move them to a separate pull request. Keep only changes that support map-transfer path construction and normalization for #3102.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 30171078-daf4-4191-a1f2-58a879ab62b0

📥 Commits

Reviewing files that changed from the base of the PR and between bfa3bbf and 2775f3dc2b0602aa72ebb07a92d09ece2a181953.

📒 Files selected for processing (5)

• Core/GameEngine/Include/Common/FileSystem.h
• Core/GameEngine/Include/GameNetwork/FileTransfer.h
• Core/GameEngine/Source/Common/System/FileSystem.cpp
• Core/GameEngine/Source/GameNetwork/FileTransfer.cpp
• Core/Libraries/Include/Lib/PathUtil.h

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Apply separator-aware parsing to fallback map labels. · GameState.cpp:1669

Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp:1669
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Apply separator-aware parsing to fallback map labels.

Both implementations now accept / map paths but still search only for \ when the map has no label. A save with an empty description can therefore display the full map path instead of the filename. (raw.githubusercontent.com)

• Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp#L1669-L1669: use getMapLeafName to set the fallback label.
• GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp#L1669-L1669: use getMapLeafName to set the fallback label.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 134fc0bd-3ee2-47a2-b598-cd66c63ef7b0

📥 Commits

Reviewing files that changed from the base of the PR and between 2775f3dc2b0602aa72ebb07a92d09ece2a181953 and 3e76237a2b146e2bd9e481096cd9da0dde8cee13.

📒 Files selected for processing (2)

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

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The supplied changes appear safe to merge.

Summary

Updates map transfer filenames to keep the map path’s separators.

• Map transfers keep companion files in the map's own path style.

No actionable issues were found in the supplied changes.

_{Reviews (1) · Last reviewed commit: "bugfix(network): Build map transfer path..." · Reviewed by Greptile}