98 lines
9.1 KiB
Markdown
98 lines
9.1 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: **dev-only `print`** behind an `@export` flag (planning choice 2026-04-25) so the bus is exercised without new HUD nodes.
|
||
|
||
**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:** optional **`print`** when `@export var log_selection_events` (name TBD) is true on **`TargetSelectionClient`** (or tiny child node); 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** focus / primary selection as `SelectionEvent` — **target lock** only for this ticket (see D6); catalog exists regardless of NEO-25 lifecycle.
|
||
- Changing server **`TargetState`** contracts (NEO-23); new HTTP routes.
|
||
|
||
## Acceptance criteria checklist
|
||
|
||
- [x] **Tab** path: after a successful selection cycle that **changes** `lockedTargetId`, a `SelectionEvent` is emitted with `cause` **`tab`** (and correct previous/next ids).
|
||
- [x] **Clear** path: clearing the lock emits **`clear`** when `lockedTargetId` transitions from non-null to `null`.
|
||
- [x] **Server correction:** a **GET**-driven authoritative update that **changes** `lockedTargetId` emits **`server_correction`**. **Id-unchanged** GETs (e.g. **`validity`** only) do **not** emit `SelectionEvent` for v1 — `test_get_validity_only_change_emits_nothing`; id change via mock in `test_get_lock_id_change_emits_server_correction`.
|
||
- [x] **E1.M4 hook:** this plan + **`client/README.md`** (“SelectionEvent (NEO-26)”) document **`TargetSelectionClient.selection_event`** and stable payload keys.
|
||
|
||
## 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` → use cause **`tab`** (same string as Tab path — planning choice 2026-04-25: no separate `direct_select`).
|
||
- `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` → **Decision (2026-04-25):** first and later **GET** completions use cause **`server_correction`**; **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:** No new scene nodes — **`TargetSelectionClient.log_selection_events`** mirrors each **`selection_event`** to **Output** via **`print`**.
|
||
|
||
6. **Docs:** Update **`client/README.md`** “Target lock + tab cycle” area with a short **“SelectionEvent (NEO-26)”** subsection: signal name, payload keys, print flag, E1.M4 subscription note.
|
||
|
||
## Files to add
|
||
|
||
| Path | Purpose |
|
||
|------|---------|
|
||
| `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). |
|
||
| `docs/manual-qa/NEO-26.md` | Manual QA for `log_selection_events` + optional subscription check. |
|
||
|
||
## Files to modify
|
||
|
||
| Path | Rationale |
|
||
|------|-----------|
|
||
| `client/scripts/target_selection_client.gd` | Track pending cause through GET/POST; compare old/new `lockedTargetId`; emit `selection_event`; optional `print` behind `@export`; document next to `target_state_changed`. |
|
||
| `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:** [`docs/manual-qa/NEO-26.md`](../../manual-qa/NEO-26.md).
|
||
|
||
## Open questions / risks
|
||
|
||
- **Validity-only updates:** If v1 stays **lock-id-only** (recommended), extend later with `kind` or dual fields if telemetry needs **`validity`** transitions on the same bus.
|
||
- **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 | Use cause **`server_correction`** for all GET completions (including first sync); **suppress** event if normalized `lockedTargetId` unchanged vs prior snapshot. |
|
||
| D3 — Non-tab `request_select_target_id` | Use cause **`tab`** (same literal as Tab path; no `direct_select`). |
|
||
| D4 — Consumer | **Print only** behind `@export` flag on emitter; no HUD panel in NEO-26. |
|
||
| D5 — Emit rule (v1) | **Lock id only:** emit `SelectionEvent` only when normalized **`lockedTargetId`** changes; **`validity`** / sequence updates without id change → no event (HUD keeps using `target_state_changed`). *Rationale (2026-04-25):* matches backlog “previous / next” id story, smallest payload, clear E1.M4 hook for cast target; avoids conflating soft-lock range telemetry with selection changes. Override if you want validity on this signal too. |
|
||
| D6 — Interactables | **Out of scope:** target lock only for NEO-26. *Rationale:* Linear / E1M3-04 acceptance is about **combat target** lock changes (`SelectionEvent` for tab-target), not interactable focus or “use now” flow ([NEO-9](NEO-9-implementation-plan.md) / catalog). Descriptor work ([NEO-25](NEO-25-implementation-plan.md)) can be **done** and this story still stays narrow: a unified “all selection kinds” bus would be **extra contract design** (payload + causes + wiring) that this ticket never asked for—not a dependency on NEO-25 being open. |
|