neon-sprawl/docs/plans/NS-23-implementation-plan.md

# NS-23 — Implementation plan

## Story reference

| Field | Value |
|--------|--------|
| **Key** | NS-23 |
| **Title** | E1.M1: Client path-follow baseline (navigation + visible locomotion) |
| **Jira** | [NS-23](https://neon-sprawl.atlassian.net/browse/NS-23) |
| **Parent context** | [NS-10 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NS-10) · [NS-1 — Epic 1](https://neon-sprawl.atlassian.net/browse/NS-1) · Slice 1 — Movement and position sync |
| **Decomposition** | [E1.M1 — InputAndMovementRuntime](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) |

**Delivery:** Ship **`docs/plans/NS-23-implementation-plan.md`** on the **same branch / PR** as the NS-23 implementation (plan + code together), matching the NS-19 pattern.

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

**Goal:** Deliver **visible** click-to-move: the avatar **follows a path** toward the clicked point using a **navigation mesh** baseline (obstacle-aware), not only steering at a goal or snapping on server ack alone. Aligns with E1.M1: path-follow baseline under server authority.

**In scope**

- **Godot:** `NavigationRegion3D` (baked mesh) over the prototype walkable floor in `main.tscn`, respecting existing static obstacles; **`NavigationAgent3D`** (on player or child node) driving **`CharacterBody3D`** motion (`move_and_slide` toward next path position — follow Godot 4.x recommended pattern).
- **Pick integration:** Reuse **`ground_pick.gd`** (`walkable` group, upward normal); clicked point becomes **`NavigationAgent3D.target_position`** (after any needed Y / agent height adjustment consistent with current capsule).
- **Server authority (unchanged contract):** One **`MoveCommand`** per successful pick with the **same world target** as today ([NS-16](NS-16-implementation-plan.md)); server **v1 snap** + **[NS-19](NS-19-implementation-plan.md)** validation unchanged.

**Out of scope (per Jira)**

- Full prediction/reconciliation, MMO-grade netcode, server-side nav mesh validation.
- Replacing **`InteractionRequest`** / range rules.

## Locked — client nav vs server authority (this slice)

| Topic | Decision |
|--------|----------|
| **When to POST** | **Once per click**, immediately with the **picked destination** (same JSON as today). No waypoint streaming to the server in this story. |
| **Server truth** | Still **snap to target** on **200** after validation; **`GET /position`** after POST remains the reconciliation read (existing `PositionAuthorityClient` flow). |
| **Straight-line vs path** | NS-19 validates **displacement** from **current authoritative position** to **target** (horizontal + vertical limits). It does **not** check path clearance through geometry; **client nav** is responsible for **visible** obstacle avoidance. |
| **After successful move** | **Do not** teleport the avatar on the verifying **`GET`** when the player is expected to walk: apply the authoritative position as a **navigation goal** and let the agent complete the path; **initial boot** `sync_from_server` remains a **hard snap** so spawn matches the server. |
| **POST 400** | Existing behavior: emit **`move_rejected`**, show NS-19 UX; **do not** start (or clear) a nav path to that target. |
| **Distance limits** | Default **`MaxHorizontalStep`** (e.g. 18 m) remains compatible with **single-command** long clicks across the prototype; if a future scene needs shorter steps, chain commands or tune limits in a later story. |

## Acceptance criteria checklist

- [x] Clicking a valid floor target produces **continuous motion** along a plausible path around static obstacles in the prototype scene (not sliding through walls).
- [x] Authoritative position still **converges** with the server (`MoveCommand` + `GET`); no regression on NS-19 rejection UX (`reasonCode` / label timeout).
- [x] Short note in **`server/README.md`** and/or **client script header**: client nav vs server authority for this slice.

## Technical approach

1. **`main.tscn` / navigation**  
   - Add **`NavigationRegion3D`** covering walkable floor; include obstacle **`StaticBody3D`** geometry in baking as required by Godot 4 so paths wrap obstacles.  
   - Bake after layout changes; document any **editor step** in `client/README.md` if bake is not fully reproducible from scene alone.

2. **Player / agent**  
   - Attach **`NavigationAgent3D`**; replace NS-14-style **direct steering toward `_goal`** with **path following** (velocity toward `get_next_path_position()` or equivalent, `move_and_slide`).  
   - Keep **`snap_to_server`** for **boot** and any **hard reconcile** case you explicitly document (default: boot only).

3. **Wiring (`main.gd` / `PositionAuthorityClient`)**  
   - On **`target_chosen`**: call **`submit_move_target`** as today (POST → GET).  
   - Distinguish **authoritative position from boot** vs **after successful move** so **`main.gd`** snaps on boot but sets **nav target** (no snap) after verify when implementing path-follow.  
   - Optional small API: second signal, enum on existing signal, or `sync_from_server`-only snap path — pick the smallest change that stays readable per [godot-client-script-organization](../../.cursor/rules/godot-client-script-organization.md).

4. **Documentation**  
   - **`client/README.md`:** Update movement section: navmesh path-follow + server authority one-liner.  
   - **`server/README.md`:** One short subsection or bullet: server validates **end target** and **step**; **client** nav is **presentational** for obstacle following in this slice.

## Resolved — agent shape vs capsule (was risk)

Prototype player collider in [`main.tscn`](../../client/scenes/main.tscn): **`CapsuleShape3D_player`** — **`radius = 0.4`**, **`height = 1.0`**.

| Setting | Locked choice |
|--------|----------------|
| **`NavigationAgent3D.radius` / `height`** | Match the **physics** capsule (**0.4** / **1.0**) so nav clearance matches what `move_and_slide` actually sweeps. |
| **Navmesh bake agent** | In **`NavigationRegion3D`** bake settings, use the **same** radius/height (or Godot’s equivalent **agent** parameters) so baked polygons match runtime pathfinding. |
| **Corner clipping vs “stuck”** | If the mesh clips into box corners, **increase** agent radius slightly and **rebake** (wider corridor on the mesh) before shrinking physics. Do not silently shrink the capsule without an explicit design change. |
| **Vertical alignment** | Keep **`path_height_offset`** (or equivalent) at **0** unless the capsule foot position and floor snap misalign; the pick already supplies floor **`Y`**. |
| **Arrival** | Align **`target_desired_distance`** (and related agent tolerances) with existing player **`ARRIVE_EPS`** (~**0.35** m) so stop distance matches prior steering feel unless QA says otherwise. |

## Resolved — navigation sync / frame order (was risk)

| Topic | Locked choice |
|--------|----------------|
| **Where motion runs** | Path queries (`get_next_path_position`, velocity toward next point) and **`move_and_slide`** run only in **`Player._physics_process`**, not in `_process` or UI thread. |
| **When `target_position` is set** | `main.gd` (or signal handler) may set the agent target in response to **`target_chosen`** / authority callbacks; the **first** consumable segment appears after the navigation map updates. |
| **First frame after new target** | If **`NavigationAgent3D.is_navigation_finished()`** or a zero-length first step appears on the **same** frame as `target_position` assignment, **defer** one tick: `await get_tree().physics_frame` **once** after setting target **or** simply rely on **next** `_physics_process` (preferred — avoid await in hot path). **Do not** poll path from `_input` without deferral. |
| **Map readiness** | After adding **`NavigationRegion3D`**, ensure the region is **baked** and enabled before F5; startup **`sync_from_server`** snap does not need a path. |

## Files to add (expected)

| Path | Purpose |
|------|---------|
| None required by name | Prefer extending existing `player.gd` / scene unless a dedicated `player_navigation.gd` reduces `main.gd` bloat. |

## Files to modify

| Path | Rationale |
|------|-----------|
| `client/scenes/main.tscn` | `NavigationRegion3D`, bake source meshes, agent node setup. |
| `client/scripts/player.gd` | Path-follow via `NavigationAgent3D`; clarify header (NS-14 → NS-23 evolution). |
| `client/scripts/main.gd` | Boot snap vs post-move nav goal wiring. |
| `client/scripts/position_authority_client.gd` | If needed: signal shape or phase so main can snap only on boot. |
| `client/README.md` | Manual check steps for path-follow + server. |
| `server/README.md` | Client nav vs authority note (AC). |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Implementation snapshot when NS-23 ships. |

## Tests

| Action | What to cover |
|--------|----------------|
| **Manual (required)** | Server running; click around obstacles — path goes around; click NS-19 reject targets — same rejection behavior as before; cold start — player matches server position. |
| **Automated** | None required for this story unless an existing headless hook can assert nav baking (unlikely); prefer manual for Godot nav. |

## Open questions / risks

None — agent/bake alignment and physics-frame path consumption are locked in **Resolved — agent shape vs capsule** and **Resolved — navigation sync / frame order** above. Reopen only if the scene scale or avatar collider changes materially.

## PR / review

Cross-check [E1.M1](../decomposition/modules/E1_M1_InputAndMovementRuntime.md), [client README](../../client/README.md), and [server README](../../server/README.md). Confirm NS-19 props in `main.tscn` still behave after nav region changes.