don
/
neon-sprawl
mirror of https://github.com/ViPro-Technologies/neon-sprawl
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity
neon-sprawl/docs/plans/NEO-24-implementation-plan.md

19 KiB
Raw Blame History

NEO-24 — Implementation plan

Story reference

Field Value
Key NEO-24
Title E1.M3: Client tab-target + lock UI synced to server
Linear NEO-24
Slug E1M3-02
Git branch NEO-24-e1m3-client-tab-target-lock-ui-synced-to-server
Parent context Epic 1 — Core Player Runtime · E1M3 prototype backlog
Depends on NEO-23 (targeting HTTP v1)
Decomposition E1.M3 — InteractionAndTargetingLayer; authority notes: client_server_authority — E1.M3

Goal, scope, and out-of-scope

Goal: Tab (or agreed input action) cycles server-eligible stub targets in a documented deterministic order; HUD / debug overlay reflects the last server-acknowledged TargetState (lockedTargetId, validity, sequence). Client sends selection intent via existing POST …/target/select; optional optimistic presentation must reconcile to the authoritative targetState in each 200 response (including denials), per E1.M3 authority doc.

In scope

  • Godot: input → TargetSelectRequest v1 → parse TargetSelectResponse / PlayerTargetStateResponse; maintain last acknowledged target state for UI.
  • Tab cycle order: match server stub contract — lexicographic ascending id over the prototype registry ids (prototype_target_alpha, then prototype_target_beta), wrapping from last → first; same ordering as server README — Targeting / PrototypeTargetRegistry comments.
  • Clear lock: POST with targetId omitted or JSON null (NEO-23 rule); binding documented in client/project.godot (proposed: Esc via target_clear).
  • UI: readable label (or small overlay) showing server lock id (or none / empty), validity, and optionally sequence for debugging.
  • Soft lock / movement: when the player moves, validity can become out_of_range without clearing lockedTargetId. Client must refresh authoritative state periodically or on hooks so the overlay stays truthful (see Technical approach).
  • Shared constants: duplicate stub target id list + order in a small GDScript constants module aligned with PrototypeTargetRegistry (NEO-23 plan expectation for NEO-24 preview/cycle).

Out of scope (per Linear / backlog)

  • Nearest-in-cone auto-target; sticky soft-target under latency.
  • AbilityCastRequest / hotbar — E1.M4.
  • Server route or DTO changes (covered by NEO-23); no new ASP.NET endpoints unless a client bug reveals a contract gap (then narrow fix + plan update).

Acceptance criteria checklist

  • With ≥2 stub targets, tab advances selection intent in ascending id order and wraps at the end of the ordered list. (PrototypeTargetConstants.next_id_after + test_tab_from_no_lock_selects_first_id / test_tab_from_alpha_requests_beta / test_tab_wraps_from_beta_to_alpha.)
  • After each successful round-trip, UI shows lockedTargetId (or — for none) and validity from the response body (targetState on POST, body on GET). (TargetSelectionClient._extract_target_state_fields + main.gd:_on_target_state_changed; test_sync_get_200_emits_state.)
  • On selectionApplied: false, UI shows authoritative targetState from the same response (no “stuck optimistic” lock id that the server denied). (_update_state_from_post replaces cached state from targetState on every 200, success or deny; test_denial_reflects_authoritative_target_state.)
  • Clear action clears lock server-side and UI updates from the response. (request_clear_target omits targetId; test_clear_issues_post_without_target_id.)
  • Input bindings for tab cycle and clear are documented in client/project.godot and in client/README.md (new “Target lock + tab cycle (NEO-24)” section).

Server-up manual QA steps live in the README section above; run the server + client to exercise the soft-lock out-of-range flow end to end.

