From 19756db952e74ede54c0a20556a959fc297546ea Mon Sep 17 00:00:00 2001 From: Chris Knight Date: Fri, 21 Aug 2026 23:15:01 -0400 Subject: [PATCH] Read and emit the WitcherScriptMerger.exe.config Vortex configures Vortex's bundled game-witcher3 extension hardcodes the .NET Framework ".exe.config" name for the script merger's config (scriptmerger.ts's MERGER_CONFIG_FILE). It parses that file for MergedModName (getMergedModName) and writes GameDirectory, VanillaScriptsDirectory and ModsDirectory into it when configuring a merger install (setMergerConfig). A modern .NET app's own config is ".dll.config", so the two never met: Vortex wrote a file WSM never read, and a user who "configured WSM through Vortex" changed nothing at all. Observed on a real install: Vortex logs [game-witcher] failed to ascertain merged mod name - using "mod0000_MergedFiles" {"code":"ENOENT","path":"...\The Witcher 3\WitcherScriptMerger\WitcherScriptMerger.exe.config"} and falls back to a hardcoded guess. The same mergerToolDir is also where it moves MergeInventory.xml on profile store/restore, so the mis-resolution reaches further than the mod name. Two halves: - The WinForms csproj now emits WitcherScriptMerger.exe.config beside the usual .dll.config, at build and at publish, and only when absent - Vortex owns that file once it has written to it, so rebuilding must not discard the paths it configured. Two targets rather than one with AfterTargets="Build;Publish": the SDK defines $(PublishDir) unconditionally, so a single "PublishDir if set, else OutDir" target wrote to the publish folder during an ordinary build and left the build output without the file. - AppSettings.GetRawValue now falls back to that sidecar when a key is blank in our own config. Emitting a file nothing reads would just be a subtler version of the same disconnect - Vortex's writes have to actually take effect. Precedence is WSM_ env var, then our own non-blank config value, then the sidecar. A fallback rather than an override, deliberately: a non-blank value in our own config is an explicit choice (the GUI writes there via Set/Save, and Vortex never writes MergedModName), while the three keys Vortex does write are exactly the ones that ship blank meaning "derive from the working directory". Settings[key] is now dereferenced with ?. so a key present only in the sidecar resolves instead of throwing first - observably identical to the old behavior for Get/Get, which caught that exception anyway. ParseAppSettingValue is a pure static over the file's text so it is unit-testable without a filesystem, an AppSettings instance, or AppState; it returns null for anything it cannot confidently read, and the read is cached and never throws or prompts, since it runs inside every settings read including scan paths. 19 new tests (192 total). Verified end-to-end against a scratch game tree with the WinForms build: with our own config blank and no env vars, a sidecar written the way setMergerConfig writes it took effect (merge succeeded, exit 0) where the same run without it failed with "Can't find Mods directory" (exit 1); a WSM_ModsDirectory override and a non-blank ModsDirectory in our own config each correctly beat the sidecar. dotnet format clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FP8H6rBLCGPBFRSVsF3Kgw --- WitcherScriptMerger.Core/AppSettings.cs | 97 ++++++++++++++++++- WitcherScriptMerger.Core/CLAUDE.md | 40 ++++++++ WitcherScriptMerger.Tests/AppSettingsTests.cs | 91 +++++++++++++++++ WitcherScriptMerger.Tests/CLAUDE.md | 8 ++ WitcherScriptMerger/CLAUDE.md | 12 ++- .../WitcherScriptMerger.csproj | 35 +++++++ 6 files changed, 279 insertions(+), 4 deletions(-) diff --git a/WitcherScriptMerger.Core/AppSettings.cs b/WitcherScriptMerger.Core/AppSettings.cs index 97fab67..c0d56cb 100644 --- a/WitcherScriptMerger.Core/AppSettings.cs +++ b/WitcherScriptMerger.Core/AppSettings.cs @@ -1,6 +1,9 @@ using System; using System.Configuration; +using System.IO; +using System.Linq; using System.Reflection; +using System.Xml.Linq; namespace WitcherScriptMerger { @@ -52,8 +55,9 @@ public static string GetEnvironmentOverride(string key) } // Resolves a key's raw string value: an environment-variable override first, then - // falling through to the existing ConfigurationManager-backed lookup. Both Get and - // Get route through this single place so an env-var-sourced value goes through + // the existing ConfigurationManager-backed lookup, then - only when that yields + // nothing usable - the Vortex-managed sidecar (see VortexSidecarFileName). Both Get + // and Get route through this single place so a value from any source goes through // the exact same downstream handling (Get's Parse-based conversion in particular) // as one read from App.config - never a separate ad-hoc parser. string GetRawValue(string key) @@ -63,12 +67,99 @@ string GetRawValue(string key) return envValue; if (CachedConfig.HasFile) - return CachedConfig.AppSettings.Settings[key].Value; + { + // Null-conditional, not a bare .Value: Settings[key] returns null for a key + // that isn't in App.config at all, which used to throw here and get swallowed + // by Get/Get's catch. Returning null instead is observably identical for + // both of those (empty string / default(T)), and it lets a key that exists + // ONLY in the sidecar still be found below rather than dying first. + var value = CachedConfig.AppSettings.Settings[key]?.Value; + if (!string.IsNullOrWhiteSpace(value)) + return value; + + // Blank in our own config. For the path settings that ship blank on purpose + // (GameDirectory/ModsDirectory/VanillaScriptsDirectory - blank means "derive + // from the working directory"), a Vortex-written sidecar value is strictly + // better information than deriving, so prefer it when there is one. + return ReadVortexSidecarSetting(key) ?? value; + } AppState.Notifier.ShowError($"Config file doesn't exist:\n\n{CachedConfig.FilePath}"); return null; } + // Vortex's bundled game-witcher3 extension reads AND writes a script-merger config + // at "\WitcherScriptMerger.exe.config" - the .NET Framework naming + // convention this project used before the .NET 10 modernization. It parses that file + // for MergedModName (scriptmerger.ts::getMergedModName) and writes GameDirectory, + // VanillaScriptsDirectory and ModsDirectory into it (scriptmerger.ts::setMergerConfig) + // when it configures a merger install. A modern .NET app's own configuration is + // ".dll.config" instead, so without this the two never meet: Vortex writes + // a file WSM never reads, and the user "configures WSM through Vortex" with no + // effect at all. + // + // Reading it as a *fallback* rather than an override is deliberate. A non-blank + // value in our own config is an explicit choice (the GUI's own settings screen + // writes there via Set/Save, and Vortex never writes MergedModName), so it must + // win; the sidecar only fills in what we'd otherwise have to guess. Env overrides + // still beat both, unchanged. + public const string VortexSidecarFileName = "WitcherScriptMerger.exe.config"; + + string _sidecarPath; + bool _sidecarChecked; + string _sidecarXml; + + string ReadVortexSidecarSetting(string key) + { + if (!_sidecarChecked) + { + _sidecarChecked = true; + try + { + _sidecarPath = Path.Combine(Path.GetDirectoryName(_assemblyPath) ?? string.Empty, VortexSidecarFileName); + if (File.Exists(_sidecarPath)) + _sidecarXml = File.ReadAllText(_sidecarPath); + } + catch + { + // Unreadable/inaccessible sidecar is not an error - it's an optional + // interop file that usually isn't there at all. Never prompt, never + // throw: this runs inside every settings read, including on scan paths. + _sidecarXml = null; + } + } + + return _sidecarXml == null ? null : ParseAppSettingValue(_sidecarXml, key); + } + + // Split out as a pure string-in/string-out function so the sidecar parsing is + // directly unit-testable without a filesystem, a live AppSettings instance, or + // AppState - see WitcherScriptMerger.Tests/CLAUDE.md's "AppState.Settings-safety + // constraints". Returns null for anything it can't confidently read (malformed XML, + // missing key, blank value), so every caller falls through to its existing + // behavior rather than acting on a half-parsed file. + public static string ParseAppSettingValue(string xml, string key) + { + if (string.IsNullOrWhiteSpace(xml) || string.IsNullOrWhiteSpace(key)) + return null; + + try + { + var value = XDocument.Parse(xml) + .Root?.Elements("appSettings") + .Elements("add") + .Where(e => string.Equals((string)e.Attribute("key"), key, StringComparison.Ordinal)) + .Select(e => (string)e.Attribute("value")) + .FirstOrDefault(); + + return string.IsNullOrWhiteSpace(value) ? null : value; + } + catch + { + return null; + } + } + // Deliberately unaware of GetEnvironmentOverride: this still only ever writes to // CachedConfig/App.config, same as before the env-var override existed. If a // WSM_ override is active for a key this call targets, Get/Get keep diff --git a/WitcherScriptMerger.Core/CLAUDE.md b/WitcherScriptMerger.Core/CLAUDE.md index 619cb2c..a449cc9 100644 --- a/WitcherScriptMerger.Core/CLAUDE.md +++ b/WitcherScriptMerger.Core/CLAUDE.md @@ -602,3 +602,43 @@ the synthetic-edge-cases + real-recorded-hash cross-check pattern described in - Merge history: `MergeInventory.xml`, via `XmlSerializer` (`Inventory/MergeInventory.cs`). - Game load order: `LoadOrder/CustomLoadOrder.cs` reads the game's own `mods.settings` file. + +### Vortex-managed sidecar config (`WitcherScriptMerger.exe.config`) + +`GetRawValue` resolves a key in three steps, first non-blank wins: + +1. `WSM_` environment variable (`GetEnvironmentOverride`). +2. Our own config — `.dll.config`, via `ConfigurationManager`. +3. **The Vortex-managed sidecar**, `AppSettings.VortexSidecarFileName` + (`WitcherScriptMerger.exe.config`) beside the entry assembly. + +Step 3 exists because Vortex's bundled `game-witcher3` extension both **reads and writes** +a script-merger config under the .NET Framework `.exe.config` name this project +stopped using at the .NET 10 modernization. It parses that file for `MergedModName` +(`scriptmerger.ts::getMergedModName`) and writes `GameDirectory`, +`VanillaScriptsDirectory` and `ModsDirectory` into it when configuring a merger install +(`scriptmerger.ts::setMergerConfig`). Without this fallback the two never meet: Vortex +writes a file WSM never reads, so a user who "configures WSM through Vortex" changes +nothing at all — and Vortex logs `failed to ascertain merged mod name - using +"mod0000_MergedFiles"` and silently falls back to a hardcoded guess. + +**A fallback, not an override**, deliberately: a non-blank value in our own config is an +explicit choice (the GUI's settings screen writes there via `Set`/`Save`, and Vortex never +writes `MergedModName`), so it must win. The sidecar only fills in what we would otherwise +have to derive — which is exactly the shape of the three keys Vortex writes, since +`GameDirectory`/`ModsDirectory`/`VanillaScriptsDirectory` all ship blank meaning "derive +from the working directory". Env overrides still beat both. + +`ParseAppSettingValue(xml, key)` is a pure static over the file's text — no filesystem, no +`AppState` — so it's directly unit-testable (`AppSettingsTests`); it returns `null` for +anything it can't confidently read (malformed XML, missing key, blank value) so every +caller falls through to existing behavior instead of acting on a half-parsed file. The +read is cached after the first attempt and never throws or prompts: it runs inside every +settings read, including scan paths where an exception would surface as a merge failure. +`Settings[key]` is dereferenced with `?.` so a key present *only* in the sidecar still +resolves rather than throwing first. + +The WinForms host's csproj emits this file at build and publish (never overwriting an +existing one — Vortex owns it once written); see `WitcherScriptMerger/CLAUDE.md`. +`WitcherScriptMerger.Headless` deliberately does not, since Vortex's extension only ever +looks for a merger named `WitcherScriptMerger.exe`. diff --git a/WitcherScriptMerger.Tests/AppSettingsTests.cs b/WitcherScriptMerger.Tests/AppSettingsTests.cs index 3d67fb2..cc4e971 100644 --- a/WitcherScriptMerger.Tests/AppSettingsTests.cs +++ b/WitcherScriptMerger.Tests/AppSettingsTests.cs @@ -149,5 +149,96 @@ static void WithEnvironmentVariable(string key, string value, Action action) Environment.SetEnvironmentVariable(envVarName, originalValue); } } + + #region Vortex sidecar config (WitcherScriptMerger.exe.config) + + // Coverage for AppSettings.ParseAppSettingValue, the parser behind the + // Vortex-managed sidecar GetRawValue falls back to when our own config leaves a key + // blank - see AppSettings.cs's own comment on VortexSidecarFileName for why that + // file exists (Vortex's bundled game-witcher3 extension reads MergedModName from it + // and writes GameDirectory/VanillaScriptsDirectory/ModsDirectory into it, under the + // .NET Framework ".exe.config" name a modern .NET app doesn't use). + // + // Exercised as a pure static over an XML string: no filesystem, no AppSettings + // instance, no AppState - see WitcherScriptMerger.Tests/CLAUDE.md's + // "AppState.Settings-safety constraints". + const string SidecarXml = @" + + + + + + + +"; + + [Theory] + [InlineData("GameDirectory", @"G:\Games\Witcher3")] + [InlineData("ModsDirectory", @"G:\Games\Witcher3\mods")] + [InlineData("MergedModName", "mod0000_MergedFiles")] + public void ParseAppSettingValue_KeyPresent_ReturnsItsValue(string key, string expected) + { + Assert.Equal(expected, AppSettings.ParseAppSettingValue(SidecarXml, key)); + } + + // Null, never string.Empty, for anything unusable - GetRawValue's `?? value` fallback + // relies on that to fall through to its own (blank) config value rather than + // treating a blank sidecar entry as an answer. + [Theory] + [InlineData("NotInTheFile")] + [InlineData("BlankOne")] + public void ParseAppSettingValue_MissingOrBlankValue_ReturnsNull(string key) + { + Assert.Null(AppSettings.ParseAppSettingValue(SidecarXml, key)); + } + + // Matching is case-sensitive, matching ConfigurationManager's own + // behavior - "gamedirectory" must not resolve "GameDirectory". + [Fact] + public void ParseAppSettingValue_KeyCaseDiffers_ReturnsNull() + { + Assert.Null(AppSettings.ParseAppSettingValue(SidecarXml, "gamedirectory")); + } + + // A malformed/truncated sidecar (Vortex interrupted mid-write, say) must degrade to + // "no answer" rather than throwing: this parser runs inside every settings read, + // including on scan paths where an exception would surface as a merge failure. + [Theory] + [InlineData("")] + [InlineData("not xml at all")] + [InlineData("")] + [InlineData("")] + public void ParseAppSettingValue_MalformedOrEmptyXml_ReturnsNullWithoutThrowing(string xml) + { + Assert.Null(AppSettings.ParseAppSettingValue(xml, "GameDirectory")); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void ParseAppSettingValue_NoXml_ReturnsNull(string xml) + { + Assert.Null(AppSettings.ParseAppSettingValue(xml, "GameDirectory")); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void ParseAppSettingValue_NoKey_ReturnsNull(string key) + { + Assert.Null(AppSettings.ParseAppSettingValue(SidecarXml, key)); + } + + // The file name is the interop contract with Vortex's hardcoded + // scriptmerger.ts::MERGER_CONFIG_FILE - it is not ours to rename. + [Fact] + public void VortexSidecarFileName_MatchesTheNameVortexLooksFor() + { + Assert.Equal("WitcherScriptMerger.exe.config", AppSettings.VortexSidecarFileName); + } + + #endregion } } diff --git a/WitcherScriptMerger.Tests/CLAUDE.md b/WitcherScriptMerger.Tests/CLAUDE.md index bd1bd0f..0c0e6e8 100644 --- a/WitcherScriptMerger.Tests/CLAUDE.md +++ b/WitcherScriptMerger.Tests/CLAUDE.md @@ -64,6 +64,14 @@ does. `DiffPlexMergeEngine` against a real `KDiff3.exe` binary, when a developer happens to have one locally (WSM no longer bundles or requires KDiff3 itself — see `docs/decisions/kdiff3-retirement.md`). +- `AppSettingsTests.cs` — `AppSettings`'s `WSM_` environment-variable override, and + (added alongside the Vortex sidecar interop) `AppSettings.ParseAppSettingValue`: the pure + parser behind the `WitcherScriptMerger.exe.config` fallback Vortex's bundled + `game-witcher3` extension reads and writes. Covers key lookup, case-sensitivity, a blank + value and a missing key both yielding `null` (which is what makes `GetRawValue`'s `??` + fall through correctly), malformed/truncated/empty XML degrading to `null` rather than + throwing, null/blank inputs, and that `VortexSidecarFileName` still matches the name + Vortex hardcodes — see Core's `CLAUDE.md`'s "Vortex-managed sidecar config" section. - `LiveInstall.cs` — see "Live-install cross-check tests" below. ## `AppState.Settings`-safety constraints diff --git a/WitcherScriptMerger/CLAUDE.md b/WitcherScriptMerger/CLAUDE.md index e846f20..8b23156 100644 --- a/WitcherScriptMerger/CLAUDE.md +++ b/WitcherScriptMerger/CLAUDE.md @@ -37,7 +37,17 @@ merges via `InteractiveMergeRunner`, and wires up async callbacks. The publish's `.dll.config` (the `App.config` copy `System.Configuration.ConfigurationManager` actually reads, via Core's `AppSettings.cs`) lands next to the executable — copy it there if deploying the exe on - its own. This resolves correctly even in a single-file publish despite + its own. A second copy, `WitcherScriptMerger.exe.config`, lands beside it via the + `EmitVortexCompatConfigForBuild`/`EmitVortexCompatConfigForPublish` targets in this + project's csproj — that's the .NET Framework name Vortex's bundled `game-witcher3` + extension hardcodes and both reads and writes (see Core's `CLAUDE.md`'s "Vortex-managed + sidecar config"). It is only written when absent: Vortex owns that file once it has + configured a merger install, and clobbering it on every rebuild would throw away the + paths it wrote. Two separate targets rather than one with + `AfterTargets="Build;Publish"` because the SDK defines `$(PublishDir)` + unconditionally (defaulting to `$(OutDir)publish\`), so a single target choosing + "PublishDir if set, else OutDir" silently wrote to the publish folder during an ordinary + build and left the build output without the file. This resolves correctly even in a single-file publish despite `Assembly.GetEntryAssembly().Location` being documented (and confirmed via a real build's `IL3000` warning) to always return `""` for a single-file-bundled assembly — `ConfigurationManager.OpenExeConfiguration("")` still finds and reads the real diff --git a/WitcherScriptMerger/WitcherScriptMerger.csproj b/WitcherScriptMerger/WitcherScriptMerger.csproj index ca170db..0aa5744 100644 --- a/WitcherScriptMerger/WitcherScriptMerger.csproj +++ b/WitcherScriptMerger/WitcherScriptMerger.csproj @@ -28,4 +28,39 @@ + + + + + + + + +