Skip to content

feat(system): Add startup working directory options - #3149

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

feat(system): Add startup working directory options#3149
CryoTheRenegade wants to merge 10 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:feat/startup-working-directory

Conversation

@CryoTheRenegade

@CryoTheRenegade CryoTheRenegade commented Aug 14, 2026

Copy link
Copy Markdown

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.

… 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 add REVIEW.md to your repo root and Qodo follows it on every PR

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

Comment thread Core/GameEngine/Source/Common/CommandLine.cpp Outdated
Comment thread Core/GameEngine/Source/Common/CommandLine.cpp Outdated
Comment thread GeneralsMD/Code/Main/WinMain.cpp Outdated
…Line

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread GeneralsMD/Code/Tools/GUIEdit/Source/WinMain.cpp Outdated
Comment thread Generals/Code/Main/WinMain.cpp Outdated
@xezon xezon added Enhancement Is new feature or request Minor Severity: Minor < Major < Critical < Blocker System Is Systems related labels Aug 19, 2026
Comment thread Core/GameEngine/Source/Common/WorkingDirectory.cpp Outdated
Have the game and tools share parseCommandLineForStartup so the working directory is applied in one place, without a GlobalData flag or a second tokenizer.

Co-authored-by: Cursor <cursoragent@cursor.com>
@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown

Greptile Summary

The PR centralizes startup command-line parsing and adds configurable working-directory behavior across both games and their main development tools.

  • Adds -useCwd and -setCwd <path>, retaining executable-directory startup as the default.
  • Records startup-consumed argument positions so tool-specific parsers can ignore those options.
  • Integrates the shared startup path into Generals, Zero Hour, GUIEdit, WorldBuilder, and MapCacheBuilder.
  • Adds shared Win32 working-directory helpers and updates GlobalData startup ownership.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope.

No blocking failure remains.

Important Files Changed

Filename Overview
Core/GameEngine/Source/Common/CommandLine.cpp Adds working-directory options, centralizes parsing on CRT arguments, and records startup-consumed positions for downstream tool parsers.
Core/GameEngine/Source/Common/WorkingDirectory.cpp Implements Win32 helpers for selecting either the executable directory or an explicitly supplied directory.
Core/Tools/MapCacheBuilder/Source/WinMain.cpp Routes startup through the shared parser and filters consumed startup options before treating remaining arguments as maps.
Generals/Code/Main/WinMain.cpp Moves shared startup parsing to the former executable-directory setup point.
GeneralsMD/Code/Main/WinMain.cpp Applies the same shared startup and working-directory flow to Zero Hour.
Generals/Code/Tools/WorldBuilder/src/WorldBuilder.cpp Integrates startup parsing and excludes already-consumed arguments from WorldBuilder-specific parsing.
GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilder.cpp Mirrors the WorldBuilder startup integration for the Zero Hour variant.
Generals/Code/Tools/GUIEdit/Source/WinMain.cpp Initializes shared startup state before GUIEdit creates and initializes its engine services.
GeneralsMD/Code/Tools/GUIEdit/Source/WinMain.cpp Mirrors the shared GUIEdit startup path for Zero Hour.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Start[Process startup] --> Parse[Parse shared startup arguments]
    Parse --> Choice{Working-directory option}
    Choice -->|setCwd path| Explicit[Use explicit directory]
    Choice -->|useCwd| Inherited[Keep inherited directory]
    Choice -->|Neither| Executable[Use executable directory]
    Explicit --> Record[Record consumed argument positions]
    Inherited --> Record
    Executable --> Record
    Record --> App{Application}
    App --> Game[Generals or Zero Hour startup]
    App --> Tool[GUIEdit, WorldBuilder, or MapCacheBuilder]
    Tool --> Filter[Filter startup-consumed arguments]
    Filter --> ToolParse[Run tool-specific parsing]
Loading

Reviews (7): Last reviewed commit: "fix(system): Use CRT command-line argume..." | Re-trigger Greptile

CryoTheRenegade and others added 2 commits August 26, 2026 12:27
VC6 does not support in-class member initializers, which broke the WorldBuilder command-line parser on CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread Core/GameEngine/Source/Common/CommandLine.cpp Outdated
Comment thread GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilder.cpp
Comment thread Generals/Code/Tools/WorldBuilder/src/WorldBuilder.cpp Outdated
@CryoTheRenegade CryoTheRenegade changed the title feat(system): Add -cwd option to keep or override the startup working directory feat(system): Add startup working directory options Sep 2, 2026
Comment thread Core/GameEngine/Source/Common/CommandLine.cpp Outdated
Comment thread Core/GameEngine/Source/Common/CommandLine.cpp Outdated
Comment thread Generals/Code/Tools/WorldBuilder/src/WorldBuilder.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.

Becomes better, but it is still sloppy.

static std::vector<Bool> s_startupParsedArguments;

static void parseCommandLine(
const CommandLineParam* params, int numParams, std::vector<Bool> *parsedArguments = nullptr)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

One line

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

int CommandLine::getStartupWorkingDirectoryOptionTokenCount(const char *arg)
bool CommandLine::isCommandLineArgumentParsedForStartup(int argIndex)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this need to be Startup specific? Maybe just make it cover all of them?

char * str = strtrim(token);
const int cwdTokenCount = CommandLine::getStartupWorkingDirectoryOptionTokenCount(str);
if (cwdTokenCount > 0)
if (!CommandLine::isCommandLineArgumentParsedForStartup(argIndex))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

wasCommandLineArgumentParsed...

{
if (CommandLine::isCommandLineArgumentParsedForStartup(m_argIndex++))
{
ParseLast(bLast);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What does this do?

constexpr const Int SIMULATE_REPLAYS_SEQUENTIAL = -1;

//-------------------------------------------------------------------------------------------------
class CommandLineData

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

While at it, add a comment above this class explaining why it is here instead of in CommandLine (The parsing result is bound to GlobalData)


// Set when a working-directory option is present so parseCommandLineForStartup
// does not force the executable directory after parsing.
static Bool s_cwdOptionSpecified = FALSE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Better move this state flag to WorkingDirectory, whether a working directory was already set, making it less dependent on CommandLine.

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

Labels

Enhancement Is new feature or request Minor Severity: Minor < Major < Critical < Blocker System Is Systems related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants