Add WSM_<key> environment-variable override for AppSettings - #13
Merged
Conversation
Lets a caller like a Vortex extension point WSM at a game/mods directory
(WSM_GameDirectory, WSM_ModsDirectory, etc.) without hand-editing
WitcherScriptMerger.exe.config on disk - the fragile, lock-free pattern
Vortex's existing, unrelated-fork integration uses today. Get/Get<T> check
Environment.GetEnvironmentVariable("WSM_" + key) first, generically for any
key, before falling through to the existing ConfigurationManager-backed
lookup; Set/Save are untouched and still only ever write to App.config.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a generic
WSM_<key>environment-variable override toAppSettings.Get/Get<T>, checked ahead of the existingConfigurationManager-backedApp.config<appSettings>lookup.This is Unit C of a larger effort to build a Vortex extension for WSM. Vortex's own built-in Witcher 3 extension (a different, unrelated fork's integration) configures WSM today by hand-editing
WitcherScriptMerger.exe.configXML directly (<add key="GameDirectory" value="..."/>, etc.) — fragile, with no lock/mutex, and race-prone against a concurrently-running WSM process that also reads that file. This override gives a new Vortex extension (or any other external caller) a clean, read-only alternative: setWSM_GameDirectory/WSM_ModsDirectory/etc. in the process environment before invoking WSM, with zeroApp.configwrites at all.Changes
WitcherScriptMerger.Core/AppSettings.csEnvironmentVariablePrefix = "WSM_"andstatic GetEnvironmentOverride(key)— a side-effect-free lookup (Environment.GetEnvironmentVariable("WSM_" + key)).GetRawValue(key)centralizes the lookup: checks the env override first (returning immediately, before ever touching the cachedConfigurationobject), then falls through to the originalCachedConfig.HasFile/Settings[key].Value/"config file doesn't exist" logic, unchanged.Get<T>andGet(string)both route throughGetRawValueinstead of duplicating the oldCachedConfig.HasFilebranch.Get<T>'s reflection-basedParseconversion is unchanged and applies identically to an env-sourced value as to a config-sourced one — no separate ad-hoc parser.GameDirectory,ModsDirectory,MergedModName,QuickBmsPath,QuickBmsPluginPath,WccLitePath,CheckBundleContents, and anything added later, with zero further code changes.Set/Saveare untouched — still only ever write toApp.config. Added a comment onSetdocumenting a known, accepted asymmetry: if aWSM_<key>override is active,Set+Savestill succeeds butGet/Get<T>keep returning the override afterward. This is existing/expected behavior per the task spec, not something this PR attempts to reconcile.WitcherScriptMerger.Tests/AppSettingsTests.cs(new)GetEnvironmentOverride(unset → null; generic across every real<appSettings>key today plus a hypothetical future one) and forGet/Get<bool>/Get<int>end-to-end with an env override in effect, including an unparsable-value case confirming the existing safe-default (default(T)) behavior is preserved for env-sourced values too.AppSettings.AppSettings's constructor callsEnvironment.Exit(1)if it can't find a config file next to the entry assembly, which underdotnet test'stesthost.dllhost kills the whole test process, not just one test (seeWitcherScriptMerger.Tests/CLAUDE.md's "AppState.Settings-safety constraints"). Instead:GetEnvironmentOverrideis exercised directly — it's static and never touchesCachedConfig/AppState.Get/Get<T>are exercised viaRuntimeHelpers.GetUninitializedObject(typeof(AppSettings)), which skips the constructor. This is safe specifically because, with an env override active,GetRawValuereturns before ever touching the lazily-initializedCachedConfigproperty — confirmed by reading the implementation, not assumed.AppSettings) and the full CLI pipeline. Both are covered by the end-to-end run below instead.Verification
dotnet build WitcherScriptMerger.sln— succeeds, 0 warnings, 0 errors.dotnet test WitcherScriptMerger.sln— 63/63 pass (57 pre-existing + 6 new), including confirming the test process doesn't die (i.e. nothing accidentally forcedAppSettings's real constructor to run).dotnet format whitespace WitcherScriptMerger.sln --verify-no-changes— clean.End-to-end scratch-tree run (proves the override actually works, not just in a unit test): built
WitcherScriptMerger.Headless(its shippedApp.config/.dll.confighasGameDirectory/ModsDirectoryboth blank by default). Created a scratch tree under the OS temp dir:Game\content\content0\scripts\testConflict.ws(vanilla)Mods\mod1000_TestModA\...\testConflict.ws(changes one field)Mods\mod2000_TestModB\...\testConflict.ws(changes a different, independent field)Set only
WSM_GameDirectory/WSM_ModsDirectory(process environment) pointing at that scratch tree — noApp.configedits at all — then ranWitcherScriptMerger.Headless.exe merge. Result:Merged 1 file(s), skipped 0.(exit code 0). Inspected the merged output: it correctly contains both mods' independent edits (proving a real 3-way merge ran against the scratch vanilla/mod files, not whateverApp.configwould otherwise have pointed at), UTF-16LE+BOM encoded matching vanilla. Re-checked the on-disk.dll.configafterward —GameDirectory/ModsDirectorywere still blank, confirming zero config-file writes occurred. Scratch tree removed afterward.Code review
Ran this repo's
/code-reviewskill against the diff before opening this PR. Of 4 findings:WSM_<key>var unconditionally in itsfinallyblock instead of restoring whatever was there before — now captures and restores the original value.Set()has no awareness of an active env override, soSet+Savecan silently have no visible effect onGet/Get<T>if an override is active for that key. Added a comment onSetcalling this out explicitly.QuickBmsPath/WccLitePath(paths fed toProcess.Start), not just directory settings — this is required by the task's explicit "must work for any key, zero per-key code" spec, and carries no more risk than the status quo (anyone who can set process environment variables for WSM can already editApp.configdirectly, which those same paths already come from).Paths.ModsDirectoryinModFile.GetModNameFromPath, out of scope for this PR.AI-assisted development disclosure
This PR was substantially produced by an AI coding agent (Claude Code), per this repo's
CONTRIBUTING.md. Commits carry the requiredCo-Authored-By/Claude-Sessiontrailers.