Skip to content

bugfix(network): Handle both map path separators - #3108

Draft
bobtista wants to merge 7 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/bugfix/map-transfer-paths
Draft

bobtista wants to merge 7 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/bugfix/map-transfer-paths

Conversation

@bobtista

@bobtista bobtista commented Aug 11, 2026 •

Copy link
Copy Markdown

Map transfer paths only recognized backslashes, so paths using /
produced incorrect preview and companion-file names.

Now both separators are recognized and companion files keep the map's
directory.

Todo:

  • Transfer a custom map between clients and compare checksums
  • Confirm Windows map transfers still work

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.

@bobtista
bobtista marked this pull request as draft August 11, 2026 15:51
@bobtista bobtista self-assigned this Aug 11, 2026
@bobtista bobtista added the Platform Work towards platform support, such as Linux, MacOS label Aug 11, 2026
@bobtista
bobtista force-pushed the bobtista/bugfix/map-transfer-paths branch from 7fb08ab to 8723ea3 Compare August 27, 2026 16:30
@bobtista
bobtista force-pushed the bobtista/bugfix/map-transfer-paths branch from 8723ea3 to 719001c Compare September 11, 2026 17:32
@bobtista
bobtista force-pushed the bobtista/bugfix/map-transfer-paths branch from 719001c to 9a69940 Compare September 14, 2026 21:06
@bobtista
bobtista force-pushed the bobtista/bugfix/map-transfer-paths branch from 9a69940 to 3767abf Compare September 14, 2026 21:30
@bobtista bobtista changed the title bugfix(network): Build map transfer paths with the platform separator bugfix(network): Handle both map path separators Sep 15, 2026
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to 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 Summary

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 ⚠️ Warning 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
Description check ✅ Passed The description accurately explains the path-separator bug, the companion-file fix, and the reported validation steps.
Title check ✅ Passed The title clearly and concisely describes the main change: handling both map path separators in network transfers.
Linked Issues check ✅ Passed The implementation satisfies #3102. getLastPathSeparator and getFileName recognize both / and \\\\. GetFileInMapDirectory preserves the map directory when it builds companion-file paths. `GetP…
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.


Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 2775f3d.

📒 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.

Comment thread Core/GameEngine/Source/GameNetwork/FileTransfer.cpp

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 2775f3d and 3e76237.

📒 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.

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

Labels

Platform Work towards platform support, such as Linux, MacOS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Map transfer does not place the received files on non-Windows builds

1 participant