diff --git a/docs/plans/NEO-26-implementation-plan.md b/docs/plans/NEO-26-implementation-plan.md new file mode 100644 index 0000000..a543a03 --- /dev/null +++ b/docs/plans/NEO-26-implementation-plan.md @@ -0,0 +1,96 @@ +# NEO-26 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-26 | +| **Title** | E1.M3: SelectionEvent surface + debug or HUD consumer | +| **Linear** | [NEO-26](https://linear.app/neon-sprawl/issue/NEO-26/e1m3-selectionevent-surface-debug-or-hud-consumer) | +| **Slug** | E1M3-04 | +| **Git branch** | `NEO-26-selectionevent-surface-debug-consumer` | +| **Parent context** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) · [E1M3 prototype backlog](E1M3-prototype-backlog.md) | +| **Depends on** | [NEO-23](NEO-23-implementation-plan.md) (`TargetState` v1), [NEO-24](NEO-24-implementation-plan.md) (tab/clear + `TargetSelectionClient`) — code paths exist on `main`; Linear may still list relations until those issues are closed. | +| **Decomposition** | [E1.M3 — InteractionAndTargetingLayer](../decomposition/modules/E1_M3_InteractionAndTargetingLayer.md) (`SelectionEvent` contract); [E1.M4 — AbilityInputScaffold](../decomposition/modules/E1_M4_AbilityInputScaffold.md) (future subscriber). | + +## Goal, scope, and out-of-scope + +**Goal:** Every **material change** to the client’s **server-acknowledged combat target lock id** (`lockedTargetId` in the cached `TargetState` mirror) is observable as a **`SelectionEvent`** carrying **previous** id, **next** id, and **cause** (`tab`, `clear`, `server_correction`, …). Ship **one** built-in consumer (debug overlay, `print`, or thin HUD) so the bus is exercised in dev runs. + +**In scope** + +- Minimal **GDScript** event shape (Dictionary or small typed helper) aligned with the illustrative table in E1.M3: `previous` (Variant `String` or `null`), `next` (same), `cause` (`String` enum-like literals). +- **Cause attribution** at the authoritative update boundary inside the existing selection client (see Technical approach) so we do not guess causes from state diffs alone. +- **Consumer:** append-only **ring buffer** of last **N** events (e.g. 8–16) shown in UI **or** dev-only `print` gated by a `@export var` — pick one primary path in implementation; document the **subscription point** for E1.M4 (`signal` name + emitter node path pattern). +- **Tests:** tab vs clear vs movement-driven GET produce **distinct** `cause` strings when they each produce a lock-id transition (or documented no-op when id unchanged). + +**Out of scope (per Linear / backlog)** + +- Full prompt / party UI; wire protobuf events. +- **Interactable** primary selection (NEO-25 catalog) — this story is **target lock** only unless a trivial extension falls out naturally (if not, say “none” interactable events in module doc follow-up). +- Changing server **`TargetState`** contracts (NEO-23); new HTTP routes. + +## Acceptance criteria checklist + +- [ ] **Tab** path: after a successful selection cycle that **changes** `lockedTargetId`, a `SelectionEvent` is emitted with `cause` **`tab`** (and correct previous/next ids). +- [ ] **Clear** path: clearing the lock emits **`clear`** when `lockedTargetId` transitions from non-null to `null`. +- [ ] **Server correction:** a **GET**-driven authoritative update (e.g. movement-triggered refresh from `on_authoritative_ack` → `request_sync_from_server`) that **changes** `lockedTargetId` emits **`server_correction`**. If the prototype server never changes id on GET (only `validity`), document that **id-unchanged** refreshes do **not** emit `SelectionEvent` for v1, and add a test or comment proving GET is still classified when id *does* change (or a focused mock scenario). +- [ ] **E1.M4 hook:** `docs/plans/NEO-26-implementation-plan.md` (and optionally `client/README.md` under targeting) states **which node** exposes **`selection_event` (or agreed name)** and that E1.M4 may `connect` without reshaping the payload. + +## Technical approach + +1. **Single emitter:** Extend **`TargetSelectionClient`** (`client/scripts/target_selection_client.gd`) rather than diffing `target_state_changed` in a second node — only this class knows whether the in-flight HTTP phase was **GET** (refresh) vs **POST** from **tab**, **clear**, or direct **`request_select_target_id`** (if used). + +2. **Intent / phase tags:** Before starting HTTP, set a private enum or String tag, e.g. `_pending_selection_cause`, cleared after handling the response: + - `request_tab_next` → `tab` (even though it calls `request_select_target_id` internally, set cause at the tab entrypoint before POST). + - `request_clear_target` → `clear`. + - `request_select_target_id` when **not** entered from `request_tab_next` → `direct_select` (reserved for tests / future UI; document). + - `request_sync_from_server` when invoked from **`on_authoritative_ack`** → mark refresh as **`server_correction`** for the next GET completion. + - Boot / one-shot sync from `main.gd` → use **`server_correction`** or **`initial_sync`** consistently; prefer **`initial_sync`** only if we want tests to ignore boot noise — **Decision (default):** use **`server_correction`** for any GET that is not tied to a tab/clear POST, including first sync, **only emit** if normalized `lockedTargetId` differs from prior snapshot (avoids spam on identical boot GET). + +3. **Emit rule:** After `_state` is updated in `_update_state_from_get` / `_update_state_from_post`, compare **previous** normalized `lockedTargetId` to **new**; if equal, **no** `SelectionEvent` (validity-only changes stay on `target_state_changed` only unless product later expands the contract). + +4. **Signal:** Add e.g. **`selection_event(event: Dictionary)`** with keys `previous`, `next`, `cause` (and optional `sequence` int passthrough for debugging). Keep **`target_state_changed`** unchanged for existing HUD. + +5. **Consumer:** New small script **`selection_event_debug_panel.gd`** (type `Control` or `Node` driving a `RichTextLabel`) under the HUD canvas, connected in **`main.gd`** only via `connect` — keeps [godot-client-script-organization](../../.cursor/rules/godot-client-script-organization.md) satisfied. + +6. **Docs:** Update **`client/README.md`** “Target lock + tab cycle” area with a short **“SelectionEvent (NEO-26)”** subsection: signal name, payload keys, E1.M4 subscription note. + +## Files to add + +| Path | Purpose | +|------|---------| +| `client/scripts/selection_event_debug_panel.gd` | Thin HUD / debug list: listens for `selection_event`, shows last N events (timestamp optional). | +| `client/test/selection_event_client_test.gd` | GdUnit4: assert causes for tab POST, clear POST, and GET-with-mock-body changing lock id (`TargetSelectionTestDouble` pattern). | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `client/scripts/target_selection_client.gd` | Track pending cause through GET/POST; compare old/new `lockedTargetId`; emit `selection_event`; document next to `target_state_changed`. | +| `client/scenes/main.tscn` | Add debug panel node (sibling under `UICanvas` or next to existing target HUD) and script attachment. | +| `client/scripts/main.gd` | Connect `TargetSelectionClient.selection_event` → debug panel; minimal wiring only. | +| `client/README.md` | Document subscription point + payload for E1.M4. | +| `docs/decomposition/modules/E1_M3_InteractionAndTargetingLayer.md` | **Implementation snapshot** bullet: replace “Still open — `SelectionEvent`” with pointer to NEO-26 + signal name (keeps decomposition aligned with shipped code). | + +## Tests + +| Test file | What it covers | +|-----------|------------------| +| `client/test/selection_event_client_test.gd` | **Add:** Tab mock POST changes lock → event `cause == "tab"`. Clear mock → `"clear"`. Simulated GET completion after flagging movement refresh → `"server_correction"` when body changes `lockedTargetId`. Assert **no** event when id unchanged. | +| `client/test/target_selection_client_test.gd` | **Change (optional):** If new behavior duplicates setup, keep NEO-24 tests focused on HTTP/state; only touch this file when sharing helpers avoids copy-paste. | + +**Manual verification:** Run server + client; tab and clear; walk until soft-lock story changes validity — confirm debug line classification matches plan; screenshot or note in story if `server_correction` only appears when GET changes id. + +## Open questions / risks + +- **Validity-only updates:** v1 emits only on **`lockedTargetId`** change; if design later needs `validity` transitions as events, extend shape with `kind: "lock_id" \| "validity"` in a follow-up. +- **Linear blocked-by:** Relations may still show NEO-23/NEO-24 until closed; implementation assumes their client/server surfaces on `main` are present. + +## Decisions + +| Topic | Decision | +|--------|----------| +| D1 — Emitter location | **`TargetSelectionClient`** owns `selection_event` (intent-aware). | +| D2 — Boot GET | Same emit rules as other GETs; use cause **`server_correction`** for GET path, **suppress** if id unchanged from empty initial snapshot. | +| D3 — `direct_select` | Expose cause for non-tab `request_select_target_id` callers (tests); not required for acceptance but avoids mis-labeling as `tab`. |