Skip to content

bugfix(map): Handle map names without a path separator - #3106

Draft
bobtista wants to merge 5 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/bugfix/map-display-name-separator
Draft

bobtista wants to merge 5 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/bugfix/map-display-name-separator

Conversation

@bobtista

@bobtista bobtista commented Aug 11, 2026 •

Copy link
Copy Markdown

Closes #3100.
Depends on #3105; its commits are included until it merges.

Both fallback display-name paths in MapCache::addMap previously added one to the result of a backslash search without checking whether a separator was found. A filename containing only forward slashes, or no separator, could therefore pass an invalid pointer to AsciiString::set.

Both paths now use getFileName from Lib/PathUtil.h, sharing the separator handling introduced by #3105. It accepts either separator and returns the whole filename when neither is present. The extension is preserved, matching the previous behavior for backslash-separated paths:

Input:    /home/user/GeneralsData/Maps/MyMap/MyMap.map
Fallback: MyMap.map

All changes are in Core.

Verification

  • Static review covers both the cached-map and newly loaded-map fallback paths; git diff --check passes.
  • The earlier description reported a successful scan of 384 maps requiring fallback names. The exact integration branch/commit for that run has not been verified, so it is not evidence of a standalone reproduction on this PR branch.
  • At this branch's head, loadMapsFromDisk still rejects slash-only filenames before reaching addMap. End-to-end non-Windows scanning also requires the separate map-cache path changes in bugfix(map): Fix map cache paths on non-Windows #3107.
  • No new game runtime test was performed for this cleanup.

@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 marked this pull request as draft August 11, 2026 15:51
@bobtista
bobtista force-pushed the bobtista/bugfix/map-display-name-separator branch from 754f164 to badbcf4 Compare August 27, 2026 16:30
@bobtista
bobtista force-pushed the bobtista/bugfix/map-display-name-separator branch from badbcf4 to 137d9a6 Compare September 11, 2026 17:32
@bobtista
bobtista force-pushed the bobtista/bugfix/map-display-name-separator branch from 137d9a6 to 5891f8d Compare September 14, 2026 21:06
@bobtista
bobtista force-pushed the bobtista/bugfix/map-display-name-separator branch from 5891f8d to de0d326 Compare September 14, 2026 21:30
@bobtista bobtista changed the title bugfix(map): Guard the fallback map display name against a missing separator bugfix(map): Handle map names without a path separator Sep 24, 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.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 05aab810-1769-4846-9b06-c112ba0eec66

📥 Commits

Reviewing files that changed from the base of the PR and between bfa3bbf and 289154b.

📒 Files selected for processing (4)
  • Core/GameEngine/Include/Common/FileSystem.h
  • Core/GameEngine/Source/Common/System/FileSystem.cpp
  • Core/GameEngine/Source/GameClient/MapUtil.cpp
  • Core/Libraries/Include/Lib/PathUtil.h

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


Walkthrough

Path utilities now recognize both slash characters and provide native-separator and filename helpers. File-system path handling uses the native separator, and map-name fallbacks use the shared filename helper.

Changes

Path handling

Layer / File(s) Summary
Path separator and filename helpers
Core/Libraries/Include/Lib/PathUtil.h
Adds native-separator detection, last-separator lookup, and filename extraction. Both getExtension overloads use the shared last-separator helpers.
File-system separator handling
Core/GameEngine/Include/Common/FileSystem.h, Core/GameEngine/Source/Common/System/FileSystem.cpp
Adds FileSystem::appendPathSeparator. isPathInDirectory checks the final character against the native separator before appending one.
Map filename fallbacks
Core/GameEngine/Source/GameClient/MapUtil.cpp
Uses getFileName in the cache-hit and map-loading fallbacks when a map name is unavailable.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 28915

The path and map fallback changes show no supported regression in the reviewed callers. No actionable merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The new public FileSystem::appendPathSeparator declaration and implementation are not used by either fallback path and do not support the MapCache::addMap fix for issue #3100. The related `isPathI… Remove FileSystem::appendPathSeparator from FileSystem.h and FileSystem.cpp, unless a directly linked coding requirement establishes its need.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change satisfies issue #3100. Both fallback paths in MapCache::addMap now call getFileName. getFileName accepts / and \\ and returns the complete input when no separator exists. This prev…
Title check ✅ Passed The title clearly identifies the primary bug fix: handling map names when the path has no separator. It is concise and relevant to the changeset.
Description check ✅ Passed The description accurately explains the fallback-name bug, the use of getFileName, supported separators, affected paths, limitations, and verification status.
Full details: Out of Scope Changes check

Explanation

The new public FileSystem::appendPathSeparator declaration and implementation are not used by either fallback path and do not support the MapCache::addMap fix for issue #3100. The related isPathInDirectory update and PathUtil helper changes support the path-separator refactor, but this new API has no demonstrated connection to the linked issue.


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

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.

Crash scanning a map with no map name on non-Windows builds

1 participant