docs(B.5): fresh-session handoff for BuildPickUp + ground-item interaction
Captures post-B.4c state, click-NPC investigation findings (chain already wired via Tell/CommunicationTransientString/etc; verify opportunistically during B.5 visual test), and B.5 scope decisions made in chat before the user requested a session handoff: - Trigger: F-key (SelectionPickUp action, already bound) - Target: requires _selectedGuid (no pick-under-cursor fallback) - Wire opcode 0x0019 (GameAction.PutItemInContainer) - Payload: itemGuid + containerGuid + placement (12 bytes) - Container = _playerServerGuid - Three changes in two existing files (~50 LOC total) Plus carry-overs from B.4c (#61 cycle-boundary flap, #62 PARTSDIAG null-guard), the B.4b ID-translation gotcha pattern to watch for, and the standard ACE session-race tip. Branch `claude/phase-b5-pickup` (renamed from `claude/investigate-npc-click`) is the workspace; the fresh session should start there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik
parent
e7842e0f1e
commit
86440ff04a
1 changed files with 235 additions and 0 deletions
235
docs/research/2026-05-13-b5-pickup-handoff.md
Normal file
235
docs/research/2026-05-13-b5-pickup-handoff.md
Normal file
|
|
@ -0,0 +1,235 @@
|
|||
# Phase B.5 — BuildPickUp + ground-item interaction — fresh-session handoff
|
||||
|
||||
**Date:** 2026-05-13 evening (after B.4c ship).
|
||||
**Branch:** `claude/phase-b5-pickup` (renamed from `claude/investigate-npc-click`).
|
||||
**Worktree:** `C:\Users\erikn\source\repos\acdream\.claude\worktrees\investigate-npc-click` (directory name kept; only the branch was renamed).
|
||||
**Predecessor on main:** `e7842e0` — `Merge branch 'claude/phase-b4c-door-anim' — Phase B.4c door swing animation`.
|
||||
|
||||
---
|
||||
|
||||
## TL;DR
|
||||
|
||||
After B.4c shipped (doors visibly swing open/close), three of M1's four
|
||||
demo targets are met: *walk through Holtburg*, *open the inn door*, and
|
||||
likely *click an NPC* (per the investigation below; not yet
|
||||
visual-verified). The remaining target is *pick up an item*, which
|
||||
needs a new outbound wire builder + F-key handler in `GameWindow`.
|
||||
|
||||
Phase **B.5** is the slice that closes M1's "click + pickup" demo
|
||||
path. Scope: ~50 LOC across two existing files (no new files).
|
||||
Implementation pattern mirrors B.4b's outbound Use chain.
|
||||
|
||||
---
|
||||
|
||||
## Investigation findings (carry forward)
|
||||
|
||||
Before starting B.5 work, the controller agent investigated whether
|
||||
B.4b's existing `BuildUse` chain already handles "click an NPC". Code
|
||||
reading produced this answer: **yes, the basic chat-dialogue case
|
||||
should already work end-to-end with zero new code**. Verify
|
||||
opportunistically during B.5's visual test by clicking an NPC while
|
||||
in-world.
|
||||
|
||||
Specifically:
|
||||
|
||||
- **ACE's `Creature.ActOnUse`** at
|
||||
`references/ACE/Source/ACE.Server/WorldObjects/Creature.cs:334`
|
||||
defers to `base.OnActivate → EmoteManager.OnUse()`. The emote
|
||||
manager walks the creature's emote table and emits `Tell`,
|
||||
`CommunicationTransientString`, `Motion`, and other game events.
|
||||
- **All those events are already wired** in
|
||||
`src/AcDream.Core.Net/GameEventWiring.cs`:
|
||||
- `Tell (0x0027)` → `chat.OnTellReceived` (line 78)
|
||||
- `CommunicationTransientString (0x028B)` → `chat.OnSystemMessage` (line 83)
|
||||
- `WeenieError / WeenieErrorWithString` → `chat.OnSystemMessage` (lines 139, 144)
|
||||
Plus `UpdateMotion (0xF74D)` is already routed for creature entities
|
||||
via `OnLiveMotionUpdated`.
|
||||
- **`UseDone (0x01C7)`** — the completion ack — has a parser at
|
||||
`GameEvents.ParseUseDone` but is **not registered** with the
|
||||
dispatcher. Silent drop. Harmless for the basic demo (the chat
|
||||
events arrive independently), but worth filing as a follow-up if not
|
||||
picked up by B.5.
|
||||
|
||||
**Conclusion:** click-NPC chain is wired; no code change needed for the
|
||||
M1 demo target 3 acceptance. Verify in-world during B.5's launch.
|
||||
|
||||
---
|
||||
|
||||
## B.5 scope (decisions already made)
|
||||
|
||||
| Decision | Value | Rationale |
|
||||
|---|---|---|
|
||||
| Trigger | F-key (`InputAction.SelectionPickUp`) | Already bound at `KeyBindings.cs:172` |
|
||||
| Target selection | Requires `_selectedGuid` (B.4b's renamed field) | Mirrors retail F-key behavior + B.4b's `UseSelected` pattern. User single-clicks the ground item to select, then F to pickup. |
|
||||
| Wire opcode | `GameAction.PutItemInContainer (0x0019)` | ACE source: `references/ACE/Source/ACE.Server/Network/GameAction/Actions/GameActionPutItemInContainer.cs` |
|
||||
| Wire payload | 12 bytes: `itemGuid (u32) + containerGuid (u32) + placement (i32)` | Same source |
|
||||
| Container destination | The player's own server guid (`_playerServerGuid`) | Single-bag pickup; bag-specific destinations are M2+ work |
|
||||
| Placement value | 0 (let server pick slot) | Simplest; placement-control UI is M2+ |
|
||||
| Visual feedback | Toast + `[pickup]` log line | No inventory UI yet; the existing `WieldObject` / `InventoryPutObjInContainer` server events already update `ItemRepository` so the state is correct internally |
|
||||
| Pick under cursor fallback | **NO** | Out of scope per user decision. Strict select-first UX. |
|
||||
|
||||
Brainstorm explicitly **NOT** done with the user (interrupted before
|
||||
the design sections were presented). The new session should re-confirm
|
||||
these decisions are still desired before writing the spec — or just
|
||||
proceed if they remain obviously right.
|
||||
|
||||
---
|
||||
|
||||
## Three changes B.5 needs to land
|
||||
|
||||
1. **`src/AcDream.Core.Net/Messages/InteractRequests.cs`** — add
|
||||
`BuildPickUp(uint gameActionSequence, uint itemGuid, uint containerGuid, int placement)`.
|
||||
Pattern: same as the existing `BuildUseWithTarget` builder at line
|
||||
51 of that file. 20-byte total body (`0xF7B1 envelope + seq + opcode
|
||||
0x0019 + 12-byte payload`).
|
||||
|
||||
2. **`src/AcDream.App/Rendering/GameWindow.cs`** — add a private helper
|
||||
`SendPickUp(uint itemGuid)`:
|
||||
- Gate on `_liveSession?.CurrentState == InWorld` (same pattern as
|
||||
B.4b's `SendUse`).
|
||||
- `seq = _liveSession.NextGameActionSequence()`.
|
||||
- `body = InteractRequests.BuildPickUp(seq, itemGuid, _playerServerGuid, 0)`.
|
||||
- `_liveSession.SendGameAction(body)`.
|
||||
- Diagnostic: `Console.WriteLine($"[pickup] item=0x{itemGuid:X8} container=0x{_playerServerGuid:X8} seq={seq}")`.
|
||||
|
||||
3. **`src/AcDream.App/Rendering/GameWindow.cs` `OnInputAction` switch**
|
||||
— add `case InputAction.SelectionPickUp:` near the other `Select*` /
|
||||
`UseSelected` cases (B.4b added those around line 8633+). Body:
|
||||
`if (_selectedGuid is uint sel) SendPickUp(sel); else _debugVm?.AddToast("Nothing selected");`.
|
||||
|
||||
That's the whole code change. ~50 LOC including diagnostics.
|
||||
|
||||
---
|
||||
|
||||
## Likely ID-translation gotcha (the L.2g slice 1c pattern)
|
||||
|
||||
B.4b's L.2g slice 1c surfaced an ID-space mismatch: the **`BuildUse`**
|
||||
wire builder takes a `targetGuid` which is `entity.ServerGuid`, but
|
||||
`ShadowObjectRegistry` keys by `entity.Id`. For `BuildPickUp`:
|
||||
|
||||
- `itemGuid` argument must be `entity.ServerGuid` (the server's
|
||||
identifier — ACE looks it up in its world). ✅ B.4b's picker returns
|
||||
`ServerGuid`, so `_selectedGuid` already carries the right value.
|
||||
- `containerGuid` argument must be `_playerServerGuid` (the server's
|
||||
identifier for the player). ✅ Already a ServerGuid in `GameWindow`.
|
||||
|
||||
So B.5 should NOT hit the same ID-mismatch trap L.2g slice 1c did. But
|
||||
re-check at implementation time.
|
||||
|
||||
---
|
||||
|
||||
## ACE inbound chain (already wired)
|
||||
|
||||
After ACE processes a `BuildPickUp`, it broadcasts:
|
||||
|
||||
- `0x019B InventoryPutObjInContainer` — moves the item record into the
|
||||
player's container. Already wired to
|
||||
`ItemRepository.MoveItem(itemGuid, containerGuid, placement)` at
|
||||
`GameEventWiring.cs:239`.
|
||||
- `RemoveObject` for the world-spawned item — already wired (existing
|
||||
despawn path removes the ground item from view).
|
||||
- Possibly `WieldObject` if the item auto-equips — already wired
|
||||
(`GameEventWiring.cs:231`).
|
||||
|
||||
No new inbound wiring needed for the minimum demo. The user will see:
|
||||
|
||||
1. Click ground item → selection updates.
|
||||
2. Press F → diagnostic logs, packet sent.
|
||||
3. ACE processes; sends inventory + despawn events.
|
||||
4. Item disappears from ground.
|
||||
5. (No inventory UI yet, but item is in `ItemRepository`.)
|
||||
|
||||
---
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] `dotnet build` green.
|
||||
- [ ] `dotnet test` green: 1046 / 8 pre-existing-baseline fail
|
||||
(unchanged from main HEAD).
|
||||
- [ ] At Holtburg, drop a test item on the ground (via `/drop` server
|
||||
command or have ACE spawn one for the test character), then:
|
||||
- [ ] Single-click the item — `_selectedGuid` updates, B.4b's
|
||||
`[pick]` diagnostic shows the item's guid.
|
||||
- [ ] Press F — log shows `[pickup] item=0x... container=0x5000000A
|
||||
seq=N`.
|
||||
- [ ] Item disappears from the ground.
|
||||
- [ ] No regressions on door interaction (B.4b/B.4c still work).
|
||||
- [ ] **Bonus: click-NPC verification.** While in-world, single-click
|
||||
an NPC and press F (or double-click). Expected: NPC chat appears in
|
||||
the chat panel. If it does → M1 demo target 3 confirmed met. If not
|
||||
→ file the gap.
|
||||
- [ ] `docs/ISSUES.md` closure entry for whatever issue (if any) was
|
||||
filed for the pickup gap.
|
||||
- [ ] Roadmap + CLAUDE.md updated.
|
||||
|
||||
---
|
||||
|
||||
## Reproducibility
|
||||
|
||||
Same launch recipe as B.4c. Per CLAUDE.md "Logout-before-reconnect",
|
||||
wait 20-45s between client launches to let ACE clear stale sessions.
|
||||
|
||||
```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"
|
||||
$env:ACDREAM_PROBE_BUILDING = "1"
|
||||
dotnet run --project src\AcDream.App\AcDream.App.csproj -c Debug 2>&1 |
|
||||
Tee-Object -FilePath "launch-b5.log"
|
||||
```
|
||||
|
||||
Log grep:
|
||||
|
||||
```powershell
|
||||
Select-String -Path launch-b5.log -Pattern "pickup|\[pick\] guid=|UseDone|\[B.4b\] pick"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Carry-overs from B.4c (don't lose track)
|
||||
|
||||
- **#61** — AnimationSequencer link→cycle frame-0 flash on door swing.
|
||||
Visible as brief flap at end of swing animation. Low-severity polish.
|
||||
- **#62** — PARTSDIAG null-guard for sequencer-driven entities.
|
||||
Latent; not currently reachable for doors. One-line fix.
|
||||
- **Worktree at `.claude/worktrees/phase-b4c-door-anim`** still on disk
|
||||
(submodules blocked `git worktree remove` per B.4b precedent). Manual
|
||||
cleanup after this session: `rm -rf` the directory + `git worktree
|
||||
prune` + `git branch -D claude/phase-b4c-door-anim`.
|
||||
|
||||
---
|
||||
|
||||
## State at handoff
|
||||
|
||||
- **Branch:** `claude/phase-b5-pickup` (renamed from
|
||||
`claude/investigate-npc-click`).
|
||||
- **Worktree directory:** `.claude/worktrees/investigate-npc-click`
|
||||
(cosmetic mismatch with branch name; harmless).
|
||||
- **Commits ahead of main:** 1 after this handoff lands.
|
||||
- **Main HEAD:** `e7842e0`.
|
||||
- **Build state:** worktree compiles cleanly (verified via
|
||||
`dotnet build -c Debug`). Tests at 1046/8 baseline.
|
||||
- **Submodule state:** `references/WorldBuilder` initialized.
|
||||
`references/ACE` NOT initialized in this worktree — use the main
|
||||
repo's `references/ACE` for ACE source reads, or init via
|
||||
`git submodule update --init --depth=1 references/ACE` if extensive
|
||||
reading is needed.
|
||||
|
||||
---
|
||||
|
||||
## Why a fresh session
|
||||
|
||||
This session accumulated ~10 hours of context across L.2g, B.4b, and
|
||||
B.4c — the working set is large enough that starting B.5 cold lets the
|
||||
new session work with a clean context budget and avoids the compaction
|
||||
risk that hit the prior B.4b session.
|
||||
|
||||
The prompt for the new session is in the controller's reply that
|
||||
created this handoff (the chat message immediately after this commit).
|
||||
Loading…
Add table
Add a link
Reference in a new issue