diff --git a/docs/plans/NEO-22-implementation-plan.md b/docs/plans/NEO-22-implementation-plan.md index f1dca4e..3887d7f 100644 --- a/docs/plans/NEO-22-implementation-plan.md +++ b/docs/plans/NEO-22-implementation-plan.md @@ -19,7 +19,7 @@ - **Input:** `project.godot` actions (or code) for **W/A/S/D**; read them in a thin place (`main.gd` or a small `locomotion_input.gd`) and drive the player’s **horizontal** intent each physics tick. - **Locomotion model:** `CharacterBody3D` + `move_and_slide()` with velocity derived from **wish direction × speed**, camera-relative or world-relative (decide in [Decisions](#decisions)); **no `NavigationAgent3D` path goal** for routine walking unless a later slice reintroduces hybrid “click to mark + auto-run” explicitly. - **Remove / disable** left-click ground pick as the default move trigger (`ground_pick.gd` → `main.gd` → `PositionAuthorityClient.submit_move_target`). Either delete the flow for this slice or gate it behind a dev flag documented as off by default. -- **Server / authority:** Resolve how WASD maps to **NEO-7 `MoveCommand`** and **authoritative position** (see [Open questions](#open-questions--risks)). Minimum acceptable outcomes are spelled in acceptance criteria — pick one path and document it in `client/README.md` + decomposition. +- **Server / authority:** **Option C** — server accepts a **stream** of small validated moves (see [Decisions](#decisions)); document wire shape and cadence in `client/README.md` + `server/README.md` + decomposition. **NEO-7** `POST …/move` may remain for click/debug targets or be narrowed once the stream path owns routine locomotion (decide before merge). - **Cleanup:** Delete or drastically narrow **NEO-11 / NEO-14 / NEO-16** workaround blocks in `player.gd` (column seam damp, nav column steering, idle trace scaffolding tied to click goals) **only** after WASD path is stable; prefer one commit that switches input then a follow-up that deletes dead code. - **Docs:** Update `client/README.md`, `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md`, and this plan’s checklist when shipped. @@ -34,7 +34,7 @@ - [ ] **WASD** moves the avatar on the default **`main.tscn`** floor with **no** left-click requirement; speed and direction are stable on flat ground and on **stepped** QA props without the prior ±`MOVE_SPEED` seam oscillation at column/stair corners (subjective + optional `debug_idle_trace` spot-check). - [ ] **Left-click** no longer starts a walk by default (or is clearly dev-only and off in normal play, documented). -- [ ] **Authoritative position** still converges with the server: document whether **continuous** sync, **throttled** `MoveCommand`, or **temporary client-only** prototype applies — and add the minimal client/server change so CI + manual steps do not lie. +- [ ] **Authoritative position** still converges with the server via the **stream / small-step** contract (option **C**); README + tests describe batching or per-tick cadence so CI + manual steps do not lie. - [ ] **Cold boot `snap_to_server`** still works; no spawn underground / stale goal regressions. - [ ] **NEO-10** rejection UX: if clicks are removed, another path still exercises reject payloads in dev (automated test or documented console-only command) **or** clicks remain only for “set goal” debug, not default locomotion. - [ ] **GdUnit** (or existing harness) covers at least one pure helper for wish-dir / speed clamp if logic is non-trivial. @@ -42,11 +42,7 @@ ## Technical approach 1. **Linear + branch:** **2026-04-17 kickoff** — Linear **In Progress**; story branch **`NEO-22-retire-click-to-move-wasd-locomotion`** (issue id first per [linear-git-naming](../../.cursor/rules/linear-git-naming.md)). -2. **Authority model (blocking):** Decide with product/server owner: - - **A)** Client integrates WASD locally only for prototype; server receives **rate-limited** position or move deltas (may need new endpoint — likely **out of this slice** unless already planned), or - - **B)** Keep **discrete `MoveCommand`** but drive target from **keyboard-sampled “stick to last intent”** (awkward), or - - **C)** Server accepts a **stream** of small moves (largest change — schedule separately). - Document chosen option in README before merging locomotion feel work. +2. **Authority model:** **Option C** — introduce a **continuous-locomotion** server path (small validated steps or **batched** steps per HTTP call in this slice; **WebSocket / tick pipe** deferred unless explicitly pulled in). Reuse or mirror **NEO-10**-style limits (max step, ΔY) where applicable; assign **sequence** / ordering rules so out-of-order repeats are safe. **NEO-7** `POST …/move` stays available for debug or one-shot targets until stream-only is intentional. Document the contract in `client/README.md` and `server/README.md` before merge. 3. **Input layer:** Map WASD to `Vector2` in screen/camera space → world XZ tangent to `Camera3D` basis (same pattern as many third-person controllers); normalize; zero when no keys. 4. **`player.gd` refactor:** Replace `_has_walk_goal` / `_auth_walk_goal` steering path with **wish velocity** from parent or injected each `_physics_process`; keep gravity, floor snap policy, and `snap_to_server` entry points. Strip `NavigationAgent3D` dependency for default walk if decision says so (agent node can stay in scene unused until a later hybrid story). 5. **Remove click wiring:** Disconnect `target_chosen` → `submit_move_target` in `main.gd` for default build; optionally keep `ground_pick.gd` for debug “teleport goal” behind `OS.is_debug_build()` flag. @@ -60,12 +56,15 @@ | **Kickoff** | **2026-04-17** | Branch `NEO-22-retire-click-to-move-wasd-locomotion`; status In Progress in Linear. | | **Camera vs world steering** | **TBD in kickoff** — default recommendation: **camera-relative XZ** | Matches isometric follow rig; document if world-axis is chosen instead. | | **NavAgent** | Default **off** for routine WASD in this slice | Removes nav-ribbon + column latch class of bugs; re-add only with explicit hybrid design. | +| **Authority (WASD → server)** | **Option C — stream of small moves** | Aligns wire semantics with continuous input; leaves room for batched HTTP now and a tick/WebSocket transport later without rebranding “every WASD sample as a click destination.” | ## Files to add | Path | Purpose | |------|---------| -| None required. | Prefer extending existing scripts unless `locomotion_input.gd` (or similar) is needed to keep `main.gd` thin per `godot-client-script-organization`. If extracted, add here and list in “Files to modify” as N/A for that line. | +| `server/NeonSprawl.Server/Game/PositionState/MoveStreamRequest.cs` (name at implementer discretion) | Versioned JSON body for **one or more** small authoritative steps (or equivalent “locomotion frame”) applied in order; pairs with new `MapPost` route. | +| `server/NeonSprawl.Server.Tests/Game/PositionState/MoveStreamApiTests.cs` (matching test file) | Happy path, rejection, unknown player, and ordering/sequence behavior for the new endpoint. | +| `client/scripts/locomotion_input.gd` (optional) | Thin WASD → wish vector if `main.gd` would otherwise grow; omit if logic stays trivially inline. | ## Files to modify @@ -75,7 +74,10 @@ | `client/scripts/main.gd` | Stop wiring click pick to `submit_move_target` by default; optionally forward WASD intent to player/authority per chosen model. | | `client/scripts/ground_pick.gd` | Disable default `_input` pick **or** gate behind export/debug; update header comments (NS-16 lineage). | | `client/scripts/player.gd` | Replace goal/nav horizontal steering with WASD-driven velocity; delete obsolete seam/nav state once stable. | -| `client/scripts/position_authority_client.gd` | Align with authority model (throttle, stream, or document client-only prototype). | +| `client/scripts/position_authority_client.gd` | Add **stream submit** path (batch or frequent small-step POSTs), backoff/coalesce while in flight, and alignment with server **sequence** after locomotion frames. | +| `server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` | Register **option C** route; delegate to store / validation (may loop `TryApplyMoveTarget` or new store helper for multi-step). | +| `server/NeonSprawl.Server/Game/PositionState/MoveCommandValidation.cs` (and/or new validator) | Reuse or factor shared rules so stream steps and legacy `MoveCommand` stay consistent where intended. | +| `server/README.md` | Document new movement stream endpoint, schema version, and relationship to NEO-7 `POST …/move`. | | `client/scenes/main.tscn` | Only if `NavigationAgent3D` or nodes are removed/changed or input singleton paths change. | | `client/README.md` | Replace “click-move” manual checks with WASD + server steps; note deprecation of click-to-move prototype. | | `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Snapshot: click-to-move retired in favor of WASD for prototype; link this plan. | @@ -86,12 +88,14 @@ | File | Coverage | |------|----------| | `client/test/player_test.gd` | **Change:** remove or replace tests that assumed `_has_walk_goal` / nav steering; add tests for any extracted **wish-direction** or speed clamp helper. | -| `client/test/position_authority_client_test.gd` | **Change:** align with new submit/sync cadence if `PositionAuthorityClient` changes. | +| `client/test/position_authority_client_test.gd` | **Change:** align with **stream** submit cadence, pending queue, and sequence handling. | +| `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs` | **Change only if** legacy `POST …/move` behavior or shared validation changes; otherwise leave as regression guard for one-shot moves. | | **Manual (required)** | README path: boot with server, WASD on flat + stepped geometry, `snap_to_server`, and whichever NEO-10 rejection path remains after click removal. | ## Open questions / risks -- **Server contract:** Today’s **`MoveCommand`** is **discrete target**-shaped ([NEO-7](NEO-7-implementation-plan.md)). WASD needs **continuous** intent — either extend the protocol, rate-limit many small commands, or temporarily diverge prototype authority. **This is the main schedule risk**; do not merge feel-only client changes without an explicit written decision in README + this plan. +- **Wire shape for C:** Single-step POST at high cadence vs **batched** array per POST vs early **WebSocket** — pick for NEO-22 to control server load and client complexity; document in README. +- **Legacy `MoveCommand`:** Whether **NEO-7** remains for debug/teleport only or is unified with stream handler — avoid two conflicting sources of truth without documenting precedence. - **Interaction vs pick:** Confirm **E** / interaction does not rely on the same mouse button in a way that breaks when pick is disabled. - **Gamepad:** Out of scope unless trivial (`Input.get_vector`); note for follow-up story. - **Jira naming:** If the team uses **NEON-*** keys elsewhere, align Linear/Git subject lines per [linear-git-naming](../../.cursor/rules/jira-git-naming.md).