diff --git a/WitcherScriptMerger.Core/CLAUDE.md b/WitcherScriptMerger.Core/CLAUDE.md index 8719d90..9c76ab9 100644 --- a/WitcherScriptMerger.Core/CLAUDE.md +++ b/WitcherScriptMerger.Core/CLAUDE.md @@ -126,6 +126,37 @@ ever going to remain, the interface indirection was deleted as premature abstrac own private `DiffPlexMergeEngine` field directly — there's no engine-selection step at startup in either host anymore. +## Vortex-fork parity fixes (`mods.settings` "VK=" lines, DLC-bundle-folder matching) + +Two small parity gaps versus a separate, Vortex-integrated fork of this project +(`IDCs/WitcherScriptMerger`, the fork Vortex's real `game-witcher3` extension actually +drives) were found by direct comparison and fixed here: + +- **`LoadOrder/CustomLoadOrder.ProcessLine`** now recognizes and ignores a `VK=` + (VortexKey) line instead of falling into the catch-all "unrecognized value" branch. + Vortex's own mod-management integration writes this key into `mods.settings`; without + this, `ProcessLine` returning `false` aborts `Refresh()`'s entire parse loop + (`IsValid` stays `false`, `Mods` stays empty) the moment a Vortex-managed + `mods.settings` is read. Deliberately a narrow, explicit `VK=` check rather than a + generic "tolerate any unrecognized key" change — the catch-all warning is intentional + malformed-file detection, and broadening it to accept-all would remove that + protection for a genuinely broken file. If another mod manager introduces another key + this parser doesn't know, it fails the same way `VK=` used to, by design; fix it the + same way, one recognized key at a time, rather than widening acceptance generically. +- **`Inventory/FileMerger.IsVanillaDlcBundleFolder`** (backing `GetUnpackedFiles`'s + vanilla-bundle lookup) now matches `"bob"` (Blood & Wine's internal DLC folder + codename) in addition to `DLC[0-9]*`/`ep[0-9]`, and matches case-insensitively. The + original regex had no `bob` alternative at all — Blood & Wine bundle-content + conflicts never matched against a vanilla bundle — and was case-sensitive, which + matters because real on-disk folder names vary in casing across different game/mod + installs regardless of platform. Exposed as a public static pure function (mirroring + `DiffPlexMergeEngine.BuildMerge`'s own public/static shape) specifically so it's + directly unit-testable. + +Both are regression-tested in `WitcherScriptMerger.Tests` +(`LoadOrder/CustomLoadOrderTests.cs`, `Inventory/FileMergerTests.cs`) — see +`WitcherScriptMerger.Tests/CLAUDE.md`. + ## CLI & MCP orchestration (`Cli/`, `Mcp/`) `Cli/MergeOperations.cs` is the scan-then-merge sequence shared by both hosts' `merge` diff --git a/WitcherScriptMerger.Core/Inventory/FileMerger.cs b/WitcherScriptMerger.Core/Inventory/FileMerger.cs index e3389cb..4eab37a 100644 --- a/WitcherScriptMerger.Core/Inventory/FileMerger.cs +++ b/WitcherScriptMerger.Core/Inventory/FileMerger.cs @@ -123,6 +123,23 @@ public class MergeReportData bool _bundleChanged; List _pendingBundleMerges = new List(); + static readonly Regex VanillaDlcBundleFolderPattern = + new Regex(@"(DLC[0-9]*|ep[0-9]|bob)$", RegexOptions.IgnoreCase | RegexOptions.Compiled); + + // Matches a DLC-folder name that has its own "bundles" subfolder to search for a + // vanilla bundle: base-game expansions (DLC1, DLC2, ...), the "ep1"/"ep2" xpac + // folders, and "bob" - Blood & Wine's internal folder codename (see this + // project's CLAUDE.md, "Vortex-fork parity fixes" section, for the fork + // comparison this was found against). IgnoreCase because real on-disk folder + // names vary in casing across different game/mod installs - e.g. a repacked or + // differently-sourced "Bob"/"BOB" folder - and .NET's Regex is case-sensitive by + // default regardless of the underlying filesystem, so the prior case-sensitive + // match could silently miss a real vanilla DLC folder on any platform, not just + // a case-sensitive one. Public and static (no instance state involved) + // specifically so it's directly unit testable, matching + // DiffPlexMergeEngine.BuildMerge's own reasoning for the same shape. + public static bool IsVanillaDlcBundleFolder(string path) => VanillaDlcBundleFolderPattern.IsMatch(path); + #endregion public FileMerger(MergeInventory inventory) @@ -714,7 +731,7 @@ bool GetUnpackedFiles(string contentRelativePath, ref MergeSource source1, ref M .Concat( Directory.Exists(Paths.DlcDirectory) ? Directory.GetDirectories(Paths.DlcDirectory) - .Where(path => new Regex("DLC[0-9]*$").IsMatch(path) || new Regex("ep[0-9]$").IsMatch(path)) + .Where(IsVanillaDlcBundleFolder) .Select(path => Path.Combine(path, Paths.BundleBase, "bundles")) : Enumerable.Empty() ) diff --git a/WitcherScriptMerger.Core/LoadOrder/CustomLoadOrder.cs b/WitcherScriptMerger.Core/LoadOrder/CustomLoadOrder.cs index 9428032..90ed7ec 100644 --- a/WitcherScriptMerger.Core/LoadOrder/CustomLoadOrder.cs +++ b/WitcherScriptMerger.Core/LoadOrder/CustomLoadOrder.cs @@ -86,6 +86,15 @@ bool ProcessLine(string line, int lineNum, ref ModLoadSetting setting) if (!ProcessPriorityLine(line, lineNum, setting)) return false; } + else if (line.StartsWith("VK=")) + { + // VortexKey - written into mods.settings by Vortex's own mod-management + // integration (see this project's CLAUDE.md, "Vortex-fork parity fixes" + // section, for the fork comparison this was found against); this parser + // has no use for it, but it's a legitimate line, not a malformed file, so + // it's recognized and ignored rather than falling into the catch-all + // warning below and aborting the whole parse. + } else if (!string.IsNullOrWhiteSpace(line) && !line.StartsWith(";")) { ShowWarningForMalformedFile($"Unrecognized value on line {lineNum}:\n\n{line}"); diff --git a/WitcherScriptMerger.Tests/CLAUDE.md b/WitcherScriptMerger.Tests/CLAUDE.md index d8ca573..7085bfa 100644 --- a/WitcherScriptMerger.Tests/CLAUDE.md +++ b/WitcherScriptMerger.Tests/CLAUDE.md @@ -20,6 +20,12 @@ does. `MergeHeadless_EncodingMismatch_...` fixture reproducing the `baseEffect.ws`-style false conflict that motivated it. - `Tools/HasherTests.cs` — `Hasher`'s xxHash32 output, including synthetic edge cases. +- `Inventory/FileMergerTests.cs` — `FileMerger.IsVanillaDlcBundleFolder`: known vanilla + DLC-folder names, case-insensitivity, and non-matches (including anchoring) — see + Core's `CLAUDE.md`'s "Vortex-fork parity fixes" section. +- `LoadOrder/CustomLoadOrderTests.cs` — `CustomLoadOrder.ProcessLine`'s tolerance for + `mods.settings` "VK=" (VortexKey) lines, via reflection — see Core's `CLAUDE.md`'s + "Vortex-fork parity fixes" section. - `Tools/KDiff3CrossCheckTests.cs` — an auto-solvable-only A/B check of `DiffPlexMergeEngine` against a real `KDiff3.exe` binary, when a developer happens to have one locally (WSM no longer bundles or requires KDiff3 itself — see diff --git a/WitcherScriptMerger.Tests/Inventory/FileMergerTests.cs b/WitcherScriptMerger.Tests/Inventory/FileMergerTests.cs new file mode 100644 index 0000000..8c3e08f --- /dev/null +++ b/WitcherScriptMerger.Tests/Inventory/FileMergerTests.cs @@ -0,0 +1,56 @@ +using WitcherScriptMerger.Inventory; +using Xunit; + +namespace WitcherScriptMerger.Tests.Inventory +{ + // Regression coverage for FileMerger.IsVanillaDlcBundleFolder - the DLC-folder-name + // filter GetUnpackedFiles uses to find a matching vanilla bundle. A different fork of + // this project (github.com/IDCs/WitcherScriptMerger) found and fixed two real gaps + // here that this repo had inherited unmodified from upstream: no "bob" (Blood & Wine's + // internal folder codename) alternative, and a case-sensitive match that only ever + // worked by luck of Windows' case-insensitive filesystem - see + // FileMerger.cs's own comment on VanillaDlcBundleFolderPattern and Core's CLAUDE.md. + public class FileMergerTests + { + [Theory] + [InlineData(@"C:\Witcher3\DLC\DLC1")] + [InlineData(@"C:\Witcher3\DLC\DLC13")] + [InlineData(@"C:\Witcher3\DLC\DLC")] + [InlineData(@"C:\Witcher3\DLC\ep1")] + [InlineData(@"C:\Witcher3\DLC\ep2")] + [InlineData(@"C:\Witcher3\DLC\bob")] + public void IsVanillaDlcBundleFolder_KnownVanillaDlcFolders_ReturnsTrue(string path) + { + Assert.True(FileMerger.IsVanillaDlcBundleFolder(path)); + } + + [Theory] + [InlineData(@"C:\Witcher3\DLC\dlc1")] + [InlineData(@"C:\Witcher3\DLC\Dlc1")] + [InlineData(@"C:\Witcher3\DLC\EP1")] + [InlineData(@"C:\Witcher3\DLC\Ep1")] + [InlineData(@"C:\Witcher3\DLC\BOB")] + [InlineData(@"C:\Witcher3\DLC\Bob")] + public void IsVanillaDlcBundleFolder_DifferentCasing_StillMatches(string path) + { + // The prior implementation used a case-sensitive regex - since .NET's Regex is + // case-sensitive by default regardless of platform, it could silently miss a real + // vanilla DLC folder whose on-disk casing simply differs (e.g. a + // differently-sourced or repacked install), on any platform. + Assert.True(FileMerger.IsVanillaDlcBundleFolder(path)); + } + + [Theory] + [InlineData(@"C:\Witcher3\DLC\some_other_mod")] + [InlineData(@"C:\Witcher3\DLC\bobsleigh")] + [InlineData(@"C:\Witcher3\DLC\episode1")] + [InlineData(@"")] + public void IsVanillaDlcBundleFolder_NonVanillaFolders_ReturnsFalse(string path) + { + // "bobsleigh"/"episode1" specifically confirm the pattern is anchored to the end + // of the path (via "$") rather than matching "bob"/"ep" + a digit as a bare + // substring anywhere earlier in a longer, unrelated folder name. + Assert.False(FileMerger.IsVanillaDlcBundleFolder(path)); + } + } +} diff --git a/WitcherScriptMerger.Tests/LoadOrder/CustomLoadOrderTests.cs b/WitcherScriptMerger.Tests/LoadOrder/CustomLoadOrderTests.cs new file mode 100644 index 0000000..9862c6c --- /dev/null +++ b/WitcherScriptMerger.Tests/LoadOrder/CustomLoadOrderTests.cs @@ -0,0 +1,63 @@ +using System.Reflection; +using System.Runtime.CompilerServices; +using WitcherScriptMerger.LoadOrder; +using Xunit; + +namespace WitcherScriptMerger.Tests.LoadOrder +{ + // Regression coverage for CustomLoadOrder.ProcessLine's handling of "VK=" lines. A + // different fork of this project (github.com/IDCs/WitcherScriptMerger) found that + // Vortex writes "VK=" (VortexKey) lines into mods.settings, which this parser had no + // tolerance for - falling into the catch-all "unrecognized value" branch and aborting + // the entire parse (IsValid stays false, no load order is usable) - see + // CustomLoadOrder.cs's own comment on the "VK=" branch and Core's CLAUDE.md. + // + // ProcessLine is a private instance method invoked via reflection rather than exposed + // publicly - unlike FileMerger.IsVanillaDlcBundleFolder (pure string/regex logic with + // no other coupling), ProcessLine is inherently stateful across a multi-line parse + // (accumulates a ModLoadSetting via `ref`) and CustomLoadOrder's constructor reads a + // real, fixed path under the current user's Documents folder - reflection avoids + // either widening ProcessLine's visibility or refactoring CustomLoadOrder's file-path + // coupling just for this test. The instance itself is created via + // RuntimeHelpers.GetUninitializedObject rather than `new CustomLoadOrder()`, skipping + // the constructor (and its Refresh() call) entirely - ProcessLine touches no instance + // state beyond the `ref` setting parameter, so it needs no initialized instance, and + // skipping construction avoids depending on the test-running machine's real + // mods.settings file, which - unlike the "file doesn't exist" case Refresh() no-ops + // on safely - could be present and locked by a running game/Vortex process on a + // developer machine with a live install, throwing IOException for a reason unrelated + // to what this test actually covers. + public class CustomLoadOrderTests + { + [Fact] + public void ProcessLine_VortexKeyLine_IsRecognizedAndIgnored() + { + var loadOrder = (CustomLoadOrder)RuntimeHelpers.GetUninitializedObject(typeof(CustomLoadOrder)); + var processLine = typeof(CustomLoadOrder).GetMethod("ProcessLine", BindingFlags.NonPublic | BindingFlags.Instance); + + ModLoadSetting setting = null; + object[] args = { "VK=1a2b3c4d", 1, setting }; + var result = (bool)processLine.Invoke(loadOrder, args); + + // A malformed line returns false and aborts the whole parse (see Refresh()) - + // true here confirms "VK=..." is treated as a recognized, ignorable line, not + // as "unrecognized value" like it would have been before this fix. + Assert.True(result); + } + + [Fact] + public void ProcessLine_TrulyUnrecognizedLine_StillFails() + { + // Confirms the VK= fix didn't accidentally widen ProcessLine to silently accept + // everything - a genuinely malformed line must still fail the parse. + var loadOrder = (CustomLoadOrder)RuntimeHelpers.GetUninitializedObject(typeof(CustomLoadOrder)); + var processLine = typeof(CustomLoadOrder).GetMethod("ProcessLine", BindingFlags.NonPublic | BindingFlags.Instance); + + ModLoadSetting setting = null; + object[] args = { "SomethingElse=1a2b3c4d", 1, setting }; + var result = (bool)processLine.Invoke(loadOrder, args); + + Assert.False(result); + } + } +}