From bd0286cb1c48c883d0bf4ca66352b39fa622c510 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Fri, 17 Apr 2026 20:57:13 -0400 Subject: [PATCH] NEO-23: add implementation plan for TargetState v1 and selection API --- docs/plans/NEO-23-implementation-plan.md | 94 ++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 docs/plans/NEO-23-implementation-plan.md diff --git a/docs/plans/NEO-23-implementation-plan.md b/docs/plans/NEO-23-implementation-plan.md new file mode 100644 index 0000000..463b24e --- /dev/null +++ b/docs/plans/NEO-23-implementation-plan.md @@ -0,0 +1,94 @@ +# 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 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 + +- [ ] **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.