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

97 lines
9.2 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# 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 players **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.
- **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 plans 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 whats 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: 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.
- [ ] **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.
## Technical approach
1. **Linear + branch:** Issue **NEO-22** exists in Backlog; on kickoff set **In Progress** and branch `neo-22-retire-click-to-move-implement-wasd-locomotion-godot-client` (or team naming).
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.
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. |
| **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. |
## 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. |
## 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; 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/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/position_authority_client_test.gd` | **Change:** align with new submit/sync cadence if `PositionAuthorityClient` changes. |
| **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:** Todays **`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.
- **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).