docs(B.5): implementation plan from writing-plans skill
This commit is contained in:
Erik
parent
54d9bb9d8d
commit
5c24f6cafe
1 changed files with 319 additions and 0 deletions
319
docs/superpowers/plans/2026-05-14-phase-b5-pickup.md
Normal file
319
docs/superpowers/plans/2026-05-14-phase-b5-pickup.md
Normal file
|
|
@ -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
|
||||
/// <summary>
|
||||
/// Pick up a ground item or move an item between containers. The
|
||||
/// server places the item in <paramref name="containerGuid"/> at
|
||||
/// the given <paramref name="placement"/> slot (pass 0 to let the
|
||||
/// server choose). For F-key ground-pickup, pass the player's own
|
||||
/// server guid as <paramref name="containerGuid"/>.
|
||||
///
|
||||
/// <para>
|
||||
/// Wire layout (ACE GameActionPutItemInContainer.Handle):
|
||||
/// <code>
|
||||
/// 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
|
||||
/// </code>
|
||||
/// </para>
|
||||
/// </summary>
|
||||
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<ITEM>` should appear in the log. The selection toast should show the item name.
|
||||
3. **Press F** — `[B.5] pickup item=0x<ITEM> container=0x5000000A seq=<N>` 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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue