acdream/docs/research/2026-05-13-b4c-shipped-handoff.md
Erik 8bb81db659 docs(B.4c): correct handoff fabrications surfaced by final review
Opus final review of B.4c flagged that Task 4's handoff doc invented
implementation details that don't exist in the code:

1. IsDoorSpawn claimed to check "spawn.WeenieObj.WeenieType == 8 OR
   IsDoorName(spawn.Name)" — the actual code is just IsDoorName(spawn.Name)
   delegating to "name == "Door"". No WeenieType lookup exists.

2. A "_doorSequencers" per-door dict was referenced in three places — that
   dict doesn't exist. The actual code reuses the existing
   _animatedEntities[entity.Id] dict (same one that holds creatures + the
   player), with Animation = null! per the existing pattern at line 7885.

3. The UM dispatch path was described as a new B.4c-added branch with
   pseudocode — that's wrong. B.4c does NOT add a new dispatch path;
   OnLiveMotionUpdated's existing TryGetValue against _animatedEntities
   handles doors automatically once Task 1's spawn-time branch registers
   them. The only UM-dispatch B.4c contribution is the [door-cycle]
   diagnostic line, gated on IsDoorName.

Corrects sections "At world load (spawn time)", "When the door opens",
"Per-frame mesh rebuild", and "Door types covered" to reflect the actual
shipped code. cmd→motion mapping (cmd=0x000C → open, cmd=0x000B → close)
left as-is — it was correct.

No code change. Verified by re-reading GameWindow.cs IsDoorSpawn /
IsDoorName helpers, the Task 1 spawn-time branch body, and the
TickAnimations sequencer dispatch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-14 08:03:45 +02:00

346 lines
16 KiB
Markdown

