NEON-30: address review — check AC boxes, add manual-check section to README
parent
0fa2c0f93b
commit
1f8c0d04d1
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Reference in New Issue