docs(B.5): ship handoff + roadmap/CLAUDE update + file #63 #64

Phase B.5 (ground-item pickup, close-range path) shipped and
visual-verified 2026-05-14 at Holtburg. M1 demo target 4/4 ("pick up
an item") met.

New ship-handoff doc captures the 5-commit history including the
post-visual-test PickupEvent (0xF74A) wire-handler fix that closes
the local-despawn gap.

Roadmap and CLAUDE.md updated to reflect the ship + the new follow-up
issues:

  - #63 (MEDIUM) — server-initiated MoveToObject auto-walk not
    honored; blocks double-click pickup + out-of-range F. Filed as
    candidate Phase B.6. holtburger has the reference implementation.
  - #64 (LOW) — local-player pickup animation does not render
    (retail observers see it correctly). Likely a self-echo filter
    dropping UpdateMotion(Pickup) on the local player.

Carry-overs from B.4c (#61 link-cycle flash, #62 PARTSDIAG null-guard)
unchanged.
This commit is contained in:
Erik 2026-05-14 16:23:20 +02:00
parent f7636a9e78
commit d132fcccfb
4 changed files with 364 additions and 17 deletions

View file

@ -0,0 +1,252 @@
# Phase B.5 shipped — handoff (visual-verified 2026-05-14)
**Date:** 2026-05-14.
**Branch:** `claude/phase-b5-pickup` (ready to merge to main; controller handles the merge after this doc lands).
**Predecessors:**
- [docs/research/2026-05-13-b4c-shipped-handoff.md](2026-05-13-b4c-shipped-handoff.md) — B.4c (door swing) shipped immediately before.
- [docs/research/2026-05-13-b5-pickup-handoff.md](2026-05-13-b5-pickup-handoff.md) — fresh-session handoff that scoped this phase.
- [docs/superpowers/plans/2026-05-14-phase-b5-pickup.md](../superpowers/plans/2026-05-14-phase-b5-pickup.md) — implementation plan (2 tasks).
---
## TL;DR
Phase B.5 **shipped end-to-end and is visual-verified 2026-05-14.** The
M1 demo target *"pick up an item"* is met for the close-range path —
single-click a ground item to select, walk within ~0.6 m of it, press
F, and the item is removed from the world and added to the player's
inventory.
The plan budgeted 2 implementation tasks (~50 LOC). Visual testing
surfaced **one wire-handler gap** that became Task 2b: ACE despawns
picked-up items via `GameMessagePickupEvent (0xF74A)`, not the
`GameMessageDeleteObject (0xF747)` we already handled — without that
fix the pickup succeeded server-side but the item kept rendering on
the ground locally. Caught and fixed in the same session.
Two known gaps remain, filed as issues for follow-up:
- **#63 (MEDIUM)** — Server-initiated auto-walk for out-of-range Use /
PickUp not honored. Double-click a ground item from > 0.6 m and the
character partially walks then snaps back; ACE's `MoveToChain` times
out. This is a separate motion-handling phase, not a B.5 regression.
- **#64 (LOW)** — Local-player pickup animation doesn't render
(retail observers see it correctly; local view is silent). Likely a
self-echo filter dropping `UpdateMotion(Pickup)` on the local player.
---
## What shipped on this branch
| # | Commit | Subject | Task |
|---|---|---|---|
| 1 | `e8a20f2` | `feat(B.5): InteractRequests.BuildPickUp — PutItemInContainer 0x0019` | Task 1 |
| 2 | `ced1b85` | `test(B.5): exercise i32 sign-correctness for BuildPickUp.placement` | Task 1 code-review fix |
| 3 | `54d9bb9` | `feat(B.5): SendPickUp helper + F-key SelectionPickUp wiring` | Task 2 |
| 4 | `5c24f6c` | `docs(B.5): implementation plan from writing-plans skill` | Plan doc |
| 5 | `f7636a9` | `fix(B.5): handle PickupEvent 0xF74A so picked-up items despawn locally` | Task 2b (post-visual-test fix) |
Plus the predecessor handoff (`86440ff`) that started the branch.
**Build:** clean.
**Tests:** `dotnet test -c Debug` shows AcDream.Core.Net.Tests 290/290
passing (was 287 at branch start; +3 from Task 2b's PickupEvent tests;
the two BuildPickUp tests landed inside the same project's existing
file). Failure count unchanged at 8 pre-existing baseline in
AcDream.Core.Tests.
---
## What the code does end-to-end
**Outbound (Tasks 1 & 2):**
1. User single-clicks a ground item near `+Acdream`.
`case InputAction.SelectLeft → PickAndStoreSelection(useImmediately: false)`
runs B.4b's `WorldPicker.Pick`, finds the item, sets `_selectedGuid`.
Log: `[B.4b] pick guid=0x… name=…`.
2. User presses F.
`case InputAction.SelectionPickUp → SendPickUp(_selectedGuid)` builds
the wire body via `InteractRequests.BuildPickUp(seq, itemGuid,
_playerServerGuid, placement: 0)` and posts it through
`_liveSession.SendGameAction`. Log: `[B.5] pickup item=… container=… seq=…`.
3. Wire layout (24 bytes): `0xF7B1 envelope | seq | 0x0019 opcode |
itemGuid u32 | containerGuid u32 | placement i32`. Verified against
`references/ACE/Source/ACE.Server/Network/GameAction/Actions/GameActionPutItemInContainer.cs`.
**Inbound (Task 2b — surfaced during visual test):**
4. ACE runs `HandleActionPutItemInContainer`. If the player is within
`WithinUseRadius` (~0.6 m), the close-range branch in
`CreateMoveToChain` skips the auto-walk and runs the pickup chain
directly: server-side `Landblock.RemoveWorldObject(item.Guid,
adjacencyMove: false, fromPickup: true)` → per-player
`Player_Tracking.RemoveTrackedObject(wo, fromPickup: true)`
broadcast `GameMessagePickupEvent (0xF74A)` to all observers.
5. Our `WorldSession.Dispatch` now routes `0xF74A` (in addition to
`0xF747 DeleteObject`) through the shared `EntityDeleted` event,
adapting the `PickupEvent.Parsed` to a `DeleteObject.Parsed` so
`OnLiveEntityDeleted → RemoveLiveEntityByServerGuid` runs unchanged.
The item disappears from the local view.
---
## Wire-handler gap (Task 2b)
ACE distinguishes two despawn opcodes:
- `0xF747 GameMessageDeleteObject` — "object is gone" (timeout / death /
out-of-LOS). Our existing handler.
- `0xF74A GameMessagePickupEvent` — "object was picked up by a player."
Sent by `Player_Tracking.RemoveTrackedObject(wo, fromPickup: true)`.
Both are functionally identical from the client's view (remove the
entity from the world), but only one was handled. Wire format adds
one `u16 objectPositionSequence` field over DeleteObject's layout, so
`PickupEvent.cs` is its own parser; the dispatcher adapts to
`DeleteObject.Parsed` for the downstream consumer.
This is exactly the kind of trap CLAUDE.md's reference-repo discipline
exists to prevent — the handoff spec said "the existing despawn path
removes the ground item from view," which was *almost* true. Took one
visual-verification round-trip to surface, ten minutes to fix with a
clean wire parser + 3 new unit tests.
---
## Visual verification — what was observed
**Test scenario:** ACE dropped a Pink Taper, then a Violet Taper, then
two more tapers near `+Acdream` at Holtburg. Player walked up close,
single-clicked, pressed F. Three pickups completed in the post-fix
log: items `0x80000725`, `0x8000072A`, `0x80000729`.
**Before Task 2b:** Server-side pickup succeeded — `[B.5] pickup …
seq=46` in log; retail observer saw item disappear from world. Local
view still rendered the item on the ground.
**After Task 2b:** Item disappears locally as soon as ACE acks the
pickup. Three successful close-range pickups recorded in the log.
**Door-interaction regression check (B.4c carry-forward):** Not
explicitly re-tested this session; no code path touched by B.5
affects door interaction.
**Click-NPC bonus (M1 demo target 3 verification):** Not visually
verified this session — log shows `[B.4b] use guid=… name=Novedion
the Gem Seller seq=…` from B.4c testing but ACE response not
re-confirmed here. Carry-forward to next session.
---
## What did NOT work (and why it's not B.5's bug)
1. **Double-click on a ground item from any distance, or F from > 0.6 m.**
ACE auto-walks the player toward the item (`CreateMoveToChain`
`PhysicsObj.MoveToObject` + `EnqueueBroadcastMotion(MoveToObject)`),
but our client doesn't handle inbound `MoveToObject` motion broadcasts.
ACE's MoveToChain times out, the chain's `success: false` path sends
`InventoryServerSaveFailed (ActionCancelled)`, and the pickup never
completes. Visible as "character drifts toward item then flips back."
**Filed as #63.** Out of B.5's stated scope (which was: select-first
+ F-key wire chain). holtburger's `simulation.rs` has the reference
implementation; would be its own phase (B.6 or similar).
2. **Local-player pickup animation doesn't render.** Retail observers
see `+Acdream` play the bend-down-and-grab animation; our local view
shows nothing. ACE broadcasts `Motion(MotionCommand.Pickup)` via
`EnqueueBroadcastMotion`, our motion routing probably filters
self-echoes for the local player (motion is normally predicted
locally, not echoed from server). Server-initiated one-shot motions
like Pickup have no local prediction trigger, so they're dropped.
**Filed as #64.** Visual feedback gap only; pickup completes
correctly.
Both are well-defined follow-up work; neither blocks M1.
---
## Carry-overs from B.4c
Both pre-existed B.5; neither was touched.
- **#61** — AnimationSequencer link→cycle boundary frame-0 flash on
door swing. Low severity polish.
- **#62** — PARTSDIAG null-guard for sequencer-driven entities.
Latent; not currently reachable for doors.
---
## M1 status after B.5
Demo targets:
1. Walk through Holtburg — met (L.2a-d + L.2g shipped earlier)
2. Open the inn door — met (B.4b + B.4c shipped 2026-05-13)
3. Click an NPC — chain wired (B.4b), not visually re-verified this
session
4. Pick up an item — met, close-range path (this phase)
Outstanding work for the M1 demo recording:
- Optionally re-verify target 3 (NPC click) once and either confirm
met or file a gap.
- Optionally resolve #63 if the demo wants to show double-click /
out-of-range pickup. The close-range path is sufficient for the
scripted demo scenario.
- Carry-overs #61, #62, #64 are polish; do before recording if
visible on tape.
---
## Reproducibility
```powershell
Get-Process -Name AcDream.App -ErrorAction SilentlyContinue | Stop-Process -Force
Start-Sleep -Seconds 20
$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"
dotnet run --project src\AcDream.App\AcDream.App.csproj -c Debug 2>&1 |
Tee-Object -FilePath "launch-b5.log"
```
Log evidence:
```powershell
Get-Content launch-b5.log -Encoding Unicode |
Select-String -Pattern "\[B\.5\] pickup|\[B\.4b\] pick"
```
Expected: a `[B.5] pickup item=… container=0x5000000A seq=…` line for
each successful F-press, preceded by `[B.4b] pick guid=…` from the
single-click that set the selection.
---
## Files touched this session
- New: `src/AcDream.Core.Net/Messages/PickupEvent.cs`
- New: `tests/AcDream.Core.Net.Tests/Messages/PickupEventTests.cs`
- New: `docs/superpowers/plans/2026-05-14-phase-b5-pickup.md`
- New: `docs/research/2026-05-14-b5-shipped-handoff.md` (this file)
- Modified: `src/AcDream.Core.Net/Messages/InteractRequests.cs`
- Modified: `src/AcDream.Core.Net/WorldSession.cs`
- Modified: `src/AcDream.App/Rendering/GameWindow.cs`
- Modified: `tests/AcDream.Core.Net.Tests/Messages/InteractRequestsTests.cs`
- Modified: `docs/ISSUES.md` (added #63, #64)
---
## State at handoff
- **Branch:** `claude/phase-b5-pickup`, 6 commits ahead of `main`
(predecessor handoff + 5 implementation commits + this docs commit
land in the same merge).
- **Main HEAD before merge:** `e7842e0` — Merge B.4c.
- **Build state:** worktree compiles cleanly under `dotnet build -c Debug`.
- **Tests:** baseline + 3 new (PickupEvent) + 2 new (BuildPickUp +
sign-correctness) — failure count unchanged.
Ready for non-fast-forward merge into `main`.