From 8add0667a5f7abaecf102b1007ac1b435dc6d56b Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 15 Aug 2026 19:04:04 +0200 Subject: [PATCH] =?UTF-8?q?fix(app,headless):=20Campaign=20CC=20slice=20CC?= =?UTF-8?q?4=20re-review=20round=20=E2=80=94=20R1=20arbiter=20+=20R2-R4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CC4 re-review returned NOT CLOSED: R1 (MEDIUM, blocking) is a new residual the F1 fix itself introduced, plus three LOW riders (R2, R3, R4). R1 — nulling UiRoot.FixedCanvasSize on chargen Close() stripped it from character-management, which stays active underneath and only sets the canvas on its own activation edge. Root cause (reviewer-named): two controllers writing one host-global with no owner. Fixed with the root-cause shape (reviewer's option (c)): UiRoot.DeclareFixedCanvas(owner, size)/RevokeFixedCanvas(owner), an owner-scoped arbiter — every declarer must agree on the canvas size (a mismatch throws instead of silently last-writer-wins), and the canvas nulls only once EVERY declarer has revoked. Both CharacterCreationUiController and CharacterManagementUi- Controller now declare/revoke instead of writing FixedCanvasSize directly; grepped for stragglers, none remain in production code (the raw setter stays public only for UiRootFixedCanvasTests' isolated scale-math coverage). New test (reviewer-specified): CharacterScreensFixedCanvasArbiterTests — two controllers sharing one UiRoot, proving the canvas stays set through chargen's Exit-confirm Close while char-management is still active, nulling only once char-management also deactivates, plus the original F1 defect's own covering case (both revoke together at world entry). R3 — HeadlessSessionHostTests.ContentLease_InstallsRealChargenOptions_ SelectHeritageIsAccepted proves F6's install actually opens the gate: a content lease carrying a real hand-built DatCharGen heritage (not ChargenOptions.Empty) is installed, and TrySelectHeritage for it succeeds. R2 — filed docs/ISSUES.md #402 for the pre-existing Streaming.LandblockBuildFactoryTests.Build_UsesTheSuppliedSharedReaderGate full-suite flake (unrelated to Campaign CC). R4 — fixed "unchached" -> "uncached" typo in InteractionRetainedUiComposition.cs. Runtime 1713/0, App 5127/13 skips (+2), Headless 166/0 (+1), full solution Release build green. Live-DAT probes 7/7 under ACDREAM_PROBE_LIVE_MOUNT=1. The known #402 flake did not fire across 3 consecutive full-suite runs this session. Co-Authored-By: Claude Fable 5 --- docs/ISSUES.md | 30 ++ .../retail-divergence-register.md | 2 +- .../2026-08-15-character-creation-campaign.md | 2 +- .../InteractionRetainedUiComposition.cs | 2 +- .../Layout/CharacterCreationUiController.cs | 43 +- .../Layout/CharacterManagementUiController.cs | 16 +- src/AcDream.App/UI/UiRoot.cs | 86 ++++ ...CharacterScreensFixedCanvasArbiterTests.cs | 430 ++++++++++++++++++ .../HeadlessSessionHostTests.cs | 198 ++++++++ 9 files changed, 780 insertions(+), 29 deletions(-) create mode 100644 tests/AcDream.App.Tests/UI/Layout/CharacterScreensFixedCanvasArbiterTests.cs diff --git a/docs/ISSUES.md b/docs/ISSUES.md index f209a429..c9131e81 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -24,6 +24,36 @@ What does NOT go here: - Every session: scan OPEN issues at start; promote/close anything we touched during the session before ending. - Promoting to a Phase: mark as `DONE (promoted to Phase X)` + commit SHA where the Phase entry landed. +## #402 — Flaky test: Streaming.LandblockBuildFactoryTests.Build_UsesTheSuppliedSharedReaderGate + +**Status:** OPEN (flake, not a regression) +**Severity:** LOW (test-infra noise; no known production defect) +**Filed:** 2026-08-15 (Campaign CC slice CC4 review fix round, R2 — noticed +while running the full App.Tests suite repeatedly for the F1/R1 +FixedCanvasSize arbiter gate) +**Component:** `tests/AcDream.App.Tests/Streaming/LandblockBuildFactoryTests.cs` + +`Build_UsesTheSuppliedSharedReaderGate` fails intermittently in full-suite +runs (observed roughly 2 of 5 runs) but passes reliably when run in +isolation (`--filter FullyQualifiedName~Build_UsesTheSuppliedSharedReaderGate`). +The test was last touched at `82f8d4f8` (2026-07-25, Slice I7's parsed- +collision-graph removal) — unrelated to any Campaign CC/CC4 chargen work, +which never touches streaming/collision code. Symptom pattern (passes +isolated, flakes under full-suite parallelism) points at shared mutable +state or a timing assumption racing another test class rather than the +factory logic itself; not yet root-caused. + +**Fix direction:** re-run the full suite a few times to reproduce and +capture the failure's actual assertion/exception (not just "sometimes +red"), then check `LandblockBuildFactoryTests`'s fixture for anything +shared across test classes (static state, a shared reader/gate instance, +file-system paths) that a parallel xUnit collection could race. + +**Acceptance:** the flake is reproduced with a captured failure detail, +root-caused, and fixed (or the test is isolated into its own collection if +the root cause is unavoidable cross-test parallelism); full-suite runs stop +intermittently failing on this test. + ## #401 — RetailUi should default ON (opt-out), not per-path forced **Status:** OPEN (product-default decision) diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 137b8230..a5d5f27f 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -189,7 +189,7 @@ readiness/requeue adaptation. See | AD-92 | **Filed 2026-08-13 at the #376/#388 review fix round (blast M6 / mechanism M4).** Two switcher adaptations with no retail counterpart: (1) the fullscreen refresh rate is the monitor's HIGHEST for the picked WxH — retail passed the device mode's own refresh as-is (`Device::ForceDisplayResolution`); (2) an invalid/unsupported fullscreen request is a logged refusal that leaves the window unchanged — retail attempted the switch and surfaced the device error. The persisted-flag divergence a refusal leaves behind is ISSUES #392. | `src/AcDream.App/Settings/DisplayModeSwitching.cs` (`TryFindRefreshRate`, the refusal paths); `src/AcDream.App/Settings/RuntimeSettingsTargets.cs` (`Apply`'s refused-mode logging) | Highest-refresh is strictly better on modern variable-refresh panels (retail predates them); refuse-and-log is #388's own no-crash requirement. | A capture comparing retail's exact chosen refresh for a mode will differ; a server/tooling flow expecting an error dialog on an invalid mode sees a console line instead. | `Device::ForceDisplayResolution @gmClient::Init 0x004047af`; docs/research/2026-08-13-376-388-{mechanism,blast}-review.md | | AD-94 | **Filed 2026-08-14 at the secure-trade feature.** Retail's `Event_AcceptTrade` payload (`Trade::Pack @0x005B9FF0`) appends two `PackableList` staged-item lists after the six fixed fields; acdream sends both as ZERO-COUNT lists. ACE parses and then discards the ENTIRE payload (`HandleActionAcceptTrade()` takes zero arguments — server trade state is fully self-derived; lane B §quirks), so the difference is unobservable against ACE; a byte-capture comparison against a real retail client would differ from offset 40. | `src/AcDream.Core.Net/Messages/TradeRequests.cs` (`BuildAcceptTrade`) | The `ContentProfile` pack layout was not byte-verified (ACE never reads it — no reader to check against), and guessing a wire struct violates the workflow; zero-count lists are well-formed `PackableList`s. | A future server that actually validates the accept echo would see empty item lists and could refuse or desync the accept. | `Trade::Pack @0x005B9FF0`; `GameActionAcceptTrade.cs:11-16`; `docs/research/2026-08-14-trade-laneB-wire.md` Table 1 | | AD-96 | **Filed 2026-08-14 at the OP8 re-gate fix round (key-name display).** Retail's `GetNameFromKey_Internal @0x00687800` falls back from the DAT string tables (key enum 4 → `0x2300000A`, meta enum 5 → `0x2300000B`) to the OS keyboard layout's own key name via DirectInput `IDirectInputDevice8::GetObjectInfo` (`tszName` — "SKIFT" on a Swedish layout). acdream reads the SAME layout-resident name data through Win32 `GetKeyNameTextW` instead (no DirectInput device exists in-process); on non-Windows hosts there is no OS lookup at all and the DIK-suffix spelling shows (un-localized English, e.g. "LSHIFT"). Mouse chords keep the pre-existing enum spelling — retail names them through the DirectInput mouse device. | `src/AcDream.App/Platform/PlatformKeyNameProvider.cs`; `src/AcDream.App/UI/Layout/RetailKeyNames.cs` (`Describe`, the mouse-device early-out) | GetKeyNameText and DirectInput's key names both come from the active keyboard-layout tables; adding a DirectInput device solely for name strings would be a heavyweight, dead-end dependency. Linux graphical work is parked at Slice L1. | A key whose GetKeyNameTextW name differs from DirectInput's `tszName` on some layout shows a slightly different caption than retail did; Linux graphical shows English DIK-suffix names where retail-on-Wine would localize; a mouse-chord caption reads as the Silk enum, not retail's device string. | `CInputManager_WIN32::GetNameFromKey_Internal @0x00687800`; `GetNameFromKey @0x00687F40`; `ControlSpecification::GetDIKName @0x0068ACB0`; `DBCache::GetDIDFromEnumStatic` category-4 probe 2026-08-14 (`KeyboardConfigLiveMountProbeTests.ProbeKeyboardFontsAndKeyNameStrings`) | -| AD-98 | **Filed 2026-08-15 at Campaign LA gate round 2 (character-select background tiling).** The LA8 root (0x1000039A) authors LeftEdge=TopEdge=RightEdge=BottomEdge=0 ("no anchor") in the installed DAT, so retail's own `UIElement::UpdateForParentSizeChange` (0x00462640) never resizes this element — it stays a fixed 800x600 rect in retail's own widget tree. Retail's generic sprite blit, `Graphic::Draw` (0x00693b20) dispatching to `Graphic::PutImage` (0x00693a30) for an exact/undersized destination or a modulo-wrapped tile loop otherwise, has no third "stretch" mode (confirmed against `BlitMode`, acclient.h ~line 3135, and `MD_Data_Image::m_drawMode`/`DrawModeType` — both are COLOR-blend selectors, not tile-vs-stretch geometry modes). The only way retail's whole pre-world scene (background AND buttons AND listbox together) can still fill an arbitrary window resolution with no element ever resizing and a blitter that can only copy-or-tile is that these "flow" screens render into a fixed 800x600 target and the WHOLE FRAME is stretched once at presentation, outside the UI element/sprite system. **COMPLETED 2026-08-15 (same gate round, misalignment follow-up):** the first substitution (resize the mounted root + stretch only its own background) stretched the ART but left the authored child widgets at 800x600 pixel positions — misaligned against a background whose painting CARRIES visual anchors (the World/Characters captions are art). The substitution now reproduces retail's whole-frame behavior: the root KEEPS its authored 800x600 extent, and while the screen is active `UiRoot.FixedCanvasSize` scales EVERY emitted quad (widgets, glyphs, art, dialogs) uniformly at `TextRenderer.AppendQuad`, with the exact inverse applied to mouse coordinates at the `UiRoot` entry points so hit-testing lives in canvas space. Non-uniform window/canvas stretch, retail-authentic (no letterbox). `UiDatElement` keeps retail's pure copy-or-tile blit; the interim `StretchOwnBackgroundToFill` flag is deleted. | `src/AcDream.App/UI/UiRoot.cs` (`FixedCanvasSize`, `CanvasScale`, `MapWindowToCanvas`, `Draw`); `src/AcDream.App/Rendering/TextRenderer.cs` (`CanvasScale`, `AppendQuad`); `src/AcDream.App/UI/Layout/CharacterManagementUiController.cs` (activate/deactivate/dispose set+clear the canvas) | Reproducing retail's literal mechanism (an offscreen fixed-resolution UI render target scaled at presentation) would add RHI surface area for an identical pixel result; scaling at the one quad-emission chokepoint with an inverse input mapping is the same math applied one stage earlier, and the world-space HUD stays native because the scale is scoped to `UiRoot.Draw`. | Glyphs stretch with the frame (retail-authentic blur at large windows). **Gate round 2 filtering follow-up (2026-08-15):** the stretch now filters bilinearly — `TextureCache.GetOrCreateLinearUiTwin` gives every nearest-sampled UI texture (dat-font glyphs, composited icons) a linear-sampled twin that `TextRenderer.DrawSprite` swaps to while `CanvasScale != One` — matching retail's own bilinear-filtered presentation blit instead of aliasing the point-sampled art. Any future fixed-canvas screen (login/disconnected/datapatch) sets `UiRoot.FixedCanvasSize` while active — per-screen opt-in, not automatic. If a genuine present-time frame-stretch pass ever lands, this collapses into it. | `Graphic::Draw` 0x00693b20; `Graphic::PutImage` 0x00693a30; `UIElement::UpdateForParentSizeChange` 0x00462640; `BlitMode` acclient.h ~3135; `UIElementManager::CreateRootElement` 0x0045d020; `CharacterManagementLiveDatTests.RootAuthorsNoEdgeAnchors_RetailNeverResizesItSelf`; `UiRootFixedCanvasTests`; `UiDatElementTests.CanvasScale_StretchesQuadGeometry_LeavesUvsAuthored`; the NON-UNIFORM (no-letterbox) aspect behaviour has no decomp citation of its own (batch review F7) — it is inferred from the mechanism chain and CONFIRMED by the user's live gate pass 2026-08-15 (stretched widescreen look accepted as matching retail memory) | +| AD-98 | **Filed 2026-08-15 at Campaign LA gate round 2 (character-select background tiling).** The LA8 root (0x1000039A) authors LeftEdge=TopEdge=RightEdge=BottomEdge=0 ("no anchor") in the installed DAT, so retail's own `UIElement::UpdateForParentSizeChange` (0x00462640) never resizes this element — it stays a fixed 800x600 rect in retail's own widget tree. Retail's generic sprite blit, `Graphic::Draw` (0x00693b20) dispatching to `Graphic::PutImage` (0x00693a30) for an exact/undersized destination or a modulo-wrapped tile loop otherwise, has no third "stretch" mode (confirmed against `BlitMode`, acclient.h ~line 3135, and `MD_Data_Image::m_drawMode`/`DrawModeType` — both are COLOR-blend selectors, not tile-vs-stretch geometry modes). The only way retail's whole pre-world scene (background AND buttons AND listbox together) can still fill an arbitrary window resolution with no element ever resizing and a blitter that can only copy-or-tile is that these "flow" screens render into a fixed 800x600 target and the WHOLE FRAME is stretched once at presentation, outside the UI element/sprite system. **COMPLETED 2026-08-15 (same gate round, misalignment follow-up):** the first substitution (resize the mounted root + stretch only its own background) stretched the ART but left the authored child widgets at 800x600 pixel positions — misaligned against a background whose painting CARRIES visual anchors (the World/Characters captions are art). The substitution now reproduces retail's whole-frame behavior: the root KEEPS its authored 800x600 extent, and while the screen is active `UiRoot.FixedCanvasSize` scales EVERY emitted quad (widgets, glyphs, art, dialogs) uniformly at `TextRenderer.AppendQuad`, with the exact inverse applied to mouse coordinates at the `UiRoot` entry points so hit-testing lives in canvas space. Non-uniform window/canvas stretch, retail-authentic (no letterbox). `UiDatElement` keeps retail's pure copy-or-tile blit; the interim `StretchOwnBackgroundToFill` flag is deleted. **Campaign CC CC4 review-fix round R1 (2026-08-15): `FixedCanvasSize` now has a single arbiter.** Character-creation can be simultaneously active on top of character-management (both author the same 800x600 canvas), so a raw property write from either controller was a last-writer-wins race with no owner — chargen's own Close() nulled the canvas out from under a still-active character-management screen underneath it. `UiRoot.DeclareFixedCanvas(object owner, Vector2 size)`/`RevokeFixedCanvas(object owner)` now own every production write: each screen declares on its activation edge and revokes on close/deactivate/dispose; the effective size is the current declaration set's value (asserted equal across every concurrent declarer — a future mismatched screen throws instead of silently winning), and it nulls only once EVERY declarer has revoked. The raw `FixedCanvasSize` setter stays public only for `UiRootFixedCanvasTests`' isolated scale-math coverage. | `src/AcDream.App/UI/UiRoot.cs` (`FixedCanvasSize`, `DeclareFixedCanvas`, `RevokeFixedCanvas`, `CanvasScale`, `MapWindowToCanvas`, `Draw`); `src/AcDream.App/Rendering/TextRenderer.cs` (`CanvasScale`, `AppendQuad`); `src/AcDream.App/UI/Layout/CharacterManagementUiController.cs` and `src/AcDream.App/UI/Layout/CharacterCreationUiController.cs` (both declare/revoke through the arbiter on activate/close/deactivate/dispose) | Reproducing retail's literal mechanism (an offscreen fixed-resolution UI render target scaled at presentation) would add RHI surface area for an identical pixel result; scaling at the one quad-emission chokepoint with an inverse input mapping is the same math applied one stage earlier, and the world-space HUD stays native because the scale is scoped to `UiRoot.Draw`. | Glyphs stretch with the frame (retail-authentic blur at large windows). **Gate round 2 filtering follow-up (2026-08-15):** the stretch now filters bilinearly — `TextureCache.GetOrCreateLinearUiTwin` gives every nearest-sampled UI texture (dat-font glyphs, composited icons) a linear-sampled twin that `TextRenderer.DrawSprite` swaps to while `CanvasScale != One` — matching retail's own bilinear-filtered presentation blit instead of aliasing the point-sampled art. Any future fixed-canvas screen (login/disconnected/datapatch) DECLARES via `UiRoot.DeclareFixedCanvas` while active and REVOKES on close — per-screen opt-in through the arbiter, not automatic and not a raw write. If a genuine present-time frame-stretch pass ever lands, this collapses into it. | `Graphic::Draw` 0x00693b20; `Graphic::PutImage` 0x00693a30; `UIElement::UpdateForParentSizeChange` 0x00462640; `BlitMode` acclient.h ~3135; `UIElementManager::CreateRootElement` 0x0045d020; `CharacterManagementLiveDatTests.RootAuthorsNoEdgeAnchors_RetailNeverResizesItSelf`; `UiRootFixedCanvasTests`; `CharacterScreensFixedCanvasArbiterTests` (the two-controller arbiter gate); `UiDatElementTests.CanvasScale_StretchesQuadGeometry_LeavesUvsAuthored`; the NON-UNIFORM (no-letterbox) aspect behaviour has no decomp citation of its own (batch review F7) — it is inferred from the mechanism chain and CONFIRMED by the user's live gate pass 2026-08-15 (stretched widescreen look accepted as matching retail memory) | | AD-97 | **Filed 2026-08-14 at Campaign LA slice LA7a (character-restore request tail).** Retail's `CharacterRestore` request (`0xF7D9`) is ≥16 bytes: `CPlayerSystem::RestoreCharacter @0x0055d760` is, in the PDB-paired binary, `push 0x008173B4; push 0x008173B4; push guid; call Proto_UI::SendAdminRestoreCharacter @0x00546cf0`, and the callee packs BOTH constant `PStringBase*` arguments (`PStringBase::Pack @0x004fc6f0` emits ≥4 bytes even empty). Binary Ninja renders the two pushes as an uninitialized `edx` local plus `this` — a rendering artifact around constant `0x008173B4` (all 3 of its other pseudo-C appearances sit in provably-broken decompiles), but the arguments are real. acdream sends the 8-byte guid-only form. What the two constant strings contain is unresolved (a live cdb `db poi(0x008173b4)` would settle it). | `src/AcDream.Core.Net/Messages/CharacterRestore.cs` (`BuildRequestBody`) | ACE reads only `ReadUInt32()` and ignores any tail (`CharacterHandler.cs:331-385`), and holtburger ships guid-only from a real client command path against ACE successfully — the tail is unread by every server we can test against, and packing two strings whose CONTENT we cannot verify would be a guess. | A byte-capture comparison against a real retail client differs from offset 8; a future server that validates the full retail shape would reject our 8-byte request. | `CPlayerSystem::RestoreCharacter @0x0055d760` (binary bytes, not the BN rendering); `Proto_UI::SendAdminRestoreCharacter @0x00546cf0`; `PStringBase::Pack @0x004fc6f0`; ACE `CharacterHandler.cs:331-385`; holtburger `character_selection.rs:79-82`; LA7a Opus review F1 (2026-08-14) | | AD-93 | **Filed 2026-08-13 at social gate round 2, item 5 (the refused-drop notice port).** Two narrow gaps in the `ServerSaysAttemptFailed @0x0058EAE0` port: (1) **latched-guid preference** — retail's 0x00A0 dispatcher (`@0x0055B342`) PREFERS `prevRequestObjectID` over the wire guid when picking the item to name; acdream's `InventoryTransactionState.OnMoveFailed` instead REQUIRES the wire guid to match the latch (unobservable against ACE, which always sends the request's own guid on 0x00A0, and it protects a stale latch from mislabeling an unrelated failure — acdream has no retail-style latch timeout). (2) **unlatched request kinds** — retail latches `IR_MOVE`/`IR_WIELD` too; acdream's kind enum has no Move/Wield rows because wields ride `AutoWieldController` outside the single-request gate, so a refused wield/3D-move shows only the generic `HandleFailureEvent` leg, never "The X can't be wielded/moved". | `src/AcDream.Core/Items/InventoryTransactionState.cs` (`OnMoveFailed`); `src/AcDream.Core/Chat/InventoryFailureMessages.cs` (`Compose`'s absent Move/Wield rows); `src/AcDream.App/UI/ItemInteractionController.cs` (`OnInventoryRequestFailed`) | The match requirement is the compensating guard for the missing latch timeout; adding Wield/Move kinds means routing those sends through the single-request gate they deliberately bypass today — a behavior change beyond this gate item. | Only observable against a server that sends 0x00A0 with a guid that differs from the request's item (ACE never does), or on a refused wield/move, which shows no "can't be wielded/moved" verb line where retail would show one. | `ACCWeenieObject::ServerSaysAttemptFailed @0x0058EAE0`; the 0x00A0 dispatcher `@0x0055B342`; `ACCWeenieObject::RecordRequest @0x0058C220`; `docs/research/2026-08-13-confirm-and-weenie-error-display.md` §2 | | AD-100 | **Filed 2026-08-15 at the Campaign CC CC2 review, finding F2 (unrequested `0xF643` handling).** When a `0xF643` (`CharGenVerificationResponse`) arrives with NO outstanding create/restore request, acdream DROPS the message with a once-per-session stderr log. Retail has no such gate: `Handle_CharGenVerificationResponse @0x0055E8B0` processes whatever arrives, discriminating create-vs-restore by its OWN persistent verification state (case 1 branches on `GetVerificationState() == PENDING` → new `CharacterIdentity` + `AddIdentity`, else unpacks into the existing identity at `slot`) — an unsolicited reply would be applied against whatever that state happens to be. acdream's transport-level latch (`PendingCharGenVerificationRequest`) is the equivalent discriminator, but when it is `None` there is no state to apply the reply against, so the honest move is drop-and-log rather than guessing a family. | `src/AcDream.Core.Net/WorldSession.cs` (the `CharGenVerificationResponse.ResponseOpcode` arm in `ProcessDatagram`; `_loggedUnexpectedCharGenVerificationResponse`) | Processing an unsolicited reply requires retail's persistent chargen verification state, which lives in CC3's Runtime owner, not the transport. Until then a reply with no outstanding request is either a server bug or a latch-lifecycle bug on our side — surfacing it in the log beats silently misrouting it to an arbitrary event. Pinned by `WorldSessionCharacterCreationTests.ResponseWithNoOutstandingRequest_IsDroppedAndNeverMisattributed`. | A server that sends a spontaneous/duplicate `0xF643` (ACE can double-send NameInUse — see the CC2 review's F3 note) has its second copy dropped here, where retail would re-process it. If CC3's verification gate ever needs retail's re-process semantics, this drop must move behind that owner's state. | `Handle_CharGenVerificationResponse @0x0055E8B0`; `CharGenState::GetVerificationState`; CC2 review F2 (2026-08-15) | diff --git a/docs/plans/2026-08-15-character-creation-campaign.md b/docs/plans/2026-08-15-character-creation-campaign.md index d5e955c3..a17cd6e0 100644 --- a/docs/plans/2026-08-15-character-creation-campaign.md +++ b/docs/plans/2026-08-15-character-creation-campaign.md @@ -251,7 +251,7 @@ the user gate. | CC1 | REVIEW-CLOSED 2026-08-15 | `04450041`, `cb4703e8` | CLOSED (fix round + narrow re-review; every citation independently re-derived) | Core model (no Chorizite leak) + Content projector; 31 math units + 6 installed-DAT gates (13 heritages). FINDING for CC3: each human heritage's "Adventurer" template IS retail's Custom entry point — attributes at the 10-floor (60/330), a real TemplateCG row, not a UI special case. **Review fix round (`cb4703e8`):** F1 doc corrected — Custom IS template index 0 (the Adventurer row), per `gmCGProfessionPage::UpdateProfession @ 0x004821b0` (case 0 → button 0x100003d9 / `ID_CharGen_CustomText`) and `CharGenState::SetTemplate @ 0x005C5A60` (commits via `CharGenState::ApplyTemplate @ 0x005C5080`, i.e. selecting Custom resets sliders to the floor spread, it does not bypass templates); F2 two-tier skill-cost fallback implemented (`ChargenOptions.GlobalSkillCostsBySkillId` from portal.dat 0x0E000004, `ChargenSkillCreditMath` checks heritage list then global list) + installed-DAT completeness assertion recording reality: the global SkillTable prices 38/54 advancement skill ids, every one of the 13 heritages ships EXACTLY one heritage-specific override (always also present in the global table), and 16 skill ids are genuinely uncostable in both tiers (retail's -1 case) — see `ChargenTableReaderInstalledDatTests.InstalledHeritages_SkillCostFallbackCoversTheKnownUncostableSkillSet`; F3 every `ChargenTableReader` collection is now frozen at projection (`ToFrozenDictionary`/`ToArray`, matching `MagicCatalog`'s pattern) including both `ChargenOptions.Empty` dictionaries; F4 a reflection guard test (`ChargenNoChoriziteLeakTests`) pins the no-Chorizite-leak contract by walking every public `AcDream.Core.CharGen` member; F5 `HasAnyAppearanceOptions`'s doc reworded to state precisely what it proves (an OR across eight lists, omitting the three color lists) + a new installed-DAT gate records per-list reality — found COMPLETE, every gender of every heritage has non-empty lists across all eight plus the three color lists, even the sparse Gear Knight/Olthoi variants; F6 `TryGetHeritage`/`TryGetStarterArea` annotated `[MaybeNullWhen(false)]` (matching the house `EmptyDatReaderWriter` pattern), all affected call sites (more than the originally estimated five) fixed across both test projects. Filed CC7 risk item 8: ACE's `PlayerFactory` heritage-override branch over-deducts skill credits when specializing a heritage-priced skill (references/ACE/Source/ACE.Server/Factories/PlayerFactory.cs:184-211) — a retail-legal build may be rejected by local ACE at the CC7 connected gate; this is an ACE bug, not an acdream defect. **Narrow re-review CLOSED:** the reviewer retro-graded F2 to HIGH (under the base commit 37 of 38 costable skills were charged zero) and confirmed the SkillBase.SpecializedCost->PrimaryCost mapping dodged the UpgradeCostFromTrainedToSpecialized trap. Residuals: R1 retail refunds +1 credit on a both-tier miss (port charges 0; unreachable via retail’s own skills listbox — NOTE FOR CC3 if any path ever exposes the 16 uncostable ids); R2 list downcast-mutability and R3 field-walking in the leak guard CLOSED at the merge-closeout commit (Array.AsReadOnly at every projection seam; GetFields walk added). Decomp fact for CC4: ApplyTemplate force-sets template_=0 for heritage 0xc/0xd — both Olthoi variants are hard-locked to Custom/template 0. | | CC2 | REVIEW-CLOSED, MERGED 2026-08-15 (`55fc51ed`) | `5eaad2c8`, `e77ebf10`, `95e95bb6` | PASS then CLOSED (fix round: F1 latch-scope narrowing + overwrite pin test, F2 register AD-100, F3 ACE double-NameInUse note, F4 creationFailed{code,reason,name}, F5 pointer, retail-discriminator citations) | Byte-exact 0xF656 (19-term checksum vs CG_Pack accumulator), shared 0xF643 type, correlation latch, status events + contract amendment. Core.Net 993 / Runtime 1667 / Launcher.Core 323, Windows+WSL | | CC3 | REVIEW-CLOSED 2026-08-15 | `9a84230c`, `397ccd62`, + the R1 closeout commit | CLOSED (dual-lens: retail fidelity PASS, architectural FAIL → F1-F16 fix round `397ccd62` → narrow re-review CLOSED, both lenses PASS. Re-review residual R1 — the cached wire count is stale by creates-since-last-CharacterList, so a SECOND create after a rejected enter got wire slot N instead of N+1 — fixed in the closeout commit: `LiveSessionController._createsSinceCharacterList` (reset on every fresh wire CharacterList apply + generation reset; applied only to the cached-wire branch — the display-roster fallback already counts prior appends), regression test `SecondCreate_AfterRejectedEnter_GetsTheNextWireSlot` drives create→Ok→rejected guid-enter→ReturnToSelection→second create and pins slots 0/1/2/3. R2: fix-round sha recorded here.) | `RuntimeCharacterCreationState` (new, `src/AcDream.Runtime/Session/`): full CharGenState mirror (heritage/gender/appearance/template/six attributes+locks/55-slot skill set/name/startArea/slot/verification state), mirroring `RuntimeCharacterSelectionState`'s exact pattern (snapshot/delta/event-stream/borrow-only view, generation-gated `Try*` internals). Ports `SetHeritageGroup`, `SetGender`, `SetTemplate`/`ApplyTemplate` (Custom = template 0, Olthoi force-lock), the six attribute setters + `GetAbsRemainingCredits` + `BalanceAttributes` (retail's literal str/end/coord/quick/focus/self round-robin order, cursor-based fairness), `SetSkillLevel` + `ResetSkillLevels`' three-way free-skill baseline (both two-tier cost lookups reuse CC1's `ChargenSkillCreditMath`/`ChargenSkillCost` verbatim — no duplicated math), `RandomizeStartArea`, and `DoFinish`'s complete gate sequence (empty name / unspent attribute credits [see F3 below] / already-Pending / client-side roster-vs-slotCount cap). `LiveSessionController` gained a sibling `IRuntimeCharacterCreationCommands` implementation (command family lands beside `IRuntimeCharacterSelectionCommands`, `IGameRuntimeCommands.CharacterCreation` added with the same default-throw shape as `CharacterSelection`), a `CharacterCreationState` property, `ILiveSessionOperations.CreateCharacter` (default method → `WorldSession.SendCharacterCreation`), and a `HandleCharacterCreationResponse` wire handler subscribed to `WorldSession.CharacterCreateResponseReceived` alongside the existing character-selection bindings. `ILiveSessionLifecycleHost` gained `ApplyCharacterCreated`/`ApplyCreationFailed` as DEFAULT interface methods (no-op) so `AcDream.App`'s existing host implementations keep compiling unchanged — wiring them to `SessionStatusWriter.CharacterCreated`/`CreationFailed` is left to CC4 (Runtime calls the hooks; the App-side forward is a future host-construction change; **F14: zero production call sites exist for these hooks until then — a headless bot cannot observe a create yet**). **Review fix round (this commit):** F1 (HIGH, blocking) the post-create log-straight-in no longer enters by roster INDEX — `WorldSession` gained a guid-based `EnterWorld(uint characterGuid, string accountName, TimeSpan?)` overload (refactored to share `EnterWorldCore` with the index-based overload) plus `ILiveSessionOperations.EnterWorldByGuid` (default method); `LiveSessionController` factored `EnterSelectedCore`/the new `EnterCreatedCharacterCore` through a shared `EnterHighlightedCore(sendEnterWorld)` — the cached wire `CharacterList` is stale for a just-created character by ACE design (ACE appends server-side and replies Ok with no CharacterList resend — `references/ACE/.../CharacterHandler.cs:170-172`), so an index-derived enter could throw (0 pre-existing characters) or enter the WRONG character (N pre-existing, display order ≠ wire order). F2 (HIGH, blocking) the post-create roster append no longer round-trips through `ApplyRoster` (which re-derives EVERY entry's `ActiveIndex` — a wire contract ACE indexes for delete, `CharacterHandler.cs:297` — from display/name-sort order); `RuntimeCharacterSelectionState` gained a real `AppendCreatedCharacter(characterId, name, wireIndex)` primitive that preserves every existing entry's `ActiveIndex` untouched and assigns the new entry's from the pre-create wire `CharacterList.Characters.Count` (0-based, read from the same cached source the index-enter path uses). F3 (MEDIUM-HIGH, blocking) the credit gate was NOT retail — `DoFinish(this, arg2)`'s real gate is `arg2 != 0 && remainingAtrbCredits > 0`: the ordinary click (`arg2=1`) warns-and-refuses, but the warning dialog's own confirm re-invokes `DoFinish(this, 0)`, which skips the check and sends with credits unspent (ACE accepts this). `TryBeginFinish`/`LiveSessionController.Finish`/`IRuntimeCharacterCreationCommands.Finish` gained a `confirmedUnspentCredits`/`confirmUnspentCredits` parameter (default `false` = retail's `arg2=1`) — the plan doc's own "retail FORCES full spend" line above (§Retail ground truth, Finish) was corrected in the same round. F4 (MEDIUM, blocking) a stale out-of-range template index surviving a heritage switch to a heritage with fewer templates now clears to `TemplateUnset` in `ApplyTemplateLocked`, mirroring `ConstrainAllByHeritage @ 0x005C65CC`'s `template_ >= count → template_ = 0xffffffff` clamp (previously it just returned, leaving the stale index to reach the wire). F5 (MEDIUM) AP-207's anchor was wrong (`SetAttribValue` never calls `FitTemplateToCharacter`) — corrected to the four real call sites, including a fourth the original filing also missed (`UpdateToDefaultAttributes @ 0x00482860`). F6 (MEDIUM) `ApplyCreationResponse`'s Pending/Undef branch no longer publishes from inside `lock(_gate)` — every branch now sets `kind` and a single `Publish` runs after the lock releases, matching every sibling method. F7 (MEDIUM) two new tests pin `BalanceAttributes`' persistent cursor: successive overspends absorb from different attributes, and the Self→Strength wrap. F8 (LOW) `ResetSkillLevels`' doc corrected — retail's real gate is BOTH costs `>= 0` (not "either tier"); the dictionary-presence equivalence is a CC1-established, installed-DAT-gated invariant, cited precisely. F9 (LOW) the `Slot` doc corrected — retail DOES assign it (`gmCharacterManagementUI::SelectCharacter @ 0x004EC160` → `SetSlot(GetSlot(...))`), just semantically stale (the last-selected PRE-EXISTING character's slot); conclusion (send 0) unchanged. F10 (LOW) AP-209's `classID` citation completed with the three heritage-dependent branch ids (ordinary/Olthoi/OlthoiAcid) plus admin variants. F11 the integration test fixture no longer stubs `EnterWorld` to a bare counter — it captures guid-based calls and the fixture now has two pre-existing characters whose wire order deliberately differs from alphabetical order, so the roster-preservation assertion actually exercises F2 instead of coinciding with it by accident. F12 filed register row AP-211 for the client-side `RosterFull` slot-cap refusal (acdream-side gate, no retail `DoFinish`-layer counterpart — same-commit rule). F13 `LiveSessionController.Finish`'s bare `catch {}` narrowed to `InvalidOperationException`/`SocketException` and `_scope` bound to a local after validation. F15 `RandomizeStartAreaLocked` now leaves `_startArea` unchanged on an empty list (matching retail's `if (var_9c > 0)` guard) instead of forcing `-1`. Filed register rows AP-207 (FitTemplateToCharacter's FPU-unrecoverable auto-detect skipped — ACE only reads `TemplateOption` for title text; anchor corrected this round), AP-208 (per-style color-count approximated by the shared gender-wide `ClothingColors` list — CC1's model has no per-style palette data), AP-209 (`classID` sent as a placeholder `0` — DAT DID lookup unavailable in Core, ACE ignores the field; branch table added this round), AP-210 (`ApplyTemplate`'s per-attribute guarded sequential set approximated as one atomic replace), AP-211 (this round — the `RosterFull` client-side slot-cap refusal). Tests: `tests/AcDream.Runtime.Tests/CharGen/RuntimeCharacterCreationStateTests.cs` (34 cases — every Finish gate including the F3 confirmed-credits path, the F4 stale-template clamp, the F7 cursor-advance/wrap pair, Ok/each-rejection-code response mapping, duplicate-NameInUse tolerance, Olthoi template lock, attribute-lock/balance interaction, uncostable-skill rejection, generation reset) + `.../Session/LiveSessionControllerCharacterCreationTests.cs` (5 cases — wire-send exactly 55 skill slots via a REAL `WorldSession` + `GameMessageCapture`, decoded byte-for-byte; the full Ok round trip via `WorldSession.ProcessDatagram` reflection asserting F1's guid-based enter + F2's ActiveIndex-preserving roster append + `ApplyCharacterCreated`; the NameInUse round trip asserting `ApplyCreationFailed` + no roster/enter side effect; the local-refusal-never-touches-the-wire gate; the F3 confirmed-unspent-credits send). Runtime 1706/0 (was 1701, was 1667), Core.Net unchanged at 994/0, full solution Release build green. OPEN for CC4+: `RuntimeCharacterCreationState`'s `ChargenOptions` currently defaults to `ChargenOptions.Empty` — threading the installed DAT's loaded options through `GameRuntime`/App startup is unresolved; the `Slot` field's real assignment source (which caller picks the target roster slot) has no decomp citation (ACE ignores it, non-load-bearing); `classID`'s real DAT-DID resolution (AP-209) if a non-ACE server ever needs it; the F14 zero-call-site status hooks. | -| CC4 | CODE-COMPLETE 2026-08-15 | original + fix-round, both "this commit" | Dual-lens review returned architectural FAIL (F1, F6) + retail-fidelity PASS-with-reservations (F2, F3, F4) + LOW findings F5/F7-F12 (F13 is a merge-mechanics note for the orchestrator, not an acdream defect). Fix round applied same-session (see the "Review fix round" paragraph at the end of this row); re-review status owed to the orchestrator. | Screen shell + form pages (App layer). **Mount:** `CharacterCreationUiController`/`CharacterCreationUiMountCoordinator` (`src/AcDream.App/UI/Layout/`) clone `CharacterManagementUiController`'s recipe — enum `0x10000039` via `RetailDataIdResolver.Resolve(dats, ..., 5u)`, root `0x100003CC` (decomp-verified: `gmCharGenMainUI::gmCharGenMainUI @ 0x004e7eb0`, NOT the plan doc's earlier `0x100003cc`-adjacent guesses — confirmed live against the installed DAT, `[CC4-DAT] enum=0x10000039 -> DID=0x21000038`), fixed-canvas AD-98 treatment shared with char-management. **CORRECTED at the review fix round (2026-08-15, F1) — the original claim above was FALSE**: `CharacterManagementUiController` does NOT do a per-tick set; it writes `UiRoot.FixedCanvasSize` ONCE on its own activation edge and NULLS it in both `Deactivate()` and `Dispose()`. This controller now matches that exact shape: `Open()` sets the canvas once, `Close()`/`Deactivate()`/`Dispose()` null it symmetrically. The un-nulled canvas was a real bug: `RuntimeCharacterCreationState` had no `CompleteEnter()` analogue to `RuntimeCharacterSelectionState`'s (added this round, wired at both `LiveSessionController` in-world edges), so the chargen view reported `IsActive=true` for an entire in-world session, and since `RetailUiRuntime.Tick` ticks char-management BEFORE chargen, chargen's un-nulled canvas would silently re-pin an 800x600 scale over the in-world UI forever once the screen had ever been opened (dormant at defaults, armed under `ACDREAM_OPEN_CHARGEN=1`). **Master shell:** progress bar `0x100003ce`, master page `0x100003d0` (state `0x10000025+page-1`), 6 page roots, 6 free-navigation tabs (`0x100003ef..f4`), nav buttons `0x100003c6..cb` — full decomp port of `gmCharGenMainUI::ListenToElementMessage @ 0x004e9450` (Back-at-Heritage→DoExit, Next capped at Summary, Finish Summary-only) and `SetProgressState @ 0x004e7a10` (the Olthoi Profession/Skills/Town tab-hide + forward/backward page redirect, keyed off the LIVE snapshot heritage id every call). Exit confirmation via `RetailDialogFactory.MakeConfirmation` + `ID_CharGen_ExitWarning` (table `0x23000002`, matching `DoExit @ 0x004e8650`); on confirm the screen just closes (visibility only — see AD-99's sibling precedent) rather than porting `gmEpilogueUI`. **Heritage page** (`CharacterCreationHeritagePage.cs`, decomp `InitializePage @ 0x00483a10` + the EXACT button-id→heritage-id map read off `ListenToElementMessage @ 0x00483860`, which is NOT numeric-order — e.g. `0x100005e8`→Tumerok(7)): all 13 buttons, composed description text (`ID_CharGen_Heritage_StartingSkills_Header/Body`, `ID_CharGen_Heritage_BonusSkills_Trained_Header` + per-heritage body — Shadowbound/Penumbraen share one string per the decomp's `case 5: case 0xa:`; Lugian/Olthoi/OlthoiAcid have no bonus-skills string in the retail table at all, confirmed by string-key absence, not guessed). Selecting a heritage ALSO auto-selects its lowest gender key (AD-101 — Appearance's real gender buttons are CC6b's). **Profession page** (`CharacterCreationProfessionPage.cs`, `InitializePage @ 0x00482d50` + `UpdateProfession @ 0x004821b0`'s template map, cited already on `ChargenTemplate`): 7 template buttons (Custom=index 0, the six presets NOT in id order), 6 attribute sliders with the exact e6/e7/e9/e8/ea/eb id↔attribute-id mapping (the documented 3/4 swap), avail/health/stamina/mana. Live-DAT probe found TWO widget-mapping surprises the decomp's `DynamicCast` calls don't predict: the slider's value display (`0x100002ef`) imports as `UiField` not `UiText` (retail's `NumberInputFilter`, `@0x00482e36`) — wired for direct numeric entry via `OnSubmit`, not just display; and all four avail/health/stamina/mana containers (and the Skills credits meter) author as `UIElement_Button` whose Type-12 value child is swallowed by `UiButton.ConsumesDatChildren` before ever becoming an addressable widget — substituted with the button's own `.Label` (AD-103). Health/Stamina/Mana formulas ported from `UpdateAttributeValues @ 0x00482450`: Health=Endurance/2 (int truncation — the decompiler elides the FPU divide at `_ftol2 @0x0048262b`, so the exact MSVC rounding mode is UNVERIFIED beyond well-established AC convention; flagged, not guessed-and-hidden), Stamina=Endurance, Mana=Self; Available=`RemainingAttributeCredits` directly (`UpdateCreditsMeter`-style, no formula). **Skills page** (`CharacterCreationSkillsPage.cs`, `InitializePage @ 0x00481dd0`): ONE flat listbox (AP-213, retail's four-bucket sorted `InsertEntrySorted`/`UpdateSkillEntry` model not ported) driven by CC3's `TrainSkill`/`SpecializeSkill`/`UntrainSkill` + the SAME two-tier `TryGetSkillCost` presence gate `RuntimeCharacterCreationState` uses (16 uncostable ids never listed, matching retail); credits meter via the AD-103 button-Label substitution; info panes `0x100003fb/fc` unbound (no info-pane content source this round). **Town page** (`CharacterCreationTownPage.cs`, `InitializePage @ 0x0047c6d0` + `SetTown @ 0x0047c360`'s literal index map): the four buttons map to LITERAL `startArea` indices (Sanamar→3, Holtburg→0, Yaraq→2, Shoushi→1 — not id order), composed "How To" + per-town description text. **Random** (`0x100003cb`, `DoRandom @ 0x004e7d70`): Heritage/Profession/Town approximated with a uniform pick over every valid option (AP-212 — no `RandomizeHeritageGroup`/`RandomizeTemplate` primitives exist); disabled outright on Skills (no `RandomizeSkills` primitive), Appearance (placeholder), Summary (CC5's warning dialog). **Options threading:** `RuntimeCharacterCreationState.InstallOptions(ChargenOptions)` (new, mirrors `RuntimeCharacterState.InstallSpellMetadata`→`Spellbook.InstallMetadata`'s "install immutable DAT metadata after construction, throw if already active" pattern) called from `ContentEffectsAudioCompositionPhase.Compose` (new `ChargenOptionsInstalled` composition point, right after `SpellMetadataInstalled`) via `IContentEffectsAudioCompositionFactory.LoadChargenOptions`/`InstallChargenOptions` — `ChargenTableReader.Load(dats)` threaded through the SAME DAT-open composition sequence spell metadata uses, always well before any session's `Begin()`. **CORRECTED at the review fix round (2026-08-15, F6)**: the original claim that headless was unaffected left a dead end — `HeadlessSessionHost` wired the `CharacterCreated`/`CreationFailed` status hooks (closing CC3's F14) but never installed `ChargenOptions`, so a content-bearing headless host could observe a create but never actually issue one (every chargen command silently refused against `ChargenOptions.Empty`). Fixed by installing options directly beside the existing `InstallSpellMetadata` call, off the same `HeadlessProcessContentLease.Dats`, whenever `contentLease` is non-null; a content-less headless host (a validated-legal configuration — see the R9 note near `_contentLease`'s other reads) still cannot issue chargen commands, matching its existing inability to resolve spell/collision data either. **Status hooks:** `LiveSessionLifecycleBindings` gained optional `CharacterCreated`/`CreationFailed` delegates (default `null` — every pre-CC4 construction site keeps compiling); `LiveSessionLifecycleHost` now overrides both `ILiveSessionLifecycleHost` methods to forward them; `LiveSessionHostBindings` gained matching optional fields threaded through `LiveSessionHost`'s constructor; both `LiveSessionRuntimeFactory.Create` (App/graphical) and `HeadlessSessionHost` wire them to `SessionStatusWriter.CharacterCreated`/`CreationFailed`, closing CC3's F14 (zero call sites). **Deferred command seam:** `IGameRuntimeView.CharacterCreation` (new default-throw member, mirrors `CharacterSelection`), `GameRuntime.CharacterCreation` (passthrough to `Session.CharacterCreation`), `CurrentGameRuntimeAdapter`'s new `CharacterCreationProjection` (IsActive-gated view+command wrapper, mirrors `CharacterSelectionProjection`), `DeferredGameRuntimeStateCommands`'s new `CharacterCreation` view getter + 9 generation-capturing wrapper methods, and `CharacterCreationRuntimeBindings` wired in `InteractionRetainedUiComposition.cs` (`CharacterCreation:` sibling of `CharacterSelection:`, `ResolveText` backed by a `DatStringResolver` cached once per composition (`characterCreationStrings`, review fix round F12 — a fresh resolver per call was allocating + re-locking on every Heritage/Town description lookup, several times per page switch) and locked under `d.DatLock` only around each `.Resolve` call, `OpenOnStart` from the new `RuntimeOptions.OpenCharacterCreationOnStart` / `ACDREAM_OPEN_CHARGEN=1` env flag — the interim open seam since Create stays ghosted). **Widget types added to `DatWidgetFactory`: NONE** — every id resolves through EXISTING factory mappings (Button=1, Text/Field=12, Scrollbar=11, ListBox=5); the two "new" findings (editable-Field slider value, button-consumed credits/vitals children) are AUTHORED-DATA-DRIVEN outcomes of the existing factory logic, not new widget classes. **Register rows filed (same commit):** AD-101 (Heritage-page auto-gender-select interim default), AD-102 (Viamontian/Sanamar ToD-account-ownership gate omitted — acdream has no account/DLC signal), AD-103 (avail/health/stamina/mana/credits-meter UiButton-Label substitution for retail's swallowed Text-child overlays), AP-212 (Random button's uniform-pick approximation), AP-213 (Skills page flat-listbox simplification), TS-82 (Appearance/Summary placeholder pages, reachable via free tab nav, content-inert pending CC5/CC6a/CC6b). **Tests:** `tests/AcDream.App.Tests/UI/Layout/CharacterCreationLiveDatTests.cs` (7 cases, `ACDREAM_PROBE_LIVE_MOUNT=1`-gated — sweeps every master-shell/page id against the installed DAT and pins the two widget-mapping surprises above) + `CharacterCreationUiControllerTests.cs` (16 cases — hand-built layout fixture, no DAT: page switching, Olthoi tab-hide+redirect, Back/Exit/Random gating, exit-confirm/cancel, per-page command dispatch including the slider/field/skill-row/town-button paths) + `tests/AcDream.Runtime.Tests/CharGen/RuntimeCharacterCreationStateTests.cs` (+4 `InstallOptions` cases) + `tests/AcDream.Runtime.Tests/Session/LiveSessionLifecycleHostTests.cs` (+2 status-hook forwarding cases). Runtime 1713/0 (was 1707), App 5117/13 skips (was 5101/6, +16 new +7 gated-skip), Headless 165/0 unaffected, full solution Release build green. **OPEN for CC5/CC6a/CC6b:** the real Appearance-page gender buttons must retire AD-101's auto-select; Summary's Finish gate, name input, and randomize-warning dialog (currently Finish/Random both hard-disabled); Skills page info-panes `0x100003fb/fc` have no content source wired yet; the four-bucket sorted skill list (AP-213) and retail's exact Random algorithms (AP-212) remain unported if a future gate demands byte-exact parity; the Health/Stamina/Mana rounding-mode residual (see above) would need a live cdb byte trace to fully pin. **Review fix round (this commit, 2026-08-15):** F1 (HIGH, blocking, architectural) — see the corrected FixedCanvasSize paragraph above; added `RuntimeCharacterCreationState.CompleteEnter()` (mirrors `RuntimeCharacterSelectionState`'s own, wired at both `LiveSessionController` in-world edges: `StartCore` and the shared `EnterHighlightedCore`) and made `CharacterCreationUiController.Open`/`Close`/`Deactivate`/`Dispose` set/null `UiRoot.FixedCanvasSize` symmetrically with `CharacterManagementUiController`'s real (not per-tick) shape; added FixedCanvasSize coverage to `CharacterCreationUiControllerTests`. F2 (MEDIUM-HIGH, blocking, fidelity) — the attribute-slider scalar mapping was NOT retail's: fixed the display scalar to `value/100f` (`UpdateAttributeValues @ 0x0048251d`) and the drag inverse to `Math.Max(10, (int)(scalar*100f))` — truncate, clamp low only, no rescale (`ListenToElementMessage @ 0x004829c0`'s scrollbar-drag case, independently re-derived against the decomp and confirmed byte-for-byte); added tests at scalar 0.5 and 0.0 (the previous single scalar=1f test coincidentally agreed with both the old wrong formula and the new correct one). F3 (MEDIUM, blocking, fidelity) — ported `ListenToElementMessage @ 0x004e9450`'s heritage-button tab-restore arm (independently re-derived from the decomp: SHOW ids `0x100003bf/c1/c2/c3/10000590/91/100005a9/bf/c4/e8`, HIDE ids `0x100005c7/c8`, with Lugian `0x100005f1` genuinely absent from both switch cases — a real retail quirk, reproduced faithfully) as `CharacterCreationUiController.ApplyHeritageTabRestore`, invoked synchronously from a new `CharacterCreationHeritagePage` ctor callback on every button click; added restore-after-Olthoi-hide and Lugian-no-restore tests. F4 (MEDIUM, fidelity, blocks the user gate) — `gmCGTownPage::SetTown @ 0x0047c360` also sets the TOWN PAGE's own retail state (a separate literal map from the master page's per-page-index cycling: Holtburg->0x10000034, Shoushi->0x10000037, Yaraq->0x10000036, Sanamar->0x10000035, re-asserted directly at the Sanamar-click site `@0x0047c518`) — independently re-derived from the decomp's tail-merged-branch pattern and ported to `CharacterCreationTownPage.Refresh` via the existing `IUiDatStateful.TrySetRetailState` seam; added a test. F5 (MEDIUM) — AD-103's "composited pixel result unchanged" claim was asserted, not measured; softened to state the equivalence is unverified rather than building a rect/justify comparison probe this round. F6 (MEDIUM, blocking, architectural) — **decision: install `ChargenOptions` in the headless content path (option (a) of the two offered), not the deferred/out-of-scope alternative** — `HeadlessSessionHost` now calls `RuntimeCharacterCreationState.InstallOptions(ChargenTableReader.Load(content.Dats))` beside the existing `InstallSpellMetadata` call whenever `contentLease` is non-null, closing the gap where CC3's F14 status hooks were wired but no content-bearing headless host could ever produce a create to observe. F7 (LOW-MEDIUM) — AP-213 already named the label format and the click/double-click substitution explicitly on inspection; no row edit needed. F8 (LOW) — AP-212 now names all SIX of `DoRandom`'s decompiled primitives (added the three the original row omitted: `RandomizeAppearance @ 0x005c4f10`, `RandomizeClothing @ 0x005c6770`, `RandomizeCharacter @ 0x005c6d80`, independently verified against the decomp alongside the three already-cited ones) and states the known landing site (Runtime, beside CC3's `CharGenState` ports). F9 (LOW) — AD-101's retirement condition corrected: must happen before CC5's Finish un-ghosts, not merely "at CC6b" (CC5 precedes CC6b in the slice order; shipping Finish first would let a create complete on an implicit gender default). F10 (LOW) — merged `ItemAppraisalTextFormatter.SkillName`'s two consecutive `` blocks into one. F11 (LOW) — TS-82's "see AP-211's sibling gate" cross-reference was wrong (AP-211 is the unrelated roster-slot-cap refusal); corrected to point at TS-82's own CC5 dependency. F12 (LOW) — cached the chargen `DatStringResolver` once per composition (`characterCreationStrings` in `InteractionRetainedUiComposition.CreateRetainedUi`) instead of constructing + DAT-locking fresh on every `ResolveText` call; the `LinesProvider` per-Refresh closure allocation already matched the house pattern used throughout `CharacterStatController.cs` and elsewhere, so it was left as-is. F13 is a merge-mechanics note (TS-82 collides with campaign-cc6a's TS-82/83) for the orchestrator at merge time — no acdream-side action taken. | +| CC4 | CODE-COMPLETE 2026-08-15 | original + fix-round, both "this commit" | Dual-lens review returned architectural FAIL (F1, F6) + retail-fidelity PASS-with-reservations (F2, F3, F4) + LOW findings F5/F7-F12 (F13 is a merge-mechanics note for the orchestrator, not an acdream defect). Fix round applied same-session (see the "Review fix round" paragraph at the end of this row); re-review status owed to the orchestrator. | Screen shell + form pages (App layer). **Mount:** `CharacterCreationUiController`/`CharacterCreationUiMountCoordinator` (`src/AcDream.App/UI/Layout/`) clone `CharacterManagementUiController`'s recipe — enum `0x10000039` via `RetailDataIdResolver.Resolve(dats, ..., 5u)`, root `0x100003CC` (decomp-verified: `gmCharGenMainUI::gmCharGenMainUI @ 0x004e7eb0`, NOT the plan doc's earlier `0x100003cc`-adjacent guesses — confirmed live against the installed DAT, `[CC4-DAT] enum=0x10000039 -> DID=0x21000038`), fixed-canvas AD-98 treatment shared with char-management. **CORRECTED at the review fix round (2026-08-15, F1) — the original claim above was FALSE**: `CharacterManagementUiController` does NOT do a per-tick set; it writes `UiRoot.FixedCanvasSize` ONCE on its own activation edge and NULLS it in both `Deactivate()` and `Dispose()`. This controller now matches that exact shape: `Open()` sets the canvas once, `Close()`/`Deactivate()`/`Dispose()` null it symmetrically. The un-nulled canvas was a real bug: `RuntimeCharacterCreationState` had no `CompleteEnter()` analogue to `RuntimeCharacterSelectionState`'s (added this round, wired at both `LiveSessionController` in-world edges), so the chargen view reported `IsActive=true` for an entire in-world session, and since `RetailUiRuntime.Tick` ticks char-management BEFORE chargen, chargen's un-nulled canvas would silently re-pin an 800x600 scale over the in-world UI forever once the screen had ever been opened (dormant at defaults, armed under `ACDREAM_OPEN_CHARGEN=1`). **Master shell:** progress bar `0x100003ce`, master page `0x100003d0` (state `0x10000025+page-1`), 6 page roots, 6 free-navigation tabs (`0x100003ef..f4`), nav buttons `0x100003c6..cb` — full decomp port of `gmCharGenMainUI::ListenToElementMessage @ 0x004e9450` (Back-at-Heritage→DoExit, Next capped at Summary, Finish Summary-only) and `SetProgressState @ 0x004e7a10` (the Olthoi Profession/Skills/Town tab-hide + forward/backward page redirect, keyed off the LIVE snapshot heritage id every call). Exit confirmation via `RetailDialogFactory.MakeConfirmation` + `ID_CharGen_ExitWarning` (table `0x23000002`, matching `DoExit @ 0x004e8650`); on confirm the screen just closes (visibility only — see AD-99's sibling precedent) rather than porting `gmEpilogueUI`. **Heritage page** (`CharacterCreationHeritagePage.cs`, decomp `InitializePage @ 0x00483a10` + the EXACT button-id→heritage-id map read off `ListenToElementMessage @ 0x00483860`, which is NOT numeric-order — e.g. `0x100005e8`→Tumerok(7)): all 13 buttons, composed description text (`ID_CharGen_Heritage_StartingSkills_Header/Body`, `ID_CharGen_Heritage_BonusSkills_Trained_Header` + per-heritage body — Shadowbound/Penumbraen share one string per the decomp's `case 5: case 0xa:`; Lugian/Olthoi/OlthoiAcid have no bonus-skills string in the retail table at all, confirmed by string-key absence, not guessed). Selecting a heritage ALSO auto-selects its lowest gender key (AD-101 — Appearance's real gender buttons are CC6b's). **Profession page** (`CharacterCreationProfessionPage.cs`, `InitializePage @ 0x00482d50` + `UpdateProfession @ 0x004821b0`'s template map, cited already on `ChargenTemplate`): 7 template buttons (Custom=index 0, the six presets NOT in id order), 6 attribute sliders with the exact e6/e7/e9/e8/ea/eb id↔attribute-id mapping (the documented 3/4 swap), avail/health/stamina/mana. Live-DAT probe found TWO widget-mapping surprises the decomp's `DynamicCast` calls don't predict: the slider's value display (`0x100002ef`) imports as `UiField` not `UiText` (retail's `NumberInputFilter`, `@0x00482e36`) — wired for direct numeric entry via `OnSubmit`, not just display; and all four avail/health/stamina/mana containers (and the Skills credits meter) author as `UIElement_Button` whose Type-12 value child is swallowed by `UiButton.ConsumesDatChildren` before ever becoming an addressable widget — substituted with the button's own `.Label` (AD-103). Health/Stamina/Mana formulas ported from `UpdateAttributeValues @ 0x00482450`: Health=Endurance/2 (int truncation — the decompiler elides the FPU divide at `_ftol2 @0x0048262b`, so the exact MSVC rounding mode is UNVERIFIED beyond well-established AC convention; flagged, not guessed-and-hidden), Stamina=Endurance, Mana=Self; Available=`RemainingAttributeCredits` directly (`UpdateCreditsMeter`-style, no formula). **Skills page** (`CharacterCreationSkillsPage.cs`, `InitializePage @ 0x00481dd0`): ONE flat listbox (AP-213, retail's four-bucket sorted `InsertEntrySorted`/`UpdateSkillEntry` model not ported) driven by CC3's `TrainSkill`/`SpecializeSkill`/`UntrainSkill` + the SAME two-tier `TryGetSkillCost` presence gate `RuntimeCharacterCreationState` uses (16 uncostable ids never listed, matching retail); credits meter via the AD-103 button-Label substitution; info panes `0x100003fb/fc` unbound (no info-pane content source this round). **Town page** (`CharacterCreationTownPage.cs`, `InitializePage @ 0x0047c6d0` + `SetTown @ 0x0047c360`'s literal index map): the four buttons map to LITERAL `startArea` indices (Sanamar→3, Holtburg→0, Yaraq→2, Shoushi→1 — not id order), composed "How To" + per-town description text. **Random** (`0x100003cb`, `DoRandom @ 0x004e7d70`): Heritage/Profession/Town approximated with a uniform pick over every valid option (AP-212 — no `RandomizeHeritageGroup`/`RandomizeTemplate` primitives exist); disabled outright on Skills (no `RandomizeSkills` primitive), Appearance (placeholder), Summary (CC5's warning dialog). **Options threading:** `RuntimeCharacterCreationState.InstallOptions(ChargenOptions)` (new, mirrors `RuntimeCharacterState.InstallSpellMetadata`→`Spellbook.InstallMetadata`'s "install immutable DAT metadata after construction, throw if already active" pattern) called from `ContentEffectsAudioCompositionPhase.Compose` (new `ChargenOptionsInstalled` composition point, right after `SpellMetadataInstalled`) via `IContentEffectsAudioCompositionFactory.LoadChargenOptions`/`InstallChargenOptions` — `ChargenTableReader.Load(dats)` threaded through the SAME DAT-open composition sequence spell metadata uses, always well before any session's `Begin()`. **CORRECTED at the review fix round (2026-08-15, F6)**: the original claim that headless was unaffected left a dead end — `HeadlessSessionHost` wired the `CharacterCreated`/`CreationFailed` status hooks (closing CC3's F14) but never installed `ChargenOptions`, so a content-bearing headless host could observe a create but never actually issue one (every chargen command silently refused against `ChargenOptions.Empty`). Fixed by installing options directly beside the existing `InstallSpellMetadata` call, off the same `HeadlessProcessContentLease.Dats`, whenever `contentLease` is non-null; a content-less headless host (a validated-legal configuration — see the R9 note near `_contentLease`'s other reads) still cannot issue chargen commands, matching its existing inability to resolve spell/collision data either. **Status hooks:** `LiveSessionLifecycleBindings` gained optional `CharacterCreated`/`CreationFailed` delegates (default `null` — every pre-CC4 construction site keeps compiling); `LiveSessionLifecycleHost` now overrides both `ILiveSessionLifecycleHost` methods to forward them; `LiveSessionHostBindings` gained matching optional fields threaded through `LiveSessionHost`'s constructor; both `LiveSessionRuntimeFactory.Create` (App/graphical) and `HeadlessSessionHost` wire them to `SessionStatusWriter.CharacterCreated`/`CreationFailed`, closing CC3's F14 (zero call sites). **Deferred command seam:** `IGameRuntimeView.CharacterCreation` (new default-throw member, mirrors `CharacterSelection`), `GameRuntime.CharacterCreation` (passthrough to `Session.CharacterCreation`), `CurrentGameRuntimeAdapter`'s new `CharacterCreationProjection` (IsActive-gated view+command wrapper, mirrors `CharacterSelectionProjection`), `DeferredGameRuntimeStateCommands`'s new `CharacterCreation` view getter + 9 generation-capturing wrapper methods, and `CharacterCreationRuntimeBindings` wired in `InteractionRetainedUiComposition.cs` (`CharacterCreation:` sibling of `CharacterSelection:`, `ResolveText` backed by a `DatStringResolver` cached once per composition (`characterCreationStrings`, review fix round F12 — a fresh resolver per call was allocating + re-locking on every Heritage/Town description lookup, several times per page switch) and locked under `d.DatLock` only around each `.Resolve` call, `OpenOnStart` from the new `RuntimeOptions.OpenCharacterCreationOnStart` / `ACDREAM_OPEN_CHARGEN=1` env flag — the interim open seam since Create stays ghosted). **Widget types added to `DatWidgetFactory`: NONE** — every id resolves through EXISTING factory mappings (Button=1, Text/Field=12, Scrollbar=11, ListBox=5); the two "new" findings (editable-Field slider value, button-consumed credits/vitals children) are AUTHORED-DATA-DRIVEN outcomes of the existing factory logic, not new widget classes. **Register rows filed (same commit):** AD-101 (Heritage-page auto-gender-select interim default), AD-102 (Viamontian/Sanamar ToD-account-ownership gate omitted — acdream has no account/DLC signal), AD-103 (avail/health/stamina/mana/credits-meter UiButton-Label substitution for retail's swallowed Text-child overlays), AP-212 (Random button's uniform-pick approximation), AP-213 (Skills page flat-listbox simplification), TS-82 (Appearance/Summary placeholder pages, reachable via free tab nav, content-inert pending CC5/CC6a/CC6b). **Tests:** `tests/AcDream.App.Tests/UI/Layout/CharacterCreationLiveDatTests.cs` (7 cases, `ACDREAM_PROBE_LIVE_MOUNT=1`-gated — sweeps every master-shell/page id against the installed DAT and pins the two widget-mapping surprises above) + `CharacterCreationUiControllerTests.cs` (16 cases — hand-built layout fixture, no DAT: page switching, Olthoi tab-hide+redirect, Back/Exit/Random gating, exit-confirm/cancel, per-page command dispatch including the slider/field/skill-row/town-button paths) + `tests/AcDream.Runtime.Tests/CharGen/RuntimeCharacterCreationStateTests.cs` (+4 `InstallOptions` cases) + `tests/AcDream.Runtime.Tests/Session/LiveSessionLifecycleHostTests.cs` (+2 status-hook forwarding cases). Runtime 1713/0 (was 1707), App 5117/13 skips (was 5101/6, +16 new +7 gated-skip), Headless 165/0 unaffected, full solution Release build green. **OPEN for CC5/CC6a/CC6b:** the real Appearance-page gender buttons must retire AD-101's auto-select; Summary's Finish gate, name input, and randomize-warning dialog (currently Finish/Random both hard-disabled); Skills page info-panes `0x100003fb/fc` have no content source wired yet; the four-bucket sorted skill list (AP-213) and retail's exact Random algorithms (AP-212) remain unported if a future gate demands byte-exact parity; the Health/Stamina/Mana rounding-mode residual (see above) would need a live cdb byte trace to fully pin. **Review fix round (this commit, 2026-08-15):** F1 (HIGH, blocking, architectural) — see the corrected FixedCanvasSize paragraph above; added `RuntimeCharacterCreationState.CompleteEnter()` (mirrors `RuntimeCharacterSelectionState`'s own, wired at both `LiveSessionController` in-world edges: `StartCore` and the shared `EnterHighlightedCore`) and made `CharacterCreationUiController.Open`/`Close`/`Deactivate`/`Dispose` set/null `UiRoot.FixedCanvasSize` symmetrically with `CharacterManagementUiController`'s real (not per-tick) shape; added FixedCanvasSize coverage to `CharacterCreationUiControllerTests`. F2 (MEDIUM-HIGH, blocking, fidelity) — the attribute-slider scalar mapping was NOT retail's: fixed the display scalar to `value/100f` (`UpdateAttributeValues @ 0x0048251d`) and the drag inverse to `Math.Max(10, (int)(scalar*100f))` — truncate, clamp low only, no rescale (`ListenToElementMessage @ 0x004829c0`'s scrollbar-drag case, independently re-derived against the decomp and confirmed byte-for-byte); added tests at scalar 0.5 and 0.0 (the previous single scalar=1f test coincidentally agreed with both the old wrong formula and the new correct one). F3 (MEDIUM, blocking, fidelity) — ported `ListenToElementMessage @ 0x004e9450`'s heritage-button tab-restore arm (independently re-derived from the decomp: SHOW ids `0x100003bf/c1/c2/c3/10000590/91/100005a9/bf/c4/e8`, HIDE ids `0x100005c7/c8`, with Lugian `0x100005f1` genuinely absent from both switch cases — a real retail quirk, reproduced faithfully) as `CharacterCreationUiController.ApplyHeritageTabRestore`, invoked synchronously from a new `CharacterCreationHeritagePage` ctor callback on every button click; added restore-after-Olthoi-hide and Lugian-no-restore tests. F4 (MEDIUM, fidelity, blocks the user gate) — `gmCGTownPage::SetTown @ 0x0047c360` also sets the TOWN PAGE's own retail state (a separate literal map from the master page's per-page-index cycling: Holtburg->0x10000034, Shoushi->0x10000037, Yaraq->0x10000036, Sanamar->0x10000035, re-asserted directly at the Sanamar-click site `@0x0047c518`) — independently re-derived from the decomp's tail-merged-branch pattern and ported to `CharacterCreationTownPage.Refresh` via the existing `IUiDatStateful.TrySetRetailState` seam; added a test. F5 (MEDIUM) — AD-103's "composited pixel result unchanged" claim was asserted, not measured; softened to state the equivalence is unverified rather than building a rect/justify comparison probe this round. F6 (MEDIUM, blocking, architectural) — **decision: install `ChargenOptions` in the headless content path (option (a) of the two offered), not the deferred/out-of-scope alternative** — `HeadlessSessionHost` now calls `RuntimeCharacterCreationState.InstallOptions(ChargenTableReader.Load(content.Dats))` beside the existing `InstallSpellMetadata` call whenever `contentLease` is non-null, closing the gap where CC3's F14 status hooks were wired but no content-bearing headless host could ever produce a create to observe. F7 (LOW-MEDIUM) — AP-213 already named the label format and the click/double-click substitution explicitly on inspection; no row edit needed. F8 (LOW) — AP-212 now names all SIX of `DoRandom`'s decompiled primitives (added the three the original row omitted: `RandomizeAppearance @ 0x005c4f10`, `RandomizeClothing @ 0x005c6770`, `RandomizeCharacter @ 0x005c6d80`, independently verified against the decomp alongside the three already-cited ones) and states the known landing site (Runtime, beside CC3's `CharGenState` ports). F9 (LOW) — AD-101's retirement condition corrected: must happen before CC5's Finish un-ghosts, not merely "at CC6b" (CC5 precedes CC6b in the slice order; shipping Finish first would let a create complete on an implicit gender default). F10 (LOW) — merged `ItemAppraisalTextFormatter.SkillName`'s two consecutive `` blocks into one. F11 (LOW) — TS-82's "see AP-211's sibling gate" cross-reference was wrong (AP-211 is the unrelated roster-slot-cap refusal); corrected to point at TS-82's own CC5 dependency. F12 (LOW) — cached the chargen `DatStringResolver` once per composition (`characterCreationStrings` in `InteractionRetainedUiComposition.CreateRetainedUi`) instead of constructing + DAT-locking fresh on every `ResolveText` call; the `LinesProvider` per-Refresh closure allocation already matched the house pattern used throughout `CharacterStatController.cs` and elsewhere, so it was left as-is. F13 is a merge-mechanics note (TS-82 collides with campaign-cc6a's TS-82/83) for the orchestrator at merge time — no acdream-side action taken. **CC4 re-review round (`ec854db0`'s own fix round, 2026-08-15) — R1 (MEDIUM, blocking, architectural, NEW residual introduced by the F1 fix above):** the F1 fix's raw `_host.FixedCanvasSize = null` in `Close()` was STILL a bug — character-creation can be simultaneously active on top of character-management (which stays active underneath, ticking its own roster), and nulling the shared host-global from either screen without regard for the OTHER screen's own active declaration strips it out from under whichever screen is still open (the exact AD-98 gate-round-2 misalignment defect resurfacing one layer up: char-select renders unstretched with dialogs centered against the raw window). Root cause per the reviewer (agreed): TWO controllers writing ONE host-global with no owner. **Fix — the root-cause shape, no workaround:** `UiRoot` gained a single arbiter, `DeclareFixedCanvas(object owner, Vector2 size)`/`RevokeFixedCanvas(object owner)` (see AD-98's own register row for the mechanism detail); both `CharacterCreationUiController` and `CharacterManagementUiController` now declare on their activation edge and revoke on close/deactivate/dispose instead of writing `FixedCanvasSize` directly — grepped for stragglers, none remain in production code; the raw property setter stays public only for `UiRootFixedCanvasTests`' isolated scale-math coverage. **Test (reviewer-specified):** `tests/AcDream.App.Tests/UI/Layout/CharacterScreensFixedCanvasArbiterTests.cs` — two controllers sharing ONE `UiRoot`, asserting the canvas across the full sequence (char-mgmt active → chargen Open → chargen Exit-confirm Close, canvas STAYS SET because char-mgmt is still active → char-mgmt deactivate, NOW it nulls) plus the original F1 defect's own covering case (both screens revoke together at world entry). **R3 (LOW):** `tests/AcDream.Headless.Tests/HeadlessSessionHostTests.cs`'s new `ContentLease_InstallsRealChargenOptions_SelectHeritageIsAccepted` proves F6's install actually opens the gate — a `HeadlessSessionHost` built with a content lease carrying a REAL hand-built `DatCharGen` heritage (not `ChargenOptions.Empty`) has that heritage present in `CharacterCreationState.Options`, and `TrySelectHeritage` for it succeeds once `Begin` is called (both called directly via this project's existing `InternalsVisibleTo` on `AcDream.Runtime`, isolating the F6 wiring from the unrelated real-network handshake needed to reach the same session state through the normal command gate). **R2 (LOW):** filed `docs/ISSUES.md` #402 for the pre-existing `Streaming.LandblockBuildFactoryTests.Build_UsesTheSuppliedSharedReaderGate` full-suite flake (passes isolated, fails ~2/5 full-suite runs, last touched `82f8d4f8` 2026-07-25 — unrelated to Campaign CC) so it stops being re-discovered. **R4 (LOW):** fixed the "unchached" → "uncached" typo in `InteractionRetainedUiComposition.cs`'s F12 comment. Runtime 1713/0 (unchanged), App 5127/13 skips (+2 new: 2 `CharacterScreensFixedCanvasArbiterTests` cases), Headless 166/0 (+1 new: R3's test), full solution Release build green. | | CC5 | — | | | | | CC6a | — | | | | | CC6b | — | | | | diff --git a/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs b/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs index fe0d13f2..84ae9175 100644 --- a/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs +++ b/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs @@ -650,7 +650,7 @@ internal sealed class RetailInteractionRetainedUiCompositionFactory // ResolveText several times per Refresh, and CharacterCreation- // UiController.ApplyProgressState forces a full refresh on // every page switch (`_lastRevision = long.MinValue`) — so an - // unchached resolver meant several fresh allocations + DatLock + // uncached resolver meant several fresh allocations + DatLock // acquisitions per click. DatStringResolver's own constructor // does no DAT I/O (only .Resolve reads), so building it here // outside the lock matches this file's existing pattern diff --git a/src/AcDream.App/UI/Layout/CharacterCreationUiController.cs b/src/AcDream.App/UI/Layout/CharacterCreationUiController.cs index 1c36cef0..ab811feb 100644 --- a/src/AcDream.App/UI/Layout/CharacterCreationUiController.cs +++ b/src/AcDream.App/UI/Layout/CharacterCreationUiController.cs @@ -201,16 +201,15 @@ internal sealed class CharacterCreationUiController : IDisposable Root.ClickThrough = false; Root.Visible = false; // AD-98: the same authored 800x600 fixed-canvas treatment as the - // character-management screen. CharacterManagementUiController sets - // UiRoot.FixedCanvasSize ONCE on its own activation edge - // (Tick's `if (!_active)` arm) and NULLS it in both Deactivate AND - // Dispose — it is NOT a per-tick set, and this controller must be - // symmetric with that exact shape (review fix round F1, 2026-08-15 - // — the earlier claim here that it was safe to leave the canvas - // pinned forever was FALSE and left an 800x600-scaled canvas - // covering the in-world UI whenever this screen had been opened). - // See Open/Close/Deactivate/Dispose below for the matching set/null - // pair. + // character-management screen. Both controllers DECLARE/REVOKE + // through UiRoot's owner-scoped arbiter (review fix round R1, + // 2026-08-15) rather than writing UiRoot.FixedCanvasSize directly — + // char-management can be simultaneously active underneath this + // screen, and a raw write from either controller is a last-writer- + // wins race with no owner (the F1 fix's own Close() null wiped + // char-management's still-active canvas out from under it). See + // Open/Close/Deactivate/Dispose below for the matching declare/ + // revoke pair. _authoredCanvas = new Vector2( Root.Width > 0f ? Root.Width : 800f, Root.Height > 0f ? Root.Height : 600f); @@ -380,17 +379,19 @@ internal sealed class CharacterCreationUiController : IDisposable /// Opens the screen at retail's authored default page /// (gmCharGenMainUI::gmCharGenMainUI's trailing - /// SetProgressState(this, ECG_HERTAGE)). Sets the fixed canvas - /// on this exact activation edge — matching - /// 's own one-shot set — - /// not per-tick; // - /// null it back out symmetrically. + /// SetProgressState(this, ECG_HERTAGE)). Declares the fixed + /// canvas on this exact activation edge through 's + /// arbiter — matching 's + /// own one-shot declare — not per-tick; / + /// / revoke it back out + /// symmetrically, and the canvas stays set for as long as ANY other + /// declarer (e.g. character-management underneath) remains active. internal void Open() { if (_disposed) return; _isOpen = true; - _host.FixedCanvasSize = _authoredCanvas; + _host.DeclareFixedCanvas(this, _authoredCanvas); ApplyProgressState(Page.Heritage); } @@ -400,7 +401,7 @@ internal sealed class CharacterCreationUiController : IDisposable return; _isOpen = false; Root.Visible = false; - _host.FixedCanvasSize = null; + _host.RevokeFixedCanvas(this); } public void Dispose() @@ -415,9 +416,11 @@ internal sealed class CharacterCreationUiController : IDisposable finally { // Matches CharacterManagementUiController.Dispose's own - // unconditional null — defends against disposing while _isOpen - // (Close() is not otherwise called on this path). - _host.FixedCanvasSize = null; + // unconditional revoke — defends against disposing while + // _isOpen (Close() is not otherwise called on this path). Idle + // if Close() already revoked (RevokeFixedCanvas is a no-op for + // an owner that already revoked). + _host.RevokeFixedCanvas(this); _back.OnClick = null; _next.OnClick = null; _finish.OnClick = null; diff --git a/src/AcDream.App/UI/Layout/CharacterManagementUiController.cs b/src/AcDream.App/UI/Layout/CharacterManagementUiController.cs index c9c5b4a3..dc6e9be2 100644 --- a/src/AcDream.App/UI/Layout/CharacterManagementUiController.cs +++ b/src/AcDream.App/UI/Layout/CharacterManagementUiController.cs @@ -114,9 +114,13 @@ internal sealed class CharacterManagementUiController : IDisposable // while this screen is active, the host stretches the ENTIRE canvas — // widgets, glyphs, and the painted background (which carries the // "World"/"Characters" captions as art) — as one unit via - // UiRoot.FixedCanvasSize. Resizing the root here instead is exactly the - // half-substitution that misaligned the widgets against the stretched - // art at the 2026-08-15 user gate. + // UiRoot.FixedCanvasSize, declared/revoked through the owner-scoped + // arbiter (review fix round R1, 2026-08-15) rather than written + // directly — character-creation can be simultaneously active on top + // of this screen, and a raw write from either controller is a last- + // writer-wins race with no owner. Resizing the root here instead of + // using the canvas is exactly the half-substitution that misaligned + // the widgets against the stretched art at the 2026-08-15 user gate. _authoredCanvas = new Vector2( Root.Width > 0f ? Root.Width : 800f, Root.Height > 0f ? Root.Height : 600f); @@ -298,7 +302,7 @@ internal sealed class CharacterManagementUiController : IDisposable { _active = true; Root.Visible = true; - _host.FixedCanvasSize = _authoredCanvas; + _host.DeclareFixedCanvas(this, _authoredCanvas); _host.BringToFront(Root); } @@ -368,7 +372,7 @@ internal sealed class CharacterManagementUiController : IDisposable } finally { - _host.FixedCanvasSize = null; + _host.RevokeFixedCanvas(this); _enter.OnClick = null; _delete.OnClick = null; _restore.OnClick = null; @@ -752,7 +756,7 @@ internal sealed class CharacterManagementUiController : IDisposable { _active = false; Root.Visible = false; - _host.FixedCanvasSize = null; + _host.RevokeFixedCanvas(this); } foreach (UiButton row in _rows) { diff --git a/src/AcDream.App/UI/UiRoot.cs b/src/AcDream.App/UI/UiRoot.cs index 087053ac..a9374c8a 100644 --- a/src/AcDream.App/UI/UiRoot.cs +++ b/src/AcDream.App/UI/UiRoot.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Numerics; namespace AcDream.App.UI; @@ -41,9 +42,94 @@ public sealed class UiRoot : UiElement /// renderer's quad chokepoint; the mouse entry points apply the inverse, /// so / and every hit test live /// in canvas space. Null (the in-world default) is native 1:1. + /// + /// + /// Campaign CC slice CC4 review-fix round R1 (2026-08-15): this raw + /// setter remains public for tests that exercise the scale/mouse- + /// mapping math in isolation (UiRootFixedCanvasTests), but + /// PRODUCTION code must go through / + /// instead of writing this property + /// directly. Two fixed-canvas screens can be active at once + /// (character-management underneath character-creation) and a raw + /// write from either one is a last-writer-wins race with no owner — + /// the F1 fix's own Close() null wiped the OTHER screen's still- + /// active canvas out from under it (see AD-98). + /// /// public Vector2? FixedCanvasSize { get; set; } + /// Screens currently declaring a fixed canvas, keyed by owner + /// (see ). + private readonly Dictionary _fixedCanvasDeclarations = new(); + + /// + /// Declares that wants the retained tree laid + /// out in while it is active. This is the single + /// arbiter for : multiple owners may declare + /// concurrently (character-management stays declared while character- + /// creation is also open on top of it), and the effective + /// is the shared declaration set's value. + /// Every current declarer must agree on the size — a mismatched second + /// declaration throws rather than silently overwriting the first + /// (Campaign CC CC4 review-fix round R1, 2026-08-15; see + /// docs/architecture/retail-divergence-register.md AD-98). Pair + /// every call with on the SAME owner at + /// deactivate/close/dispose. + /// + public void DeclareFixedCanvas(object owner, Vector2 size) + { + ArgumentNullException.ThrowIfNull(owner); + if (_fixedCanvasDeclarations.TryGetValue(owner, out Vector2 existing)) + { + if (existing == size) + return; // idempotent re-declare (e.g. a re-ticked activation edge) + throw new InvalidOperationException( + $"UiRoot.DeclareFixedCanvas: owner {owner} re-declared a different " + + $"canvas ({existing} -> {size}) without revoking first."); + } + + foreach (Vector2 declared in _fixedCanvasDeclarations.Values) + { + if (declared != size) + { + throw new InvalidOperationException( + $"UiRoot.DeclareFixedCanvas: owner {owner} declared {size} but " + + $"another active owner already declared {declared} — every " + + "concurrently-active fixed-canvas screen must author the SAME " + + "canvas size (see AD-98)."); + } + } + + _fixedCanvasDeclarations[owner] = size; + FixedCanvasSize = size; + } + + /// Revokes 's declaration from + /// . + /// becomes null only once EVERY declarer has revoked; while another + /// owner is still declared, it stays set to that shared value. A + /// revoke from an owner that never declared (or already revoked) is a + /// no-op, matching the idempotent shutdown paths (Deactivate + /// AND Dispose can both revoke the same owner). + public void RevokeFixedCanvas(object owner) + { + ArgumentNullException.ThrowIfNull(owner); + if (!_fixedCanvasDeclarations.Remove(owner)) + return; + + if (_fixedCanvasDeclarations.Count == 0) + { + FixedCanvasSize = null; + return; + } + + foreach (Vector2 declared in _fixedCanvasDeclarations.Values) + { + FixedCanvasSize = declared; + break; + } + } + /// /// The coordinate space the retained tree currently lays out in: the fixed /// authored canvas while one is active, else the window itself. Anything diff --git a/tests/AcDream.App.Tests/UI/Layout/CharacterScreensFixedCanvasArbiterTests.cs b/tests/AcDream.App.Tests/UI/Layout/CharacterScreensFixedCanvasArbiterTests.cs new file mode 100644 index 00000000..4e1480fc --- /dev/null +++ b/tests/AcDream.App.Tests/UI/Layout/CharacterScreensFixedCanvasArbiterTests.cs @@ -0,0 +1,430 @@ +using System.Numerics; +using AcDream.App.UI; +using AcDream.App.UI.Layout; +using AcDream.Core.CharGen; +using AcDream.Runtime; +using AcDream.Runtime.Session; + +namespace AcDream.App.Tests.UI.Layout; + +/// +/// Campaign CC slice CC4 review-fix round R1 (2026-08-15): +/// and +/// can be SIMULTANEOUSLY +/// active against the SAME — character-creation opens +/// on top of character-management, which keeps ticking underneath it. Both +/// controllers share , and before this +/// fix each wrote it directly — a raw write from either screen was a last- +/// writer-wins race with no owner: the F1 fix's own chargen +/// Close() nulled the canvas out from under a STILL-ACTIVE +/// character-management screen underneath it (the exact AD-98 gate-round-2 +/// defect resurfacing one layer up). These tests drive +/// / +/// across BOTH controllers on one shared host, per the reviewer's required +/// sequence. +/// +public sealed class CharacterScreensFixedCanvasArbiterTests +{ + private static readonly Vector2 AuthoredCanvas = new(800f, 600f); + + [Fact] + public void CanvasStaysSetWhileEitherScreenIsActive_AndNullsOnlyWhenBothRevoke() + { + using var environment = new TwoControllerHarness(); + + // char-mgmt alone: declared on its own activation edge (Tick's + // `if (!_active)` arm). + Assert.Equal(AuthoredCanvas, environment.Host.FixedCanvasSize); + + // chargen opens ON TOP of the still-active char-mgmt screen. + environment.Chargen.Controller.Open(); + Assert.Equal(AuthoredCanvas, environment.Host.FixedCanvasSize); + + // chargen Exit-confirms and closes -- char-mgmt is STILL ACTIVE, so + // the canvas must stay set. This is R1's regression: the pre-fix + // Close() nulled UiRoot.FixedCanvasSize unconditionally here, + // stripping it from char-management underneath. + environment.Chargen.Button(CharacterCreationUiController.ExitElementId) + .OnClick!(); + environment.Chargen.ConfirmActiveDialog(confirmed: true); + Assert.Equal(AuthoredCanvas, environment.Host.FixedCanvasSize); + Assert.False(environment.Chargen.Controller.Root.Visible); + + // char-mgmt deactivates (e.g. entering the world) -- now NEITHER + // screen declares, so the canvas nulls. + environment.Management.Runtime.SetLifecycle( + RuntimeCharacterSelectionLifecycle.InWorld); + environment.Management.Controller.Tick(); + Assert.Null(environment.Host.FixedCanvasSize); + } + + /// The original F1 defect's own covering case: both screens + /// revoke together (world entry while chargen was ALSO still open) + /// still nulls the canvas -- not just "one revokes while the other + /// holds," the scenario above. + [Fact] + public void CanvasNulls_WhenBothScreensRevokeAtWorldEntry() + { + using var environment = new TwoControllerHarness(); + environment.Chargen.Controller.Open(); + Assert.Equal(AuthoredCanvas, environment.Host.FixedCanvasSize); + + environment.Management.Runtime.SetLifecycle( + RuntimeCharacterSelectionLifecycle.InWorld); + environment.Management.Controller.Tick(); + environment.Chargen.Runtime.ProvideView = false; + environment.Chargen.Controller.Tick(); + + Assert.Null(environment.Host.FixedCanvasSize); + } + + // ── Fixture: one shared UiRoot, both controllers ──────────────────── + + private sealed class TwoControllerHarness : IDisposable + { + public TwoControllerHarness() + { + Host = new UiRoot { Width = 800f, Height = 600f }; + Management = new ManagementHarness(Host); + Chargen = new ChargenHarness(Host); + } + + public UiRoot Host { get; } + public ManagementHarness Management { get; } + public ChargenHarness Chargen { get; } + + public void Dispose() + { + Chargen.Dispose(); + Management.Dispose(); + } + } + + private sealed class ManagementHarness : IDisposable + { + private readonly RetailDialogFactory _dialogs; + + public ManagementHarness(UiRoot host) + { + ImportedLayout screen = BuildManagementScreen(); + Runtime = new ManagementFakeRuntime(); + _dialogs = new RetailDialogFactory( + host, + type => RetailDialogFactoryTests.BuildDialogLayout(type)); + Controller = Assert.IsType( + CharacterManagementUiController.Bind( + host, + screen, + static (_, _) => BuildRow(), + _dialogs, + Runtime.Bindings, + new CharacterManagementUiController.DialogStrings( + name => $"WARNING! {name}", + "DELETE", + "Please Wait", + "Entering World", + "Are you sure you want to leave?"))); + } + + public ManagementFakeRuntime Runtime { get; } + public CharacterManagementUiController Controller { get; } + + public void Dispose() + { + Controller.Dispose(); + _dialogs.Dispose(); + } + + private static UiElement BuildRow() => LayoutImporter.Build( + new ElementInfo { Id = 0x100003A5u, Type = 1u, Width = 160f, Height = 16f }, + _ => (0u, 0, 0), + null).Root; + + private static ImportedLayout BuildManagementScreen() + { + var root = new ElementInfo + { + Id = CharacterManagementUiController.RootElementId, + Type = 3u, + Width = 800f, + Height = 600f, + }; + var list = new ElementInfo + { + Id = CharacterManagementUiController.ListElementId, + Type = 5u, + X = 42f, + Y = 212f, + Width = 160f, + Height = 320f, + }; + list.TemplateList.Add(new UiTemplateListEntry(0x21000004u, 0x100003A5u)); + root.Children.Add(list); + root.Children.Add(new ElementInfo + { + Id = CharacterManagementUiController.WorldTextElementId, + Type = 12u, + Width = 193f, + Height = 110f, + }); + root.Children.Add(ButtonInfo(CharacterManagementUiController.CreateElementId)); + root.Children.Add(ButtonInfo(CharacterManagementUiController.EnterElementId)); + root.Children.Add(ButtonInfo(CharacterManagementUiController.DeleteElementId)); + root.Children.Add(ButtonInfo(CharacterManagementUiController.RestoreElementId)); + root.Children.Add(ButtonInfo(CharacterManagementUiController.CreditsElementId)); + root.Children.Add(ButtonInfo(CharacterManagementUiController.ExitElementId)); + return LayoutImporter.Build(root, _ => (0u, 0, 0), null); + } + } + + private sealed class ManagementFakeRuntime + { + private static readonly RuntimeGenerationToken Generation = new(11u); + private readonly FakeManagementView _view = new(); + + public ManagementFakeRuntime() + { + _view.Entries = [new RuntimeCharacterSelectionEntry(0, 0x50000001u, "Alpha", 0u)]; + _view.Snapshot = new RuntimeCharacterSelectionSnapshot( + Generation, + RuntimeCharacterSelectionLifecycle.AwaitingSelection, + Revision: 1, + AccountName: "account", + SlotCount: 5, + RosterCount: _view.Entries.Length, + WorldName: "sawato", + HighlightedCharacterId: 0x50000001u, + HighlightedDisplayIndex: 0, + PendingDeleteCharacterId: 0u, + LastRestoreRequestedCharacterId: 0u, + Operation: RuntimeCharacterSelectionOperation.None, + Error: null, + Buttons: new RuntimeCharacterSelectionButtons(true, true, false, true, false)); + Bindings = new CharacterSelectionRuntimeBindings( + View: () => _view, + Highlight: _ => Result(), + Enter: Result, + RequestDelete: Result, + ConfirmDelete: Result, + Restore: Result, + Cancel: Result, + RequestExit: () => { }); + } + + public CharacterSelectionRuntimeBindings Bindings { get; } + + private static RuntimeCommandResult Result() => + new(RuntimeCommandStatus.Accepted, Generation); + + public void SetLifecycle(RuntimeCharacterSelectionLifecycle lifecycle) + { + RuntimeCharacterSelectionSnapshot current = _view.Snapshot; + _view.Snapshot = current with { Lifecycle = lifecycle, Revision = current.Revision + 1 }; + } + + private sealed class FakeManagementView : IRuntimeCharacterSelectionView + { + public RuntimeCharacterSelectionEntry[] Entries { get; set; } = []; + public RuntimeCharacterSelectionSnapshot Snapshot { get; set; } + + public bool TryGetAt(int displayIndex, out RuntimeCharacterSelectionEntry character) + { + if ((uint)displayIndex >= (uint)Entries.Length) + { + character = default; + return false; + } + character = Entries[displayIndex]; + return true; + } + + public bool TryGet(uint characterId, out RuntimeCharacterSelectionEntry character) + { + int index = Array.FindIndex(Entries, entry => entry.CharacterId == characterId); + if (index < 0) + { + character = default; + return false; + } + character = Entries[index]; + return true; + } + + public void Visit(IRuntimeCharacterSelectionVisitor visitor) + { + foreach (RuntimeCharacterSelectionEntry character in Entries) + visitor.Visit(in character); + } + + public IDisposable Subscribe(IRuntimeCharacterSelectionObserver observer) => + NullSubscription.Instance; + + private sealed class NullSubscription : IDisposable + { + public static readonly NullSubscription Instance = new(); + public void Dispose() { } + } + } + } + + private sealed class ChargenHarness : IDisposable + { + private readonly List _dialogLayouts = []; + private readonly RetailDialogFactory _dialogs; + + public ChargenHarness(UiRoot host) + { + Screen = BuildChargenScreen(); + Runtime = new ChargenFakeRuntime(); + _dialogs = new RetailDialogFactory(host, type => + { + ImportedLayout layout = RetailDialogFactoryTests.BuildDialogLayout(type); + _dialogLayouts.Add(layout); + return layout; + }); + Controller = Assert.IsType( + CharacterCreationUiController.CreateDetached( + host, + Screen, + static (_, _) => null, + _dialogs, + Runtime.Bindings, + new CharacterCreationUiController.DialogStrings( + "Are you sure you want to leave?"))); + Controller.AttachAndTick(); + } + + public ImportedLayout Screen { get; } + public ChargenFakeRuntime Runtime { get; } + public CharacterCreationUiController Controller { get; } + + public UiButton Button(uint id) => + Assert.IsType(Screen.FindElement(id)); + + public void ConfirmActiveDialog(bool confirmed) + { + ImportedLayout dialog = _dialogLayouts[^1]; + uint buttonId = confirmed + ? RetailConfirmationDialogView.AcceptButtonId + : RetailConfirmationDialogView.RejectButtonId; + UiButton button = Assert.IsType(dialog.FindElement(buttonId)); + button.OnClick!(); + } + + public void Dispose() + { + Controller.Dispose(); + _dialogs.Dispose(); + } + + private static ImportedLayout BuildChargenScreen() + { + var root = new ElementInfo + { + Id = CharacterCreationUiController.RootElementId, + Type = 3u, + Width = 800f, + Height = 600f, + }; + root.Children.Add(ContainerInfo(CharacterCreationUiController.ProgressBarElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.BackElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.NextElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.FinishElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.HelpElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.ExitElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.RandomElementId)); + root.Children.Add(ContainerInfo(CharacterCreationUiController.MasterPageElementId)); + root.Children.Add(ContainerInfo(CharacterCreationUiController.HeritagePageElementId)); + root.Children.Add(ContainerInfo(CharacterCreationUiController.ProfessionPageElementId)); + root.Children.Add(ContainerInfo(CharacterCreationUiController.SkillsPageElementId)); + root.Children.Add(ContainerInfo(CharacterCreationUiController.AppearancePageElementId)); + root.Children.Add(ContainerInfo(CharacterCreationUiController.TownPageElementId)); + root.Children.Add(ContainerInfo(CharacterCreationUiController.SummaryPageElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.HeritageTabElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.ProfessionTabElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.SkillsTabElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.AppearanceTabElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.TownTabElementId)); + root.Children.Add(ButtonInfo(CharacterCreationUiController.SummaryTabElementId)); + return LayoutImporter.Build(root, _ => (0u, 0, 0), null); + } + + private static ElementInfo ContainerInfo(uint id) => + new() { Id = id, Type = 3u, Width = 200f, Height = 60f }; + } + + private sealed class ChargenFakeRuntime + { + private static readonly RuntimeGenerationToken Generation = new(13u); + + public ChargenFakeRuntime() + { + View = new FakeChargenView(); + Bindings = new CharacterCreationRuntimeBindings( + () => ProvideView ? View : null, + _ => Result(), + _ => Result(), + _ => Result(), + (_, _) => Result(), + (_, _) => Result(), + _ => Result(), + _ => Result(), + _ => Result(), + _ => Result(), + _ => Result(), + RequestExit: () => { }, + ResolveText: _ => null, + OpenOnStart: false); + } + + public FakeChargenView View { get; } + public CharacterCreationRuntimeBindings Bindings { get; } + public bool ProvideView { get; set; } = true; + + private static RuntimeCommandResult Result() => + new(RuntimeCommandStatus.Accepted, Generation); + + public sealed class FakeChargenView : IRuntimeCharacterCreationView + { + public RuntimeCharacterCreationSnapshot Snapshot { get; set; } = + new( + Generation, + IsActive: true, + Revision: 1, + HeritageId: 0u, + GenderKey: 0u, + Appearance: RuntimeCharacterCreationAppearance.Default, + Template: RuntimeCharacterCreationSnapshot.TemplateUnset, + Attributes: default, + AttributeLockMask: 0u, + TotalAttributeCredits: 0u, + RemainingAttributeCredits: 0, + TotalSkillCredits: 0u, + RemainingSkillCredits: 0, + Name: string.Empty, + StartArea: -1, + Slot: 0u, + VerificationPending: false, + LastLocalRefusal: default, + LastRejection: null, + LastCreated: null); + + public ChargenOptions Options => ChargenOptions.Empty; + + public ChargenSkillAdvancementClass GetSkillLevel(uint skillId) => + ChargenSkillAdvancementClass.Inactive; + + public IDisposable Subscribe(IRuntimeCharacterCreationObserver observer) => + NullSubscription.Instance; + + private sealed class NullSubscription : IDisposable + { + public static readonly NullSubscription Instance = new(); + public void Dispose() { } + } + } + } + + private static ElementInfo ButtonInfo(uint id) => + new() { Id = id, Type = 1u, Width = 100f, Height = 30f }; +} diff --git a/tests/AcDream.Headless.Tests/HeadlessSessionHostTests.cs b/tests/AcDream.Headless.Tests/HeadlessSessionHostTests.cs index 7a7081ac..45bea702 100644 --- a/tests/AcDream.Headless.Tests/HeadlessSessionHostTests.cs +++ b/tests/AcDream.Headless.Tests/HeadlessSessionHostTests.cs @@ -1,10 +1,15 @@ using System.Buffers.Binary; using System.Collections.Immutable; +using System.Collections.ObjectModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Net; using System.Numerics; using System.Reflection; +using System.Runtime.InteropServices; using System.Text.Json; +using AcDream.Content; +using AcDream.Content.CharGen; using AcDream.Core.Net; using AcDream.Core.Net.Messages; using AcDream.Core.Physics; @@ -19,11 +24,68 @@ using AcDream.Runtime.Gameplay; using AcDream.Runtime.Physics; using AcDream.Runtime.Session; using AcDream.Runtime.World; +using DatCharGen = DatReaderWriter.DBObjs.CharGen; +using DatHeritageGroupCG = DatReaderWriter.Types.HeritageGroupCG; +using DatIDBObj = DatReaderWriter.Lib.IO.IDBObj; +using DatDatabaseImpl = DatReaderWriter.DatDatabase; +using DatPStringBaseByte = DatReaderWriter.Types.PStringBase; namespace AcDream.Headless.Tests; public sealed class HeadlessSessionHostTests { + /// + /// Review fix round R3 (2026-08-15): proves the F6 fix actually opens + /// the gate, not just that InstallOptions was called. A content + /// lease carrying a REAL chargen table (one heritage that does NOT + /// exist in ) is + /// installed by the host constructor; selecting that heritage — refused + /// against the pre-F6 empty default — is ACCEPTED once the session + /// reaches character-creation-active. Begin/TrySelectHeritage + /// are called directly (both internal, reachable via this + /// project's InternalsVisibleTo on AcDream.Runtime) to isolate the + /// F6 wiring from the unrelated real-network handshake that would + /// otherwise be needed to reach the same session state. + /// + [Fact] + public void ContentLease_InstallsRealChargenOptions_SelectHeritageIsAccepted() + { + var factory = new ChargenFixtureContentFactory(); + using var owner = new HeadlessProcessContentOwner( + ContentDescriptor(), + _ => { }, + factory); + HeadlessProcessContentOwner.HeadlessProcessContentLease lease = + owner.AcquireLease("chargen-fixture"); + using var credential = new HeadlessCredentialSecret( + "fixture", + "password"); + using var diagnosticsOutput = new StringWriter(); + using var host = new HeadlessSessionHost( + Descriptor(), + credential, + new HeadlessDiagnosticWriter(diagnosticsOutput), + new FixtureSessionOperations(), + contentLease: lease); + + RuntimeCharacterCreationState creation = + host.Runtime.Session.CharacterCreationState; + + // F6 proof #1: the fixture heritage (absent from ChargenOptions.Empty) + // is actually installed off the content lease's real Dats. + Assert.True(creation.Options.HeritagesById.ContainsKey( + ChargenFixtureContentFactory.HeritageId)); + + // F6 proof #2: with real options installed, selecting that heritage + // is accepted once character-creation is active (Begin mirrors the + // real wire trigger this unit test bypasses). + creation.Begin(host.Runtime.Generation); + Assert.True(creation.TrySelectHeritage(ChargenFixtureContentFactory.HeritageId)); + Assert.Equal( + ChargenFixtureContentFactory.HeritageId, + creation.View.Snapshot.HeritageId); + } + [Fact] public void LoginCommandsUseTheHeadlessLiveBusAndPreserveWireOrder() { @@ -3685,4 +3747,140 @@ public sealed class HeadlessSessionHostTests } } + private static HeadlessContentDescriptor ContentDescriptor() => new() + { + DatDirectory = "fixture-dats", + PreparedAssetPath = "fixture.pak", + }; + + /// Review fix round R3 (2026-08-15): a content factory whose + /// Dats is a REAL (hand-built, not DispatchProxy-stubbed) + /// serving one heritage off + /// — the existing + /// TestResourceProxy pattern elsewhere in this project always + /// returns null/default and can't serve typed record data. + private sealed class ChargenFixtureContentFactory : IHeadlessProcessContentFactory + { + internal const uint HeritageId = 1u; + + public HeadlessOpenedProcessContent Open( + HeadlessContentDescriptor descriptor, + Action diagnostic) => + new( + new FixtureChargenDatReaderWriter(), + DispatchProxy.Create(), + MagicCatalog.Empty, + ImmutableArray.CreateRange(new float[256])); + } + + /// Minimal mirroring + /// ChargenTableReaderTests.EmptyDatReaderWriter's shape, except + /// serves a real for + /// — everything else + /// still misses, matching the missing-table tolerance + /// already has tests for. + private sealed class FixtureChargenDatReaderWriter : IDatReaderWriter + { + private readonly StubDatabase _db = new(); + private readonly DatCharGen _chargenTable = BuildChargenTable(); + + public string SourceDirectory => string.Empty; + public IDatDatabase Portal => _db; + public IDatDatabase Cell => _db; + public ReadOnlyDictionary CellRegions { get; } = + new(new Dictionary()); + public IDatDatabase HighRes => _db; + public IDatDatabase Language => _db; + public IDatDatabase Local => _db; + public ReadOnlyDictionary RegionFileMap { get; } = + new(new Dictionary()); + public int PortalIteration => 0; + public int CellIteration => 0; + public int HighResIteration => 0; + public int LanguageIteration => 0; + + public bool TryGetFileBytes( + uint regionId, + uint fileId, + ref byte[] bytes, + out int bytesRead) + { + bytesRead = 0; + return false; + } + + public IEnumerable GetAllIdsOfType() where T : DatIDBObj => + Array.Empty(); + + public IEnumerable ResolveId(uint id) => + Array.Empty(); + + public bool TrySave(T obj, int iteration = 0) where T : DatIDBObj => + throw new NotSupportedException(); + + public bool TrySave(uint regionId, T obj, int iteration = 0) where T : DatIDBObj => + throw new NotSupportedException(); + + [return: MaybeNull] + public T Get(uint fileId) where T : DatIDBObj => + fileId == ChargenTableReader.ChargenTableDid + && typeof(T) == typeof(DatCharGen) + ? (T)(object)_chargenTable + : default; + + public bool TryGet(uint fileId, [MaybeNullWhen(false)] out T value) + where T : DatIDBObj + { + value = Get(fileId); + return value is not null; + } + + public void Dispose() { } + + private static DatCharGen BuildChargenTable() + { + var table = new DatCharGen(); + var heritage = new DatHeritageGroupCG + { + Name = Str("FixtureHeritage"), + AttributeCredits = 60u, + SkillCredits = 50u, + }; + table.HeritageGroups.Add(ChargenFixtureContentFactory.HeritageId, heritage); + return table; + } + + private static DatPStringBaseByte Str(string value) + { + var s = new DatPStringBaseByte(); + s.Value = value; + return s; + } + + private sealed class StubDatabase : IDatDatabase + { + public DatDatabaseImpl Db => null!; + public int Iteration => 0; + public IEnumerable GetAllIdsOfType() where T : DatIDBObj => + Array.Empty(); + public bool TryGet(uint fileId, [MaybeNullWhen(false)] out T value) + where T : DatIDBObj + { + value = default; + return false; + } + public bool TryGetFileBytes(uint fileId, [MaybeNullWhen(false)] out byte[] value) + { + value = default; + return false; + } + public bool TryGetFileBytes(uint fileId, ref byte[] bytes, out int bytesRead) + { + bytesRead = 0; + return false; + } + public bool TrySave(T obj, int iteration = 0) where T : DatIDBObj => false; + public void Dispose() { } + } + } }