From 86440ff04ab09e1ac413829a670baa4f31b0181b Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 14 May 2026 14:35:52 +0200 Subject: [PATCH] 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) --- docs/research/2026-05-13-b5-pickup-handoff.md | 235 ++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 docs/research/2026-05-13-b5-pickup-handoff.md diff --git a/docs/research/2026-05-13-b5-pickup-handoff.md b/docs/research/2026-05-13-b5-pickup-handoff.md new file mode 100644 index 0000000..5013242 --- /dev/null +++ b/docs/research/2026-05-13-b5-pickup-handoff.md @@ -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).