diff --git a/CLAUDE.md b/CLAUDE.md index 8b59c35..0a94fa2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,18 +14,22 @@ This is a fork of the upstream `AnotherSymbiote/WitcherScriptMerger` repo (still - Run (GUI, no args): launch the built `WitcherScriptMerger.exe`, or `dotnet run --project WitcherScriptMerger/WitcherScriptMerger.csproj`. At startup the app validates `KDiff3Path`/`QuickBmsPath`/`QuickBmsPluginPath`/`WccLitePath` from `App.config` (`Paths.ValidateDependencyPaths` in `WitcherScriptMerger/Paths.cs`) and shows a blocking `DependencyForm` if any are missing — the external binaries (KDiff3, QuickBMS, wcc_lite) are **not** in source control (see "External tool dependencies" below), so a fresh checkout won't run end-to-end without sourcing them separately. - Run (CLI, headless): `WitcherScriptMerger.exe merge [--order-file ]` — see "CLI mode" below. Any arguments at all route to the CLI path instead of the GUI. - Run (MCP server): `WitcherScriptMerger.exe mcp` — see "MCP mode" below. Speaks MCP over stdio; not meant to be run interactively from a terminal. -- Two projects, one `.sln`: `WitcherScriptMerger.Core` (WinForms-free class library, `net10.0`) and `WitcherScriptMerger` (the WinForms host — GUI + CLI + MCP entry points, `net10.0-windows7.0`, references Core). See "Architecture" below for what lives where. +- Three projects, one `.sln`: `WitcherScriptMerger.Core` (WinForms-free class library, `net10.0`), `WitcherScriptMerger` (the WinForms host — GUI + CLI + MCP entry points, `net10.0-windows7.0`, references Core), and `WitcherScriptMerger.Tests` (xunit, `net10.0`, references Core only). See "Architecture" below for what lives where. ### Tests -There is no test project in this repo (`dotnet test` has nothing to run). The precedent set in `HANDOFF.md` for verifying logic changes — especially anything touching hash output, `MergeInventory.xml` schema, or KDiff3 invocation — is a disposable, non-committed `dotnet new console` scratch app: exercise synthetic edge cases plus a cross-check against a real value already recorded in a live `MergeInventory.xml`. Follow that pattern rather than assuming API docs alone are sufficient, particularly for anything hash- or serialization-related (see "Compatibility constraints" below). +`WitcherScriptMerger.Tests` (xunit) covers `WitcherScriptMerger.Core` — run with `dotnet test WitcherScriptMerger.Tests/WitcherScriptMerger.Tests.csproj` (or `dotnet test WitcherScriptMerger.sln`). It does **not** reference the host project (no WinForms, no `Tools/KDiff3.cs`), so `KDiff3MergeEngine`/`KDiff3.cs` still have no automated coverage — only `DiffPlexMergeEngine` and other Core-side logic (`Hasher`, `FileEncoding`) do. +- Tests never construct `FileMerger.MergeSource` via `MergeSource.FromFlatFile`/`FromBundle` (both call `ModFile.GetModNameFromPath` → `Paths.ModsDirectory` → `AppState.Settings`) or otherwise force `AppState.Settings` to construct outside a real GUI/CLI/MCP entry point: `AppSettings`'s constructor calls `Environment.Exit(1)` if it can't find a config file next to `Assembly.GetEntryAssembly().Location`, and in a `dotnet test` host (`testhost.dll`, no matching `.config`) that kills the entire test process, not just one test. `AppState.Settings` is a lazy property specifically so that merely touching `AppState.Notifier` (which Core code — e.g. `DiffPlexMergeEngine`'s headless skip/guard messages — legitimately does on its own) doesn't also force `Settings` to construct; see `AppState.cs` and "Startup flow" below. `Paths.cs`'s own properties (`ScriptsDirectory`/`ModsDirectory`/`IsScriptsDirectoryDerived`/`IsModsDirectoryDerived`) read `AppState.Settings.Get(...)` on every access rather than caching the result via a static field initializer, for the identical reason one layer further out: a field initializer there would've forced `Settings` to construct merely from touching an unrelated static member of `Paths` (e.g. `GetRelativePath`), via C#'s beforefieldinit semantics, which would've silently undermined `AppState.Settings`' own laziness. `Tools/DiffPlexMergeEngine.GetConflictMarkerPath` reads only the compile-time-literal `Paths.DiffPlexConflictsDirectory` const, which never triggers `Paths`' type initializer at all, so it's safe to call from tests unconditionally. Tests that need a `FileMerger.MergeSource` build it directly via object-initializer syntax instead (its fields are all public). +- A few tests optionally cross-check against a real Witcher 3 + WitcherScriptMerger install (a live `MergeInventory.xml`'s recorded hashes, or the real `KDiff3.exe` binary for an auto-solvable-only A/B check against `DiffPlexMergeEngine`) via `WitcherScriptMerger.Tests/LiveInstall.cs`, gated entirely on the `WSM_TEST_GAME_DIR` environment variable (unset by default) — never a hardcoded or scanned path, per this repo's "scrub machine-specific paths" rule (see `CONTRIBUTING.md`). They silently no-op when it's unset. +- For anything not covered by the test project — especially further hash-output, `MergeInventory.xml` schema, or KDiff3-invocation changes — the precedent set in `HANDOFF.md` still applies: a disposable, non-committed `dotnet new console` scratch app, exercising synthetic edge cases plus a cross-check against a real value already recorded in a live `MergeInventory.xml`. Follow that pattern rather than assuming API docs alone are sufficient, particularly for anything hash- or serialization-related (see "Compatibility constraints" below). ## Architecture -Two projects as of the Core/host split (see git history around "Split into WitcherScriptMerger.Core + host GUI/CLI/MCP project" for the full rationale): +Three projects as of the Core/host split plus the later addition of a test project (see git history around "Split into WitcherScriptMerger.Core + host GUI/CLI/MCP project" for the Core/host rationale): - **`WitcherScriptMerger.Core`** (`WitcherScriptMerger.Core/WitcherScriptMerger.Core.csproj`, `net10.0`, no WinForms reference — deliberately cross-platform-capable, toward eventual Linux support) holds all domain logic: file scanning, merge orchestration, load-order handling, settings/paths, and the CLI/MCP entry-point logic. Nothing in Core references `System.Windows.Forms`. - **`WitcherScriptMerger`** (`WitcherScriptMerger/WitcherScriptMerger.csproj`, `net10.0-windows7.0`, `WinExe`, `UseWindowsForms=true`) is the host: WinForms GUI (`Forms/`, `Controls/`), the three entry points (`Program.cs`), and the one remaining external-tool wrapper with Win32 P/Invoke (`Tools/KDiff3.cs`). References Core via `ProjectReference`. +- **`WitcherScriptMerger.Tests`** (`WitcherScriptMerger.Tests/WitcherScriptMerger.Tests.csproj`, xunit, `net10.0`) references Core only — see "Tests" above for what it covers and its `AppState.Settings`-safety constraints. There is no MVC/MVP split within the host — `Forms/MainForm.cs` (~1000 lines) is a monolithic orchestrator that directly owns the tree controls, constructs `ModFileIndex`/drives merges, and wires up async callbacks. @@ -35,7 +39,7 @@ Folder map — **Core**: - `FileIndex/` — scans the mods folder and builds the conflict index: `ModFileIndex.cs` (`BuildAsync` → `Conflicts`), `ModFile.cs`, `ModFileCategory.cs`. - `Inventory/` — core merge domain + persistence: `FileMerger.cs` (headless orchestration plus TreeNode-free interactive orchestration — see "Interactive vs. headless split" below), `Merge.cs`, `MergeInventory.cs`, `FileHash.cs`, `MergeProgressInfo.cs`. - `LoadOrder/` — mod load-order logic: `CustomLoadOrder.cs`, `LoadOrderComparer.cs`, `LoadOrderValidator.cs`, `ModLoadSetting.cs`. -- `Tools/` — wrappers that shell out to bundled external executables, plus the merge-engine abstraction: `QuickBms.cs`, `WccLite.cs`, `Hasher.cs`, `IMergeEngine.cs` (see "Interactive vs. headless split" below). +- `Tools/` — wrappers that shell out to bundled external executables, plus the merge-engine abstraction: `QuickBms.cs`, `WccLite.cs`, `Hasher.cs`, `IMergeEngine.cs` (see "Interactive vs. headless split" below), `FileEncoding.cs` (UTF-16LE+BOM normalization shared by every merge engine — see "KDiff3 input encoding" under Compatibility constraints), `DiffPlexMergeEngine.cs` (the DiffPlex-based `IMergeEngine` implementation — see "Interactive vs. headless split" below). - `Cli/` — `MergeOperations.cs`: the scan-then-merge sequence shared by the `merge` CLI verb and the MCP tools. - `Mcp/` — `WsmMcpTools.cs`: the MCP server's tool implementations. See "MCP mode" below. - Root: `AppState.cs` (shared mutable state — `Notifier`/`Settings`/`LoadOrder`/`Inventory`/`MergeEngine`), `AppSettings.cs`, `Paths.cs`, `StringExtensions.cs`, `IMergeNotifier.cs`, `NotifyTypes.cs` (the neutral `NotifyResult`/`NotifyButtons`/`DialogIcon` enums), `HeadlessMergeNotifier.cs`. @@ -44,20 +48,24 @@ Folder map — **host**: - `Forms/` — WinForms screens: `MainForm.cs` (the hub, also implements `IMergeNotifier`, translating to/from real WinForms types), `OptionsForm.cs`, `DependencyForm.cs` (startup blocker if tool paths are invalid), `MergeReportForm.cs`, `PackReportForm.cs`, `PriorityPrompt.cs`, `MessageBoxManager.cs`. - `Controls/` — custom `TreeView` subclasses: `SMTree.cs` (base, metadata/context-menu logic), `ConflictTree.cs` (detected conflicts), `MergeTree.cs` (existing merges), `SMTreeSorter.cs`, `ToolStripRegion.cs`. - `Inventory/` — `InteractiveMergeRunner.cs`: the host-side counterpart to Core's `FileMerger` for the interactive path (see "Interactive vs. headless split" below). -- `Tools/` — `KDiff3.cs` (Win32 P/Invoke for window-title polling — stays host-only for now; a later unit removes it entirely), `KDiff3MergeEngine.cs` (the one real `IMergeEngine` implementation). +- `Tools/` — `KDiff3.cs` (Win32 P/Invoke for window-title polling — stays host-only for now; a later unit removes it entirely), `KDiff3MergeEngine.cs` (the one real `IMergeEngine` implementation using KDiff3 — `DiffPlexMergeEngine`, Core, is the other one; see "Interactive vs. headless split" below for both). - Root: `Program.cs` (entry point: GUI, CLI, and MCP), `Extensions.cs` (WinForms-specific `TreeNode`/`TreeView` helpers and Win32 P/Invoke — pure string helpers live in Core's `StringExtensions.cs` instead), `TaskbarProgress.cs`, `App.config`. ### Interactive vs. headless split (`FileMerger` / `IMergeEngine`) Core's `FileMerger` never sees a `TreeNode`, `BackgroundWorker`, or `Forms.*` type. Its headless methods (`MergeConflictsHeadless` et al.) are unchanged in shape from before the split. Its interactive methods (`MergeFilesInteractive`, `MergeFlatFileInteractive`, `MergeBundleFileInteractive`, `MergeTextInteractive`) take a plain `InteractiveMergeRequest` (relative path, bundle flag, vanilla file path, ordered `MergeSource[]`) instead of `TreeNode[]`, and report back through `OnMergeReport`/`OnPackReport` callbacks instead of constructing `MergeReportForm`/`PackReportForm` directly. The host's `Inventory/InteractiveMergeRunner.cs` is the thing `MainForm` actually talks to: it extracts `InteractiveMergeRequest`s from checked `TreeNode`s (inside its `BackgroundWorker`'s `DoWork`, matching the pre-split threading model — extracting outside `DoWork` let a bad node throw synchronously on the UI thread instead of being captured by `BackgroundWorker`), owns the `BackgroundWorker`, and supplies the `OnMergeReport`/`OnPackReport` callbacks (report forms, completion sounds). -Both `FileMerger.MergeText*` methods talk to KDiff3 through `IMergeEngine` (`Merge`/`MergeHeadless`, mirroring `KDiff3.Run`/`KDiff3.RunHeadless`) rather than calling `Tools/KDiff3.cs` directly, since that file's Win32 P/Invoke has to stay in the host project. `KDiff3MergeEngine` (host) is the one real implementation, supplied via `AppState.MergeEngine` — set once, as the first line of `Program.Main`, before anything else runs. This is explicitly scaffolding for the Core/host split, not a permanent pluggable-engine abstraction — a later unit removing KDiff3 entirely will likely delete this interface and inline its replacement directly into `FileMerger`. +Both `FileMerger.MergeText*` methods talk to the active text-merge engine through `IMergeEngine` (`Merge`/`MergeHeadless`) rather than calling a specific tool directly. Two implementations exist: +- **`KDiff3MergeEngine`** (host) wraps `Tools/KDiff3.Run`/`KDiff3.RunHeadless` (Win32 P/Invoke has to stay in the host project) — see the "KDiff3 input encoding"/"Verify KDiff3 process behavior"/"KDiff3's pop-up window"/`RunHeadless`'s poll interval bullets under Compatibility constraints for what it's actually doing. +- **`DiffPlexMergeEngine`** (Core, `Tools/DiffPlexMergeEngine.cs`) is an in-process alternative built on the DiffPlex NuGet package (MIT-licensed), needing no external binary. It builds its own merge loop around `DiffPlex.ThreeWayDiffer.CreateDiffs` rather than calling `ThreeWayDiffer.CreateMerge` directly, so it can intercept `ThreeWayChangeType.Conflict` blocks itself: a conflict whose two sides are equal once whitespace is collapsed (joined-and-collapsed comparison over the classic ASCII whitespace set — space/tab/CR/LF/form-feed/vertical-tab, deliberately narrower than .NET regex's Unicode-aware `\s` so a genuine content difference that happens to be NBSP-vs-space isn't misclassified as whitespace-only — not per-line `Trim()`, and never applied when either side has zero pieces, since a real deletion must never be conflated with "the surviving side happens to collapse to empty too") auto-resolves by taking the first mod's side verbatim, mirroring KDiff3's `--cs "WhiteSpace3FileMergeDefault=2"` (confirmed against the KDiff3 source: value 2 means "always pick input B", and KDiff3's own file order — vanilla, source1, source2 — maps source1 to B). A genuine conflict instead produces git/diff3-style conflict markers (`<<<<<<< ` / `||||||| Vanilla` / `=======` / `>>>>>>> `) written to a **sidecar** file under `Paths.DiffPlexConflictsDirectory` (a dedicated top-level `DiffPlexConflicts` folder — via `GetConflictMarkerPath`, keyed by an `XxHash32` of the full output path plus its filename) rather than to `outputPath` itself — writing markers to `outputPath` would make `FileMerger`'s pre-merge `File.Exists(_outputPath)` overwrite guard treat it as an already-completed merge and permanently skip retrying, since `HeadlessMergeNotifier` always declines the overwrite prompt. The sidecar went through two prior locations before landing here, each ruled out by direct end-to-end testing against the real CLI rather than by inspection alone: it originally lived right beside `outputPath` (`.conflict`), which code review flagged for two real problems (a flat-file conflict's `outputPath` sits inside the live `Paths.ModsDirectory` tree, which nothing ever cleans up; a bundle-content conflict's `outputPath` sits inside `Paths.MergedBundleContent`, which `Tools/WccLite.PackBundle` packs *wholesale* with no filtering, so a leftover `.conflict` file there could get embedded as bogus content into a later-packed `blob0.bundle`); it then moved under `Paths.TempBundleContent`, which fixed both of those but broke immediately in end-to-end testing for a third reason neither review nor unit tests caught - `FileMerger.CleanUpTempFiles()` deletes the entire `TempBundleContent` tree wholesale at the end of every headless merge run (to clear QuickBMS-unpacked bundle scratch content), so the sidecar was gone by the time the CLI process exited, before a user could ever see it. `Paths.DiffPlexConflictsDirectory` is an unrelated top-level name specifically to avoid that collision - see its own comment in `Paths.cs` for the full story. A conflict-marker file that later becomes an auto-solve on retry has its stale sidecar deleted before writing the fresh output. Two loose ends worth stating plainly rather than leaving implicit: nothing automatically deletes the `DiffPlexConflicts` directory itself (the same "accumulates until manually cleared" property `TempBundleContent` has, and the CLI-mode section below already tells users to clear that one between runs - `DiffPlexConflicts` needs the same manual housekeeping, just without an automated sweep); and `Paths.DiffPlexConflictsDirectory` is a relative path, resolved against `Environment.CurrentDirectory` - `Program.RunCli` sets that to `AppContext.BaseDirectory` before anything else runs (see "Startup flow" below), so in CLI mode sidecars land predictably next to the installed exe, but the GUI path never does that reset, so a GUI-mode DiffPlex conflict's sidecar lands wherever the process's CWD happened to be at launch (typically the exe's own directory too, for a normal double-click launch, but not guaranteed). Not a functional problem given the GUI-mode DiffPlex path is already incomplete UI-wise (no way to open the sidecar from the report dialog yet either), but worth knowing before relying on the sidecar's location being deterministic outside CLI mode. There's no UI here at all (unlike KDiff3's own GUI), so `Merge()` (interactive) just runs `MergeHeadless()` and maps `NeedsManualResolution` to `Failed`, matching `IMergeEngine.Merge`'s contract — this also means the "merging an updated mod file into an existing merge chain" outdated-hash guard (mirrored from `KDiff3.RunHeadless`, not hoisted into shared `FileMerger` orchestration) surfaces as a silent `Failed` on the interactive path here, where `KDiff3MergeEngine`'s interactive `Run()` opens KDiff3's GUI for manual review instead — an accepted gap given there's no interactive UI for DiffPlex conflicts at all yet. A 3-way merge with no vanilla version at all (expected mainly on the bundle-content path, when no matching vanilla bundle is found, but the guard applies unconditionally to any conflict missing one) is refused outright (`NeedsManualResolution`, nothing written) rather than attempted with an empty base string — `DiffPlex.ThreeWayDiffer` degrades silently to zero diff blocks and a "successful" empty merge in that case, confirmed empirically, which would otherwise produce a truncated output file; this is a deliberate divergence from `KDiff3MergeEngine`, which has no equivalent guard and always attempts a real (if vanilla-less, degraded 2-way) `--auto` merge instead, since KDiff3 has a coherent notion of a 2-file merge and DiffPlex's `ThreeWayDiffer` as used here does not. See the "DiffPlex's `ThreeWayDiffer` can produce internally inconsistent diff blocks" bullet under Compatibility constraints below for a confirmed upstream DiffPlex bug this engine has to defend against on every merge, regardless of any of the above. Selected via the `MergeEngine` `App.config` key (`kdiff3`, the default, or `diffplex`) — see "Startup flow" below; `KDiff3MergeEngine` remains the default, both because `DiffPlexMergeEngine` hasn't been cross-checked against KDiff3 on enough real conflicting files yet (only synthetic fixtures in `WitcherScriptMerger.Tests`, plus an optional, narrowly-scoped real-KDiff3 A/B check gated on `WSM_TEST_GAME_DIR` — see "Tests" above) and because of that confirmed upstream DiffPlex bug's measured non-trivial failure rate even at realistic edit densities. + +Either way, `AppState.MergeEngine` is supplied once, as (part of) the first line of `Program.Main`, before anything else runs. `IMergeEngine` is explicitly scaffolding for the Core/host split, not a permanent pluggable-engine abstraction — a later unit removing KDiff3 entirely will likely delete this interface and inline its replacement directly into `FileMerger`. ### Startup flow (`Program.cs`) -The shared mutable state that used to be static fields directly on `Program` (`Notifier`/`Settings`/`LoadOrder`/`Inventory`) now lives on Core's `AppState` instead — domain code that moved to Core needs to read/write it, and Core can never reference the host assembly. `Program.Notifier`/`Settings`/`LoadOrder`/`Inventory` are pass-through properties onto `AppState` so every pre-existing host call site kept working unchanged. `AppState.Notifier` defaults to `HeadlessMergeNotifier` via field initializer (before anything else, including `Settings = new AppSettings()`, can run) so any startup error is safe to report even before it's known whether this is a GUI or CLI run. `AppState` has an explicit (empty) static constructor so this ordering is deterministic rather than left to `beforefieldinit`'s discretion — `Program` needs the same treatment for its own remaining field initializer (`_consoleAttached = MaybeAttachConsole()`) for the identical reason, now that nothing in `Main()` necessarily touches a `Program`-owned field anymore (its former fields became properties). `MaybeAttachConsole()` runs as a field initializer, ahead of everything, so early failures are visible in the invoking terminal when there are CLI args. +The shared mutable state that used to be static fields directly on `Program` (`Notifier`/`Settings`/`LoadOrder`/`Inventory`) now lives on Core's `AppState` instead — domain code that moved to Core needs to read/write it, and Core can never reference the host assembly. `Program.Notifier`/`Settings`/`LoadOrder`/`Inventory` are pass-through properties onto `AppState` so every pre-existing host call site kept working unchanged. `AppState.Notifier` defaults to `HeadlessMergeNotifier` via field initializer, unconditionally, so any startup error is safe to report even before it's known whether this is a GUI or CLI run. `AppState.Settings` is a **lazy property**, not a field initializer (`_settings ?? (_settings = new AppSettings())`) — deliberately decoupled from `Notifier`'s eager init: `AppSettings`'s constructor calls `Environment.Exit(1)` if it can't find a config file next to the entry assembly, appropriate for the real GUI/CLI/MCP entry points (where that's genuinely fatal) but not for `WitcherScriptMerger.Tests`, whose `dotnet test` host has no matching `.config` — Core code legitimately reads `AppState.Notifier` on its own (e.g. `DiffPlexMergeEngine`'s headless skip/guard messages), and before this change that alone was enough to also force `Settings` to construct (C# runs all of a type's static field initializers together on first touch of *any* static member) and crash the whole test process. First real access to `Settings` still runs the identical `new AppSettings()` and identical crash-on-missing-config behavior for the real app, just deferred to that access instead of bundled with `Notifier`'s. `AppState` has an explicit (empty) static constructor so its own init ordering is deterministic rather than left to `beforefieldinit`'s discretion — `Program` needs the same treatment for its own remaining field initializer (`_consoleAttached = MaybeAttachConsole()`) for the identical reason, now that nothing in `Main()` necessarily touches a `Program`-owned field anymore (its former fields became properties). `MaybeAttachConsole()` runs as a field initializer, ahead of everything, so early failures are visible in the invoking terminal when there are CLI args. -`[STAThread] Main(string[] args)`: first sets `AppState.MergeEngine = new KDiff3MergeEngine()` (the one real `IMergeEngine` implementation — see "Interactive vs. headless split" above; must happen before anything calls `Paths.ValidateDependencyPaths()` or constructs a `FileMerger`, in any of the paths below). Then: if `args` is non-empty, hands off entirely to the CLI path (see below) and returns — the GUI is never touched. Otherwise: `Application.EnableVisualStyles()` → check `Settings.HasConfigFile` → `Paths.ValidateDependencyPaths()` (shows `DependencyForm` if KDiff3/QuickBMS/wcc_lite paths are invalid) → construct `MainForm`, reassign `Program.Notifier = MainForm`, `Application.Run(MainForm)`. +`[STAThread] Main(string[] args)`: first sets `AppState.MergeEngine` to either `KDiff3MergeEngine` (default) or `DiffPlexMergeEngine`, based on the `MergeEngine` `App.config` setting (`kdiff3`/`diffplex`) — see "Interactive vs. headless split" above; must happen before anything calls `Paths.ValidateDependencyPaths()` or constructs a `FileMerger`, in any of the paths below. Then: if `args` is non-empty, hands off entirely to the CLI path (see below) and returns — the GUI is never touched. Otherwise: `Application.EnableVisualStyles()` → check `Settings.HasConfigFile` → `Paths.ValidateDependencyPaths()` (shows `DependencyForm` if KDiff3/QuickBMS/wcc_lite paths are invalid) → construct `MainForm`, reassign `Program.Notifier = MainForm`, `Application.Run(MainForm)`. ### Merge flow @@ -66,7 +74,7 @@ No hand-rolled diff algorithm lives in this codebase — it's an orchestrator ar 1. `FileIndex/ModFileIndex.BuildAsync` scans `Paths.ModsDirectory`, groups files by relative path, flags conflicts. 2. `MainForm` feeds the results into `ConflictTree`/`MergeTree`; the user checks nodes to merge. 3. `Inventory/FileMerger.MergeByTreeNodesAsync` runs on a `BackgroundWorker`, building/reusing an `Inventory/Merge` record per file and dispatching to `MergeFlatFileNode` (plain `.ws`/`.xml`) or `MergeBundleFileNode` (bundle-packed files, which first go through `Tools/QuickBms.UnpackFile`). -4. `FileMerger.MergeText` calls `Tools/KDiff3.Run(source1, source2, vanillaFile, outputPath)`, which shells out to `KDiff3.exe` (`--auto` for auto-solvable 3-way merges, or opens its GUI for manual resolution). Before building the command line, `KDiff3.Run` normalizes each input file to UTF-16LE with a BOM (matching vanilla's encoding) via `EnsureUtf16Encoding`, writing a temp copy under `Paths.TempBundleContent\Encoding\...` when a file isn't already in that encoding — see "KDiff3 input encoding" below for why. +4. `FileMerger.MergeText` calls `Tools/KDiff3.Run(source1, source2, vanillaFile, outputPath)`, which shells out to `KDiff3.exe` (`--auto` for auto-solvable 3-way merges, or opens its GUI for manual resolution). Before building the command line, `KDiff3.Run` normalizes each input file to UTF-16LE with a BOM (matching vanilla's encoding) via `Tools/FileEncoding.EnsureUtf16File` (Core - shared with `DiffPlexMergeEngine`, see "Interactive vs. headless split" above), writing a temp copy under `Paths.TempBundleContent\Encoding\...` when a file isn't already in that encoding — see "KDiff3 input encoding" below for why. 5. On success, `Inventory/MergeInventory.AddModToMerge` hashes the result via `Tools/Hasher` (xxHash32, `System.IO.Hashing`) and persists the merge record to `MergeInventory.xml` (`XmlSerializer`). `MergeInventory.HasResolvedConflict` re-checks these hashes on refresh to detect merges made stale by upstream mod file changes. 6. Bundle content changes additionally go through `FileMerger.PackNewBundle` → `Tools/WccLite.PackBundle` + `GenerateMetadata` to repack `blob0.bundle`. @@ -94,7 +102,8 @@ The CLI path (`FileMerger.MergeConflictsHeadless`) is a separate, parallel orche - **Hash format is load-bearing.** `MergeInventory.xml` (including real, already-populated files on developer machines) stores per-file hashes compared by string equality to detect when a mod source file has changed since it was last merged. Any change to `Tools/Hasher.cs` must produce byte-for-byte identical output to the current implementation, or every existing recorded merge silently "goes stale." Verify with the synthetic-edge-cases + real-recorded-hash cross-check pattern described under Tests. - **TFM must keep the explicit `7.0` OS-version suffix.** The project uses `false` (to keep the hand-written `Properties/AssemblyInfo.cs`), which also suppresses the SDK's auto-generated `[assembly: SupportedOSPlatform("windows")]` attribute — that attribute is added manually in `AssemblyInfo.cs` instead. The TFM must stay `net10.0-windows7.0` (not bare `net10.0-windows`); dropping the `7.0` suffix reintroduces ~800 spurious `CA1416` platform-compatibility warnings. -- **KDiff3 input encoding must stay normalized to UTF-16LE, never down to UTF-8.** Vanilla `.ws` files are UTF-16LE with a BOM; mod authors' files are often plain UTF-8/ASCII with no BOM (confirmed against real files on a live install). KDiff3 has no command-line flag to specify per-input encoding, and a mismatch can make it treat an entire file as unmatchable, falling back to manual (GUI) conflict resolution instead of auto-solving — empirically confirmed real-world false-conflict case: `baseEffect.ws` failed to auto-solve with mismatched encodings and succeeded cleanly once normalized, with correct merged output. `Tools/KDiff3.cs::EnsureUtf16Encoding` handles this by writing a UTF-16LE+BOM temp copy of any non-UTF-16LE input before invoking KDiff3, matching vanilla's encoding — never normalize toward UTF-8, since the game may not load a merged `.ws` file in that encoding. KDiff3 itself has no config option to ignore whitespace generally during diff/merge (only `WhiteSpace3FileMergeDefault` auto-picks a side for *purely* whitespace-only conflicts) — confirmed from the bundled `doc/options.html`, not assumed. +- **KDiff3 input encoding must stay normalized to UTF-16LE, never down to UTF-8.** Vanilla `.ws` files are UTF-16LE with a BOM; mod authors' files are often plain UTF-8/ASCII with no BOM (confirmed against real files on a live install). KDiff3 has no command-line flag to specify per-input encoding, and a mismatch can make it treat an entire file as unmatchable, falling back to manual (GUI) conflict resolution instead of auto-solving — empirically confirmed real-world false-conflict case: `baseEffect.ws` failed to auto-solve with mismatched encodings and succeeded cleanly once normalized, with correct merged output. `Tools/FileEncoding.cs` (Core, shared by both merge engines) handles this: `EnsureUtf16File` writes a UTF-16LE+BOM temp copy of any non-UTF-16LE input file before invoking KDiff3 (matching vanilla's encoding), while `ReadAnyEncoding`/`WriteUtf16` give `DiffPlexMergeEngine` the same normalization without needing a temp file at all, since it merges in-process text rather than shelling out to a tool that needs a file path — never normalize toward UTF-8, since the game may not load a merged `.ws` file in that encoding. `ReadAnyEncoding` deliberately uses `File.ReadAllText(path)`'s built-in BOM auto-detection rather than decoding raw bytes with a fixed `Encoding` instance - the latter does not strip a detected BOM, leaving a stray U+FEFF glued to the first line, confirmed empirically to reproduce the exact `baseEffect.ws`-style false conflict this whole mechanism exists to avoid (see `WitcherScriptMerger.Tests`' `MergeHeadless_EncodingMismatch_...` fixture). KDiff3 itself has no config option to ignore whitespace generally during diff/merge (only `WhiteSpace3FileMergeDefault` auto-picks a side for *purely* whitespace-only conflicts, confirmed against the KDiff3 source: value `2` means "always pick input B", i.e. the first mod/`source1`, given KDiff3's own file order of vanilla/source1/source2) — confirmed from the bundled `doc/options.html`, not assumed. `DiffPlexMergeEngine` mirrors this for conflicts that are purely whitespace once collapsed (see "Interactive vs. headless split" above), rather than only literal-identical-content merges. +- **DiffPlex's `ThreeWayDiffer` (1.9.0) can produce internally inconsistent diff blocks - `DiffPlexMergeEngine.BuildMerge` must never trust its output without checking.** Confirmed as a genuine upstream library bug, not a defect in this repo's own merge loop: `BuildMerge`'s block-iteration loop is a faithful port of DiffPlex's own `ThreeWayDiffer.CreateMerge` (same index-chasing shape), and a throwaway scratch console app (per this repo's testing convention) calling DiffPlex's own `CreateMerge` directly - with both `LineChunker` (DiffPlex's own default, and the *only* chunker its own `Facts.DiffPlex/ThreeWayDifferFacts.cs` test suite ever exercises for 3-way diffs) and `LineEndingsPreservingChunker` (the one this engine actually uses) - reproduced the identical failure on the identical input either way. When old-side and new-side edits interleave/overlap relative to base in certain ways, `CreateThreeWayDiffBlocks` can emit a block list whose `OldCount`/`NewCount` don't actually correspond to the real `PiecesOld`/`PiecesNew` arrays. This surfaces two ways: an outright `ArgumentOutOfRangeException` from direct indexer access, or - confirmed via a minimal repro (base `"a();/b();/c();"`, one side inserts a line after `a()`, the other independently changes `b()` to `B()`) - no exception at all, but silently wrong output (content lost or duplicated), because the running `oldIndex`/`newIndex` end up not matching `PiecesOld.Count`/`PiecesNew.Count` even though no single block's own bookkeeping looked wrong in isolation. A large randomized stress test (varying edit density and file length, run against the real, fixed `BuildMerge`) measured combined failure rates of **0.35%** at one independent single-line edit per side on 50-200 line files (the closest analogue to a typical two-mod `.ws` conflict), rising to **0.88%** (1-2 edits/side), **2.65%** (2-3 edits/side), **4.99%** (1-6 edits/side on 50-200 line files), and **38.89%** on the original dense adversarial case (1-6 edits/side on 1-19 line files) - zero cases of any exception type other than the one this bug produces, across 100,000 total trials. `BuildMerge` defends against both failure modes: the block-processing loop is wrapped in `try`/`catch (ArgumentOutOfRangeException)`, and a post-loop check verifies `oldIndex`/`newIndex` actually reached `PiecesOld.Count`/`PiecesNew.Count` (accounting for a legitimate trailing-unchanged gap needing the exact same lockstep advance as the per-block gap-catchup above it - an early, incorrect version of this check that skipped that trailing advance produced a ~33% false-positive "inconsistent" rate on the exact same benign inputs). Either failure mode throws a `DiffPlexMergeEngine.DiffAlgorithmException`, which `MergeHeadless` catches and reports as `NeedsManualResolution` **without writing anything, including a conflict-marker sidecar** - the marker content itself would have been built from the same untrustworthy piece indices, so this is the one case where `DiffPlexMergeEngine` can't even offer a conflict-marker starting point the way KDiff3 always can. This is the primary reason `DiffPlexMergeEngine` isn't the default engine (see "Interactive vs. headless split" above) - a measured, non-negligible failure rate even at realistic edit density is a real reliability gap `KDiff3MergeEngine` doesn't share. Regression-tested via `DiffPlexMergeEngineTests`' `BuildMerge_InterleavedIndependentEdits_...`/`MergeHeadless_InterleavedIndependentEdits_...` fixtures (the minimal repro above). Do not "fix" this by switching chunkers - the bug reproduces under DiffPlex's own default/tested `LineChunker` too, just at a somewhat lower rate, so it isn't a `LineEndingsPreservingChunker`-specific problem and switching would trade a real, working byte-for-byte line-ending-preservation property for no actual safety gain. - **Verify KDiff3 process behavior via `Process.Start(fileName, argsString)` (the two-string overload, `UseShellExecute=false` by default on modern .NET), not via a shell.** A prior verification pass tested KDiff3 invocation through Git Bash/MSYS2 and concluded `damageManagerProcessor.ws` (a second real conflict) still needed manual GUI resolution even after encoding normalization. Re-tested later through .NET's `Process.Start` — the actual code path this app uses — the same file (both raw and normalized) auto-solved cleanly every time; the bash-based test was an invocation-environment artifact, not real KDiff3 behavior. A guaranteed-genuine conflict (two synthetic mods editing the identical line differently) confirmed what actually distinguishes the two outcomes: KDiff3 always briefly shows a window titled exactly `Conflicts` on startup (auto-solves or not — this is not a "needs manual resolution" signal), but only a genuine unresolved conflict leaves a second window open whose title ends in `" - KDiff3"` (the actual comparison/merge editor, e.g. `Vanilla <-> modA <-> modB - KDiff3`) — that one persists indefinitely until closed, while an auto-solve's process exits within a few seconds regardless of file size (3400+ line files exited in under 3s in testing). Any headless/non-interactive invocation path must detect on window persistence past a short grace period (~2-3s, to let the transient `Conflicts` window close), not on elapsed time alone and not by assuming a visible window means failure. - **KDiff3's pop-up window can't be suppressed without breaking the merge — don't try.** Five techniques were tested empirically (scratch harness in a session's `scratchpad/detector-test/`) against both an auto-solve case and a guaranteed-conflict case: `ProcessStartInfo.WindowStyle = Hidden` and `= Minimized` are both silently ignored by KDiff3/Qt (window shows full-size regardless, confirmed via `IsIconic` for the minimized case) but don't break anything; `ShowWindow(hwnd, SW_HIDE)`, `SetWindowPos` moved off-screen, and launching on a separate non-interactive Windows desktop (`CreateDesktop`) all three genuinely succeed at making the window invisible — and all three reliably make KDiff3 hang forever at its "Conflicts" splash instead of ever auto-solving (confirmed against a clean control: the identical launch mechanism, untouched, auto-solves in 1.6–6.5s every time). The pattern held across three independent suppression mechanisms, which is strong evidence KDiff3's Qt runtime needs the window genuinely composited on the real, interactive desktop to make progress at all — not something fixable from outside the process. The window does steal foreground focus while shown (confirmed via `GetForegroundWindow()`). Given that hard constraint, `KDiff3.RunHeadless` accepts the window appearing and instead attempts to restore focus to whatever had it beforehand (captured before launching KDiff3, restored in a `finally` once KDiff3's own window is confirmed gone — `proc.Kill` is async, so the kill path also waits up to 2s via `WaitForExit` before restoring, or the restore could race a still-alive window). **This restoration is unverified in practice — treat it as attempted, not guaranteed.** Plain `SetForegroundWindow` was empirically denied every time in testing (Windows' foreground-lock policy: WSM's own process never owned the foreground to begin with, since KDiff3's window did, so it isn't a privileged caller by the time it tries to restore). `RestoreForegroundWindow` upgrades to the standard `AttachThreadInput` workaround (temporarily share input state with whatever thread currently owns the foreground, then call `SetForegroundWindow`) — but this was *also* observed to be denied in every test run in this session's sandboxed automation environment. Whether that's a real limit or an artifact of that specific environment (its own automation harness aggressively reclaiming focus) is unresolved; it hasn't been tested from a normal interactive user session. Treat the restore as a good-faith best-effort mitigation, not a proven one, until someone verifies it from an ordinary desktop session. Do not reintroduce any of the three broken suppression techniques without re-verifying they still hang; if a future KDiff3 update changes this behavior, that verification needs to be redone before trusting a different result. - **`RunHeadless`'s ~250ms detection-loop poll interval is load-bearing — don't tighten it.** Discovered by accident while testing the suppression techniques above: polling every 15ms for the first second — with *zero* window manipulation, just `EnumWindows`/`GetWindowText` read-only queries — reliably hung KDiff3 the same way the suppression techniques did; the identical untouched launch polled at 200ms auto-solved normally every time. `GetWindowText` issues a cross-process `SendMessage(WM_GETTEXT)` to the target window, which is a blocking call the target thread must service — a plausible mechanism is that polling fast enough starves or reorders KDiff3's own message loop during the window it needs to actually compute the merge. "Poll faster to detect the conflict window sooner" is a natural-looking optimization that would silently turn every merge into a hang — don't make this change without re-verifying against both an auto-solve case and a guaranteed-conflict case first. diff --git a/WitcherScriptMerger.Core/AppState.cs b/WitcherScriptMerger.Core/AppState.cs index 241a263..e0c673c 100644 --- a/WitcherScriptMerger.Core/AppState.cs +++ b/WitcherScriptMerger.Core/AppState.cs @@ -1,4 +1,5 @@ -using WitcherScriptMerger.Inventory; +using System.Threading; +using WitcherScriptMerger.Inventory; using WitcherScriptMerger.LoadOrder; using WitcherScriptMerger.Tools; @@ -19,16 +20,54 @@ namespace WitcherScriptMerger // rather than at some unspecified point the CLR chooses - load-bearing here // because Program.MaybeAttachConsole() must run before Settings' constructor // can report a missing-config error to the invoking terminal (see CLAUDE.md's - // Startup flow), and because Paths' own static field initializers read - // Settings.Get(...), transitively depending on this class being fully - // initialized first. + // Startup flow). Paths.cs used to have the same beforefieldinit hazard one hop + // further out (its own static field initializers read Settings.Get(...) + // eagerly) - fixed by making those Paths properties compute on every access + // instead of caching via a field initializer, so Settings' laziness isn't + // undermined transitively; see Paths.cs. public static class AppState { // Defaults to the headless implementation so it's safe to use from the very // first line of Main() - the GUI path swaps it out for MainForm once // constructed. See CLAUDE.md's IMergeNotifier section. public static IMergeNotifier Notifier = new HeadlessMergeNotifier(); - public static AppSettings Settings = new AppSettings(); + + // Lazy rather than a field initializer: AppSettings' constructor calls + // Environment.Exit(1) if it can't find a config file next to the entry + // assembly (see AppSettings.cs) - appropriate for the real GUI/CLI/MCP entry + // points, where that's genuinely fatal, but not for WitcherScriptMerger.Tests, + // whose test host has no matching .config. Since C# runs ALL of a type's + // static field initializers together on first touch of ANY static member, + // Settings being a plain field-with-initializer meant merely reading + // AppState.Notifier (which Core code - e.g. DiffPlexMergeEngine's headless + // skip/guard messages - legitimately does on its own, unprompted by test code) + // silently also ran `new AppSettings()` and crashed the whole test process. + // Making Settings lazy decouples the two: touching Notifier alone no longer + // forces Settings to construct. Confirmed no call site assigns AppState.Settings + // or Program.Settings, so keeping this settable (for symmetry with the other + // fields here, and in case a future test wants to inject a stub) is a safe, + // behavior-preserving change for every existing GUI/CLI/MCP call site: first + // real access still runs the identical `new AppSettings()` and identical + // crash-on-missing-config behavior, just deferred to that first access instead + // of eagerly. + // + // LazyInitializer.EnsureInitialized (rather than the simpler + // `_settings ?? (_settings = new AppSettings())`) makes this thread-safe: the + // simpler form is a classic non-atomic check-then-act race that could, under + // concurrent first access, construct AppSettings() more than once (each with + // its own real side effects, including a possible Environment.Exit(1)). + // Currently unreachable from any shipped entry point or the test suite (all + // single-threaded at this point in startup) - flagged in code review as a + // latent risk anyway, since other Core statics (e.g. QuickBms.cs/WccLite.cs) + // also read AppState.Settings.Get(...) from their own static field + // initializers, and nothing prevents a future concurrent caller. + static AppSettings _settings; + public static AppSettings Settings + { + get => LazyInitializer.EnsureInitialized(ref _settings, () => new AppSettings()); + set => _settings = value; + } + public static CustomLoadOrder LoadOrder = null; public static MergeInventory Inventory = null; diff --git a/WitcherScriptMerger.Core/Inventory/FileMerger.cs b/WitcherScriptMerger.Core/Inventory/FileMerger.cs index 5fa3633..9e3e78d 100644 --- a/WitcherScriptMerger.Core/Inventory/FileMerger.cs +++ b/WitcherScriptMerger.Core/Inventory/FileMerger.cs @@ -274,7 +274,11 @@ void MergeBundleFileInteractive(InteractiveMergeRequest file, Merge merge, bool FileInfo MergeTextInteractive(Merge merge, MergeSource source1, MergeSource source2) { - ProgressInfo.CurrentAction = $"Merging {source1.Name} && {source2.Name} — waiting for KDiff3 to close"; + // Deliberately engine-neutral wording: this used to name KDiff3 explicitly + // ("waiting for KDiff3 to close"), which is wrong when MergeEngine is + // DiffPlexMergeEngine instead - no external process or window is involved + // there at all. Flagged in code review, see CLAUDE.md. + ProgressInfo.CurrentAction = $"Merging {source1.Name} && {source2.Name}"; var result = MergeEngine.Merge(source1, source2, _vanillaFile, _outputPath); diff --git a/WitcherScriptMerger.Core/Paths.cs b/WitcherScriptMerger.Core/Paths.cs index 91b9016..1f38c36 100644 --- a/WitcherScriptMerger.Core/Paths.cs +++ b/WitcherScriptMerger.Core/Paths.cs @@ -9,6 +9,22 @@ public static class Paths public const string TempBundleContent = "tempbundlecontent"; public static string MergedBundleContent = "Merged Bundle Content"; public static string MergedBundleContentAbsolute = Path.Combine(Environment.CurrentDirectory, MergedBundleContent); + + // A dedicated top-level directory for DiffPlexMergeEngine's conflict-marker + // sidecar files (Tools/DiffPlexMergeEngine.cs::GetConflictMarkerPath) - + // deliberately NOT a subdirectory of TempBundleContent, even though both are + // "scratch-ish" locations conceptually: FileMerger.CleanUpTempFiles() deletes + // the entire TempBundleContent tree wholesale at the end of every headless + // merge run (to clear QuickBMS-unpacked bundle scratch content), which would + // otherwise delete every sidecar moments after DiffPlexMergeEngine wrote it - + // confirmed by direct observation running the real CLI end-to-end: the sidecar + // briefly existed during the run (the "conflict markers written to..." message + // printed a real path) but was gone by the time the process exited. A separate, + // unrelated top-level name sidesteps that collision entirely while keeping the + // same original benefits (out of the live Paths.ModsDirectory tree, out of + // Paths.MergedBundleContent's wholesale-packed tree - see DiffPlexMergeEngine's + // own comment on GetConflictMarkerPath for those two reasons). + public const string DiffPlexConflictsDirectory = "DiffPlexConflicts"; public const string Inventory = "MergeInventory.xml"; public static string ModScriptBase = Path.Combine("content", "scripts"); public static string VanillaScriptBase = Path.Combine("content", "content0", "scripts"); @@ -22,31 +38,43 @@ public static class Paths public static string DlcDirectory => Path.Combine(GameDirectory, "DLC"); - static string _scriptsDirSetting = AppState.Settings.Get("VanillaScriptsDirectory"); + // Deliberately not cached in a static field (as these two used to be): a field + // initializer here would run alongside every other static field initializer of + // this type on first touch of ANY of them (C#'s beforefieldinit semantics), + // which would eagerly call AppState.Settings.Get(...) - forcing + // AppState.Settings to construct (see its own lazy-property comment in + // AppState.cs) merely from touching an unrelated static member of Paths, e.g. a + // plain string helper like GetRelativePath with no settings dependency at all. + // That's exactly the crash-in-a-dotnet-test-host scenario AppState.Settings' + // laziness exists to avoid, one hop removed - flagged in code review, see + // CLAUDE.md. AppState.Settings.Get(...) already reads from AppSettings' own + // cached ConfigurationManager state, so re-reading it on every call here (rather + // than caching again at this layer) costs nothing meaningful. public static string ScriptsDirectory { get { - return (!string.IsNullOrWhiteSpace(_scriptsDirSetting) - ? _scriptsDirSetting + var setting = AppState.Settings.Get("VanillaScriptsDirectory"); + return (!string.IsNullOrWhiteSpace(setting) + ? setting : Path.Combine(GameDirectory, VanillaScriptBase)); } } - static string _modsDirSetting = AppState.Settings.Get("ModsDirectory"); public static string ModsDirectory { get { - return (!string.IsNullOrWhiteSpace(_modsDirSetting) - ? _modsDirSetting + var setting = AppState.Settings.Get("ModsDirectory"); + return (!string.IsNullOrWhiteSpace(setting) + ? setting : Path.Combine(GameDirectory, "Mods")); } } - public static bool IsScriptsDirectoryDerived => string.IsNullOrWhiteSpace(_scriptsDirSetting); + public static bool IsScriptsDirectoryDerived => string.IsNullOrWhiteSpace(AppState.Settings.Get("VanillaScriptsDirectory")); - public static bool IsModsDirectoryDerived => string.IsNullOrWhiteSpace(_modsDirSetting); + public static bool IsModsDirectoryDerived => string.IsNullOrWhiteSpace(AppState.Settings.Get("ModsDirectory")); public static string GetRelativePath(string fullPath, string basePath) { diff --git a/WitcherScriptMerger.Core/Tools/DiffPlexMergeEngine.cs b/WitcherScriptMerger.Core/Tools/DiffPlexMergeEngine.cs new file mode 100644 index 0000000..00d73da --- /dev/null +++ b/WitcherScriptMerger.Core/Tools/DiffPlexMergeEngine.cs @@ -0,0 +1,510 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.IO.Hashing; +using System.Linq; +using System.Text; +using System.Text.RegularExpressions; +using DiffPlex; +using DiffPlex.Chunkers; +using DiffPlex.Model; +using WitcherScriptMerger.Inventory; + +namespace WitcherScriptMerger.Tools +{ + // In-process, external-binary-free alternative to KDiff3MergeEngine (host project), + // built on DiffPlex (MIT-licensed NuGet package)'s ThreeWayDiffer. Not the active + // engine by default - see Program.Main (host project) for the "MergeEngine" App.config + // switch. See Tools/IMergeEngine.cs for why this interface exists at all: it's + // Core/host split scaffolding, not a permanent pluggable-engine abstraction, and a + // later unit that removes KDiff3 entirely may delete the interface and inline this + // engine's logic directly into FileMerger. + // + // There's no UI here at all - unlike KDiff3MergeEngine, which can open KDiff3's own + // GUI for the interactive path - so "interactive" and "headless" collapse to the same + // underlying logic. Merge() just runs MergeHeadless() and maps NeedsManualResolution + // to Failed, since IMergeEngine.Merge's contract explicitly forbids ever returning + // NeedsManualResolution (that's a headless-only concept - see the interface's doc + // comment). One real behavior difference from KDiff3MergeEngine as a result: the + // "ReviewEachMerge" setting (show the merge UI even for auto-solvable merges, so the + // user can double check it) has nothing to open here and is silently not honored - + // there is no in-process equivalent to implement it against. + public class DiffPlexMergeEngine : IMergeEngine + { + #region Types + + public readonly struct MergeTextResult + { + public string MergedText { get; } + public bool HasConflicts { get; } + + public MergeTextResult(string mergedText, bool hasConflicts) + { + MergedText = mergedText; + HasConflicts = hasConflicts; + } + } + + // Thrown by BuildMerge when DiffPlex's ThreeWayDiffer itself produces + // internally inconsistent diff-block metadata for a given base/old/new triple + // - a genuine, confirmed upstream bug (DiffPlex 1.9.0), not a defect in this + // class's own loop. See BuildMerge's comment for the full empirical writeup + // and CLAUDE.md's Compatibility constraints for measured failure rates. Kept + // separate from ArgumentNullException (a caller-error guard) so + // MergeHeadless can catch specifically this and only this as "the algorithm + // itself can't be trusted here" rather than accidentally swallowing an + // unrelated bug. + public sealed class DiffAlgorithmException : Exception + { + public DiffAlgorithmException(string message) : base(message) { } + } + + #endregion + + #region Members + + // Deliberately the classic ASCII whitespace set (space, tab, CR, LF, form feed, + // vertical tab), not \s+: .NET's \s matches the full Unicode whitespace + // category too (NBSP, U+2028/2029, ideographic space, etc.), and collapsing + // those away could misclassify a genuine content difference as "purely + // whitespace" - e.g. two mods' string-literal dialogue text differing only by + // NBSP vs. a regular space (plausible in localized text) would otherwise be + // silently auto-resolved instead of flagged as a conflict. CR/LF stay included + // so a side that merely adds/removes a blank line (see + // BuildMerge_WhitespaceOnlyConflict_ToleratesDifferingLineCounts) still + // collapses the same as before - only the extra-exotic Unicode members of \s + // are excluded. Flagged in code review, see CLAUDE.md. + static readonly Regex WhitespaceRun = new Regex(@"[ \t\r\n\f\v]+", RegexOptions.Compiled); + + #endregion + + public MergeEngineResult Merge( + FileMerger.MergeSource source1, + FileMerger.MergeSource source2, + FileInfo vanillaFile, + string outputPath) + { + var result = MergeHeadless(source1, source2, vanillaFile, outputPath); + return result == MergeEngineResult.NeedsManualResolution ? MergeEngineResult.Failed : result; + } + + public MergeEngineResult MergeHeadless( + FileMerger.MergeSource source1, + FileMerger.MergeSource source2, + FileInfo vanillaFile, + string outputPath) + { + var hasVanillaVersion = vanillaFile != null && vanillaFile.Exists; + + // A 3-way merge is meaningless without a base to diff against - confirmed + // empirically in this change's verification scratch app that feeding + // ThreeWayDiffer an empty base string doesn't degrade gracefully to some + // reasonable 2-way behavior: CreateThreeWayDiffBlocks' main loop is + // `while (baseIndex < basePieces.Count)`, which never executes when + // basePieces.Count is 0, so it silently returns zero diff blocks and a + // merge result with IsSuccessful=true but a completely empty MergedPieces - + // i.e. it would happily "auto-solve" straight to an empty output file. In + // practice this is expected mainly on the bundle-content path, when no + // vanilla bundle containing this file could be found (FileMerger. + // GetUnpackedFiles leaves _vanillaFile null), but this guard applies + // unconditionally to any conflict with no vanilla file, flat or bundled - + // safest, and consistent with HeadlessMergeNotifier's non-destructive + // defaults, is to refuse rather than guess. Note this is a real, deliberate + // behavior difference from KDiff3MergeEngine: Tools/KDiff3.cs's BuildArgs has + // no equivalent guard and always attempts a real (if degraded, vanilla-less) + // 2-way --auto merge in this situation instead of refusing outright, because + // KDiff3 itself has a coherent notion of a 2-file diff/merge - DiffPlex's + // ThreeWayDiffer, as used here, does not, so there's no equally meaningful + // fallback to attempt. Which conflicts even get attempted can therefore differ + // depending on which engine is configured; flagged in code review, not fixed + // by building a parallel 2-way DiffPlex merge path since that's new scope + // beyond what this engine set out to replicate - see CLAUDE.md. + if (!hasVanillaVersion) + { + AppState.Notifier.ShowMessage( + $"Skipped {source1.Name} + {source2.Name}: no vanilla version of this file could be found, " + + "so a 3-way merge isn't possible.", + "Skipped", NotifyButtons.OK, DialogIcon.Warning); + return MergeEngineResult.NeedsManualResolution; + } + + // Same "merging an updated mod file into an existing merge chain" guard + // KDiff3.RunHeadless applies (see its comment for the full reasoning) - kept + // duplicated here rather than hoisted into FileMerger since that's shared + // orchestration code outside this unit's scope; a later unit collapsing the + // merge engines should consider moving it there instead of keeping two copies. + // One real consequence of the duplication (vs. hoisting into FileMerger, + // which both Merge and MergeHeadless funnel through) worth calling out: since + // Merge() (the interactive path) just delegates straight to MergeHeadless() + // here (see this class's header comment - there's no UI to fall back to), + // this outdated-hash case comes back as Failed on the interactive path too, + // where KDiff3MergeEngine's own interactive Run() instead opens KDiff3's GUI + // for manual review. That gap already exists for every other kind of conflict + // on the DiffPlex interactive path (no UI here at all yet), so it isn't a new + // asymmetry this guard introduces - flagged in code review, see CLAUDE.md. + if (source1.TextFile.FullName.EqualsIgnoreCase(outputPath) + && source2.Hash != null && source2.Hash.IsOutdated) + { + AppState.Notifier.ShowMessage( + $"Skipped {source1.Name} + {source2.Name}: merging an updated mod file into a merge " + + "created with a previous version needs manual review (auto-solving could keep changes " + + "from the previous version that have been removed in the new one).", + "Skipped", NotifyButtons.OK, DialogIcon.Warning); + return MergeEngineResult.NeedsManualResolution; + } + + var baseText = FileEncoding.ReadAnyEncoding(vanillaFile.FullName); + var oldText = FileEncoding.ReadAnyEncoding(source1.TextFile.FullName); + var newText = FileEncoding.ReadAnyEncoding(source2.TextFile.FullName); + + MergeTextResult result; + try + { + result = BuildMerge(baseText, oldText, newText, source1.Name, source2.Name); + } + catch (DiffAlgorithmException ex) + { + // DiffPlex's own diff algorithm produced output it isn't safe to trust + // (see BuildMerge's comment) - don't write anything, including a sidecar: + // the "conflict marker" content itself would have been built from the + // same inconsistent piece indices, so it can't be trusted either. This is + // the one case where DiffPlexMergeEngine can't even offer a conflict-marker + // starting point the way KDiff3 always can - genuinely needs the source + // files opened side by side. + AppState.Notifier.ShowMessage( + $"Skipped {source1.Name} + {source2.Name}: the automatic 3-way merge algorithm hit " + + $"an internal inconsistency it couldn't safely recover from ({ex.Message}) - a known " + + "limitation of the underlying DiffPlex library for certain multi-edit conflicts, see " + + "CLAUDE.md. Needs manual resolution (e.g. via KDiff3).", + "Skipped", NotifyButtons.OK, DialogIcon.Warning); + return MergeEngineResult.NeedsManualResolution; + } + + if (!result.HasConflicts) + { + // A prior attempt at this same conflict may have left a sidecar marker + // file behind (see below) - if this attempt now auto-solves (e.g. the + // mod files were updated to no longer conflict), remove it so it doesn't + // sit next to the fresh output indefinitely, stale and misleading. + DeleteIfExists(GetConflictMarkerPath(outputPath)); + + FileEncoding.WriteUtf16(outputPath, result.MergedText); + return MergeEngineResult.AutoSolved; + } + + // Never write conflict markers to outputPath itself: FileMerger's headless + // callers (MergeFlatConflictHeadless/MergeBundleConflictHeadless) check + // `File.Exists(_outputPath)` BEFORE attempting a merge and, if it exists, + // prompt to overwrite via ConfirmOutputOverwrite - which HeadlessMergeNotifier + // always answers "no". A marker file left at outputPath would therefore + // permanently block every future retry of this same conflict without ever + // attempting the merge again. Writing to a separate sidecar location instead + // (see GetConflictMarkerPath) keeps outputPath itself untouched (so retries + // behave exactly as if this merge had never been attempted) while still + // producing well-formed conflict-marker output at a predictable, computable + // location for a later unit to open in the user's default text editor. + FileEncoding.WriteUtf16(GetConflictMarkerPath(outputPath), result.MergedText); + + AppState.Notifier.ShowMessage( + $"Skipped {source1.Name} + {source2.Name}: genuine conflict, needs manual resolution. " + + $"Conflict markers were written to {GetConflictMarkerPath(outputPath)} for review.", + "Skipped", NotifyButtons.OK, DialogIcon.Warning); + return MergeEngineResult.NeedsManualResolution; + } + + // No external executable - DiffPlex is an in-process managed library, so there's + // nothing to validate a path for. Note this doesn't remove QuickBMS/wcc_lite from + // Paths.ValidateDependencyPaths()'s checks - those are still required for bundle + // content regardless of which text-merge engine is active. + public bool ValidateExePath() => true; + + // Where a conflict-marker file is written when a merge can't be auto-solved - + // never at outputPath itself (see MergeHeadless's comment above). Originally + // this wrote to ".conflict", right beside the real output - but code + // review (see CLAUDE.md) caught two real problems with that: (1) for a flat-file + // (.ws/.xml) conflict, outputPath sits inside the live, user-facing + // Paths.ModsDirectory tree, and nothing ever cleans up a sidecar left there, + // unlike Paths.TempBundleContent, which is documented as safe to clear between + // runs; (2) for a bundle-content conflict, outputPath sits inside + // Paths.MergedBundleContent, which Tools/WccLite.PackBundle packs *wholesale* + // (no filtering) - a leftover ".conflict" text file there would get embedded as + // bogus content into the shipped blob0.bundle on any later successful pack of + // that same bundle. Relocating under Paths.DiffPlexConflictsDirectory avoids + // both - and deliberately does NOT nest under Paths.TempBundleContent either, + // despite both being "scratch-ish" locations conceptually: an earlier version + // of this fix did nest there, and end-to-end testing against the real CLI + // caught a real regression - FileMerger.CleanUpTempFiles() deletes the entire + // TempBundleContent tree wholesale at the end of every headless merge run (to + // clear QuickBMS-unpacked bundle scratch content), which silently deleted every + // sidecar moments after this method wrote it, before a user could ever see it. + // See Paths.DiffPlexConflictsDirectory's own comment for the full story. The + // XxHash32 of the full absolute outputPath (Core already depends on + // System.IO.Hashing for Tools/Hasher.cs) keeps the result collision-free without + // needing to know which of those two root trees outputPath came from, and + // without the unbounded path length a naive "flatten the whole absolute path + // into one filename" scheme would risk for a deeply-nested bundle-content path. + // string.GetHashCode() was deliberately not used here - .NET randomizes string + // hash codes per process by default, so it isn't stable across runs, unlike + // XxHash32. This is still a computable, not merely a discoverable-by-browsing, + // location: a later unit wiring up "open in editor" can call this same method. + public static string GetConflictMarkerPath(string outputPath) + { + var pathHash = XxHash32.HashToUInt32(Encoding.UTF8.GetBytes(outputPath), 0); + var fileName = Path.GetFileName(outputPath) + "." + pathHash.ToString("X8") + ".conflict"; + return Path.Combine(Paths.DiffPlexConflictsDirectory, fileName); + } + + // Swallows every exception (locked file, permission denial, etc.) rather than + // surfacing a failed delete - deliberate, not an oversight: this only ever + // removes a stale sidecar right before writing a fresh, correct output to + // outputPath, which happens regardless of whether this cleanup succeeds. The + // only consequence of a failed delete is a stale ".conflict" file left sitting + // next to a now-correct output - mildly confusing if someone stumbles on it, but + // never incorrect or data-lossy, so it isn't worth a user-facing notification for + // what's already a low-probability failure on a best-effort cleanup step. + // Flagged in code review, see CLAUDE.md. + static void DeleteIfExists(string path) + { + try { if (File.Exists(path)) File.Delete(path); } catch { } + } + + #region Merge algorithm + + // The actual 3-way merge, factored out as a public static method (independent of + // any FileMerger/MergeSource/disk I/O) specifically so it's directly unit + // testable. Mirrors DiffPlex's own ThreeWayDiffer.CreateMerge loop (see its + // source for the shape this follows) but adds two things CreateMerge doesn't do: + // - Purely-whitespace-only conflicts auto-resolve instead of producing markers, + // mirroring KDiff3's --cs "WhiteSpace3FileMergeDefault=2" (verified against the + // KDiff3 source: value 2 means "always pick input B", which is oldText/oldLabel + // here, matching KDiff3.BuildArgs' own file order of vanilla/source1/source2 - + // see CLAUDE.md's KDiff3 compatibility notes). + // - Genuine conflicts are rendered as git/diff3-style conflict markers labeled + // with the actual mod names, not DiffPlex's generic "old"/"base"/"new". + // Uses LineEndingsPreservingChunker (not DiffPlex's default LineChunker) so + // unchanged/single-side-changed content round-trips through unmodified, keeping + // each such line's original line-ending byte-for-byte - only synthetic content + // this method itself adds (conflict marker lines) uses an explicit "\r\n" to + // match vanilla .ws files' own DOS line endings (KDiff3.BuildArgs' own + // --cs "LineEndStyle=1" - confirmed against the KDiff3 source, value 1 is DOS). + public static MergeTextResult BuildMerge(string baseText, string oldText, string newText, string oldLabel, string newLabel) + { + // ThreeWayDiffer.CreateDiffs throws its own ArgumentNullException for a null + // baseText/oldText/newText, but from inside DiffPlex rather than at this + // method's own boundary - guard here instead so a caller gets a clear + // exception pointing at this public entry point. + if (baseText == null) throw new ArgumentNullException(nameof(baseText)); + if (oldText == null) throw new ArgumentNullException(nameof(oldText)); + if (newText == null) throw new ArgumentNullException(nameof(newText)); + + var chunker = LineEndingsPreservingChunker.Instance; + var diffResult = ThreeWayDiffer.Instance.CreateDiffs( + baseText, oldText, newText, ignoreWhiteSpace: false, ignoreCase: false, chunker); + + var merged = new StringBuilder(); + var hasConflicts = false; + + var baseIndex = 0; + var oldIndex = 0; + var newIndex = 0; + + // CONFIRMED UPSTREAM BUG (DiffPlex 1.9.0), not a defect in this loop's own + // bookkeeping: this loop is a faithful port of DiffPlex's own + // ThreeWayDiffer.CreateMerge (same index-chasing shape), and DiffPlex's own + // CreateMerge was verified - via a throwaway scratch console app per this + // repo's testing convention, calling DiffPlex's ThreeWayDiffer.CreateMerge + // directly - to exhibit the exact same two failure modes on the exact same + // inputs, with both LineChunker (DiffPlex's own default/only-tested chunker + // for 3-way diffs - its own Facts.DiffPlex/ThreeWayDifferFacts.cs never + // exercises any other chunker) and LineEndingsPreservingChunker: when old-side + // and new-side edits interleave/overlap relative to base in certain ways, + // CreateThreeWayDiffBlocks can produce a block list whose OldCount/NewCount + // don't actually correspond to the real PiecesOld/PiecesNew arrays. This + // surfaces two ways: (1) an outright ArgumentOutOfRangeException from the + // direct indexer accesses below, or (2) - confirmed via a minimal repro + // (base "a();/b();/c();", one side inserts a line, the other independently + // changes "b()" to "B()") - no exception at all, but content is silently + // lost or duplicated, because the running oldIndex/newIndex end up not + // matching PiecesOld.Count/PiecesNew.Count even though no single block's own + // bookkeeping ever looked wrong in isolation. A large randomized stress test + // (varying edit density and file length) measured combined failure rates from + // ~0.35% (one independent single-line edit per side, 50-200 line files - the + // closest analogue to a typical two-mod .ws conflict) up to double digits for + // denser multi-edit-per-side cases - see CLAUDE.md's Compatibility + // constraints for the full numbers. Given real, measured, non-negligible + // rates of both failure modes, this is caught here (an exception) and + // verified for (the silent case, via the post-loop count check below) rather + // than trusted - MergeHeadless treats either as "needs manual resolution" + // rather than ever risking corrupted merge output. This is also a primary + // reason DiffPlexMergeEngine isn't the default engine yet (see Program.cs). + try + { + foreach (var block in diffResult.DiffBlocks) + { + while (baseIndex < block.BaseStart) + { + merged.Append(diffResult.PiecesBase[baseIndex]); + ++baseIndex; + ++oldIndex; + ++newIndex; + } + + switch (block.ChangeType) + { + case ThreeWayChangeType.Unchanged: + for (var i = 0; i < block.BaseCount; ++i) + merged.Append(diffResult.PiecesBase[baseIndex + i]); + break; + + case ThreeWayChangeType.OldOnly: + for (var i = 0; i < block.OldCount; ++i) + merged.Append(diffResult.PiecesOld[oldIndex + i]); + break; + + case ThreeWayChangeType.NewOnly: + for (var i = 0; i < block.NewCount; ++i) + merged.Append(diffResult.PiecesNew[newIndex + i]); + break; + + case ThreeWayChangeType.BothSame: + // Both sides made the same change - take either (old, matching + // DiffPlex's own CreateMerge convention). + for (var i = 0; i < block.OldCount; ++i) + merged.Append(diffResult.PiecesOld[oldIndex + i]); + break; + + case ThreeWayChangeType.Conflict: + var oldPieces = diffResult.PiecesOld.Skip(oldIndex).Take(block.OldCount).ToList(); + var newPieces = diffResult.PiecesNew.Skip(newIndex).Take(block.NewCount).ToList(); + + if (IsWhitespaceOnlyDifference(oldPieces, newPieces)) + { + foreach (var piece in oldPieces) + merged.Append(piece); + } + else + { + hasConflicts = true; + var basePieces = diffResult.PiecesBase.Skip(baseIndex).Take(block.BaseCount).ToList(); + AppendConflictMarkers(merged, oldLabel, oldPieces, basePieces, newLabel, newPieces); + } + break; + } + + baseIndex += block.BaseCount; + oldIndex += block.OldCount; + newIndex += block.NewCount; + } + + while (baseIndex < diffResult.PiecesBase.Count) + { + merged.Append(diffResult.PiecesBase[baseIndex]); + ++baseIndex; + ++oldIndex; + ++newIndex; + } + } + catch (ArgumentOutOfRangeException ex) + { + throw new DiffAlgorithmException( + "DiffPlex's ThreeWayDiffer produced diff-block metadata that doesn't match " + + "its own piece arrays for this file (" + ex.Message + ")."); + } + + // Even when nothing threw, the same underlying inconsistency can silently + // produce WRONG merged content instead - confirmed via the minimal repro + // described above, where oldIndex/newIndex end up one past + // PiecesOld.Count/PiecesNew.Count with no exception anywhere. Verifying the + // running counters actually landed on the true totals (rather than trusting + // that "no exception" means "correct") is what catches that case. + if (oldIndex != diffResult.PiecesOld.Count || newIndex != diffResult.PiecesNew.Count) + { + throw new DiffAlgorithmException( + "DiffPlex's ThreeWayDiffer produced diff-block metadata that doesn't fully " + + "(or doubly) account for this file's content, without throwing an exception."); + } + + return new MergeTextResult(merged.ToString(), hasConflicts); + } + + static void AppendConflictMarkers( + StringBuilder merged, + string oldLabel, + List oldPieces, + List basePieces, + string newLabel, + List newPieces) + { + EnsureLineBreakBeforeMarker(merged); + + merged.Append("<<<<<<< ").Append(oldLabel).Append("\r\n"); + foreach (var piece in oldPieces) + merged.Append(piece); + EnsureLineBreakBeforeMarker(merged); + + merged.Append("||||||| Vanilla\r\n"); + foreach (var piece in basePieces) + merged.Append(piece); + EnsureLineBreakBeforeMarker(merged); + + merged.Append("=======\r\n"); + foreach (var piece in newPieces) + merged.Append(piece); + EnsureLineBreakBeforeMarker(merged); + + merged.Append(">>>>>>> ").Append(newLabel).Append("\r\n"); + } + + // Pieces from LineEndingsPreservingChunker only carry a line ending when the + // original text had one at that point - a file (or a conflicting region right at + // EOF) that doesn't end in a newline would otherwise glue a marker line onto the + // preceding content instead of starting a new line. Checks for a trailing '\r' + // as well as '\n': the original check only excluded '\n', so a region ending in + // a lone '\r' (old Mac-style line ending, or a genuinely incomplete line) would + // still get "\r\n" appended, producing a stray "\r\r\n" right before the marker + // - flagged in code review, see CLAUDE.md. + static void EnsureLineBreakBeforeMarker(StringBuilder sb) + { + if (sb.Length > 0 && sb[sb.Length - 1] != '\n' && sb[sb.Length - 1] != '\r') + sb.Append("\r\n"); + } + + // KDiff3's WhiteSpace3FileMergeDefault only auto-resolves a conflict that's + // "purely whitespace" - i.e. once whitespace differences are ignored entirely, + // both sides agree. Comparing the whole joined-and-collapsed region (rather than + // piece-by-piece) is deliberate: two sides can disagree on how many lines a + // change spans (e.g. one side also adds a blank line) while still being + // whitespace-equivalent overall - confirmed against DiffPlex's actual block + // output in this change's verification scratch app, where such a case produces a + // single Conflict block with different OldCount/NewCount. A stricter + // element-wise comparison would misclassify that as a genuine conflict. + static bool IsWhitespaceOnlyDifference(IReadOnlyList oldPieces, IReadOnlyList newPieces) + { + // A genuine deletion (one side has zero pieces in this region) must never be + // treated as "whitespace-only", even if the surviving side's content happens + // to collapse to "" once whitespace runs are stripped - confirmed via a + // synthetic case: base has a whitespace-only separator line, one mod merely + // trims its trailing spaces (still present, still blank), the other mod + // deletes the line outright as part of a real edit. Both sides normalize to + // "", which would otherwise misclassify a genuine content-vs-deletion + // conflict as auto-resolvable and silently discard the deletion. If both + // sides happen to have zero pieces (e.g. both independently deleted the same + // region), this correctly falls through to producing empty conflict markers + // rather than assuming anything about whether DiffPlex would even classify + // that case as Conflict in the first place - see BuildMerge's comment on why + // this library's block metadata isn't assumed trustworthy without checking. + // Flagged in code review, see CLAUDE.md. + if (oldPieces.Count == 0 || newPieces.Count == 0) + return false; + + return NormalizeWhitespace(oldPieces) == NormalizeWhitespace(newPieces); + } + + static string NormalizeWhitespace(IEnumerable pieces) + { + return WhitespaceRun.Replace(string.Concat(pieces), " ").Trim(); + } + + #endregion + } +} diff --git a/WitcherScriptMerger.Core/Tools/FileEncoding.cs b/WitcherScriptMerger.Core/Tools/FileEncoding.cs new file mode 100644 index 0000000..4b62f93 --- /dev/null +++ b/WitcherScriptMerger.Core/Tools/FileEncoding.cs @@ -0,0 +1,93 @@ +using System.IO; +using System.Text; + +namespace WitcherScriptMerger.Tools +{ + // Shared UTF-16LE+BOM normalization, used by every merge engine. KDiff3MergeEngine + // (host project) needs an on-disk temp copy since it shells out to an external exe + // that only accepts file paths; DiffPlexMergeEngine (Core) merges in-process and only + // needs the text itself, via ReadAnyEncoding/WriteUtf16 below - EnsureUtf16File exists + // for the former, kept here too so any future file-based tool can reuse it instead of + // duplicating this logic (this method used to be a private copy inside + // WitcherScriptMerger/Tools/KDiff3.cs::EnsureUtf16Encoding). + // + // Vanilla .ws files are UTF-16LE with a BOM; mod authors' files are often plain + // UTF-8/ASCII with no BOM (confirmed against real files on a live install) - see + // CLAUDE.md's "KDiff3 input encoding" compatibility constraint for why normalizing UP + // to UTF-16LE (never down to UTF-8) matters: the game may not load a merged .ws file + // that isn't UTF-16LE. + public static class FileEncoding + { + // UTF-16LE with BOM - matches vanilla .ws file encoding. Never normalize merge + // output toward UTF-8; the game may not load it. + public static readonly Encoding Utf16LEWithBom = new UnicodeEncoding(bigEndian: false, byteOrderMark: true); + + // A UTF-16LE BOM (FF FE) is also a byte-for-byte prefix of UTF-32LE's own BOM + // (FF FE 00 00) - reading only 2 bytes would misidentify a UTF-32LE file as + // already UTF-16LE, skipping normalization and producing garbled comparison/ + // merge output (flagged in code review, see CLAUDE.md; pre-existing limitation + // carried over unchanged from the original KDiff3.cs::EnsureUtf16Encoding this + // was ported from, now fixed here since it's shared by both merge engines). + // UTF-32 isn't a realistic encoding for real .ws/.xml mod files, but reading 2 + // extra bytes to rule it out is cheap and removes the ambiguity outright. + public static bool HasUtf16LeBom(string path) + { + using (var stream = File.OpenRead(path)) + { + var bom = new byte[4]; + var bytesRead = stream.Read(bom, 0, 4); + if (bytesRead < 2 || bom[0] != 0xFF || bom[1] != 0xFE) + return false; + + var looksLikeUtf32Le = bytesRead >= 4 && bom[2] == 0x00 && bom[3] == 0x00; + return !looksLikeUtf32Le; + } + } + + // Reads a file's text regardless of whether it's UTF-16LE+BOM (vanilla's usual + // encoding) or plain UTF-8/ASCII with no BOM (common for mod authors' files). + // File.ReadAllText(path) without an explicit encoding auto-detects a BOM (UTF-16LE + // included) and falls back to UTF-8 when none is present, which is exactly the two + // cases this codebase needs - and, importantly, StreamReader strips a detected BOM + // from the returned text. Decoding the raw bytes manually with a fixed Encoding + // instead (e.g. Encoding.Unicode.GetString(File.ReadAllBytes(path))) does NOT strip + // it, leaving a stray U+FEFF glued to the first line - confirmed empirically in this + // change's verification scratch app. That stray character would make a UTF-16LE + // vanilla file's first line never equal a UTF-8 mod file's first line, silently + // reproducing the exact class of false conflict this method exists to avoid (see + // CLAUDE.md's baseEffect.ws case). + public static string ReadAnyEncoding(string path) => File.ReadAllText(path); + + // Writes text as UTF-16LE with BOM - the encoding every merge engine's output must + // use, matching vanilla's own encoding (see class remarks above). + public static void WriteUtf16(string path, string text) + { + var dir = Path.GetDirectoryName(path); + if (!string.IsNullOrEmpty(dir) && !Directory.Exists(dir)) + Directory.CreateDirectory(dir); + + File.WriteAllText(path, text, Utf16LEWithBom); + } + + // Ensures an on-disk copy of `file` is UTF-16LE+BOM, writing a temp copy under + // Paths.TempBundleContent\Encoding\\ only when a copy is actually needed. + // For tools that must be handed a file path (KDiff3.exe's command line) rather than + // raw text - an in-process engine that reads/writes strings directly doesn't need + // this at all, just ReadAnyEncoding/WriteUtf16 above. + public static string EnsureUtf16File(FileInfo file, string role) + { + if (HasUtf16LeBom(file.FullName)) + return file.FullName; + + var text = File.ReadAllText(file.FullName, Encoding.UTF8); + + var tempDir = Path.Combine(Paths.TempBundleContent, "Encoding", role); + Directory.CreateDirectory(tempDir); + + var tempPath = Path.Combine(tempDir, file.Name); + File.WriteAllText(tempPath, text, Utf16LEWithBom); + + return tempPath; + } + } +} diff --git a/WitcherScriptMerger.Core/WitcherScriptMerger.Core.csproj b/WitcherScriptMerger.Core/WitcherScriptMerger.Core.csproj index d1e1e8d..ed68f3c 100644 --- a/WitcherScriptMerger.Core/WitcherScriptMerger.Core.csproj +++ b/WitcherScriptMerger.Core/WitcherScriptMerger.Core.csproj @@ -15,6 +15,7 @@ + diff --git a/WitcherScriptMerger.Tests/LiveInstall.cs b/WitcherScriptMerger.Tests/LiveInstall.cs new file mode 100644 index 0000000..2633c70 --- /dev/null +++ b/WitcherScriptMerger.Tests/LiveInstall.cs @@ -0,0 +1,59 @@ +using System; +using System.IO; + +namespace WitcherScriptMerger.Tests +{ + // Opt-in discovery of a real Witcher 3 + WitcherScriptMerger install, for tests that + // cross-check against real recorded data (a live MergeInventory.xml's hashes) or a + // real KDiff3.exe binary. Deliberately NOT a hardcoded path, and deliberately NOT a + // drive-letter scan either: CONTRIBUTING.md requires scrubbing machine-specific + // absolute paths from committed diffs/tests, so discovery here is opt-in only, via an + // environment variable a developer sets locally before running `dotnet test` - never + // a default that would silently vary test behavior across machines or in CI. + public static class LiveInstall + { + // Point this at a Witcher 3 game install directory (the one containing Mods\ and + // WitcherScriptMerger\) to opt in to the tests gated on this class. + public static string GameDirectory + { + get + { + var dir = Environment.GetEnvironmentVariable("WSM_TEST_GAME_DIR"); + return string.IsNullOrWhiteSpace(dir) ? null : dir; + } + } + + public static string MergeInventoryPath + { + get + { + var gameDir = GameDirectory; + if (gameDir == null) + return null; + var path = Path.Combine(gameDir, "WitcherScriptMerger", "MergeInventory.xml"); + return File.Exists(path) ? path : null; + } + } + + public static string ModsDirectory + { + get + { + var gameDir = GameDirectory; + return gameDir == null ? null : Path.Combine(gameDir, "Mods"); + } + } + + public static string Kdiff3ExePath + { + get + { + var gameDir = GameDirectory; + if (gameDir == null) + return null; + var path = Path.Combine(gameDir, "WitcherScriptMerger", "Tools", "KDiff3", "KDiff3.exe"); + return File.Exists(path) ? path : null; + } + } + } +} diff --git a/WitcherScriptMerger.Tests/Tools/DiffPlexMergeEngineTests.cs b/WitcherScriptMerger.Tests/Tools/DiffPlexMergeEngineTests.cs new file mode 100644 index 0000000..252767f --- /dev/null +++ b/WitcherScriptMerger.Tests/Tools/DiffPlexMergeEngineTests.cs @@ -0,0 +1,395 @@ +using System; +using System.IO; +using System.Text; +using WitcherScriptMerger.Inventory; +using WitcherScriptMerger.Tools; +using Xunit; + +namespace WitcherScriptMerger.Tests.Tools +{ + // Regression coverage for DiffPlexMergeEngine - the fixtures CLAUDE.md's history + // calls out specifically: a purely-whitespace-only conflict auto-resolving (mirroring + // KDiff3's --cs "WhiteSpace3FileMergeDefault=2"), a genuine conflict producing + // well-formed conflict markers, and an encoding-mismatch case (UTF-8/no-BOM mod file + // against a UTF-16LE+BOM vanilla file) normalizing correctly - the same class of + // false conflict CLAUDE.md documents as the real baseEffect.ws case. + // + // Deliberately never constructs FileMerger.MergeSource via + // MergeSource.FromFlatFile/FromBundle: those call ModFile.GetModNameFromPath, which + // reads Paths.ModsDirectory, which reads AppState.Settings - and AppState.Settings's + // constructor calls Environment.Exit(1) if it can't find a config file next to the + // entry assembly (see AppSettings.cs), which in a test-host process would abort the + // entire test run, not just fail one test. MergeSource's fields are all public, so + // tests build it directly instead - this exercises DiffPlexMergeEngine exactly the + // same way, since it only ever reads TextFile/Hash/Name off the struct. + public class DiffPlexMergeEngineTests + { + [Fact] + public void BuildMerge_WhitespaceOnlyConflict_AutoResolvesToOldSideVerbatim() + { + var baseText = "function f() {\r\n\tx = 1;\r\n}\r\n"; + var oldText = "function f() {\r\n x = 1;\r\n}\r\n"; // source1: 4-space indent + var newText = "function f() {\r\n x = 1;\r\n}\r\n"; // source2: 2-space indent + + var result = DiffPlexMergeEngine.BuildMerge(baseText, oldText, newText, "modA", "modB"); + + Assert.False(result.HasConflicts); + // WhiteSpace3FileMergeDefault=2 means "always pick input B" in KDiff3 terms, + // which is oldText/source1 here (see KDiff3.BuildArgs' file order: vanilla, + // source1, source2 map to A, B, C) - so the merge should take oldText's exact + // whitespace, not some averaged/normalized form. + Assert.Equal(oldText, result.MergedText); + Assert.DoesNotContain("<<<<<<<", result.MergedText); + } + + [Fact] + public void BuildMerge_WhitespaceOnlyConflict_ToleratesDifferingLineCounts() + { + // One side's whitespace-only edit also happens to add a blank line - still + // purely whitespace once collapsed, so this should still auto-resolve rather + // than being misclassified as a genuine conflict just because the two sides' + // piece counts differ (confirmed against DiffPlex's actual block output in + // this change's verification scratch app before writing this fixture). + var baseText = "a();\r\nx=1;\r\nb();\r\n"; + var oldText = "a();\r\n x=1;\r\n\r\nb();\r\n"; + var newText = "a();\r\n\tx=1;\r\nb();\r\n"; + + var result = DiffPlexMergeEngine.BuildMerge(baseText, oldText, newText, "modA", "modB"); + + Assert.False(result.HasConflicts); + Assert.DoesNotContain("<<<<<<<", result.MergedText); + } + + [Fact] + public void BuildMerge_GenuineConflict_ProducesGitStyleMarkersLabeledWithModNames() + { + var baseText = "x = 1;\r\n"; + var oldText = "x = 2;\r\n"; + var newText = "x = 3;\r\n"; + + var result = DiffPlexMergeEngine.BuildMerge(baseText, oldText, newText, "modA", "modB"); + + Assert.True(result.HasConflicts); + Assert.Equal( + "<<<<<<< modA\r\n" + + "x = 2;\r\n" + + "||||||| Vanilla\r\n" + + "x = 1;\r\n" + + "=======\r\n" + + "x = 3;\r\n" + + ">>>>>>> modB\r\n", + result.MergedText); + } + + [Fact] + public void BuildMerge_NonOverlappingEdits_MergeBothCleanlyWithoutConflict() + { + var baseText = "a();\r\nb();\r\nc();\r\n"; + var oldText = "a();\r\nMOD1();\r\nb();\r\nc();\r\n"; + var newText = "a();\r\nb();\r\nc();\r\nMOD2();\r\n"; + + var result = DiffPlexMergeEngine.BuildMerge(baseText, oldText, newText, "modA", "modB"); + + Assert.False(result.HasConflicts); + Assert.Equal("a();\r\nMOD1();\r\nb();\r\nc();\r\nMOD2();\r\n", result.MergedText); + } + + [Fact] + public void BuildMerge_InterleavedIndependentEdits_ThrowsDiffAlgorithmExceptionRatherThanCorruptingOutput() + { + // Regression test for a confirmed upstream DiffPlex 1.9.0 bug in + // ThreeWayDiffer.CreateThreeWayDiffBlocks (see BuildMerge's own comment for + // the full writeup and CLAUDE.md for measured failure rates): one mod + // inserts a line right after "a();", the other independently changes "b()" + // to "B()". Before the try/catch + post-loop consistency check this fixture + // guards, this exact input silently produced WRONG merged output (base's + // "b();" escaped both the conflict markers and its correct position, while + // "new"'s edit was captured against an empty base region) with no exception + // at all - confirmed via a throwaway scratch console app directly against + // both this engine's BuildMerge and DiffPlex's own official + // ThreeWayDiffer.CreateMerge. This must now come back as a clearly-typed + // failure instead of ever risking a corrupted merge. + var baseText = "a();\r\nb();\r\nc();\r\n"; + var oldText = "a();\r\nnewline();\r\nb();\r\nc();\r\n"; + var newText = "a();\r\nB();\r\nc();\r\n"; + + var ex = Assert.Throws( + () => DiffPlexMergeEngine.BuildMerge(baseText, oldText, newText, "modA", "modB")); + Assert.NotNull(ex.Message); + } + + [Fact] + public void MergeHeadless_InterleavedIndependentEdits_SkipsWithoutWritingAnythingIncludingSidecar() + { + // Same scenario as the BuildMerge-level fixture above, exercised through the + // full MergeHeadless path: since the "conflict marker" content itself would + // have been built from the same untrustworthy piece indices, MergeHeadless + // must not write a sidecar here either - this is the one case where + // DiffPlexMergeEngine can't even offer a conflict-marker starting point. + var dir = CreateTempDir(); + try + { + var vanillaPath = Path.Combine(dir, "vanilla.ws"); + FileEncoding.WriteUtf16(vanillaPath, "a();\r\nb();\r\nc();\r\n"); + var mod1Path = Path.Combine(dir, "mod1.ws"); + FileEncoding.WriteUtf16(mod1Path, "a();\r\nnewline();\r\nb();\r\nc();\r\n"); + var mod2Path = Path.Combine(dir, "mod2.ws"); + FileEncoding.WriteUtf16(mod2Path, "a();\r\nB();\r\nc();\r\n"); + + var outputPath = Path.Combine(dir, "merged.ws"); + + var source1 = new FileMerger.MergeSource { TextFile = new FileInfo(mod1Path), Name = "modA" }; + var source2 = new FileMerger.MergeSource { TextFile = new FileInfo(mod2Path), Name = "modB" }; + + var result = new DiffPlexMergeEngine().MergeHeadless(source1, source2, new FileInfo(vanillaPath), outputPath); + + Assert.Equal(MergeEngineResult.NeedsManualResolution, result); + Assert.False(File.Exists(outputPath)); + Assert.False(File.Exists(DiffPlexMergeEngine.GetConflictMarkerPath(outputPath))); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void BuildMerge_DeletionVersusWhitespaceReformat_IsNotMisclassifiedAsWhitespaceOnly() + { + // A genuine content-vs-deletion conflict must never be silently auto-resolved + // just because the surviving side's content happens to collapse to "" once + // whitespace is stripped. Base has a whitespace-only separator line; mod1 + // merely trims its trailing spaces (still blank); mod2 deletes the line + // outright as part of a real edit. Before the fix, both normalized to "" and + // were treated as equal, silently discarding mod2's deletion. + var baseText = "a();\r\n \r\nb();\r\n"; + var oldText = "a();\r\n\r\nb();\r\n"; // mod1: trims trailing spaces, line stays blank + var newText = "a();\r\nb();\r\n"; // mod2: deletes the blank line entirely + + var result = DiffPlexMergeEngine.BuildMerge(baseText, oldText, newText, "modA", "modB"); + + Assert.True(result.HasConflicts); + Assert.Contains("<<<<<<< modA", result.MergedText); + Assert.Contains(">>>>>>> modB", result.MergedText); + } + + [Fact] + public void MergeHeadless_EncodingMismatch_NormalizesAndProducesUtf16LEWithBomOutput() + { + // Mirrors the real baseEffect.ws false-conflict case CLAUDE.md documents: + // vanilla is UTF-16LE+BOM, one mod file is plain UTF-8 with no BOM. Reading + // raw bytes with a fixed Encoding (rather than the auto-detecting + // File.ReadAllText this engine actually uses) would leave a stray U+FEFF + // glued to the vanilla file's first line, making it never equal the mod + // file's first line and turning this into a spurious conflict - which is + // exactly the failure mode this fixture guards against. + var dir = CreateTempDir(); + try + { + var vanillaPath = Path.Combine(dir, "vanilla.ws"); + FileEncoding.WriteUtf16(vanillaPath, "function f() {\r\n\tx = 1;\r\n}\r\n"); + + var mod1Path = Path.Combine(dir, "mod1.ws"); + File.WriteAllText(mod1Path, "function f() {\r\n\tx = 1;\r\n\ty = 2;\r\n}\r\n", new UTF8Encoding(false)); + + var mod2Path = Path.Combine(dir, "mod2.ws"); + File.WriteAllText(mod2Path, "function f() {\r\n\tx = 1;\r\n}\r\n", new UTF8Encoding(false)); + + var outputPath = Path.Combine(dir, "merged.ws"); + + var source1 = new FileMerger.MergeSource { TextFile = new FileInfo(mod1Path), Name = "modA" }; + var source2 = new FileMerger.MergeSource { TextFile = new FileInfo(mod2Path), Name = "modB" }; + + var result = new DiffPlexMergeEngine().MergeHeadless(source1, source2, new FileInfo(vanillaPath), outputPath); + + Assert.Equal(MergeEngineResult.AutoSolved, result); + Assert.True(File.Exists(outputPath)); + Assert.True(FileEncoding.HasUtf16LeBom(outputPath)); + + var outputBytes = File.ReadAllBytes(outputPath); + Assert.Equal(0xFF, outputBytes[0]); + Assert.Equal(0xFE, outputBytes[1]); + + var mergedText = File.ReadAllText(outputPath); + // Assert.DoesNotContain(string, string) does a culture-aware substring + // search (CompareInfo, not ordinal) - under which U+FEFF, a zero-width + // Unicode format character, is collation-ignorable and reports a "match" + // in any string, even one that doesn't contain it at all (confirmed + // empirically: mergedText.Contains("\uFEFF") - ordinal - is false, while + // Assert.DoesNotContain("\uFEFF", mergedText) still fails). The + // char/IEnumerable overload below does an exact ordinal element + // comparison instead, which is what this assertion actually means. + Assert.DoesNotContain('\uFEFF', mergedText); + Assert.Equal("function f() {\r\n\tx = 1;\r\n\ty = 2;\r\n}\r\n", mergedText); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void MergeHeadless_GenuineConflict_WritesSidecarMarkerFileNotOutputPath() + { + var dir = CreateTempDir(); + try + { + var vanillaPath = Path.Combine(dir, "vanilla.ws"); + FileEncoding.WriteUtf16(vanillaPath, "x = 1;\r\n"); + var mod1Path = Path.Combine(dir, "mod1.ws"); + FileEncoding.WriteUtf16(mod1Path, "x = 2;\r\n"); + var mod2Path = Path.Combine(dir, "mod2.ws"); + FileEncoding.WriteUtf16(mod2Path, "x = 3;\r\n"); + + var outputPath = Path.Combine(dir, "merged.ws"); + + var source1 = new FileMerger.MergeSource { TextFile = new FileInfo(mod1Path), Name = "modA" }; + var source2 = new FileMerger.MergeSource { TextFile = new FileInfo(mod2Path), Name = "modB" }; + + var result = new DiffPlexMergeEngine().MergeHeadless(source1, source2, new FileInfo(vanillaPath), outputPath); + + Assert.Equal(MergeEngineResult.NeedsManualResolution, result); + + // Never poison the real output path with conflict markers - see + // DiffPlexMergeEngine.MergeHeadless's comment for why (it would + // permanently block every future retry of this same conflict, since + // FileMerger's headless callers treat any existing file at outputPath as + // "already merged, don't overwrite"). + Assert.False(File.Exists(outputPath)); + + var sidecarPath = DiffPlexMergeEngine.GetConflictMarkerPath(outputPath); + Assert.True(File.Exists(sidecarPath)); + Assert.True(FileEncoding.HasUtf16LeBom(sidecarPath)); + + var sidecarText = File.ReadAllText(sidecarPath); + Assert.Contains("<<<<<<< modA", sidecarText); + Assert.Contains(">>>>>>> modB", sidecarText); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void MergeHeadless_RetryAfterConflictThatNowAutoSolves_RemovesStaleSidecar() + { + // A conflicting retry leaves a sidecar marker file behind (see the fixture + // above). If a later retry against updated inputs auto-solves cleanly, the + // stale sidecar from the earlier failed attempt must not be left sitting next + // to the fresh output indefinitely - MergeHeadless deletes it on the + // AutoSolved path specifically to avoid that. + var dir = CreateTempDir(); + try + { + var vanillaPath = Path.Combine(dir, "vanilla.ws"); + FileEncoding.WriteUtf16(vanillaPath, "x = 1;\r\n"); + var mod1Path = Path.Combine(dir, "mod1.ws"); + FileEncoding.WriteUtf16(mod1Path, "x = 2;\r\n"); + var mod2Path = Path.Combine(dir, "mod2.ws"); + FileEncoding.WriteUtf16(mod2Path, "x = 3;\r\n"); + + var outputPath = Path.Combine(dir, "merged.ws"); + var sidecarPath = DiffPlexMergeEngine.GetConflictMarkerPath(outputPath); + + var source1 = new FileMerger.MergeSource { TextFile = new FileInfo(mod1Path), Name = "modA" }; + var source2 = new FileMerger.MergeSource { TextFile = new FileInfo(mod2Path), Name = "modB" }; + + var engine = new DiffPlexMergeEngine(); + Assert.Equal(MergeEngineResult.NeedsManualResolution, engine.MergeHeadless(source1, source2, new FileInfo(vanillaPath), outputPath)); + Assert.True(File.Exists(sidecarPath)); + + // Now "fix" mod2 so this pairing no longer conflicts, and retry. + FileEncoding.WriteUtf16(mod2Path, "x = 2;\r\n"); + var retrySource2 = new FileMerger.MergeSource { TextFile = new FileInfo(mod2Path), Name = "modB" }; + + Assert.Equal(MergeEngineResult.AutoSolved, engine.MergeHeadless(source1, retrySource2, new FileInfo(vanillaPath), outputPath)); + Assert.True(File.Exists(outputPath)); + Assert.False(File.Exists(sidecarPath)); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void MergeHeadless_NoVanillaFile_SkipsWithoutWritingAnything() + { + // A 3-way merge is meaningless without a base - see MergeHeadless's comment + // for the empty-base bug this guard exists to avoid (confirmed empirically: + // feeding ThreeWayDiffer an empty base string produces zero diff blocks and a + // "successful" empty merge, i.e. it would silently produce an empty output + // file instead of refusing). + var dir = CreateTempDir(); + try + { + var mod1Path = Path.Combine(dir, "mod1.ws"); + FileEncoding.WriteUtf16(mod1Path, "x = 2;\r\n"); + var mod2Path = Path.Combine(dir, "mod2.ws"); + FileEncoding.WriteUtf16(mod2Path, "x = 3;\r\n"); + + var outputPath = Path.Combine(dir, "merged.ws"); + var missingVanilla = new FileInfo(Path.Combine(dir, "does-not-exist.ws")); + + var source1 = new FileMerger.MergeSource { TextFile = new FileInfo(mod1Path), Name = "modA" }; + var source2 = new FileMerger.MergeSource { TextFile = new FileInfo(mod2Path), Name = "modB" }; + + var result = new DiffPlexMergeEngine().MergeHeadless(source1, source2, missingVanilla, outputPath); + + Assert.Equal(MergeEngineResult.NeedsManualResolution, result); + Assert.False(File.Exists(outputPath)); + Assert.False(File.Exists(DiffPlexMergeEngine.GetConflictMarkerPath(outputPath))); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void Merge_Interactive_NeverReturnsNeedsManualResolution() + { + // IMergeEngine.Merge's contract explicitly forbids ever returning + // NeedsManualResolution (that's a headless-only concept) - DiffPlexMergeEngine + // has no UI to resolve a conflict interactively, so a genuine conflict must + // come back as Failed instead. + var dir = CreateTempDir(); + try + { + var vanillaPath = Path.Combine(dir, "vanilla.ws"); + FileEncoding.WriteUtf16(vanillaPath, "x = 1;\r\n"); + var mod1Path = Path.Combine(dir, "mod1.ws"); + FileEncoding.WriteUtf16(mod1Path, "x = 2;\r\n"); + var mod2Path = Path.Combine(dir, "mod2.ws"); + FileEncoding.WriteUtf16(mod2Path, "x = 3;\r\n"); + + var outputPath = Path.Combine(dir, "merged.ws"); + + var source1 = new FileMerger.MergeSource { TextFile = new FileInfo(mod1Path), Name = "modA" }; + var source2 = new FileMerger.MergeSource { TextFile = new FileInfo(mod2Path), Name = "modB" }; + + var result = new DiffPlexMergeEngine().Merge(source1, source2, new FileInfo(vanillaPath), outputPath); + + Assert.Equal(MergeEngineResult.Failed, result); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void ValidateExePath_AlwaysTrue_NoExternalBinaryToValidate() + { + Assert.True(new DiffPlexMergeEngine().ValidateExePath()); + } + + static string CreateTempDir() + { + var dir = Path.Combine(Path.GetTempPath(), "wsm-tests-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(dir); + return dir; + } + } +} diff --git a/WitcherScriptMerger.Tests/Tools/FileEncodingTests.cs b/WitcherScriptMerger.Tests/Tools/FileEncodingTests.cs new file mode 100644 index 0000000..c2f7db8 --- /dev/null +++ b/WitcherScriptMerger.Tests/Tools/FileEncodingTests.cs @@ -0,0 +1,180 @@ +using System; +using System.IO; +using System.Text; +using WitcherScriptMerger.Tools; +using Xunit; + +namespace WitcherScriptMerger.Tests.Tools +{ + // Direct coverage of the shared encoding helper both merge engines rely on - see + // FileEncoding.cs's remarks for why File.ReadAllText(path) (auto-detecting) is used + // for reads instead of decoding raw bytes with a fixed Encoding. + // + // Deliberately never references WitcherScriptMerger.Paths beyond its TempBundleContent + // const (a compile-time literal, so referencing it can't trigger Paths' static field + // initializers) - see DiffPlexMergeEngineTests' class remarks for why touching + // Paths/AppState.Settings from a test host is unsafe. + public class FileEncodingTests + { + [Fact] + public void HasUtf16LeBom_DetectsBomCorrectly() + { + var dir = CreateTempDir(); + try + { + var utf16Path = Path.Combine(dir, "utf16.ws"); + File.WriteAllText(utf16Path, "hello", new UnicodeEncoding(false, true)); + var utf8Path = Path.Combine(dir, "utf8.ws"); + File.WriteAllText(utf8Path, "hello", new UTF8Encoding(false)); + + Assert.True(FileEncoding.HasUtf16LeBom(utf16Path)); + Assert.False(FileEncoding.HasUtf16LeBom(utf8Path)); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void HasUtf16LeBom_DoesNotMisidentifyUtf32LeAsUtf16LE() + { + // UTF-16LE's BOM (FF FE) is a byte-for-byte prefix of UTF-32LE's own BOM + // (FF FE 00 00) - a 2-byte-only check would misidentify this file and skip + // normalization, producing garbled output. Flagged in code review; see + // FileEncoding.HasUtf16LeBom's remarks. + var dir = CreateTempDir(); + try + { + var utf32Path = Path.Combine(dir, "utf32.ws"); + File.WriteAllText(utf32Path, "hello", new UTF32Encoding(bigEndian: false, byteOrderMark: true)); + + Assert.False(FileEncoding.HasUtf16LeBom(utf32Path)); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void HasUtf16LeBom_TinyBomOnlyFileStillDetectedAsUtf16LE() + { + // A file containing only the 2-byte UTF-16LE BOM and no content at all - the + // 4-byte read this method now does for the UTF-32LE disambiguation above must + // not require 4 bytes to actually exist on disk. + var dir = CreateTempDir(); + try + { + var path = Path.Combine(dir, "bomonly.ws"); + File.WriteAllBytes(path, new byte[] { 0xFF, 0xFE }); + + Assert.True(FileEncoding.HasUtf16LeBom(path)); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void ReadAnyEncoding_AgreesRegardlessOfSourceEncodingAndStripsBom() + { + var dir = CreateTempDir(); + try + { + var text = "line1\r\nline2\r\n"; + var utf16Path = Path.Combine(dir, "utf16.ws"); + File.WriteAllText(utf16Path, text, new UnicodeEncoding(false, true)); + var utf8Path = Path.Combine(dir, "utf8.ws"); + File.WriteAllText(utf8Path, text, new UTF8Encoding(false)); + + var fromUtf16 = FileEncoding.ReadAnyEncoding(utf16Path); + var fromUtf8 = FileEncoding.ReadAnyEncoding(utf8Path); + + Assert.Equal(text, fromUtf16); + Assert.Equal(text, fromUtf8); + Assert.Equal(fromUtf16, fromUtf8); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void WriteUtf16_ProducesExactBomBytesAndCreatesMissingDirectory() + { + var dir = CreateTempDir(); + try + { + var path = Path.Combine(dir, "nested", "out.ws"); + FileEncoding.WriteUtf16(path, "content\r\n"); + + var bytes = File.ReadAllBytes(path); + Assert.Equal(0xFF, bytes[0]); + Assert.Equal(0xFE, bytes[1]); + Assert.Equal("content\r\n", File.ReadAllText(path)); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void EnsureUtf16File_ReturnsOriginalPathWhenAlreadyUtf16LE() + { + var dir = CreateTempDir(); + try + { + var path = Path.Combine(dir, "vanilla.ws"); + File.WriteAllText(path, "content", new UnicodeEncoding(false, true)); + + var result = FileEncoding.EnsureUtf16File(new FileInfo(path), "TestRole"); + + Assert.Equal(path, result); + } + finally + { + Directory.Delete(dir, true); + } + } + + [Fact] + public void EnsureUtf16File_WritesNormalizedTempCopyWhenNotUtf16LE() + { + var dir = CreateTempDir(); + string tempCopyDir = null; + try + { + var path = Path.Combine(dir, "mod.ws"); + File.WriteAllText(path, "content", new UTF8Encoding(false)); + + var result = FileEncoding.EnsureUtf16File(new FileInfo(path), "TestRole"); + tempCopyDir = Path.GetDirectoryName(result); + + Assert.NotEqual(path, result); + Assert.True(FileEncoding.HasUtf16LeBom(result)); + Assert.Equal("content", File.ReadAllText(result)); + } + finally + { + Directory.Delete(dir, true); + // EnsureUtf16File's temp copy goes under the relative "tempbundlecontent" + // directory (Paths.TempBundleContent's literal value), not under `dir` - + // clean it up too so repeated test runs don't accumulate copies, matching + // CLAUDE.md's own noted precedent for clearing this directory between runs. + if (tempCopyDir != null && Directory.Exists(tempCopyDir)) + Directory.Delete(tempCopyDir, true); + } + } + + static string CreateTempDir() + { + var dir = Path.Combine(Path.GetTempPath(), "wsm-tests-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(dir); + return dir; + } + } +} diff --git a/WitcherScriptMerger.Tests/Tools/HasherTests.cs b/WitcherScriptMerger.Tests/Tools/HasherTests.cs new file mode 100644 index 0000000..1d95915 --- /dev/null +++ b/WitcherScriptMerger.Tests/Tools/HasherTests.cs @@ -0,0 +1,166 @@ +using System; +using System.IO; +using System.IO.Hashing; +using System.Text; +using System.Xml.Linq; +using WitcherScriptMerger.Tools; +using Xunit; + +namespace WitcherScriptMerger.Tests.Tools +{ + // Regression coverage for Tools/Hasher.cs. CLAUDE.md's Compatibility constraints call + // this format load-bearing: MergeInventory.xml compares these hashes by plain string + // equality to detect when a mod file has changed since it was last merged, so any + // change to Hasher.ComputeHash - not just its numeric result, but its exact output + // format - would silently make every already-recorded merge "go stale". Expected + // values below were computed by actually running Hasher's exact algorithm against + // synthetic inputs in a disposable scratch console app (this repo's own established + // verification pattern - see CLAUDE.md's Tests section), not hand-derived, to avoid + // transcription error. + public class HasherTests + { + [Fact] + public void ComputeHash_EmptyFile_MatchesKnownXxHash32Vector() + { + // xxHash32 of a zero-length input with seed 0 is a well-known published test + // vector (0x02CC5D05) - this confirms Hasher's seed/algorithm choice hasn't + // silently drifted, independent of the scratch-app cross-check this class + // otherwise relies on. + var path = WriteTempFile(Array.Empty()); + try + { + Assert.Equal("2CC5D05", Hasher.ComputeHash(path)); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public void ComputeHash_SmallAsciiContent_MatchesRecordedValue() + { + var path = WriteTempFile(Encoding.ASCII.GetBytes("abc")); + try + { + Assert.Equal("32D153FF", Hasher.ComputeHash(path)); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public void ComputeHash_DoesNotZeroPadLeadingNibble() + { + // ComputeHash formats with "{0:X}", which never zero-pads - confirmed against + // a real recorded hash in a live install's MergeInventory.xml, which contains + // Hash="D830FD" (6 hex digits, i.e. unpadded from the usual 8). Reformatting + // to a fixed-width "X8" would be exactly the kind of silent output-format + // change CLAUDE.md warns would make every already-recorded merge hash + // comparison fail. + var path = WriteTempFile(Encoding.ASCII.GetBytes("candidate-41")); + try + { + var hash = Hasher.ComputeHash(path); + Assert.Equal("19AD22", hash); + Assert.True(hash.Length < 8); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public void ComputeHash_InputLargerThanReadBuffer_MatchesOneShotHash() + { + // ComputeHash streams through an 81920-byte buffer in a loop instead of + // hashing the whole file in a single call - this independently verifies that + // chunking via repeated XxHash32.Append calls produces the same result as + // hashing the same bytes in one call, i.e. the loop's chunk-boundary handling + // is correct. 100000 bytes deliberately crosses the 81920-byte boundary. + var bytes = new byte[100000]; + for (var i = 0; i < bytes.Length; ++i) + bytes[i] = (byte)(i % 251); + + var path = WriteTempFile(bytes); + try + { + var expected = string.Format("{0:X}", XxHash32.HashToUInt32(bytes)); + Assert.Equal(expected, Hasher.ComputeHash(path)); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public void ComputeHash_MissingFile_ThrowsFileNotFoundException() + { + var path = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N") + ".ws"); + Assert.Throws(() => Hasher.ComputeHash(path)); + } + + // Cross-checks a freshly computed hash against a real value recorded in a live + // install's MergeInventory.xml, per this repo's Tests precedent (CLAUDE.md / + // CONTRIBUTING.md). Gated entirely on WSM_TEST_GAME_DIR (see LiveInstall.cs) - + // silently does nothing when unset, so it never fails a machine or CI run that + // doesn't have a live install configured. Writes a one-line Console message + // either way (visible via `dotnet test --logger "console;verbosity=detailed"`) + // stating whether it actually cross-checked anything - a silent no-assertion + // pass here would look identical to a real cross-check in every other way, + // which is exactly the ambiguity CLAUDE.md's Tests section warns about. + [Fact] + public void ComputeHash_LiveInstallCrossCheck() + { + var inventoryPath = LiveInstall.MergeInventoryPath; + var modsDir = LiveInstall.ModsDirectory; + if (inventoryPath == null || modsDir == null) + { + Console.WriteLine("ComputeHash_LiveInstallCrossCheck: WSM_TEST_GAME_DIR not set or no MergeInventory.xml found - skipped."); + return; + } + + var doc = XDocument.Load(inventoryPath); + foreach (var mergeEl in doc.Root.Elements("Merge")) + { + var relativePath = (string)mergeEl.Element("RelativePath"); + if (relativePath == null) + continue; + + foreach (var modEl in mergeEl.Elements("IncludedMod")) + { + var recordedHash = (string)modEl.Attribute("Hash"); + var modName = modEl.Value; + if (recordedHash == null || string.IsNullOrEmpty(modName)) + continue; + + var modFilePath = Path.Combine(modsDir, modName, "content", "scripts", relativePath); + if (!File.Exists(modFilePath)) + continue; + + // One real cross-check is enough to catch a format regression - + // return as soon as we find (and assert against) one. + Console.WriteLine($"ComputeHash_LiveInstallCrossCheck: cross-checked {modName}'s {relativePath} against recorded hash {recordedHash}."); + Assert.Equal(recordedHash, Hasher.ComputeHash(modFilePath)); + return; + } + } + + // Reached only when a live inventory exists but none of its recorded mod + // source files are present on disk anymore - nothing to cross-check against, + // so this intentionally asserts nothing rather than failing. + Console.WriteLine("ComputeHash_LiveInstallCrossCheck: found a live MergeInventory.xml, but none of its recorded mod source files are still on disk - nothing cross-checked."); + } + + static string WriteTempFile(byte[] bytes) + { + var path = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N") + ".bin"); + File.WriteAllBytes(path, bytes); + return path; + } + } +} diff --git a/WitcherScriptMerger.Tests/Tools/KDiff3CrossCheckTests.cs b/WitcherScriptMerger.Tests/Tools/KDiff3CrossCheckTests.cs new file mode 100644 index 0000000..1d8eac8 --- /dev/null +++ b/WitcherScriptMerger.Tests/Tools/KDiff3CrossCheckTests.cs @@ -0,0 +1,127 @@ +using System; +using System.Diagnostics; +using System.IO; +using WitcherScriptMerger.Tools; +using Xunit; + +namespace WitcherScriptMerger.Tests.Tools +{ + // Optional A/B check: does DiffPlexMergeEngine agree with the real KDiff3.exe binary + // on auto-solvable merges? Gated entirely on WSM_TEST_GAME_DIR (see LiveInstall.cs) - + // never runs by default, and never fails a run where it's unset; per this repo's + // CONTRIBUTING.md, a committed test must not require or hardcode a machine-specific + // path. + // + // Deliberately narrow in scope: only auto-solvable scenarios are compared here + // (whitespace-only, and non-overlapping edits). A genuine two-sided conflict is NOT + // cross-checked against the real binary from this test project, because safely + // automating KDiff3 headlessly for that case needs the window-persistence detection + // documented in CLAUDE.md's compatibility constraints (a ~250ms poll interval that's + // itself load-bearing, and a window that can't be hidden without hanging the merge + // entirely) - that logic (Win32 P/Invoke) lives in the host project's Tools/KDiff3.cs, + // which this Core-only test project intentionally doesn't reference. Below uses a + // single bounded Process.WaitForExit with a kill-on-timeout fallback instead, safe + // only because both scenarios here are designed to be cleanly auto-solvable - per + // CLAUDE.md, an untouched, auto-solvable KDiff3 launch reliably exits in a few + // seconds regardless of file size. + // + // Running this locally (WSM_TEST_GAME_DIR set) will briefly show KDiff3's window and + // steal foreground focus, twice - the same documented behavior CLAUDE.md describes + // for the real headless CLI path. That's expected, not a bug in this test. + public class KDiff3CrossCheckTests + { + [Fact] + public void WhitespaceOnlyConflict_RealKDiff3AgreesWithDiffPlexEngine() + { + var kdiff3Path = LiveInstall.Kdiff3ExePath; + if (kdiff3Path == null) + return; + + var baseText = "function f() {\r\n\tx = 1;\r\n}\r\n"; + var oldText = "function f() {\r\n x = 1;\r\n}\r\n"; + var newText = "function f() {\r\n x = 1;\r\n}\r\n"; + + RunComparison(kdiff3Path, baseText, oldText, newText); + } + + [Fact] + public void NonOverlappingEdits_RealKDiff3AgreesWithDiffPlexEngine() + { + var kdiff3Path = LiveInstall.Kdiff3ExePath; + if (kdiff3Path == null) + return; + + var baseText = "a();\r\nb();\r\nc();\r\n"; + var oldText = "a();\r\nMOD1();\r\nb();\r\nc();\r\n"; + var newText = "a();\r\nb();\r\nc();\r\nMOD2();\r\n"; + + RunComparison(kdiff3Path, baseText, oldText, newText); + } + + static void RunComparison(string kdiff3Path, string baseText, string oldText, string newText) + { + var dir = Path.Combine(Path.GetTempPath(), "wsm-tests-kdiff3-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(dir); + try + { + var vanillaPath = Path.Combine(dir, "vanilla.ws"); + var oldPath = Path.Combine(dir, "old.ws"); + var newPath = Path.Combine(dir, "new.ws"); + var kdiff3OutPath = Path.Combine(dir, "kdiff3-out.ws"); + + FileEncoding.WriteUtf16(vanillaPath, baseText); + FileEncoding.WriteUtf16(oldPath, oldText); + FileEncoding.WriteUtf16(newPath, newText); + + var kdiff3Text = RunRealKDiff3(kdiff3Path, vanillaPath, oldPath, newPath, kdiff3OutPath); + if (kdiff3Text == null) + return; // didn't exit cleanly within the bounded wait - see RunRealKDiff3 + + var diffPlexResult = DiffPlexMergeEngine.BuildMerge(baseText, oldText, newText, "old", "new"); + + Assert.False(diffPlexResult.HasConflicts); + Assert.Equal(kdiff3Text, diffPlexResult.MergedText); + } + finally + { + Directory.Delete(dir, true); + } + } + + // Invokes the real kdiff3.exe with the same --cs settings KDiff3.BuildArgs uses + // (WhiteSpace3FileMergeDefault=2, LineEndStyle=1) plus --auto, via the two-string + // Process.Start(fileName, argsString) overload - CLAUDE.md's compatibility notes + // call out that this specific overload (not a shell) is the one that matches this + // app's real invocation path. Returns null (never throws/fails) if the process + // doesn't exit cleanly within the bounded wait, so a flaky or unexpectedly slow + // KDiff3 run degrades to "comparison skipped", not a build-breaking test failure. + static string RunRealKDiff3(string kdiff3Path, string vanillaPath, string oldPath, string newPath, string outputPath) + { + var args = + $"\"{vanillaPath}\" \"{oldPath}\" \"{newPath}\" " + + $"-o \"{outputPath}\" " + + "--cs \"WhiteSpace3FileMergeDefault=2\" " + + "--cs \"CreateBakFiles=0\" " + + "--cs \"LineEndStyle=1\" " + + "--auto"; + + var proc = Process.Start(kdiff3Path, args); + try + { + if (!proc.WaitForExit(15000)) + { + try { proc.Kill(entireProcessTree: true); } catch { } + return null; + } + + return (proc.ExitCode == 0 && File.Exists(outputPath)) + ? File.ReadAllText(outputPath) + : null; + } + finally + { + proc.Dispose(); + } + } + } +} diff --git a/WitcherScriptMerger.Tests/WitcherScriptMerger.Tests.csproj b/WitcherScriptMerger.Tests/WitcherScriptMerger.Tests.csproj new file mode 100644 index 0000000..25aa5c4 --- /dev/null +++ b/WitcherScriptMerger.Tests/WitcherScriptMerger.Tests.csproj @@ -0,0 +1,24 @@ + + + + net10.0 + + disable + disable + false + + + + + + + + + + + + + + diff --git a/WitcherScriptMerger.sln b/WitcherScriptMerger.sln index a2f5726..30174bb 100644 --- a/WitcherScriptMerger.sln +++ b/WitcherScriptMerger.sln @@ -7,20 +7,54 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "WitcherScriptMerger", "Witc EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "WitcherScriptMerger.Core", "WitcherScriptMerger.Core\WitcherScriptMerger.Core.csproj", "{339EF28F-A6D3-4878-A03E-0EE691B74FDE}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "WitcherScriptMerger.Tests", "WitcherScriptMerger.Tests\WitcherScriptMerger.Tests.csproj", "{401B0543-E5DB-4AAA-86BF-A7B84E6C6175}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU + Debug|x64 = Debug|x64 + Debug|x86 = Debug|x86 Release|Any CPU = Release|Any CPU + Release|x64 = Release|x64 + Release|x86 = Release|x86 EndGlobalSection GlobalSection(ProjectConfigurationPlatforms) = postSolution {B0417CBE-445D-47A0-8502-717BCFE63013}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {B0417CBE-445D-47A0-8502-717BCFE63013}.Debug|Any CPU.Build.0 = Debug|Any CPU + {B0417CBE-445D-47A0-8502-717BCFE63013}.Debug|x64.ActiveCfg = Debug|Any CPU + {B0417CBE-445D-47A0-8502-717BCFE63013}.Debug|x64.Build.0 = Debug|Any CPU + {B0417CBE-445D-47A0-8502-717BCFE63013}.Debug|x86.ActiveCfg = Debug|Any CPU + {B0417CBE-445D-47A0-8502-717BCFE63013}.Debug|x86.Build.0 = Debug|Any CPU {B0417CBE-445D-47A0-8502-717BCFE63013}.Release|Any CPU.ActiveCfg = Release|Any CPU {B0417CBE-445D-47A0-8502-717BCFE63013}.Release|Any CPU.Build.0 = Release|Any CPU + {B0417CBE-445D-47A0-8502-717BCFE63013}.Release|x64.ActiveCfg = Release|Any CPU + {B0417CBE-445D-47A0-8502-717BCFE63013}.Release|x64.Build.0 = Release|Any CPU + {B0417CBE-445D-47A0-8502-717BCFE63013}.Release|x86.ActiveCfg = Release|Any CPU + {B0417CBE-445D-47A0-8502-717BCFE63013}.Release|x86.Build.0 = Release|Any CPU {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Debug|Any CPU.Build.0 = Debug|Any CPU + {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Debug|x64.ActiveCfg = Debug|Any CPU + {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Debug|x64.Build.0 = Debug|Any CPU + {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Debug|x86.ActiveCfg = Debug|Any CPU + {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Debug|x86.Build.0 = Debug|Any CPU {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Release|Any CPU.ActiveCfg = Release|Any CPU {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Release|Any CPU.Build.0 = Release|Any CPU + {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Release|x64.ActiveCfg = Release|Any CPU + {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Release|x64.Build.0 = Release|Any CPU + {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Release|x86.ActiveCfg = Release|Any CPU + {339EF28F-A6D3-4878-A03E-0EE691B74FDE}.Release|x86.Build.0 = Release|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Debug|Any CPU.Build.0 = Debug|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Debug|x64.ActiveCfg = Debug|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Debug|x64.Build.0 = Debug|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Debug|x86.ActiveCfg = Debug|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Debug|x86.Build.0 = Debug|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Release|Any CPU.ActiveCfg = Release|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Release|Any CPU.Build.0 = Release|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Release|x64.ActiveCfg = Release|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Release|x64.Build.0 = Release|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Release|x86.ActiveCfg = Release|Any CPU + {401B0543-E5DB-4AAA-86BF-A7B84E6C6175}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/WitcherScriptMerger/App.config b/WitcherScriptMerger/App.config index 72fc1c6..35a78ea 100644 --- a/WitcherScriptMerger/App.config +++ b/WitcherScriptMerger/App.config @@ -27,6 +27,12 @@ KDiff3Path Where KDiff3.exe is located QuickBmsPath Where quickbms.exe is located QuickBmsPluginPath Where the witcher3.bms plugin for QuickBMS is located WccLitePath Where wcc_lite.exe is located + +MergeEngine Which text-merge engine to use: "kdiff3" (default) or "diffplex". + DiffPlex is an in-process alternative to KDiff3 that needs no + external binary - see CLAUDE.md's Interactive vs. headless split + section. Not the default yet; still being verified against KDiff3 + on real conflicts. --> @@ -52,6 +58,7 @@ WccLitePath Where wcc_lite.exe is located + diff --git a/WitcherScriptMerger/Program.cs b/WitcherScriptMerger/Program.cs index 71f8e56..47f2957 100644 --- a/WitcherScriptMerger/Program.cs +++ b/WitcherScriptMerger/Program.cs @@ -66,11 +66,19 @@ public static MergeInventory Inventory [STAThread] static void Main(string[] args) { - // The one real IMergeEngine implementation, supplied here since it needs - // Tools/KDiff3.cs's Win32 P/Invoke (host-only) - see Tools/IMergeEngine.cs. + // The default IMergeEngine implementation, supplied here since KDiff3MergeEngine + // needs Tools/KDiff3.cs's Win32 P/Invoke (host-only) - see Tools/IMergeEngine.cs. // Must be set before anything calls Paths.ValidateDependencyPaths() or - // constructs a FileMerger, in any of the GUI/CLI/MCP paths below. - AppState.MergeEngine = new KDiff3MergeEngine(); + // constructs a FileMerger, in any of the GUI/CLI/MCP paths below. The + // "MergeEngine" App.config setting can switch to DiffPlexMergeEngine (Core, no + // external binary) instead - not the default yet, since it hasn't been + // cross-checked against KDiff3 on enough real conflicting files (see CLAUDE.md + // and the PR that introduced it); this switch exists so it can be tried without + // recompiling, not as a signal that it's considered production-ready. + AppState.MergeEngine = + Settings.Get("MergeEngine").EqualsIgnoreCase("diffplex") + ? (IMergeEngine)new DiffPlexMergeEngine() + : new KDiff3MergeEngine(); if (args.Length > 0) { diff --git a/WitcherScriptMerger/Tools/KDiff3.cs b/WitcherScriptMerger/Tools/KDiff3.cs index d1cc945..2a76ad5 100644 --- a/WitcherScriptMerger/Tools/KDiff3.cs +++ b/WitcherScriptMerger/Tools/KDiff3.cs @@ -211,9 +211,9 @@ static string BuildArgs( { hasVanillaVersion = (vanillaFile != null && vanillaFile.Exists); - var vanillaPath = hasVanillaVersion ? EnsureUtf16Encoding(vanillaFile, "Vanilla") : null; - var source1Path = EnsureUtf16Encoding(source1.TextFile, "Source1"); - var source2Path = EnsureUtf16Encoding(source2.TextFile, "Source2"); + var vanillaPath = hasVanillaVersion ? FileEncoding.EnsureUtf16File(vanillaFile, "Vanilla") : null; + var source1Path = FileEncoding.EnsureUtf16File(source1.TextFile, "Source1"); + var source2Path = FileEncoding.EnsureUtf16File(source2.TextFile, "Source2"); var args = (hasVanillaVersion ? "\"" + vanillaPath + "\" " @@ -298,31 +298,5 @@ static class NativeMethods public static extern bool AttachThreadInput(uint idAttach, uint idAttachTo, bool fAttach); } - // Vanilla .ws files are UTF-16LE with a BOM, but mod authors' files are often - // plain UTF-8/ASCII with no BOM. KDiff3 has no way to be told each input's - // encoding on the command line, so a mismatch makes it treat an entire file as - // unmatchable and fall back to manual (GUI) conflict resolution instead of - // auto-solving. Normalizing non-UTF-16LE inputs up to match vanilla's encoding - // (never down to UTF-8, which the game might not load) fixes this without - // touching the original files. - static string EnsureUtf16Encoding(FileInfo file, string role) - { - using (var stream = File.OpenRead(file.FullName)) - { - var bom = new byte[2]; - if (stream.Read(bom, 0, 2) == 2 && bom[0] == 0xFF && bom[1] == 0xFE) - return file.FullName; - } - - var text = File.ReadAllText(file.FullName, Encoding.UTF8); - - var tempDir = Path.Combine(Paths.TempBundleContent, "Encoding", role); - Directory.CreateDirectory(tempDir); - - var tempPath = Path.Combine(tempDir, file.Name); - File.WriteAllText(tempPath, text, Encoding.Unicode); - - return tempPath; - } } }