# Code review — NEO-14 idle / rest stability (Godot client)

**Date:** 2026-04-05  
**Scope:** Branch `NEO-14-idle-vibration-fix` vs merge-base `3283f6d04c8724f6536ca591a2a4c8ca06ea7b61` (tracked as `origin/main` at review time). Issue **NEO-14**; no PR URL supplied.  
**Base:** `origin/main` @ `3283f6d04c8724f6536ca591a2a4c8ca06ea7b61`

**Follow-up:** Review **suggestions** below are **done** (strikethrough + **Done.**); **nits** remain optional.

## Verdict

**Approve with nits**

## Summary

This branch delivers **NEO-14** by hardening the prototype **`CharacterBody3D`** idle path (`player.gd`): dual **`floor_max_angle`** behavior, rim/straddle detection, idle nudges, bump proximity escape, **`floor_block_on_wall`**, **`NavigationAgent3D.avoidance_enabled = false`**, and **physics interpolation off** at project + player node level. It fixes **compound internal-edge** issues by spawning each QA bump as its **own `StaticBody3D`** (`random_floor_bumps.gd`), adds **Jolt** in **`project.godot`**, and reduces **z-fighting** via capsule **mesh Y lift** (visual-only vs collision shape). **`client/README.md`** and **`server/README.md`** (bump height copy) are updated. **Server contracts** (`MoveCommand` / `PositionState`) are not changed; risk is localized to client feel and QA geometry. Manual QA by the author reports **no remaining jitter** in exercised poses.

## Documentation checked

| Document | Result |
|----------|--------|
| `docs/plans/NEO-14-implementation-plan.md` | **Matches** — **Resolution**, checked AC, **Shipped note**, updated **Open questions** (`015ca13`). |
| `docs/plans/NEO-11-implementation-plan.md` | **Matches** at intent level (client nav visual, server authority unchanged); no contract drift observed in the diff. |
| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches.** Prototype client locomotion and reconciliation surface remain consistent; changes are polish under “movement-loop” / QA, not authority or wire shape. |
| `docs/decomposition/modules/module_dependency_register.md` | **N/A** for code behavior; cited as the hub for module IDs. No register **Status** change required for this client-only jitter fix. |
| `docs/decomposition/modules/client_server_authority.md` | **N/A** (no change to who owns `PositionState` or move validation). |
| `client/README.md` | **Matches** NEO-14 call to document the pattern and manual steps (idle 10+ s, bumps). |

## Blocking issues

*(none)*

## Suggestions

**All done.**

1. ~~**Close the loop in `docs/plans/NEO-14-implementation-plan.md`:**~~ **Done.** Check off acceptance criteria; add **Resolution** (shipped mitigations) and **Shipped note** under technical approach — `015ca13`.
2. ~~**DRY bump collision constants** (`BUMP_COLLISION_*` aligned across `player.gd` / `random_floor_bumps.gd`):~~ **Done.** `client/scripts/random_floor_bump_collision_constants.gd` + preload from both scripts; README references — `015ca13`, doc sync `87d8ee0` (file later renamed off **`ns19_*`** identifiers).
3. ~~**Future character yaw** (`_snap_capsule_upright()` full identity basis):~~ **Done.** `TODO(NEO-14 follow-on)` on `_snap_capsule_upright()` in `player.gd`; plan **Open questions** — `015ca13`.

## Nits

- **`get_tree().get_nodes_in_group(...)`** on **`random_floor_bump_mesh`** each idle tick is fine at **two** bumps; if the group grows, cache references after spawn or guard cost.
- ~~**Commit history:**~~ **Done.** Branch history squashed to a **single** **`NEO-14:`** commit before merge; no stray **`fix:`** / **`docs:`** subjects on the slice.

## Verification

Per **`NEO-14-implementation-plan.md`** and **`client/README.md`**:

1. Run **`main.tscn`** with server as in NEO-7/NEO-11 README steps.
2. **Idle 10+ s** at spawn and after a move (no click goal); confirm **no visible vibration** on slab and on **random green bumps** (new positions each run).
3. **Walking**, **arrival**, **`snap_to_server`** on cold boot; **NEO-10** reject UX (pedestal / far pad) unchanged.
4. **Obstacle / descend** flows per NEO-11 tradeoff (multi-click around tall obstacle if needed).
5. Spot-check **bump rim**, **cylinder side**, and **floor–bump** contact (prior jitter loci).

**Automated:** None required for NEO-14 (no Godot harness in repo).