NS-19: lock move validation plan to reject policy
parent
a9d418d9c6
commit
95f770d8ea
|
|
@ -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<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.
|
||||
|
||||
### 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.
|
||||
Loading…
Reference in New Issue