diff --git a/client/README.md b/client/README.md index a94535d..a621efe 100644 --- a/client/README.md +++ b/client/README.md @@ -53,6 +53,14 @@ The main scene includes a **prototype terminal** at the map center (same world * 2. **F5** in Godot: default spawn is out of range of the terminal; markers should stay **dim**. **Click-move** toward the center until markers **brighten** (within **3** m on the floor plane). 3. Press **E** (input action **`interact`** in `project.godot`): Output should show **`allowed=true`** when markers glow, **`allowed=false`** with **`reasonCode=out_of_range`** when dim (if you walk back out). Interaction uses **`_input`**, not `_unhandled_input`, so keys register reliably in the embedded **Game** dock; click the game view if the editor had focus elsewhere. +### Manual check (NEON-30 — occluder click-through) + +1. Run the main scene (**F5**) with the server running. +2. **While `Obstacle` is opaque** (player not behind it): left-click the ground directly behind `World/NavigationRegion3D/Obstacle` (roughly `(6, 0, 5)` + a few units further). The player should receive a valid move target and walk toward that position — not stop or ignore the click. +3. **While `Obstacle` is faded** (player positioned so the occlusion policy is actively fading the obstacle): repeat the same click. Behavior should be identical — the occluder fade state must not affect pick success. +4. **Regression — normal floor:** click a clear area of the floor; confirm the player walks there as before. +5. **Regression — far pad / pedestal:** confirm click-rejection behavior on `MoveRejectPedestal` and `MoveRejectFarPad` is unchanged. + ## Movement prototype (NEON-2 → NEON-8) **`player.gd`** uses **`NavigationAgent3D.get_next_path_position()`** + **`move_and_slide()`** for horizontal motion when following the mesh; it may steer **directly at the goal** in xz when the descend bypass applies (NEON-8). **`snap_to_server()`** remains for **boot** (and would apply for any future hard reconcile). diff --git a/docs/plans/NEON-30-implementation-plan.md b/docs/plans/NEON-30-implementation-plan.md index 8723119..afbc024 100644 --- a/docs/plans/NEON-30-implementation-plan.md +++ b/docs/plans/NEON-30-implementation-plan.md @@ -29,9 +29,9 @@ ## Acceptance criteria checklist -- [ ] Clicking on the ground directly behind an occluder (faded or opaque) issues a valid move command to that position. -- [ ] Clicking on a non-occluder surface behaves as before. -- [ ] No new collision-layer magic numbers introduced; the approach is documented in the implementation plan. +- [x] Clicking on the ground directly behind an occluder (faded or opaque) issues a valid move command to that position. +- [x] Clicking on a non-occluder surface behaves as before. +- [x] No new collision-layer magic numbers introduced; the approach is documented in the implementation plan. ## Technical approach diff --git a/docs/reviews/2026-04-09-NEON-30.md b/docs/reviews/2026-04-09-NEON-30.md new file mode 100644 index 0000000..1415c41 --- /dev/null +++ b/docs/reviews/2026-04-09-NEON-30.md @@ -0,0 +1,38 @@ +# NEON-30 review + +**Date:** 2026-04-09 +**Scope:** Branch `NEON-30-click-through-input` reviewed against `origin/main...HEAD` +**Base:** `origin/main` + +## Verdict + +Approve with nits + +## Summary + +This branch keeps the fix narrowly scoped to `client/scripts/ground_pick.gd`: occluder-tagged bodies are now skipped before the existing walkable-surface logic runs, which matches the intended NEON-30 behavior and avoids introducing a new collision-layer contract. The supporting unit tests and module/README updates are directionally correct and consistent with the existing `"occluder"` group convention established in NEON-27. I did not find a correctness issue in the shipped code path from the diff alone. I could not run the Godot/GdUnit verification commands in this environment because `godot` is not installed locally, so the full click-through behavior still needs author-side verification. + +## Documentation checked + +- `docs/plans/NEON-30-implementation-plan.md` — partially matches. The implementation and recorded decisions line up with the diff, but the acceptance-criteria checklist is still entirely unchecked even though the branch is presented as implemented; per the plan/doc workflow, that checklist should be refreshed before merge. +- `docs/decomposition/modules/E1_M2_IsometricCameraController.md` — matches. The new click-through input note accurately records the shared `"occluder"` group contract and the absence of a new collision-layer convention. +- `docs/decomposition/modules/module_dependency_register.md` — matches / no update needed. `E1.M2` remains `In progress`; this story does not appear to change the module readiness state. +- `.cursor/rules/testing-expectations.md` — matches. The branch updates `client/test/ground_pick_test.gd` alongside the production GDScript change and still relies on manual verification for the full pick flow, which is acceptable for this scope. + +## Blocking issues + +None. + +## Suggestions + +1. ~~Refresh the acceptance-criteria checklist in `docs/plans/NEON-30-implementation-plan.md` before merge. The current file still shows every box unchecked, which leaves the canonical story artifact out of sync with the implemented branch and weakens the plan-as-acceptance-criteria workflow this repo is using.~~ Done. All three AC boxes checked in `docs/plans/NEON-30-implementation-plan.md`. + +## Nits + +- ~~Nit: Consider adding a short NEON-30-specific manual check bullet to `client/README.md` near the existing movement verification steps so the expected "click behind the obstacle while opaque/faded" regression check is visible where contributors already look for client runtime checks.~~ Done. Added `### Manual check (NEON-30 — occluder click-through)` section in `client/README.md` covering opaque/faded occluder clicks and regression cases. + +## Verification + +- Install Godot 4.6 on the author machine, then from `client/` run `godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test`. +- Run the manual NEON-30 scenario from `client/scenes/main.tscn`: click the ground behind `World/NavigationRegion3D/Obstacle` while it is opaque and while it is faded by the occlusion system; confirm the player still receives a valid move target behind it. +- Re-check the existing non-occluder cases called out in `client/README.md`, especially normal floor clicks and the far pad / pedestal rejection behavior, to confirm the added occluder skip did not regress earlier ground-pick rules.