Fixes every finding from the dual-lens review of 34c6fceab0 (architectural
PASS-with-items, retail-fidelity FAIL). Re-derived every decomp citation
against docs/research/named-retail/acclient_2013_pseudo_c.txt directly
rather than trusting the reviewer's transcription.
Wrap/normalize semantics (F1): CycleIndex's decrement-from-Unset landed on
0; the decomp's shared decrement tail (label_47f065/label_47f6d9, the same
switch the headgear ring was ported from) computes new=cur-1=-2 on the raw
signed int32, which wraps to count-1 — matching headgear's own ring shape.
Also ports the spin body-click normalize-and-write-back retail's cases
0xa5-0xae all share (NormalizeChoiceOnSelect), which acdream had dropped
entirely. Flips the one test that pinned the wrong expectation and adds
select-zone coverage no prior test isolated.
Heritage gate (F3): Update's Gearknight/Olthoi/OlthoiAcid branches reset
SetChoice(FACE)/SetSelection(HAIR) unconditionally, not only when Clothes
was showing — a conditional gate stranded Nose/Mouth as the current part
under a Face-tab session.
Doc corrections propagated everywhere they repeated (F4, F5, F6, plus the
plan doc's own CC6b-MOUNT ledger row for F1/F3): the gmBarberUI heading
citation conflated PostInit with InitializePage; Random's Appearance
disable was mislabeled a placeholder when it's really AP-212's unported
RandomizeAppearance/RandomizeClothing gap; the master-page doc still called
the Appearance page content-inert after this campaign made it real.
Visual substitutions widened (F2): AP-215 named only two of the Appearance
page's swatch/spin substitutions. Ports the two cheap ones directly —
current-part highlight via SetSelection's SetState(1)/SetState(6), routed
through the existing UiButtonStateMachine.Normal/Highlight ids and
IUiDatStateful.TrySetRetailState seam (installed-DAT-confirmed
ToggleBehavior=true on all nine spins); the shade scrollbar's SetVisible(0)
for Eyes vs acdream's Enabled=false. Files the other five (DoColorSpots,
the inert GradCircle, spin-caption/heritage-caption loss, the Skin-spin
MoveTo reposition, the Gearknight-boundary randomize calls) as new register
rows AP-216..AP-220 and corrects the plan doc's false claim that AP-215
already named the GradCircle.
Unlocked DAT read (F7, BLOCKER): ChargenPreviewController.Rebuild called
ChargenAppearanceFactory.TryCompose outside _datLock while the very next
line correctly locked TryBuildAnimated — CC6a's own F4 class of bug,
reintroduced at this catalog's first production call site. Wrapped in the
same lock; documented the invariant on ChargenAppearanceCatalog itself.
One-shot preview mount (F8): LivePresentationComposition reads
ChargenPreviewViewportWidget once, but its underlying mount
(CharacterCreationUiMountCoordinator) is explicitly retryable while this
GPU-resource composition pass is not — unlike PaperdollViewportWidget,
which IS eager/non-retryable, so the "mirrors Paperdoll" doc claim was
false. Retrofitting cross-frame retry here would mean restructuring this
composition's one-shot contract for every private viewport (paperdoll,
creature appraisal) and FrameRootComposition's fixed frame-group array —
out of this round's blast radius. Corrected the doc and made the failure
loud (a diagnostic log) instead of silent.
Dispose leak (F9): ChargenPreviewController.Dispose left the preview
WorldEntity referenced by the leased renderer until the renderer's own,
later disposal. Releases it on its own teardown now.
Test-quality items (F10, F11, F13): pinned the spin arrow widths
(47px, both arrows) the 174 zone boundary is derived from, plus a
controller test for the previously-uncovered select zone. Measured the
shade scrollbar's authored orientation instead of assuming it — it is
VERTICAL (33x85) — which is a real production bug: UiScrollbar only routed
scalar-mode mouse events when Horizontal was true, so the shade control
never fired in production. Added OnVerticalScalarEvent/DrawVerticalScalar
mirroring the existing horizontal scalar path. Converted
ChargenPreviewControllerTests from silent-pass [Fact] to the shared
InstalledDatFactAttribute skip-reporting pattern.
Adjudication (F12): AD-101's retirement leaves TryBeginFinish's four local
refusals (NoName/AttributeCreditsUnspent/AlreadyPending/RosterFull) with no
heritage/gender gate — currently latent since Finish stays hard-disabled
this round. Amended the campaign plan's CC5 slice scope to require BOTH a
heritage/gender refusal AND a real RandomizeCharacter port before the
connected user gate opens Finish; noted the interaction on AP-214's own
register row. No CC5 implementation in this commit.
Gates: dotnet build -c Release green across the full solution. App suite
(Release, ACDREAM_PROBE_LIVE_MOUNT=1) 5223/3 skips, Runtime suite
1713/0 — both clean across repeated runs. A full-solution run surfaced
three pre-existing, previously-documented flakes unrelated to this change
(Streaming.LandblockBuildFactoryTests/LandblockPresentationPipelineTests
#402, Core.Net.Tests.NakEmissionTests loss soak) — each confirmed passing
in isolation, consistent with their known full-suite-parallelism-timing
history; none touch any file this commit changes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The page-mount half CC6b-PRE deferred: CharacterCreationAppearancePage
(gender buttons, Face/Clothes sub-tabs, nine spin controls with retail's
decrement/increment/select-as-current-part OnClickAt zones, nine color
swatches, shade scrollbar, zoom/rotate wiring) plus ChargenPreviewController,
which bridges the ChargenPreviewRenderer/ChargenPreviewZoomController
camera-injection gap CC6a/CC6b-PRE left open and mounts as the third private
creature viewport beside paperdoll/creature-appraisal.
Color-wheel scouting (campaign risk item 4): live-DAT probe found every
color-wheel-family id resolves through existing DatWidgetFactory mappings
(Button/Scrollbar/generic fallback) — no new widget type needed.
The @140355 gender-flip-on-init oddity (risk item 5): resolved via decomp
alone — gmCharGenMainUI's own ctor calls CharGenState::RandomizeCharacter
before any page constructs, so retail's chargen screen is never actually
blank on open; the Appearance page's gender-flip code always fires against
a real, randomly-rolled gender. Filed AP-214 (acdream doesn't port
RandomizeCharacter this round, so it opens honestly blank instead) and
AP-215 (two narrow visual substitutions: swatch .Selected highlight vs
retail's separate overlay, ordinal labels vs retail's icon-only spins).
AD-101 retired: the Heritage page's auto-gender-select interim default is
deleted now that the Appearance page's real gender buttons exist. TS-82
narrowed to Summary-only.
Scope addendum: ChargenPreviewRotationController's parameterless-constructor
default changes from 0f to a new RetailDefaultHeadingDegrees=180f constant
(retail's InitializePage override, not the ctor's raw 0) — every real
gmCG3DView owner converges on 180 before its first frame, so a controller
defaulting to 0 was a trap for future consumers.
Runtime 1713/0, Core 4786/1 skip, Content 147/0, App 5220/3 skips (Release,
ACDREAM_PROBE_LIVE_MOUNT=1) — zero failures across two clean full-solution
runs; the one Core.Net.Tests NakEmissionTests flake observed on a third run
is the same pre-existing, previously-documented timing flake (zero files
under src/AcDream.Core.Net/ touched, passes 100% in isolation).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
F1 (BLOCKING, doc-only) — the idle-by-default rationale rested on an unsound
"uninitialized C++ member defaults to 0" argument (heap operator-new memory
is indeterminate, not zero). Verified and replaced with the real evidence:
gmCGAppearancePage::InitializePage @0x0047FDD0 writes an EXPLICIT
this->m_bZoomedIn = 0; at 0x004802C3, immediately after that same function
points the camera at the zoomed-IN per-heritage eye (0x00480286-0x0048029E).
Fixed in all three places: the register's TS-83 retirement clause,
ChargenPreviewAnimator's class doc, ChargenPreviewZoomController.IsZoomedIn's
doc. Recorded the retail quirk this implies: the character starts framed
close-up while not-zoomed-in, so the first Zoom In click (once mounted)
tweens close-eye->close-eye (visually null) while still freezing the
animation — the port reproduces this faithfully.
F2 — ChargenPreviewZoomController and ChargenPreviewAnimator kept
independent _zoomedIn bools synced only via a nullable animator parameter,
risking desync. Retail's m_bZoomedIn is a single field gating both camera
and animation, so the fix makes the animator the sole state owner:
ChargenPreviewZoomController now takes its ChargenPreviewAnimator as a
required constructor dependency, IsZoomedIn reads straight through to it,
and ZoomIn/ZoomOut no longer take a parameter at all — there is no second
bool left to disagree.
F3 — documented the DoRotation counter-clockwise branch's x87-stack
decompiler artifact (BN renders x87_r7_1 = x87_r6_3 at 0x0047CAEB, which
would store delta-degrees instead of the timestamp for CCW only); the port
already stores "now" in both branches, cited against
feedback_bn_decomp_field_names.md.
F4 — ChargenPreviewAnimator.ApplyIdleFrame now double-buffers two
List<MeshRef> instead of allocating fresh every 30fps tick.
F5 — filed docs/ISSUES.md #402 tracking the RetailAnimationCyclePlayback /
LiveEntityAnimationPresenter duplication as an owned post-CC follow-up,
referenced from the new type's own doc.
F6 — reworded the ChargenPreviewEntityBuilder.TryBuild "byte-identical"
claim to result-identical (TryBuildAnimated now also resolves the idle DID
and loads the idle Animation before the wrapper discards them).
F7 — added the missing clockwise >360 clamp test (readable decomp
polarity, unlike F3's CCW artifact).
ALSO — rewrote the CC6b ledger row's m_alternateSetupID MUST-COVER note per
the reviewer's F11 concession: all five write sites belong to gmBarberUI
(the post-creation barber shop), not gmCGAppearancePage, which has no
option-checkbox-equivalent field at all. Added the enclosing-function
citations and an explicit directive that CC6b-mount must NOT build a
crown/no-flame checkbox on the Appearance page.
Tests: ChargenPreviewRotationControllerTests +1 (10 total),
ChargenPreviewZoomControllerTests +2 and every case rewritten for the
required-animator constructor (9 total). Core.Tests 4786/1 skip (unchanged),
Content.Tests 147/0, App.Tests 5152/6 skips (+3) — zero failures in
isolation, full solution Release build green. Two pre-existing flakes
observed across repeated full-solution runs, neither caused by this round
and neither reproducing standalone: Core.Net.Tests' NakEmissionTests loss
soak, and Content.Tests' DecodedTextureCacheTests concurrency race.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Idle animation loop: decomp re-read of gmCGAppearancePage::Update's trailing
StartAnimation/StopAnimation gate (~0x0047EF01-0x0047EF12) plus the ctor
evidence that m_bZoomedIn is a decompiler-elided bool (never explicitly set
away from its zero default, unlike its two sibling bools) establishes that
retail's chargen preview defaults to the idle loop PLAYING, not the frozen
rest pose CC6a shipped as a deliberate simplification (TS-83) — the rest pose
only appears once Zoom In fires. New Core primitive
RetailAnimationCyclePlayback ports CPhysicsObj::set_sequence_animation's
advance-with-wrap + lerp/slerp effect (the same algorithm
LiveEntityAnimationPresenter's legacy NPC-idle branch already carries inline;
not consolidated this round — out of blast radius for a preview-only
feature, noted in the new type's own doc). New ChargenPreviewAnimator drives
the per-tick swap; ChargenPreviewEntityBuilder gained TryBuildAnimated
alongside the byte-behavior-unchanged TryBuild. Olthoi/OlthoiAcid use the
SAME enum key for idle and rest DIDs (decomp-confirmed quirk). TS-83 retired
in the register (§4 count 50->49).
Rotation controller: ChargenPreviewRotationController ports
Rotate/DoRotation (0x0047CB50/0x0047CA80) verbatim — toggle-to-stop,
deltaDegrees = ((now-last)/RotationSecondsPerRevolution)*360, single-pass
+-360 clamp (not a full modulo, matching retail's own tail), the -1.0
invalidation sentinel. Applies to the entity's heading via the existing
MoveToMath.SetHeading port, not the camera, confirming CC6a's own note.
Zoom tween: ChargenPreviewZoomController ports ZoomIn/ZoomOut/
DoZoomAnimation (0x0047CF00/0x0047D050/0x0047C960) — a LINEAR 0.6s tween
(no easing curve in the decomp) between the already-recorded camera eye
profiles, calling into the animator's zoom swap IMMEDIATELY at button-press
time, matching retail's call order exactly.
m_alternateSetupID (research correction): re-reading the decomp
function-by-function found all five m_alternateSetupID write sites —
including the two the CC6a review cited — belong to gmBarberUI (the
post-creation barber shop), not gmCGAppearancePage, which has no
m_pOption1Checkbox-equivalent field and never writes the field. For
character creation the field is always INVALID_DID in retail. TryCompose
still gained a real, decomp-cited alternateSetupIdOverride parameter
(default no-op) implementing gmCG3DView::Update's generic override
precedence, for a future non-chargen consumer.
RetailHeldPose extraction: shared ResolvePoseDid/ComposePartTransform
between RetailPaperdollPoseApplicator and ChargenPreviewEntityBuilder — a
clean mechanical extraction, behavior-identical on the paperdoll side.
Bookkeeping: CC6a ledger row now cites its real commit SHAs (55bfd9ca,
1774d8b2); new CC6b-PRE ledger row records scope done + the page-mount half
still owed.
Tests: RetailAnimationCyclePlaybackTests (10, Core), ChargenAppearanceFactoryTests
(+4), ChargenPreviewRotationControllerTests (9), ChargenPreviewZoomControllerTests
(7), ChargenPreviewAnimatorTests (7, hand-built fixtures), ChargenPreviewEntityBuilderTests
(+5, installed-DAT). Core.Tests 4786/1 skip, Content.Tests 147/0, App.Tests
5149/6 skips — zero failures, full solution Release build green. One
pre-existing, unrelated flake noted: Core.Net.Tests' NakEmissionTests loss
soak failed once in the full-suite run, passed 1/1 isolated.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Erik