From 8e421d09c13ea189a0f67f0370a7d4b86f40dc11 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 4 Apr 2026 23:14:47 -0400 Subject: [PATCH] NS-19: add files to add/modify and tests sections to plan --- docs/plans/NS-19-implementation-plan.md | 37 ++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/docs/plans/NS-19-implementation-plan.md b/docs/plans/NS-19-implementation-plan.md index 1828798..37a4ea6 100644 --- a/docs/plans/NS-19-implementation-plan.md +++ b/docs/plans/NS-19-implementation-plan.md @@ -79,11 +79,40 @@ Invalid moves are **rejected**: return the error payload; **do not** change auth - 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 +## Files to add -- **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). +| 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` (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. | +| `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 (if needed):** Run Godot client, attempt an oversized click-to-move (after tightening limits in dev config) and confirm log/UI surfaces rejection. + +## Open questions / risks + +None. ## PR / review