Merge pull request #36 from ViPro-Technologies/NEON-30-click-through-input

Neon 30 click through input

client/README.md

@@ -25,7 +25,7 @@ With the game server running ([`server/README.md`](../server/README.md)), each v
 
 **Idle stability (NEON-16):** **Jolt Physics**; **TPS** **120**. **`physics/common/physics_interpolation`** is **off** — with **on**, small physics **position** changes on bump **edges** were **blended** across render frames and looked like **ghosting / extra jitter**. **`snap_to_server()`** still calls **`reset_physics_interpolation()`** for compatibility if you turn interpolation on later. **`floor_max_angle`** **~50°** walking / **~35°** idle; **loose** angle **~0.8 s** after walk stops. **Walk step assist**. **Idle rim / straddle:** **moving** `floor_max_angle` when floor normal is **shallow** or slide hits mix **floor + wall**. One idle **`move_and_slide()`**, rim **settle**, **`random_floor_bump_mesh`** **lip / rim / vertical-wall** escape (**`IDLE_BUMP_ESCAPE_STEP`**, **`PLAYER_CAPSULE_RADIUS`**, collider fudge from **`random_floor_bump_collision_constants.gd`**). **`Player`:** interp **Off**, **`avoidance_enabled`** **false**, **`floor_block_on_wall`** **true** — do **not** rewrite **`global_transform`** in **`_process`** (can **ghost** **`CharacterBody3D`**). **`_snap_capsule_upright()`** after motion. Idle **`FLOOR_SNAP_IDLE`** ~**11 cm**; walking **`FLOOR_SNAP_MOVING`** ~**0.32**. **Rendering:** **`Mat_player_capsule`**, **`cast_shadow = 0`**, capsule mesh **+Y ~3.4 cm** (visual-only vs **`CapsuleShape3D`** — less **z-fight** vs floor/bump tops), **`light_specular = 0`**, **`msaa_3d = 2`**. **`safe_margin`** **0.055**.
 
-- **Scripts:** `scripts/ground_pick.gd` (walkable pick + `target_chosen`), `scripts/position_authority_client.gd` (`PositionAuthorityClient`: POST move, GET verify; second signal arg = boot snap vs nav goal), `scripts/player.gd` (path-follow), `scripts/isometric_follow_camera.gd` + `scripts/camera_state.gd` (NEON-25 follow), thin `scripts/main.gd` (nav bake on first frame, then wiring).
+- **Scripts:** `scripts/ground_pick.gd` (walkable pick + `target_chosen`; occluder bodies tagged `"occluder"` are passed through unconditionally so clicks reach the ground behind them — NEON-30), `scripts/position_authority_client.gd` (`PositionAuthorityClient`: POST move, GET verify; second signal arg = boot snap vs nav goal), `scripts/player.gd` (path-follow), `scripts/isometric_follow_camera.gd` + `scripts/camera_state.gd` (NEON-25 follow), thin `scripts/main.gd` (nav bake on first frame, then wiring).
 - **Scene:** `scenes/main.tscn` — walkable **`StaticBody3D`** geometry lives under **`World/NavigationRegion3D`**. `main.gd` waits one **`process_frame`**, spawns **random test bumps** on **`Floor`**, then **`bake_navigation_mesh(false)`** (main-thread bake), then waits two **`physics_frame`**s so **`NavigationServer3D`** has a map before agents query paths. The baked **`NavigationMesh`** asset in the scene file is **stale** until you run or re-bake in the editor.
 - **Inspector:** select **`PositionAuthorityClient`** on the main scene to set **`base_url`** (default `http://127.0.0.1:5253`) and **`dev_player_id`** (default `dev-local-1`, must match server `Game:DevPlayerId`).
 
@@ -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).
@@ -106,7 +114,7 @@ On **Linux** (including GitHub Actions), the path must be **`gdUnit4`** with a c
 
 **CI:** The **GDScript** workflow fails the job if Godot prints **`SCRIPT ERROR:`** or **`ERROR: Failed to load script`** while running GdUnit. GdUnit alone can still exit **0** when a test file fails to parse (that suite is skipped); the workflow guards against that.
 
