From 0fa2c0f93b1ec0ab282d844fa1afc49a112b7f39 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Thu, 9 Apr 2026 22:51:22 -0400 Subject: [PATCH] NEON-30: skip occluder bodies in ground-pick raycast loop ground_pick.gd now checks _collider_is_occluder before the walkable break: when the ray hits a body in the "occluder" group it advances OCCLUDER_PICK_THROUGH past the hit point and continues, unconditionally and regardless of OcclusionPolicy fade state. Non-occluder click targeting is unchanged. Three unit tests added to ground_pick_test.gd covering occluder ancestry detection. Plan open questions and module doc updated. --- client/README.md | 4 +- client/scripts/ground_pick.gd | 21 ++++- client/test/ground_pick_test.gd | 39 ++++++++ .../E1_M2_IsometricCameraController.md | 4 +- docs/plans/NEON-30-implementation-plan.md | 90 +++++++++++++++++++ 5 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 docs/plans/NEON-30-implementation-plan.md diff --git a/client/README.md b/client/README.md index 150b09b..a94535d 100644 --- a/client/README.md +++ b/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`). @@ -106,7 +106,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. diff --git a/client/scripts/ground_pick.gd b/client/scripts/ground_pick.gd index 3322ed7..7b2188b 100644 --- a/client/scripts/ground_pick.gd +++ b/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 diff --git a/client/test/ground_pick_test.gd b/client/test/ground_pick_test.gd index 152766d..1c4b916 100644 --- a/client/test/ground_pick_test.gd +++ b/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() diff --git a/docs/decomposition/modules/E1_M2_IsometricCameraController.md b/docs/decomposition/modules/E1_M2_IsometricCameraController.md index 3ec9079..d886726 100644 --- a/docs/decomposition/modules/E1_M2_IsometricCameraController.md +++ b/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). diff --git a/docs/plans/NEON-30-implementation-plan.md b/docs/plans/NEON-30-implementation-plan.md new file mode 100644 index 0000000..8723119 --- /dev/null +++ b/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 + +- [ ] 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. + +## 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.