Skip to content

feat(system): Add -cwd option to keep or override the startup working directory - #3149

Open
CryoTheRenegade wants to merge 3 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:feat/startup-working-directory
Open

feat(system): Add -cwd option to keep or override the startup working directory#3149
CryoTheRenegade wants to merge 3 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:feat/startup-working-directory

Conversation

@CryoTheRenegade

@CryoTheRenegade CryoTheRenegade commented Aug 14, 2026

Copy link
Copy Markdown

Summary

This recreates the abandoned #1445 feature and applies the review feedback from that PR:

  • xezon: GUIEdit, MapCacheBuilder, and WorldBuilder now use the same -cwd behavior
  • xezon: The force-set-cwd logic lives in one place in Core instead of being copied in each WinMain
  • OmniBlade: Full location-agnostic data paths are out of scope here, but -cwd <path> is a small step toward a customizable read-only data location without rewriting every relative file load

Example Visual Studio usage: add -cwd to Command Arguments and set Working Directory to the game install path.

Considerations from #1445:

  • DLLs: The current working directory remains on the DLL search path, after the system folders. mss32.dll and BINKW32.DLL are not system DLLs, so this is fine. See Win32 DLL search order
  • Win32 file access: Relative paths (LoadImageA, LoadCursorFromFile, ...) search the executable path, then the current working directory, then %PATH%. See OpenFile remarks
  • C runtime functions such as fopen always use the current working directory

This change was drafted with LLM assistance

… directory

Co-authored-by: Cursor <cursoragent@cursor.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add -cwd flag to control startup working directory across game and tools

✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Add -cwd / -cwd  to keep or override the process working directory.
• Centralize startup CWD behavior in CommandLine::applyStartupWorkingDirectory().
• Replace duplicated WinMain CWD forcing in game and tool entrypoints.
Diagram