-**Scope:** Unit tests cover **`player.gd`**, **`position_authority_client.gd`**, **`ground_pick.gd`** (walkable collider check), **`isometric_follow_camera.gd`** (eye math + effective zoom distance), **`camera_state.gd`**, and **`zoom_band_config.gd`**. **`main.gd`**, full **`_input` / ray pick** flows, and scene wiring are **not** automated here—use manual checks above.
+**Scope:** Unit tests cover **`player.gd`**, **`position_authority_client.gd`**, **`ground_pick.gd`** (walkable collider check + occluder ancestry check), **`isometric_follow_camera.gd`** (eye math + effective zoom distance), **`camera_state.gd`**, and **`zoom_band_config.gd`**. **`main.gd`**, full **`_input` / ray pick** flows, and scene wiring are **not** automated here—use manual checks above.
 
 **Camera zoom:** Input actions **`camera_zoom_in`** / **`camera_zoom_out`** (mouse wheel, **`=`** / **`-`**, keypad **+** / **−**) are defined in **`project.godot`**. On layouts where **`+`** requires **Shift**, remap or add a binding under **Project → Project Settings → Input Map** if needed.
 

client/scripts/ground_pick.gd

@@ -13,6 +13,9 @@ const MIN_WALKABLE_NORMAL_DOT_UP: float = 0.64
 ## Nudge past a steep walkable triangle so the ray can hit ground behind (meters along view ray).
 const STEEP_PICK_THROUGH: float = 0.14
 
+## Nudge past an occluder body so the ray reaches walkable ground behind it (meters along view ray).
+const OCCLUDER_PICK_THROUGH: float = 0.14
+
 ## Max ray segments (steep walkable → advance) before giving up.
 const MAX_PICK_SEGMENTS: int = 24
 
@@ -55,7 +58,14 @@ func _try_pick(screen_pos: Vector2) -> void:
 		var hit: Dictionary = space.intersect_ray(query)
 		if hit.is_empty():
 			break
-		if not _collider_is_walkable(hit.get("collider")):
+		var hit_collider: Variant = hit.get("collider")
+		if _collider_is_occluder(hit_collider):
+			var occluder_pos: Variant = hit.get("position")
+			if occluder_pos is not Vector3:
+				break
+			from = (occluder_pos as Vector3) + ray_dir * OCCLUDER_PICK_THROUGH
+			continue
+		if not _collider_is_walkable(hit_collider):
 			break
 		var hit_normal: Variant = hit.get("normal", Vector3.ZERO)
 		if hit_normal is not Vector3:
@@ -86,3 +96,12 @@ func _collider_is_walkable(collider: Variant) -> bool:
 			return true
 		n = n.get_parent()
 	return false
+
+
+func _collider_is_occluder(collider: Variant) -> bool:
+	var n: Node = collider as Node
+	while n:
+		if n.is_in_group("occluder"):
+			return true
+		n = n.get_parent()
+	return false

client/test/ground_pick_test.gd

@@ -28,3 +28,42 @@ func test_collider_is_walkable_false_without_walkable_group() -> void:
 	gp.add_child(body)
 	var ok := bool(gp.call("_collider_is_walkable", body))
 	assert_that(ok).is_false()
+
+
+func test_collider_is_occluder_true_when_ancestor_in_occluder_group() -> void:
+	var gp := Node3D.new()
+	gp.set_script(GroundPickScript)
+	auto_free(gp)
+	add_child(gp)
+	var obstacle := StaticBody3D.new()
+	obstacle.add_to_group("occluder")
+	var mesh_child := MeshInstance3D.new()
+	obstacle.add_child(mesh_child)
+	gp.add_child(obstacle)
+	var is_occluder := bool(gp.call("_collider_is_occluder", mesh_child))
+	assert_that(is_occluder).is_true()
+
+
+func test_collider_is_occluder_false_without_occluder_group() -> void:
+	var gp := Node3D.new()
+	gp.set_script(GroundPickScript)
+	auto_free(gp)
+	add_child(gp)
+	var body := StaticBody3D.new()
+	gp.add_child(body)
+	var is_occluder := bool(gp.call("_collider_is_occluder", body))
+	assert_that(is_occluder).is_false()
+
+
+func test_collider_is_occluder_false_for_walkable_only_ancestor() -> void:
+	var gp := Node3D.new()
+	gp.set_script(GroundPickScript)
+	auto_free(gp)
+	add_child(gp)
+	var floor_root := StaticBody3D.new()
+	floor_root.add_to_group("walkable")
+	var mesh_child := MeshInstance3D.new()
+	floor_root.add_child(mesh_child)
+	gp.add_child(floor_root)
+	var is_occluder := bool(gp.call("_collider_is_occluder", mesh_child))
+	assert_that(is_occluder).is_false()

