解耦修改器核心、存檔與兵種預設流程(含程式碼審查修正)

AI_HANDOFF.md

@@ -1,5 +1,32 @@
 # AI Handoff - Live Project Memory
 
+## Decoupling code-review fixes (2026-07-15)
+
+- Reviewed the full `4acd542` decoupling commit line-by-line on `claude/decoupled-code-review-3ly42q` (fast-forwarded from `codex/decouple-core-and-file-workflows`) and applied the surviving findings: `TroopPresetCodec.Write` now uses invariant shortest round-trip formatting instead of `0.##`, so exported presets keep more than two decimals (regression added to `TroopPresetCodecTests`); the `TroopPresetForm` export loop now checks every `TryReadValidatedCell` result like the apply path instead of discarding them; `SaveBackupService.RestoreBackup` no longer embeds a Chinese prefix in the temp-cleanup warning (the UI's `LogRestoreBackupCleanupFailed` prefix was being doubled) and restored the pre-decoupling double-failure semantics — a rollback failure is reported through a new optional `onRollbackFailure` callback (UI logs the legacy `還原存檔失敗後回復原存檔也失敗:` message) while the original restore exception is rethrown instead of an `AggregateException`; `ModifierForm.Data.cs` builds `BuildCurrentPatchProfile` once per default-stats reload rather than once per unit row.
+- Intentionally NOT changed from the review: `CleanEparaBaseline` trailing-whitespace trimming (comment-only, epara is never read back for detection, and restoring it would reintroduce `git diff --check` noise) and `TeamDatFeatureDetector` keeping partial evidence plus always running the RomanEndless EXE reconciliation on exceptions (both are improvements over the monolith).
+- Verification status: this session has no local .NET SDK; `git diff --check` is clean and the changes are syntax-reviewed only. Build and full xUnit must be confirmed by CI on the pushed branch before claiming green.
+
+## Typed feature catalog decoupling (2026-07-15)
+
+- Completed the recommended UI/file-boundary follow-up. `SaveBackupService`/manifest models now own save scanning, preview reads, ZIP creation, transactional restore, cache invalidation, and deletes; `ModifierForm.SaveManager.cs` dropped from 617 to 247 lines and retains only DataGridView/dialog/log coordination. `TroopPresetCodec` owns 4/6/9-column compatibility parsing and six editable-field serialization, while `UnitStatsEditorService` removes `TroopPresetForm`'s direct dependency on `ModifierForm` and centralizes independent-field normalization. Temporary-directory tests cover create/scan/preview/restore/delete, legacy manifest-less ZIP names, unsafe names, and rejection of a ZIP without `save.ini` while preserving the existing save; codec tests cover all supported row shapes and round trip. A pre-commit focused run exposed a hidden test-order dependency: several core readers called `Encoding.GetEncoding(1251)` before `Program` or another test registered the code-page provider. Core readers now consistently use `PatchText.GameEncoding`, whose static initializer registers the provider; the formerly isolated failure now passes 5/5. Full isolated xUnit passed 186/186; isolated Release build completed with 0 warnings/errors; `git diff --check` is clean; handoff remains UTF-8 BOM. The first normal Release-path command and a `UseAppHost=false` retry were blocked by manifest-elevated smoke PID 35644 locking the EXE/DLL, so build/test validation used SDK `--artifacts-path` under `%TEMP%`; do not mistake this lock for a code regression. No new UI smoke was launched because the preceding elevated smoke remains visible and cannot be terminated from this lower-integrity terminal.
+- Completed the final identified decoupling pass. `BackupManager.BackupFiles` is now a `ReadOnlyDictionary` view; mutation remains behind the internal cache-invalidating `SetBackupFile` boundary used by focused tests. Added `UnitStatsProjectionService`, so the default-stats UI no longer duplicates balance/custom/independent modifier composition, and centralized UI-to-`PatchProfile` construction for Apply and preview. Replaced `PatchFileComposer` target knowledge with eight `IPatchFileContributor` implementations (EXE, cl_script, epara, partgeo, scint, ress, objdef, team.dat) plus an injectable contributor regression. The first full test run compiled and passed 178/179; the new projection test used the nonexistent key `FigKelPri00_Druide`, so the expected priest range was 30000 while projection correctly returned unknown-unit baseline 0. Correcting it to `FigKelPri00_Priester` produced full xUnit 179/179. Final serial Release build completed with 0 warnings/errors and `git diff --check` is clean. UI smoke rendered the full main window and remained responsive; cleanup via `Stop-Process`/`taskkill` was denied because the manifest-elevated process outranks the current terminal, and injected Alt+F4/click input could not cross that elevation boundary. PID 35644 remains open and must be closed manually from its visible window. Final core sizes: `FeatureDetector` 44 lines, `PatchEngine` 138, `BackupManager` 184, and `PatchFileComposer` 28; remaining `ModifierForm.Data.cs` code is WinForms grid/file display orchestration rather than duplicated patch-domain policy.
+- Began the next behavior-preserving decoupling pass. Added typed `FeatureKey<T>` declarations as the single source of feature IDs/default values; `PatchProfile` named properties and the WinForms toggle map now consume those keys instead of repeating string literals. `FeatureRegistry` distinguishes ordinary toggles from specialized controls and validates the complete UI toggle map at startup, so a newly registered toggle cannot be silently omitted. Registry tests cover declared-key registration, typed defaults/round trips, and missing/unexpected UI mappings. Verification: focused `FeatureRegistryTests` 5/5 passed; Release build 0 warnings/errors; full xUnit 163/163 passed.
+- Extracted EXE-backed detection from the monolithic `FeatureDetector` into `Features/Exe/ExeFeatureDetector`. It owns EXE reading and detection for focus-loss, village range, spell altar, game speed, civilian batch production, and unit recruitment, while returning the RomanEndless EXE state to the coordinator for the existing EXE/team.dat mismatch policy. A synthetic-EXE regression covers every extracted field. Focused detector/integration/characterization verification: 10/10 passed. Final verification for the combined decoupling pass: serial Release build 0 warnings/errors, full xUnit 164/164 passed, hidden startup smoke remained alive after 3 seconds, and `git diff --check` clean. A preceding parallel build/test run produced one transient `MSB3026` DLL-lock retry; the serial rerun was clean.
+- Extracted all `cl_script.ini` detection (civilian delay, morale, spell damage/healing/resurrection/radius, and general skills) plus its private regex/scaling helpers into `Features/Ini/ClScriptFeatureDetector`; the coordinator now delegates to it, and the focused test adds `SpellRange3x` apply/detect coverage. The new detector uses only `BackupManager.HasFile/GetBackupBytes` for its baseline dependency. The machine SDK briefly disappeared during this work, then was restored as .NET SDK 10.0.302. Final verification after restoration: focused cl-script/integration/characterization tests 10/10, full xUnit 164/164, and serial Release build 0 warnings/errors.
+- Extracted `ress.ini` detection into `Features/Ini/RessFeatureDetector`, covering FreeProduction, FreeUpgrade, and NoSpellCost through the same CSV index ranges used previously. Its first focused run failed only in the new synthetic fixture: the `Ger_Kampf` row had 71 columns while `VolkresIndex.UnitUpgradeStart` begins at 264, so no upgrade fields existed to patch/detect. After correcting the fixture to cover the real index range, focused detector/integration/characterization tests passed 10/10. Final verification: full xUnit 165/165 and serial Release build 0 warnings/errors.
+- Extracted the complete `objdef.dau` detection body and its dedicated parsing/rule helpers into `Features/Objdef/ObjdefFeatureDetector`. This preserves the existing ordering for capacity/build/HP/projectile checks, Balance false-positive exclusions, LeaderGlory, speed/range, and priest casting distance. The detector now accesses its baseline through `BackupManager.HasFile/GetBackupBytes`. A direct composer-output regression covers every objdef-backed feature and Balance exclusion. Focused detector/integration/characterization verification: 10/10 passed.
+- Extracted team.dat detection into `Features/Map/TeamDatFeatureDetector`, which owns per-file enumeration, four-state reconstruction, unknown-state conservative fallback, and aggregate MaxPopulation/RomanPlayer evidence; the coordinator retains the cross-source RomanEndless EXE/team.dat reconciliation policy. The first focused command stopped at compile because removing the old helper also removed the still-needed `AgainstRomeModifier.Core.Patches` using for `ExeRomanEndlessPatchState` (`CS0103`); after restoring the using, focused team/RomanEndless/objdef/integration tests passed 19/19. Final combined verification: `FeatureDetector` is 79 lines, full xUnit 166/166 passed, and serial Release build completed with 0 warnings/errors.
+- Completed detector responsibility extraction: `Features/Bci/BciFeatureDetector` now owns Endless AI module and FoodHealing detection, while `Features/Install/InstallFeatureDetector` owns LanguagePack and DgVoodoo state detection. `FeatureDetector` is now a 44-line target-detector orchestrator plus the intentional RomanEndless EXE/team.dat reconciliation policy; formatting was normalized after the extraction. Focused BCI/install/integration/characterization verification: 14/14 passed. Final verification: full xUnit 166/166 and serial Release build 0 warnings/errors.
+- Completed the first Apply-side extraction: `Services/PatchFileComposer` now composes the ordinary EXE/INI/DAU/team.dat target bytes as a dry-run `PatchFilePlan`, while `PatchEngine` retains transactional writes, shared BCI flush, language-pack, dgVoodoo, and restore coordination. The first focused run passed 9/10; the new direct composer test alone failed because it enabled `ProjectileArcHeight` against `BackupZipGameFixture`, whose baseline intentionally lacks `partgeo.dau`, correctly triggering the existing safety exception. After restricting the test to files present in that fixture, composer/Apply/characterization tests passed 10/10 and confirmed composition does not mutate the fixture before transaction commit. Final verification: full xUnit 167/167 and serial Release build 0 warnings/errors.
+- Extracted category restore rules into `Services/PatchRestoreService`. It now owns Stats/Compat/Language restoration, EXE preservation rules, original-memory file restoration, team.dat population reset, BCI restore/flush coordination, and install-feature removal. `PatchEngine` keeps its public restore facade and Apply ordering and is reduced to 138 lines. Focused composer/Apply/restore/BCI-cache verification passed 11/11; final verification passed full xUnit 167/167 and serial Release build with 0 warnings/errors.
+- Completed the first `BackupManager` decomposition slice with `Services/UnitBaselineCatalog` and `UnitStatParser`. The catalog owns the cached `objdef.dau` unit-row projection, original/balanced/custom baseline selection, and cache invalidation; the parser owns pure weapon damage/reload/range parsing and custom-layer merging. `BackupManager` keeps compatibility facade methods and dropped from 864 to 651 lines. A direct regression confirms `Clear()` invalidates previously parsed unit rows/stats; focused unit/objdef/Apply tests passed 12/12. Final verification: full xUnit 168/168 and serial Release build 0 warnings/errors.
+- Extracted ZIP source selection/parsing into `Services/BackupSourceLoader` and required-resource rules into `BackupValidator`. Embedded ZIP still takes precedence over local `Backup.zip`, unsafe rooted/parent-segment entries remain rejected, and `partgeo.dau` intentionally remains outside the general required-resource list. `BackupManager` is reduced to 581 lines. Focused ZIP safety/validator/physical-backup/baseline/Apply tests passed 8/8. Final verification: full xUnit 170/170 and serial Release build 0 warnings/errors.
+- Extracted physical `.bak` creation/loading into `Services/GameDirectoryBackupLoader` and EXE/objdef/partgeo baseline-signature checks into `OriginalFileValidator`. The loader retains existing-file `.bak` precedence, first-run `.bak` creation, custom-map exclusion, clean `cl_epara.ini` injection, fail-closed dirty-file handling, and `showError` behavior. `BackupManager` is reduced from 581 to 391 lines.
+- A new invalid-payload validator regression initially passed 8/9: `IsObjdefOriginal([1,2,3,4])` returned true because the legacy algorithm treated a successfully decompressed payload with no verified leader row as original. This real fail-open gap is fixed: objdef now requires an explicitly parsed `FigRomAnf00_Anfuehrer` HP value of 400. EXE and partgeo already failed closed. Invalid-payload and known-good baseline regressions now cover both directions; focused physical-backup/validator/Apply tests passed 10/10. Final verification: full xUnit 172/172 and serial Release build 0 warnings/errors.
+- Extracted missing-entry recovery into `Services/BackupAutoHealer` and the canonical `cl_epara.ini` text/PFIL generation into `CleanEparaBaseline`. The healer preserves `.bak` preference, partgeo validation/backup, custom-map exclusion, and localized logging, and now reports mutations so `BackupManager` can invalidate its unit-row cache after public auto-heal retries. After removing obsolete imports, `BackupManager` is reduced from 391 to 175 lines. Cache-reset, epara PFIL round-trip, physical-backup, Apply, and ClScript focused tests passed 13/13. Final verification: full xUnit 174/174 and serial Release build 0 warnings/errors.
+- Added `Services/PatchOperationRunner` as the UI-neutral transaction executor for Apply and all four Restore flows. It creates/disposes `FileRollbackScope`, runs the mutation on a worker thread, commits on success, and restores/rethrows on failure; `ModifierForm` retains validation, button state, dialogs, toggle reset, and post-success reload. Direct commit/rollback regressions plus Apply/Restore characterization tests passed 11/11; `ModifierForm.Patches.cs` dropped from 391 to 328 lines. Full xUnit passed 176/176 and serial Release build completed with 0 warnings/errors. A hidden startup smoke remained alive after 3 seconds. Its first forced-cleanup attempts returned Access denied, but a final process check confirmed PID 45128 had exited and no smoke process remained.
+- Removed the duplicate unit-layer, weapon damage/reload, and maximum-range parsers from `ModifierForm.Data.cs`; both default/current UI tables now consume the same `Services/UnitStatParser` used by backup baselines and objdef detection. A direct mixed melee/ranged slot regression covers damage, reload, ordinary range, and priest sight/range. Focused parser/baseline/objdef/characterization tests passed 12/12; `ModifierForm.Data.cs` dropped to 943 lines. Final verification: full xUnit 177/177 and serial Release build 0 warnings/errors.
+
 ## Runtime-verified player batch controls (2026-07-15)
 
 - The user confirmed in-game that `CiviProduce20` makes one residential-tent click produce/queue 20 civilians and that `UnitRecruit20` makes one villager-to-unit conversion click select 20. Both are now formal Resource & Combat Upgrades toggles (`pnlSwitchesCard`) and participate in `BtnEnableAll`; they were removed from `pnlExperimentalCard`. Patch, detection, and restore behavior remain unchanged. Canonical runtime evidence is recorded in `docs/reverse-engineering/known-patches.md` and both READMEs. Verification: Release build 0 warnings/errors, full xUnit 161/161 passed, and `git diff --check` clean.

src/Core/Features/Bci/BciFeatureDetector.cs

@@ -0,0 +1,37 @@
+using AgainstRomeModifier.Core.Services;
+
+namespace AgainstRomeModifier.Core.Features.Bci;
+
+internal sealed class BciFeatureDetector
+{
+    private readonly ILogger _logger;
+
+    internal BciFeatureDetector(ILogger logger) => _logger = logger;
+
+    internal void Detect(string gamePath, PatchProfile profile)
+    {
+        try
+        {
+            var orchestrator = new EndlessAiOrchestrator();
+            foreach (var module in orchestrator.UserModules)
+            {
+                PatchState state = orchestrator.DetectModule(gamePath, module);
+                profile.EndlessAiModules[module.Id] = state == PatchState.Ultimate;
+            }
+        }
+        catch (Exception ex)
+        {
+            _logger.Log(string.Format(Loc.Get("SvcLogDetectFailed"), "Endless AI", ex.Message));
+        }
+
+        try
+        {
+            FoodHealingFeature.TryDetect(gamePath, out bool foodHealing);
+            profile.FoodHealing10x = foodHealing;
+        }
+        catch (Exception ex)
+        {
+            _logger.Log(string.Format(Loc.Get("SvcLogDetectFailed"), "food healing", ex.Message));
+        }
+    }
+}

src/Core/Features/Exe/ExeFeatureDetector.cs

@@ -0,0 +1,56 @@
+using AgainstRomeModifier.Core.Patches;
+using AgainstRomeModifier.Core.Services;
+
+namespace AgainstRomeModifier.Core.Features.Exe;
+
+internal readonly record struct ExeFeatureDetection(ExeRomanEndlessPatchState RomanEndlessState);
+
+internal sealed class ExeFeatureDetector
+{
+    private readonly ILogger _logger;
+
+    internal ExeFeatureDetector(ILogger logger) => _logger = logger;
+
+    internal ExeFeatureDetection Detect(string gamePath, PatchProfile profile)
+    {
+        ExeRomanEndlessPatchState romanEndlessState = ExeRomanEndlessPatchState.Unknown;
+        string exePath = Path.Combine(gamePath, "Against_Rome.exe");
+        if (!File.Exists(exePath)) return new(romanEndlessState);
+
+        try
+        {
+            byte[] exeBytes = File.ReadAllBytes(exePath);
+            ExePatchState exeState = ExePatchModel.GetExePatchState(exeBytes);
+            if (exeState != ExePatchState.Unknown)
+                profile.FocusLoss = exeState == ExePatchState.FocusPatched;
+
+            ExeVillageSetterPatchState setterState = ExePatchModel.GetVillageSetterPatchState(exeBytes);
+            if (setterState != ExeVillageSetterPatchState.Unknown)
+            {
+                profile.VillageBuildRange = setterState is
+                    ExeVillageSetterPatchState.Legacy2x or
+                    ExeVillageSetterPatchState.Legacy2Point5x or
+                    ExeVillageSetterPatchState.Legacy3x or
+                    ExeVillageSetterPatchState.Legacy5x or
+                    ExeVillageSetterPatchState.EntireMap;
+            }
+
+            ExeSpellAltarPatchState altarState = ExePatchModel.GetSpellAltarPatchState(exeBytes);
+            if (altarState != ExeSpellAltarPatchState.Unknown)
+                profile.NoSpellAltar = altarState == ExeSpellAltarPatchState.Patched;
+
+            romanEndlessState = ExePatchModel.GetRomanEndlessPatchState(exeBytes);
+            profile.GameSpeed = ExePatchModel.GetGameSpeedMultiplier(exeBytes);
+            profile.CiviProduce20 =
+                ExePatchModel.GetCiviProduce20PatchState(exeBytes) == ExeCiviProduce20PatchState.Patched;
+            profile.UnitRecruit20 =
+                ExePatchModel.GetUnitRecruit20PatchState(exeBytes) == ExeUnitRecruit20PatchState.Patched;
+        }
+        catch (Exception ex)
+        {
+            _logger.Log(string.Format(Loc.Get("SvcLogDetectFailed"), "Against_Rome.exe", ex.Message));
+        }
+
+        return new(romanEndlessState);
+    }
+}