feat(system): Add startup working directory options

[CryoTheRenegade]

Summary

• Supersedes feat(system): Add command line option to skip force set of cwd #1445
• Adds -useCwd to keep the inherited working directory and -setCwd <path> to select an explicit startup directory
• Keeps the existing default: without either option, startup uses the executable directory
• Parses the options once through CommandLine::parseCommandLineForStartup() for the games, GUIEdit, WorldBuilder, and MapCacheBuilder
• Records the argument positions consumed during startup parsing and filters those positions before tool-specific parsing

For Visual Studio, add -useCwd to Command Arguments and set Working Directory to the game install path.

To select an explicit directory, pass its path after -setCwd. Quote paths that contain spaces:

-setCwd "C:\Games\Command and Conquer Generals Zero Hour"

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

• xezon: GUIEdit, MapCacheBuilder, and WorldBuilder use the same startup command-line path as the games
• xezon: The working-directory choice is applied directly during startup parsing without a GlobalData middleman flag
• bobtista: Startup uses one command-line parse path, with Win32 directory operations isolated in WorkingDirectory
• OmniBlade: Full location-agnostic data paths remain out of scope; this is a smaller step that supports executables outside the install tree

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. See Win32 DLL search order.
• Win32 file access: Relative paths such as LoadImageA and LoadCursorFromFile search the executable path, then the current working directory, then %PATH%. See OpenFile remarks.
• C runtime functions: Calls such as fopen use the current working directory.

This change was drafted with LLM assistance.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

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

✨ Enhancement 🕐 20-40 Minutes

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]

Code Review by Qodo

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

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

---

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[xezon]

Needs cleanup

[greptile-apps]

Greptile Summary

This PR adds shared startup working-directory handling across the games and supported tools.

• Adds -useCwd and -setCwd <path> startup options while preserving executable-directory fallback.
• Tracks consumed startup arguments so tool-specific parsers can ignore them.
• Integrates startup parsing into Generals, Zero Hour, GUIEdit, WorldBuilder, and MapCacheBuilder.
• Updates startup-directory capture so the first capture attempt is cached.

Confidence Score: 5/5

The PR appears safe to merge with no outstanding actionable findings.

No new correctness or repository-rule violations were established. The earlier startup-capture fallback thread was manually resolved without explanation and does not remain outstanding.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/Common/WorkingDirectory.cpp	Implements cached startup-directory capture and executable, inherited, and custom working-directory selection.
Core/GameEngine/Source/Common/CommandLine.cpp	Adds startup working-directory options and records arguments consumed across parsing phases.
Core/Tools/MapCacheBuilder/Source/WinMain.cpp	Adopts shared startup parsing and filters consumed arguments before tool-specific processing.
Generals/Code/Tools/WorldBuilder/src/WorldBuilder.cpp	Integrates startup parsing and prevents MFC from treating consumed option values as filenames.
GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilder.cpp	Mirrors WorldBuilder startup parsing and consumed-argument filtering for Zero Hour.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Process starts] --> B[Capture inherited working directory]
    B --> C[parseCommandLineForStartup]
    C --> D{Startup option}
    D -->|useCwd| E[Restore inherited directory]
    D -->|setCwd path| F[Set explicit directory]
    D -->|Neither| G[Set executable directory]
    E --> H[Record consumed arguments]
    F --> H
    G --> H
    H --> I[Continue game or tool initialization]
    I --> J[Tool-specific parser skips consumed arguments]

Loading

_{Reviews (14): Last reviewed commit: "refactor(system): Remove startup initial..." | Re-trigger Greptile}

[xezon]

Becomes better, but it is still sloppy.

[xezon]

Needs to unsloppify more

[OmarAglan]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: abdddd99ad

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Accept valid paths beginning with option characters

When the requested working directory is a valid relative directory whose name starts with - (for example, -setCwd -install), this check treats the path as a missing argument and silently falls back to the executable directory; quoting the path does not help because the CRT removes the quotes. Root-relative paths beginning with / are rejected similarly. The parser should distinguish recognized options from path values or provide an escaping mechanism rather than rejecting paths solely by their first character.

Useful? React with 👍 / 👎.

[xezon]

Hmm ok this is unfortunate. So there is some racing for initialization. Can we do it some way without these pragmas?