docs/decomposition/modules/E1_M2_IsometricCameraController.md

@@ -55,7 +55,7 @@ Delivers an isometric follow camera that keeps the player readable during motion
 
 See Epic 1 **Slice 2 — Locked isometric camera**: follow-center, zoom bands, occlusion policy, **yaw fixed for players in prototype** (implementation still models yaw per [mid-project rotation policy](#mid-project-rotation-policy-practical-compromise)); optional telemetry such as throttled `camera_zoom_changed` and perf stress markers for occluders.
 
-## Implementation snapshot (NEON-25 + NEON-26 + NEON-27, 2026-04-08)
+## Implementation snapshot (NEON-25 + NEON-26 + NEON-27 + NEON-30, 2026-04-08 / 2026-04-09)
 
 - **Godot:** `client/scripts/isometric_follow_camera.gd` on **`World/IsometricFollowCamera`**; child **`Camera3D`** (current). **`scripts/camera_state.gd`** — `RefCounted` tick snapshot (`yaw` = orbit component, **0** while **`allow_yaw`** is false; **`distance`** = effective follow distance; **`zoom_band_index`**; **`focus_world`**, **`follow_target_path`**).
 - **Seam:** **`allow_yaw`** / **`max_yaw_deg`** on the rig; orbit would adjust **`_orbit_yaw_rad`** (no input bound yet). **Presentation** compass uses **`presentation_yaw_deg`** separately from orbit **`yaw`** in state.
@@ -64,6 +64,8 @@ See Epic 1 **Slice 2 — Locked isometric camera**: follow-center, zoom bands, o
 
   > **Prototype demo readability gate:** before shipping the prototype demo, the PR for NEON-27 must include a before/after screenshot or clip demonstrating that the player is no longer fully hidden by the `Obstacle`. This serves as the first data point for the occlusion-hiding-telegraphs risk documented below.
 
+- **Click-through input (NEON-30):** `scripts/ground_pick.gd` unconditionally skips bodies in the `"occluder"` group during click-to-move ground-pick raycasts. When a cast hits an occluder the ray origin advances `OCCLUDER_PICK_THROUGH` meters past the hit point and continues, independent of whether `OcclusionPolicy` is currently fading that body. This keeps the `"occluder"` group as the sole tagging convention shared between the camera occlusion system and ground-pick input — no dedicated collision layer is added for this case.
+
 ## Jira backlog
 
 Parent epic: [NEON-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NEON-1).

docs/plans/NEON-30-implementation-plan.md

@@ -0,0 +1,90 @@
+# NEON-30 — Implementation plan
+
+## Story reference
+
+| Field | Value |
+|--------|--------|
+| **Key** | NEON-30 |
+| **Title** | E1.M2: Click-through input — occluder geometry must not block ground-pick raycasts |
+| **Jira** | [NEON-30](https://neon-sprawl.atlassian.net/browse/NEON-30) |
+| **Parent** | [NEON-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NEON-1) |
+| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10) |
+
+## Goal, scope, and out-of-scope
+
+**Goal:** Ensure geometry tagged `"occluder"` never intercepts click-to-move ground picks, so clicking the ground behind faded or opaque occluders still yields the intended world target.
+
+**In scope**
+
+- Inspect `client/scripts/ground_pick.gd` and preserve its current walkable-surface rules while making occluders transparent to picking.
+- Make the exclusion unconditional: the click ray must ignore occluders whether or not NEON-27's `OcclusionPolicy` is currently fading them.
+- Keep non-occluder click targeting behavior unchanged on walkable surfaces.
+- Document the chosen approach in this implementation plan without introducing unexplained collision-layer constants.
+
+**Out of scope** (per Jira)
+
+- Changing the movement or navigation systems.
+- Server-side awareness of occluder state.
+- Input pass-through for geometry that is not tagged `"occluder"`.
+
+## Acceptance criteria checklist
+
+- [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
+
+### Preferred approach: explicit occluder skip in `ground_pick.gd`
+
+Keep the existing stepped raycast loop in `ground_pick.gd`, but treat `"occluder"` hits as a first-class skip case before the walkable-surface check. When the ray hits an occluder, advance the ray origin a small distance past the hit point along the ray direction and continue the loop, similar to the existing steep-surface pass-through behavior. This keeps the fix local to input picking, preserves NEON-27's scene convention (`"occluder"` group), and avoids changing the collision-layer contract that the occlusion system already uses.
+
+### Why not a dedicated occluder collision layer first
+
+Moving occluders to a new layer would also require updating the occlusion raycast mask in `isometric_follow_camera.gd`, plus scene data changes for every occluder. That is a broader contract change than this story needs and would add a new physics-layer convention that the acceptance criteria explicitly ask us to document carefully. Group-based exclusion in the pick loop is smaller, easier to verify, and reuses the convention already established by NEON-27.
+
+### Step-by-step implementation
+
+1. Add a small occluder helper path in `ground_pick.gd` (for example `_collider_is_occluder`) that walks collider ancestry the same way `_collider_is_walkable` does.
+2. Introduce a named pass-through distance constant for occluder skips rather than an inline float literal, mirroring the existing `STEEP_PICK_THROUGH` style so the code remains self-documenting.
+3. In the ray loop, if the hit collider is an occluder, move `from` slightly forward along the ray and continue without selecting or rejecting the click.
+4. Leave the current walkable / wall / steep-surface handling intact after the occluder check so non-occluder behavior stays stable.
+5. Extend the unit tests around collider ancestry helpers and document the remaining full click-through behavior as manual verification.
+
+## Decisions
+
+| Topic | Choice | Rationale |
+|-------|--------|-----------|
+| **Occluder exclusion mechanism** | Skip `"occluder"` hits in the ground-pick loop | Smallest change set; preserves NEON-27's established group tag; no new collision-layer convention needed. |
+| **Collision-layer strategy** | Do not add or repurpose layers for this story | Avoids extra scene churn and occlusion-ray regressions; satisfies the AC against unexplained magic numbers. |
+| **Scope boundary** | Fix only click picking, not movement or nav | Matches Jira scope and keeps this story focused on input selection correctness. |
+| **Debug warning on occluder skip** | No `push_warning` — silent pass-through | An occluder skip is intended, non-exceptional behavior; a warning would flood logs during normal play. Unlike the occlusion policy's skip of non-`StandardMaterial3D` surfaces (which signals a content mismatch), advancing past an occluder is the correct outcome. |
+
+## Files to add
+
+None. The expected change is a focused update to existing pick logic, tests, and supporting docs rather than a new script or scene resource.
+
+## Files to modify
+
+| Path | Rationale |
+|------|-----------|
+| `client/scripts/ground_pick.gd` | Add unconditional occluder-skip handling to the stepped raycast loop without changing existing walkable filtering semantics. |
+| `client/test/ground_pick_test.gd` | Add focused tests for occluder ancestry detection and any extracted helper logic that can be verified headlessly. |
+| `client/README.md` | Refresh the input-picking documentation so the current click-through behavior and its limits are recorded near other client runtime notes. |
+| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Update the module backlog / implementation snapshot if the final implementation adds a lasting input contract around occluders for this camera slice. |
+
+## Tests
+
+| File | Coverage |
+|------|----------|
+| `client/test/ground_pick_test.gd` | **Change.** Add helper-level tests confirming that a collider under an `"occluder"` ancestor is recognized as skippable, while non-occluder colliders are not. If logic is extracted into a pure helper, cover the helper directly. |
+
+**Manual verification:** Run the client and click the ground behind the `Obstacle` in `client/scenes/main.tscn` while it is opaque and while it is faded by the camera occlusion logic. Confirm the player receives a valid move target behind the obstacle, and confirm clicks on normal non-occluder walkable surfaces still behave as before.
+
+## Open questions / risks
+
+**Resolved during implementation:**
+
+- **Segment cap:** `MAX_PICK_SEGMENTS = 24` leaves ample headroom in the prototype with a single `Obstacle`. Each occluder skip consumes one iteration; the remaining segments are still available for steep-surface pass-through. No cap change needed.
+- **Pass-through epsilon:** Named constant `OCCLUDER_PICK_THROUGH = 0.14` introduced (same value as `STEEP_PICK_THROUGH`; separate name for self-documentation since the two cases are conceptually distinct). Chosen to match the already-validated steep-surface nudge distance.
+- **Module doc update:** E1_M2_IsometricCameraController.md updated with the NEON-30 click-through contract (group-based exclusion, no new collision layer) to record the shared `"occluder"` group convention for future readers.

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.