# Code review — NEO-22 WASD locomotion (wall scrape / floor seam)

**Date:** 2026-04-17  
**Scope:** Branch `NEO-22-retire-click-to-move-wasd-locomotion`; working-tree diff for `client/scripts/player.gd` only (unstaged vs `HEAD`). No PR URL supplied.  
**Base:** `origin/main` @ merge-base `aee3ca80d8451952c62fff7e8e72899e0d62bf68` (inferred).

## Verdict

**Approve** (blocking CI line-count issue resolved; remaining bullets are non-blocking suggestions.)

## Summary

The WASD tick implements floor coyote time, wall-scrape detection (including slide normals when `is_on_floor` / `is_on_wall` disagree with Jolt), wish-aligned velocity cleanup when a floor continuation probe says walkable floor exists ahead, micro XZ projection onto the wish line to damp seam lip slip, median floor-ray feet Y sampling, and post–`move_and_slide` feet Y smoothing / one-frame absorption (in `player_locomotion_wasd.gd` + callbacks on `player.gd`). Intent matches NEO-22 acceptance language (stable WASD on stepped geometry; NEO-14-class seam mitigation). **Repository CI:** `player.gd` is split so `gdlint` `max-file-lines` should pass. Secondary risks are behavioral (global coyote zeros vertical velocity for several ticks after *any* floor loss) and maintainability (hot-path branching split across two files).

## Documentation checked

| Path | Assessment |
|------|------------|
| `docs/plans/NEO-22-implementation-plan.md` | ~~**Partially matches** — WASD seam stability and `player.gd` locomotion work align with goal and acceptance bullets; remaining plan checklist / cleanup bullets not re-verified line-by-line in this review pass.~~ *Reviewed; defer remaining checklist + doc sync to story close-out.* |
| `docs/plans/NEO-14-implementation-plan.md` | **N/A** for literal checklist (not re-read end-to-end); comments in code cite NEO-14 seam behavior — **matches** spirit of prior seam/jitter work applied to WASD. |
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E1.M1 remains the owning module; no new contract surfaces in this diff. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | ~~**Partially matches** — implementation snapshot describes WASD + move-stream; **Purpose** still reads “click-to-move or path-follow baseline,” legacy until a doc pass.~~ *Reviewed; Purpose wording update deferred to doc-only work.* |
| `docs/decomposition/modules/client_server_authority.md` | **N/A** — no server or wire changes; client-side `CharacterBody3D` nudges remain in local prediction territory. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **N/A** — no register status row change required for this slice alone. |

## Blocking issues

1. ~~**`gdlint` `max-file-lines` will fail after merge.** Repo `gdlintrc` sets `max-file-lines: 1600` with an explicit note that `player.gd` is a large controller. `origin/main`’s `client/scripts/player.gd` is **1483** lines; the working tree is **~1781** lines, i.e. **over the 1600 limit**. CI (`gdlint` on `client/scripts/`) should fail until lines are trimmed, config is temporarily raised with a tracked follow-up to split the script, or locomotion helpers move to another file under the NEO-22 refactor plan.~~ Done. WASD locomotion tick moved to `client/scripts/player_locomotion_wasd.gd`; `player.gd` is back under the 1600 line cap (`gdlint` / `gdformat` clean).

## Suggestions

1. ~~**Coyote vs ledge fall:** Floor coyote refreshes on `is_on_floor()` or `scrape_probe_hold`, otherwise decrements. While grounded-by-coyote is true, `velocity.y` is forced to `0.0` before `move_and_slide()`. That means **several physics ticks with no gravity** after leaving walkable support even on an open ledge (no wall scrape), which may feel like a short hover. If coyote is only meant to paper over Jolt floor flicker during wall scrape, consider narrowing when `velocity.y` is zeroed (e.g. require scrape vertical contact or probe hold), and keep normal air gravity otherwise.~~ *Reviewed; not implementing in this pass — revisit only if open-ledge playtest shows unacceptable hover.*

2. ~~**Hot-path calls:** Scrape vertical contact checks can scan all slide collisions and are invoked multiple times in one WASD tick. Caching the result once per tick would reduce work and keep behavior deterministic if slide ordering ever changes.~~ Done. `run_wasd` caches `_has_scrape_vertical_contact()` once **before** `move_and_slide()` (`scrape_vertical_pre_slide`) and once **after** (`scrape_vertical_post_slide`); pre vs post split preserves correct floor/slide state.

3. ~~**NEO-22 test AC:** The plan asks for GdUnit coverage when wish/scrape logic is non-trivial. Floor-ray feet Y, micro-slip projection, and wish alignment helpers in `player_locomotion_wasd.gd` are good candidates for small pure tests with a stubbed `PhysicsDirectSpaceState3D` or extracted math-only helpers.~~ Done. `client/test/player_locomotion_wasd_test.gd` covers the extracted static helpers; full `run_wasd` + `move_and_slide` remains manual / integration territory.

4. ~~**Readability:** The post–`move_and_slide` block repeats the same scrape-contact predicates; folding into a single boolean or early section may reduce mistake surface when tuning.~~ Done. `wish_active_scrape_vertical_pre`, `post_slide_wish_floor_scrape`, and related locals in `player_locomotion_wasd.gd`.

## Nits

- ~~**Nit:** In floor-ray median sampling, `var y_vals: Array = []` could use a typed array (`Array[float]`) for consistency with typed GDScript elsewhere.~~ *Reviewed; source already uses `Array[float]` in `_floor_ray_feet_y_median`.*

- ~~**Nit:** `wish_probe` duplicates direction already in `_locomotion_wish_world_xz`; low cost, but if refactors continue, a single source avoids drift.~~ *Reviewed; not deduplicating in this pass.*

## Verification

- Install tooling per `gdscript-style` / CI: `pip install "gdtoolkit==4.5.0"`, then `gdlint client/scripts client/test` and `gdformat --check client/scripts client/test`.
- Godot manual: WASD along stepped props and column-like seams from NEO-14 QA; confirm no regression on flat idle (NEO-14 idle path unchanged in diff but worth a smoke check).
- ~~Walk off an open ledge with no wall: confirm fall start timing matches design (see coyote suggestion).~~ *Reviewed; optional QA tied to suggestion #1 (not pursuing coyote change here).*
