don
/
neon-sprawl
mirror of https://github.com/ViPro-Technologies/neon-sprawl
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #47 from ViPro-Technologies/NEO-24-e1m3-client-tab-target-lock-ui-synced-to-server 

NEO-24 e1m3 client tab target lock UI synced to server
pull/49/head
VinPropane 2026-04-24 20:10:53 -04:00 committed by GitHub
parent 37f44a4944 d6ef4fd811
commit 82f382353d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
31 changed files with 1832 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
.cursor/rules/planning-implementation-docs.md
View File

@ -13,8 +13,22 @@ Whenever you **decide** something during **story planning** or **implementation*
|-----------|--------|
| Ticketed story work | **`docs/plans/{LINEAR_ISSUE_ID}-implementation-plan.md`** (e.g. `NEO-7-…`) — add or edit a **Decisions** subsection, **resolve** former open questions, refresh **Technical approach** / **Acceptance criteria** checkboxes, and **Files to add/modify** if reality diverged from kickoff. |
| Cross-cutting or module contract | Relevant **`docs/decomposition/modules/*.md`**, **`docs/game-design/`**, or **`README`** / **`server/README.md`** / **`client/README.md`** when the decision affects how others integrate. |
| Manual QA steps for ticketed story work | **`docs/manual-qa/{LINEAR_ISSUE_ID}.md`** — see **Manual QA checklist** below. |
| Review findings | **`docs/reviews/…`** when using the code-review agent; link back to the plan if the review changed direction. When feedback is fixed, **strike through + `Done.`** on the original bullets in that review file — see **Code review follow-up** below. |
## Manual QA checklist
For ticketed story work that produces **user-visible** behavior (UI, input bindings, HTTP surfaces, scene wiring, runtime config, etc.), generate a checklist at **`docs/manual-qa/{LINEAR_ISSUE_ID}.md`** (example: `docs/manual-qa/NEO-24.md`) during **implementation**—not after handoff. The checklist complements automated tests; it captures the things a human needs to exercise end-to-end against real processes (server + client, database, input hardware) that unit / integration tests do not cover.
- **When to create:** at the end of implementation, once code is stable enough to exercise. Commit it on the **story branch** alongside the implementation commits; do not defer to a separate PR.
- **When to skip:** pure refactors, docs-only changes, or stories whose acceptance is fully covered by automated tests (state that reason in one line in the plan's **Tests** section instead of creating an empty checklist).
- **File header:** a small table with **Key**, **Title**, **Linear** link, **Plan** link back to `docs/plans/{KEY}-implementation-plan.md`, and the story **Branch**.
- **Body:** **numbered sections** grouped by concern (e.g. *Setup sanity*, *Happy path*, *Edge cases / failure modes*, *No regressions*), each with **`- [ ]` checkbox bullets** that are **concrete and observable**—name the UI label, the server route, the console message, the expected state. Avoid vague asks like "test it works."
- **Coverage cues:** include at least (a) a **boot / baseline** section, (b) each **acceptance criterion** from the plan exercised by a concrete user action, (c) relevant **denial / failure** paths (HTTP 4xx, server down, invalid input), (d) **coexistence / no regressions** with neighboring features the story touches.
- **Keep it current:** if implementation reveals a new failure mode or a changed binding, update the checklist in the same commit. Any `Done.` / `Deferred` follow-ups from a QA pass belong back in the plan's **Decisions** or **Open questions**, not as strikethroughs on the checklist.
The checklist is for the **user** (or a reviewer) to execute; the agent generates it but does **not** tick the boxes on the user's behalf unless the user explicitly confirms a section was exercised.
## Code review follow-up (resolved suggestions)
When **blocking issues**, **suggestions**, or agreed **nits** from a saved **`docs/reviews/YYYY-MM-DD-*.md`** are **fixed** in code or other docs, **edit that same review file** in the **same** change-set or the **next** commit—do **not** only fix the code and leave the review implying feedback is still open.
@ -33,7 +47,8 @@ When **blocking issues**, **suggestions**, or agreed **nits** from a saved **`do
## Agent behavior
- After **kickoff**, the plan is living: **amend it** when implementation choices differ from the draft.
- Before **closing** or **handing off** a story, skim the plan: open questions should be **resolved** or restated as follow-up tickets with pointers.
- During **implementation**, generate **`docs/manual-qa/{KEY}.md`** per **Manual QA checklist** above for any story with user-visible behavior; commit it on the story branch before handoff.
- Before **closing** or **handing off** a story, skim the plan: open questions should be **resolved** or restated as follow-up tickets with pointers, and the manual QA checklist should exist (or be explicitly declined in the plan).
- After **implementing** review feedback, **edit the corresponding `docs/reviews/…` file** with **strikethrough + Done** on the original bullets per **Code review follow-up** above—this step is easy to skip; treat it as **required** whenever suggestions were listed in that review.
This complements [story-kickoff](story-kickoff.md) (plan creation) and [code-review-agent](code-review-agent.md) (documentation checked vs plans and decomposition).

2
AGENTS.md
View File

@ -7,6 +7,6 @@ Optional **personas** for Cursor. Enable by **@ mentioning** the rule in chat or
| **Code review** | [`.cursor/rules/code-review-agent.md`](.cursor/rules/code-review-agent.md) | PR / diff / pre-merge review; **always writes** `docs/reviews/YYYY-MM-DD-{slug}.md` with **Documentation checked** vs `docs/plans/` and `docs/decomposition/modules/`; **when suggestions are fixed, strike through those bullets + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); short chat pointer |
| **Docs review** | [`.cursor/rules/docs-review-agent.md`](.cursor/rules/docs-review-agent.md) | Coherence / links / dev-guide fitness for `docs/` (especially `docs/game-design/` + decomposition); **writes** `docs/reviews/YYYY-MM-DD-{slug}.md`; **when suggestions are fixed, strike through + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); use when working in **documents** or @ mention |
Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** agents may **commit** at discretion on story/ticket work; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md); after merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **Linear:** ask which **state** to set (e.g. **In Test** vs **Done**), or wait for the user to say which in the same message, **before** updating the issue; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file).
Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Manual QA checklists** for user-visible ticketed work are generated during implementation at `docs/manual-qa/{KEY}.md` (same rule). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** agents may **commit** at discretion on story/ticket work; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md); after merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **Linear:** ask which **state** to set (e.g. **In Test** vs **Done**), or wait for the user to say which in the same message, **before** updating the issue; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file).
**Commits tied to a Linear issue** must start the subject with the **issue id** and a colon (e.g. `NEO-8: …`). Branch naming and exceptions (`chore:` when there is no ticket) — [`.cursor/rules/linear-git-naming.md`](.cursor/rules/linear-git-naming.md).

30
bruno/neon-sprawl-server/targeting/Post target select beta with hint.bru 100644
View File

@ -0,0 +1,30 @@
meta {
name: POST target select - prototype_target_beta (with positionHint)
type: http
seq: 13
}
post {
url: {{baseUrl}}/game/players/{{playerId}}/target/select
body: json
auth: none
}
headers {
content-type: application/json
}
body:json {
{
"schemaVersion": 1,
"targetId": "prototype_target_beta",
"positionHint": { "x": 3.0, "y": 0.5, "z": 5.0 }
}
}
docs {
NEO-24 follow-up #5: the optional `positionHint` lets the server run the radius check against
the client's live capsule position instead of the stored `move-stream` snap. Useful to verify
a "stopped near beta but store is stale" case without having to drive the capsule manually.
Server treats the hint as advisory — it is NOT written to the position store.
}

22
client/README.md
View File

@ -91,6 +91,28 @@ The main scene includes a **prototype terminal** at the map center (same world *
2. **F5** in Godot: default spawn is out of range of the terminal; markers should stay **dim**. **WASD** toward the center until markers **brighten** (within **3** m on the floor plane).
3. Press **E** (input action **`interact`** in `project.godot`): Output should show **`allowed=true`** when markers glow, **`allowed=false`** with **`reasonCode=out_of_range`** when dim (if you walk back out). Interaction uses **`_input`**, not `_unhandled_input`, so keys register reliably in the embedded **Game** dock; click the game view if the editor had focus elsewhere.
## Target lock + tab cycle (NEO-24)
`scripts/target_selection_client.gd` wires the server targeting API from [NEO-23](../docs/plans/NEO-23-implementation-plan.md) to a tab-cycle input and a HUD overlay.
- **Bindings** (in `project.godot`): **`target_tab`** → **Tab** cycles targets in the order defined by **`scripts/prototype_target_constants.gd`** (`prototype_target_alpha`, then `prototype_target_beta`; matches `PrototypeTargetRegistry.cs` ascending id), wrapping from last → first. When the player reference is wired (via `set_freshness_kick`, which `main.gd` does), Tab **skips client-computed out-of-range anchors** and selects the first in-range id in cycle order so pressing Tab while visibly standing next to an anchor actually locks it instead of trying the next-in-order id that happens to be across the map. Falls back to the plain cycle when nothing is in range so the server still picks the denial reason. **`target_clear`** → **Esc** POSTs with `targetId` omitted to clear the server lock.
- **HUD:** `UICanvas/TargetLockLabel` shows the last **server-acknowledged** state: `lockedTargetId` (or **`—`** when no lock), `validity` (`none` / `ok` / `out_of_range` / `invalid_target`), `sequence`, one **display-only** line per known target in the form `<id>: <dist> m / <radius> (in|out)` (computed client-side from the live capsule position against `prototype_target_constants.gd`), and (on denials) `Denied: <reasonCode>` on the last line. **UI never shows an optimistic lock the server denied** — it always paints from the authoritative `targetState` echoed by each `200` response. Distance lines are a lag diagnostic: if the HUD shows `Validity: ok` but `alpha: 7.1 m / 6.0 (out)`, the server's position snapshot is lagging the visible capsule, and the next `authoritative_ack` refresh will reconcile. `UICanvas/PlayerPositionLabel` also shows the last **server-acknowledged** world position + horizontal delta from the visible capsule — when that `Δ` is large, targeting denials are being evaluated against a stale server snapshot.
- **`positionHint` on select (NEO-24 follow-up #5):** when `TargetSelectionClient.set_freshness_kick(authority, player)` is wired (as `main.gd` does), every `POST /target/select` body also carries `positionHint: {x,y,z}` = the live capsule position. The server uses the hint for the radius check and for the `validity` field in the echoed `targetState`, eliminating the cross-request race between `move-stream` and `target/select` POSTs — a Tab pressed several seconds after stopping (with an old stored snap on the server) now still succeeds when you are visibly in range. The hint is advisory (server never writes it to the position store); `move-stream` remains the sole write path. Tests that omit `set_freshness_kick(...)` fall back to the no-hint code path.
- **Freshness kick (ancillary):** alongside the hint, `set_freshness_kick(...)` also has the target client nudge the authority with `submit_stream_targets([player.global_position])` before each select POST. That write is no longer load-bearing for denial correctness (the hint handles that), but it keeps the stored snap reasonably fresh so the movement-triggered validity GET that runs on the next `authoritative_ack` sees current data.
- **Visible anchors:** `scripts/prototype_target_markers.gd` (under `World/PrototypeTargetMarkers`) spawns a small colored mast + a flat translucent ring at every `ANCHORS` entry so the lock radius is visible in-world. All prototype targets share a single radius (`PrototypeTargetConstants.SHARED_LOCK_RADIUS` = `PrototypeTargetRegistry.SharedLockRadius` = **6 m**); anchors are placed at `(-3, 0.5, -3)` and `(3, 0, 3)` so the two rings overlap at origin (~3.5 m wide), letting you flip between targets with Tab without locomotion. The script and `prototype_target_constants.gd` mirror the server's `PrototypeTargetRegistry.cs` — change them together in the same commit so the markers never drift from server authority. Per-target radii will return with real combat design (E5.M1).
- **Soft lock refresh:** the client does **not** poll on a timer. It refreshes via `GET …/target` on boot, on every `POST …/target/select` response (body already carries `targetState`), and — while a lock is currently held — on **`authoritative_ack`** from `PositionAuthorityClient`, throttled to at most one GET per **250 ms**. `authoritative_ack` fires on every server-confirmed position (boot `GET` 200 **and** successful `move-stream` 200), so a held lock revalidates during normal WASD locomotion without re-introducing snap rubber-banding. Boot is naturally skipped because no lock exists yet. Purely server-driven state flips (future PvP / combat categories) would require revisiting this policy; today NEO-23 has none.
- **Inspector:** match **`base_url`** / **`dev_player_id`** on the `TargetSelectionClient` node to the other HTTP clients / `Game:DevPlayerId`.
### Manual check (NEO-24)
1. Run the server and client as in NEO-7 / NEO-9; confirm spawn snap at `(-5, 0.9, -5)`.
2. Press **Tab**: HUD → `Target: prototype_target_alpha`, `Validity: ok` (spawn is ~2.83 m from alpha's anchor `(-3, -3)`, well inside the shared 6 m radius).
3. Press **Tab** again: server denies (beta anchor `(3, 3)` is ~11.3 m from spawn, outside 6 m). The range-aware picker skips the currently-locked alpha and falls back to the cycle so the server gets a clean "swap to beta" request and can deny it; per NEO-23 soft-lock policy the persisted lock does not change, so HUD stays at `Target: prototype_target_alpha, Validity: ok` with `Denied: out_of_range` appended.
4. **WASD** away from spawn past ~6 m horizontally from alpha; HUD flips to `Validity: out_of_range` on the next movement-triggered refresh (≤250 ms cadence) while `lockedTargetId` stays `alpha` — this is the **soft lock** round-tripped from the server.
5. Walk to the **origin** (both rings visibly overlap): HUD shows `alpha` and `beta` distance lines both `(in)` (~4.24 m each). Press **Tab** to flip alpha → beta → alpha → … without moving. This is the main overlap sanity.
6. Walk toward beta (inside `(3, 3)` within 6 m, outside alpha's ring) and press **Tab** from a cleared lock: the range-aware picker skips alpha and locks beta directly. A further **Tab** from beta-only-in-range wraps to `alpha` and the server denies (same soft-lock behavior as step 3).
7. Press **Esc**: HUD → `Target: —`, `Validity: none`, sequence advances.
## Movement prototype (NEO-5 → NEO-11)
**`player.gd`** steers **XZ** toward the authoritative goal and uses **`NavigationAgent3D`**’s **current path** only for the **vertical-routing** case above (on sufficiently flat floor); **`move_and_slide()`** does the motion. **`snap_to_server()`** remains for **boot** (and would apply for any future hard reconcile).

10
client/project.godot
View File

@ -44,6 +44,16 @@ interact={
"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":69,"physical_keycode":69,"key_label":0,"unicode":101,"location":0,"echo":false,"script":null)
]
}
target_tab={
"deadzone": 0.5,
"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":4194306,"physical_keycode":4194306,"key_label":0,"unicode":0,"location":0,"echo":false,"script":null)
]
}
target_clear={
"deadzone": 0.5,
"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":4194305,"physical_keycode":4194305,"key_label":0,"unicode":0,"location":0,"echo":false,"script":null)
]
}
camera_zoom_in={
"deadzone": 0.5,
"events": [Object(InputEventMouseButton,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"button_mask":0,"position":Vector2(0, 0),"global_position":Vector2(0, 0),"factor":1.0,"button_index":4,"canceled":false,"pressed":false,"double_click":false,"script":null)

30
client/scenes/main.tscn
View File

@ -4,6 +4,8 @@
[ext_resource type="Script" uid="uid://1jimgt3d4bjj" path="res://scripts/player.gd" id="2_player"]
[ext_resource type="Script" uid="uid://ds5fkbscljkxi" path="res://scripts/position_authority_client.gd" id="4_auth"]
[ext_resource type="Script" uid="uid://bv0xprp660hib" path="res://scripts/interaction_request_client.gd" id="5_ix"]
[ext_resource type="Script" uid="uid://gh6yv5p1y0vq" path="res://scripts/target_selection_client.gd" id="11_tgt_sel"]
[ext_resource type="Script" uid="uid://crd46axw4gkjn" path="res://scripts/prototype_target_markers.gd" id="12_tgt_mk"]
[ext_resource type="Script" uid="uid://n3p4f3fky3nv" path="res://scripts/interaction_radius_indicators.gd" id="6_rad"]
[ext_resource type="Script" uid="uid://caaj3ohikfdx2" path="res://scripts/isometric_follow_camera.gd" id="7_iso_cam"]
[ext_resource type="Resource" path="res://resources/isometric_zoom_bands.tres" id="8_zoom_bands"]
@ -1069,6 +1071,9 @@ surface_material_override/0 = SubResource("Mat_physics_ramp_test")
[node name="CollisionShape3D" type="CollisionShape3D" parent="World/PhysicsRampTestSteepRamp" unique_id=4201032]
shape = SubResource("BoxShape3D_physics_ramp_test_steep")
[node name="PrototypeTargetMarkers" type="Node3D" parent="World" unique_id=1700010]
script = ExtResource("12_tgt_mk")
[node name="InteractionMarkers" type="Node3D" parent="World" unique_id=1700003]
script = ExtResource("6_rad")
@ -1111,6 +1116,9 @@ script = ExtResource("4_auth")
[node name="InteractionRequestClient" type="Node" parent="." unique_id=2500003]
script = ExtResource("5_ix")
[node name="TargetSelectionClient" type="Node" parent="." unique_id=2500004]
script = ExtResource("11_tgt_sel")
[node name="UICanvas" type="CanvasLayer" parent="." unique_id=9000001]
[node name="MoveRejectLabel" type="Label" parent="UICanvas" unique_id=9000002]
@ -1128,8 +1136,8 @@ autowrap_mode = 3
[node name="PlayerPositionLabel" type="Label" parent="UICanvas" unique_id=9000003]
offset_left = 8.0
offset_top = 8.0
offset_right = 220.0
offset_bottom = 72.0
offset_right = 360.0
offset_bottom = 106.0
grow_horizontal = 0
grow_vertical = 0
theme_override_colors/font_color = Color(0.92, 0.95, 0.98, 1)
@ -1138,4 +1146,20 @@ theme_override_constants/outline_size = 6
theme_override_font_sizes/font_size = 15
text = "x: —
y: —
z: —"
z: —
srv: —"
[node name="TargetLockLabel" type="Label" parent="UICanvas" unique_id=9000004]
offset_left = 8.0
offset_top = 122.0
offset_right = 360.0
offset_bottom = 306.0
grow_horizontal = 0
grow_vertical = 0
theme_override_colors/font_color = Color(0.86, 0.94, 1, 1)
theme_override_colors/font_outline_color = Color(0.08, 0.08, 0.1, 1)
theme_override_constants/outline_size = 6
theme_override_font_sizes/font_size = 15
text = "Target: —
Validity: —
Seq: —"

99
client/scripts/main.gd
View File

@ -37,9 +37,20 @@ const AUTH_SNAP_LOCOMOTION_SKIP_MAX_DH: float = 0.55
## Horizontal speed² above this (m²/s²) counts as “moving” for snap suppression (~0.35 m/s).
const AUTH_SNAP_LOCOMOTION_VEL_HZ_SQ: float = 0.12
const PrototypeTargetConstants := preload("res://scripts/prototype_target_constants.gd")
## Bump on each rejection so older one-shot timers do not clear a newer message.
var _move_reject_msg_token: int = 0
var _stream_physics_counter: int = 0
## Cached latest `target_state_changed` payload so the HUD can re-render with live
## distance readouts on every physics tick without missing server-side fields.
var _last_target_state: Dictionary = {}
## Last server-acknowledged world position (from `PositionAuthorityClient.authoritative_ack`;
## boot GET 200 or `move-stream` 200). Used purely to render the server-vs-client divergence
## line in the HUD so "target says denied but I'm obviously in range" becomes self-explanatory.
var _last_ack_world: Vector3 = Vector3.ZERO
var _have_last_ack: bool = false
var _last_ack_msec: int = 0
## After the first `apply_as_snap` authority update, micro-snaps may be skipped
## (boot always applies).
var _allow_authority_soft_snap_skip: bool = false
@ -62,6 +73,8 @@ var _dev_obstacle_smoke: Node3D
@onready var _radius_preview: Node3D = $World/InteractionMarkers
@onready var _move_reject_label: Label = $UICanvas/MoveRejectLabel
@onready var _player_pos_label: Label = $UICanvas/PlayerPositionLabel
@onready var _target_lock_label: Label = $UICanvas/TargetLockLabel
@onready var _target_client: Node = $TargetSelectionClient
@onready var _floor: StaticBody3D = $World/NavigationRegion3D/Floor
@ -81,7 +94,20 @@ func _ready() -> void:
"authoritative_position_received", Callable(self, "_on_authoritative_position")
)
_authority.connect("move_rejected", Callable(self, "_on_move_rejected"))
# NEO-24: movement-driven refresh — `TargetSelectionClient` no-ops on boot (no lock yet)
# and throttles bursts (see Decision 2 in NEO-24 plan). `authoritative_ack` fires on
# every server-confirmed position (boot GET 200 + `move-stream` 200) so a held lock is
# revalidated during ordinary WASD locomotion without re-introducing snap rubber-banding.
_authority.connect("authoritative_ack", Callable(_target_client, "on_authoritative_ack"))
_authority.connect("authoritative_ack", Callable(self, "_on_authoritative_ack_for_hud"))
_target_client.connect("target_state_changed", Callable(self, "_on_target_state_changed"))
# NEO-24 post-merge: tell the target client how to nudge the authority with the current
# capsule position before each POST /target/select. Shrinks the common "Tab right after
# stopping" false-denial race by making the server's stored snapshot as fresh as possible.
if _target_client.has_method("set_freshness_kick"):
_target_client.call("set_freshness_kick", _authority, _player)
_authority.call("sync_from_server")
_target_client.call("request_sync_from_server")
if _radius_preview.has_method("setup_player"):
_radius_preview.call("setup_player", _player)
@ -102,7 +128,25 @@ func _physics_process(_delta: float) -> void:
else:
_stream_physics_counter = 0
var p: Vector3 = _player.global_position
_player_pos_label.text = "x: %.3f\ny: %.3f\nz: %.3f" % [p.x, p.y, p.z]
_player_pos_label.text = _build_player_position_text(p)
_render_target_lock_label(p)
## HUD: client position on top, and (once a `move-stream` / boot GET has landed) the
## **server**-acknowledged position + horizontal delta underneath. When the server line
## is far from the client line the capsule has outrun the authoritative state — any
## targeting denials are being computed against the stale server position, and the next
## `move-stream` 200 (or the recovery resync on a 400) will reconcile.
func _build_player_position_text(p: Vector3) -> String:
var lines: PackedStringArray = ["x: %.3f" % p.x, "y: %.3f" % p.y, "z: %.3f" % p.z]
if _have_last_ack:
var s: Vector3 = _last_ack_world
var dh: float = Vector2(p.x - s.x, p.z - s.z).length()
var age_msec: int = max(0, Time.get_ticks_msec() - _last_ack_msec)
lines.append("srv: (%.2f, %.2f, %.2f) Δ=%.2fm age=%dms" % [s.x, s.y, s.z, dh, age_msec])
else:
lines.append("srv: — (no ack yet)")
return "\n".join(lines)
func _locomotion_wish_world_xz_from_camera() -> Vector3:
@ -146,6 +190,59 @@ func _on_authoritative_position(world: Vector3, apply_as_snap: bool) -> void:
_player.snap_to_server(world)
## NEO-24 diagnostic: cache the server's last authoritative position so the HUD can
## surface client-vs-server divergence. This is the same signal `TargetSelectionClient`
## hooks for movement-driven lock refresh — we do not snap the capsule to this value
## (that is `authoritative_position_received`'s job on boot + move rejection).
func _on_authoritative_ack_for_hud(world: Vector3) -> void:
_last_ack_world = world
_have_last_ack = true
_last_ack_msec = Time.get_ticks_msec()
## NEO-24: cache the last server-acknowledged target state; the label text is
## re-rendered every physics tick in [method _render_target_lock_label] so the
## live distance readouts stay in sync with the player capsule even while
## the server state is idle.
func _on_target_state_changed(state: Dictionary) -> void:
_last_target_state = state.duplicate()
if is_instance_valid(_player):
_render_target_lock_label(_player.global_position)
## Combines cached server state (`_last_target_state`) with per-anchor horizontal
## distance computed from the live capsule position. Distances are a **display-only**
## hint — the server remains authoritative for `validity`. If the HUD shows
## `Validity: ok` but `alpha: 10.3 m` (> 8 m radius), that means the server's
## position snapshot is lagging the visible capsule (NEO-23 `move-stream` 20 Hz +
## one-way latency); the refresh GET on `authoritative_ack` will reconcile within
## ~250 ms cooldown the next time a `move-stream` lands.
func _render_target_lock_label(world: Vector3) -> void:
if not is_instance_valid(_target_lock_label):
return
var state: Dictionary = _last_target_state
var locked_variant: Variant = state.get("lockedTargetId", null)
var locked_text: String = "—"
if locked_variant is String and not (locked_variant as String).is_empty():
locked_text = locked_variant as String
var validity: String = str(state.get("validity", "none"))
var sequence: int = int(state.get("sequence", 0))
var lines: PackedStringArray = [
"Target: %s" % locked_text,
"Validity: %s" % validity,
"Seq: %d" % sequence,
]
for id_variant: Variant in PrototypeTargetConstants.ordered_ids():
var tid: String = id_variant as String
var dist: float = PrototypeTargetConstants.horizontal_distance_to(tid, world)
var radius: float = PrototypeTargetConstants.anchor_radius(tid)
var marker: String = "in" if dist <= radius else "out"
lines.append("%s: %.2f m / %.1f (%s)" % [tid, dist, radius, marker])
if state.has("reasonCode"):
lines.append("Denied: %s" % str(state["reasonCode"]))
_target_lock_label.text = "\n".join(lines)
func _on_move_rejected(reason_code: String) -> void:
_authority_force_snap_next = true
# Rejected stream: server state may differ; next snap is forced.

58
client/scripts/position_authority_client.gd
View File

@ -3,10 +3,14 @@ extends Node
## `GET` position for boot snap; `submit_stream_targets` POSTs `move-stream` (WASD locomotion).
## NS-19: failed validation (HTTP 400 + rejection body) emits `move_rejected`.
## NS-23: second signal arg on `authoritative_position_received` — always `true` (boot snap only).
## A successful `move-stream` 200 is ack-only (no position signal; avoids RTT rubber-band).
## A successful `move-stream` 200 is ack-only (no snap signal; avoids RTT rubber-band).
## NEO-24: `authoritative_ack` fires on **every** server-confirmed position (BOOT_GET 200 **and**
## successful move-stream 200), giving target / cooldown-throttled systems a heartbeat during
## normal locomotion without re-introducing snap rubber-banding.
## (No `class_name` so headless/CI can load the project before `.godot` import exists.)
signal authoritative_position_received(world: Vector3, apply_as_snap: bool)
signal authoritative_ack(world: Vector3)
signal move_rejected(reason_code: String)
enum Phase { BOOT_GET, STREAM_POST }
@ -148,11 +152,12 @@ func _on_request_completed(
return
_stream_in_flight_count = 0
_busy = false
if response_code == 200:
# Do not emit `authoritative_position_received` here. The body echoes server position
# after applying the batch, which typically lags the client's integrated capsule by
# roughly one RTT — snapping caused visible rubber-banding during WASD.
pass
# Do not emit `authoritative_position_received` here. The body echoes server position
# after applying the batch, which typically lags the client's integrated capsule by
# roughly one RTT — snapping caused visible rubber-banding during WASD. We do emit
# `authoritative_ack` (NEO-24) so cooldown-throttled consumers like
# `TargetSelectionClient` get a heartbeat during normal locomotion.
_emit_ack_if_position_present(text)
_try_flush_pending()
@ -172,20 +177,47 @@ func _emit_move_rejection_if_present(json_text: String) -> void:
func _emit_position_from_response(json_text: String, apply_as_snap: bool) -> void:
var world_variant: Variant = _parse_world_from_response(json_text, true)
if world_variant == null:
return
var world: Vector3 = world_variant
authoritative_position_received.emit(world, apply_as_snap)
# NEO-24: boot / rejection-resync is also an authoritative ack for cooldown-throttled
# consumers. Emit both so subscribers do not have to dedupe across two code paths.
authoritative_ack.emit(world)
## NEO-24: `move-stream` 200 path — echo position is authoritative but intentionally NOT snapped
## (see rubber-band comment in `_on_request_completed`). Still emit `authoritative_ack` so
## `TargetSelectionClient` can run its throttled refresh during ordinary locomotion.
func _emit_ack_if_position_present(json_text: String) -> void:
var world_variant: Variant = _parse_world_from_response(json_text, false)
if world_variant == null:
return
authoritative_ack.emit(world_variant as Vector3)
## Shared `{"position":{"x","y","z"}}` parser. `warn_on_missing` lets BOOT_GET complain about
## malformed boot responses while STREAM_POST stays quiet (some 200 bodies may legitimately
## omit `position` in future response shapes; the ack then simply does not fire).
func _parse_world_from_response(json_text: String, warn_on_missing: bool) -> Variant:
var parsed: Variant = JSON.parse_string(json_text)
if parsed == null:
push_warning("PositionAuthorityClient: position response JSON parse failed")
return
if warn_on_missing:
push_warning("PositionAuthorityClient: position response JSON parse failed")
return null
if not parsed is Dictionary:
push_warning("PositionAuthorityClient: position response is not a JSON object")
return
if warn_on_missing:
push_warning("PositionAuthorityClient: position response is not a JSON object")
return null
var data: Dictionary = parsed
var pos_variant: Variant = data.get("position", null)
if not pos_variant is Dictionary:
push_warning("PositionAuthorityClient: position response missing `position` object")
return
if warn_on_missing:
push_warning("PositionAuthorityClient: position response missing `position` object")
return null
var pos: Dictionary = pos_variant
var x: float = float(pos.get("x", 0.0))
var y: float = float(pos.get("y", 0.0))
var z: float = float(pos.get("z", 0.0))
authoritative_position_received.emit(Vector3(x, y, z), apply_as_snap)
return Vector3(x, y, z)

133
client/scripts/prototype_target_constants.gd 100644
View File

@ -0,0 +1,133 @@
extends Object
## NEO-24: prototype combat target ids in **tab cycle order** — must match
## `server/NeonSprawl.Server/Game/Targeting/PrototypeTargetRegistry.cs`
## (`PrototypeTargetAlphaId`, `PrototypeTargetBetaId`; ascending id order).
## Anchors + radii are a **display-only** mirror of the C# registry so the scene markers
## (`prototype_target_markers.gd`) and HUD distance readouts match server-authoritative
## range decisions. If the server registry moves, update this file in the same commit —
## the two must stay in lock-step to avoid confusing "ok at 10 m / denied at 5 m" bugs.
## Static accessors only (no `class_name`; headless / CI loads before editor import — see client
## README Godot CLI section).
const ORDERED_IDS: Array[String] = ["prototype_target_alpha", "prototype_target_beta"]
## All prototype targets share a single radius (`PrototypeTargetRegistry.SharedLockRadius`
## on the server). Per-target radii will come back with real combat design (E5.M1).
const SHARED_LOCK_RADIUS: float = 6.0
## Keys match [constant ORDERED_IDS]; values mirror `PrototypeTargetRegistry.cs` anchors
## (world space) and horizontal lock radii (XZ meters). Anchors are placed so the two rings
## overlap at origin (~3.5 m wide), letting Tab flip between targets without locomotion.
## Keep in lock-step with the C# registry.
const ANCHORS: Dictionary = {
"prototype_target_alpha":
{
"position": Vector3(-3.0, 0.5, -3.0),
"radius": SHARED_LOCK_RADIUS,
"color": Color(1.0, 0.55, 0.2, 1.0),
},
"prototype_target_beta":
{
"position": Vector3(3.0, 0.0, 3.0),
"radius": SHARED_LOCK_RADIUS,
"color": Color(0.7, 0.45, 1.0, 1.0),
},
}
## Returns the world anchor position for [param target_id], or [code]Vector3.ZERO[/code]
## if the id is not in [constant ANCHORS].
static func anchor_position(target_id: String) -> Vector3:
var data: Variant = ANCHORS.get(target_id, null)
if not data is Dictionary:
return Vector3.ZERO
var pos: Variant = (data as Dictionary).get("position", Vector3.ZERO)
return pos if pos is Vector3 else Vector3.ZERO
## Horizontal (XZ) lock radius for [param target_id], or [code]0.0[/code] if unknown.
static func anchor_radius(target_id: String) -> float:
var data: Variant = ANCHORS.get(target_id, null)
if not data is Dictionary:
return 0.0
return float((data as Dictionary).get("radius", 0.0))
## Marker tint for [param target_id]; falls back to white if unknown.
static func anchor_color(target_id: String) -> Color:
var data: Variant = ANCHORS.get(target_id, null)
if not data is Dictionary:
return Color.WHITE
var c: Variant = (data as Dictionary).get("color", Color.WHITE)
return c if c is Color else Color.WHITE
## Horizontal (XZ) distance from [param world] to the anchor of [param target_id], or
## [code]INF[/code] if the id is unknown. Display-only helper — **not** authoritative for
## range decisions (server owns that; see `PlayerTargetStateReader.ComputeValidity`).
static func horizontal_distance_to(target_id: String, world: Vector3) -> float:
if not ANCHORS.has(target_id):
return INF
var anchor: Vector3 = anchor_position(target_id)
var dx: float = world.x - anchor.x
var dz: float = world.z - anchor.z
return sqrt(dx * dx + dz * dz)
static func ordered_ids() -> Array[String]:
return ORDERED_IDS.duplicate()
## Returns the next id in [constant ORDERED_IDS] that is **in range** of [param world]
## (client-side horizontal distance ≤ anchor radius) **and not equal to [param current]**,
## starting after [param current] and wrapping last → first. Returns [code]""[/code] when
## no such anchor is in range so callers can fall back to [method next_id_after] and let
## the server pick the denial reason (this preserves the "Tab on an out-of-reach swap =
## visible denial" UX from NEO-23's soft-lock rule).
##
## Client math here is a UX convenience (same purpose as the HUD distance lines): the
## server remains authoritative for the actual lock decision in
## `TargetingApi.MapPost(".../target/select")`. If the client says "out" but the server
## says "in" at ~1 radius (float noise / position drift), the request still flows through
## the fallback on the next tick — no state gets stuck.
static func next_in_range_id_after(current: Variant, world: Vector3) -> String:
if ORDERED_IDS.is_empty():
return ""
var current_str: String = ""
var start_idx: int = 0
if current is String:
current_str = (current as String).strip_edges()
if not current_str.is_empty():
var found: int = ORDERED_IDS.find(current_str)
if found >= 0:
start_idx = (found + 1) % ORDERED_IDS.size()
for i in range(ORDERED_IDS.size()):
var probe_idx: int = (start_idx + i) % ORDERED_IDS.size()
var probe_id: String = ORDERED_IDS[probe_idx]
if probe_id == current_str:
continue
var radius: float = anchor_radius(probe_id)
if radius <= 0.0:
continue
if horizontal_distance_to(probe_id, world) <= radius:
return probe_id
return ""
## Returns the next id in [constant ORDERED_IDS] after [param current]. Returns the **first** id
## when [param current] is [code]null[/code], empty, or not found (start of cycle). Wraps from
## last → first.
static func next_id_after(current: Variant) -> String:
if ORDERED_IDS.is_empty():
return ""
if not current is String:
return ORDERED_IDS[0]
var current_str: String = (current as String).strip_edges()
if current_str.is_empty():
return ORDERED_IDS[0]
var idx: int = ORDERED_IDS.find(current_str)
if idx < 0:
return ORDERED_IDS[0]
var next_idx: int = (idx + 1) % ORDERED_IDS.size()
return ORDERED_IDS[next_idx]

1
client/scripts/prototype_target_constants.gd.uid 100644
View File

@ -0,0 +1 @@
uid://bvcj2yn4v6nrh

109
client/scripts/prototype_target_markers.gd 100644
View File

@ -0,0 +1,109 @@
extends Node3D
## NEO-24 UX: the targeting registry anchors (`prototype_target_alpha`,
## `prototype_target_beta`) are invisible in the world, so "Tab says ok but I feel
## far away" is impossible to reason about without debug markers. This node spawns
## one small mast + a flat radius ring per id in
## `PrototypeTargetConstants.ANCHORS`, so the player can see exactly where each
## anchor sits and how big its lock radius is.
##
## Display-only; no game logic hangs off these nodes. If the server registry moves,
## update `prototype_target_constants.gd` — markers respawn on next scene load.
const PrototypeTargetConstants := preload("res://scripts/prototype_target_constants.gd")
## Visual tuning — keep markers unmistakable but not dominant.
const _MAST_HEIGHT: float = 2.5
const _MAST_RADIUS: float = 0.18
const _RING_RADIUS_SEGMENTS: int = 72
const _RING_Y: float = 0.05
const _RING_THICKNESS: float = 0.06
func _ready() -> void:
for id_variant: Variant in PrototypeTargetConstants.ANCHORS.keys():
if not id_variant is String:
continue
_spawn_marker(id_variant as String)
func _spawn_marker(target_id: String) -> void:
var anchor: Vector3 = PrototypeTargetConstants.anchor_position(target_id)
var radius: float = PrototypeTargetConstants.anchor_radius(target_id)
var tint: Color = PrototypeTargetConstants.anchor_color(target_id)
if radius <= 0.0:
return
var group := Node3D.new()
group.name = "Marker_%s" % target_id
group.position = anchor
add_child(group)
var mast := MeshInstance3D.new()
mast.name = "Mast"
var mast_mesh := CylinderMesh.new()
mast_mesh.top_radius = _MAST_RADIUS
mast_mesh.bottom_radius = _MAST_RADIUS
mast_mesh.height = _MAST_HEIGHT
mast.mesh = mast_mesh
# CylinderMesh is Y-centered at its origin; raise so the base sits on the anchor Y.
mast.position = Vector3(0.0, _MAST_HEIGHT * 0.5, 0.0)
mast.material_override = _make_unshaded_material(tint, 1.0)
group.add_child(mast)
var ring: MeshInstance3D = _make_radius_ring(radius, tint)
ring.name = "RadiusRing"
ring.position = Vector3(0.0, _RING_Y - anchor.y, 0.0)
group.add_child(ring)
func _make_unshaded_material(tint: Color, alpha: float) -> StandardMaterial3D:
var m := StandardMaterial3D.new()
m.shading_mode = BaseMaterial3D.SHADING_MODE_UNSHADED
var c := tint
c.a = alpha
m.albedo_color = c
if alpha < 0.999:
m.transparency = BaseMaterial3D.TRANSPARENCY_ALPHA
return m
## Builds a thin flat ring (annulus) by triangulating the band between `radius - t/2`
## and `radius + t/2`. `ImmediateMesh` with `SurfaceTool` would also work, but an
## `ArrayMesh` keeps this a static resource with no per-frame cost.
func _make_radius_ring(radius: float, tint: Color) -> MeshInstance3D:
var mi := MeshInstance3D.new()
var arr_mesh := ArrayMesh.new()
var verts := PackedVector3Array()
var normals := PackedVector3Array()
var indices := PackedInt32Array()
var inner: float = max(0.01, radius - _RING_THICKNESS * 0.5)
var outer: float = radius + _RING_THICKNESS * 0.5
var up := Vector3.UP
for i in range(_RING_RADIUS_SEGMENTS):
var a: float = TAU * float(i) / float(_RING_RADIUS_SEGMENTS)
var c: float = cos(a)
var s: float = sin(a)
verts.append(Vector3(inner * c, 0.0, inner * s))
verts.append(Vector3(outer * c, 0.0, outer * s))
normals.append(up)
normals.append(up)
for i in range(_RING_RADIUS_SEGMENTS):
var i_next: int = (i + 1) % _RING_RADIUS_SEGMENTS
var a0: int = i * 2
var a1: int = i * 2 + 1
var b0: int = i_next * 2
var b1: int = i_next * 2 + 1
indices.append_array([a0, a1, b1, a0, b1, b0])
var surface: Array = []
surface.resize(Mesh.ARRAY_MAX)
surface[Mesh.ARRAY_VERTEX] = verts
surface[Mesh.ARRAY_NORMAL] = normals
surface[Mesh.ARRAY_INDEX] = indices
arr_mesh.add_surface_from_arrays(Mesh.PRIMITIVE_TRIANGLES, surface)
mi.mesh = arr_mesh
mi.material_override = _make_unshaded_material(tint, 0.45)
return mi

1
client/scripts/prototype_target_markers.gd.uid 100644
View File

@ -0,0 +1 @@
uid://crd46axw4gkjn

287
client/scripts/target_selection_client.gd 100644
View File

@ -0,0 +1,287 @@
extends Node
## NEO-24 (E1M3-02): client tab-target + lock UI synced to server (NEO-23 `TargetState` v1).
##
## Flow:
## - `request_sync_from_server()` → `GET …/target` returns `PlayerTargetStateResponse` v1.
## - `request_tab_next()` picks the next id from `PrototypeTargetConstants.ORDERED_IDS`
## (first id if no lock) and POSTs `TargetSelectRequest` v1.
## - `request_select_target_id(id)` POSTs a specific id.
## - `request_clear_target()` POSTs with `targetId` omitted (NEO-23 clear rule).
## - Movement-triggered refresh: while a lock is held, `on_authoritative_ack(...)` fires a
## throttled `GET` so `validity` flips to `out_of_range` after locomotion without duplicating
## server radius math on the client (plan Decision 2). The ack signal fires on every
## server-confirmed position — both boot and successful `move-stream` — so a held lock is
## revalidated during normal WASD locomotion without re-introducing snap rubber-banding.
##
## Not an `HTTPRequest` subclass — tests inject a `Node` exposing `request()` +
## `request_completed` (same pattern as `position_authority_client.gd`).
signal target_state_changed(state: Dictionary)
enum Phase { IDLE, GET, POST_SELECT }
const PrototypeTargetConstants := preload("res://scripts/prototype_target_constants.gd")
## Cooldown between movement-triggered refresh GETs. Drops further snaps inside the window so
## one `move-stream` burst does not produce one GET per snap.
const _REFRESH_COOLDOWN_MSEC: int = 250
@export var base_url: String = "http://127.0.0.1:5253"
@export var dev_player_id: String = "dev-local-1"
var _http: Node
var _busy: bool = false
var _phase: Phase = Phase.IDLE
## Optional "freshness kick": before each `POST /target/select` we ask the authority to
## submit a `move-stream` with the current player position so the server's stored snapshot
## catches up with the visible capsule. Since NEO-24 follow-up #5 the `target/select` decision
## itself is made against the `positionHint` in the request body (race-free), so the kick is
## no longer load-bearing for denial correctness — its role is to keep the stored snap fresh
## for the movement-triggered validity GET that runs on the next `authoritative_ack`. Both
## refs are optional so tests that inject a plain script can still run without wiring.
var _freshness_authority: Node = null
var _freshness_player: Node3D = null
## Last acknowledged state from the server. Mirrors `PlayerTargetStateResponse` v1:
## `schemaVersion` (int), `playerId` (String), `lockedTargetId` (String or `null`),
## `validity` (String: `none` | `ok` | `out_of_range` | `invalid_target`),
## `sequence` (int). POST responses additionally carry `selectionApplied` (bool) and an optional
## `reasonCode` (String).
var _state: Dictionary = {}
## `-_REFRESH_COOLDOWN_MSEC` so the first snap is never blocked by the window on boot.
var _last_refresh_msec: int = -_REFRESH_COOLDOWN_MSEC
func _create_http_request() -> Node:
return HTTPRequest.new()
func _ready() -> void:
_http = _create_http_request()
add_child(_http)
if _http is HTTPRequest:
(_http as HTTPRequest).timeout = 30.0
@warning_ignore("unsafe_method_access")
_http.request_completed.connect(_on_request_completed)
## Matches `interaction_request_client.gd`: `_input` is more reliable than `_unhandled_input`
## inside the embedded Game dock / focus quirks. Tab is captured before UI focus navigation,
## which is fine while the prototype HUD has no focusable controls.
func _input(event: InputEvent) -> void:
if event.is_action_pressed("target_tab"):
request_tab_next()
elif event.is_action_pressed("target_clear"):
request_clear_target()
func request_sync_from_server() -> void:
if _busy:
return
_busy = true
_phase = Phase.GET
_last_refresh_msec = Time.get_ticks_msec()
var url := "%s/game/players/%s/target" % [_base_root(), _player_path_segment()]
var err: Error = _http.request(url)
if err != OK:
push_warning("TargetSelectionClient: GET failed to start (%s)" % err)
_busy = false
_phase = Phase.IDLE
func request_tab_next() -> void:
var current: Variant = _state.get("lockedTargetId", null)
var next_id: String = ""
# Prefer a client-side in-range pick when the player ref is wired so Tab locks the
# anchor the user is visibly standing next to instead of cycling to whatever happens
# to come first in `ORDERED_IDS`. Fall back to the plain cycle order (and let the
# server pick the denial reason) when nothing is in range or no player is wired.
if _freshness_player != null and is_instance_valid(_freshness_player):
next_id = PrototypeTargetConstants.next_in_range_id_after(
current, _freshness_player.global_position
)
if next_id.is_empty():
next_id = PrototypeTargetConstants.next_id_after(current)
if next_id.is_empty():
return
request_select_target_id(next_id)
func request_select_target_id(target_id: String) -> void:
var trimmed := target_id.strip_edges()
if trimmed.is_empty():
push_warning("TargetSelectionClient: refusing empty targetId; use request_clear_target()")
return
var payload: Dictionary = {"schemaVersion": 1, "targetId": trimmed}
_maybe_attach_position_hint(payload)
_post_select(payload)
func request_clear_target() -> void:
# NEO-23: omit `targetId` to clear. JSON `null` also works; omission keeps the body minimal.
_post_select({"schemaVersion": 1})
## NEO-24 follow-up #5: attach the live capsule position as an advisory `positionHint` so the
## server's range check runs against what the player sees *right now* instead of the last
## `move-stream` sample the server happens to have stored. Only populated when the player ref
## is wired (via [method set_freshness_kick]) — tests and other consumers that skip the wiring
## fall through to today's stored-snap behavior. Server remains authoritative (hint is advisory
## and does not write to the position store).
func _maybe_attach_position_hint(payload: Dictionary) -> void:
if _freshness_player == null or not is_instance_valid(_freshness_player):
return
var p: Vector3 = _freshness_player.global_position
payload["positionHint"] = {"x": p.x, "y": p.y, "z": p.z}
## Connected to `PositionAuthorityClient.authoritative_ack` in `main.gd`.
## Boot is naturally skipped because no lock is held yet; once a lock exists, subsequent acks
## (boot resync or `move-stream` 200) fire a refresh GET subject to a cooldown so bursts
## collapse to one GET per window.
func on_authoritative_ack(_world: Vector3) -> void:
if not _has_lock():
return
if _busy:
return
var now_msec: int = Time.get_ticks_msec()
if now_msec - _last_refresh_msec < _REFRESH_COOLDOWN_MSEC:
return
request_sync_from_server()
func cached_state() -> Dictionary:
return _state.duplicate()
## Wire the authority + player so each `POST /target/select` is preceded by a fresh
## `move-stream` submit (see `_freshness_authority`). Pass `null` for either to disable.
## Safe to call multiple times; the last wiring wins.
func set_freshness_kick(authority: Node, player: Node3D) -> void:
_freshness_authority = authority
_freshness_player = player
func _has_lock() -> bool:
if _state.is_empty():
return false
var v: Variant = _state.get("lockedTargetId", null)
return v is String and not (v as String).is_empty()
func _post_select(payload: Dictionary) -> void:
if _busy:
return
_kick_freshness_stream()
_busy = true
_phase = Phase.POST_SELECT
var url := "%s/game/players/%s/target/select" % [_base_root(), _player_path_segment()]
var body := JSON.stringify(payload)
var headers := PackedStringArray(["Content-Type: application/json"])
var err: Error = _http.request(url, headers, HTTPClient.METHOD_POST, body)
if err != OK:
push_warning("TargetSelectionClient: POST failed to start (%s)" % err)
_busy = false
_phase = Phase.IDLE
## Nudges the authority with the current capsule position so the server's stored snapshot
## catches up. Paired with the `positionHint` on the select POST (follow-up #5), this keeps
## subsequent validity GETs honest; the select itself no longer depends on which request
## arrives at the server first.
func _kick_freshness_stream() -> void:
if _freshness_authority == null or _freshness_player == null:
return
if not is_instance_valid(_freshness_authority) or not is_instance_valid(_freshness_player):
return
if not _freshness_authority.has_method("submit_stream_targets"):
return
_freshness_authority.call("submit_stream_targets", [_freshness_player.global_position])
func _base_root() -> String:
return base_url.strip_edges().rstrip("/")
func _player_path_segment() -> String:
# ASCII-safe ids (e.g. `dev-local-1`); percent-encode if ids gain reserved URL characters.
return dev_player_id.strip_edges()
func _on_request_completed(
_result: int, response_code: int, _headers: PackedStringArray, body: PackedByteArray
) -> void:
var phase: Phase = _phase
_busy = false
_phase = Phase.IDLE
if _result != HTTPRequest.RESULT_SUCCESS:
push_warning("TargetSelectionClient: HTTP failed (result=%s)" % _result)
return
var text := body.get_string_from_utf8()
var parsed: Variant = JSON.parse_string(text)
if not parsed is Dictionary:
if response_code != 200:
push_warning("TargetSelectionClient: HTTP %s non-JSON body" % response_code)
return
var data: Dictionary = parsed
match phase:
Phase.GET:
if response_code == 200:
_update_state_from_get(data)
Phase.POST_SELECT:
# Server includes authoritative `targetState` on 200 (apply and denial). 400 / 404
# bodies may lack it — swallow quietly; `_state` keeps last-good.
if response_code == 200:
_update_state_from_post(data)
_:
pass
func _update_state_from_get(data: Dictionary) -> void:
var state := _extract_target_state_fields(data)
if state.is_empty():
return
_state = state
target_state_changed.emit(_state.duplicate())
func _update_state_from_post(data: Dictionary) -> void:
var target_state_variant: Variant = data.get("targetState", null)
if not target_state_variant is Dictionary:
push_warning("TargetSelectionClient: POST 200 missing targetState")
return
var state := _extract_target_state_fields(target_state_variant as Dictionary)
if state.is_empty():
return
state["selectionApplied"] = bool(data.get("selectionApplied", false))
var reason_variant: Variant = data.get("reasonCode", null)
if reason_variant is String and not (reason_variant as String).is_empty():
state["reasonCode"] = reason_variant as String
_state = state
target_state_changed.emit(_state.duplicate())
## Shape-only parse of the v1 target state payload (works for GET body and POST echo).
## `lockedTargetId` is normalized to either a non-empty [String] or Variant [code]null[/code].
func _extract_target_state_fields(data: Dictionary) -> Dictionary:
if not data.has("validity"):
return {}
var out: Dictionary = {}
out["schemaVersion"] = int(data.get("schemaVersion", 1))
var pid_variant: Variant = data.get("playerId", "")
out["playerId"] = (pid_variant as String) if pid_variant is String else ""
var locked_variant: Variant = data.get("lockedTargetId", null)
if locked_variant is String and not (locked_variant as String).is_empty():
out["lockedTargetId"] = locked_variant as String
else:
out["lockedTargetId"] = null
var validity_variant: Variant = data.get("validity", "none")
out["validity"] = (validity_variant as String) if validity_variant is String else "none"
out["sequence"] = int(data.get("sequence", 0))
return out

1
client/scripts/target_selection_client.gd.uid 100644
View File

@ -0,0 +1 @@
uid://gh6yv5p1y0vq

28
client/test/position_authority_client_test.gd
View File

@ -65,6 +65,16 @@ func test_sync_boot_get_200_emits_snap() -> void:
assert_signal(c).is_emitted("authoritative_position_received", Vector3(1.0, 0.9, -5.0), true)
# NEO-24: boot sync also emits the cooldown-throttled heartbeat.
func test_sync_boot_get_200_emits_authoritative_ack() -> void:
var transport := MockHttpTransport.new()
transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"position":{"x":1,"y":0.9,"z":-5}}')
var c := _make_client(transport)
monitor_signals(c)
c.sync_from_server()
assert_signal(c).is_emitted("authoritative_ack", Vector3(1.0, 0.9, -5.0))
func test_sync_boot_get_200_malformed_position_does_not_emit() -> void:
var transport := MockHttpTransport.new()
transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"position":"invalid"}')
@ -118,3 +128,21 @@ func test_move_stream_post_200_does_not_emit_position() -> void:
monitor_signals(c)
c.submit_stream_targets([Vector3(-4.9, 0.9, -5.0)])
assert_signal(c).wait_until(400).is_not_emitted("authoritative_position_received")
# NEO-24: move-stream 200 must emit `authoritative_ack` so a held target lock can re-validate
# during normal locomotion (without re-introducing the snap rubber-band).
func test_move_stream_post_200_emits_authoritative_ack() -> void:
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS,
200,
(
'{"schemaVersion":1,"playerId":"dev-local-1","position":{"x":-4.9,"y":0.9,"z":-5},'
+ '"sequence":3}'
)
)
var c := _make_client(transport)
monitor_signals(c)
c.submit_stream_targets([Vector3(-4.9, 0.9, -5.0)])
assert_signal(c).is_emitted("authoritative_ack", Vector3(-4.9, 0.9, -5.0))

148
client/test/target_refresh_on_locomotion_test.gd 100644
View File

@ -0,0 +1,148 @@
extends GdUnitTestSuite
## NEO-24 integration test (code-review follow-up to the blocking issue):
##
## `main.gd` connects `PositionAuthorityClient.authoritative_ack` →
## `TargetSelectionClient.on_authoritative_ack`. The unit suites cover each side
## in isolation, but do not prove the two actually meet during normal locomotion
## (a successful `move-stream` POST, not a boot resync). This suite wires the
## **real** scripts together with mock HTTP transports and exercises that path
## end-to-end: with a lock held, a `move-stream` 200 must produce a throttled
## `GET /target` refresh on the target client's transport.
const PositionAuthorityTestDouble := preload("res://test/position_authority_test_double.gd")
const TargetSelectionTestDouble := preload("res://test/target_selection_test_double.gd")
const ALPHA_ID := "prototype_target_alpha"
# Mirrors the transports from the per-client suites: a queue-backed `Node` with a
# `request()` that synchronously emits `request_completed`. Counts requests so the
# test can assert exactly how many GETs reached the target-client transport.
class MockHttpTransport:
extends Node
signal request_completed(
result: int, response_code: int, headers: PackedStringArray, body: PackedByteArray
)
var request_count: int = 0
var last_url: String = ""
var last_method: HTTPClient.Method = HTTPClient.METHOD_GET
var _queue: Array[Dictionary] = []
func enqueue(result: int, code: int, body: String) -> void:
_queue.append({"result": result, "code": code, "body": body})
func request(
url: String,
_custom_headers: PackedStringArray = PackedStringArray(),
method: HTTPClient.Method = HTTPClient.METHOD_GET,
_request_data: String = ""
) -> Error:
request_count += 1
last_url = url
last_method = method
if _queue.is_empty():
return ERR_UNAVAILABLE
var r: Dictionary = _queue.pop_front()
var body_bytes: PackedByteArray = str(r["body"]).to_utf8_buffer()
request_completed.emit(r["result"], r["code"], PackedStringArray(), body_bytes)
return OK
func _make_authority(transport: Node) -> Node:
var a: Node = PositionAuthorityTestDouble.new()
a.set("injected_http", transport)
auto_free(transport)
auto_free(a)
add_child(a)
return a
func _make_target_client(transport: Node) -> Node:
var c: Node = TargetSelectionTestDouble.new()
c.set("injected_http", transport)
auto_free(transport)
auto_free(c)
add_child(c)
return c
func _select_response_json(locked_id: String, validity: String, sequence: int) -> String:
var state := (
'{"schemaVersion":1,"playerId":"dev-local-1","lockedTargetId":"%s",' % locked_id
+ '"validity":"%s","sequence":%d}' % [validity, sequence]
)
return '{"schemaVersion":1,"selectionApplied":true,"targetState":%s}' % state
func _target_state_json(locked_id: String, validity: String, sequence: int) -> String:
return (
'{"schemaVersion":1,"playerId":"dev-local-1","lockedTargetId":"%s",' % locked_id
+ '"validity":"%s","sequence":%d}' % [validity, sequence]
)
func test_move_stream_200_triggers_target_refresh_while_locked() -> void:
var auth_transport := MockHttpTransport.new()
var target_transport := MockHttpTransport.new()
# 1) Target client locks alpha via POST.
target_transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(ALPHA_ID, "ok", 1)
)
# 2) After the move-stream POST, the target client refreshes with GET.
target_transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _target_state_json(ALPHA_ID, "out_of_range", 1)
)
# 3) The authority's move-stream POST echoes position (triggers `authoritative_ack`).
auth_transport.enqueue(
HTTPRequest.RESULT_SUCCESS,
200,
(
'{"schemaVersion":1,"playerId":"dev-local-1","position":{"x":-20,"y":0.9,"z":-20},'
+ '"sequence":3}'
)
)
var authority := _make_authority(auth_transport)
var target_client := _make_target_client(target_transport)
# Same wiring as `main.gd` so this test fails loudly if someone renames the signal
# or handler without updating both ends.
authority.connect("authoritative_ack", Callable(target_client, "on_authoritative_ack"))
# Establish the lock first.
target_client.request_select_target_id(ALPHA_ID)
assert_that(target_client.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID)
var get_count_baseline: int = target_transport.request_count
# Simulate WASD locomotion: authority POSTs move-stream, server echoes 200.
authority.submit_stream_targets([Vector3(-20.0, 0.9, -20.0)])
# The target client must have issued exactly one refresh GET in response to the ack.
var new_requests: int = target_transport.request_count - get_count_baseline
assert_that(new_requests).is_equal(1)
assert_that(target_transport.last_url).contains("/game/players/dev-local-1/target")
assert_that(target_transport.last_method).is_equal(HTTPClient.METHOD_GET)
assert_that(target_client.cached_state().get("validity")).is_equal("out_of_range")
# Soft lock is preserved on out-of-range.
assert_that(target_client.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID)
func test_move_stream_200_does_not_refresh_without_lock() -> void:
var auth_transport := MockHttpTransport.new()
var target_transport := MockHttpTransport.new()
auth_transport.enqueue(
HTTPRequest.RESULT_SUCCESS,
200,
'{"schemaVersion":1,"playerId":"dev-local-1","position":{"x":0,"y":0.9,"z":0},"sequence":1}'
)
var authority := _make_authority(auth_transport)
var target_client := _make_target_client(target_transport)
authority.connect("authoritative_ack", Callable(target_client, "on_authoritative_ack"))
authority.submit_stream_targets([Vector3.ZERO])
assert_that(target_transport.request_count).is_equal(0)

1
client/test/target_refresh_on_locomotion_test.gd.uid 100644
View File

@ -0,0 +1 @@
uid://b3jgb0gf52s24

397
client/test/target_selection_client_test.gd 100644
View File

@ -0,0 +1,397 @@
extends GdUnitTestSuite
## NEO-24: GdUnit4 suite for `TargetSelectionClient`. Mirrors
## `position_authority_client_test.gd` — mock HTTP transport enqueues JSON responses and
## (optionally) tracks request count so the movement-triggered refresh cooldown is testable.
const TargetSelectionTestDouble := preload("res://test/target_selection_test_double.gd")
const ALPHA_ID := "prototype_target_alpha"
const BETA_ID := "prototype_target_beta"
# In-process transport: not an HTTPRequest subclass (Godot 4 will not call script `request()`
# on those). Counts requests so "no lock → no GET" and "cooldown collapses snaps" are testable.
class MockHttpTransport:
extends Node
signal request_completed(
result: int, response_code: int, headers: PackedStringArray, body: PackedByteArray
)
var request_count: int = 0
var last_url: String = ""
var last_body: String = ""
var last_method: HTTPClient.Method = HTTPClient.METHOD_GET
var _queue: Array[Dictionary] = []
func enqueue(result: int, code: int, body: String) -> void:
_queue.append({"result": result, "code": code, "body": body})
func request(
url: String,
_custom_headers: PackedStringArray = PackedStringArray(),
method: HTTPClient.Method = HTTPClient.METHOD_GET,
request_data: String = ""
) -> Error:
request_count += 1
last_url = url
last_method = method
last_body = request_data
if _queue.is_empty():
return ERR_UNAVAILABLE
var r: Dictionary = _queue.pop_front()
var body_bytes: PackedByteArray = str(r["body"]).to_utf8_buffer()
request_completed.emit(r["result"], r["code"], PackedStringArray(), body_bytes)
return OK
func _make_client(http_transport: Node) -> Node:
var c: Node = TargetSelectionTestDouble.new()
c.set("injected_http", http_transport)
auto_free(http_transport)
auto_free(c)
add_child(c)
return c
func _target_state_json(locked_id_or_null: Variant, validity: String, sequence: int = 0) -> String:
var locked := "null"
if locked_id_or_null is String and not (locked_id_or_null as String).is_empty():
locked = '"%s"' % (locked_id_or_null as String)
return (
'{"schemaVersion":1,"playerId":"dev-local-1","lockedTargetId":%s,' % locked
+ '"validity":"%s","sequence":%d}' % [validity, sequence]
)
func _select_response_json(
applied: bool,
locked_id_or_null: Variant,
validity: String,
sequence: int = 0,
reason: String = ""
) -> String:
var target_state := _target_state_json(locked_id_or_null, validity, sequence)
if applied:
return '{"schemaVersion":1,"selectionApplied":true,"targetState":%s}' % target_state
var reason_segment := ""
if not reason.is_empty():
reason_segment = ',"reasonCode":"%s"' % reason
return (
'{"schemaVersion":1,"selectionApplied":false%s,"targetState":%s}'
% [reason_segment, target_state]
)
func test_sync_get_200_emits_state() -> void:
var transport := MockHttpTransport.new()
transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, _target_state_json(null, "none"))
var c := _make_client(transport)
monitor_signals(c)
c.request_sync_from_server()
assert_signal(c).is_emitted("target_state_changed")
var state: Dictionary = c.cached_state()
assert_that(state.get("validity", "")).is_equal("none")
assert_that(state.get("lockedTargetId", "non-null-sentinel")).is_null()
func test_tab_from_no_lock_selects_first_id() -> void:
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
)
var c := _make_client(transport)
monitor_signals(c)
c.request_tab_next()
assert_that(transport.last_url).contains("/target/select")
assert_that(transport.last_method).is_equal(HTTPClient.METHOD_POST)
assert_that(transport.last_body).contains('"targetId":"%s"' % ALPHA_ID)
assert_signal(c).is_emitted("target_state_changed")
assert_that(c.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID)
func test_tab_from_alpha_requests_beta() -> void:
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
)
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, BETA_ID, "ok", 2)
)
var c := _make_client(transport)
c.request_tab_next()
c.request_tab_next()
assert_that(transport.last_body).contains('"targetId":"%s"' % BETA_ID)
assert_that(c.cached_state().get("lockedTargetId")).is_equal(BETA_ID)
func test_tab_wraps_from_beta_to_alpha() -> void:
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
)
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, BETA_ID, "ok", 2)
)
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 3)
)
var c := _make_client(transport)
c.request_tab_next()
c.request_tab_next()
c.request_tab_next()
assert_that(transport.last_body).contains('"targetId":"%s"' % ALPHA_ID)
assert_that(c.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID)
func test_denial_reflects_authoritative_target_state() -> void:
var transport := MockHttpTransport.new()
# Server denies: selectionApplied=false, lockedTargetId=null, reasonCode=out_of_range.
transport.enqueue(
HTTPRequest.RESULT_SUCCESS,
200,
_select_response_json(false, null, "none", 0, "out_of_range")
)
var c := _make_client(transport)
c.request_select_target_id(ALPHA_ID)
var state: Dictionary = c.cached_state()
assert_that(bool(state.get("selectionApplied", true))).is_false()
assert_that(state.get("reasonCode", "")).is_equal("out_of_range")
assert_that(state.get("lockedTargetId", "sentinel")).is_null()
func test_clear_issues_post_without_target_id() -> void:
var transport := MockHttpTransport.new()
transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, null, "none", 4))
var c := _make_client(transport)
c.request_clear_target()
assert_that(transport.last_url).contains("/target/select")
assert_that(transport.last_method).is_equal(HTTPClient.METHOD_POST)
assert_that(transport.last_body).not_contains("targetId")
func test_authoritative_ack_while_locked_fires_refresh_get() -> void:
var transport := MockHttpTransport.new()
# Step 1: lock alpha via POST.
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
)
# Step 2: movement-triggered GET shows out_of_range (soft lock).
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _target_state_json(ALPHA_ID, "out_of_range", 1)
)
var c := _make_client(transport)
c.request_select_target_id(ALPHA_ID)
var count_before_ack: int = transport.request_count
c.on_authoritative_ack(Vector3.ZERO)
assert_that(transport.request_count).is_equal(count_before_ack + 1)
assert_that(c.cached_state().get("validity")).is_equal("out_of_range")
assert_that(c.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID)
func test_authoritative_ack_without_lock_fires_nothing() -> void:
var transport := MockHttpTransport.new()
var c := _make_client(transport)
c.on_authoritative_ack(Vector3.ZERO)
assert_that(transport.request_count).is_equal(0)
func test_two_acks_within_cooldown_collapse_to_one_get() -> void:
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
)
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _target_state_json(ALPHA_ID, "out_of_range", 1)
)
var c := _make_client(transport)
c.request_select_target_id(ALPHA_ID)
var count_before: int = transport.request_count
c.on_authoritative_ack(Vector3.ZERO)
c.on_authoritative_ack(Vector3.ZERO)
assert_that(transport.request_count).is_equal(count_before + 1)
# Authority stub that captures the last `submit_stream_targets` batch so we can verify
# `TargetSelectionClient._post_select` kicks a freshness stream before POSTing.
class FreshnessAuthorityStub:
extends Node
var last_batch: Array = []
var submit_count: int = 0
func submit_stream_targets(targets: Array) -> void:
submit_count += 1
last_batch = targets.duplicate()
func _make_player(pos: Vector3) -> Node3D:
var n := Node3D.new()
n.global_position = pos
auto_free(n)
add_child(n)
return n
func test_select_post_kicks_freshness_stream_before_posting() -> void:
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
)
var c := _make_client(transport)
var authority := FreshnessAuthorityStub.new()
auto_free(authority)
add_child(authority)
var player := _make_player(Vector3(1.5, 0.5, -2.25))
c.set_freshness_kick(authority, player)
c.request_select_target_id(ALPHA_ID)
# Exactly one freshness submit, matching the pre-POST capsule position.
assert_that(authority.submit_count).is_equal(1)
assert_that(authority.last_batch.size()).is_equal(1)
assert_that(authority.last_batch[0]).is_equal(Vector3(1.5, 0.5, -2.25))
# And the select POST still went out as usual.
assert_that(transport.last_url).contains("/target/select")
func test_select_post_includes_position_hint_when_player_wired() -> void:
# NEO-24 follow-up #5: the select POST body must carry the live capsule position as
# `positionHint` so the server's range check can bypass any stale stored snap.
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, BETA_ID, "ok", 1)
)
var c := _make_client(transport)
var authority := FreshnessAuthorityStub.new()
auto_free(authority)
add_child(authority)
# Values chosen so `JSON.stringify` emits exact decimal representations (no float precision
# drift): 3.0 → "3", 0.5 → "0.5", 5.0 → "5". Re-parse the JSON for precise assertions.
var player := _make_player(Vector3(3.0, 0.5, 5.0))
c.set_freshness_kick(authority, player)
c.request_select_target_id(BETA_ID)
var parsed: Variant = JSON.parse_string(transport.last_body)
assert_that(parsed is Dictionary).is_true()
var body: Dictionary = parsed
assert_that(body.get("targetId")).is_equal(BETA_ID)
assert_that(body.has("positionHint")).is_true()
var hint: Dictionary = body["positionHint"]
assert_float(hint.get("x")).is_equal_approx(3.0, 0.0001)
assert_float(hint.get("y")).is_equal_approx(0.5, 0.0001)
assert_float(hint.get("z")).is_equal_approx(5.0, 0.0001)
func test_select_post_without_freshness_wiring_is_a_noop() -> void:
# Regression: tests that construct the client without `set_freshness_kick(...)` must
# still POST normally, *without* a `positionHint` (hint is opt-in per wiring).
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
)
var c := _make_client(transport)
c.request_select_target_id(ALPHA_ID)
assert_that(transport.last_url).contains("/target/select")
# No `positionHint` key in the body when no player is wired — re-parse so the assertion
# is robust against JSON whitespace / key ordering.
var parsed: Variant = JSON.parse_string(transport.last_body)
assert_that(parsed is Dictionary).is_true()
assert_that((parsed as Dictionary).has("positionHint")).is_false()
assert_that(c.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID)
func test_tab_from_no_lock_skips_out_of_range_and_picks_beta() -> void:
# Player stands next to beta (well outside alpha's ring). Without the range-aware
# tab cycle the server denies with `out_of_range` on alpha — see NEO-24 follow-up #3.
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, BETA_ID, "ok", 1)
)
var c := _make_client(transport)
var authority := FreshnessAuthorityStub.new()
auto_free(authority)
add_child(authority)
# Near beta anchor (3, 3), ~1.4 m horizontal — inside 6 m. Far from alpha at (-3, -3).
var player := _make_player(Vector3(4.0, 0.5, 4.0))
c.set_freshness_kick(authority, player)
c.request_tab_next()
assert_that(transport.last_body).contains('"targetId":"%s"' % BETA_ID)
assert_that(c.cached_state().get("lockedTargetId")).is_equal(BETA_ID)
func test_tab_from_no_lock_with_nothing_in_range_falls_back_to_first_id() -> void:
# No anchors are in range → cycle-order fallback. Server owns the denial reason.
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS,
200,
_select_response_json(false, null, "none", 0, "out_of_range")
)
var c := _make_client(transport)
var authority := FreshnessAuthorityStub.new()
auto_free(authority)
add_child(authority)
var player := _make_player(Vector3(20.0, 0.5, 20.0))
c.set_freshness_kick(authority, player)
c.request_tab_next()
assert_that(transport.last_body).contains('"targetId":"%s"' % ALPHA_ID)
assert_that(c.cached_state().get("reasonCode", "")).is_equal("out_of_range")
func test_tab_from_locked_alpha_at_origin_swaps_to_beta_when_both_in_range() -> void:
# With alpha (-3,-3) and beta (3,3) sharing r=6, origin is in both rings. Locked to
# alpha, Tab should request beta (normal cycle behavior; range-aware skip-current
# does not interfere when the next id is also in range).
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
)
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, BETA_ID, "ok", 2)
)
var c := _make_client(transport)
var authority := FreshnessAuthorityStub.new()
auto_free(authority)
add_child(authority)
var player := _make_player(Vector3(0.0, 0.5, 0.0))
c.set_freshness_kick(authority, player)
c.request_tab_next()
c.request_tab_next()
assert_that(transport.last_body).contains('"targetId":"%s"' % BETA_ID)
assert_that(c.cached_state().get("lockedTargetId")).is_equal(BETA_ID)
func test_tab_when_only_current_lock_is_in_range_falls_back_to_cycle_for_denial() -> void:
# Locked to alpha, only alpha is in range — Tab should express a *swap* intent and
# fall through to the plain cycle so the server denies with `out_of_range` (NEO-23
# soft-lock rule). If the picker returned alpha instead, the user would see no
# visible reaction to Tab.
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
)
transport.enqueue(
HTTPRequest.RESULT_SUCCESS,
200,
_select_response_json(false, ALPHA_ID, "ok", 1, "out_of_range")
)
var c := _make_client(transport)
var authority := FreshnessAuthorityStub.new()
auto_free(authority)
add_child(authority)
# Near alpha (-3, -3), ~1.4 m in — beta (3, 3) is ~8.5 m out.
var player := _make_player(Vector3(-2.0, 0.5, -2.0))
c.set_freshness_kick(authority, player)
c.request_select_target_id(ALPHA_ID)
c.request_tab_next()
assert_that(transport.last_body).contains('"targetId":"%s"' % BETA_ID)
assert_that(c.cached_state().get("reasonCode", "")).is_equal("out_of_range")
assert_that(c.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID)
func test_tab_without_player_ref_still_uses_plain_cycle_order() -> void:
# Regression: headless unit tests and any client that skips `set_freshness_kick(...)`
# must keep the pre-existing cycle-order semantics. Alpha is requested from no-lock.
var transport := MockHttpTransport.new()
transport.enqueue(
HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1)
)
var c := _make_client(transport)
c.request_tab_next()
assert_that(transport.last_body).contains('"targetId":"%s"' % ALPHA_ID)

1
client/test/target_selection_client_test.gd.uid 100644
View File

@ -0,0 +1 @@
uid://csqopvd66fsp0

9
client/test/target_selection_test_double.gd 100644
View File

@ -0,0 +1,9 @@
extends "res://scripts/target_selection_client.gd"
## Test-only: inject transport node before `add_child` (NEO-24; mirrors
## `position_authority_test_double.gd`).
var injected_http: Node
func _create_http_request() -> Node:
return injected_http

1
client/test/target_selection_test_double.gd.uid 100644
View File

@ -0,0 +1 @@
uid://bq6776s8atb8

2
docs/decomposition/modules/documentation_and_implementation_alignment.md
View File

@ -48,7 +48,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not
|--------|-----------------|----------|-------------------|
| E1.M1 | Ready | Prototype milestone **Done** ([E1.M1](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4)). Authoritative `PositionState` + **MoveCommand** over HTTP (JSON v1 snap); **in-memory** store by default, **Postgres** when configured ([NEO-8](../../plans/NEO-8-implementation-plan.md)); shared **NpgsqlDataSource** disposed on host shutdown ([NEO-13](../../plans/NEO-13-implementation-plan.md)); Godot sync + path-follow ([NEO-7](../../plans/NEO-7-implementation-plan.md), [NEO-11](../../plans/NEO-11-implementation-plan.md)); **InteractionRequest** + horizontal range ([NEO-9](../../plans/NEO-9-implementation-plan.md)). Follow-on: prediction/reconciliation, Slice 1 telemetry, Protobuf wire per [contracts.md](contracts.md). | [NEO-6](../../plans/NEO-6-implementation-plan.md), [NEO-7](../../plans/NEO-7-implementation-plan.md), [NEO-8](../../plans/NEO-8-implementation-plan.md), [NEO-9](../../plans/NEO-9-implementation-plan.md), [NEO-10](../../plans/NEO-10-implementation-plan.md), [NEO-11](../../plans/NEO-11-implementation-plan.md), [NEO-13](../../plans/NEO-13-implementation-plan.md); `server/NeonSprawl.Server/Game/PositionState/`, `Game/Interaction/`; [server README](../../../server/README.md) |
| E1.M2 | Ready | **Slice 2 prototype slice closed:** follow + `CameraState` ([NEO-15](https://linear.app/neon-sprawl/issue/NEO-15)); zoom bands ([NEO-16](https://linear.app/neon-sprawl/issue/NEO-16)); occlusion ([NEO-17](https://linear.app/neon-sprawl/issue/NEO-17)); occluder pick-through ([NEO-20](https://linear.app/neon-sprawl/issue/NEO-20)); contracts + hardening ([NEO-18](https://linear.app/neon-sprawl/issue/NEO-18)). Client-local; no server use of camera pose. | [NEO-15](../../plans/NEO-15-implementation-plan.md), [NEO-16](../../plans/NEO-16-implementation-plan.md), [NEO-17](../../plans/NEO-17-implementation-plan.md), [NEO-18](../../plans/NEO-18-implementation-plan.md), [NEO-20](../../plans/NEO-20-implementation-plan.md); `client/scripts/isometric_follow_camera.gd`, `camera_state.gd`, `zoom_band_config.gd`, `occlusion_policy.gd`, `ground_pick.gd`; [E1_M2_IsometricCameraController.md](E1_M2_IsometricCameraController.md) |
| E1.M3 | In Progress | **NEO-23 landed:** JSON v1 targeting read + select (`GET`/`POST …/target`, `Game/Targeting/`, [NEO-23](../../plans/NEO-23-implementation-plan.md)) — `PlayerTargetStateResponse` / soft lock / `reasonCode` denials; wire name differs from illustrative **`TargetState`** in module doc (called out in plan + README). **`InteractableDescriptor`**, **`SelectionEvent`**, and client tab-target ([NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)) still open. **Precursor (E1.M1):** [NEO-9](../../plans/NEO-9-implementation-plan.md) `POST …/interact`. | Linear [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)–[NEO-27](https://linear.app/neon-sprawl/issue/NEO-27); [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md); [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md); [server README — Targeting](../../../server/README.md#targeting-neo-23), [Interaction](../../../server/README.md#interaction-neo-9) |
| E1.M3 | In Progress | **NEO-23 landed:** JSON v1 targeting read + select (`GET`/`POST …/target`, `Game/Targeting/`, [NEO-23](../../plans/NEO-23-implementation-plan.md)) — `PlayerTargetStateResponse` / soft lock / `reasonCode` denials; wire name differs from illustrative **`TargetState`** in module doc (called out in plan + README). **NEO-24 landed:** client tab-target + lock HUD synced to server ([NEO-24](../../plans/NEO-24-implementation-plan.md)) — `TargetSelectionClient` ([`client/scripts/target_selection_client.gd`](../../../client/scripts/target_selection_client.gd)), Tab / Esc bindings, `TargetLockLabel` HUD, hybrid movement-triggered refresh via new `PositionAuthorityClient.authoritative_ack` signal (fires on every server-confirmed position, boot + `move-stream` 200) with a 250 ms cooldown; manual QA in [`docs/manual-qa/NEO-24.md`](../../manual-qa/NEO-24.md). **`InteractableDescriptor`** and **`SelectionEvent`** still open. **Precursor (E1.M1):** [NEO-9](../../plans/NEO-9-implementation-plan.md) `POST …/interact`. | Linear [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)–[NEO-27](https://linear.app/neon-sprawl/issue/NEO-27); [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md); [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md); [server README — Targeting](../../../server/README.md#targeting-neo-23), [Interaction](../../../server/README.md#interaction-neo-9) |
---

121
docs/manual-qa/NEO-24.md 100644
View File

@ -0,0 +1,121 @@
# NEO-24 — Manual QA checklist
| Field | Value |
|--------|--------|
| **Key** | NEO-24 |
| **Title** | E1.M3: Client tab-target + lock UI synced to server |
| **Linear** | [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24/e1m3-client-tab-target-lock-ui-synced-to-server) |
| **Plan** | [`docs/plans/NEO-24-implementation-plan.md`](../plans/NEO-24-implementation-plan.md) |
| **Branch** | `NEO-24-e1m3-client-tab-target-lock-ui-synced-to-server` |
Pair the client (Godot **F5**) with the server (`cd server/NeonSprawl.Server && dotnet run`). Keep the server console visible so you can see route hits, and the Godot **Output** dock visible for client warnings.
## 0. Setup sanity
- [ ] Server prints a URL around `http://localhost:5253` on boot; `GET /health` returns JSON.
- [ ] In Godot, select `TargetSelectionClient` in the scene tree: **`base_url`** matches the server URL and **`dev_player_id`** matches `Game:DevPlayerId` (default `dev-local-1`).
- [ ] (Optional) Bruno collection `bruno/neon-sprawl-server/targeting/` is configured for the same port — use it to cross-check server state after client actions.
## 1. Boot state
- [ ] On run, the player snaps to spawn (`-5, 0.9, -5`) — existing NEO-7 behavior, unchanged.
- [ ] `UICanvas/TargetLockLabel` (top-left, under `PlayerPositionLabel`) shows:
- `Target: —`
- `Validity: none`
- `Seq: 0`
- `prototype_target_alpha: 2.83 m / 6.0 (in)` (spawn `(-5,-5)` is ~2.83 m from alpha anchor `(-3,-3)`)
- `prototype_target_beta: 11.31 m / 6.0 (out)`
- [ ] `UICanvas/PlayerPositionLabel` shows an `srv: (x, y, z) Δ=<d>m age=<ms>` line within ~1 s of boot (boot `GET /position` populates it). At rest the `Δ` should be near 0.
- [ ] **Visible anchors** are rendered in-world with a **shared 6 m radius**: an **orange** mast + flat ring at `(-3, 0.5, -3)` (alpha) and a **violet** mast + ring at `(3, 0, 3)` (beta). Both rings lie flat on the floor and **visibly overlap** in a band of ~3.5 m centered at origin. The player capsule should sit inside the alpha ring (only) at spawn.
- [ ] Godot **Output** has no `TargetSelectionClient:` warnings. Server console shows a single `GET /game/players/dev-local-1/target` from the boot sync.
## 2. Tab cycle — happy path
- [ ] Press **Tab**. HUD flips to:
- `Target: prototype_target_alpha`
- `Validity: ok`
- `Seq: 1`
- [ ] Server log shows `POST /game/players/dev-local-1/target/select` with `{"schemaVersion":1,"targetId":"prototype_target_alpha"}`.
- [ ] No `Denied:` line is appended to the HUD (success ⇒ no `reasonCode`).
## 3. Tab to out-of-range target — soft lock preserved
- [ ] With alpha locked at spawn, press **Tab** again. The server denies the swap to beta (spawn is ~11.3 m from `(3, 3)`; shared radius is 6 m). Because the range-aware picker **skips the currently-locked id**, Tab falls back to the plain cycle (→ beta), the server rejects, and the soft lock is preserved.
- [ ] HUD **stays** at `Target: prototype_target_alpha`, `Validity: ok`, `Seq: 1` with **`Denied: out_of_range`** appended. That is the NEO-23 soft-lock rule: a failed swap does not change the persisted lock or its sequence.
- [ ] Server log shows the denial: `POST` → `200` with `selectionApplied: false`.
- [ ] Press **Tab** several more times from here. Each press cleanly denies; HUD never drifts or shows a `beta` / optimistic lock between responses.
## 4. Soft-lock out-of-range via locomotion (plan Decision 2)
- [ ] Still locked to alpha, **WASD** away from spawn until you are ~6 m horizontally from `(-3, -3)` (easiest route: head toward the -x/-z side of the map away from both rings). Within ~250 ms of the next `move-stream` 200 (each successful POST emits `authoritative_ack` → throttled target `GET`), HUD flips to:
- `Target: prototype_target_alpha` (unchanged — soft lock)
- `Validity: out_of_range`
- `Seq: 1` (unchanged — no lock id change)
- [ ] Walk back inside the alpha ring. HUD returns to `Validity: ok` within ~250 ms of the next `move-stream` ack.
- [ ] While walking, Godot Output is **not** spammed with targeting GETs. Server log shows no more than roughly one targeting `GET` per 250 ms during sustained motion, even though `move-stream` POSTs fire at ~20 Hz.
- [ ] Stand still with no lock changes. Server log shows **no** targeting GETs at idle (no `move-stream` is sent while idle, so there is no ack to trigger a refresh).
- [ ] **Distance lines match the ring**: while moving, the `prototype_target_alpha: <d> m / 6.0 (in|out)` line flips `in`/`out` as you cross the visible orange ring edge. Any time the HUD shows `Validity: ok` but the distance line reads `(out)`, the server position snapshot is lagging the visible capsule — the next `authoritative_ack` refresh should reconcile within 250 ms.
## 5. Tab-flip at origin (overlap zone) — shared-radius sanity
- [ ] WASD toward the **origin** `(0, 0)`. Both rings overlap there; the HUD should show **both** distance lines with `(in)` (each ~4.24 m / 6.0).
- [ ] Press **Tab** from no-lock (if Esc'd first): HUD flips to `Target: prototype_target_alpha`, `Validity: ok`, `Seq` +1. Cycle-first.
- [ ] Press **Tab** again: HUD flips to `Target: prototype_target_beta`, `Validity: ok`, `Seq` +1. The range-aware picker returns beta (next in cycle, still in range).
- [ ] Press **Tab** a third time: HUD flips back to `Target: prototype_target_alpha`. This is the core "flip between targets without moving" sanity — if it does **not** flip cleanly, check the server log; you should see a successful `POST /target/select` per press.
- [ ] **Walk slightly off-center** so only one ring covers you (e.g. toward alpha's anchor): distance lines flip `(in)` / `(out)` accordingly, and Tab swaps behave like Section 3 (denial on the out-of-range direction, preserved soft lock).
## 6. Swap to beta when in range (via walking, not the overlap)
- [ ] WASD from spawn straight toward beta at **(3, 3)** until the capsule is inside the violet ring (≤6 m). Press **Tab**. HUD swaps cleanly:
- `Target: prototype_target_beta`
- `Validity: ok`
- `Seq` increments by 1
- [ ] Press **Tab** once more from deep inside beta (alpha out of range). Server denies — HUD stays at `beta / ok` with `Denied: out_of_range` appended (same soft-lock UX as Section 3, other direction).
- [ ] **Range-aware cycle sanity:** press **Esc** to clear the lock (`Target: —`), then — while still inside beta's ring but outside alpha's — press **Tab**. The client skips alpha (out of range) and locks **beta** directly (`Target: prototype_target_beta`, `Validity: ok`). A plain cycle would pick alpha first and get denied with `out_of_range`; if that happens, the range-aware pick in `PrototypeTargetConstants.next_in_range_id_after` is not running (e.g. `set_freshness_kick` not wired).
- [ ] **`positionHint` sanity (NEO-24 follow-up #5):** walk to the edge of beta's ring, stop, and **immediately** press Tab. The select POST should succeed (`Validity: ok`). The client now attaches `positionHint: {x,y,z}` to the `target/select` body, so the server runs the radius check against the live capsule position instead of the stored `move-stream` snap — even when `PlayerPositionLabel`'s `age=…` is several seconds old or `Δ>0`. Denials here would mean the hint is not being attached (confirm `set_freshness_kick` is wired in `main.gd`) or the client's sense of "in range" genuinely disagrees with the server's radius math (check the per-target distance lines).
- [ ] **Soft-lock roam regression (follow-up #5 repro):** lock alpha at spawn, walk out of alpha's ring and into beta's ring (HUD shows `Validity: out_of_range`, beta distance `< 6 m (in)`), **stop**, wait until `age=…` climbs past ~3 s, and press **Tab**. Select swaps to beta successfully (`Target: prototype_target_beta`, `Validity: ok`). Regression before follow-up #5: this path denied with `out_of_range` because the select POST raced the freshness kick's `move-stream` POST on separate HTTP connections.
## 7. Clear
- [ ] Press **Esc** at any point with a lock held. HUD flips to:
- `Target: —`
- `Validity: none`
- `Seq` increments by 1
- [ ] Any `Denied: …` line from the previous state is gone.
- [ ] Server log shows `POST /target/select` with body `{"schemaVersion":1}` (no `targetId` key).
- [ ] Press **Esc** again with no lock. Server returns the same cleared state; HUD stays `—` / `none` (server may or may not bump sequence — whatever it returns is what the HUD shows).
## 8. Denial UI hygiene
- [ ] Press **Tab** from `—` while far from both targets. If the server denies (possible depending on exact spawn distance), HUD shows `Target: —`, `Validity: none`, `Denied: out_of_range`. The lock id does **not** get "stuck" at something the server refused.
- [ ] After any successful Tab / Esc, the `Denied: …` line goes away on the next response.
## 9. Input behavior
- [ ] **Tab** does **not** make focus jump around UI during play (we intercept in `_input` before UI focus navigation).
- [ ] **Esc** does not quit or minimize the game.
- [ ] Holding **Tab** does not auto-repeat into a firehose of POSTs (pressed action only fires on key press, not echo).
- [ ] **E** (NEO-9 interact) and **WASD** (NEO-22 move) still work independently of the target lock; having a lock does not block interact or movement.
## 10. Coexistence with movement rejection (NEO-19 / move-stream 400)
- [ ] Walk at the orange `MoveRejectPedestal` (~(7.5, −6.5)) while holding a lock. When `move-stream` returns **400** (`vertical_step_exceeded`), `PositionAuthorityClient` runs a boot-style re-sync which emits both `authoritative_position_received` (for the snap) and `authoritative_ack` (for cooldown-throttled consumers).
- [ ] HUD's `MoveRejectLabel` shows the rejection message as before (NEO-19).
- [ ] `TargetSelectionClient` treats that ack like any other: if a lock is held, it fires at most one refresh GET (subject to cooldown). No duplicate requests, no stuck busy state.
## 11. Server-down / transport failure
- [ ] Stop the server. Press **Tab** / **Esc**. Godot **Output** prints `TargetSelectionClient: HTTP failed …` warnings; the client is not wedged — you can press again (busy flag is released on failure).
- [ ] Restart the server and confirm the next **Tab** succeeds; HUD updates from the response.
## 12. No regressions
- [ ] Boot snap still works (NEO-7).
- [ ] WASD locomotion + `move-stream` still works (NEO-22); `PlayerPositionLabel` updates every physics tick.
- [ ] `E` interact with the prototype terminal still prints `allowed=true/false` in Output (NEO-9).
- [ ] Camera zoom / occluder obstacle toggle (Ctrl+Shift+K) still work.
- [ ] No new **red** lines in Godot Output during a typical 1–2 minute session. Benign warnings from other systems are fine; nothing from `TargetSelectionClient` during normal play.
---
When sections 1–11 are ticked and 12 shows no regressions, the story is ready for **In Test** / PR. Record any deviations or follow-ups in the **Decisions** or **Open questions** sections of the [implementation plan](../plans/NEO-24-implementation-plan.md).

2
docs/plans/NEO-23-implementation-plan.md
View File

@ -46,7 +46,7 @@
4. **`TargetState` fields (v1 JSON, shared by GET body and nested POST echo):** `schemaVersion`, `playerId` (echo), **`lockedTargetId`** (string or JSON **`null`** — key **always present**), `validity` (`ok`, `out_of_range`, `invalid_target`, `none`), `sequence` (int). **Validity** is **computed** on each read: no lock ⇒ `validity: none` and `lockedTargetId: null`; unknown id in store (should not happen) ⇒ `invalid_target`; else horizontal distance vs **`lockRadius`**.
5. **`TargetSelectRequest` / `TargetSelectResponse` v1:** Request: `schemaVersion`, optional `targetId`. Response: `schemaVersion`, **`selectionApplied`** (bool), **`targetState`** (object matching the GET field set above — always present), **`reasonCode`** (required when `selectionApplied` is `false`, omitted when `true` — same spirit as `InteractionResponse`). Successful **clear** ⇒ `selectionApplied: true`, `targetState` shows `lockedTargetId: null`, `validity: none`.
5. **`TargetSelectRequest` / `TargetSelectResponse` v1:** Request: `schemaVersion`, optional `targetId`, optional `positionHint` (`{x,y,z}` — NEO-24 follow-up #5, non-breaking additive field). When `positionHint` is present the server uses it for the radius check and for the echoed `targetState.validity`, making the select race-free against `move-stream`. The hint is advisory: it does **not** write to `IPositionStateStore` (`move-stream` remains the only position-write path). Response: `schemaVersion`, **`selectionApplied`** (bool), **`targetState`** (object matching the GET field set above — always present), **`reasonCode`** (required when `selectionApplied` is `false`, omitted when `true` — same spirit as `InteractionResponse`). Successful **clear** ⇒ `selectionApplied: true`, `targetState` shows `lockedTargetId: null`, `validity: none`.
6. **Routes** (exact paths in README; suggested):
- `GET /game/players/{id}/target` → **envelope** aligned with **`PositionStateResponse`**: top-level `schemaVersion`, `playerId`, plus lock fields (`lockedTargetId`, `validity`, `sequence`) so read and move APIs share the same “version + player echo + payload” pattern.

112
docs/plans/NEO-24-implementation-plan.md 100644
View File

@ -0,0 +1,112 @@
# 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](https://linear.app/neon-sprawl/issue/NEO-24/e1m3-client-tab-target-lock-ui-synced-to-server) |
| **Slug** | E1M3-02 |
| **Git branch** | `NEO-24-e1m3-client-tab-target-lock-ui-synced-to-server` |
| **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) (targeting HTTP v1) |
| **Decomposition** | [E1.M3 — InteractionAndTargetingLayer](../decomposition/modules/E1_M3_InteractionAndTargetingLayer.md); authority notes: [client_server_authority — E1.M3](../decomposition/modules/client_server_authority.md#e1m3-interactionandtargetinglayer) |
## 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](../../server/README.md#targeting-neo-23) / `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](../decomposition/modules/E1_M4_AbilityInputScaffold.md).
- 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
- [x] 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`.)
- [x] 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`.)
- [x] 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`.)
- [x] **Clear** action clears lock server-side and UI updates from the response. (`request_clear_target` omits `targetId`; `test_clear_issues_post_without_target_id`.)
- [x] 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](../reviews/2026-04-21-NEO-24.md)). 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](../../.cursor/rules/testing-expectations.md) 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](../reviews/2026-04-21-NEO-24.md)): 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 `<id>: <d> m / <radius> (in|out)` distance lines. Anchors/radii are mirrored in `prototype_target_constants.gd` as **display-only** data — server remains authoritative. If `PrototypeTargetRegistry.cs` moves, update both files in the same commit. |
| 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. |

95
docs/reviews/2026-04-21-NEO-24.md 100644
View File

@ -0,0 +1,95 @@
# Review — NEO-24
Date: 2026-04-21
Scope: `NEO-24-e1m3-client-tab-target-lock-ui-synced-to-server` (`origin/main...HEAD`)
Base: `origin/main` (merge-base `37f44a494400f4fdd369077c52dfc012df73a5d5`)
Follow-up: blocking issue + suggestions below are **done** (strikethrough + **Done.**). See the [NEO-24 plan](../plans/NEO-24-implementation-plan.md) Decision 3 for the signal change.
**Post-merge UX follow-up #5 (2026-04-21):** user reported "targeted A, moved out of A range and into B range — Tab gets denied incorrectly." HUD confirmed `Δ=0.00m` with `age≈7964ms` and beta distance `2.04 m / 6.0 (in)`, so client and server *agreed* on the player's stopped position and beta was clearly in range — but the select POST still denied. Root cause: follow-up #2's freshness kick fires 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 race against a stale stored snap (or, for that matter, an ASP.NET request executing on another thread while the stream is still in flight). Fix: extend the NEO-23 `TargetSelectRequest` v1 with an **optional** `positionHint: {x,y,z}`. When present, `TargetingApi` uses the hint for both the radius check and the `validity` field in the echoed `targetState` (so an accepted select never reports a stale `out_of_range`). The hint is advisory — it does **not** write to `IPositionStateStore`, so `move-stream` remains the sole position-write path. `TargetSelectionClient` attaches the hint automatically whenever `set_freshness_kick(...)` is called (same player ref), keeping the headless unit suites that skip that wiring on the unchanged code path. The freshness kick stays in place to keep the stored snap fresh for subsequent validity GETs, but is no longer load-bearing for denial correctness. Tests: `test_select_post_includes_position_hint_when_player_wired` (client), `PostSelect_ShouldUsePositionHint_WhenStoredSnapIsStale` + `PostSelect_ShouldDenyOutOfRange_WhenHintIsFar` + `PostSelect_PositionHint_ShouldNotWriteToPositionStore` (server). Plan Decision 8 + NEO-23 contract notes updated.
**Post-merge UX follow-up #4 (2026-04-21):** user asked for a single shared radius across all prototype targets and overlapping rings in the test scene so Tab-flipping is observable without walking. `PrototypeTargetRegistry` now exposes a `SharedLockRadius = 6.0` constant used by both anchors; alpha moved to `(-3, 0.5, -3)` and beta to `(3, 0, 3)` so the rings overlap ~3.5 m wide centered at origin. Spawn at `(-5, -5)` keeps alpha in range / beta out (existing soft-lock denial QA path untouched); walking to origin flips both rings in-range and the range-aware Tab picker swaps alpha ↔ beta on every press. `PlayerTargetStateReaderTests` updated to the new `(3, -3)` alpha boundary plus a new origin-in-both test. `prototype_target_constants.gd` mirrors the server's `SHARED_LOCK_RADIUS`. Picker tightened to **skip the currently-locked id** — so with only one target in range and that target already locked, Tab falls back to the cycle and the server denies, keeping NEO-23's "Tab = visible denial when no swap is possible" rule. New tests: `test_tab_from_locked_alpha_at_origin_swaps_to_beta_when_both_in_range`, `test_tab_when_only_current_lock_is_in_range_falls_back_to_cycle_for_denial`. Plan Decision 7.
**Post-merge UX follow-up #3 (2026-04-21):** user reported "tab here does not acquire target" while standing 2.41 m from beta (well inside the 4 m ring) with `Δ=0.00m age=2357ms` — i.e. client and server agreed on position. Server denied with `out_of_range`. Root cause was the Tab cycle: with no lock held, `request_tab_next()` returned `ORDERED_IDS[0]` = `prototype_target_alpha`, which was 17.90 m away. The user was visibly next to `beta` but Tab was always trying `alpha` first. Fix: `PrototypeTargetConstants.next_in_range_id_after(current, world)` 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 and falls back to `next_id_after(current)` when nothing is in range (so the server still owns the denial reason). Server contract untouched. Tests: `test_tab_from_no_lock_skips_out_of_range_and_picks_beta`, `test_tab_from_no_lock_with_nothing_in_range_falls_back_to_first_id`, `test_tab_without_player_ref_still_uses_plain_cycle_order`.
**Post-merge UX follow-up #2 (2026-04-21):** user reported "when in range of beta, pressing Tab says denied" — HUD distance line confirmed `prototype_target_beta: 0.58 m / 4.0 (in)` but server denied with `out_of_range` (and `prototype_target_alpha: 18.95 m` — almost exactly the spawn-to-current distance, strongly suggesting the server's stored position was still near spawn). Root cause is a race between the `move-stream` 20 Hz sampler and the target-select POST: a Tab press right after you stop moving validates against the last sampled position, which can trail the visible capsule. Two changes:
- `PlayerPositionLabel` now shows the server-acknowledged position + horizontal Δ + ack age (cached from `PositionAuthorityClient.authoritative_ack` via a new `_on_authoritative_ack_for_hud` handler). Makes the divergence directly visible in future repro.
- `TargetSelectionClient` exposes `set_freshness_kick(authority, player)`; `main.gd` wires it so every `POST /target/select` is immediately preceded by a `submit_stream_targets([player.global_position])` nudge on the authority. The select POST does not wait for the stream response, but the kick shrinks the race window enough in practice that "stop, Tab, denied" stops firing. Tests cover both the kick happening (`test_select_post_kicks_freshness_stream_before_posting`) and no regression when the wiring is absent (`test_select_post_without_freshness_wiring_is_a_noop`).
Server contract (NEO-23) untouched; this is pure client-side.
**Post-merge UX follow-up #1 (2026-04-21):** user reported "targeting result feels very inconsistent and doesn't seem to matter whether i'm within range of the console or not." Root cause was not a correctness bug — the `PrototypeTargetRegistry` anchors (`alpha (-5,-5)` r=8, `beta (8,8)` r=4) have **no visible scene meshes**, so players were reasoning about targeting against the `PrototypeTerminal` box at origin (which belongs to NEO-9 interaction, not NEO-23/24 targeting). Added:
- `client/scripts/prototype_target_markers.gd` — spawns an unshaded colored mast + flat radius ring per anchor (mirrors the server registry via `prototype_target_constants.gd`).
- Per-anchor distance readout on `UICanvas/TargetLockLabel`: `<id>: <d> m / <radius> (in|out)` computed client-side from the live capsule position. Labeled explicitly as a lag diagnostic — if `Validity: ok` shows while the distance line reads `(out)`, the server's position snapshot is trailing the visible capsule and the next `authoritative_ack` refresh reconciles it.
- Updated `client/README.md` and `docs/manual-qa/NEO-24.md` (Section 1 boot state and Section 4 locomotion check) to describe the markers and distance lines.
## Verdict
~~Request changes~~ — **Addressed.** Blocking issue fixed; integration test added; doc-alignment row updated.
## Summary
This branch adds the client-side target selection node, HUD label, input bindings, tests, and story docs for NEO-24. The overall direction matches the E1.M3 plan well: selection intent is POSTed to the server, the HUD paints only server-acknowledged target state, and denial handling is intentionally authoritative.
Follow-up verification confirms the earlier review findings were addressed. The movement-triggered soft-lock refresh is now reachable during normal locomotion via `PositionAuthorityClient.authoritative_ack`, the new integration test covers the real authority-to-target wiring, and the E1.M3 implementation-alignment row was updated to reflect NEO-24 landing.
## Documentation checked
- `docs/plans/NEO-24-implementation-plan.md` — **matches**. Decision 2 now uses `PositionAuthorityClient.authoritative_ack`, and Decision 3 records why the new signal was added.
- `docs/manual-qa/NEO-24.md` — **matches**. Section 4’s locomotion-driven refresh path is now implemented by the new authority ack signal and target refresh wiring.
- `docs/decomposition/modules/E1_M3_InteractionAndTargetingLayer.md` — **matches** for server-authoritative target state and denial reconciliation.
- `docs/decomposition/modules/client_server_authority.md` — **matches** for client-as-intent / server-as-truth behavior.
- `docs/decomposition/modules/module_dependency_register.md` — **matches**. `E1.M3` remains `In Progress`.
- `docs/decomposition/modules/documentation_and_implementation_alignment.md` — **matches**. The `E1.M3` tracking row now reflects NEO-24 landed (HUD + `authoritative_ack` refresh) and points back to the plan/manual QA docs.
## Blocking issues
1. ~~The movement-driven refresh path described by the plan, README, and manual QA is not reachable during normal locomotion. `main.gd` connects `TargetSelectionClient.on_authoritative_position_snap()` to `PositionAuthorityClient.authoritative_position_received`, but `position_authority_client.gd` only emits that signal from `_emit_position_from_response()` during `sync_from_server()` and the move-rejection recovery path. Successful `move-stream` POSTs explicitly do not emit the signal, so walking around with a held lock never triggers the throttled `GET /target` refresh that NEO-24 depends on. As a result, the HUD can stay stuck at `Validity: ok` after the player walks out of range, which breaks the plan's Decision 2, the acceptance criterion for soft-lock refresh, and manual QA section 4.~~ **Done.** Added a separate `authoritative_ack(world)` signal on `PositionAuthorityClient` that fires on both `BOOT_GET` 200 and successful `move-stream` 200 (extracted a shared `_parse_world_from_response` helper). `authoritative_position_received` keeps its current snap-only semantics so rubber-band suppression is untouched. `TargetSelectionClient.on_authoritative_ack(world)` replaces `on_authoritative_position_snap`; cooldown + lock gate unchanged. `main.gd` connects to the new signal. See `client/scripts/position_authority_client.gd`, `target_selection_client.gd`, `main.gd`, and plan Decision 3.
Relevant behavior in `client/scripts/position_authority_client.gd`:
```text
match _phase:
Phase.BOOT_GET:
_busy = false
if response_code == 200:
_emit_position_from_response(text, true)
Phase.STREAM_POST:
...
if response_code == 200:
# Do not emit `authoritative_position_received` here.
pass
```
Meanwhile `client/scripts/target_selection_client.gd` only refreshes from that signal:
```text
func on_authoritative_position_snap(_world: Vector3, apply_as_snap: bool) -> void:
if not apply_as_snap:
return
if not _has_lock():
return
...
request_sync_from_server()
```
The fix needs to either emit a suitable movement/reconciliation signal during normal locomotion or hook the target refresh to another authoritative movement event that actually occurs while moving.
## Suggestions
1. ~~Add an integration-level test for the real movement/targeting wiring. The current `client/test/target_selection_client_test.gd` suite calls `on_authoritative_position_snap()` directly, which is why it misses the broken connection to `PositionAuthorityClient`. A focused test around `main.gd` wiring or `PositionAuthorityClient` signal behavior would prevent this regression.~~ **Done.** Added `client/test/target_refresh_on_locomotion_test.gd`: wires a real `PositionAuthorityClient` and `TargetSelectionClient` with mock transports and the same `authoritative_ack` → `on_authoritative_ack` connection `main.gd` uses. Verifies (a) a `move-stream` 200 with a held lock produces exactly one throttled `GET /target` and the cached state flips to `out_of_range`, and (b) the same path is a no-op when no lock is held. Also added `test_sync_boot_get_200_emits_authoritative_ack` and `test_move_stream_post_200_emits_authoritative_ack` to `position_authority_client_test.gd` so renames on either side will break the per-script suites too.
2. ~~Update `docs/decomposition/modules/documentation_and_implementation_alignment.md` if this story lands. Its `E1.M3` snapshot still says client tab-target is open, which will be stale after merge.~~ **Done.** E1.M3 row now reflects NEO-24 landed (HUD + `authoritative_ack` refresh) with links to the plan and manual QA file.
## Nits
None.
## Verification
- Ran `godot --headless --import --path . --quit-after 10` from `client/`.
- Ran `godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test/position_authority_client_test.gd` from `client/` — suite passed, including the new `authoritative_ack` assertions.
- Ran `godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test/target_selection_client_test.gd` from `client/` — suite passed, including the renamed ack-based refresh tests.
- Ran `godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test/target_refresh_on_locomotion_test.gd` from `client/` — integration suite passed, confirming `move-stream` 200 triggers exactly one throttled target refresh while locked and none while unlocked.
- Read diagnostics for the touched client/scripts, client/test, plan, and review files — no linter errors found.
- I did not run the full manual client/server checklist; `docs/manual-qa/NEO-24.md` section 4 remains the highest-value end-to-end sanity check if you want one more runtime pass before merge.

20
server/NeonSprawl.Server.Tests/Game/Targeting/PlayerTargetStateReaderTests.cs
View File

@ -23,8 +23,8 @@ public class PlayerTargetStateReaderTests
[Fact]
public void ComputeValidity_AtExactLockRadius_ReturnsOk()
{
// Alpha anchor XZ (-5, -5), lockRadius 8 → horizontal distance 8 from (3, -5) is on boundary (inclusive).
var snap = new PositionSnapshot(3, 0, -5, 0);
// Alpha anchor XZ (-3, -3), lockRadius 6 → horizontal distance 6 from (3, -3) is on boundary (inclusive).
var snap = new PositionSnapshot(3, 0, -3, 0);
Assert.Equal(
TargetValidity.Ok,
PlayerTargetStateReader.ComputeValidity(PrototypeTargetRegistry.PrototypeTargetAlphaId, in snap));
@ -34,9 +34,23 @@ public class PlayerTargetStateReaderTests
public void ComputeValidity_JustOutsideLockRadius_ReturnsOutOfRange()
{
const double eps = 1e-6;
var snap = new PositionSnapshot(3 + eps, 0, -5, 0);
var snap = new PositionSnapshot(3 + eps, 0, -3, 0);
Assert.Equal(
TargetValidity.OutOfRange,
PlayerTargetStateReader.ComputeValidity(PrototypeTargetRegistry.PrototypeTargetAlphaId, in snap));
}
[Fact]
public void ComputeValidity_AtOrigin_ShouldReturnOk_ForBothPrototypeTargets()
{
// NEO-24 post-merge: alpha (-3,-3) r=6 and beta (3,3) r=6 overlap at origin so Tab can
// flip without locomotion. Origin is ~4.24 m from each anchor (< 6 m).
var snap = new PositionSnapshot(0, 0, 0, 0);
Assert.Equal(
TargetValidity.Ok,
PlayerTargetStateReader.ComputeValidity(PrototypeTargetRegistry.PrototypeTargetAlphaId, in snap));
Assert.Equal(
TargetValidity.Ok,
PlayerTargetStateReader.ComputeValidity(PrototypeTargetRegistry.PrototypeTargetBetaId, in snap));
}
}

80
server/NeonSprawl.Server.Tests/Game/Targeting/TargetingApiTests.cs
View File

@ -96,6 +96,86 @@ public class TargetingApiTests
Assert.Null(body.TargetState.LockedTargetId);
}
[Fact]
public async Task PostSelect_ShouldUsePositionHint_WhenStoredSnapIsStale()
{
// NEO-24 follow-up #5: without the hint, spawn's stored position (-5, -5) denies beta (r=6,
// anchor (3, 3)) — ~11.3 m out. With an advisory `positionHint` pinning the capsule inside
// beta's ring, the server must accept. This is exactly the race-free path the client uses.
await using var factory = new InMemoryWebApplicationFactory();
var client = factory.CreateClient();
var response = await client.PostAsJsonAsync(
"/game/players/dev-local-1/target/select",
new TargetSelectRequest
{
SchemaVersion = TargetSelectRequest.CurrentSchemaVersion,
TargetId = PrototypeTargetRegistry.PrototypeTargetBetaId,
PositionHint = new PositionVector { X = 3.0, Y = 0.5, Z = 5.0 },
});
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
var body = await response.Content.ReadFromJsonAsync<TargetSelectResponse>();
Assert.NotNull(body);
Assert.True(body!.SelectionApplied);
Assert.Null(body.ReasonCode);
Assert.Equal(PrototypeTargetRegistry.PrototypeTargetBetaId, body.TargetState.LockedTargetId);
}
[Fact]
public async Task PostSelect_ShouldDenyOutOfRange_WhenHintIsFar()
{
// Hint that places the capsule outside beta's ring must deny — hint is advisory only;
// it does not bypass the radius check.
await using var factory = new InMemoryWebApplicationFactory();
var client = factory.CreateClient();
var response = await client.PostAsJsonAsync(
"/game/players/dev-local-1/target/select",
new TargetSelectRequest
{
SchemaVersion = TargetSelectRequest.CurrentSchemaVersion,
TargetId = PrototypeTargetRegistry.PrototypeTargetBetaId,
PositionHint = new PositionVector { X = 50.0, Y = 0.0, Z = 50.0 },
});
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
var body = await response.Content.ReadFromJsonAsync<TargetSelectResponse>();
Assert.NotNull(body);
Assert.False(body!.SelectionApplied);
Assert.Equal(TargetingApi.ReasonOutOfRange, body.ReasonCode);
}
[Fact]
public async Task PostSelect_PositionHint_ShouldNotWriteToPositionStore()
{
// Hint must be purely advisory for the range check. Confirm the stored `PositionState`
// is untouched by a select-with-hint by reading it back through `GET /position`.
await using var factory = new InMemoryWebApplicationFactory();
var client = factory.CreateClient();
var before = await client.GetFromJsonAsync<PositionStateResponse>("/game/players/dev-local-1/position");
Assert.NotNull(before);
Assert.Equal(
HttpStatusCode.OK,
(await client.PostAsJsonAsync(
"/game/players/dev-local-1/target/select",
new TargetSelectRequest
{
SchemaVersion = TargetSelectRequest.CurrentSchemaVersion,
TargetId = PrototypeTargetRegistry.PrototypeTargetBetaId,
PositionHint = new PositionVector { X = 3.0, Y = 0.5, Z = 5.0 },
})).StatusCode);
var after = await client.GetFromJsonAsync<PositionStateResponse>("/game/players/dev-local-1/position");
Assert.NotNull(after);
Assert.Equal(before!.Position.X, after!.Position.X);
Assert.Equal(before.Position.Y, after.Position.Y);
Assert.Equal(before.Position.Z, after.Position.Z);
Assert.Equal(before.Sequence, after.Sequence);
}
[Fact]
public async Task PostSelect_ShouldClearLock_WhenTargetIdOmitted()
{

18
server/NeonSprawl.Server/Game/Targeting/PrototypeTargetRegistry.cs
View File

@ -3,16 +3,24 @@ namespace NeonSprawl.Server.Game.Targeting;
/// <summary>Prototype combat target ids → world anchor + horizontal lock radius (NEO-23). Keys are lowercase.</summary>
public static class PrototypeTargetRegistry
{
/// <summary>Near default dev spawn <c>(-5, -5)</c> on XZ.</summary>
/// <summary>
/// All prototype combat targets share a single <c>LockRadius</c>. Scoped to prototype
/// stubs — per-target radii will come back with real combat design (E5.M1). Changing
/// this requires updating <c>client/scripts/prototype_target_constants.gd</c> in the
/// same commit so markers/HUD match server range checks.
/// </summary>
public const double SharedLockRadius = 6.0;
/// <summary>On the -x/-z side of spawn; overlaps <see cref="PrototypeTargetBeta"/>'s ring at origin so Tab can flip without moving.</summary>
public const string PrototypeTargetAlphaId = "prototype_target_alpha";
/// <summary>Farther stub for out-of-range tests; ascending id order for NEO-24 tab cycle.</summary>
/// <summary>On the +x/+z side; mirrors alpha across the origin for symmetric overlap (NEO-24 Tab cycle).</summary>
public const string PrototypeTargetBetaId = "prototype_target_beta";
/// <summary>Alpha anchor aligned with spawn horizontal cell; <see cref="PrototypeTargetEntry.LockRadius"/> on XZ only.</summary>
public static readonly PrototypeTargetEntry PrototypeTargetAlpha = new(-5, 0.5, -5, 8.0);
/// <summary>Alpha anchor; <see cref="PrototypeTargetEntry.LockRadius"/> is XZ-only. Distance to beta ~8.49 m ⇒ ~3.5 m overlap at origin.</summary>
public static readonly PrototypeTargetEntry PrototypeTargetAlpha = new(-3, 0.5, -3, SharedLockRadius);
public static readonly PrototypeTargetEntry PrototypeTargetBeta = new(8, 0, 8, 4.0);
public static readonly PrototypeTargetEntry PrototypeTargetBeta = new(3, 0, 3, SharedLockRadius);
private static readonly Dictionary<string, PrototypeTargetEntry> ById = new(StringComparer.Ordinal)
{

13
server/NeonSprawl.Server/Game/Targeting/TargetStateDtos.cs
View File

@ -1,4 +1,5 @@
using System.Text.Json.Serialization;
using NeonSprawl.Server.Game.PositionState;
namespace NeonSprawl.Server.Game.Targeting;
@ -40,6 +41,18 @@ public sealed class TargetSelectRequest
/// <summary>Registry key after trim + case-insensitive lookup; <c>null</c> or omitted ⇒ clear intent.</summary>
[JsonPropertyName("targetId")]
public string? TargetId { get; init; }
/// <summary>
/// Optional client-reported current position. When present the server uses these coordinates
/// for the range check instead of the stored <c>PositionState</c> snapshot, eliminating the
/// race between <c>move-stream</c> and <c>target/select</c> POSTs (NEO-24 follow-up #5). Purely
/// advisory: it does <em>not</em> write to the position store (<c>move-stream</c> remains the
/// only write path). Real combat (E5.M1) will bound the hint against the stored snap / movement
/// budget so this cannot be used to "teleport" the radius check — prototype does not yet.
/// </summary>
[JsonPropertyName("positionHint")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public PositionVector? PositionHint { get; init; }
}
/// <summary>POST response for target selection (NEO-23).</summary>

16
server/NeonSprawl.Server/Game/Targeting/TargetingApi.cs
View File

@ -75,10 +75,20 @@ public static class TargetingApi
});
}
if (!HorizontalReach.IsWithinHorizontalRadius(snap.X, snap.Z, entry.X, entry.Z, entry.LockRadius))
// Prefer the client-reported `positionHint` when present: the stored `snap` can
// trail the visible capsule by several seconds when `move-stream` has been idle,
// which caused false `out_of_range` denials (NEO-24 follow-up #5). The hint is
// advisory — position writes stay the sole responsibility of `move-stream`. Real
// combat (E5.M1) will bound the hint vs. stored snap before trusting it.
// The hinted snap is used for the radius check *and* for building the response's
// `validity` so a hint-accepted select does not echo a stale `out_of_range`.
var effective = body.PositionHint is { } hint
? new PositionSnapshot(hint.X, hint.Y, hint.Z, snap.Sequence)
: snap;
if (!HorizontalReach.IsWithinHorizontalRadius(effective.X, effective.Z, entry.X, entry.Z, entry.LockRadius))
{
var (lockFar, seqFar) = locks.GetLockState(id);
var stateFar = PlayerTargetStateReader.Build(id, lockFar, seqFar, in snap);
var stateFar = PlayerTargetStateReader.Build(id, lockFar, seqFar, in effective);
return Results.Json(
new TargetSelectResponse
{
@ -90,7 +100,7 @@ public static class TargetingApi
}
var applied = locks.ApplySet(id, lookupKey);
var stateOk = PlayerTargetStateReader.Build(id, applied.LockIdLowercase, applied.Sequence, in snap);
var stateOk = PlayerTargetStateReader.Build(id, applied.LockIdLowercase, applied.Sequence, in effective);
return Results.Json(
new TargetSelectResponse
{