neon-sprawl/docs/plans/NS-19-implementation-plan.md

# 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. Defaults must sit **between** the **small bump** and **tall platform** heights described below so manual QA is obvious without editing numbers every run.
- **Godot manual-test props** in [`main.tscn`](../../client/scenes/main.tscn): **(1)** one or more **low bumps**—`StaticBody3D` nodes in the **`walkable`** group (same as [`ground_pick.gd`](../../client/scripts/ground_pick.gd) expects) with a **shallow** top surface (e.g. **0.08–0.2 m** above the main floor) so a normal click on the bump keeps **|ΔY|** **under** `MaxVerticalStep` and the server **accepts**; **(2)** a **tall “reject” pedestal**—also **`walkable`**, with its **clickable top** high enough (e.g. **≥ 1.5–2 m** above floor, exact value vs. default `MaxVerticalStep` documented in README) so a click from the floor sends a target the server **rejects** with **`vertical_step_exceeded`**; **(3)** optional **far pad** at similar **Y** to spawn but at **horizontal distance** past `MaxHorizontalStep` to exercise **`horizontal_step_exceeded`**. Use distinct materials (e.g. muted green bumps, warning stripe pedestal) so testers can see what to click.
- **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

- [x] **Oversized** move (horizontal and/or vertical per configured rules) is **rejected**; client can observe failure (existing logging and/or minimal UI).
- [x] 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).
- [x] **Constants / thresholds** live in **one configuration surface** (options pattern bound to `Game:`), not scattered magic numbers.
- [x] **Integration tests** on `PositionStateApi` / HTTP: valid move still **200** + `PositionStateResponse`; invalid move returns the agreed status + **`reasonCode`**; unknown player still **404**.
- [x] **Godot scene:** **`walkable`** bumps (small ΔY) and tall **`walkable`** pedestal (and optional far pad) per §5b; manual pass confirms bump **accepts** vs pedestal **rejects** with expected **`reasonCode`**.

## 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<GamePositionOptions>`** (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.

### 5b. Scene props for manual movement validation

- Only surfaces in group **`walkable`** receive ray picks; the **main floor** already uses this. Add bumps and the tall pedestal as **`walkable`** `StaticBody3D` (or parent in group) so clicks produce world targets. **Do not** mark the existing opaque **`Obstacle`** as walkable unless we explicitly want clicks on its sides/top (default: leave non-walkable so it stays visual/collision clutter only, or repurpose it with a **thin walkable cap** if we want a second tall reject target).
- After placing nodes, document in **`server/README.md`** the **intended** default `MaxVerticalStep` / `MaxHorizontalStep` relative to those props (e.g. “bumps ~0.15 m, pedestal top ~2 m above floor”).

### 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).

## Files to add

| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/PositionState/MoveCommandValidation.cs` | Pure validation API: current snapshot + target + options → pass/fail + `reasonCode` (horizontal step, vertical step, optional bounds). |
| `server/NeonSprawl.Server/Game/PositionState/MoveCommandRejectedResponse.cs` | JSON DTO for failed moves: `schemaVersion`, `reasonCode`. |
| `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandValidationTests.cs` | Unit tests for validation math (at limit, inside, outside; float-safe; bounds on/off). |

## Files to modify

| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs` | Add nested **`MovementValidation`** (or sibling options type still bound under `Game:`): max horizontal step, max vertical delta, district bounds enable + min/max vector (or per-axis), defaults for prototype. |
| `server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` | After `TryGetPosition`, run validation before `TryApplyMoveTarget`; on failure return **400** + `MoveCommandRejectedResponse`; inject `IOptions<GamePositionOptions>` (or `IOptionsSnapshot<>`). |
| `server/NeonSprawl.Server/Game/PositionState/PositionStateServiceCollectionExtensions.cs` | Only if binding shape changes (e.g. new subsection); ensure new options properties bind from configuration. |
| `server/NeonSprawl.Server/appsettings.json` and/or `appsettings.Development.json` | Document example keys for movement limits and optional bounds (sane defaults for click-to-move). |
| `server/README.md` | Document move validation, config keys, rejection status/body, and `reasonCode` values. |
| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Optional: small helper or documented pattern to override `Game` options with **strict** limits for HTTP tests without relying on production defaults. |
| `client/scripts/position_authority_client.gd` | Handle non-**200** from move POST: log **`reasonCode`** (and optional user-visible hint); do not emit authoritative snap as success. |
| `client/scenes/main.tscn` | Add **walkable** low bumps + tall **walkable** pedestal (and optional far pad) with meshes/materials for manual NS-19 validation; keep floor/interactable layout coherent. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Implementation snapshot: NS-19 move validation + link to this plan. |

## Tests

| Action | Path | What to cover |
|--------|------|----------------|
| **Add** | `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandValidationTests.cs` | Horizontal step: at `MaxHorizontalStep`, epsilon inside, epsilon outside; vertical `|ΔY|` same pattern; bounds disabled → no reject on extreme coords; bounds enabled → `out_of_bounds`; order of checks if multiple fail (document fixed precedence in code + test one case). |
| **Change** | `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs` | Existing happy-path moves still **200** under new defaults; add cases: step too large → **400** + body contains expected `reasonCode`; unknown player still **404**; schema/body errors unchanged **400** (no `reasonCode` vs rejection body—document chosen behavior). |
| **Optional** | `server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs` | **No change expected** — validation runs before store write; skip unless a regression appears. |

**Manual (required for props):** With server defaults documented for the scene: **(1)** Click **onto a small bump** from nearby floor → move **accepted**, player reconciles to server position. **(2)** Click **tall pedestal top** from floor → **`vertical_step_exceeded`** (or chosen code), log/UI visible, position unchanged. **(3)** If a far pad is added → **`horizontal_step_exceeded`** from a standing position that keeps vertical sane. **(4)** Optionally lower `MaxVerticalStep` in dev config until a bump **rejects** to confirm thresholds feel right.

## Open questions / risks

None.

## 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.