neon-sprawl/docs/plans/NEO-22-implementation-plan.md

# NEO-22 — Implementation plan

## Story reference

| Field | Value |
|--------|--------|
| **Key** | NEO-22 |
| **Title** | Retire prototype click-to-move; ship WASD locomotion on the client |
| **Linear** | [NEO-22](https://linear.app/neon-sprawl/issue/NEO-22) |
| **Parent** | [E1.M1 InputAndMovementRuntime](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4) (or current movement project) |
| **Supersedes / deprecates (prototype)** | [NEO-5](NEO-5-implementation-plan.md), [NEO-11](NEO-11-implementation-plan.md) click-to-move surface; [NEO-14](NEO-14-implementation-plan.md) seam/jitter mitigations tied to nav + goal steering |

## Goal, scope, and out-of-scope

**Goal:** Replace **mouse click → ground pick → `MoveCommand` → nav path → `player.gd` goal steering** with **keyboard WASD** (and optional gamepad later) as the **primary** locomotion input in the Godot prototype. Movement should feel predictable on stepped geometry without the column / nav-ribbon / slide-normal fight that click-to-move accumulated.

**In scope**

- **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:** **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.

**Out of scope**

- Full **client prediction + server reconciliation** for FPS-grade netcode (unless explicitly pulled into this story by product).
- **Click-to-interact** changes (NEO-12-style interaction) except where click conflicts with removed move binding (document if unchanged).
- **Occlusion / camera** feature work (NEO-17/20) beyond what’s needed so WASD direction feels correct with the follow camera.
- **Production** navmesh tooling for NPCs — only player move input changes here.

## Acceptance criteria checklist

- [ ] **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 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.
- [x] **GdUnit** (or existing harness) covers pure helpers on `player_locomotion_wasd.gd` (floor-ray continuation, median feet Y, micro-slip XZ projection, wish-aligned horizontal velocity).

## 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:** **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.
6. **Tests + manual:** Run README manual path; add unit test for direction math if extracted.

## Decisions

| Topic | Choice | Rationale |
|------|--------|-----------|
| **Issue id** | **NEO-22** | Created in Linear 2026-04-16; plan filename matches this key. |
| **Kickoff** | **2026-04-17** | Branch `NEO-22-retire-click-to-move-wasd-locomotion`; status In Progress in Linear. |
| **Camera vs world steering** | **Camera-relative XZ** | Implemented in `main.gd` via `_camera` basis and `Input.get_vector` on **`move_*`** actions. |
| **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 |
|------|---------|
| `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. |
| `client/scripts/player_locomotion_wasd.gd` | WASD wall-scrape / seam tick extracted from `player.gd` so **`gdlint` `max-file-lines`** (1600) stays green without raising the cap. |

## Files to modify

| Path | Rationale |
|------|-----------|
| `client/project.godot` | Declare **`move_forward` / `move_back` / `move_left` / `move_right`** (or reuse Godot defaults) bound to WASD. |
| `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 (delegates active WASD tick to `player_locomotion_wasd.gd`); delete obsolete seam/nav state once stable. |
| `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. |
| `docs/plans/NEO-11-implementation-plan.md` | Add one-line “superseded for locomotion input by NEO-22” in header or Resolution if policy is to annotate predecessors. |

## Tests

| 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/player_locomotion_wasd_test.gd` | **Add:** unit tests for static seam/locomotion math (`evaluate_floor_ray_hit_y`, median samples, micro-slip projection, horizontal velocity alignment). |
| `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

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

## Client follow-up (done)

- **Click-to-move removed:** Deleted `client/scripts/ground_pick.gd` and `client/test/ground_pick_test.gd`, removed **`GroundPick`** from `client/scenes/main.tscn`, dropped **`PositionAuthorityClient.submit_move_target`** and **`POST_MOVE` / `VERIFY_GET`** phases. **`main.gd`** no longer wires pick → move. Boot **`GET`** still emits **`authoritative_position_received(..., true)`** for **`snap_to_server`**; the `apply_as_snap == false` branch is unused until a future server-driven nav goal exists.