NEO-22: Cache scrape contact in WASD tick; close review items
run_wasd: one _has_scrape_vertical_contact() before move_and_slide and one after; fold repeated predicates into wish_active_scrape_vertical_pre and post_slide_wish_floor_scrape. Review doc: mark suggestions #2/#4 done; suggestion #1, nits, verification ledge bullet, and partial doc rows reviewed/deferred with strikethrough notes.pull/42/head
parent
c79c58c298
commit
866c973af7
|
|
@ -203,9 +203,11 @@ func run_wasd(delta: float) -> void:
|
|||
var locomotion_probe_floor: bool = (
|
||||
wish_probe_active and _player._walk_has_close_floor_probe_below(wish_probe)
|
||||
)
|
||||
# Slide normals reflect the **previous** tick until [method CharacterBody3D.move_and_slide] runs.
|
||||
var scrape_vertical_pre_slide: bool = _has_scrape_vertical_contact()
|
||||
var scrape_probe_hold: bool = (
|
||||
locomotion_probe_floor
|
||||
and _has_scrape_vertical_contact()
|
||||
and scrape_vertical_pre_slide
|
||||
and _player.velocity.y >= LOCOMOTION_WALL_SCRAPE_PROBE_HOLD_VY_MAX
|
||||
)
|
||||
if _player.is_on_floor():
|
||||
|
|
@ -215,16 +217,15 @@ func run_wasd(delta: float) -> void:
|
|||
else:
|
||||
_floor_coyote = maxi(0, _floor_coyote - 1)
|
||||
var locomotion_grounded_y: bool = _player.is_on_floor() or _floor_coyote > 0
|
||||
var scrape_micro_slip_xz: bool = (
|
||||
wish_probe_active and locomotion_grounded_y and _has_scrape_vertical_contact()
|
||||
)
|
||||
var wish_active_scrape_vertical_pre: bool = wish_probe_active and scrape_vertical_pre_slide
|
||||
var scrape_micro_slip_xz: bool = locomotion_grounded_y and wish_active_scrape_vertical_pre
|
||||
if locomotion_grounded_y:
|
||||
_player.velocity.y = 0.0
|
||||
else:
|
||||
_player._apply_walk_air_gravity(delta, feet_y, Vector2.ZERO)
|
||||
_player._walk_clip_horizontal_velocity_against_vertical_contacts(wish)
|
||||
_player.floor_snap_length = _floor_snap_moving
|
||||
if locomotion_grounded_y and wish_probe_active and _has_scrape_vertical_contact():
|
||||
if locomotion_grounded_y and wish_active_scrape_vertical_pre:
|
||||
if locomotion_probe_floor:
|
||||
_player.floor_snap_length = 0.0
|
||||
else:
|
||||
|
|
@ -235,7 +236,12 @@ func run_wasd(delta: float) -> void:
|
|||
if scrape_micro_slip_xz:
|
||||
_project_global_xz_on_wish_line_if_micro_slip(scrape_ray_anchor_feet_xz, wish_probe)
|
||||
var feet_after: float = _feet_y()
|
||||
if wish_probe_active and _player.is_on_floor() and _has_scrape_vertical_contact():
|
||||
var scrape_vertical_post_slide: bool = _has_scrape_vertical_contact()
|
||||
var on_floor_post_slide: bool = _player.is_on_floor()
|
||||
var post_slide_wish_floor_scrape: bool = (
|
||||
wish_probe_active and on_floor_post_slide and scrape_vertical_post_slide
|
||||
)
|
||||
if post_slide_wish_floor_scrape:
|
||||
var ray_absorbed: bool = false
|
||||
if locomotion_probe_floor:
|
||||
var w3d := _player.get_world_3d()
|
||||
|
|
@ -291,7 +297,7 @@ func run_wasd(delta: float) -> void:
|
|||
_project_global_xz_on_wish_line_if_micro_slip(scrape_ray_anchor_feet_xz, wish_probe)
|
||||
else:
|
||||
reset_scrape_feet_lock()
|
||||
if wish_probe_active and _player.is_on_floor() and _has_scrape_vertical_contact():
|
||||
if post_slide_wish_floor_scrape:
|
||||
if locomotion_probe_floor:
|
||||
_align_velocity_xz_to_wish()
|
||||
if locomotion_grounded_y:
|
||||
|
|
|
|||
|
|
@ -16,10 +16,10 @@ The WASD tick implements floor coyote time, wall-scrape detection (including sli
|
|||
|
||||
| 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. This diff does **not** address stream/authority wiring, README updates, click removal verification, or GdUnit coverage listed in the plan; treat those as story-level gaps before calling NEO-22 done. Cleanup bullet (“narrow NEO-11/NEO-14 blocks once WASD stable”) is **not** exercised here (additions are NEO-14-flavored mitigations for WASD, not deletion of legacy nav blocks). |
|
||||
| `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 already describes WASD + move-stream; **Purpose** still reads “click-to-move or path-follow baseline,” which is legacy wording until NEO-22 doc pass lands. |
|
||||
| `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. |
|
||||
|
||||
|
|
@ -29,22 +29,22 @@ The WASD tick implements floor coyote time, wall-scrape detection (including sli
|
|||
|
||||
## 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.
|
||||
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.
|
||||
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.
|
||||
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.
|
||||
- ~~**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.
|
||||
- ~~**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).
|
||||
- ~~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).*
|
||||
|
|
|
|||
Loading…
Reference in New Issue