Technical approach

  1. prototype_target_constants.gd (or rename if clearer): export ordered Array[String] of stub ids matching PrototypeTargetRegistry (alpha, beta) for tab math only; single source for “eligible cycle list” on the client.

  2. target_selection_client.gd: Node analogous to interaction_request_client.gd / position_authority_client.gd: owns injectable HTTP transport, base_url, dev_player_id, _busy guard. Responsibilities:

    • request_sync_from_server() — GET …/target → parse PlayerTargetStateResponse v1 → cache + emit signal.
    • request_select_target_id(target_id: String) — POST …/target/select with body {"schemaVersion":1,"targetId":…}.
    • 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).
    • 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, PositionAuthorityClient emits a new authoritative_ack(world) signal — fired on every server-confirmed position (boot GET 200 and successful move-stream 200). 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). Why authoritative_ack instead of authoritative_position_received: the latter only fires on boot / move-rejection resync — normal move-stream 200s are intentionally silent to avoid RTT rubber-banding — so hooking the target refresh to it leaves the soft-lock path unreachable during WASD locomotion (blocking issue raised in 2026-04-21-NEO-24 review). The new signal carries only the world position (no apply_as_snap flag) and is emitted after every authoritative position regardless of whether the client snaps.

  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. Also connect PositionAuthorityClient.authoritative_ack → target_selection_client.on_authoritative_ack(...) (the handler is a no-op when no lock is held, so boot is naturally skipped; see step 2 refresh policy).

  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.

Files to add

Path Purpose
client/scripts/prototype_target_constants.gd Ordered stub targetId list + helpers for “next in cycle” matching server registry.
client/scripts/target_selection_client.gd HTTP GET/POST targeting APIs; tab/clear entrypoints; throttled refresh; signals for UI.
client/test/target_selection_test_double.gd Test subclass injecting mock HTTP (pattern from position_authority_test_double.gd).
client/test/target_selection_client_test.gd GdUnit4: tab order wrap, POST success path, denial keeps server lockedTargetId, clear POST, GET parse.

Files to modify

Path Rationale
client/scenes/main.tscn Add TargetSelectionClient node + TargetLockLabel (or reuse a debug panel) bound to scripts. Post-merge follow-up (Decision 4) also adds World/PrototypeTargetMarkers (colored mast + translucent radius ring per anchor).
client/scripts/main.gd Wire @onready target client + label; connect target_state_changed; trigger initial request_sync_from_server() after authority boot path; connect authoritative_ack → on_authoritative_ack (see Decision 2). Re-render TargetLockLabel each physics tick with live per-anchor distance lines from PrototypeTargetConstants (Decision 4).
client/scripts/prototype_target_constants.gd Display-only mirror of PrototypeTargetRegistry.cs anchors + radii used by the markers and HUD distance readouts (Decision 4).
client/scripts/prototype_target_markers.gd Spawns a visible mast + flat ring at every PrototypeTargetConstants.ANCHORS entry so players can reason about range visually (Decision 4).
client/scripts/position_authority_client.gd Add authoritative_ack(world) signal and emit it from both BOOT_GET 200 and STREAM_POST 200 so held locks re-validate during normal WASD locomotion without re-introducing snap rubber-banding. Extracted _parse_world_from_response(...) helper to avoid duplicating JSON parse across the two paths.
client/project.godot Register target_tab and target_clear input actions with defaults above.
client/README.md One short subsection: tab-target prototype, bindings, and how to read the lock overlay (parity with interaction/move docs).

Tests

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; movement-triggered refresh: calling on_authoritative_ack(...) while a lock is held fires a GET, while no lock is held fires nothing, and two back-to-back acks within cooldown produce one GET. Use MockHttpTransport pattern from position_authority_client_test.gd.
client/test/position_authority_client_test.gd Extend the existing suite to assert that authoritative_ack is emitted on both BOOT_GET 200 and successful move-stream 200 (existing no-snap-on-stream-200 assertion stays).
client/test/target_refresh_on_locomotion_test.gd Integration (added during review follow-up): wires a real PositionAuthorityClient + TargetSelectionClient with mock transports and the same signal wiring main.gd uses; asserts that with a lock held, a move-stream 200 produces exactly one throttled refresh GET on the target client, and that the same path is a no-op with no lock. Catches renames / mis-wires between the two scripts that the unit suites miss.

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 for HTTP contract changes).

Open questions / risks

  • 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

