neon-sprawl/docs/reviews/2026-04-05-NEO-14.md

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