From 1774d8b29847ab5ee5f91fc890d82cdd7fdbabef Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 15 Aug 2026 17:51:14 +0200 Subject: [PATCH] =?UTF-8?q?fix(chargen):=20Campaign=20CC=20CC6a=20review?= =?UTF-8?q?=20fix=20round=20=E2=80=94=20F1-F12?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the CC6a dual-lens review (architectural PASS with reservations, retail fidelity PASS with reservations, merge after F1/F2/F3). F1 (BLOCKING) - AlternateSetup/setupId tested the wrong sentinel (0) instead of retail's INVALID_DID (0xFFFFFFFF, CharGenState::GetSetupID @0x005C5B22). A hair style storing that value would have been adopted as a literal Setup id, nulling Get and killing the whole preview. Fixed both sites with a new InvalidDid constant; added two hand-built tests plus an installed-DAT sweep of every hair style across all 26 heritage/gender combinations (869 selections, zero unresolved Setup ids). F2 (BLOCKING) - TS-82's register row, ChargenClothingTable.cs's doc, and the plan's ledger row all understated Undead's measured clothing-coverage gap as "headgear/trousers/footwear" (3 slots) with a self-contradicting "4 of 4 non-shirt slots" aside. Corrected everywhere to the true measured ALL FOUR slots (headgear, trousers, shirt, footwear). F3 (BLOCKING) - the palette-math "three independent sources" claim overcounted: ACViewer's ClothingTableList.xaml.cs:97 computes a different expression for a different problem, and its vendored PaletteSet.cs is ACE's own file, not an independent implementation. Rewrote the evidence paragraph in ChargenPalSetMath.cs to the two sources that actually hold (decomp control flow + ACE's "Taken from acclient.c" port). F4 (MEDIUM) - ChargenPreviewEntityBuilder.TryBuild did unlocked dat reads; DatCollection is not thread-safe and every sibling dat-touching resolver in this layer takes a shared datLock. Added a required datLock parameter; every dat read now happens inside one lock, mirroring RetailPaperdollPoseApplicator.Apply's shape. F5 (LOW) - noted the pre-existing Streaming.LandblockBuildFactoryTests timing flake in the ledger so a future session doesn't chase it. F6 (LOW) - fixed ChargenPreviewCamera.cs's rotation doc, which cited a nonexistent identifier in a dimensionally-wrong expression; corrected to retail's actual DoRotation @0x0047CAC7 per-tick formula. F7 (LOW-MEDIUM) - the TS-82 measurement was WriteLine-only; pinned with real assertions (zero gaps for the 9 standard heritages, exactly the 4 measured Undead table ids on both genders). Kept the existing env-gated skip pattern (confirmed house convention). F8 (LOW) - the inner PalSet-miss loop recorded-and-continued past a miss; retail's own loop returns immediately on a miss (~0x005A7B32), aborting every remaining choice in that garment. Changed continue to break; added a test proving a subsequent present PalSet is correctly not applied. F9 (LOW) - fixed three dangling doc references (the method is TryCompose). F10 (LOW) - the packed (byte)(range/8) narrowing was unchecked; a real NumColors of 2048 happened to wrap to the correct "whole palette" 0 sentinel by unchecked-cast accident. Replaced with explicit PackOffset/ PackNumColors helpers that document the 2048->0 equivalence deliberately and throw on any other unrepresentable shape. F11/F12 (LOW, CC6b scope) - noted in the plan's CC6b row: the second m_alternateSetupID override source is unmodelled, and a shared RetailHeldPose helper is worth extracting before a fourth consumer. Test counts: Core.Tests 4772/1 skip (+5), Content.Tests 147/0 (+1), App.Tests 5121/6 skips (unchanged; F5's named flake did not reproduce) - zero failures, full solution Release build green. Co-Authored-By: Claude Fable 5 --- .../retail-divergence-register.md | 4 +- .../2026-08-15-character-creation-campaign.md | 6 +- .../Rendering/ChargenPreviewCamera.cs | 10 +- .../Rendering/ChargenPreviewEntityBuilder.cs | 130 ++++++++----- .../CharGen/ChargenAppearanceFactory.cs | 95 +++++++-- .../CharGen/ChargenAppearanceSelection.cs | 2 +- .../CharGen/ChargenClothingTable.cs | 46 +++-- src/AcDream.Core/CharGen/ChargenPalSetMath.cs | 30 +-- .../ChargenPreviewEntityBuilderTests.cs | 6 +- ...argenAppearanceCatalogInstalledDatTests.cs | 182 ++++++++++++++++-- .../CharGen/ChargenAppearanceFactoryTests.cs | 170 ++++++++++++++++ 11 files changed, 561 insertions(+), 120 deletions(-) diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 1d9d05d4..1f0370f4 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -389,12 +389,12 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | AP-210 | **Filed 2026-08-15 at Campaign CC slice CC3.** Retail's `ApplyTemplate @ 0x005C5080` applies a chosen template's six attributes one at a time through the individually-guarded setters (`SetStrength(this, row.strength, 0)` … `SetSelf(this, row.self, 0)`), each of which can silently refuse to RAISE its value when `GetAbsRemainingCredits` for that specific attribute is exactly zero at the moment it runs — a narrow but real cross-attribute ordering effect when switching heritage/template leaves stale attribute values from a PRIOR selection still resident during the sequential apply. `RuntimeCharacterCreationState.ApplyTemplateLocked` instead assigns `_attributes = row.Attributes` as one atomic replacement. | `src/AcDream.Runtime/Session/RuntimeCharacterCreationState.cs` (`ApplyTemplateLocked`) | Every template row in the installed CharGen DAT is curated, self-consistent data (CC1's installed-DAT gates), so the guard is not expected to trip for any real heritage/template pair in isolation; the ordering effect only matters when switching directly between two heritages/templates with very different attribute totals, which is a corner case not yet gated by a connected test. | A rapid heritage-switch-then-template-switch sequence could theoretically leave an attribute at a value retail's sequential guard would have refused to reach; unreachable through this slice's own commands (heritage selection always re-derives the FULL budget before applying), but a future direct-attribute-manipulation caller bypassing `TrySelectHeritage`/`TrySelectTemplate` could differ from retail. | `CharGenState::ApplyTemplate @ 0x005C5080`; `CharGenState::SetStrength @ 0x005C4660` (representative of all six) | | AP-211 | **Filed 2026-08-15 at the Campaign CC slice CC3 review-fix round (F12).** `RuntimeCharacterCreationState.TryBeginFinish` refuses locally (`RuntimeCharacterCreationLocalRefusal.RosterFull`) when `rosterCount >= slotCount`, gating a Finish attempt against the account's CharacterSet slot cap. `gmCharGenMainUI::DoFinish @ 0x004E9170` itself has NO such check — the decomp shows only the name/credit/verification-state gates (see the row's own doc comment history). Retail instead enforces the slot cap ONE LAYER UP, in the char-select UI that ghosts/un-ghosts the Create button, not inside chargen's own Finish path — this campaign's plan doc records the finding as risk item 3 ("Slot cap is client-enforced only (ACE never checks on create) — honor `slotCount` like retail's UI did", `docs/plans/2026-08-15-character-creation-campaign.md` §Risks item 3) without a specific decomp citation for the UI-layer enforcement site (not yet located). ACE never checks the cap server-side either way. | `src/AcDream.Runtime/Session/RuntimeCharacterCreationState.cs` (`TryBeginFinish`, `RuntimeCharacterCreationLocalRefusal.RosterFull`) | A full roster still needs SOME refusal before the wire send — CC4's Create-button flow has not been built yet (no ghosted-button layer exists to enforce the cap earlier), so `TryBeginFinish` is the only chokepoint available today; ACE itself never validates the cap, so refusing one layer earlier than retail's own UI has no server-visible consequence. | If CC4 later adds the ghosted Create button matching retail's own enforcement layer, this row's gate becomes redundant defense-in-depth rather than the sole enforcement point — revisit whether to keep both or retire this one; until then, a caller that bypasses the ghosted button (a headless bot, a future scripted client) still gets a locally-refused Finish exactly where retail's UI would have blocked the click. | `gmCharGenMainUI::DoFinish @ 0x004E9170` (no slot-cap check present); `docs/plans/2026-08-15-character-creation-campaign.md` (Risks item 3) | -## 4. Temporary stopgap (TS) — 50 active rows (TS-83 filed 2026-08-15 at Campaign CC slice CC6a — the chargen 3D preview holds a static rest-pose final frame instead of retail's live 30fps idle loop, explicitly staged for CC6b to retire; TS-82 filed 2026-08-15 at Campaign CC slice CC6a — the chargen 3D preview's un-ported `ClothingTable::BuildObjDesc` Setup-substitution chain, measured (not assumed) to leave Undead's default headgear/trousers/footwear preview unclothed; TS-81 filed 2026-08-12 at Campaign FA slice FA2 — the AllegianceLoginNotification chat-text gap, BN-mislabeled string symbols pending DAT lookup; TS-80 partially narrowed same slice — the fellowship-create shareXp wire mechanism now exists, the option-bit reader is still FA4 scope; TS-75..TS-80 filed and TS-73 NARROWED 2026-08-11 at Campaign OP slice OP4 — the Character tab's 50-row consumer wiring: TS-73 narrowed to `DisableMostWeatherEffects`/`PersistentAtDay` only (`ViewCombatTarget`/`DisableDistanceFog` now work via App-layer poll bindings, not `TrySetOption`'s own switch); TS-75 "Always Daylight Outdoors" has no day/night time-of-day force (and corrects the plan's own `ForcedDayGroupIndex` mechanism-mismatch citation — that field is the WEATHER-VARIETY selector, not a time-of-day force); TS-76 five Character-tab rows with no consumer surface at all (3D tooltips, side-by-side vitals, spell durations, advanced combat UI, stay-in-chat-mode); TS-77 "Filter Language" has no profanity-filter subsystem; TS-78 "Use Main Pack as Default" has no client-side preferred-container consumer; TS-79 Group D salvage/housing (no salvage UI, no housing subsystem); TS-80 "Share Fellowship Experience and Luminance" is client-sourced (needs the fellowship-CREATE packet field, not just the stored bit) and unaudited this slice; TS-74 filed 2026-08-11 at Campaign OP slice OP3 — the Options panel's "Use Mouse Turning Settings" macro sends `PlayerOption.UseMouseTurning` and persists its five client-local siblings, but acdream has no persistent mouse-turning camera MODE for the bit to drive; TS-73 filed 2026-08-11 at the Campaign OP OP1 review-fix round — `RuntimeCharacterOptionsState.TrySetOption`'s port of `CPlayerModule::OnChanged`'s local side-effect switch (MF-2) covers only the two `PlayerModule`-state-mutating cases (0x02/0x12 fellowship mutual exclusion); the four presentation-binding cases (weather/day/combat-target/fog) remain unmodeled, pre-anchored to Campaign OP OP4's Group B consumer binds (see the row below); TS-71 RETIRED 2026-08-11 at the same round — both remaining `SetCharacterOptions (0x01A1)` flush triggers (the 480 s auto-save timer, the pre-logoff flush) are now wired through `LiveSessionController`'s own tick/stop transaction (`ConfigureAutoSaveTick`/`ConfigurePreLogoffFlush`, wired once by `GameRuntime`'s constructor), matching the plan's stated target; TS-72 RETIRED 2026-08-11 at the Campaign OP OP2 rework (double-REJECT fix round) — the click-toggle bit math is now decomp-CONFIRMED against `UIOption_CheckboxBitfield64::ListenToElementMessage @0x00485AE0` (`BitUtils::SetBitsOnOrOff`: OR-in-on / AND-NOT-off, which was already correct) and `::Refresh @0x004859C0` (the checked-state predicate, which WAS wrong — the shipped code required ALL mask bits set; retail checks on ANY mask bit — and is now fixed to match); the widget is still not reachable by any user (Campaign OP slice OP5 wires it), but nothing about its own click/checked mechanism remains genuinely unverified, so the row is retired rather than rewritten; TS-70 RETIRED 2026-08-09 at Campaign CH user-gate round 1, item E (#362) — `ClientCommandResponses.cs` now parses and renders all four named inbound GameEvents (`ChannelIndex 0x0149`, `ChannelList 0x0148`, `AvailableHouses 0x0271`, `AllegianceInfoResponse 0x027C`), each wired into `GameEventWiring.cs` and rendering retail-shaped `LogTextType 0x00` lines ported from the named-retail decomp (`Handle_Communication__ChannelIndex`/`ChannelList` @0x0057d0c0/@0x0057d230, `Handle_House__Recv_AvailableHouses` + `DisplayListOfCoords` @0x00585d50/@0x00585c20, `Handle_Allegiance__AllegianceInfoResponseEvent` @0x0056a1d0); the row's `@on`/`@off` mention was never itself missing a handler (both already resolve through the pre-existing `WeenieErrorWithString` registration) so nothing there needed a fix; TS-68/TS-69 filed 2026-08-09, Campaign CH slice CH4 — the deferred allegiance/house subcommand dispatchers, the three unported pure-local commands (day/log/render); TS-66/TS-67 filed and TS-29 retired 2026-08-08, Campaign A slice A5 — the region ambient system landed, so TS-29's ambient half is ported and its music half turned out to have nothing to port; TS-66 is the omitted `seen_outside` interior case and TS-67 the in-plane contribution weight. TS-64/TS-65 filed 2026-08-08, Campaign A slice A2 — TS-64 the two unimplemented retail sound preferences (unfocused-app silence, pan disable) plus the three enable bools; TS-65 the volume-squared quirk, applied on the ambient path where two lanes byte-confirmed it and deliberately NOT on the hook path where the pre-multiplying overload is unpinned. TS-62/TS-63 filed 2026-08-02, continuation-executor slice; TS-4 and TS-8 retired 2026-07-31; Campaign P's goal-enumerated physics stopgaps are now zero. TS-4's graph/flat Path-6 branches match retail's foot SetCollide/Adjusted and head CollisionNormal/Collided split with no BSP-layer sliding-normal write; TS-8's live 0x02C2 carries its complete StatMod through the canonical enchantment record and updates effective stats immediately. Campaign P P7 2026-07-30: TS-25 retired — outbound stance has shipped via RawState.CurrentStyle since #219; TS-24 re-argued to AD-57; TS-40 re-argued to AD-58; TS-35 retired at P5; earlier same campaign: TS-1/TS-5/TS-23/TS-46 retired by ports; TS-23 retired 2026-07-30 at Campaign P Slice P3 — every mover-flags call site (local player world-entry ×2, remote DR sweep ×2, remote teleport, ordinary movers) now ORs in the mover's real PK/PKLite/Impenetrable `ObjectInfoState` bits via the new `ClientObjectTable`-backed `EntityCollisionFlagsExt.ResolveMoverPvpState` — **narrative corrected 2026-08-03 (#297): "real" only became true at #297. Until then the bits existed but the source `PublicWeenieBitfield` was frozen at CreateObject, so every one of those sites read a stale value for the whole session. The site enumeration is also incomplete: `RuntimeSetPositionMoverPreparation.cs:183-188` is a SEVENTH mover-flags site that decodes `record.Snapshot.ObjectDescriptionFlags` directly rather than calling `ResolveMoverPvpState`, and it also derives `ObjectInfoState.IsPlayer` from the PWD bit, contradicting `EntityCollisionFlags.cs:119-123`'s claim that every site uses a GUID-prefix heuristic. See AP-134.** — and `PlayerWeenie.JumpStaminaCost`'s `pk` parameter reads the real `PlayerKillerStatus`/`LastPkAttackTimestamp` pair against a 20-second window instead of a hardcoded `false`; the non-PK invariant (every ACE default-created character) is bit-identical to the pre-P3 value since `ResolveMoverPvpState` and the PK-timer predicate both resolve to a no-op for `PublicWeenieBitfield` absent/0; TS-46 retired 2026-07-30 at Campaign P Slice P3 — the Setup's verbatim ≤2-sphere list (`CPhysicsObj::transition` 0x00512dc0 → `SPHEREPATH::init_sphere` 0x0050c670) now seeds the sweep for the local player, remote dead-reckoning, and ordinary movers alike, replacing the two-scalar (radius, height) capsule reconstruction; remote/ordinary step-up/step-down are now Setup-derived (`CPartArray::GetStepUpHeight`/`GetStepDownHeight`, 0x005180d0/0x005180f0, ×ObjScale) instead of a hardcoded 0.4 m, closing both residuals the row named; TS-5 retired 2026-07-30 at Campaign P Slice P1 — real burden-gated CanJump + real JumpStaminaCost, both decomp-verbatim; TS-1 retired 2026-07-30 at Campaign P Slice P2 — the row was stale; the EdgeSlide → PrecipiceSlide/CliffSlide chain is already a real, tested port; TS-57..TS-61 filed 2026-07-29 during Campaign N — no outbound RejectRetransmit; TS-27 narrowed same slice to the inbound direction) + TS-37 historical note (TS-20 retired 2026-07-16 — the later named-retail audit disproved the proposed DrawingBSP polygon filter; TS-37 is a retired-row historical note, not an active count; TS-39 retired R5-V3 — sticky seams bound to the ported PositionManager/StickyManager, radii threaded; TS-45 retired 2026-07-07 — hand-rolled `SphereCollision` replaced by the faithful CSphere family port, fixing the player-vs-monster crowd wedge; TS-3 retired 2026-07-07 — `frames_stationary_fall` accounting ported in the #182 verbatim UpdateObjectInternal rebuild, fixing the airborne falling-animation wedge; TS-41 retired 2026-07-07 — SERVERVEL synth-velocity remote body-drive replaced by the retail interp catch-up + unconditional MovementManager::UseTime, the remote-creature de-overlap #184; TS-42 retired 2026-07-19 — semantic animation completion now precedes the ordered Target/Movement/PartArray/Position tail; TS-44 narrowed again 2026-07-19 — complete orientation joined interpolation, only during-stick enqueue suppression remains) +## 4. Temporary stopgap (TS) — 50 active rows (TS-83 filed 2026-08-15 at Campaign CC slice CC6a — the chargen 3D preview holds a static rest-pose final frame instead of retail's live 30fps idle loop, explicitly staged for CC6b to retire; TS-82 filed 2026-08-15 at Campaign CC slice CC6a, corrected at the same-session review fix round (F2/F7) — the chargen 3D preview's un-ported `ClothingTable::BuildObjDesc` Setup-substitution chain, measured (not assumed) and now PINNED by a real assertion to leave Undead's default preview unclothed on ALL FOUR clothing slots (not three); TS-81 filed 2026-08-12 at Campaign FA slice FA2 — the AllegianceLoginNotification chat-text gap, BN-mislabeled string symbols pending DAT lookup; TS-80 partially narrowed same slice — the fellowship-create shareXp wire mechanism now exists, the option-bit reader is still FA4 scope; TS-75..TS-80 filed and TS-73 NARROWED 2026-08-11 at Campaign OP slice OP4 — the Character tab's 50-row consumer wiring: TS-73 narrowed to `DisableMostWeatherEffects`/`PersistentAtDay` only (`ViewCombatTarget`/`DisableDistanceFog` now work via App-layer poll bindings, not `TrySetOption`'s own switch); TS-75 "Always Daylight Outdoors" has no day/night time-of-day force (and corrects the plan's own `ForcedDayGroupIndex` mechanism-mismatch citation — that field is the WEATHER-VARIETY selector, not a time-of-day force); TS-76 five Character-tab rows with no consumer surface at all (3D tooltips, side-by-side vitals, spell durations, advanced combat UI, stay-in-chat-mode); TS-77 "Filter Language" has no profanity-filter subsystem; TS-78 "Use Main Pack as Default" has no client-side preferred-container consumer; TS-79 Group D salvage/housing (no salvage UI, no housing subsystem); TS-80 "Share Fellowship Experience and Luminance" is client-sourced (needs the fellowship-CREATE packet field, not just the stored bit) and unaudited this slice; TS-74 filed 2026-08-11 at Campaign OP slice OP3 — the Options panel's "Use Mouse Turning Settings" macro sends `PlayerOption.UseMouseTurning` and persists its five client-local siblings, but acdream has no persistent mouse-turning camera MODE for the bit to drive; TS-73 filed 2026-08-11 at the Campaign OP OP1 review-fix round — `RuntimeCharacterOptionsState.TrySetOption`'s port of `CPlayerModule::OnChanged`'s local side-effect switch (MF-2) covers only the two `PlayerModule`-state-mutating cases (0x02/0x12 fellowship mutual exclusion); the four presentation-binding cases (weather/day/combat-target/fog) remain unmodeled, pre-anchored to Campaign OP OP4's Group B consumer binds (see the row below); TS-71 RETIRED 2026-08-11 at the same round — both remaining `SetCharacterOptions (0x01A1)` flush triggers (the 480 s auto-save timer, the pre-logoff flush) are now wired through `LiveSessionController`'s own tick/stop transaction (`ConfigureAutoSaveTick`/`ConfigurePreLogoffFlush`, wired once by `GameRuntime`'s constructor), matching the plan's stated target; TS-72 RETIRED 2026-08-11 at the Campaign OP OP2 rework (double-REJECT fix round) — the click-toggle bit math is now decomp-CONFIRMED against `UIOption_CheckboxBitfield64::ListenToElementMessage @0x00485AE0` (`BitUtils::SetBitsOnOrOff`: OR-in-on / AND-NOT-off, which was already correct) and `::Refresh @0x004859C0` (the checked-state predicate, which WAS wrong — the shipped code required ALL mask bits set; retail checks on ANY mask bit — and is now fixed to match); the widget is still not reachable by any user (Campaign OP slice OP5 wires it), but nothing about its own click/checked mechanism remains genuinely unverified, so the row is retired rather than rewritten; TS-70 RETIRED 2026-08-09 at Campaign CH user-gate round 1, item E (#362) — `ClientCommandResponses.cs` now parses and renders all four named inbound GameEvents (`ChannelIndex 0x0149`, `ChannelList 0x0148`, `AvailableHouses 0x0271`, `AllegianceInfoResponse 0x027C`), each wired into `GameEventWiring.cs` and rendering retail-shaped `LogTextType 0x00` lines ported from the named-retail decomp (`Handle_Communication__ChannelIndex`/`ChannelList` @0x0057d0c0/@0x0057d230, `Handle_House__Recv_AvailableHouses` + `DisplayListOfCoords` @0x00585d50/@0x00585c20, `Handle_Allegiance__AllegianceInfoResponseEvent` @0x0056a1d0); the row's `@on`/`@off` mention was never itself missing a handler (both already resolve through the pre-existing `WeenieErrorWithString` registration) so nothing there needed a fix; TS-68/TS-69 filed 2026-08-09, Campaign CH slice CH4 — the deferred allegiance/house subcommand dispatchers, the three unported pure-local commands (day/log/render); TS-66/TS-67 filed and TS-29 retired 2026-08-08, Campaign A slice A5 — the region ambient system landed, so TS-29's ambient half is ported and its music half turned out to have nothing to port; TS-66 is the omitted `seen_outside` interior case and TS-67 the in-plane contribution weight. TS-64/TS-65 filed 2026-08-08, Campaign A slice A2 — TS-64 the two unimplemented retail sound preferences (unfocused-app silence, pan disable) plus the three enable bools; TS-65 the volume-squared quirk, applied on the ambient path where two lanes byte-confirmed it and deliberately NOT on the hook path where the pre-multiplying overload is unpinned. TS-62/TS-63 filed 2026-08-02, continuation-executor slice; TS-4 and TS-8 retired 2026-07-31; Campaign P's goal-enumerated physics stopgaps are now zero. TS-4's graph/flat Path-6 branches match retail's foot SetCollide/Adjusted and head CollisionNormal/Collided split with no BSP-layer sliding-normal write; TS-8's live 0x02C2 carries its complete StatMod through the canonical enchantment record and updates effective stats immediately. Campaign P P7 2026-07-30: TS-25 retired — outbound stance has shipped via RawState.CurrentStyle since #219; TS-24 re-argued to AD-57; TS-40 re-argued to AD-58; TS-35 retired at P5; earlier same campaign: TS-1/TS-5/TS-23/TS-46 retired by ports; TS-23 retired 2026-07-30 at Campaign P Slice P3 — every mover-flags call site (local player world-entry ×2, remote DR sweep ×2, remote teleport, ordinary movers) now ORs in the mover's real PK/PKLite/Impenetrable `ObjectInfoState` bits via the new `ClientObjectTable`-backed `EntityCollisionFlagsExt.ResolveMoverPvpState` — **narrative corrected 2026-08-03 (#297): "real" only became true at #297. Until then the bits existed but the source `PublicWeenieBitfield` was frozen at CreateObject, so every one of those sites read a stale value for the whole session. The site enumeration is also incomplete: `RuntimeSetPositionMoverPreparation.cs:183-188` is a SEVENTH mover-flags site that decodes `record.Snapshot.ObjectDescriptionFlags` directly rather than calling `ResolveMoverPvpState`, and it also derives `ObjectInfoState.IsPlayer` from the PWD bit, contradicting `EntityCollisionFlags.cs:119-123`'s claim that every site uses a GUID-prefix heuristic. See AP-134.** — and `PlayerWeenie.JumpStaminaCost`'s `pk` parameter reads the real `PlayerKillerStatus`/`LastPkAttackTimestamp` pair against a 20-second window instead of a hardcoded `false`; the non-PK invariant (every ACE default-created character) is bit-identical to the pre-P3 value since `ResolveMoverPvpState` and the PK-timer predicate both resolve to a no-op for `PublicWeenieBitfield` absent/0; TS-46 retired 2026-07-30 at Campaign P Slice P3 — the Setup's verbatim ≤2-sphere list (`CPhysicsObj::transition` 0x00512dc0 → `SPHEREPATH::init_sphere` 0x0050c670) now seeds the sweep for the local player, remote dead-reckoning, and ordinary movers alike, replacing the two-scalar (radius, height) capsule reconstruction; remote/ordinary step-up/step-down are now Setup-derived (`CPartArray::GetStepUpHeight`/`GetStepDownHeight`, 0x005180d0/0x005180f0, ×ObjScale) instead of a hardcoded 0.4 m, closing both residuals the row named; TS-5 retired 2026-07-30 at Campaign P Slice P1 — real burden-gated CanJump + real JumpStaminaCost, both decomp-verbatim; TS-1 retired 2026-07-30 at Campaign P Slice P2 — the row was stale; the EdgeSlide → PrecipiceSlide/CliffSlide chain is already a real, tested port; TS-57..TS-61 filed 2026-07-29 during Campaign N — no outbound RejectRetransmit; TS-27 narrowed same slice to the inbound direction) + TS-37 historical note (TS-20 retired 2026-07-16 — the later named-retail audit disproved the proposed DrawingBSP polygon filter; TS-37 is a retired-row historical note, not an active count; TS-39 retired R5-V3 — sticky seams bound to the ported PositionManager/StickyManager, radii threaded; TS-45 retired 2026-07-07 — hand-rolled `SphereCollision` replaced by the faithful CSphere family port, fixing the player-vs-monster crowd wedge; TS-3 retired 2026-07-07 — `frames_stationary_fall` accounting ported in the #182 verbatim UpdateObjectInternal rebuild, fixing the airborne falling-animation wedge; TS-41 retired 2026-07-07 — SERVERVEL synth-velocity remote body-drive replaced by the retail interp catch-up + unconditional MovementManager::UseTime, the remote-creature de-overlap #184; TS-42 retired 2026-07-19 — semantic animation completion now precedes the ordered Target/Movement/PartArray/Position tail; TS-44 narrowed again 2026-07-19 — complete orientation joined interpolation, only during-stick enqueue suppression remains) | # | Divergence | Where (file:line) | Why it is safe / justified | Risk if assumption breaks | Retail oracle | |---|---|---|---|---|---| | TS-83 | Chargen 3D preview (Campaign CC slice CC6a foundation): the preview holds a STATIC final-frame rest pose (`ChargenPreviewEntityBuilder.ApplyHeldPose`, retail's `m_didAnimationRest` DID resolution) instead of retail's live 30fps idle loop (`gmCG3DView`'s `m_didAnimation`/`m_didAnimArray` family, driven via `set_sequence_animation`). Deliberately staged, not discovered late: the campaign plan's own CC6 slice row names this exact split ("CC6a static-pose preview... register row for the missing idle loop, CC6b idle animation... retire the row"). | `src/AcDream.App/Rendering/ChargenPreviewEntityBuilder.cs` (`ApplyHeldPose`); `src/AcDream.App/Rendering/ChargenPreviewRenderer.cs` | Explicitly staged per `docs/plans/2026-08-15-character-creation-campaign.md`'s CC6 slice split; the identical held-pose technique is the paperdoll's own PERMANENT (not staged) design (`RetailPaperdollPoseApplicator.Apply`), so the mechanism itself is proven, only the "hold forever vs. play then hold" choice is temporary here. | The chargen preview shows a motionless character instead of retail's idle sway/breathing loop — cosmetic only; does not affect the composed appearance data (setup id, palette, part/texture overrides) CC6b's page will bind to. | `gmCG3DView` ctor + `::Update @ 0x004EE9D0` (`m_didAnimation`/`m_didAnimArray`/`m_didAnimationRest` DID assignments, pseudo-C ~0x004EE7C6-0x004EE995); `CreatureMode::set_sequence_animation` (idle-loop playback entry point, not yet located precisely — CC6b to find) | -| TS-82 | Chargen 3D preview (Campaign CC slice CC6a foundation): `ChargenClothingTable`'s composer skips retail's ~8-branch Setup-id substitution chain (`ClothingTable::BuildObjDesc @ 0x005A7900`'s Umbraen/Penumbraen/Undead/Anakshay fallback) when a garment's `ClothingBaseEffects` has no entry for the resolved body Setup. MEASURED (not assumed) against the installed EoR dat across all 26 heritage/gender combinations via `ChargenAppearanceCatalogInstalledDatTests`: the 9 standard heritages whose UI actually shows clothing controls resolve every default gear choice with zero coverage gaps. Undead is a real gap — its default headgear/trousers/footwear choices (both genders) have NO base-effect entry for Undead's own live body Setup (male 0x02001A9C / female 0x02001AA0), because that Setup is one of the skeleton/zombie variants the un-ported chain exists to redirect. Gear Knight and both Olthoi variants also show gaps under a synthetic "select every offered option" sweep, but retail hides the clothing controls entirely for those three heritages (`gmCGAppearancePage::Update @ 0x0047E8F0`'s `m_pClothesButton->SetVisible(0)` branches for `mHeritageGroup == 6` and `== 0xc \|\| == 0xd`), so a real chargen selection never reaches them — not a live gap. | `src/AcDream.Core/CharGen/ChargenClothingTable.cs`; `src/AcDream.Core/CharGen/ChargenAppearanceFactory.cs` (`ComposeClothingSlot`) | CC6a is explicitly the rendering-foundation slice (index→ObjDesc factory + static-pose offscreen renderer, no page mount yet); porting the ~8-branch substitution chain is bounded follow-up work once CC6b wires real clothing-slot UI, not a blocker for the foundation deliverable — and the installed-DAT test proves the gap is narrow (one heritage) rather than pervasive. | Undead's default headgear/trousers/footwear preview renders the bare body mesh for those three slots (no clothing part/texture override applied, though the dye subpalette contribution — gated on a DIFFERENT lookup — is unaffected) until the chain, or an equivalent per-heritage default-clothing-Setup map, is ported. | `ClothingTable::BuildObjDesc @ 0x005A7900` (Umbraen/Penumbraen/Undead/Anakshay Setup-substitution branches); `gmCGAppearancePage::Update @ 0x0047E8F0` (clothes-button visibility gate); `tests/AcDream.Content.Tests/CharGen/ChargenAppearanceCatalogInstalledDatTests.cs` | +| TS-82 | Chargen 3D preview (Campaign CC slice CC6a foundation): `ChargenClothingTable`'s composer skips retail's ~8-branch Setup-id substitution chain (`ClothingTable::BuildObjDesc @ 0x005A7900`'s Umbraen/Penumbraen/Undead/Anakshay fallback) when a garment's `ClothingBaseEffects` has no entry for the resolved body Setup. MEASURED (not assumed) against the installed EoR dat across all 26 heritage/gender combinations via `ChargenAppearanceCatalogInstalledDatTests`, with the measurement now PINNED by a real assertion rather than diagnostic-only output (review fix round F7): the 9 standard heritages whose UI actually shows clothing controls resolve every default gear choice with zero coverage gaps. Undead is a real gap — its default gear choices (both genders) have NO base-effect entry on **ALL FOUR clothing slots — headgear, trousers, shirt, AND footwear** (not the three-slot "headgear/trousers/footwear" this row originally understated, with a self-contradicting "4 of 4 non-shirt slots" aside — corrected at the review fix round F2) — for Undead's own live body Setup (male 0x02001A9C / female 0x02001AA0), because that Setup is one of the skeleton/zombie variants the un-ported chain exists to redirect. The four measured missing clothing-table ids are identical on both genders and in a fixed order: `0x10000009, 0x100000F9, 0x10000001, 0x10000007` (Headgear, Trousers, Shirt, Footwear — the factory's own composition order). Gear Knight and both Olthoi variants also show gaps under a synthetic "select every offered option" sweep, but retail hides the clothing controls entirely for those three heritages (`gmCGAppearancePage::Update @ 0x0047E8F0`'s `m_pClothesButton->SetVisible(0)` branches for `mHeritageGroup == 6` and `== 0xc \|\| == 0xd`), so a real chargen selection never reaches them — not a live gap. | `src/AcDream.Core/CharGen/ChargenClothingTable.cs`; `src/AcDream.Core/CharGen/ChargenAppearanceFactory.cs` (`ComposeClothingSlot`) | CC6a is explicitly the rendering-foundation slice (index→ObjDesc factory + static-pose offscreen renderer, no page mount yet); porting the ~8-branch substitution chain is bounded follow-up work once CC6b wires real clothing-slot UI, not a blocker for the foundation deliverable — and the installed-DAT test proves the gap is narrow (one heritage, all four of ITS slots) rather than pervasive. | Undead's default clothing preview renders the bare body mesh for ALL FOUR slots — headgear, trousers, shirt, AND footwear (no clothing part/texture override applied on any of them, though the dye subpalette contribution — gated on a DIFFERENT lookup — is unaffected) — until the chain, or an equivalent per-heritage default-clothing-Setup map, is ported. | `ClothingTable::BuildObjDesc @ 0x005A7900` (Umbraen/Penumbraen/Undead/Anakshay Setup-substitution branches); `gmCGAppearancePage::Update @ 0x0047E8F0` (clothes-button visibility gate); `tests/AcDream.Content.Tests/CharGen/ChargenAppearanceCatalogInstalledDatTests.cs` | | TS-73 | **NARROWED 2026-08-11 at Campaign OP slice OP4.** `RuntimeCharacterOptionsState.TrySetOption`'s port of `CPlayerModule::OnChanged @0x0059A8E0`'s local side-effect switch (step 2) still covers only the two `PlayerModule`-state-mutating cases (`case 2`/`case 0x12` fellowship mutual exclusion) — that part is unchanged. Of the four presentation-binding cases, TWO are now closed: `0x07 ViewCombatTarget` (re-pointed `ICombatGameplaySettingsSource` reads `RuntimeCharacterOptionsState` live — `CharacterOptionCombatSettingsSource`, `src/AcDream.App/Combat/LiveCombatAttackOperations.cs`) and `0x30 DisableDistanceFog` (`WeatherSystem.DisableDistanceFogSource`, a poll bound once in `GameWindow.cs`, forces `FogMode.Off` in `WeatherSystem.Snapshot`) — NEITHER lives inside `TrySetOption`'s own switch; both are separate App-layer poll bindings, so the literal claim in this row's title ("this Runtime-only seam can reach") stays true, but the user-observable symptom is fixed for these two ids. The remaining two, `0x04 DisableMostWeatherEffects` and `0x05 PersistentAtDay`, stay open — see TS-6 (weather-particle subsystem not yet located) and TS-75 (day/night force) respectively; this row no longer duplicates either. | `src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs` (`RuntimeCharacterOptionsState.TrySetOption`) | The remaining two options are correctly scoped to their OWN pre-existing/new rows (TS-6, TS-75) rather than re-litigated here. | Toggling `DisableMostWeatherEffects`/`PersistentAtDay` writes the bit and dirties/auto-saves it correctly, but produces NONE of retail's immediate local presentation change (weather doesn't stop, day/night doesn't force) — see TS-6/TS-75 for why. `ViewCombatTarget`/`DisableDistanceFog` are retired from this row's risk: both now behave correctly. | `CPlayerModule::OnChanged @0x0059A8E0`; `docs/research/2026-08-10-character-options-map.md` §1.5 | | TS-75 | "Always Daylight Outdoors" (`PlayerOption PersistentAtDay`, `CPlayerModule::OnChanged` case `0x05` → `LScape::SetDay(value)`) has no acdream consumer. The campaign plan's own Group-B binding table cites `RuntimeWorldEnvironmentDefinition.ForcedDayGroupIndex` as the target seam — **that citation is a mechanism mismatch, corrected here**: `ForcedDayGroupIndex` selects which WEATHER-VARIETY day-group (`RuntimeWorldDayGroupDefinition`, e.g. a Clear/Overcast/Rain/Snow/Storm pick) is always chosen — the SAME deterministic-per-day-RNG mechanism `WeatherSystem`'s own roll uses (see TS-6) — NOT retail's time-of-day day/night force. No acdream mechanism currently overrides the sky cycle's TIME to stay in daytime lighting; wiring this option correctly needs that mechanism built first, not just a poll into the wrong field. | `src/AcDream.Runtime/World/RuntimeWorldEnvironmentState.cs` (`RuntimeWorldEnvironmentDefinition.ForcedDayGroupIndex` — NOT the right target); no current consumer exists | Filed rather than silently wired to the wrong field — a poll into `ForcedDayGroupIndex` would have SILENTLY changed the character's weather-variety odds instead of forcing daytime, an incorrect fix masquerading as a correct one (CLAUDE.md's "no workarounds" rule). | Toggling the option writes the bit and dirties/auto-saves it correctly, but night still falls normally — no observable daylight-forcing behavior. | `CPlayerModule::OnChanged @0x0059A8E0` case 5; `LScape::SetDay` (not yet located in the decomp) | | TS-76 | Five Character-tab rows have no acdream consumer at all (research doc §4.2's own "state-only, no consumer" list, narrowed to the ids NOT already closed by Campaign OP's Group-C re-points): "Display 3D Tooltips" (`ShowTooltips`), "Side By Side Vitals" (`SideBySideVitals`), "Display Spell Durations" (`SpellDuration`), "Advanced Combat Interface" (`AdvancedCombatUI`), "Stay in Chat Mode After Sending a Message" (`StayInChatMode`) — retail renders 3D item tooltips, an alternate side-by-side vitals layout, remaining-duration overlays on enchantment icons, an expanded combat panel, and a chat-input-stays-open behavior respectively; acdream has none of the four rendering surfaces and no chat-input-close-on-send behavior to gate in the first place. | `src/AcDream.App/UI/Layout/CharacterOptionsPageController.cs` (the rows wire+store only) | Each needs a real UI/behavior feature built before the option means anything — inventing a stand-in now would be exactly the workaround CLAUDE.md forbids. | Toggling any of the five writes the bit and dirties/auto-saves it correctly, but no observable client behavior changes. | `gmGamePlayUI::RecvNotice_PlayerOptionChanged @0x004e9da0`; `EffectInfoRegion::Update @0x004f1c00`; `gmCombatUI::RecvNotice_SetCombatMode @0x004cc620`; `ChatInterface::HandleEnterKey @0x004f52d0`; `UIElement_SmartBoxWrapper::RecvNotice_SmartBoxObjectFound @0x004e5ad0` | diff --git a/docs/plans/2026-08-15-character-creation-campaign.md b/docs/plans/2026-08-15-character-creation-campaign.md index fd27cd37..bea19fab 100644 --- a/docs/plans/2026-08-15-character-creation-campaign.md +++ b/docs/plans/2026-08-15-character-creation-campaign.md @@ -253,6 +253,8 @@ the user gate. | 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 | — | | | | | CC5 | — | | | | -| CC6a | CODE-COMPLETE 2026-08-15 (foundation only — narrowed scope per the CC4∥CC6a parallelism contract: no page mount, no spin/color-wheel controls, no rotate/zoom behavior; all deferred to CC6b after CC4 merges) | single commit, HEAD of `campaign-cc6a` | PENDING (Opus dual-lens not yet run this session) | **Index→ObjDesc factory** (`ChargenAppearanceFactory.TryCompose`, `src/AcDream.Core/CharGen/`, pure — no Chorizite types on its public surface, verified by the existing `ChargenNoChoriziteLeakTests` reflection guard, which walks the whole `AcDream.Core.CharGen` namespace and now covers these new types too): ports `gmCG3DView::Update @ 0x004EE9D0`'s ObjDesc rebuild in its EXACT decompiled append order — base body → hair style → **Headgear → Trousers → Shirt → Footwear** (verified from the decompiled control flow, NOT the UI tab order 5/6/7/8 or the CC2 wire's field order, both of which are headgear/shirt/trousers/footwear and would have been wrong) → eyes (bald-aware) → nose → mouth → skin subpalette (UNCONDITIONAL, no selection gate, unlike every other slot) → hair color → eye color. New pure Core types: `ChargenPalSet`/`ChargenPalSetMath` (shade→index), `ChargenClothingTable`/`ChargenClothingBaseEffect`/`ChargenClothingPaletteTemplate`/`ChargenClothingSubPaletteChoice` (pure ClothingTable projection), `IChargenPalSetSource`/`IChargenClothingTableSource` (DAT-touching work pushed behind these, implemented by the new Content-layer `ChargenAppearanceCatalog`, `src/AcDream.Content/CharGen/`, a cached dat reader mirroring `ChargenTableReader`'s discipline), `ChargenAppearanceSelection` (mirrors `RuntimeCharacterCreationAppearance`'s 14-index/6-shade shape field-for-field so CC6b's Runtime→Core mapping is a trivial copy — kept as a separate type since Core cannot depend on Runtime). **Palette resolution — three-way agreement, no guessing:** `PalSet::GetPaletteID`'s FPU-elided body (`(int)((count - 0.000001) * shade)`, clamped) is corroborated by ACE's `PaletteSet.GetPaletteID` (comment: "Taken from acclient.c"), ACViewer's identical `ClothingTableList.xaml.cs:97` slider math, AND the decomp's own control-flow shape. Skin/hair use `PalSet`+shade indirection (skin: `sex.SkinPalSet`; hair: `sex.HairColors[i]` is ITSELF a PalSet id — confirmed against `PlayerFactory.cs:96`); eye color is the ONE exception — a raw Palette id used directly with NO shade indirection (confirmed against `PlayerFactory.cs:100`'s `EyesPalette = sex.EyeColorList[eyeColor]`, no `GetPaletteID` call, unlike the two lines above it). Hard-coded overlay ranges recovered from the decomp's literal bytes: skin (real offset 0, count 192 → packed 0/24), hair (192/64 → packed 24/8), eyes (256/64 → packed 32/8) — all three independently cross-checked against `PaletteOverride`'s pre-existing `*8` packing doc comment. **Clothing dye resolution, installed-DAT-verified:** `CharGenState::GetHeadgearPaletteTemplateID`/Shirt/Trousers/Footwear (0x005C38F0-0x005C3980) each read a PER-SLOT cached array, but all four are populated from the SAME single `Sex_CG::ClothingColors` dat field — there is no per-slot color list in the schema at all. This CONFIRMS (not merely approximates, contra the original AP-208 framing) that CC3's shared-list design is exactly retail's own mechanism; live-DAT probe: Aluvian male `ClothingColors = {9,6,4,8,7,5,2,3,13}` and the "Cloth Cap" headgear's `ClothingSubPalEffects` keys include every one of those values directly. **Chargen preview renderer** (`ChargenPreviewRenderer`, `ChargenPreviewCamera`/`ChargenPreviewViewportCamera`, `ChargenPreviewEntityBuilder`, all new files under `src/AcDream.App/Rendering/`): follows `PrivateEntityViewportRenderer`'s exact architecture (offscreen target → texture table → `UiViewport` sprite later), a THIRD facade beside `PaperdollViewportRenderer`/`CreatureAppraisalViewportRenderer` — no existing file touched. `ChargenPreviewEntityBuilder.TryBuild` resolves Setup/GfxObj/Surface/Animation dat data itself (there is no live entity yet) using the SAME algorithms as `DatLiveEntityProjectionMaterializer` (surface-override resolution ported verbatim) and `RetailPaperdollPoseApplicator` (final-frame held pose), generalized to the per-heritage rest-pose DID retail actually uses (`m_didAnimationRest`: enum `0x10000005` for every standard heritage — the SAME id the paperdoll's own pose reads — `0x10000011` for Olthoi, `0x10000013` for OlthoiAcid, all resolved through master-map slot 7). **Camera** (`gmCGAppearancePage::Update @ 0x0047E8F0`, cross-checked against the identical literals in `ZoomIn`/`ZoomOut @ 0x0047CF00`/`0x0047D050`): four distinct default (zoomed-in) eye profiles across the 13 heritages — Olthoi (0,-1.85,1.85), OlthoiAcid (0,-3.05,2.75), Tumerok (0,-0.85,1.65), everyone else including Gearknight (0,-0.55,1.65) — direction always identity (zero yaw/pitch, same convention `DollCamera` already established); zoomed-OUT profiles also recorded for CC6b (Olthoi (0,-3.80,1.15), OlthoiAcid (0,-5.70,1.65), everyone else (0,-2.50,0.95) — no Tumerok special case on the OUT side). Rotation is NOT a camera property: retail's continuous-rotation button spins the CHARACTER (`CPhysicsObj::set_heading`), not the camera — CC6b's heading parameter belongs on the entity builder. **Constants recovered, not just cited (deliverable #4):** `RotationSecondsPerRevolution = 3.0` (clean in the decomp, no reconstruction needed) and `ZoomTweenDurationSeconds = 0.6` — the plan's own risk list flagged this SECOND constant as "decompiler-garbled"; it is NOT unrecoverable: reinterpreting the decompiler's garbled float literal as the raw low-32-bit store and pairing it with the (clean) high dword reconstructs the exact IEEE-754 double both at `DoZoomAnimation`'s reset-default site (→ 0.6) AND independently at `ZoomIn`/`ZoomOut`'s `-0.1` invalidation sentinel (→ exactly the textbook IEEE-754 bit pattern for -0.1, cross-confirming the reconstruction technique itself). **Register rows filed (same commit):** TS-83 (the CC6a static-pose-vs-retail-idle-loop staging, explicitly named by the plan, to be retired by CC6b) and TS-82 (a MEASURED, not assumed, scope cut — CC6a's composer does not port retail's ~8-branch clothing Setup-substitution chain; the installed-DAT catalog test proves this costs nothing for the 9 standard heritages whose UI shows clothing controls, but Undead's default headgear/trousers/footwear choices genuinely miss `ClothingBaseEffects` coverage for Undead's own live body Setup on both genders — a real, narrow, documented gap, not a "confirmed unreachable" overclaim). **Tests:** `ChargenPalSetMathTests` (10 cases, the shade-index formula), `ChargenAppearanceFactoryTests` (19 hand-built-fixture cases covering setup resolution, retail append order, bald-strip selection, unconditional skin, missing-dat diagnostics, out-of-range indices), `ChargenAppearanceCatalogInstalledDatTests` (installed-DAT sweep, all 26 heritage/gender combinations, zero missing PalSet/ClothingTable ids — PASSED live against the installed EoR dat), `ChargenPreviewCameraTests` (17 cases, every per-heritage literal + the two recovered constants), `ChargenPreviewEntityBuilderTests` (3 cases, installed-DAT-gated, proves a real Aluvian-male 34-part mesh + Olthoi's distinct pose DID both resolve without touching a live entity). Final counts this session: Core.Tests 4767/1 skip, Content.Tests 146/0 skips, App.Tests 5121/6 skips — all pre-existing skips, zero failures, full solution Release build green. | -| CC6b | — | | | | +| CC6a | CODE-COMPLETE 2026-08-15 (foundation only — narrowed scope per the CC4∥CC6a parallelism contract: no page mount, no spin/color-wheel controls, no rotate/zoom behavior; all deferred to CC6b after CC4 merges) | single commit, HEAD of `campaign-cc6a` (plus a same-session review fix-round commit, F1-F12) | Dual-lens review returned architectural PASS with reservations + retail fidelity PASS with reservations, merge after F1/F2/F3 — all three (plus F4-F10) landed this round; F11/F12 are CC6b-scope notes only (see below) | **Index→ObjDesc factory** (`ChargenAppearanceFactory.TryCompose`, `src/AcDream.Core/CharGen/`, pure — no Chorizite types on its public surface, verified by the existing `ChargenNoChoriziteLeakTests` reflection guard, which walks the whole `AcDream.Core.CharGen` namespace and now covers these new types too): ports `gmCG3DView::Update @ 0x004EE9D0`'s ObjDesc rebuild in its EXACT decompiled append order — base body → hair style → **Headgear → Trousers → Shirt → Footwear** (verified from the decompiled control flow, NOT the UI tab order 5/6/7/8 or the CC2 wire's field order, both of which are headgear/shirt/trousers/footwear and would have been wrong) → eyes (bald-aware) → nose → mouth → skin subpalette (UNCONDITIONAL, no selection gate, unlike every other slot) → hair color → eye color. New pure Core types: `ChargenPalSet`/`ChargenPalSetMath` (shade→index), `ChargenClothingTable`/`ChargenClothingBaseEffect`/`ChargenClothingPaletteTemplate`/`ChargenClothingSubPaletteChoice` (pure ClothingTable projection), `IChargenPalSetSource`/`IChargenClothingTableSource` (DAT-touching work pushed behind these, implemented by the new Content-layer `ChargenAppearanceCatalog`, `src/AcDream.Content/CharGen/`, a cached dat reader mirroring `ChargenTableReader`'s discipline), `ChargenAppearanceSelection` (mirrors `RuntimeCharacterCreationAppearance`'s 14-index/6-shade shape field-for-field so CC6b's Runtime→Core mapping is a trivial copy — kept as a separate type since Core cannot depend on Runtime). **Palette resolution — two sources, no guessing (corrected at the review fix round — see F3 below):** `PalSet::GetPaletteID`'s FPU-elided body (`(int)((count - 0.000001) * shade)`, clamped) is corroborated by ACE's `PaletteSet.GetPaletteID` (comment: "Taken from acclient.c") AND the decomp's own control-flow shape (the `>= 0.0` gate at `0x005AC5A0`). ACViewer's `ClothingTableList.xaml.cs:97` does NOT corroborate this — it computes a different expression (`Shades.Maximum - 0.000001`, i.e. `count-1`, not `count`) for a different problem (mapping a shade back to a UI slider position), and `references/ACViewer`'s vendored `PaletteSet.cs` is ACE's own file, not an independent reimplementation — the original "three independent sources" claim overcounted by one. Skin/hair use `PalSet`+shade indirection (skin: `sex.SkinPalSet`; hair: `sex.HairColors[i]` is ITSELF a PalSet id — confirmed against `PlayerFactory.cs:96`); eye color is the ONE exception — a raw Palette id used directly with NO shade indirection (confirmed against `PlayerFactory.cs:100`'s `EyesPalette = sex.EyeColorList[eyeColor]`, no `GetPaletteID` call, unlike the two lines above it). Hard-coded overlay ranges recovered from the decomp's literal bytes: skin (real offset 0, count 192 → packed 0/24), hair (192/64 → packed 24/8), eyes (256/64 → packed 32/8) — all three independently cross-checked against `PaletteOverride`'s pre-existing `*8` packing doc comment. **Clothing dye resolution, installed-DAT-verified:** `CharGenState::GetHeadgearPaletteTemplateID`/Shirt/Trousers/Footwear (0x005C38F0-0x005C3980) each read a PER-SLOT cached array, but all four are populated from the SAME single `Sex_CG::ClothingColors` dat field — there is no per-slot color list in the schema at all. This CONFIRMS (not merely approximates, contra the original AP-208 framing) that CC3's shared-list design is exactly retail's own mechanism; live-DAT probe: Aluvian male `ClothingColors = {9,6,4,8,7,5,2,3,13}` and the "Cloth Cap" headgear's `ClothingSubPalEffects` keys include every one of those values directly. **Chargen preview renderer** (`ChargenPreviewRenderer`, `ChargenPreviewCamera`/`ChargenPreviewViewportCamera`, `ChargenPreviewEntityBuilder`, all new files under `src/AcDream.App/Rendering/`): follows `PrivateEntityViewportRenderer`'s exact architecture (offscreen target → texture table → `UiViewport` sprite later), a THIRD facade beside `PaperdollViewportRenderer`/`CreatureAppraisalViewportRenderer` — no existing file touched. `ChargenPreviewEntityBuilder.TryBuild` resolves Setup/GfxObj/Surface/Animation dat data itself (there is no live entity yet) using the SAME algorithms as `DatLiveEntityProjectionMaterializer` (surface-override resolution ported verbatim) and `RetailPaperdollPoseApplicator` (final-frame held pose), generalized to the per-heritage rest-pose DID retail actually uses (`m_didAnimationRest`: enum `0x10000005` for every standard heritage — the SAME id the paperdoll's own pose reads — `0x10000011` for Olthoi, `0x10000013` for OlthoiAcid, all resolved through master-map slot 7). **Camera** (`gmCGAppearancePage::Update @ 0x0047E8F0`, cross-checked against the identical literals in `ZoomIn`/`ZoomOut @ 0x0047CF00`/`0x0047D050`): four distinct default (zoomed-in) eye profiles across the 13 heritages — Olthoi (0,-1.85,1.85), OlthoiAcid (0,-3.05,2.75), Tumerok (0,-0.85,1.65), everyone else including Gearknight (0,-0.55,1.65) — direction always identity (zero yaw/pitch, same convention `DollCamera` already established); zoomed-OUT profiles also recorded for CC6b (Olthoi (0,-3.80,1.15), OlthoiAcid (0,-5.70,1.65), everyone else (0,-2.50,0.95) — no Tumerok special case on the OUT side). Rotation is NOT a camera property: retail's continuous-rotation button spins the CHARACTER (`CPhysicsObj::set_heading`), not the camera — CC6b's heading parameter belongs on the entity builder. **Constants recovered, not just cited (deliverable #4):** `RotationSecondsPerRevolution = 3.0` (clean in the decomp, no reconstruction needed) and `ZoomTweenDurationSeconds = 0.6` — the plan's own risk list flagged this SECOND constant as "decompiler-garbled"; it is NOT unrecoverable: reinterpreting the decompiler's garbled float literal as the raw low-32-bit store and pairing it with the (clean) high dword reconstructs the exact IEEE-754 double both at `DoZoomAnimation`'s reset-default site (→ 0.6) AND independently at `ZoomIn`/`ZoomOut`'s `-0.1` invalidation sentinel (→ exactly the textbook IEEE-754 bit pattern for -0.1, cross-confirming the reconstruction technique itself). **Register rows filed (same commit):** TS-83 (the CC6a static-pose-vs-retail-idle-loop staging, explicitly named by the plan, to be retired by CC6b) and TS-82 (a MEASURED, not assumed, scope cut — CC6a's composer does not port retail's ~8-branch clothing Setup-substitution chain; the installed-DAT catalog test proves this costs nothing for the 9 standard heritages whose UI shows clothing controls, but Undead's default gear choices genuinely miss `ClothingBaseEffects` coverage on ALL FOUR clothing slots — headgear, trousers, shirt, AND footwear, not the three-slot "headgear/trousers/footwear" an earlier draft of the row understated — for Undead's own live body Setup on both genders; the review fix round pinned this exact 4-table-id measurement with a real assertion rather than a WriteLine (F7), and corrected the row/doc-comment undercount (F2) — a real, narrow, documented gap, not a "confirmed unreachable" overclaim). **Tests (final, post-fix-round counts):** `ChargenPalSetMathTests` (10 cases, the shade-index formula), `ChargenAppearanceFactoryTests` (24 hand-built-fixture cases — the original 19 plus F1's 2 INVALID_DID-sentinel cases, F8's 1 abort-on-PalSet-miss case, F10's 2 packed-byte-conversion cases — covering setup resolution, retail append order, bald-strip selection, unconditional skin, missing-dat diagnostics, out-of-range indices), `ChargenAppearanceCatalogInstalledDatTests` (2 methods: the original installed-DAT sweep — all 26 heritage/gender combinations, zero missing PalSet/ClothingTable ids, PLUS F7's pinned TS-82 assertions — and F1's new 869-selection hair-style Setup-resolution sweep — PASSED live against the installed EoR dat), `ChargenPreviewCameraTests` (17 cases, every per-heritage literal + the two recovered constants), `ChargenPreviewEntityBuilderTests` (3 cases, installed-DAT-gated, proves a real Aluvian-male 34-part mesh + Olthoi's distinct pose DID both resolve without touching a live entity, now exercising the F4 `datLock` parameter). + +**Review fix round (F1-F12, same session):** F1 (BLOCKING) — `hairStyle.AlternateSetup != 0` / `setupId == 0` tested the wrong sentinel; retail's Setup "unset" is `INVALID_DID` (0xFFFFFFFF — `CharGenState::GetSetupID @0x005C5B22`), not 0, so an `AlternateSetup` field storing that value would have been ADOPTED as a literal Setup id, nulling `Get` and killing the whole preview. Fixed at both sites (`ChargenAppearanceFactory.cs`, new `InvalidDid` constant); two new hand-built tests plus a new installed-DAT sweep (`EveryHairStyleOfEveryHeritageGender_ComposesToARealInstalledSetupId`, 869 selections across all 26 heritage/gender combinations, zero unresolved). F2 (BLOCKING) — TS-82's register row, `ChargenClothingTable.cs`'s doc comment, and this ledger row all understated Undead's measured gap as "headgear/trousers/footwear" (3 slots) with a self-contradicting "4 of 4 non-shirt slots" aside; corrected everywhere to the true measured ALL FOUR slots (headgear, trousers, shirt, footwear). F3 (BLOCKING) — the "three independent sources" palette-math claim overcounted; corrected to the two that actually hold (decomp control flow + ACE's cited port) in `ChargenPalSetMath.cs`'s doc and this row (see above). F4 (MEDIUM, landed despite no CC6a call site yet) — `ChargenPreviewEntityBuilder.TryBuild` did unlocked dat reads; `DatCollection` is not thread-safe and every sibling dat-touching resolver in this layer takes a shared `object datLock`. Added a required `datLock` parameter; every dat read (Setup fetch, held-pose resolution, per-part GfxObj checks, surface-override resolution) now happens inside one `lock`, mirroring `RetailPaperdollPoseApplicator.Apply`'s "resolve under lock, process after" shape. F5 (LOW) — `Streaming.LandblockBuildFactoryTests.Build_UsesTheSuppliedSharedReaderGate` is a PRE-EXISTING timing flake unrelated to any chargen code (passes 15/15 in isolation per the reviewer); noted here so a future session doesn't chase it as a CC6a regression. F6 (LOW) — `ChargenPreviewCamera.cs`'s rotation doc cited a nonexistent `RotationDegreesPerSecond` identifier in a dimensionally-wrong expression; corrected to retail's actual per-tick formula (`DoRotation @0x0047CAC7`: `deltaDegrees = ((now - lastRotateTime) / RotationSecondsPerRevolution) * 360`). F7 (LOW-MEDIUM) — the installed-DAT tests' env-gated skip returns green with a console note when no dat dir is configured (confirmed this IS the house pattern — no Content installed-DAT test in the project uses `Assert.Skip`, so it was kept rather than diverging), but the TS-82 measurement was WriteLine-only; now pinned with real assertions (zero gaps for the 9 standard heritages, exactly the 4 measured Undead table ids on both genders — `[0x10000009, 0x100000F9, 0x10000001, 0x10000007]`, same order both genders). F8 (LOW) — the inner PalSet-miss loop recorded-and-continued past a miss; retail's own loop (`ClothingTable::BuildObjDesc` ~0x005A7B24-0x005A7BD3) returns 0 immediately on a miss at ~0x005A7B32, ABORTING every remaining choice in that garment — `continue` changed to `break`, new test proves a second (present) PalSet's choice is correctly NOT applied when it follows a missing one. F9 (LOW) — three dangling `` doc-comment references (the method is `TryCompose`) fixed. F10 (LOW) — the packed `(byte)(range.Offset/8)`/`(byte)(range.NumColors/8)` narrowing on dat-sourced data was unchecked (a real `NumColors` of 2048 wraps 256→0 as an unchecked byte cast, which HAPPENS to match retail's own "0 means whole palette" sentinel); replaced with explicit `PackOffset`/`PackNumColors` helpers that document the 2048→0 equivalence deliberately and throw `ArgumentOutOfRangeException` on any other unrepresentable shape, with two new tests (the sentinel case, the throwing case). F11/F12 (LOW, CC6b scope, no code this round) — noted in the CC6b row below: the second `m_alternateSetupID` override source (the appearance-page option checkbox — Penumbraen crown `@0x004DFB3F`, Undead no-flame `@0x004E0C54`, precedence at `@0x004EEA51`) is unmodelled; a shared `RetailHeldPose` helper is worth extracting before a fourth held-pose consumer exists (paperdoll, appraisal's live-target case is different, chargen — a third, not yet fourth). **Test counts after the fix round (measured, not projected):** Core.Tests 4772/1 skip (+5 from F1's two hand-built tests, F8's one, F10's two), Content.Tests 147/0 skips (+1 from F1's new installed-DAT sweep — F7 added assertions to the EXISTING installed-DAT test rather than a new one), App.Tests 5121/6 skips (unchanged pass count; F5's named flake did NOT reproduce in this session's full-suite run) — zero failures, full solution Release build green. | +| CC6b | NOT STARTED | | | **MUST-COVER, carried from the CC6a review fix round (F11/F12):** (1) retail's SECOND Setup-override source — `gmCG3DView`'s `m_alternateSetupID`, set from the Appearance page's option checkbox (Penumbraen crown variant `@0x004DFB3F`, Undead no-flame variant `@0x004E0C54`), takes precedence over the hair style's `AlternateSetup` at `gmCG3DView::Update`'s own resolution (`@0x004EEA51`) — CC6a's factory only ports the hair-style source; this second source is completely unmodelled and needs its own citation-backed port + register-row bookkeeping if CC6b doesn't fully close it. (2) Before adding a FOURTH consumer of the "resolve a rest-pose DID via master-map slot 7, load its Animation, hold the final frame" algorithm (paperdoll's `RetailPaperdollPoseApplicator`, CC6a's `ChargenPreviewEntityBuilder.ApplyHeldPose`/`ResolvePoseDid` are the second and third), extract a shared `RetailHeldPose` helper rather than copying it a third time. | | CC7 | — | | | | diff --git a/src/AcDream.App/Rendering/ChargenPreviewCamera.cs b/src/AcDream.App/Rendering/ChargenPreviewCamera.cs index eeb907ad..138dab5f 100644 --- a/src/AcDream.App/Rendering/ChargenPreviewCamera.cs +++ b/src/AcDream.App/Rendering/ChargenPreviewCamera.cs @@ -93,9 +93,13 @@ public sealed class ChargenPreviewCamera : ICamera /// (gmCGAppearancePage::m_dRotationPerSec, ctor pseudo-C /// ~137523-137524 / ~226652-226653: raw double bits low32=0x00000000, /// high32=0x40080000 → exactly 3.0 — the decompiler shows this cleanly, - /// no reconstruction needed). Consumed by CC6b's rotation controller as - /// 360f / RotationDegreesPerSecond — NOT applied here; see this - /// class's own doc comment on why rotation is not a camera concern. + /// no reconstruction needed). Retail's own per-tick formula + /// (gmCGAppearancePage::DoRotation @ 0x0047CA80, pseudo-C + /// ~0x0047CAC7): deltaDegrees = ((now - lastRotateTime) / + /// RotationSecondsPerRevolution) * 360 — CC6b's rotation controller + /// consumes this constant in exactly that shape, not as a + /// degrees-per-second rate. NOT applied here; see this class's own doc + /// comment on why rotation is not a camera concern. /// public const float RotationSecondsPerRevolution = 3.0f; diff --git a/src/AcDream.App/Rendering/ChargenPreviewEntityBuilder.cs b/src/AcDream.App/Rendering/ChargenPreviewEntityBuilder.cs index 3b79de88..0717430a 100644 --- a/src/AcDream.App/Rendering/ChargenPreviewEntityBuilder.cs +++ b/src/AcDream.App/Rendering/ChargenPreviewEntityBuilder.cs @@ -60,74 +60,85 @@ internal static class ChargenPreviewEntityBuilder /// failure shape treats /// as "drop this spawn"). /// + /// + /// Shared exclusion object for every dat read this method performs. + /// DatCollection is NOT thread-safe (see + /// claude-memory/feedback_phase_a1_hotfix_saga.md) — every other + /// dat-touching renderer/resolver in this layer + /// (RetailPaperdollPoseApplicator, PlayerModeController, + /// DatProjectileSetupResolver, EquippedChildRenderController) + /// takes the SAME object datLock the composition root threads + /// through as RuntimeOptions/d.DatLock; callers MUST pass + /// that same shared instance, not a private lock, or this method's reads + /// race every other consumer's. + /// public static WorldEntity? TryBuild( IDatReaderWriter dats, IAnimationLoader animations, ChargenAppearanceResult appearance, uint heritageId, - Quaternion heading) + Quaternion heading, + object datLock) { ArgumentNullException.ThrowIfNull(dats); ArgumentNullException.ThrowIfNull(animations); ArgumentNullException.ThrowIfNull(appearance); + ArgumentNullException.ThrowIfNull(datLock); - Setup? setup = dats.Get(appearance.SetupId); - if (setup is null) - return null; + List meshRefs; + uint setupId = appearance.SetupId; + PaletteOverride? paletteOverride; + PartOverride[] partOverrides; - var flattened = new List(SetupMesh.Flatten(setup)); - - foreach (ChargenAnimPartChange change in appearance.ObjDesc.AnimPartChanges) + // Every dat read this method performs — the Setup fetch, the held- + // pose animation resolution, the per-part GfxObj drawable checks, + // and the texture-change surface resolution — happens inside this + // one lock, mirroring RetailPaperdollPoseApplicator.Apply's "resolve + // everything under lock, then do pure processing" shape. + lock (datLock) { - if (change.PartIndex < flattened.Count) - flattened[change.PartIndex] = new MeshRef(change.PartId, flattened[change.PartIndex].PartTransform); - } + Setup? setup = dats.Get(setupId); + if (setup is null) + return null; - ApplyHeldPose(dats, animations, setup, heritageId, flattened); + var flattened = new List(SetupMesh.Flatten(setup)); - Dictionary>? surfaceOverrides = - ResolveSurfaceOverrides(dats, flattened, appearance.ObjDesc.TextureChanges); - - var meshRefs = new List(flattened.Count); - for (int partIndex = 0; partIndex < flattened.Count; partIndex++) - { - MeshRef part = flattened[partIndex]; - if (dats.Get(part.GfxObjId) is null) - continue; // matches DatLiveEntityProjectionMaterializer's drawable filter. - - IReadOnlyDictionary? overrides = null; - if (surfaceOverrides is not null && surfaceOverrides.TryGetValue(partIndex, out var perPart)) - overrides = perPart; - - meshRefs.Add(new MeshRef(part.GfxObjId, part.PartTransform) { SurfaceOverrides = overrides }); - } - if (meshRefs.Count == 0) - return null; - - PaletteOverride? paletteOverride = null; - if (appearance.ObjDesc.SubPalettes.Count > 0) - { - var ranges = new PaletteOverride.SubPaletteRange[appearance.ObjDesc.SubPalettes.Count]; - for (int i = 0; i < appearance.ObjDesc.SubPalettes.Count; i++) + foreach (ChargenAnimPartChange change in appearance.ObjDesc.AnimPartChanges) { - ChargenSubPalette sub = appearance.ObjDesc.SubPalettes[i]; - ranges[i] = new PaletteOverride.SubPaletteRange(sub.SubPaletteId, sub.Offset, sub.NumColors); + if (change.PartIndex < flattened.Count) + flattened[change.PartIndex] = new MeshRef(change.PartId, flattened[change.PartIndex].PartTransform); } - paletteOverride = new PaletteOverride(appearance.BasePaletteId, ranges); - } - var partOverrides = new PartOverride[appearance.ObjDesc.AnimPartChanges.Count]; - for (int i = 0; i < appearance.ObjDesc.AnimPartChanges.Count; i++) - { - ChargenAnimPartChange change = appearance.ObjDesc.AnimPartChanges[i]; - partOverrides[i] = new PartOverride(change.PartIndex, change.PartId); + ApplyHeldPose(dats, animations, setup, heritageId, flattened); + + Dictionary>? surfaceOverrides = + ResolveSurfaceOverrides(dats, flattened, appearance.ObjDesc.TextureChanges); + + meshRefs = new List(flattened.Count); + for (int partIndex = 0; partIndex < flattened.Count; partIndex++) + { + MeshRef part = flattened[partIndex]; + if (dats.Get(part.GfxObjId) is null) + continue; // matches DatLiveEntityProjectionMaterializer's drawable filter. + + IReadOnlyDictionary? overrides = null; + if (surfaceOverrides is not null && surfaceOverrides.TryGetValue(partIndex, out var perPart)) + overrides = perPart; + + meshRefs.Add(new MeshRef(part.GfxObjId, part.PartTransform) { SurfaceOverrides = overrides }); + } + if (meshRefs.Count == 0) + return null; + + paletteOverride = BuildPaletteOverride(appearance); + partOverrides = BuildPartOverrides(appearance); } return new WorldEntity { Id = PreviewRenderId, ServerGuid = PreviewServerGuid, - SourceGfxObjOrSetupId = appearance.SetupId, + SourceGfxObjOrSetupId = setupId, Position = Vector3.Zero, Rotation = heading, MeshRefs = meshRefs, @@ -137,6 +148,35 @@ internal static class ChargenPreviewEntityBuilder }; } + /// No dat access — pure projection of the already-composed + /// ObjDesc's subpalettes, safe to call outside datLock. + private static PaletteOverride? BuildPaletteOverride(ChargenAppearanceResult appearance) + { + if (appearance.ObjDesc.SubPalettes.Count == 0) + return null; + + var ranges = new PaletteOverride.SubPaletteRange[appearance.ObjDesc.SubPalettes.Count]; + for (int i = 0; i < appearance.ObjDesc.SubPalettes.Count; i++) + { + ChargenSubPalette sub = appearance.ObjDesc.SubPalettes[i]; + ranges[i] = new PaletteOverride.SubPaletteRange(sub.SubPaletteId, sub.Offset, sub.NumColors); + } + return new PaletteOverride(appearance.BasePaletteId, ranges); + } + + /// No dat access — pure projection, safe to call outside + /// datLock. + private static PartOverride[] BuildPartOverrides(ChargenAppearanceResult appearance) + { + var partOverrides = new PartOverride[appearance.ObjDesc.AnimPartChanges.Count]; + for (int i = 0; i < appearance.ObjDesc.AnimPartChanges.Count; i++) + { + ChargenAnimPartChange change = appearance.ObjDesc.AnimPartChanges[i]; + partOverrides[i] = new PartOverride(change.PartIndex, change.PartId); + } + return partOverrides; + } + /// /// Overwrites every part's transform from the resolved rest pose's /// FINAL frame — same "hold the settled last frame at zero frame rate" diff --git a/src/AcDream.Core/CharGen/ChargenAppearanceFactory.cs b/src/AcDream.Core/CharGen/ChargenAppearanceFactory.cs index c091471a..d2fa1d29 100644 --- a/src/AcDream.Core/CharGen/ChargenAppearanceFactory.cs +++ b/src/AcDream.Core/CharGen/ChargenAppearanceFactory.cs @@ -1,7 +1,7 @@ namespace AcDream.Core.CharGen; /// -/// The resolved render description +/// The resolved render description /// produces: a body Setup id plus the composed ObjDesc a mesh builder applies /// to it (CPhysicsObj::DoObjDescChangesFromDefault @ 0x0050F9B0 is /// retail's equivalent apply step). The three diagnostic lists let callers @@ -11,11 +11,15 @@ namespace AcDream.Core.CharGen; /// /// The body Setup dat id (0x02......) to build the preview mesh from — /// gender.SetupId, overridden by the selected hair style's -/// AlternateSetup when nonzero (Gear Knight / Undead / Tumerok body -/// variants), falling back to -/// when both are zero (retail: CPhysicsObj::makeObject(setupId)'s own -/// HUMAN_SETUP_ID fallback, gmCG3DView ctor pseudo-C ~0x004EE79D and -/// gmCG3DView::Update ~0x004EEA61). +/// AlternateSetup when it is neither 0 nor retail's INVALID_DID +/// (0xFFFFFFFF — Gear Knight / Undead / Tumerok body variants), falling back +/// to when the resolved +/// id is 0 OR INVALID_DID (retail: CharGenState::GetSetupID @ +/// 0x005C5B22 and gmCG3DView::Update's own check at +/// ~0x004EEA51/0x004EEA5F both test against INVALID_DID, not zero — +/// acclient.h:39909 types the field as IDClass, whose "unset" +/// value is 0xFFFFFFFF; CPhysicsObj::makeObject(setupId)'s own +/// HUMAN_SETUP_ID fallback, gmCG3DView ctor pseudo-C ~0x004EE79D). /// /// /// gender.BasePaletteId (retail Sex_CG.BasePalette) — the @@ -28,7 +32,7 @@ namespace AcDream.Core.CharGen; /// /// /// The composed subpalette/texture/part-swap deltas, in retail's exact -/// application order (see ). +/// application order (see ). /// public sealed record ChargenAppearanceResult( uint SetupId, @@ -86,6 +90,18 @@ public static class ChargenAppearanceFactory /// public const uint HumanSetupId = 0x02000001u; + /// + /// Retail's IDClass "unset" sentinel (INVALID_DID, + /// 0xFFFFFFFF — acclient.h:39909). CharGenState::GetSetupID @ + /// 0x005C5B22 and gmCG3DView::Update's own checks + /// (~0x004EEA51/0x004EEA5F) both test a Setup id against THIS value, not + /// zero — a hair style whose AlternateSetup field happens to + /// store this sentinel must be treated as "no override," exactly like + /// zero, or the factory would hand a bogus Setup id to + /// Get<Setup> and produce no preview at all. + /// + private const uint InvalidDid = 0xFFFFFFFFu; + /// /// Skin subpalette overlay range, retail's hard-coded literal at /// gmCG3DView::Update ~0x004EF066-0x004EF07E: real byte offset 0, @@ -151,10 +167,10 @@ public static class ChargenAppearanceFactory && selection.HairStyle < (uint)gender.HairStyles.Count) { hairStyle = gender.HairStyles[(int)selection.HairStyle]; - if (hairStyle.AlternateSetup != 0) + if (hairStyle.AlternateSetup != 0 && hairStyle.AlternateSetup != InvalidDid) setupId = hairStyle.AlternateSetup; } - if (setupId == 0) + if (setupId == 0 || setupId == InvalidDid) setupId = HumanSetupId; // ── 2. ObjDesc accumulation, retail's exact append order ─────── @@ -322,15 +338,22 @@ public static class ChargenAppearanceFactory uint paletteTemplateId = clothingColors[(int)colorIndex]; if (!table.PaletteTemplatesById.TryGetValue(paletteTemplateId, out ChargenClothingPaletteTemplate? template)) - return; // retail: hash miss on the palette-template lookup is a silent no-op. + return; // retail: hash miss on the OUTER palette-template lookup is a silent no-op. foreach (ChargenClothingSubPaletteChoice choice in template.Choices) { ChargenPalSet? palSet = palSets.TryGetPalSet(choice.PalSetId); if (palSet is null) { + // Retail's own inner loop (ClothingTable::BuildObjDesc + // ~0x005A7B24-0x005A7BD3) returns 0 IMMEDIATELY when + // DBObj::Get fails for one subpalEffect entry's PalSet + // (~0x005A7B32) — aborting every REMAINING choice in this + // same garment's palette template, not merely skipping the + // failed one. `break`, not `continue`, matches that; the + // miss is still recorded so callers can see it happened. missingPalSets.Add(choice.PalSetId); - continue; + break; } int index = ChargenPalSetMath.GetPaletteIndex(palSet.PaletteIds.Count, shade); @@ -342,9 +365,55 @@ public static class ChargenAppearanceFactory { subPalettes.Add(new ChargenSubPalette( paletteId, - (byte)(range.Offset / 8), - (byte)(range.NumColors / 8))); + PackOffset(range.Offset), + PackNumColors(range.NumColors))); } } } + + /// + /// Converts a real (unpacked) clothing subpalette offset into + /// 's packed *8 on-disk unit. Throws + /// rather than silently truncating on a shape we've never seen and + /// don't know how to represent losslessly (guards against the + /// unchecked-narrowing footgun a plain (byte)(value / 8) cast + /// would otherwise hide). + /// + private static byte PackOffset(uint realOffset) + { + if (realOffset % 8u != 0 || realOffset > 2040u) + { + throw new ArgumentOutOfRangeException( + nameof(realOffset), + realOffset, + "Clothing subpalette range offset does not fit the packed *8 byte " + + "convention (expected a multiple of 8 in [0, 2040])."); + } + return (byte)(realOffset / 8u); + } + + /// + /// Same packing as , plus retail's own explicit + /// "whole palette" sentinel: a packed NumColors of 0 means "the + /// entire palette" ('s + /// doc: "Length=0 is a sentinel meaning entire palette... defaulting to + /// 256*8"). A real count of exactly 2048 (256*8) IS that same value + /// spelled out in real units, so it packs to 0 BY DESIGN — not because + /// an unchecked (byte) cast happens to wrap 256 back to 0. + /// + private static byte PackNumColors(uint realNumColors) + { + if (realNumColors == 2048u) + return 0; + if (realNumColors % 8u != 0 || realNumColors > 2040u) + { + throw new ArgumentOutOfRangeException( + nameof(realNumColors), + realNumColors, + "Clothing subpalette range color count does not fit the packed *8 byte " + + "convention (expected a multiple of 8 in [0, 2040], or exactly 2048 " + + "for the whole-palette sentinel)."); + } + return (byte)(realNumColors / 8u); + } } diff --git a/src/AcDream.Core/CharGen/ChargenAppearanceSelection.cs b/src/AcDream.Core/CharGen/ChargenAppearanceSelection.cs index efe2d421..f26b6427 100644 --- a/src/AcDream.Core/CharGen/ChargenAppearanceSelection.cs +++ b/src/AcDream.Core/CharGen/ChargenAppearanceSelection.cs @@ -2,7 +2,7 @@ namespace AcDream.Core.CharGen; /// /// The fourteen style/color indices plus the six f64 shades -/// needs to build a preview +/// needs to build a preview /// description — field-for-field the same shape as CC3's /// AcDream.Runtime.Session.RuntimeCharacterCreationAppearance (and, /// through it, CharacterCreate.Appearance's wire fields), kept as a diff --git a/src/AcDream.Core/CharGen/ChargenClothingTable.cs b/src/AcDream.Core/CharGen/ChargenClothingTable.cs index f46227f9..3601bf48 100644 --- a/src/AcDream.Core/CharGen/ChargenClothingTable.cs +++ b/src/AcDream.Core/CharGen/ChargenClothingTable.cs @@ -85,31 +85,35 @@ public sealed record ChargenClothingBaseEffect( /// Penumbraen, Undead skeleton/zombie, Anakshay) when /// has no direct entry for the requested /// body Setup. CC6a's composer looks up -/// directly and skips a slot's part/texture contribution on a miss -/// (matching retail's own "hash miss → BuildObjDesc returns failure, caller -/// does not check it, ObjDesc keeps whatever it already had" behavior) -/// rather than porting the substitution chain. The installed-DAT catalog -/// test (ChargenAppearanceCatalogInstalledDatTests) MEASURED this -/// directly across all 26 heritage/gender combinations rather than assuming -/// it: for the 9 standard heritages where retail's own UI actually shows -/// clothing controls (everything except Gear Knight and the two Olthoi -/// variants, which retail hides the clothes button for entirely — -/// gmCGAppearancePage::Update @ 0x0047E8F0's +/// directly and skips a slot's part/texture contribution on a miss (this is +/// the OUTER lookup — ClothingTable::_cloBaseHash — whose retail +/// miss behavior is genuinely a no-op the caller never checks; the SEPARATE +/// inner per-choice PalSet lookup inside the same function's subpalette loop +/// has its own, stricter, abort-on-miss behavior — see +/// ChargenAppearanceFactory.ComposeClothingSlot's own doc, ported +/// faithfully there) rather than porting the Setup-substitution chain. The +/// installed-DAT catalog test (ChargenAppearanceCatalogInstalledDatTests) +/// MEASURED this directly across all 26 heritage/gender combinations rather +/// than assuming it: for the 9 standard heritages where retail's own UI +/// actually shows clothing controls (everything except Gear Knight and the +/// two Olthoi variants, which retail hides the clothes button for entirely +/// — gmCGAppearancePage::Update @ 0x0047E8F0's /// m_pClothesButton->SetVisible(0) branches for /// mHeritageGroup == 6 and == 0xc || == 0xd), the default /// gear choices resolve against their own body Setup with ZERO missing /// coverage. Undead IS a real gap — retail DOES show clothing -/// controls for Undead, but its default headgear/trousers/footwear choices -/// have no entry for either gender's -/// live Setup id (measured: 4 of 4 non-shirt slots miss, on both genders), -/// because Undead's live body Setup IS one of the skeleton/zombie variants -/// the un-ported substitution chain exists to redirect. A live preview for -/// Undead will therefore render its default headgear/trousers/footwear -/// choice with NO part/texture override applied (the underlying body shows -/// through unclothed for those slots) until the substitution chain — or an -/// equivalent per-heritage default-clothing-setup mapping — lands. Filed as -/// a known CC6a limitation for CC6b/a follow-up rather than silently -/// "confirmed unreachable." +/// controls for Undead, and MEASURED coverage is missing for ALL FOUR +/// clothing slots (headgear, trousers, shirt, AND footwear — not just three +/// of the four), on both genders: neither gender's live body Setup has a +/// entry in any of its four default gear +/// choices' clothing tables, because Undead's live body Setup IS one of the +/// skeleton/zombie variants the un-ported substitution chain exists to +/// redirect. A live preview for Undead will therefore render its default +/// clothing selection with NO part/texture override applied on any of the +/// four slots (the underlying body shows through unclothed) until the +/// substitution chain — or an equivalent per-heritage default-clothing-setup +/// mapping — lands. Filed as a known CC6a limitation for CC6b/a follow-up +/// rather than silently "confirmed unreachable." /// /// public sealed record ChargenClothingTable( diff --git a/src/AcDream.Core/CharGen/ChargenPalSetMath.cs b/src/AcDream.Core/CharGen/ChargenPalSetMath.cs index 68cdb042..2076013b 100644 --- a/src/AcDream.Core/CharGen/ChargenPalSetMath.cs +++ b/src/AcDream.Core/CharGen/ChargenPalSetMath.cs @@ -5,17 +5,25 @@ namespace AcDream.Core.CharGen; /// (PalSet::GetPaletteID @ 0x005AC570, invoked from /// gmCG3DView::Update @ 0x004EE9D0 for the skin/hair subpalette /// build and from ClothingTable::BuildObjDesc @ 0x005A7900 for every -/// clothing-slot dye choice). The decompiled body is FPU-elided (the x87 -/// bounds-compare against 0.0/1.0 and the truncating _ftol2() cast -/// lose their operands to the decompiler), but ACE's -/// ACE.DatLoader.FileTypes.PaletteSet.GetPaletteID carries the -/// explicit comment "Taken from acclient.c (PalSet::GetPaletteID)" with the -/// exact formula below — corroborated by the decomp's own control-flow -/// shape (a two-sided FPU compare consistent with a [0,1] bounds -/// check, then one truncating cast) and independently by ACViewer's -/// ClothingTableList.xaml.cs:97 UI slider, which reimplements the -/// identical (count - 0.000001) * shade expression for its own shade -/// preview. Three independent sources agree. +/// clothing-slot dye choice). The decompiled body is genuinely FPU-elided — +/// the _ftol2() truncating-cast operand is lost to the decompiler, +/// and can only be read as "some product of -ish and +/// -ish operands" from the surrounding x87 stack +/// traffic — but the decomp's own control-flow SHAPE is still verifiable +/// independent of that lost operand: a two-sided FPU compare at +/// 0x005AC5A0 gating on >= 0.0, consistent with a +/// [0,1] shade bounds check before the cast. What resolves the +/// elided operand is ACE's ACE.DatLoader.FileTypes.PaletteSet.GetPaletteID, +/// which carries the explicit comment "Taken from acclient.c +/// (PalSet::GetPaletteID)" against the exact formula below. That is TWO +/// sources (decomp control flow + ACE's cited port), not three: the +/// PaletteSet.cs file present in the vendored ACViewer checkout is +/// ACE's own file, not an independent reimplementation, and ACViewer's +/// ClothingTableList.xaml.cs:97 UI slider computes a DIFFERENT +/// expression for a DIFFERENT problem (mapping a shade back to a slider tick +/// position against Shades.Maximum, i.e. count-1, not +/// count) — neither corroborates this formula and both are dropped +/// from the evidence chain here. /// public static class ChargenPalSetMath { diff --git a/tests/AcDream.App.Tests/Rendering/ChargenPreviewEntityBuilderTests.cs b/tests/AcDream.App.Tests/Rendering/ChargenPreviewEntityBuilderTests.cs index 8a3f45d3..656b27eb 100644 --- a/tests/AcDream.App.Tests/Rendering/ChargenPreviewEntityBuilderTests.cs +++ b/tests/AcDream.App.Tests/Rendering/ChargenPreviewEntityBuilderTests.cs @@ -53,7 +53,7 @@ public sealed class ChargenPreviewEntityBuilderTests var animations = new RetailAnimationLoader(adapter); var entity = ChargenPreviewEntityBuilder.TryBuild( - adapter, animations, appearance, heritageId: 1u, Quaternion.Identity); + adapter, animations, appearance, heritageId: 1u, Quaternion.Identity, new object()); Assert.NotNull(entity); Assert.NotEmpty(entity!.MeshRefs); @@ -85,7 +85,7 @@ public sealed class ChargenPreviewEntityBuilderTests ClothingTablesMissingBaseEffectForSetup: []); var entity = ChargenPreviewEntityBuilder.TryBuild( - adapter, animations, bogusAppearance, heritageId: 1u, Quaternion.Identity); + adapter, animations, bogusAppearance, heritageId: 1u, Quaternion.Identity, new object()); Assert.Null(entity); } @@ -115,7 +115,7 @@ public sealed class ChargenPreviewEntityBuilderTests Assert.True(composed); var entity = ChargenPreviewEntityBuilder.TryBuild( - adapter, animations, appearance, heritageId: 12u, Quaternion.Identity); + adapter, animations, appearance, heritageId: 12u, Quaternion.Identity, new object()); // Just proves the Olthoi branch doesn't throw / silently fall through to // "no mesh" — the exact pose DID differs internally (0x10000011 vs diff --git a/tests/AcDream.Content.Tests/CharGen/ChargenAppearanceCatalogInstalledDatTests.cs b/tests/AcDream.Content.Tests/CharGen/ChargenAppearanceCatalogInstalledDatTests.cs index b4d08248..98c0d0a4 100644 --- a/tests/AcDream.Content.Tests/CharGen/ChargenAppearanceCatalogInstalledDatTests.cs +++ b/tests/AcDream.Content.Tests/CharGen/ChargenAppearanceCatalogInstalledDatTests.cs @@ -13,19 +13,52 @@ namespace AcDream.Content.Tests.CharGen; /// everywhere, mid shade" selection and asserts it resolves with no missing /// PalSet or ClothingTable dat ids — the CC6a task's explicit acceptance /// bar ("every heritage/gender's default selection resolves to a complete -/// description with no missing dat ids"). Also records (without asserting -/// zero — see the class doc on 's -/// deliberate scope cut) how many clothing slots have no -/// ClothingBaseEffects entry for their own gender's body Setup, so a -/// future session can see at a glance whether CC6a's decision to skip -/// retail's Setup-substitution fallback chain ever actually costs -/// coverage on the real dat. +/// description with no missing dat ids"). ALSO pins the TS-82 measurement +/// with real assertions (not WriteLine-only diagnostics, per the CC6a +/// review fix round F7): the nine standard heritages with clothing UI shown +/// resolve zero ClothingBaseEffects gaps, and Undead resolves +/// EXACTLY the four measured gaps on both genders — see the class doc on +/// 's deliberate scope cut. +/// +/// Env-gated skip (house pattern, matched from +/// ChargenTableReaderInstalledDatTests/ContentConformanceDats): +/// returns green with a console SKIP note when no installed dat directory is +/// configured, rather than a true xUnit Skipped status — no other Content +/// installed-DAT test in this project uses Assert.Skip, so this stays +/// consistent with the rest of the suite rather than introducing a new +/// convention. /// public sealed class ChargenAppearanceCatalogInstalledDatTests { private readonly ITestOutputHelper _out; public ChargenAppearanceCatalogInstalledDatTests(ITestOutputHelper output) => _out = output; + // ACE ACE.Entity.Enum.HeritageGroup ids. Gearknight (6)/Olthoi (12)/ + // OlthoiAcid (13) are deliberately not named here — see the WriteLine-only + // comment in the loop below for why they carry no pinned expectation. + private const uint TumerokId = 7u; + private const uint UndeadId = 11u; + + /// + /// The 9 standard heritages whose UI actually shows clothing controls + /// AND whose default gear resolves with zero ClothingBaseEffects + /// gaps (measured, not the full "clothing UI shown" set — Undead is + /// ALSO clothing-UI-shown but is the one real gap, asserted separately + /// below). Aluvian/Gharu'ndim/Sho/Viamontian/Shadowbound/Tumerok/Lugian/ + /// Empyrean/Penumbraen = every heritage id 1-10 except Gearknight (6). + /// + private static readonly uint[] StandardZeroGapHeritageIds = [1u, 2u, 3u, 4u, 5u, TumerokId, 8u, 9u, 10u]; + + /// + /// Measured (installed EoR dat, both genders, identical order): Undead's + /// default headgear/trousers/shirt/footwear choices' clothing tables, in + /// the factory's own Headgear→Trousers→Shirt→Footwear composition order. + /// ALL FOUR slots miss — not "headgear/trousers/footwear" (a three-slot + /// undercount an earlier draft of this row stated in error). + /// + private static readonly uint[] UndeadMeasuredMissingClothingTableIds = + [0x10000009u, 0x100000F9u, 0x10000001u, 0x10000007u]; + private static string? ResolveDatDir() { string? fromEnv = Environment.GetEnvironmentVariable("ACDREAM_DAT_DIR"); @@ -55,8 +88,8 @@ public sealed class ChargenAppearanceCatalogInstalledDatTests var catalog = new ChargenAppearanceCatalog(adapter); int composed = 0; - int absentBaseEffectTotal = 0; var missingSummaries = new List(); + var baseEffectGapFailures = new List(); foreach (ChargenHeritageOptions heritage in options.HeritagesById.Values) { @@ -79,23 +112,134 @@ public sealed class ChargenAppearanceCatalogInstalledDatTests + $"missingClothingTables=[{string.Join(",", result.MissingClothingTableIds.Select(id => $"0x{id:X8}"))}]"); } - absentBaseEffectTotal += result.ClothingTablesMissingBaseEffectForSetup.Count; - if (result.ClothingTablesMissingBaseEffectForSetup.Count > 0) + _out.WriteLine( + $"heritage={heritage.Name} (0x{heritage.HeritageId:X}) gender={genderKey} setup=0x{result.SetupId:X8}: " + + $"{result.ClothingTablesMissingBaseEffectForSetup.Count} clothing table(s) with no " + + "ClothingBaseEffects entry for this body setup " + + $"[{string.Join(",", result.ClothingTablesMissingBaseEffectForSetup.Select(id => $"0x{id:X8}"))}]"); + + // TS-82's pinned measurement — real assertions, not WriteLine-only. + if (StandardZeroGapHeritageIds.Contains(heritage.HeritageId)) { - _out.WriteLine( - $"heritage={heritage.Name} gender={genderKey} setup=0x{result.SetupId:X8}: " - + $"{result.ClothingTablesMissingBaseEffectForSetup.Count} clothing table(s) with no " - + "ClothingBaseEffects entry for this body setup " - + $"[{string.Join(",", result.ClothingTablesMissingBaseEffectForSetup.Select(id => $"0x{id:X8}"))}]"); + if (result.ClothingTablesMissingBaseEffectForSetup.Count != 0) + { + baseEffectGapFailures.Add( + $"heritage={heritage.Name} gender={genderKey}: expected ZERO ClothingBaseEffects " + + $"gaps (a standard heritage with clothing UI shown), measured " + + $"{result.ClothingTablesMissingBaseEffectForSetup.Count}: " + + $"[{string.Join(",", result.ClothingTablesMissingBaseEffectForSetup.Select(id => $"0x{id:X8}"))}]"); + } + } + else if (heritage.HeritageId == UndeadId) + { + if (!result.ClothingTablesMissingBaseEffectForSetup.SequenceEqual(UndeadMeasuredMissingClothingTableIds)) + { + baseEffectGapFailures.Add( + $"heritage=Undead gender={genderKey}: expected EXACTLY " + + $"[{string.Join(",", UndeadMeasuredMissingClothingTableIds.Select(id => $"0x{id:X8}"))}], measured " + + $"[{string.Join(",", result.ClothingTablesMissingBaseEffectForSetup.Select(id => $"0x{id:X8}"))}]"); + } + } + // Gearknight/Olthoi/OlthoiAcid: retail hides the clothing UI + // entirely for these three (gmCGAppearancePage::Update + // @0x0047E8F0's SetVisible(0) branches), so a real chargen + // selection never reaches this composer's clothing slots for + // them — no pinned expectation either way, WriteLine above + // is diagnostic only. + } + } + + _out.WriteLine($"composed {composed} heritage/gender selections."); + Assert.True( + missingSummaries.Count == 0, + "Missing dat ids found:\n" + string.Join('\n', missingSummaries)); + Assert.True( + baseEffectGapFailures.Count == 0, + "TS-82 measurement drifted from its pinned expectation:\n" + string.Join('\n', baseEffectGapFailures)); + Assert.True(composed >= 13, $"Expected at least 13 heritage/gender combinations, composed {composed}."); + } + + /// + /// CC6a review fix round F1: retail's Setup-id "unset" sentinel is + /// INVALID_DID (0xFFFFFFFF), not 0 + /// (CharGenState::GetSetupID @ 0x005C5B22). Sweeps EVERY hair + /// style of all 26 heritage/gender combinations and asserts the composed + /// SetupId always resolves to a REAL installed Setup dat entry — proving + /// neither sentinel value, wherever a hair style's AlternateSetup + /// field happens to store one, ever reaches Get<Setup> as a + /// literal id. + /// + [Fact] + public void EveryHairStyleOfEveryHeritageGender_ComposesToARealInstalledSetupId() + { + string? datDir = ResolveDatDir(); + if (datDir is null) + { + _out.WriteLine("SKIP: installed retail DAT directory is unavailable."); + return; + } + + using var dats = new DatCollection(datDir, DatAccessType.Read); + using var adapter = new DatCollectionAdapter(dats); + + ChargenOptions options = ChargenTableReader.Load(adapter); + Assert.NotEmpty(options.HeritagesById); + var catalog = new ChargenAppearanceCatalog(adapter); + + int sweptHairStyles = 0; + var unresolvedSetups = new List(); + + foreach (ChargenHeritageOptions heritage in options.HeritagesById.Values) + { + foreach ((int genderKey, ChargenGenderOptions gender) in heritage.GendersByKey) + { + for (uint hairStyleIndex = 0; hairStyleIndex < (uint)gender.HairStyles.Count; hairStyleIndex++) + { + ChargenAppearanceSelection selection = ChargenAppearanceSelection.Default with + { + HairStyle = hairStyleIndex, + SkinShade = 0.5, + }; + + bool ok = ChargenAppearanceFactory.TryCompose( + options, heritage.HeritageId, genderKey, selection, + catalog, catalog, out ChargenAppearanceResult result); + Assert.True(ok); + sweptHairStyles++; + + if (adapter.Get(result.SetupId) is null) + { + unresolvedSetups.Add( + $"heritage={heritage.Name} gender={genderKey} hairStyle={hairStyleIndex}: " + + $"composed SetupId=0x{result.SetupId:X8} does not resolve to an installed Setup"); + } + } + + // Every gender is swept even with zero hair styles (still + // exercises the "no hair style selected" default-setup path). + if (gender.HairStyles.Count == 0) + { + bool ok = ChargenAppearanceFactory.TryCompose( + options, heritage.HeritageId, genderKey, + ChargenAppearanceSelection.Default with { SkinShade = 0.5 }, + catalog, catalog, out ChargenAppearanceResult result); + Assert.True(ok); + sweptHairStyles++; + if (adapter.Get(result.SetupId) is null) + { + unresolvedSetups.Add( + $"heritage={heritage.Name} gender={genderKey} (no hair styles): " + + $"composed SetupId=0x{result.SetupId:X8} does not resolve to an installed Setup"); + } } } } - _out.WriteLine($"composed {composed} heritage/gender selections; {absentBaseEffectTotal} absent-base-effect slots total."); + _out.WriteLine($"swept {sweptHairStyles} hair-style/no-hair-style selections across 26 heritage/gender combinations."); Assert.True( - missingSummaries.Count == 0, - "Missing dat ids found:\n" + string.Join('\n', missingSummaries)); - Assert.True(composed >= 13, $"Expected at least 13 heritage/gender combinations, composed {composed}."); + unresolvedSetups.Count == 0, + "Composed SetupId(s) that don't resolve to a real installed Setup:\n" + string.Join('\n', unresolvedSetups)); + Assert.True(sweptHairStyles > 26, $"Expected more than 26 swept selections (multiple hair styles per gender), got {sweptHairStyles}."); } /// diff --git a/tests/AcDream.Core.Tests/CharGen/ChargenAppearanceFactoryTests.cs b/tests/AcDream.Core.Tests/CharGen/ChargenAppearanceFactoryTests.cs index b405a70f..8f4ffa7b 100644 --- a/tests/AcDream.Core.Tests/CharGen/ChargenAppearanceFactoryTests.cs +++ b/tests/AcDream.Core.Tests/CharGen/ChargenAppearanceFactoryTests.cs @@ -259,6 +259,49 @@ public sealed class ChargenAppearanceFactoryTests Assert.Equal(ChargenAppearanceFactory.HumanSetupId, result.SetupId); } + /// + /// CC6a review fix round F1: retail's "unset" sentinel for a Setup id is + /// INVALID_DID (0xFFFFFFFF — CharGenState::GetSetupID @ + /// 0x005C5B22), not 0. A hair style whose AlternateSetup field + /// stores 0xFFFFFFFF must NOT be adopted as the body Setup id — before + /// this fix the factory would hand 0xFFFFFFFF straight to a caller's + /// Get<Setup>, which nulls, and the whole preview build + /// would fail silently. + /// + [Fact] + public void TryCompose_HairStyleAlternateSetupIsInvalidDid_IsTreatedAsUnsetNotAdopted() + { + ChargenOptions options = MakeOptions(MakeGender(alternateHairSetup: 0xFFFFFFFFu)); + var (pal, clothing) = MakeSources(); + var selection = ChargenAppearanceSelection.Default with { HairStyle = 0u }; + + ChargenAppearanceFactory.TryCompose( + options, HeritageId, GenderKey, selection, pal, clothing, out ChargenAppearanceResult result); + + Assert.Equal(BodySetupId, result.SetupId); // gender.SetupId, NOT the INVALID_DID sentinel. + } + + /// + /// Companion to : + /// the resolved Setup id can ALSO be stuck at INVALID_DID (rather than 0) + /// when the gender's own SetupId dat field happens to be + /// 0xFFFFFFFF — the fallback to + /// must catch that case too. + /// + [Fact] + public void TryCompose_GenderSetupIdIsInvalidDid_FallsBackToHumanSetupId() + { + ChargenGenderOptions gender = MakeGender() with { SetupId = 0xFFFFFFFFu }; + ChargenOptions options = MakeOptions(gender); + var (pal, clothing) = MakeSources(bodySetupId: 0xFFFFFFFFu); + + ChargenAppearanceFactory.TryCompose( + options, HeritageId, GenderKey, ChargenAppearanceSelection.Default, + pal, clothing, out ChargenAppearanceResult result); + + Assert.Equal(ChargenAppearanceFactory.HumanSetupId, result.SetupId); + } + [Fact] public void TryCompose_EyeStripSelected_UsesNonBaldObjDesc_WhenHairStyleIsNotBald() { @@ -368,6 +411,133 @@ public sealed class ChargenAppearanceFactoryTests Assert.DoesNotContain(result.ObjDesc.SubPalettes, sp => sp.Offset == 10 && sp.NumColors == 2); } + /// + /// CC6a review fix round F8: retail's inner subpalette loop + /// (ClothingTable::BuildObjDesc ~0x005A7B24-0x005A7BD3) returns 0 + /// IMMEDIATELY when a PalSet read fails for one choice (~0x005A7B32), + /// aborting every REMAINING choice in that garment's palette template — + /// not merely skipping the failed one and continuing. A two-choice + /// template with the FIRST choice's PalSet missing must therefore emit + /// NEITHER choice's subpalette, even though the second choice's own + /// PalSet is present and would resolve fine on its own. + /// + [Fact] + public void TryCompose_PalSetMissingMidLoop_AbortsRemainingChoicesInThatGarment() + { + const uint missingPalSetId = 0x0F00_00AAu; + const uint presentPalSetId = 0x0F00_00BBu; + + var firstChoice = new ChargenClothingSubPaletteChoice( + missingPalSetId, [new ChargenClothingSubPaletteRange(80u, 16u)]); + var secondChoice = new ChargenClothingSubPaletteChoice( + presentPalSetId, [new ChargenClothingSubPaletteRange(160u, 8u)]); + var baseEffects = new Dictionary + { + [BodySetupId] = ChargenClothingBaseEffect.Empty, + }; + var templates = new Dictionary + { + [7u] = new ChargenClothingPaletteTemplate([firstChoice, secondChoice]), + }; + var table = new ChargenClothingTable(baseEffects, templates); + + ChargenOptions options = MakeOptions(MakeGender()); + var (pal, clothing) = MakeSources(); + clothing.Add(HeadgearClothingTableId, table); // override the shared fixture's single-choice table. + pal.Add(presentPalSetId, 0x0400_0055u); // deliberately NOT adding missingPalSetId. + + var selection = ChargenAppearanceSelection.Default with + { + HeadgearStyle = 0u, + HeadgearColor = 0u, + HeadgearShade = 0.0, + }; + + bool ok = ChargenAppearanceFactory.TryCompose( + options, HeritageId, GenderKey, selection, pal, clothing, out ChargenAppearanceResult result); + + Assert.True(ok); + Assert.Contains(missingPalSetId, result.MissingPalSetIds); + // Real range (160, 8) would pack to (20, 1) if the second choice were + // (incorrectly) still applied after the first choice's miss. + Assert.DoesNotContain(result.ObjDesc.SubPalettes, sp => sp.Offset == 20 && sp.NumColors == 1); + // Nothing from EITHER choice's own range landed. + Assert.DoesNotContain(result.ObjDesc.SubPalettes, sp => sp.Offset == 10 && sp.NumColors == 2); + } + + /// + /// CC6a review fix round F10: a real dat NumColors of exactly + /// 2048 (256*8) is retail's own "whole palette" value spelled out in + /// real units — it packs to the byte 0 sentinel + /// ('s documented + /// "Length=0 means entire palette") EXPLICITLY, not via an unchecked + /// narrowing coincidence. + /// + [Fact] + public void TryCompose_ClothingRangeNumColorsIsWholePaletteSentinel_PacksToZeroExplicitly() + { + var choice = new ChargenClothingSubPaletteChoice( + 0x0F00_0003u, [new ChargenClothingSubPaletteRange(0u, 2048u)]); + var baseEffects = new Dictionary + { + [BodySetupId] = ChargenClothingBaseEffect.Empty, + }; + var table = new ChargenClothingTable( + baseEffects, + new Dictionary { [7u] = new([choice]) }); + + ChargenOptions options = MakeOptions(MakeGender()); + var (pal, clothing) = MakeSources(); + clothing.Add(HeadgearClothingTableId, table); + + var selection = ChargenAppearanceSelection.Default with + { + HeadgearStyle = 0u, + HeadgearColor = 0u, + HeadgearShade = 0.0, + }; + + ChargenAppearanceFactory.TryCompose( + options, HeritageId, GenderKey, selection, pal, clothing, out ChargenAppearanceResult result); + + Assert.Contains(result.ObjDesc.SubPalettes, sp => sp.Offset == 0 && sp.NumColors == 0); + } + + /// + /// CC6a review fix round F10: a shape the packed *8 byte convention + /// cannot represent losslessly (not a multiple of 8, and not the 2048 + /// whole-palette sentinel) must THROW rather than silently truncate via + /// an unchecked (byte) cast. + /// + [Fact] + public void TryCompose_ClothingRangeDoesNotFitThePackedByteConvention_Throws() + { + var choice = new ChargenClothingSubPaletteChoice( + 0x0F00_0003u, [new ChargenClothingSubPaletteRange(0u, 2041u)]); // not a multiple of 8, not 2048. + var baseEffects = new Dictionary + { + [BodySetupId] = ChargenClothingBaseEffect.Empty, + }; + var table = new ChargenClothingTable( + baseEffects, + new Dictionary { [7u] = new([choice]) }); + + ChargenOptions options = MakeOptions(MakeGender()); + var (pal, clothing) = MakeSources(); + clothing.Add(HeadgearClothingTableId, table); + + var selection = ChargenAppearanceSelection.Default with + { + HeadgearStyle = 0u, + HeadgearColor = 0u, + HeadgearShade = 0.0, + }; + + Assert.Throws(() => + ChargenAppearanceFactory.TryCompose( + options, HeritageId, GenderKey, selection, pal, clothing, out _)); + } + [Fact] public void TryCompose_UnknownClothingTableId_IsRecordedAsMissingAndSkipped() {