diff --git a/docs/plans/NS-19-implementation-plan.md b/docs/plans/NS-19-implementation-plan.md new file mode 100644 index 0000000..1828798 --- /dev/null +++ b/docs/plans/NS-19-implementation-plan.md @@ -0,0 +1,90 @@ +# NS-19 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NS-19 | +| **Title** | E1.M1: Server movement validation (speed + bounds) | +| **Jira** | [NS-19](https://neon-sprawl.atlassian.net/browse/NS-19) | +| **Parent context** | [NS-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NS-1) · [NS-10 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NS-10) | +| **Decomposition** | [E1.M1 — InputAndMovementRuntime](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) (`MoveCommand` / `PositionState` authority) | + +**Delivery:** Ship **`docs/plans/NS-19-implementation-plan.md`** on the **same branch / PR** as the NS-19 implementation (plan + code pushed together). + +## Goal, scope, and out-of-scope + +**Goal:** Server-side **sanity rules** on `MoveCommand` so impossible steps (teleport, out-of-world targets) are **not** applied to authoritative `PositionState`. Failures return a **stable `reasonCode`** for logs and future telemetry. + +**Design alignment — floor play (XZ emphasis)** + +Match [NS-18](NS-18-implementation-plan.md): prototype gameplay is **on the floor**. **Primary** move cap should be **horizontal displacement on X/Z** (same spirit as `HorizontalReach`). Optionally cap **|ΔY|** separately so small floor bumps are allowed but vertical teleports are not. Document the exact rule in code and server README. + +**In scope** + +- **Configurable limits** under the existing `Game` options section (extend [`GamePositionOptions`](../../server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs) or add a nested `MovementValidationOptions` bound from `appsettings` / env): at minimum **max horizontal step** (XZ) and **max absolute vertical delta** per command; tunable defaults suitable for the current Godot click-to-move stride. +- **Optional axis-aligned “district” bounds** (min/max per axis or min/max XZ + Y range): when **enabled**, reject targets **outside** the box; when **disabled**, skip the check (default for minimal friction in local dev). +- **`POST /game/players/{id}/move`:** after resolving the **current** authoritative position, run validation **before** `TryApplyMoveTarget`. On violation, **do not** mutate position or sequence. +- **Error payload:** JSON body with **`schemaVersion`** (`1`) and **`reasonCode`** (non-empty, stable string). **HTTP status** — prefer **400** for “valid request shape but move **rejected**” vs **404** for unknown player (keep **404** unchanged); finalize in PR if needed. + +**Out of scope (per Jira)** + +- Navmesh, full anti-cheat, client prediction / reconciliation. + +**Dependencies** + +- **`MoveCommand` endpoint** and stores ([NS-15](NS-15-implementation-plan.md), [NS-16](NS-16-implementation-plan.md), [NS-17](NS-17-implementation-plan.md)) — **done**. + +## Policy — reject (locked) + +Invalid moves are **rejected**: return the error payload; **do not** change authoritative position or sequence. Client can log **`reasonCode`** or show a short label. **Clamp** (silent or partial apply) is **out of scope** for NS-19; a future story would need explicit product rules if we ever add it. + +## Acceptance criteria checklist + +- [ ] **Oversized** move (horizontal and/or vertical per configured rules) is **rejected**; client can observe failure (existing logging and/or minimal UI). +- [ ] Rules implemented in **testable** pure logic (e.g. static `MoveCommandValidation` or small service) with **unit tests** on the math and edge cases (at limit, just inside, just outside; floating-point safe). +- [ ] **Constants / thresholds** live in **one configuration surface** (options pattern bound to `Game:`), not scattered magic numbers. +- [ ] **Integration tests** on `PositionStateApi` / HTTP: valid move still **200** + `PositionStateResponse`; invalid move returns the agreed status + **`reasonCode`**; unknown player still **404**. + +## Technical approach + +### 1. Validation helper (C#) + +- Inputs: **from** `PositionSnapshot` (or x,y,z + sequence if only coords matter), **to** `PositionVector` target, **options** snapshot. +- Outputs: `bool ok` + **`reasonCode`** when not ok. +- **Horizontal step:** \( \sqrt{(x_t-x_f)^2 + (z_t-z_f)^2} \) compared to **`MaxHorizontalStep`** (or chosen name). +- **Vertical:** \( |y_t - y_f| \) compared to **`MaxVerticalStep`**. +- **Bounds:** when enabled, require `min.X <= x_t <= max.X` (and Y, Z similarly) — document inclusive vs exclusive boundaries (recommend **inclusive** min/max for a closed district box). + +### 2. Wire-up in `PositionStateApi` + +- Inject **`IOptions`** (or a dedicated options type) into the minimal API delegate via method injection / scoped helper already used elsewhere in the app. +- Flow: validate body (existing) → **`TryGetPosition`** → if missing **404** → **validate move** → if fail return **error JSON** → else **`TryApplyMoveTarget`** → **200** + response. + +### 3. Response DTO for failures + +- New type e.g. **`MoveCommandRejectedResponse`** with `schemaVersion`, `reasonCode`. +- **Stable `reasonCode` values (v1 draft — finalize in PR):** e.g. `horizontal_step_exceeded`, `vertical_step_exceeded`, `out_of_bounds` (names snake_case, stable for telemetry). + +### 4. Configuration defaults + +- Document in **`server/README.md`** (Position / move section): keys, units (world meters), and example `appsettings.Development.json` overrides. +- Defaults should allow normal prototype play from current spawn / interactable layout; tighten in tests to force rejections without flaky timing. + +### 5. Client (minimal) + +- If the Godot client treats every **200** as success today, add a **branch** for **400** (or chosen code): log **`reasonCode`** and optionally show a **Label** or status text — enough to satisfy “client can surface failure” without building full UI. + +### 6. Documentation hygiene + +- After merge: add one line to [E1_M1_InputAndMovementRuntime.md](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) **implementation snapshot** (movement validation + link to this plan). + +## Testing notes + +- **Unit:** validation helper only — no web host. +- **Integration:** extend [`MoveCommandApiTests`](../../server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs) (or parallel file) with a factory override for **tight** validation options so tests do not depend on production defaults. +- Postgres path: validation runs **before** DB write; **in-memory** tests are sufficient for NS-19 unless a regression is found in store-specific code (none expected). + +## PR / review + +Paste or adapt when opening the PR; align **`reasonCode`** strings and HTTP status with final DTOs (**reject-only** behavior). Cross-check [server README](../../server/README.md) and [E1.M1](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) for consistency.