neon-sprawl/docs/plans/NEO-23-implementation-plan.md

95 lines
9.2 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# 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 **locked target id** (empty when none), **`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 modules 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
- [ ] **Server source of truth:** Client cannot persist a lock the server rejects; every **`POST`** response includes the **authoritative** `targetState` after evaluation (success or denial).
- [ ] **Denials:** **`selectionApplied`: `false`** (or equivalent boolean agreed in implementation) ⇒ **required non-empty `reasonCode`**; **HTTP 200** for logical denials; **400** / **404** rules match NEO-9 spirit (unknown player → **404** before body; malformed v1 → **400**).
- [ ] **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).
- [ ] **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` DTO (v1 JSON):** `schemaVersion`, `lockedTargetId` (null or absent in JSON when none — pick one style and document), `validity` enum serialized as stable strings (`ok`, `out_of_range`, `invalid_target`, `none` for “no lock”), `sequence` (int). **Validity** is **computed** on each read: no lock ⇒ `none`; 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`** (always), **`reasonCode`** (required when `selectionApplied` is false, omitted or null when true — mirror `InteractionResponse` / `reasonCode` rules). Successful **clear**`selectionApplied: true`, `targetState` shows no lock.
6. **Routes** (exact paths in README; suggested):
- `GET /game/players/{id}/target``TargetState` envelope (or bare `TargetState` if we prefer — **decide in PR** and keep README consistent).
- `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 (pre-implementation)**
- **Validity vs persistence:** Lock **id** persists until cleared or swapped; **`validity`** reflects live range so GET after move shows `out_of_range` without auto-clearing (matches “soft lock” until NEO-24 reconciliation policy). *If product prefers auto-clear when out of range, update this plan in the PR.*
## 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
- **Auto-clear on prolonged `out_of_range`:** deferred; current plan keeps id until explicit clear/swap (document for NEO-24 UX).
- **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.