graph TD
  WM["WinMain (Game/Tools)"] --> APPLY["CommandLine::applyStartupWorkingDirectory()"] --> PARSE["Parse raw command line"] --> FOUND{"-cwd present?"}
  FOUND -- "no" --> EXE["Executable dir"] --> SETEXE["SetCurrentDirectory(exe)"]
  FOUND -- "yes" --> HASARG{"Has path arg?"}
  HASARG -- "yes" --> CUSTOM[("Custom dir")] --> SETPATH["SetCurrentDirectory(path)"]
  HASARG -- "no" --> KEEP[("OS working dir")]
  subgraph Legend
    direction LR
    _cmp["Component / function"] ~~~ _dec{"Decision"} ~~~ _data[("Directory state")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Base-path abstraction instead of changing process CWD
  • ➕ Avoids side effects on C runtime relative file I/O and DLL search behavior
  • ➕ Makes data path resolution explicit at each load site
  • ➖ Much larger scope: requires auditing and rewriting many relative file loads
  • ➖ Harder to roll out consistently across game and tools
2. Split flags: `-keepcwd` and `-cwd `
  • ➕ Clearer intent; avoids ambiguity of -cwd with/without argument
  • ➕ Less chance of accidental no-op if a user forgets the path
  • ➖ Breaks compatibility with the superseded PR’s proposed UX
  • ➖ Adds another user-facing option to document and support
3. Use Win32 DLL-directory APIs to decouple DLL search from CWD
  • ➕ Reduces risk from CWD being on the DLL search path
  • ➕ More controlled module loading behavior
  • ➖ Only addresses DLL search; does not help C runtime relative file paths
  • ➖ More Windows-version nuances and additional implementation complexity

Recommendation: The chosen approach (single -cwd flag + centralized startup helper) is the best incremental step: it preserves current default behavior, avoids pervasive path refactors, and ensures consistent behavior across all entrypoints. Consider documenting the -cwd-without-arg vs -cwd semantics prominently, since the same flag serves two related but distinct use cases.

Files changed (9) +70 / -54

Enhancement (2) +58 / -0
CommandLine.hExpose startup working-directory helper +5/-0

Expose startup working-directory helper

• Adds a new 'CommandLine::applyStartupWorkingDirectory()' API and documents the '-cwd' behavior (default to exe dir; optional override/keep semantics).

Core/GameEngine/Include/Common/CommandLine.h

CommandLine.cppImplement '-cwd' flag and centralized CWD application +53/-0

Implement '-cwd' flag and centralized CWD application

• Introduces 'parseCwd()' so '-cwd' (and its optional path) is consumed during startup parsing. Implements 'CommandLine::applyStartupWorkingDirectory()' to scan the raw command line early, apply an override directory when provided, or fall back to forcing the executable directory when the flag is absent.

Core/GameEngine/Source/Common/CommandLine.cpp

Refactor (7) +12 / -54
WinMain.cppUse shared startup working-directory logic +2/-8

Use shared startup working-directory logic

• Replaces inline WinMain code that forced CWD to the executable directory with a call to 'CommandLine::applyStartupWorkingDirectory()', and includes the needed header.

Core/Tools/MapCacheBuilder/Source/WinMain.cpp

WinMain.cppDelegate CWD setup to 'CommandLine' helper +1/-8

Delegate CWD setup to 'CommandLine' helper

• Removes the local force-set working directory block and calls 'CommandLine::applyStartupWorkingDirectory()' early in startup to honor '-cwd' while preserving the default behavior.

Generals/Code/Main/WinMain.cpp

WinMain.cppUnify GUIEdit CWD behavior with game via '-cwd' +2/-8

Unify GUIEdit CWD behavior with game via '-cwd'

• Adds the CommandLine include and replaces duplicated CWD forcing logic with 'CommandLine::applyStartupWorkingDirectory()' so GUIEdit matches the game’s '-cwd' semantics.

Generals/Code/Tools/GUIEdit/Source/WinMain.cpp

WorldBuilder.cppApply shared startup working-directory logic in WorldBuilder +2/-7

Apply shared startup working-directory logic in WorldBuilder

• Adds the CommandLine include and swaps the local 'SetCurrentDirectory'-to-exe implementation for 'CommandLine::applyStartupWorkingDirectory()' during app initialization.

Generals/Code/Tools/WorldBuilder/src/WorldBuilder.cpp

WinMain.cppDelegate Zero Hour CWD setup to 'CommandLine' helper +1/-8

Delegate Zero Hour CWD setup to 'CommandLine' helper

• Removes the duplicated force-CWD block and uses 'CommandLine::applyStartupWorkingDirectory()' to keep default behavior while enabling '-cwd' overrides.

GeneralsMD/Code/Main/WinMain.cpp

WinMain.cppUnify Zero Hour GUIEdit CWD behavior with '-cwd' +2/-8

Unify Zero Hour GUIEdit CWD behavior with '-cwd'

• Includes 'Common/CommandLine.h' and replaces the inline CWD forcing logic with 'CommandLine::applyStartupWorkingDirectory()' for consistent flag behavior.

GeneralsMD/Code/Tools/GUIEdit/Source/WinMain.cpp

WorldBuilder.cppApply shared startup working-directory logic in Zero Hour WorldBuilder +2/-7

Apply shared startup working-directory logic in Zero Hour WorldBuilder

• Adds the CommandLine include and uses 'CommandLine::applyStartupWorkingDirectory()' instead of per-app 'SetCurrentDirectory' code to support '-cwd' consistently.

GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilder.cpp

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

qodo-free-for-open-source-projects Bot commented Aug 14, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Silent cwd change failure ✓ Resolved 🐞 Bug ☼ Reliability
Description
CommandLine::applyStartupWorkingDirectory() calls SetCurrentDirectory() for "-cwd <path>" but
ignores the return value, so an invalid/inaccessible/empty path silently leaves the process in the
inherited OS working directory while the code assumes the override was applied. Because the function
returns whenever -cwd is present, it also skips the fallback to the executable directory in this
failure case.
Code

Core/GameEngine/Source/Common/CommandLine.cpp[R1465-1468]

+		if (arg + 1 < argc && argv[arg + 1] != nullptr && argv[arg + 1][0] != '-')
+		{
+			::SetCurrentDirectory(argv[arg + 1]);
+		}
Evidence
The new code returns immediately after calling SetCurrentDirectory for -cwd <path>, without checking
success and without invoking the executable-directory fallback; other code in the repo demonstrates
that SetCurrentDirectory failures are expected to be checked and logged.

Core/GameEngine/Source/Common/CommandLine.cpp[1448-1473]
Generals/Code/GameEngine/Source/Common/System/Directory.cpp[74-82]

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

## Issue description
`CommandLine::applyStartupWorkingDirectory()` applies `-cwd <path>` via `SetCurrentDirectory(...)` but does not check for success. When `SetCurrentDirectory` fails (invalid path, permissions, empty string), the process remains in the inherited OS working directory, and the code returns without falling back to the executable directory.
### Issue Context
The repo already uses a pattern of checking `SetCurrentDirectory(...) == 0` and logging/reporting failures (e.g., `Directory::Directory`). This new startup helper should provide similar safety/observability.
### Fix Focus Areas
- Core/GameEngine/Source/Common/CommandLine.cpp[1448-1473]
### Suggested fix
- Capture the return value of `::SetCurrentDirectory(argv[arg + 1])`.
- If it fails, log a warning (or `DEBUG_LOG`) including the attempted path and `GetLastError()`.
- Decide a deterministic fallback behavior (recommended: call `setCurrentDirectoryToExecutablePath()` when the explicit override fails), so relative file loads remain predictable.
- Optionally treat an empty string argument as "no override" (i.e., behave like `-cwd` with no path).

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


2. Unchecked module path result ✓ Resolved 🐞 Bug ☼ Reliability
Description
setCurrentDirectoryToExecutablePath() ignores GetModuleFileName()’s return value and assumes the
buffer contains a valid, null-terminated executable path; on API failure (or truncation) this can
produce an invalid directory string passed to strrchr()/SetCurrentDirectory(). This risk is now
centralized because all updated entrypoints rely on this helper when -cwd isn’t provided.
Code

Core/GameEngine/Source/Common/CommandLine.cpp[R1439-1442]

+	Char buffer[_MAX_PATH];
+	GetModuleFileName(nullptr, buffer, sizeof(buffer));
+	if (Char *pEnd = strrchr(buffer, '\\'))
+	{
Evidence
The helper added in this PR uses GetModuleFileName without checking its result before manipulating
the buffer and calling SetCurrentDirectory; elsewhere in the repo, GetModuleFileName success is
checked before further processing.

Core/GameEngine/Source/Common/CommandLine.cpp[1437-1446]
Core/Libraries/Source/debug/debug_stack.cpp[73-83]

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

## Issue description
`setCurrentDirectoryToExecutablePath()` calls `GetModuleFileName(...)` into a fixed `_MAX_PATH` buffer and proceeds to parse/use it without validating the returned length or handling the failure case. If `GetModuleFileName` fails (returns 0), the buffer contents are undefined; if the path is longer than the buffer, the resulting string may be unusable (and historically may be non-null-terminated), leading to incorrect `SetCurrentDirectory` behavior.
### Issue Context
Other areas in the repo gate subsequent operations on `GetModuleFileName(...)` success (e.g., debug stack initialization). This helper should similarly validate success and handle failure/truncation safely.
### Fix Focus Areas
- Core/GameEngine/Source/Common/CommandLine.cpp[1437-1446]
### Suggested fix
- Store `DWORD len = GetModuleFileNameA(nullptr, buffer, ARRAY_SIZE(buffer));`.
- If `len == 0`, log and return/fallback (do not call `strrchr` on an undefined buffer).
- If `len >= ARRAY_SIZE(buffer)` (or `len == ARRAY_SIZE(buffer)` depending on your convention), treat as truncated: ensure `buffer[ARRAY_SIZE(buffer)-1] = '\0'`, log, and consider using a dynamically sized buffer approach (loop-resize) if long paths must be supported.
- Check the result of `SetCurrentDirectory(buffer)` and log/fallback on failure.

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


Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread Core/GameEngine/Source/Common/CommandLine.cpp Outdated
Comment thread Core/GameEngine/Source/Common/CommandLine.cpp Outdated

@xezon xezon 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.

Needs cleanup

}
}

static Bool setCurrentDirectoryToExecutablePath()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To me this looks like beyond the responsibility of a command line class. This code should probably be somewhere else.

// TheSuperHackers @feature 14/08/2026
// Working directory is applied earlier by CommandLine::applyStartupWorkingDirectory().
// Consume an optional path argument here so it is not treated as another flag.
if (num > 1 && args[1] != nullptr && args[1][0] != '-')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This does not look well designed. It should do the real thing in here.

Comment thread GeneralsMD/Code/Main/WinMain.cpp Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can probabaly move this higher before the working directory is set?

…Line

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

2 participants