From ddfc78a0448b864e933d7832e9018aa7a5858a54 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Tue, 21 Apr 2026 22:02:33 -0400 Subject: [PATCH] NEO-24: lock kickoff decisions (Tab/Esc; hybrid movement-triggered refresh) --- docs/plans/NEO-24-implementation-plan.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/plans/NEO-24-implementation-plan.md b/docs/plans/NEO-24-implementation-plan.md index 91c057a..5c448f4 100644 --- a/docs/plans/NEO-24-implementation-plan.md +++ b/docs/plans/NEO-24-implementation-plan.md @@ -50,13 +50,13 @@ - **`request_clear_target()`** — `POST` with no `targetId` / `null` per NEO-23. - **`request_tab_next()`** — read cached `lockedTargetId` + `validity`; compute **next** id in the ordered list (if no lock, choose **first** id in order); POST select; on deny, signal carries **server** `targetState` only (no client-only lock). - Signal e.g. **`target_state_changed(state: Dictionary)`** or typed fields for consumers (`lockedTargetId` Variant string-or-null, `validity` string, `sequence` int, `selection_applied` bool optional for last POST). - - **Polling:** while last known `lockedTargetId` is non-null, issue **`GET …/target`** on a **throttled** timer (e.g. every **0.25–0.5 s**) so **`out_of_range`** updates after locomotion without requiring new tab presses. Stop or slow polling when no lock if desired for noise (document choice in plan **Decisions** during implementation). + - **Refresh policy (hybrid, movement-triggered — Decision 2):** no periodic polling. Issue **`GET …/target`** when (a) boot sync runs, (b) any `POST …/target/select` completes (covered by response body already — no extra GET needed), and (c) **while a lock is currently held**, the `PositionAuthorityClient` emits **`authoritative_position_received`** *after* boot (i.e. movement-driven snaps, not the initial boot snap). A small **cooldown** (e.g. **250 ms**) coalesces bursts so many consecutive `move-stream` acks cannot produce a GET per ack. No timer otherwise. Rationale: stays honest about `out_of_range` without duplicating radius math on the client or holding an always-on polling timer. Trade-off: will not pick up purely server-driven state changes (none exist in NEO-23; revisit if combat/PvP adds them). 3. **Optimistic highlight (optional slice):** If implemented, keep **pending** selection separate from **acknowledged** state; on **200** denial, drop pending and paint from **`targetState`**. If timeboxed, **v1** may ship **ack-only** UI (still satisfies “optional optimistic” — none used). -4. **`main.tscn` / `main.gd`:** Add child node with `target_selection_client.gd`; connect signal to a **Label** (new `TargetLockLabel` under `UICanvas`). Call **`request_sync_from_server()`** once after boot position sync (or same frame as first authority ready) so HUD matches server on start. +4. **`main.tscn` / `main.gd`:** Add child node with `target_selection_client.gd`; connect signal to a **Label** (new `TargetLockLabel` under `UICanvas`). Call **`request_sync_from_server()`** once after boot position sync (or same frame as first authority ready) so HUD matches server on start. **Also** connect `PositionAuthorityClient.authoritative_position_received` → `target_selection_client.on_authoritative_position_snap(...)` (the client ignores the boot snap per `apply_as_snap` flag and only fires a GET when a lock is currently held; see step 2 refresh policy). -5. **`project.godot`:** `target_tab` → **Tab**; `target_clear` → **Escape** (document; user may request alternatives before implementation). +5. **`project.godot`:** `target_tab` → **Tab**; `target_clear` → **Escape** (locked — Decision 1). 6. **Manual QA:** Run server + client; tab between stubs; walk out of radius and confirm **`validity`** flips to **`out_of_range`** on overlay while id remains; clear with Esc. @@ -82,16 +82,18 @@ | Test file | What to cover | |-----------|----------------| -| `client/test/target_selection_client_test.gd` | **Mock transport** enqueues JSON: GET returns a v1 envelope; POST **apply** returns `selectionApplied: true` + `targetState`; POST **deny** returns `selectionApplied: false` + `reasonCode` + authoritative `targetState`; **tab** from no lock selects first id in order; **tab** from `alpha` requests `beta`; wrap from `beta` to `alpha`; **clear** issues POST without `targetId`. Use `MockHttpTransport` pattern from `position_authority_client_test.gd`. | +| `client/test/target_selection_client_test.gd` | **Mock transport** enqueues JSON: GET returns a v1 envelope; POST **apply** returns `selectionApplied: true` + `targetState`; POST **deny** returns `selectionApplied: false` + `reasonCode` + authoritative `targetState`; **tab** from no lock selects first id in order; **tab** from `alpha` requests `beta`; wrap from `beta` to `alpha`; **clear** issues POST without `targetId`; **movement-triggered refresh:** calling `on_authoritative_position_snap(...)` while a lock is held fires a GET, while **no** lock is held fires **nothing**, and two back-to-back snaps within cooldown produce **one** GET. Use `MockHttpTransport` pattern from `position_authority_client_test.gd`. | No new **C#** tests (client-only story). **Bruno:** NEO-23 targeting requests already live under `bruno/neon-sprawl-server/targeting/`; **no** new `.bru` required unless implementation discovers a **server** contract change (then add per repo [testing expectations](../../.cursor/rules/testing-expectations.md) for HTTP contract changes). ## Open questions / risks -- **Input bindings:** Plan assumes **Tab** + **Esc**. Confirm or specify alternatives (e.g. mouse thumb buttons) before locking `project.godot`. -- **Polling cost:** Throttled GET while locked is simple but adds HTTP traffic; acceptable for prototype. If objection, switch to polling only when HUD visible or tie to `move-stream` cadence with plan update. - **Scene visuals:** This story does **not** require 3D reticles on stub meshes; overlay text satisfies AC. World-space highlights can be a follow-up. +- **Server-driven state changes:** The hybrid refresh policy (Decision 2) **will not** pick up purely server-driven flips to `invalid_target` or future auto-clears — none exist in NEO-23, but if combat/PvP (E5.M1 / E6.M1) adds them later, revisit (either add a narrow server push or reintroduce a throttled poll). ## Decisions -*(None locked yet — update this table during implementation chat / PR.)* +| # | Topic | Choice | +|---|-------|--------| +| 1 | **Input bindings** | `target_tab` → **Tab**; `target_clear` → **Escape**. Registered in `client/project.godot`. | +| 2 | **Soft-lock refresh policy** | **Hybrid, movement-triggered.** No periodic polling. Refresh sources: boot `GET`, each `POST` response echo, **and** `GET` fired from `PositionAuthorityClient.authoritative_position_received` (non-boot snaps) **only while a lock is currently held**, with a **~250 ms** cooldown to coalesce bursts. Does **not** duplicate radius math on the client; does **not** cover future purely server-driven state changes (revisit when E5.M1 / E6.M1 introduce them). |