# 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_ack (new signal — fires on every server-confirmed position, including move-stream 200) 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).
3 Signal for the refresh hook Review follow-up (see 2026-04-21-NEO-24 review): the initial wiring hooked authoritative_position_received, which only fires on boot / rejection resync — normal move-stream 200s are intentionally silent to avoid RTT rubber-banding. A new authoritative_ack(world) signal was added to PositionAuthorityClient and emitted from both 200 paths so the movement-driven refresh is actually reachable during WASD. The snap signal kept its current semantics; the ack signal is pure heartbeat.
4 Post-merge UX: anchor visibility Users reported "targeting feels inconsistent" because the PrototypeTargetRegistry anchors (alpha (-5,-5) r=8, beta (8,8) r=4) have no visible meshes — the only visible console in the scene is the NEO-9 PrototypeTerminal at origin, which is unrelated to targeting. Added client/scripts/prototype_target_markers.gd + PrototypeTargetMarkers node in main.tscn (colored mast + translucent flat ring per anchor) and extended TargetLockLabel with per-anchor `: m / (in
5 Post-merge UX: position-drift race Follow-up report: "when in range of beta, pressing Tab says denied." Root cause is a race between move-stream 20 Hz sampling and the target-select POST — a Tab press right after you stop moving is validated against the last sampled position, which can trail the visible capsule by several meters. Two changes: (a) PlayerPositionLabel now renders a srv: (x,y,z) Δ=<d>m age=<ms> line from PositionAuthorityClient.authoritative_ack, making the server/client divergence directly visible; (b) TargetSelectionClient.set_freshness_kick(authority, player) (wired in main.gd) has the target client call authority.submit_stream_targets([player.global_position]) immediately before every POST /target/select, so the server's stored snapshot is as fresh as possible when the range check runs. The freshness kick does not wait on the stream 200 — both requests race on separate HTTP connections — but it shrinks the window enough in practice that "stop, Tab, denied" stops firing. Both pieces are pure client changes; the server contract (NEO-23) is untouched.
6 Post-merge UX: range-aware Tab cycle Follow-up report: "tab here does not acquire target" while standing 2.41 m from beta (inside 4 m) with Δ=0 — server and client agreed on position but Tab still denied. Root cause: with no lock held, request_tab_next() returned ORDERED_IDS[0] = alpha (17.90 m away), not the anchor the user was visibly standing next to. Added PrototypeTargetConstants.next_in_range_id_after(current, world) which walks the cycle and returns the first anchor whose client-side horizontal distance ≤ radius; TargetSelectionClient.request_tab_next() prefers that pick when the player ref is wired (via the existing set_freshness_kick path) and falls back to next_id_after(current) when nothing is in range so the server still owns the denial reason. Pure client-side UX improvement; server contract untouched. The picker also skips the currently-locked id so Tab always expresses a swap intent — if the only in-range target is what you already have, the fallback picks the next cycle id and lets the server deny (preserves the NEO-23 soft-lock "Tab = visible denial when no swap possible" rule).
7 Prototype target geometry: shared radius + overlap Follow-up request: make all targetable objects share a single radius and make the rings overlap in the test scene so Tab-flipping is observable without locomotion. Introduced PrototypeTargetRegistry.SharedLockRadius = 6.0 and moved anchors to alpha (-3, 0.5, -3) and beta (3, 0, 3) (distance ~8.49 m ⇒ ~3.5 m-wide overlap centered at origin). Spawn (-5, -5) keeps alpha in range (~2.83 m) and beta out (~11.31 m) so the existing manual QA "Tab-to-out-of-range = soft-lock denial" path still holds; walking to the origin puts both rings in range and Tab cleanly flips alpha ↔ beta. client/scripts/prototype_target_constants.gd mirrors the new values as display-only data (+ a SHARED_LOCK_RADIUS constant to match the server). Keep the two in lock-step; NEO-23 reader tests updated to the new boundary ((3, -3) for alpha radius 6). Per-target radii will return with real combat design (E5.M1); this is a prototype stub only.
8 Post-merge UX: positionHint on target/select (NEO-23 contract extension) Follow-up report: "targeted A, moved out of A range and into B range — Tab gets denied incorrectly" at Δ=0, age≈8 s. Root cause: the freshness kick from Decision 5 kicks a move-stream POST and the select POST on separate HTTP connections — nothing guarantees the server processes the stream first, so the select's range check can still run against the stale stored snap. Fix: extend TargetSelectRequest v1 with an optional positionHint: {x,y,z}; when present, the server uses it for the radius check and for the validity field in the response (both the soft-lock and applied-lock paths). The hint is purely advisory — it does not write to IPositionStateStore (move-stream stays the only write path) and remains trust-on-client for the prototype; E5.M1 combat will bound the hint against the stored snap / movement budget before shipping to players. Client wires the hint automatically when set_freshness_kick(...) is called (same player ref), so existing headless unit tests that skip that wiring continue to omit the hint. The freshness kick is kept in place to refresh the stored snap for subsequent validity GETs, but is no longer load-bearing for denial correctness.