# Phase B.4c shipped — handoff (visual-verified 2026-05-13)
**Date:** 2026-05-13.
**Branch:** `claude/phase-b4c-door-anim` (ready to merge to main; do NOT merge here — controller handles that after code review).
**Predecessors:**
- [docs/research/2026-05-13-b4b-shipped-handoff.md](2026-05-13-b4b-shipped-handoff.md) — B.4b shipped handoff; interaction was the upstream dependency (Use message, SetState handling, collision exemption, double-click detection — all shipped there).
- [docs/superpowers/specs/2026-05-13-phase-b4c-design.md](../superpowers/specs/2026-05-13-phase-b4c-design.md) — B.4c design spec.
- [docs/superpowers/plans/2026-05-13-phase-b4c-plan.md](../superpowers/plans/2026-05-13-phase-b4c-plan.md) — B.4c implementation plan (4 tasks).
---
## TL;DR
Phase B.4c **shipped end-to-end and is visual-verified 2026-05-13.** The M1
demo target *"open the inn door"* now has **full visual feedback** — the door
swings open when double-clicked and swings closed again when ACE toggles it
back. 4 implementation commits implement and fix door-specific spawn-time
`AnimationSequencer` registration + `UpdateMotion` routing + stance-value
correctness.
The plan estimated "2 tasks, door spawn-time registration + UM diagnostic."
Visual testing surfaced **two bonus discoveries** beyond the plan:
1. The plan's `NonCombatStance` constant was wrong: `0x80000001` (from
creature motion table conventions) should be `0x8000003D` (from AC's
`MotionStance.NonCombat = 0x0000003D`). Wrong constant → wrong
`HasCycle` lookup → `SetCycle` never fires → sequencer empty →
per-frame part rebuild collapses to entity origin → doors render halfway
underground.
2. The `AnimationSequencer`'s link→cycle boundary transition produces a
brief one-frame flash through the prior pose at the end of the door-swing
animation. Not B.4c-specific — it is the sequencer's general link+cycle
queue mechanics. Deferred as issue #61.
Issue #58 (door swing animation) is closed. Issues #61 + #62 (cycle-boundary
flash; PARTSDIAG null-guard) are filed as M1-deferred polish.
---
## What shipped on this branch
| # | Commit | Subject | Task |
|---|---|---|---|
| 1 | `9053860` | `feat(B.4c): door spawn-time AnimationSequencer with state-seeded initial cycle` | Task 1 |
| 2 | `b89f004` | `feat(B.4c): [door-cycle] diagnostic in OnLiveMotionUpdated` | Task 2 |
| 3 | `8a9b15e` | `refactor(B.4c): share IsDoorName predicate + durable comment + use UM locals` | Task 2 review |
| 4 | `454d88e` | `fix(B.4c): correct NonCombat stance value (0x3D, not 0x01) + read spawn.MotionState` | Bonus: stance fix |
Plus plan/spec commits earlier in the branch session:
- `b4f131e` — B.4c design spec.
- `6ae38f7` — B.4c implementation plan (4 tasks).
**Build:** clean. **Tests:** existing test suite passes; no new unit tests added
(the door-cycle registration path runs in-process with a live GameWindow; pure
unit tests would require a MotionTable + AnimationSequencer integration harness).
---
## What the code does end-to-end
When the world loads, any entity whose name contains "Door" (checked via the
shared `GameWindow.IsDoorName(string)` helper, committed as part of Task 2
review) is registered in the **door-animation side-track** at spawn time. This
happens inside `GameWindow.OnLiveEntitySpawnedLocked`, which branches on
`IsDoorSpawn(spawn)` before reaching the standard creature/player paths.
### At world load (spawn time)
1. `IsDoorSpawn(spawn)` — delegates to `IsDoorName(spawn.Name)`, which
returns `name == "Door"`. Detection by server-sent name string only.
Cheap, exact, no WeenieType lookup. If a future ACE localizes "Door"
or sends a different name, those entities silently won't animate —
acceptable per B.4c's "doors only at English Holtburg" scope.
2. **Initial state seed** — the door's `PhysicsState` from `spawn` carries the
open/closed bit. The code reads `spawn.PhysicsState` (or
`spawn.MotionState?.Stance` as a fallback for unusual doors with explicit
stance data) to determine whether to seed the sequencer with the `Off`
(closed) or `On` (open) cycle.
3. **AnimationSequencer registration** — a fresh `AnimationSequencer` is
created for the door entity's `MotionTableId` (from `spawn`). Then:
```csharp
var style = 0x80000000u | (uint)MotionStance.NonCombat; // = 0x8000003D
var cycleCmd = isOpen ? MotionCommand.On : MotionCommand.Off;
sequencer.SetCycle(style, (uint)cycleCmd, speed: 0f);
```
The fully-initialized `AnimatedEntity` (with the seeded `Sequencer`) is
registered into the existing `_animatedEntities` dict keyed by `entity.Id`
— same dict that holds creatures and the player. `Animation = null!`
(the null-forgiving suppression matches an existing pattern at
`GameWindow.cs:7885` for sequencer-driven entities where the legacy
`Animation` field is unused). At the first per-frame `Advance(dt)`
call from `TickAnimations`, the sequencer produces the correct
rest-pose frames for the door's current state.
4. **Log evidence at spawn:**
```
[door-anim] registered guid=0x7A9B403A entityId=0x000F4291 mtable=0x09000202 initialStyle=0x8000003D initialCycle=0x4000000C
```
`0x4000000C` = `MotionCommand.Off` with the upper flag bits — the door is
closed at spawn, matching the initial world state.
### When the door opens (UpdateMotion arrives)
ACE broadcasts `UpdateMotion (0xF74D)` with `stance=0x003D` (NonCombat) and
wire `cmd=0x000C` (which `MotionCommandResolver.ReconstructFullCommand`
maps to full motion `0x4000000B` = `MotionCommand.On` = door open).
B.4c does NOT add a new dispatch path here — the existing
`OnLiveMotionUpdated` handler already routes via the `_animatedEntities`
dict + per-entity `Sequencer`, the same code path creatures use. The
only B.4c contribution at UM dispatch is the new `[door-cycle]`
diagnostic gated on `IsDoorName(doorInfo.Name)`. Before B.4c, doors
silently dropped at the `_animatedEntities.TryGetValue` check at
`GameWindow.cs:3036` because doors weren't registered; B.4c's Task 1
spawn-time branch fixed that.
The sequencer transitions from the `Off` cycle (static closed pose) through
the door-swing link animation to the `On` cycle (static open pose).
**Log evidence:**
```
UM guid=0x7A9B403A mt=0x00 stance=0x003D cmd=0x000C spd=0.00 | seq now style=0x8000003D motion=0x4000000B
[door-cycle] guid=0x7A9B403A stance=0x003D cmd=0x000C
```
The `[door-cycle]` line is the new B.4c diagnostic (gated on
`ACDREAM_PROBE_BUILDING=1`). The `seq now motion=0x4000000B` shows the
sequencer's current motion state after the `SetCycle` call.
### SetState chain (from B.4b + L.2g, unchanged)
Simultaneously with `UpdateMotion`, ACE also sends `SetState (0xF74B)`:
```
[setstate] guid=0x7A9B... state=0x0001000C
```
This is the B.4b / L.2g chain: `ShadowObjectRegistry.UpdatePhysicsState` flips
the door's cached state, `CollisionExemption.ShouldSkip` exempts on ETHEREAL-alone,
and the player can walk through. B.4c is additive — it only adds the animation
layer; it does not touch the collision path.
### When the door closes
ACE toggles on the next Use: `UpdateMotion` with `cmd=0x000B` (Off = close).
The sequencer transitions from the `On` cycle (open pose) through the door-swing
link animation (reversed) to the `Off` cycle (closed pose).
**Log evidence:**
```
UM guid=0x7A9B403A mt=... cmd=0x000B ... motion=0x4000000C
[door-cycle] guid=0x7A9B... cmd=0x000B
[setstate] guid=0x7A9B... state=0x00010008
```
### Per-frame mesh rebuild
The door sequencer integrates into `GameWindow.TickAnimations` via the same
`_animatedEntities` dict that holds creatures. Each frame, `ae.Sequencer.Advance(dt)`
is called and the resulting per-part transforms drive the same `MeshRefs` rebuild
that creature entities use (sequencer branch at `GameWindow.cs:7497`; doors
never enter the legacy slerp `else` branch). This is the reason the stance-value
bug produced underground doors: with the wrong style key (`0x80000001`)
`HasCycle` returned false, the sequencer was empty at spawn, `Advance` returned
identity frames, and the per-frame part-matrix rebuild received `Vector3.Zero /
Quaternion.Identity` for every part — collapsing them all to the entity origin.
---
## The two bonus discoveries
### 1. NonCombatStance constant was wrong: 0x01 vs 0x3D (`454d88e`) — THE render blocker
**Root cause:** The B.4c design spec specified the initial-cycle style key as:
```csharp
uint style = 0x80000000u | (uint)MotionStance.NonCombat; // spec said 0x80000001
```
The spec's comment was wrong. `MotionStance.NonCombat` in acdream (and retail)
is `0x0000003D`, not `0x00000001`. The value `0x01` is a creature-specific
variant. The style key for the door's cycle lookup must be `0x8000003D`.
With the wrong style key:
- `sequencer.HasCycle(0x80000001, MotionCommand.Off)` → false.
- `SetCycle(0x80000001, ...)` enqueued a cycle that was never reachable.
- On first `Advance(dt)`, the sequencer returned 0 part-frames.
- The per-frame mesh rebuild at `GameWindow.cs:7691` iterated 0 frames, leaving
every door part at the entity root origin (which is the door's structural
pivot, typically near the hinge). For inn doors this pivot is at roughly
floor level, so all the door's mesh parts collapsed to that single point,
rendering as a thin sliver partway underground.
**Fix:** Corrected the constant. Additionally, added a defensive read of
`spawn.MotionState?.Stance` as the source of the stance value where available,
so unusual doors with explicit motion state (possible in custom ACE content) use
their actual stance rather than the hardcoded NonCombat assumption:
```csharp
var stance = spawn.MotionState?.Stance ?? MotionStance.NonCombat;
uint style = 0x80000000u | (uint)stance;
```
**Verification:** After this fix, the `[door-anim]` log line showed
`initialStyle=0x8000003D` (correct), and doors appeared at the correct floor
level and height at world load.
### 2. AnimationSequencer link→cycle boundary flash (deferred as #61)
**Observed:** User reports "weird flapping at end of animation when it opens.
It is like it flaps back to closed quickly then open. Like really quickly."
Both open and close animations exhibit this flash.
**Root cause hypothesis:** `AnimationSequencer.SetCycle` enqueues a transition
link (the actual swing animation) followed by the target cycle (the door's
rest pose — likely a single-frame static "open" or "closed" pose). At the link→
cycle boundary, the sequencer evaluates the cycle's frame 0 before the cycle
settles into its natural rest position. If the link's last frame and the
cycle's frame 0 don't match exactly (which is common for one-shot door motions
versus the continuous idle cycles the sequencer was designed for), the renderer
sees one frame of the "wrong" pose at the link boundary.
**Why not B.4c-specific:** This is the sequencer's general link+cycle queue
boundary semantics. Any entity that uses a one-shot `SetCycle` transition
(rather than a continuous idle cycle) will exhibit this if the link/cycle
boundary frames diverge. The door case just makes it visible because the
swing duration is short (1-2 seconds) and the user is watching closely.
**Deferred:** Filed as issue #61. Workaround: the flash is brief (~1 frame,
~16ms at 60 FPS) and does not affect the door's usability. M1 is met without
this fix.
---
## Open notes / follow-ups
### #61 — AnimationSequencer link→cycle frame-0 flash (filed this session)
See Bonus discovery #2 above. Deferred as M1-deferred polish. Low severity.
Acceptance: door swing animations play cleanly with no intermediate closed/open
pose flash at the link→cycle transition.
### #62 — PARTSDIAG null-guard for sequencer-driven entities (filed this session)
The PARTSDIAG block at `GameWindow.cs:7657` reads `ae.Animation.PartFrames`
without a null-guard. B.4c introduced `Animation = null!` for sequencer-driven
door entities. Today this is safe (doors never enter `_remoteDeadReckon` because
ACE never sends UpdatePosition for them). Deferred as low-severity latent crash.
One-line fix when addressed.
### Chests, levers, traps
The `IsDoorName` / `IsDoorSpawn` predicate correctly gates on door entities only.
Other interactable non-creature entities (chests, levers, traps) will still
silently drop their `UpdateMotion` commands — they are not covered by B.4c and
no issue has been filed for them yet. When those animations become relevant
(M2/M3 inventory + dungeon content), the same spawn-time registration pattern
can be extended: broaden the detection predicate beyond `name == "Door"` and
register additional entity types in the existing `_animatedEntities` dict via
the same sibling branch.
### Door toggle behavior
Unchanged from B.4b. ACE doors toggle on each Use: first double-click opens,
subsequent double-click closes. Both transitions now play the correct swing
animation (open swing on open, close swing on close).
---
## Next session
**M1 demo progress as of this branch:**
- "Walk through Holtburg without getting stuck" — Phase L.2 in progress (outdoor collision works; `CBuildingObj` interior still deferred to L.2d).
- "Open the inn door" — **DONE with full visual feedback** (B.4b interaction + B.4c animation, this branch). Door swings open AND closed.
- "Click an NPC" — pick + Use wiring exists (from B.4b); depends on ACE NPC handler responding to Use correctly.
- "Pick up an item" — `BuildPickUp` + F-key wiring not yet in `OnInputAction`. Post-B.4b/B.4c deferred.
**Recommended next steps (in M1 critical-path order):**
1. **"Click an NPC" verification spike** — B.4b's WorldPicker + Use messaging
is already wired. The question is whether ACE NPCs respond to Use and what
they broadcast back. A quick spike: stand near an NPC in Holtburg,
double-click, check what ACE sends back. If ACE sends recognizable response
messages, wire them; if it is silent, investigate ACE's NPC handler
configuration for testaccount.
2. **Phase B.5 — Ground item pickup (F key)** — `SelectionPickUp` input action
+ F-key binding exist but `OnInputAction` has no case. `BuildUse` is the
same wire format as `BuildPickUp`. Adding the `SelectionPickUp` case to
the switch and routing to `InteractRequests.BuildPickUp` is a one-commit
addition.
3. **Triage chronic open-issue list** — #2 (lightning), #4 (sky horizon-glow),
#28 (aurora), #29 (cloud thinness), #37 (humanoid coat), #41
(remote-motion blips) have been open since April/early-May. Link each to
a future phase or downgrade. ~1 hour.
4. **#61 fix (cycle-boundary flash)** — low-severity M1 polish. If the user
finds the flash distracting during the M1 demo record, address before
milestone wrap; otherwise defer to M2 animation quality pass.
---
## Reproducibility
Same launch recipe as B.4b. For reproducing the visual test:
```powershell
$env:ACDREAM_DAT_DIR = "$env:USERPROFILE\Documents\Asheron's Call"
$env:ACDREAM_LIVE = "1"
$env:ACDREAM_TEST_HOST = "127.0.0.1"
$env:ACDREAM_TEST_PORT = "9000"
$env:ACDREAM_TEST_USER = "testaccount"
$env:ACDREAM_TEST_PASS = "testpassword"
$env:ACDREAM_DEVTOOLS = "1"
$env:ACDREAM_PROBE_BUILDING = "1"
$env:ACDREAM_PROBE_RESOLVE = "1"
dotnet run --project src\AcDream.App\AcDream.App.csproj -c Debug 2>&1 |
Tee-Object -FilePath "launch-b4c.log"
```
Walk to the Holtburg inn doorway. Watch the `[door-anim]` lines appear in the
log as each door entity spawns (verifies correct style=0x8000003D and initial
cycle). Double-left-click a closed door. Watch the swing animation. Walk
through. Wait ~30s (ACE auto-close). Watch the close animation.
After closing the client, grep for:
```powershell
Select-String -Path launch-b4c.log -Pattern "door-anim|door-cycle|setstate"
```
Expected:
- `[door-anim] registered guid=... initialStyle=0x8000003D initialCycle=0x4000000C` — correct style + Off initial cycle for each closed door.
- `[door-cycle] guid=... stance=0x003D cmd=0x000C` — open UpdateMotion processed.
- `[setstate] guid=... state=0x0001000C` — ACE collision-flip processed (from B.4b / L.2g).
- `[door-cycle] guid=... cmd=0x000B` — close UpdateMotion processed.
- `[setstate] guid=... state=0x00010008` — ACE close collision-flip processed.
---
## Worktree state at handoff
- Branch `claude/phase-b4c-door-anim`.
- 6 commits ahead of `3e08e10` (the B.4b+L.2g merge from this morning):
2 docs/spec/plan commits + 4 implementation commits.
- Controller should run a code review, then merge to main.
- Do NOT rebase or squash — each commit tells a diagnostic story that the
next phase's debugging may need.