86440ff04a
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>
235 lines
10 KiB
Markdown
235 lines
10 KiB
Markdown
# 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).
|