From 5c24f6cafe3cd8929cf30ca376092de8ffc7ccd5 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 14 May 2026 15:10:23 +0200 Subject: [PATCH] docs(B.5): implementation plan from writing-plans skill --- .../plans/2026-05-14-phase-b5-pickup.md | 319 ++++++++++++++++++ 1 file changed, 319 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-14-phase-b5-pickup.md diff --git a/docs/superpowers/plans/2026-05-14-phase-b5-pickup.md b/docs/superpowers/plans/2026-05-14-phase-b5-pickup.md new file mode 100644 index 0000000..32d22a6 --- /dev/null +++ b/docs/superpowers/plans/2026-05-14-phase-b5-pickup.md @@ -0,0 +1,319 @@ +# Phase B.5 — BuildPickUp + ground-item interaction — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Close M1 demo target 4/4 — make F-key pick up the currently-selected ground item by sending ACE's `PutItemInContainer (0x0019)` GameAction. + +**Architecture:** Mirror B.4b's outbound `BuildUse` chain. Add one new wire-builder (`InteractRequests.BuildPickUp`) and one new GameWindow helper (`SendPickUp`); wire the F-key action into the existing `OnInputAction` switch using the existing `_selectedGuid` field. The ACE inbound handlers (`InventoryPutObjInContainer 0x019B`, `RemoveObject`) are already wired in `GameEventWiring.cs` and will despawn the ground item once ACE acknowledges the pickup — no inbound work needed. + +**Tech Stack:** C# / .NET 10 / xUnit (existing). Wire format: `BinaryPrimitives.WriteUInt32LittleEndian` little-endian envelope same as every other `0xF7B1` GameAction in the file. + +--- + +## Predecessor & branch state + +- **Branch:** `claude/phase-b5-pickup` in worktree `.claude/worktrees/investigate-npc-click`. +- **Main HEAD at start:** `e7842e0` — Merge B.4c. +- **Existing commits on branch:** `86440ff` — the B.5 handoff doc (`docs/research/2026-05-13-b5-pickup-handoff.md`). + +--- + +## Wire format (verified against ACE source) + +`references/ACE/Source/ACE.Server/Network/GameAction/Actions/GameActionPutItemInContainer.cs`: + +```csharp +var itemGuid = message.Payload.ReadUInt32(); +var containerGuid = message.Payload.ReadUInt32(); +var placement = message.Payload.ReadInt32(); +``` + +`references/ACE/Source/ACE.Server/Network/GameAction/GameActionType.cs:13`: +``` +PutItemInContainer = 0x0019, +``` + +Therefore the full **24-byte** GameAction body is: + +| Offset | Field | Bytes | +|---|---|---| +| 0 | `0xF7B1` (GameAction envelope) | 4 | +| 4 | `gameActionSequence` | 4 | +| 8 | `0x0019` (PutItemInContainer subopcode) | 4 | +| 12 | `itemGuid` (u32, the server guid of the ground item) | 4 | +| 16 | `containerGuid` (u32, the player's server guid) | 4 | +| 20 | `placement` (i32, 0 = let server choose slot) | 4 | + +**NB:** The handoff doc (`2026-05-13-b5-pickup-handoff.md`) said "20-byte total body." That was an arithmetic error in the handoff — corrected here. + +--- + +## File structure (which files touched) + +- **Modify:** `src/AcDream.Core.Net/Messages/InteractRequests.cs` — add `PutItemInContainerOpcode` constant + `BuildPickUp(seq, itemGuid, containerGuid, placement)` builder. +- **Modify:** `tests/AcDream.Core.Net.Tests/Messages/InteractRequestsTests.cs` — add a unit test for `BuildPickUp` covering byte layout + opcode. +- **Modify:** `src/AcDream.App/Rendering/GameWindow.cs` — add private `SendPickUp(uint itemGuid)` helper next to `SendUse`; add `case InputAction.SelectionPickUp` in the `OnInputAction` switch. + +No new files. No tests for the GameWindow helper (it's a thin pass-through wrapper around `_liveSession.SendGameAction` mirroring `SendUse`, which itself has no unit test for the same reason). + +--- + +## Task 1: TDD — InteractRequests.BuildPickUp + +**Files:** +- Modify: `tests/AcDream.Core.Net.Tests/Messages/InteractRequestsTests.cs` +- Modify: `src/AcDream.Core.Net/Messages/InteractRequests.cs` + +- [ ] **Step 1: Write the failing test.** Append this new `[Fact]` immediately after `BuildUseWithTarget_WritesBothGuids` in `tests/AcDream.Core.Net.Tests/Messages/InteractRequestsTests.cs`: + +```csharp + [Fact] + public void BuildPickUp_WritesOpcode0x0019AndPayload() + { + byte[] body = InteractRequests.BuildPickUp( + gameActionSequence: 5, + itemGuid: 0xABCDu, + containerGuid: 0x5000000Au, + placement: 0); + + Assert.Equal(24, body.Length); + Assert.Equal(InteractRequests.GameActionEnvelope, + BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(0))); + Assert.Equal(5u, + BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(4))); + Assert.Equal(InteractRequests.PutItemInContainerOpcode, + BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(8))); + Assert.Equal(0xABCDu, + BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(12))); + Assert.Equal(0x5000000Au, + BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(16))); + Assert.Equal(0, + BinaryPrimitives.ReadInt32LittleEndian(body.AsSpan(20))); + } +``` + +- [ ] **Step 2: Run test to verify it fails.** + +```powershell +dotnet test tests/AcDream.Core.Net.Tests/AcDream.Core.Net.Tests.csproj ` + --filter "FullyQualifiedName~InteractRequestsTests.BuildPickUp" +``` + +Expected: build fails with `CS0117: 'InteractRequests' does not contain a definition for 'BuildPickUp'` (and a second error for `PutItemInContainerOpcode`). + +- [ ] **Step 3: Add the constant + builder.** Edit `src/AcDream.Core.Net/Messages/InteractRequests.cs`. Add this opcode constant immediately after the existing `TeleToLifestoneOpcode` declaration (~line 31): + +```csharp + public const uint PutItemInContainerOpcode = 0x0019u; +``` + +Then append the new builder method immediately after `BuildTeleToLifestone` (~line 75, just before the closing `}` of the class). Use the same `XmlDoc + BinaryPrimitives` style as the existing builders: + +```csharp + /// + /// Pick up a ground item or move an item between containers. The + /// server places the item in at + /// the given slot (pass 0 to let the + /// server choose). For F-key ground-pickup, pass the player's own + /// server guid as . + /// + /// + /// Wire layout (ACE GameActionPutItemInContainer.Handle): + /// + /// u32 0xF7B1 + /// u32 gameActionSequence + /// u32 0x0019 // PutItemInContainer + /// u32 itemGuid // server guid of the item + /// u32 containerGuid // destination container (player or bag) + /// i32 placement // 0 = server picks slot + /// + /// + /// + public static byte[] BuildPickUp( + uint gameActionSequence, uint itemGuid, uint containerGuid, int placement) + { + byte[] body = new byte[24]; + BinaryPrimitives.WriteUInt32LittleEndian(body, GameActionEnvelope); + BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(4), gameActionSequence); + BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(8), PutItemInContainerOpcode); + BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(12), itemGuid); + BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(16), containerGuid); + BinaryPrimitives.WriteInt32LittleEndian (body.AsSpan(20), placement); + return body; + } +``` + +- [ ] **Step 4: Run test to verify it passes.** + +```powershell +dotnet test tests/AcDream.Core.Net.Tests/AcDream.Core.Net.Tests.csproj ` + --filter "FullyQualifiedName~InteractRequestsTests.BuildPickUp" +``` + +Expected: 1 passing, 0 failing. + +- [ ] **Step 5: Commit.** + +```powershell +git add src/AcDream.Core.Net/Messages/InteractRequests.cs ` + tests/AcDream.Core.Net.Tests/Messages/InteractRequestsTests.cs +git commit -m "feat(B.5): InteractRequests.BuildPickUp — PutItemInContainer 0x0019" +``` + +--- + +## Task 2: GameWindow integration — SendPickUp + OnInputAction case + +**Files:** +- Modify: `src/AcDream.App/Rendering/GameWindow.cs` + +This task does NOT include new tests. `SendPickUp` is a 6-line passthrough that gates on `InWorld` state and routes to `_liveSession.SendGameAction`. It mirrors `SendUse` (also untested by unit tests) and is verified end-to-end via the in-world visual check in Task 3. + +- [ ] **Step 1: Add the `SendPickUp` helper.** Locate `SendUse` (currently around line 8870 in `src/AcDream.App/Rendering/GameWindow.cs`). Insert this new helper immediately after `SendUse`'s closing brace: + +```csharp + private void SendPickUp(uint itemGuid) + { + if (_liveSession is null + || _liveSession.CurrentState != AcDream.Core.Net.WorldSession.State.InWorld) + { + _debugVm?.AddToast("Not in world"); + return; + } + var seq = _liveSession.NextGameActionSequence(); + var body = AcDream.Core.Net.Messages.InteractRequests.BuildPickUp( + seq, itemGuid, _playerServerGuid, placement: 0); + _liveSession.SendGameAction(body); + Console.WriteLine($"[B.5] pickup item=0x{itemGuid:X8} container=0x{_playerServerGuid:X8} seq={seq}"); + } +``` + +- [ ] **Step 2: Wire the F-key action.** Locate the `OnInputAction` switch's `case InputAction.UseSelected` (currently around line 8745). Insert a new case immediately after it: + +```csharp + case AcDream.UI.Abstractions.Input.InputAction.SelectionPickUp: + if (_selectedGuid is uint pickupTarget) + SendPickUp(pickupTarget); + else + _debugVm?.AddToast("Nothing selected"); + break; +``` + +- [ ] **Step 3: Build the whole solution.** + +```powershell +dotnet build -c Debug +``` + +Expected: build succeeds with zero errors. (Existing warnings unchanged.) + +- [ ] **Step 4: Run the full test suite.** + +```powershell +dotnet test -c Debug --nologo +``` + +Expected: 1047 passing, 8 failing (8 pre-existing baseline failures from main HEAD; same count as before B.5). The new `BuildPickUp_WritesOpcode0x0019AndPayload` test contributes +1 passing. + +- [ ] **Step 5: Commit.** + +```powershell +git add src/AcDream.App/Rendering/GameWindow.cs +git commit -m "feat(B.5): SendPickUp helper + F-key SelectionPickUp wiring" +``` + +--- + +## Task 3: Visual verification in live client + +**Not a code task — user-driven acceptance test.** Run the launch recipe, drop a test item, click-then-F. + +- [ ] **Step 1: Kill stale client + wait for ACE session cleanup.** + +```powershell +Get-Process -Name AcDream.App -ErrorAction SilentlyContinue | Stop-Process -Force +Start-Sleep -Seconds 20 +``` + +- [ ] **Step 2: Launch the client (background).** + +```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" +dotnet run --project src\AcDream.App\AcDream.App.csproj -c Debug 2>&1 | + Tee-Object -FilePath "launch-b5.log" +``` + +- [ ] **Step 3: User-driven test scenario.** Hand off to user for visual verification: + + 1. ACE drops a test item near `+Acdream` (server `/drop` slash command, or whatever ACE supports for the testaccount). Specify what item type was dropped in the verification report. + 2. **Single-click the item** — `[B.4b] pick guid=0x` should appear in the log. The selection toast should show the item name. + 3. **Press F** — `[B.5] pickup item=0x container=0x5000000A seq=` should appear in the log. + 4. **Item should disappear from the ground** (ACE acks → `RemoveObject` → existing despawn path removes it from view). + 5. **No regressions on door interaction:** double-click the inn door, observe it swings open as in B.4c. + 6. **Bonus: NPC chat check.** Click an NPC, observe the chat panel. If NPC dialogue appears → M1 demo target 3 confirmed met. If silent → file an issue. + +- [ ] **Step 4: Grep the log for evidence.** + +```powershell +Select-String -Path launch-b5.log -Pattern "\[B\.5\] pickup|\[B\.4b\] pick|UseDone|InventoryPutObjInContainer|RemoveObject" +``` + +Expected: at least one `[B.5] pickup` line and a subsequent `RemoveObject` for the same guid. + +--- + +## Task 4: Ship handoff + docs + merge + +- [ ] **Step 1: Write the ship handoff doc** at `docs/research/2026-05-14-b5-shipped-handoff.md`. Follow the B.4c handoff structure: TL;DR, three-commit table (TDD builder + GameWindow integration + handoff itself), wire-format evidence, visual-verification evidence (with launch log excerpt), open issues carried forward (#61 #62 from B.4c), what shipped, what was left for later. + +- [ ] **Step 2: Update `docs/plans/2026-04-11-roadmap.md`** — move Phase B.5 from "Next phase candidates" into the shipped-table with the merge SHA placeholder + link to the handoff doc. + +- [ ] **Step 3: Update `CLAUDE.md`** — update the "Currently in Phase L.2" paragraph's M1 demo status from "3 of 4 met" to "4 of 4 met", reference the B.5 handoff, and add a fresh "Next phase candidates" list (chronic-issue triage, Phase C visual fidelity, N.6 slice 2, etc.). + +- [ ] **Step 4: Update `docs/ISSUES.md`** — if any new issues surfaced during visual verification, file them. If the click-NPC bonus check succeeded, note it in the recent-progress section. + +- [ ] **Step 5: Commit docs.** + +```powershell +git add docs/research/2026-05-14-b5-shipped-handoff.md ` + docs/plans/2026-04-11-roadmap.md ` + CLAUDE.md ` + docs/ISSUES.md +git commit -m "docs(B.5): ship handoff + roadmap/CLAUDE update + M1 4/4 met" +``` + +- [ ] **Step 6: Merge to main.** + +```powershell +git checkout main +git merge --no-ff claude/phase-b5-pickup -m "Merge branch 'claude/phase-b5-pickup' — Phase B.5 pickup" +``` + +- [ ] **Step 7: Optional worktree cleanup.** (Per B.4c precedent, submodules block `git worktree remove`; do `git worktree prune` after manually deleting the directory if disk pressure warrants. Otherwise skip — the directory is small.) + +--- + +## Acceptance criteria summary + +- [ ] `dotnet build -c Debug` green. +- [ ] `dotnet test -c Debug` shows +1 new passing (the `BuildPickUp_WritesOpcode0x0019AndPayload` test). +- [ ] Total pass count = baseline + 1; failure count unchanged (8 pre-existing). +- [ ] Visual: click ground item → F → log shows `[B.5] pickup ...` → item disappears. +- [ ] No regression on B.4c door interaction (double-click inn door still swings). +- [ ] Bonus: click NPC → chat appears in chat panel (or filed as a follow-up issue). +- [ ] Branch merged into main with non-fast-forward merge commit. + +--- + +## Carry-overs from B.4c (do not lose track) + +- **#61** — AnimationSequencer link→cycle frame-0 flash. Low severity. Not blocking M1 demo. +- **#62** — PARTSDIAG null-guard. Latent (not reachable for doors currently). One-line fix. + +Neither blocks B.5. Address before recording the M1 demo video if the door-swing flap is distracting on tape.