100 lines
10 KiB
Markdown
100 lines
10 KiB
Markdown
# NEO-23 — Implementation plan
|
||
|
||
## Story reference
|
||
|
||
| Field | Value |
|
||
|--------|--------|
|
||
| **Key** | NEO-23 |
|
||
| **Title** | E1.M3: TargetState v1 + server selection intent |
|
||
| **Linear** | [NEO-23](https://linear.app/neon-sprawl/issue/NEO-23/e1m3-targetstate-v1-server-selection-intent) |
|
||
| **Git branch** | `NEO-23-e1m3-targetstate-selection-api` |
|
||
| **Parent context** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) · [E1.M3 prototype backlog](E1M3-prototype-backlog.md) — **E1M3-01** |
|
||
| **Decomposition** | [E1.M3 — InteractionAndTargetingLayer](../decomposition/modules/E1_M3_InteractionAndTargetingLayer.md) (key contracts, boundary vs [NEO-9](NEO-9-implementation-plan.md)) |
|
||
|
||
## Goal, scope, and out-of-scope
|
||
|
||
**Goal:** Ship **versioned JSON** over the **existing HTTP** prototype channel: **authoritative `TargetState`** per player plus a **client→server selection intent** (working name **`TargetSelectRequest`**) such that the **server** is the only source of truth for who is locked, with **`reasonCode`** denials aligned with [NEO-9](NEO-9-implementation-plan.md) / `InteractionResponse` conventions (handled attempt → **200** + structured body; **400** / **404** only where the interaction route already does).
|
||
|
||
**In scope**
|
||
|
||
- **Stub eligible combat targets:** static registry (≥ **2** distinct ids with world anchors + horizontal **lock radius**) so [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24) can assume deterministic cycling without expanding scope here beyond “exists and validates.”
|
||
- **Selection intent:** `POST` with v1 body; **clear lock** when `targetId` is **omitted or JSON `null`** (explicit rule in XML + README). **Trim** non-null ids; **whitespace-only** after trim → **400** (same spirit as empty `interactableId` on NEO-9).
|
||
- **Authoritative read model:** `TargetState` includes **`lockedTargetId`** (**always** serialized; JSON **`null`** when no lock — never omit the key), **`validity`** derived from **current** `PositionState` vs stub anchor + radius using **`HorizontalReach`** (X/Z only, inclusive boundary — same policy as NEO-9), and **`sequence`** bumped only when the **persisted lock id** changes (aligns with “optional sequence” in the module’s illustrative table).
|
||
- **`GET`** current `TargetState` for the same player id normalization rules as position/interact APIs.
|
||
- **Unit + integration tests** mirroring `InteractionApiTests` patterns; **server README** subsection documenting routes, curl, and `reasonCode` table.
|
||
|
||
**Out of scope (per Linear / backlog)**
|
||
|
||
- Protobuf wire; full entity sim; replacing **`POST …/interact`** (remains separate).
|
||
- PvP / faction gating beyond a single optional **`invalid_target`** (or reserved code) if we need a stub deny path for API shape — **no** real [E6.M1](../decomposition/modules/E6_M1_PvPEligibilityAndFlagState.md) logic.
|
||
- Godot tab-target UI, HUD, optimistic highlight — **[NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)**.
|
||
|
||
## Acceptance criteria checklist
|
||
|
||
- [x] **Server source of truth:** Client cannot persist a lock the server rejects; every **`POST`** response includes the **authoritative** `targetState` after evaluation (success or denial).
|
||
- [x] **Denials:** **`selectionApplied`: `false`** ⇒ **required non-empty `reasonCode`**; **HTTP 200** for logical denials; **400** / **404** rules match NEO-9 spirit (unknown player → **404** before body; malformed v1 → **400**).
|
||
- [x] **Movement coupling:** After **`POST /move`** or **`POST /move-stream`**, the same **`GET`**/ **`POST`** response reflects updated **`validity`** (e.g. walk out of stub radius ⇒ `out_of_range` on read without requiring a new select).
|
||
- [x] **Contract names** in README + XML match the **illustrative** E1.M3 table ([Key contracts](../decomposition/modules/E1_M3_InteractionAndTargetingLayer.md#key-contracts)); any renames are visible in this plan and in PR text, not only in code.
|
||
|
||
## Technical approach
|
||
|
||
1. **New feature folder** `NeonSprawl.Server/Game/Targeting/` (keeps E1.M3 surface distinct from `Game/Interaction/` NEO-9 “use now” interactable path).
|
||
|
||
2. **Prototype stub registry** (e.g. `PrototypeTargetRegistry`): lowercase keys → anchor **X/Z** (Y stored but ignored for reach) + **`lockRadius`**. At least **two** rows at distinct horizontal positions so lock + range tests and NEO-24 ordering are unblocked. Constants duplicated in Godot **only when** NEO-24 adds preview; **not** required for NEO-23 if server-only.
|
||
|
||
3. **Mutable per-player lock** (e.g. `IPlayerTargetLockStore` + `InMemoryPlayerTargetLockStore`): stores **nullable locked target id** + **monotonic `sequence`** incremented **only** when the lock id changes (clear or swap). Thread-safe in-memory singleton for prototype (same lifetime pattern as `InMemoryPositionStateStore`).
|
||
|
||
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`, 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.
|
||
- `POST /game/players/{id}/target/select` → body `TargetSelectRequest`, response `TargetSelectResponse`.
|
||
|
||
7. **`Program.cs`:** register store + `app.MapTargetingApi()` (static extension parallel to `MapInteractionApi`).
|
||
|
||
8. **README:** new subsection **Targeting (NEO-23)** with curl, status table, `reasonCode` list (`unknown_target`, `out_of_range`, optional `invalid_target`, …).
|
||
|
||
**Decisions (locked — stakeholder 2026-04-18)**
|
||
|
||
| Topic | Choice |
|
||
|--------|--------|
|
||
| **`lockedTargetId` when no lock** | Property **always present**; value JSON **`null`** (never omit the key). |
|
||
| **GET body shape** | **Envelope** like `PositionStateResponse`: `schemaVersion`, `playerId`, then lock fields (easier evolution and parity with position GET). |
|
||
| **POST success flag** | **`selectionApplied`** (distinct from interact’s `allowed` — avoids “permission” ambiguity). |
|
||
| **Out of range vs stored lock** | **Soft lock:** persisted lock **id** unchanged until clear/swap; **`validity`** becomes `out_of_range` when the player moves away until they clear or select again. **No auto-clear** on read or timer in NEO-23. |
|
||
|
||
## Files to add
|
||
|
||
| Path | Purpose |
|
||
|------|---------|
|
||
| `server/NeonSprawl.Server/Game/Targeting/TargetingApi.cs` | Maps GET/POST routes; normalizes player id; composes store + position + registry. |
|
||
| `server/NeonSprawl.Server/Game/Targeting/TargetStateDtos.cs` | JSON v1 DTOs: `TargetStateResponse` / `TargetStateBody` (split if needed), `TargetSelectRequest`, `TargetSelectResponse`. |
|
||
| `server/NeonSprawl.Server/Game/Targeting/PrototypeTargetRegistry.cs` | Static stub targets (≥2 ids) with anchors + `lockRadius`. |
|
||
| `server/NeonSprawl.Server/Game/Targeting/IPlayerTargetLockStore.cs` | Abstraction for lock + sequence mutations. |
|
||
| `server/NeonSprawl.Server/Game/Targeting/InMemoryPlayerTargetLockStore.cs` | Prototype in-memory implementation. |
|
||
| `server/NeonSprawl.Server/Game/Targeting/PlayerTargetStateReader.cs` (optional single file) | Pure helper: given lock id + `PositionSnapshot` + registry → `validity` (keeps handler thin). |
|
||
| `server/NeonSprawl.Server.Tests/Game/Targeting/TargetingApiTests.cs` | Integration tests: select, deny unknown, deny out of range, clear, move then re-read validity, unknown player 404, bad schema 400. |
|
||
|
||
## Files to modify
|
||
|
||
| Path | Rationale |
|
||
|------|-----------|
|
||
| `server/NeonSprawl.Server/Program.cs` | Register `IPlayerTargetLockStore` and call `MapTargetingApi()`. |
|
||
| `server/README.md` | Document NEO-23 endpoints, request/response fields, and `reasonCode` table next to Interaction (NEO-9). |
|
||
|
||
## Tests
|
||
|
||
| Test file | What to cover |
|
||
|-----------|----------------|
|
||
| `server/NeonSprawl.Server.Tests/Game/Targeting/TargetingApiTests.cs` | **Apply lock** when in range; **deny** unknown id with `reasonCode` + `selectionApplied` false + unchanged authoritative state; **deny** out of range when in range for another stub only if requesting the far one (or walk seed position); **clear** lock via POST without `targetId`; **GET** after **POST /move** updates `validity` without a new select; **404** unknown player; **400** bad `schemaVersion` / whitespace `targetId`; **case-insensitive** target id lookup. |
|
||
| *(optional)* `server/NeonSprawl.Server.Tests/Game/Targeting/PlayerTargetStateReaderTests.cs` | **Only if** validation is extracted: unit tests for validity transitions at inclusive radius boundary (same spirit as `HorizontalReachTests`). |
|
||
|
||
No automated Godot harness for NEO-23; manual verification is optional HTTP client / curl per README.
|
||
|
||
## Open questions / risks
|
||
|
||
- **NEO-24 UX:** document that UI may show **out of range** while **`lockedTargetId`** stays set until an explicit clear or a successful swap (soft lock).
|
||
- **Postgres vs in-memory target store:** prototype lock store is in-memory only for NEO-23; multi-instance deployment would need shared storage — out of scope until infra story exists.
|