97 lines
8.9 KiB
Markdown
97 lines
8.9 KiB
Markdown
# 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`. |
|