From 185af5a7f698f298d127bb7d76309864792a3a3e Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 23:21:51 -0400 Subject: [PATCH 1/8] NEON-27: add implementation plan for OcclusionPolicy Documents RayCast-based material-fade approach, files to add/modify, GdUnit4 test coverage, and readability risk gate note. --- docs/plans/NEON-27-implementation-plan.md | 115 ++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 docs/plans/NEON-27-implementation-plan.md diff --git a/docs/plans/NEON-27-implementation-plan.md b/docs/plans/NEON-27-implementation-plan.md new file mode 100644 index 0000000..1478dee --- /dev/null +++ b/docs/plans/NEON-27-implementation-plan.md @@ -0,0 +1,115 @@ +# NEON-27 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEON-27 | +| **Title** | E1.M2: OcclusionPolicy — keep player readable through geometry | +| **Jira** | [NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27) | +| **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:** Implement `OcclusionPolicy` so static geometry between the camera and the player does not fully hide the player silhouette or combat telegraphs. + +**In scope** + +- Choose and implement a prototype-appropriate occlusion technique: **RayCast-based material fade** (see [Technical approach](#technical-approach)). +- New `OcclusionPolicy` **Resource** (`occlusion_policy.gd`) — data-driven config for fade alpha, fade duration, affected collision mask, cast depth cap, and optional perf-log threshold; follows the same no-`class_name` convention as `ZoomBandConfig`. +- Wire `OcclusionPolicy` into `isometric_follow_camera.gd` as an `@export` so it runs consistently in the same camera controller stack as follow + zoom. +- Tag the prototype district `Obstacle` node (a 2 × 2 × 2 box at `(6, 0, 5)`) with the **`"occluder"`** group; assign the new `.tres` on the `IsometricFollowCamera` rig in `main.tscn`. +- Optional perf marker: when `occluder_count_log_threshold > 0` and the active occluder count meets or exceeds the threshold, emit `push_warning` for later stress-test reference. +- Update `E1_M2_IsometricCameraController.md` with an implementation snapshot and the readability risk gate note required by acceptance criteria. + +**Out of scope** (per Jira) + +- Perfect transparency for all scene assets; full art pipeline integration. +- Server awareness of occlusion. +- Smooth occlusion handling for moving/dynamic objects (static geometry only for prototype). +- Yaw orbit or camera nudge approach (would break framing; fixed rig stays). + +## Acceptance criteria checklist + +- [ ] In a documented test setup, occlusion **no longer fully hides** the player in the common worst case (before/after screenshot or short clip note in PR). +- [ ] Policy is **configurable or data-driven** enough to iterate without rewriting core follow logic (`OcclusionPolicy` resource swappable in inspector; `fade_alpha` and `fade_duration` are exported). +- [ ] Risk called out in module doc: occlusion hiding telegraphs — link to **readability checklist** or gate note for prototype demo. + +## Technical approach + +### Technique: RayCast-based per-surface material fade + +Each `_process` frame, after the camera eye is positioned, cast a ray from the smoothed eye position (`_smoothed_eye`) to the player's focus point. Any `StaticBody3D` (or similar) intersecting the ray that is tagged with the **`"occluder"`** group has its `MeshInstance3D` children' surfaces overridden with a **faded copy** of their materials (alpha reduced to `fade_alpha`). When a body leaves the ray path, its original surface materials are restored. + +**Why this technique:** pure GDScript — no custom shader required for prototype; configurable; reversible; respects combat readability by making geometry semi-transparent rather than fully hiding it. Camera nudge would break the fixed isometric framing. A cutout shader would require art pipeline work outside the prototype scope. + +**Why group-based (`"occluder"`) rather than collision-layer filtering alone:** the prototype `Floor` (`StaticBody3D`) shares the default collision layer 1 with the `Obstacle`. Filtering by group is explicit and avoids false hits from the floor geometry without requiring separate collision-layer assignments. + +**Player exclusion:** the `Player` is on collision layer 2 (independent of layer 1); the default `occluder_collision_mask` of `1` already excludes the player body without needing to add it to the ray exclusion list. Confirmed from `main.tscn`: `Player.collision_layer = 2`. + +**Material override strategy:** for each `MeshInstance3D` child of a blocking body, iterate surfaces. For each surface, save the current `surface_override_material` (may be null). Create a transparent override: if the surface's mesh material is a `StandardMaterial3D`, `duplicate()` it; otherwise create a new `StandardMaterial3D` inheriting only `albedo_color`. Set `transparency = BaseMaterial3D.TRANSPARENCY_ALPHA` and `albedo_color.a = fade_alpha`. Restore by setting `surface_override_material` back to the saved value (or `null`) when the body is no longer blocking. + +**Fade timing:** use a `Tween` on the camera rig to animate the alpha from 1.0 → `fade_alpha` over `fade_duration` seconds (and back when restoring), avoiding jarring pop. State: `_occluder_overrides: Dictionary` mapping `Node3D → Array[{mesh, surface, original_mat, tween}]`. + +### Step-by-step implementation + +1. **`occlusion_policy.gd`** — `extends Resource`, no `class_name`. Exported fields: + - `enabled: bool = true` + - `fade_alpha: float = 0.25` — target alpha when occluding (0 = invisible, 1 = opaque). + - `fade_duration: float = 0.15` — tween duration in seconds for appear/disappear. + - `occluder_group: String = "occluder"` — group that tags scene nodes as potential occluders. + - `occluder_collision_mask: int = 1` — physics layers the ray checks. + - `max_occluder_cast_depth: int = 4` — max successive intersect_ray calls per frame (caps cost). + - `occluder_count_log_threshold: int = 0` — if > 0, push_warning when active count ≥ threshold. + - Validation helper: `func is_valid() -> bool` — returns `true` if `enabled` and `fade_alpha >= 0`. + +2. **`isometric_follow_camera.gd`** changes: + - Add `@export var occlusion_policy: Resource`. + - Add private state: `_occluder_overrides: Dictionary = {}`. + - Add `_occlusion_policy_valid() -> bool` (mirrors `_zoom_config_valid`): checks script identity and `is_valid()`. + - Call `_update_occlusion(focus)` at end of `_process` after the eye / look_at update. + - `_update_occlusion(focus: Vector3) -> void`: if policy invalid, call `_restore_all_occluders()` and return. Otherwise, cast iterative rays (up to `max_occluder_cast_depth`), filter hits by `occluder_group`; restore bodies no longer blocking; fade newly blocking bodies. Emit perf warning when threshold exceeded. + - `_apply_occluder_fade(body: Node3D) -> void`: enumerate `MeshInstance3D` descendants, save overrides, create faded materials, tween alpha in. + - `_restore_occluder(body: Node3D) -> void`: tween alpha back to 1.0 then restore original materials, remove from `_occluder_overrides`. + - `_restore_all_occluders() -> void`: call `_restore_occluder` for all entries (on disable/null policy). + +3. **`isometric_occlusion_policy.tres`** — default `OcclusionPolicy` resource (fade_alpha=0.25, fade_duration=0.15, mask=1, depth=4, threshold=0). Assign to `IsometricFollowCamera` in `main.tscn`. + +4. **`main.tscn`** — Add `"occluder"` group to the `Obstacle` node; assign `occlusion_policy` resource on `IsometricFollowCamera`. + +5. **`E1_M2_IsometricCameraController.md`** — Add NEON-27 implementation snapshot; add readability risk gate note (links to acceptance criteria: before-/after screenshot in PR required at prototype demo gate). + +## Files to add + +| Path | Purpose | +|------|---------| +| `client/scripts/occlusion_policy.gd` | `Resource` type holding fade and detection config; `is_valid()` helper; no `class_name`. | +| `client/resources/isometric_occlusion_policy.tres` | Default `OcclusionPolicy` instance for the prototype rig. | +| `client/test/occlusion_policy_test.gd` | GdUnit4 unit tests for the policy resource (see [Tests](#tests)). | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `client/scripts/isometric_follow_camera.gd` | Add `occlusion_policy` export, `_update_occlusion` pipeline, occluder override state, fade/restore helpers, `_occlusion_policy_valid` guard. | +| `client/scenes/main.tscn` | Tag `Obstacle` with `"occluder"` group; assign `isometric_occlusion_policy.tres` on `IsometricFollowCamera`. | +| `client/test/isometric_follow_camera_test.gd` | Add test for `_occlusion_policy_valid`-equivalent static path (null policy → false; wrong-script resource → false; valid resource → true) via a static helper if possible; otherwise document as manual-only. | +| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Add NEON-27 implementation snapshot; add readability risk gate note per AC. | + +## Tests + +| File | Coverage | +|------|----------| +| `client/test/occlusion_policy_test.gd` | **New.** `is_valid()` returns true with defaults; returns false when `enabled = false`; `fade_alpha` edge cases (0.0 valid, negative invalid if applicable). Default field values match resource. | +| `client/test/isometric_follow_camera_test.gd` | **Change.** Add a static-helper test for occlusion policy guard: confirm that a null resource and a wrong-script resource are rejected, and a correct-script `OcclusionPolicy` resource is accepted (no physics engine needed — tests the guard function logic only). | + +**Manual verification:** Walk the player behind the `Obstacle` node in the running client. The obstacle should fade to semi-transparent (alpha ~0.25) as the player passes behind it, then restore when the player moves clear. Confirm `CameraState` / camera framing are unaffected. Capture a brief clip or before/after screenshot for the PR. + +## Open questions / risks + +- **Tween lifecycle:** `Tween`s created on the rig must be killed (`tween.kill()`) when a body is re-acquired as an occluder before the restore tween completes, to avoid competing animations on the same material. Track the active tween per body in `_occluder_overrides`. +- **Shared vs. per-instance materials:** if the `Obstacle` in the future uses a shared `StandardMaterial3D` from the mesh resource directly (no override set), `duplicate()` ensures we don't mutate the shared asset. This is already handled by the override strategy above, but must be verified at runtime. +- **`max_occluder_cast_depth` performance:** each `intersect_ray` call with growing exclusion list is O(scene complexity). Cap of 4 is conservative for the prototype district; tune if the perf log fires. +- **Scene grows (NEON-29):** when the prototype district expands (NEON-29), add representative occluders to the `"occluder"` group and rerun the manual verification pass. +- **Readability gate:** the risk documented in the module doc (occlusion hiding telegraphs) must be surfaced in the prototype demo checklist. Before-/after evidence in this PR's description serves as the first data point. From b2d2500e6ab86c33623ed63dd42267d9d51a72a5 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 23:28:02 -0400 Subject: [PATCH 2/8] NEON-27: record planning decisions in implementation plan Occluder ID: explicit group tag. Fade: instant (no Tween). Non-StandardMaterial3D surfaces: skip + push_warning. Rationale documented in Decisions table. --- docs/plans/NEON-27-implementation-plan.md | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/docs/plans/NEON-27-implementation-plan.md b/docs/plans/NEON-27-implementation-plan.md index 1478dee..7bbde6f 100644 --- a/docs/plans/NEON-27-implementation-plan.md +++ b/docs/plans/NEON-27-implementation-plan.md @@ -48,16 +48,15 @@ Each `_process` frame, after the camera eye is positioned, cast a ray from the s **Player exclusion:** the `Player` is on collision layer 2 (independent of layer 1); the default `occluder_collision_mask` of `1` already excludes the player body without needing to add it to the ray exclusion list. Confirmed from `main.tscn`: `Player.collision_layer = 2`. -**Material override strategy:** for each `MeshInstance3D` child of a blocking body, iterate surfaces. For each surface, save the current `surface_override_material` (may be null). Create a transparent override: if the surface's mesh material is a `StandardMaterial3D`, `duplicate()` it; otherwise create a new `StandardMaterial3D` inheriting only `albedo_color`. Set `transparency = BaseMaterial3D.TRANSPARENCY_ALPHA` and `albedo_color.a = fade_alpha`. Restore by setting `surface_override_material` back to the saved value (or `null`) when the body is no longer blocking. +**Material override strategy:** for each `MeshInstance3D` child of a blocking body, iterate surfaces. For each surface, save the current `surface_override_material` (may be null). Resolve the effective material: prefer the surface override; fall back to `mesh.surface_get_material(i)`. If that material is a `StandardMaterial3D`, `duplicate()` it, set `transparency = BaseMaterial3D.TRANSPARENCY_ALPHA` and `albedo_color.a = fade_alpha`, then apply as the surface override. If it is not a `StandardMaterial3D` (e.g. `ShaderMaterial` or null), **skip that surface silently and emit `push_warning`** — do not substitute a plain gray material. Restore by setting `surface_override_material` back to the saved value (or `null`) when the body is no longer blocking. -**Fade timing:** use a `Tween` on the camera rig to animate the alpha from 1.0 → `fade_alpha` over `fade_duration` seconds (and back when restoring), avoiding jarring pop. State: `_occluder_overrides: Dictionary` mapping `Node3D → Array[{mesh, surface, original_mat, tween}]`. +**Fade timing:** instant — set `albedo_color.a` to `fade_alpha` on entry, restore immediately on exit. No Tween; no `fade_duration` export. A smooth-fade pass can be added post-prototype if the pop is distracting during the demo playtest. State: `_occluder_overrides: Dictionary` mapping `Node3D → Array[{mesh, surface, original_mat}]`. ### Step-by-step implementation 1. **`occlusion_policy.gd`** — `extends Resource`, no `class_name`. Exported fields: - `enabled: bool = true` - `fade_alpha: float = 0.25` — target alpha when occluding (0 = invisible, 1 = opaque). - - `fade_duration: float = 0.15` — tween duration in seconds for appear/disappear. - `occluder_group: String = "occluder"` — group that tags scene nodes as potential occluders. - `occluder_collision_mask: int = 1` — physics layers the ray checks. - `max_occluder_cast_depth: int = 4` — max successive intersect_ray calls per frame (caps cost). @@ -70,8 +69,8 @@ Each `_process` frame, after the camera eye is positioned, cast a ray from the s - Add `_occlusion_policy_valid() -> bool` (mirrors `_zoom_config_valid`): checks script identity and `is_valid()`. - Call `_update_occlusion(focus)` at end of `_process` after the eye / look_at update. - `_update_occlusion(focus: Vector3) -> void`: if policy invalid, call `_restore_all_occluders()` and return. Otherwise, cast iterative rays (up to `max_occluder_cast_depth`), filter hits by `occluder_group`; restore bodies no longer blocking; fade newly blocking bodies. Emit perf warning when threshold exceeded. - - `_apply_occluder_fade(body: Node3D) -> void`: enumerate `MeshInstance3D` descendants, save overrides, create faded materials, tween alpha in. - - `_restore_occluder(body: Node3D) -> void`: tween alpha back to 1.0 then restore original materials, remove from `_occluder_overrides`. + - `_apply_occluder_fade(body: Node3D) -> void`: enumerate `MeshInstance3D` descendants, save overrides, create faded `StandardMaterial3D` copies (instant); skip non-`StandardMaterial3D` surfaces with `push_warning`. + - `_restore_occluder(body: Node3D) -> void`: restore original surface override materials (instant), remove from `_occluder_overrides`. - `_restore_all_occluders() -> void`: call `_restore_occluder` for all entries (on disable/null policy). 3. **`isometric_occlusion_policy.tres`** — default `OcclusionPolicy` resource (fade_alpha=0.25, fade_duration=0.15, mask=1, depth=4, threshold=0). Assign to `IsometricFollowCamera` in `main.tscn`. @@ -80,6 +79,14 @@ Each `_process` frame, after the camera eye is positioned, cast a ray from the s 5. **`E1_M2_IsometricCameraController.md`** — Add NEON-27 implementation snapshot; add readability risk gate note (links to acceptance criteria: before-/after screenshot in PR required at prototype demo gate). +## Decisions + +| Topic | Choice | Rationale | +|-------|--------|-----------| +| **Occluder identification** | Explicit `"occluder"` group tag (Option A) | Precise; failure mode is obvious ("wall doesn't fade — tag it"); no false positives as scene grows; collision layers stay single-purpose. | +| **Fade timing** | Instant alpha override — no Tween | Eliminates Tween race conditions (stuck-transparent geometry); simpler state; satisfies AC; smooth-fade pass deferred to post-prototype if pop is distracting at demo. | +| **Non-`StandardMaterial3D` surfaces** | Skip silently + `push_warning` | Avoids unexpected appearance change; prototype `Obstacle` is unaffected (uses `StandardMaterial3D`); warning surfaces future mismatches without hiding them. | + ## Files to add | Path | Purpose | @@ -108,7 +115,7 @@ Each `_process` frame, after the camera eye is positioned, cast a ray from the s ## Open questions / risks -- **Tween lifecycle:** `Tween`s created on the rig must be killed (`tween.kill()`) when a body is re-acquired as an occluder before the restore tween completes, to avoid competing animations on the same material. Track the active tween per body in `_occluder_overrides`. +- **Rapid toggle at wall edges:** with instant override, a player sidestepping at the edge of an occluder may cause per-frame toggle. This is visually acceptable for prototype; if it proves distracting, a one-frame hysteresis (require N consecutive frames clear before restoring) can be added without a structural change. - **Shared vs. per-instance materials:** if the `Obstacle` in the future uses a shared `StandardMaterial3D` from the mesh resource directly (no override set), `duplicate()` ensures we don't mutate the shared asset. This is already handled by the override strategy above, but must be verified at runtime. - **`max_occluder_cast_depth` performance:** each `intersect_ray` call with growing exclusion list is O(scene complexity). Cap of 4 is conservative for the prototype district; tune if the perf log fires. - **Scene grows (NEON-29):** when the prototype district expands (NEON-29), add representative occluders to the `"occluder"` group and rerun the manual verification pass. From 15d405261f5a15178aa697415d7e499eb1ac138f Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 23:34:00 -0400 Subject: [PATCH 3/8] =?UTF-8?q?NEON-27:=20implement=20OcclusionPolicy=20?= =?UTF-8?q?=E2=80=94=20RayCast-based=20material=20fade?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds OcclusionPolicy resource and wires it into IsometricFollowCamera. Each _process frame, iterative rays from the camera eye to the player focus detect bodies tagged "occluder"; their MeshInstance3D surfaces are overridden with a transparent StandardMaterial3D (fade_alpha=0.25, instant). Materials restore when the body clears the ray. Null-material surfaces (e.g. the prototype Obstacle with no material assigned) receive a plain transparent StandardMaterial3D so they fade correctly. - client/scripts/occlusion_policy.gd — new Resource (enabled, fade_alpha, occluder_group, occluder_collision_mask, max_occluder_cast_depth, occluder_count_log_threshold, is_valid) - client/resources/isometric_occlusion_policy.tres — default instance - client/scripts/isometric_follow_camera.gd — occlusion_policy export, _update_occlusion pipeline, _apply_occluder_fade/_restore_occluder helpers, static occlusion_policy_is_valid guard, _exit_tree cleanup - client/scenes/main.tscn — Obstacle tagged "occluder", policy assigned - client/test/occlusion_policy_test.gd — new GdUnit4 unit tests - client/test/isometric_follow_camera_test.gd — policy guard static tests - docs/decomposition/modules/E1_M2_IsometricCameraController.md — NEON-27 snapshot + readability demo gate note - docs/plans/NEON-27-implementation-plan.md — null-material decision recorded --- .../resources/isometric_occlusion_policy.tres | 12 ++ client/scenes/main.tscn | 4 +- client/scripts/isometric_follow_camera.gd | 124 ++++++++++++++++++ client/scripts/occlusion_policy.gd | 30 +++++ client/test/isometric_follow_camera_test.gd | 23 +++- client/test/occlusion_policy_test.gd | 43 ++++++ .../E1_M2_IsometricCameraController.md | 7 +- docs/plans/NEON-27-implementation-plan.md | 7 +- 8 files changed, 243 insertions(+), 7 deletions(-) create mode 100644 client/resources/isometric_occlusion_policy.tres create mode 100644 client/scripts/occlusion_policy.gd create mode 100644 client/test/occlusion_policy_test.gd diff --git a/client/resources/isometric_occlusion_policy.tres b/client/resources/isometric_occlusion_policy.tres new file mode 100644 index 0000000..3e7c44d --- /dev/null +++ b/client/resources/isometric_occlusion_policy.tres @@ -0,0 +1,12 @@ +[gd_resource type="Resource" load_steps=2 format=3] + +[ext_resource type="Script" path="res://scripts/occlusion_policy.gd" id="1_occ_pol"] + +[resource] +script = ExtResource("1_occ_pol") +enabled = true +fade_alpha = 0.25 +occluder_group = "occluder" +occluder_collision_mask = 1 +max_occluder_cast_depth = 4 +occluder_count_log_threshold = 0 diff --git a/client/scenes/main.tscn b/client/scenes/main.tscn index 1e98450..e954510 100644 --- a/client/scenes/main.tscn +++ b/client/scenes/main.tscn @@ -8,6 +8,7 @@ [ext_resource type="Script" uid="uid://n3p4f3fky3nv" path="res://scripts/interaction_radius_indicators.gd" id="6_rad"] [ext_resource type="Script" uid="uid://b8x7c4m3r5t8t2" path="res://scripts/isometric_follow_camera.gd" id="7_iso_cam"] [ext_resource type="Resource" path="res://resources/isometric_zoom_bands.tres" id="8_zoom_bands"] +[ext_resource type="Resource" path="res://resources/isometric_occlusion_policy.tres" id="9_occ_pol"] [sub_resource type="NavigationMesh" id="NavigationMesh_district"] vertices = PackedVector3Array(-0.6999998, 0.25, -0.6999998, -0.10000038, 0.25, -0.6999998, -0.10000038, 0.25, -9.55, -9.55, 0.25, 0.05000019, -1, 0.25, 0.05000019, -9.55, 0.25, -9.55, 8.75, 0.25, -7.4500003, 8.75, 0.25, -6.4000006, 9.65, 0.25, -6.4000006, 9.65, 0.25, -9.55, 0.5, 0.25, -0.6999998, 6.6499996, 0.25, -5.2000003, 6.200001, 0.25, -5.5, 8.450001, 0.25, -7.75, 6.5, 0.25, -7.75, 6.200001, 0.25, -7.4500003, 7.1000004, 2.8000002, -6.8500004, 7.1000004, 2.8000002, -6.1000004, 7.8500004, 2.8000002, -6.1000004, 7.8500004, 2.8000002, -6.8500004, 7.25, 0.25, -6.7000003, 7.25, 0.25, -6.25, 7.700001, 0.25, -6.25, 7.700001, 0.25, -6.7000003, 8.75, 0.25, -5.5, 7.550001, 0.25, 3.8000002, 7.550001, 0.25, 5, 9.65, 0.25, 5, 7.25, 0.25, 3.5, 0.9499998, 0.25, 0.5, 0.5, 0.25, 0.8000002, 4.55, 0.25, 3.9499998, 4.8500004, 0.25, 3.5, 8.450001, 0.25, -5.2000003, 0.9499998, 0.25, -0.39999962, -0.6999998, 0.25, 0.8000002, 4.55, 0.25, 6.200001, 4.55, 0.25, 9.65, -9.55, 0.25, 9.65, 5.45, 2.2, 4.4000006, 5.45, 2.2, 5.6000004, 6.6499996, 2.2, 5.6000004, 6.6499996, 2.2, 4.4000006, 5.6000004, 0.25, 4.55, 5.6000004, 0.25, 5.45, 6.5, 0.25, 5.45, 6.5, 0.25, 4.55, 7.550001, 0.25, 6.200001, 7.25, 0.25, 6.5, 9.800001, 0.40000004, 9.800001, 4.8500004, 0.25, 6.5) @@ -91,6 +92,7 @@ shadow_enabled = true transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 12, 10, 12) script = ExtResource("7_iso_cam") zoom_band_config = ExtResource("8_zoom_bands") +occlusion_policy = ExtResource("9_occ_pol") [node name="Camera3D" type="Camera3D" parent="World/IsometricFollowCamera" unique_id=1124088857] current = true @@ -141,7 +143,7 @@ surface_material_override/0 = SubResource("Mat_move_reject_farpad") [node name="CollisionShape3D" type="CollisionShape3D" parent="World/NavigationRegion3D/MoveRejectFarPad" unique_id=1900033] shape = SubResource("BoxShape3D_move_reject_farpad") -[node name="Obstacle" type="StaticBody3D" parent="World/NavigationRegion3D" unique_id=1638845763] +[node name="Obstacle" type="StaticBody3D" parent="World/NavigationRegion3D" unique_id=1638845763 groups=["occluder"]] transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 6, 0, 5) [node name="MeshInstance3D" type="MeshInstance3D" parent="World/NavigationRegion3D/Obstacle" unique_id=750473628] diff --git a/client/scripts/isometric_follow_camera.gd b/client/scripts/isometric_follow_camera.gd index 3b5ac20..01ccefd 100644 --- a/client/scripts/isometric_follow_camera.gd +++ b/client/scripts/isometric_follow_camera.gd @@ -12,6 +12,7 @@ signal zoom_band_changed(new_index: int, distance: float) const CameraStateScript := preload("res://scripts/camera_state.gd") const ZoomBandConfigScript := preload("res://scripts/zoom_band_config.gd") +const OcclusionPolicyScript := preload("res://scripts/occlusion_policy.gd") ## Tuned to match pre-NEON-25 static `Camera3D` at (12,10,12) vs player at (-5,0.9,-5). ## When [member zoom_band_config] is null or has no bands, this is the sole follow distance. @@ -24,6 +25,9 @@ const ZoomBandConfigScript := preload("res://scripts/zoom_band_config.gd") ## Designer-tunable discrete bands (`res://resources/isometric_zoom_bands.tres` on main rig). @export var zoom_band_config: Resource +## RayCast-based occluder fade config (`res://resources/isometric_occlusion_policy.tres`). +@export var occlusion_policy: Resource + @export var allow_yaw: bool = false @export var max_yaw_deg: float = 45.0 @@ -42,6 +46,8 @@ var _smoothed_eye: Vector3 = Vector3.ZERO var _orbit_yaw_rad: float = 0.0 var _zoom_band_index: int = 0 var _warned_missing_follow_target: bool = false +## Keys: Node3D occluder bodies. Values: Array of {mesh, surface, original} restore entries. +var _occluder_overrides: Dictionary = {} @onready var camera: Camera3D = $Camera3D @@ -103,6 +109,11 @@ func _process(delta: float) -> void: global_position = _smoothed_eye camera.look_at(focus, Vector3.UP) _sync_camera_state(focus, effective_dist) + _update_occlusion(focus) + + +func _exit_tree() -> void: + _restore_all_occluders() func _unhandled_input(event: InputEvent) -> void: @@ -196,3 +207,116 @@ static func desired_eye_world( ) -> Vector3: var h: float = distance * cos(pitch_elevation_rad) return focus + Vector3(h * sin(yaw_rad), distance * sin(pitch_elevation_rad), h * cos(yaw_rad)) + + +## Returns true when [member occlusion_policy] is a valid, enabled [OcclusionPolicy]. +## Mirrors [method _zoom_config_valid]; delegates to the static helper so tests can call +## it without a live Node. +func _occlusion_policy_valid() -> bool: + return occlusion_policy_is_valid(occlusion_policy) + + +## Static guard — safe to call from tests without a live scene. +static func occlusion_policy_is_valid(policy: Resource) -> bool: + return ( + policy != null + and policy.get_script() == OcclusionPolicyScript + and policy.is_valid() + ) + + +## Cast iterative rays from the smoothed eye to the player focus. +## Bodies in the [member occluder_group] that intersect the ray are faded; +## bodies that leave the path have their materials restored. +func _update_occlusion(focus: Vector3) -> void: + if not _occlusion_policy_valid(): + _restore_all_occluders() + return + + var policy = occlusion_policy + var space_state := get_world_3d().direct_space_state + var newly_blocking: Array[Node3D] = [] + var excluded: Array[RID] = [] + + for _i in range(policy.max_occluder_cast_depth): + var params := PhysicsRayQueryParameters3D.create( + _smoothed_eye, focus, policy.occluder_collision_mask + ) + params.exclude = excluded + var result := space_state.intersect_ray(params) + if result.is_empty(): + break + var body = result.get("collider") + if body is Node3D and (body as Node3D).is_in_group(policy.occluder_group): + newly_blocking.append(body as Node3D) + excluded.append(result.get("rid")) + + if ( + policy.occluder_count_log_threshold > 0 + and newly_blocking.size() >= policy.occluder_count_log_threshold + ): + push_warning( + "OcclusionPolicy: %d occluder(s) active (threshold %d)" % [ + newly_blocking.size(), policy.occluder_count_log_threshold + ] + ) + + var to_restore: Array = _occluder_overrides.keys() + for body in to_restore: + if body not in newly_blocking: + _restore_occluder(body) + + for body in newly_blocking: + if body not in _occluder_overrides: + _apply_occluder_fade(body) + + +## Apply per-surface fade overrides to all [MeshInstance3D] children of [param body]. +## - [StandardMaterial3D]: duplicated and alpha-set to [member OcclusionPolicy.fade_alpha]. +## - null (no material): a plain [StandardMaterial3D] is created so the surface fades. +## - Anything else (e.g. [ShaderMaterial]): skipped with [method push_warning]. +func _apply_occluder_fade(body: Node3D) -> void: + var policy = occlusion_policy + var overrides: Array = [] + for mesh_node in body.find_children("*", "MeshInstance3D", true, false): + var mi: MeshInstance3D = mesh_node + var n: int = mi.get_surface_override_material_count() + for s in range(n): + var saved: Material = mi.get_surface_override_material(s) + var effective: Material = saved + if effective == null and mi.mesh != null: + effective = mi.mesh.surface_get_material(s) + var faded: StandardMaterial3D + if effective is StandardMaterial3D: + faded = (effective as StandardMaterial3D).duplicate() + elif effective == null: + faded = StandardMaterial3D.new() + else: + push_warning( + "OcclusionPolicy: %s surface %d is %s — skipping fade." % [ + mi.get_path(), s, effective.get_class() + ] + ) + continue + faded.transparency = BaseMaterial3D.TRANSPARENCY_ALPHA + faded.albedo_color.a = policy.fade_alpha + mi.set_surface_override_material(s, faded) + overrides.append({"mesh": mi, "surface": s, "original": saved}) + _occluder_overrides[body] = overrides + + +## Restore saved surface override materials on [param body] and remove it from tracking. +func _restore_occluder(body: Node3D) -> void: + if not _occluder_overrides.has(body): + return + for entry in _occluder_overrides[body]: + var mi: MeshInstance3D = entry["mesh"] + if is_instance_valid(mi): + mi.set_surface_override_material(entry["surface"], entry["original"]) + _occluder_overrides.erase(body) + + +## Restore all currently faded occluders (called on policy disable or node exit). +func _restore_all_occluders() -> void: + for body in _occluder_overrides.keys(): + _restore_occluder(body) diff --git a/client/scripts/occlusion_policy.gd b/client/scripts/occlusion_policy.gd new file mode 100644 index 0000000..6d716c3 --- /dev/null +++ b/client/scripts/occlusion_policy.gd @@ -0,0 +1,30 @@ +extends Resource + +## OcclusionPolicy — config for the RayCast-based per-surface material fade used by +## [IsometricFollowCamera] (NEON-27). No `class_name` — see repo Godot headless / CI notes. + +## Master switch. When false, no rays are cast and any active overrides are restored. +@export var enabled: bool = true + +## Target alpha applied to occluder surfaces (0 = fully transparent, 1 = opaque). +@export_range(0.0, 1.0) var fade_alpha: float = 0.25 + +## Scene group that opts a [CollisionObject3D] into occlusion fading. +@export var occluder_group: String = "occluder" + +## Physics collision mask for the ray. Default 1 (world geometry layer). +## The Player is on layer 2 and is already excluded with this default. +@export_flags_3d_physics var occluder_collision_mask: int = 1 + +## Max successive [method PhysicsDirectSpaceState3D.intersect_ray] calls per frame. +## Caps cost when occluders are stacked along the ray. +@export var max_occluder_cast_depth: int = 4 + +## When > 0, emits [method push_warning] when the active occluder count reaches this +## value. Set to 0 to disable. Marker for stress-test reference (TODO E9.M1 telemetry). +@export var occluder_count_log_threshold: int = 0 + + +## Returns true when the policy should be applied this frame. +func is_valid() -> bool: + return enabled and fade_alpha >= 0.0 diff --git a/client/test/isometric_follow_camera_test.gd b/client/test/isometric_follow_camera_test.gd index 5bae4d4..542d935 100644 --- a/client/test/isometric_follow_camera_test.gd +++ b/client/test/isometric_follow_camera_test.gd @@ -1,8 +1,9 @@ -# Static eye-position helper for res://scripts/isometric_follow_camera.gd (NEON-25 framing). +# Static helpers for res://scripts/isometric_follow_camera.gd (NEON-25 framing, NEON-27 guard). extends GdUnitTestSuite const IsoScript := preload("res://scripts/isometric_follow_camera.gd") const ZoomBandConfigScript := preload("res://scripts/zoom_band_config.gd") +const OcclusionPolicyScript := preload("res://scripts/occlusion_policy.gd") func test_desired_eye_matches_previous_static_camera_frame() -> void: @@ -41,3 +42,23 @@ func test_effective_follow_distance_fallback_when_band_non_positive() -> void: var cfg = ZoomBandConfigScript.new() cfg.band_distances = PackedFloat32Array([10.0, 0.0, 30.0]) assert_that(IsoScript.effective_follow_distance(cfg, 2, 99.0)).is_equal(99.0) + + +func test_occlusion_policy_is_valid_null() -> void: + assert_that(IsoScript.occlusion_policy_is_valid(null)).is_false() + + +func test_occlusion_policy_is_valid_wrong_script() -> void: + var wrong = ZoomBandConfigScript.new() + assert_that(IsoScript.occlusion_policy_is_valid(wrong)).is_false() + + +func test_occlusion_policy_is_valid_correct_script() -> void: + var p = OcclusionPolicyScript.new() + assert_that(IsoScript.occlusion_policy_is_valid(p)).is_true() + + +func test_occlusion_policy_is_valid_false_when_disabled() -> void: + var p = OcclusionPolicyScript.new() + p.enabled = false + assert_that(IsoScript.occlusion_policy_is_valid(p)).is_false() diff --git a/client/test/occlusion_policy_test.gd b/client/test/occlusion_policy_test.gd new file mode 100644 index 0000000..acca22d --- /dev/null +++ b/client/test/occlusion_policy_test.gd @@ -0,0 +1,43 @@ +# Tests res://scripts/occlusion_policy.gd (NEON-27 occlusion config). +extends GdUnitTestSuite + +const OcclusionPolicyScript := preload("res://scripts/occlusion_policy.gd") + + +func test_defaults_are_valid() -> void: + var p = OcclusionPolicyScript.new() + assert_that(p.is_valid()).is_true() + + +func test_defaults_match_spec() -> void: + var p = OcclusionPolicyScript.new() + assert_that(p.enabled).is_true() + assert_that(p.fade_alpha).is_equal(0.25) + assert_that(p.occluder_group).is_equal("occluder") + assert_that(p.occluder_collision_mask).is_equal(1) + assert_that(p.max_occluder_cast_depth).is_equal(4) + assert_that(p.occluder_count_log_threshold).is_equal(0) + + +func test_is_valid_false_when_disabled() -> void: + var p = OcclusionPolicyScript.new() + p.enabled = false + assert_that(p.is_valid()).is_false() + + +func test_is_valid_true_when_fade_alpha_zero() -> void: + var p = OcclusionPolicyScript.new() + p.fade_alpha = 0.0 + assert_that(p.is_valid()).is_true() + + +func test_is_valid_false_when_fade_alpha_negative() -> void: + var p = OcclusionPolicyScript.new() + p.fade_alpha = -0.1 + assert_that(p.is_valid()).is_false() + + +func test_is_valid_true_at_full_opaque() -> void: + var p = OcclusionPolicyScript.new() + p.fade_alpha = 1.0 + assert_that(p.is_valid()).is_true() diff --git a/docs/decomposition/modules/E1_M2_IsometricCameraController.md b/docs/decomposition/modules/E1_M2_IsometricCameraController.md index e466ee4..3ec9079 100644 --- a/docs/decomposition/modules/E1_M2_IsometricCameraController.md +++ b/docs/decomposition/modules/E1_M2_IsometricCameraController.md @@ -7,7 +7,7 @@ | **Module ID** | E1.M2 | | **Epic** | [Epic 1 — Core Player Runtime](../epics/epic_01_core_player_runtime.md) | | **Stage target** | Prototype | -| **Status** | In progress — follow + `CameraState` + **discrete zoom bands** shipped ([NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25), [NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26)); occlusion + hardening open ([NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27)–[NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28)); see [dependency register](module_dependency_register.md) | +| **Status** | In progress — follow + `CameraState` + **discrete zoom bands** + **occlusion policy** shipped ([NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25)–[NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27)); hardening open ([NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28)); see [dependency register](module_dependency_register.md) | | **Jira** | Feature [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10); prototype backlog stories under parent [NEON-1](https://neon-sprawl.atlassian.net/browse/NEON-1) — [Jira backlog](#jira-backlog) | ## Purpose @@ -55,11 +55,14 @@ 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, 2026-04-08) +## Implementation snapshot (NEON-25 + NEON-26 + NEON-27, 2026-04-08) - **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. - **Zoom:** **`ZoomBandConfig`** resource (`client/scripts/zoom_band_config.gd`); default **`client/resources/isometric_zoom_bands.tres`** on main rig. Bands are **distance-only** (ascending near→far); **pitch / FOV** unchanged vs NEON-25. Input: **`camera_zoom_in`** / **`camera_zoom_out`** in `client/project.godot` (mouse wheel + `=` / `-` + keypad **+** / **−**). **`follow_distance`** remains **fallback** when config is null or has no bands; state **`zoom_band_index`** is **0** in that case. **`zoom_band_changed`** signal on rig; **TODO(E9.M1)** for throttled `camera_zoom_changed` telemetry. +- **Occlusion:** **`OcclusionPolicy`** resource (`client/scripts/occlusion_policy.gd`); default **`client/resources/isometric_occlusion_policy.tres`** on main rig. **Technique:** each `_process` frame, iterative `intersect_ray` from `_smoothed_eye` to player focus (up to `max_occluder_cast_depth` calls). Bodies tagged `"occluder"` that intersect the ray have their `MeshInstance3D` surfaces overridden with a transparent `StandardMaterial3D` copy (`fade_alpha = 0.25`). Materials restored instantly when the body clears the ray. **Null-material surfaces** (no mesh or override material set) receive a plain transparent `StandardMaterial3D`; other non-`StandardMaterial3D` surfaces are skipped with `push_warning`. **Prototype district:** `Obstacle` node in `main.tscn` tagged `"occluder"`. **Perf marker:** `occluder_count_log_threshold` export (default 0 = disabled) emits `push_warning` for stress-test reference. See [risks and telemetry](#risks-and-telemetry) for readability gate. + + > **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. ## Jira backlog diff --git a/docs/plans/NEON-27-implementation-plan.md b/docs/plans/NEON-27-implementation-plan.md index 7bbde6f..94f2678 100644 --- a/docs/plans/NEON-27-implementation-plan.md +++ b/docs/plans/NEON-27-implementation-plan.md @@ -33,7 +33,7 @@ ## Acceptance criteria checklist - [ ] In a documented test setup, occlusion **no longer fully hides** the player in the common worst case (before/after screenshot or short clip note in PR). -- [ ] Policy is **configurable or data-driven** enough to iterate without rewriting core follow logic (`OcclusionPolicy` resource swappable in inspector; `fade_alpha` and `fade_duration` are exported). +- [ ] Policy is **configurable or data-driven** enough to iterate without rewriting core follow logic (`OcclusionPolicy` resource swappable in inspector; `fade_alpha`, `occluder_group`, and `occluder_collision_mask` are exported). - [ ] Risk called out in module doc: occlusion hiding telegraphs — link to **readability checklist** or gate note for prototype demo. ## Technical approach @@ -73,7 +73,7 @@ Each `_process` frame, after the camera eye is positioned, cast a ray from the s - `_restore_occluder(body: Node3D) -> void`: restore original surface override materials (instant), remove from `_occluder_overrides`. - `_restore_all_occluders() -> void`: call `_restore_occluder` for all entries (on disable/null policy). -3. **`isometric_occlusion_policy.tres`** — default `OcclusionPolicy` resource (fade_alpha=0.25, fade_duration=0.15, mask=1, depth=4, threshold=0). Assign to `IsometricFollowCamera` in `main.tscn`. +3. **`isometric_occlusion_policy.tres`** — default `OcclusionPolicy` resource (fade_alpha=0.25, mask=1, depth=4, threshold=0). Assign to `IsometricFollowCamera` in `main.tscn`. 4. **`main.tscn`** — Add `"occluder"` group to the `Obstacle` node; assign `occlusion_policy` resource on `IsometricFollowCamera`. @@ -85,7 +85,8 @@ Each `_process` frame, after the camera eye is positioned, cast a ray from the s |-------|--------|-----------| | **Occluder identification** | Explicit `"occluder"` group tag (Option A) | Precise; failure mode is obvious ("wall doesn't fade — tag it"); no false positives as scene grows; collision layers stay single-purpose. | | **Fade timing** | Instant alpha override — no Tween | Eliminates Tween race conditions (stuck-transparent geometry); simpler state; satisfies AC; smooth-fade pass deferred to post-prototype if pop is distracting at demo. | -| **Non-`StandardMaterial3D` surfaces** | Skip silently + `push_warning` | Avoids unexpected appearance change; prototype `Obstacle` is unaffected (uses `StandardMaterial3D`); warning surfaces future mismatches without hiding them. | +| **Non-`StandardMaterial3D` surfaces** | Skip silently + `push_warning` | Avoids unexpected appearance change; warning surfaces future mismatches without hiding them. | +| **Null surface material** | Create plain `StandardMaterial3D` with fade alpha | The prototype `Obstacle` (`BoxMesh`, no material assigned) hits this path. A null material means Godot renders the default; overriding with a transparent `StandardMaterial3D` achieves the fade, and restoring to `null` returns to the default exactly. | ## Files to add From 8bc2efb854e500e1fa39740f3a1e552782159db7 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 23:42:31 -0400 Subject: [PATCH 4/8] NEON-27: check off all acceptance criteria; record PR screenshots Before/after evidence: obstacle fades to ~25% alpha when occluding the player; restores on clear. All three ACs satisfied. --- docs/plans/NEON-27-implementation-plan.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/plans/NEON-27-implementation-plan.md b/docs/plans/NEON-27-implementation-plan.md index 94f2678..ccc97a3 100644 --- a/docs/plans/NEON-27-implementation-plan.md +++ b/docs/plans/NEON-27-implementation-plan.md @@ -32,9 +32,11 @@ ## Acceptance criteria checklist -- [ ] In a documented test setup, occlusion **no longer fully hides** the player in the common worst case (before/after screenshot or short clip note in PR). -- [ ] Policy is **configurable or data-driven** enough to iterate without rewriting core follow logic (`OcclusionPolicy` resource swappable in inspector; `fade_alpha`, `occluder_group`, and `occluder_collision_mask` are exported). -- [ ] Risk called out in module doc: occlusion hiding telegraphs — link to **readability checklist** or gate note for prototype demo. +- [x] In a documented test setup, occlusion **no longer fully hides** the player in the common worst case (before/after screenshot or short clip note in PR). + - **Before:** obstacle fully opaque, player visible only when clear of the geometry. + - **After:** obstacle fades to ~25% alpha when between the camera and player; player silhouette and slot indicator remain readable. Screenshots: `assets/Screenshot_20260408_234113-*.png` (faded), `assets/Screenshot_20260408_234125-*.png` (restored). +- [x] Policy is **configurable or data-driven** enough to iterate without rewriting core follow logic (`OcclusionPolicy` resource swappable in inspector; `fade_alpha`, `occluder_group`, and `occluder_collision_mask` are exported). +- [x] Risk called out in module doc: occlusion hiding telegraphs — link to **readability checklist** or gate note for prototype demo. ## Technical approach From 5e16fdccc822396be3cd3b6b1a6b74b3eb3d6f67 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 23:50:48 -0400 Subject: [PATCH 5/8] NEON-27: restore occluders on all _process early-return paths Fixes review blocking issue: when the follow target is null (freed, renamed, or path unresolved) or the Camera3D child is missing, _restore_all_occluders() is now called before returning so geometry cannot be left stuck semi-transparent. --- client/scripts/isometric_follow_camera.gd | 2 + docs/reviews/2026-04-08-NEON-27.md | 45 +++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 docs/reviews/2026-04-08-NEON-27.md diff --git a/client/scripts/isometric_follow_camera.gd b/client/scripts/isometric_follow_camera.gd index 01ccefd..74fd654 100644 --- a/client/scripts/isometric_follow_camera.gd +++ b/client/scripts/isometric_follow_camera.gd @@ -80,10 +80,12 @@ func _ready() -> void: func _process(delta: float) -> void: if camera == null or _state == null: + _restore_all_occluders() return var target: Node3D = _resolve_target() if target == null: # No eye/state update until the path resolves — avoids chasing a freed or miswired node. + _restore_all_occluders() _warn_missing_follow_target_once() return _warned_missing_follow_target = false diff --git a/docs/reviews/2026-04-08-NEON-27.md b/docs/reviews/2026-04-08-NEON-27.md new file mode 100644 index 0000000..29024c4 --- /dev/null +++ b/docs/reviews/2026-04-08-NEON-27.md @@ -0,0 +1,45 @@ +# Code review — NEON-27 (OcclusionPolicy) + +**Date:** 2026-04-08 +**Scope:** Branch `NEON-27-occlusion-policy` vs `origin/main` (NEON-27 occlusion policy resource, camera integration, scene wiring, tests, and docs). Issue **NEON-27**; no PR URL supplied. +**Base:** `origin/main` @ `1d891eb8bb42ccc5e8a74b4fcdb349b9165f3454` (merge-base with HEAD at review time). + +## Verdict + +**Request changes** + +## Summary + +The branch adds a data-driven `OcclusionPolicy` resource, wires it into `IsometricFollowCamera`, tags the prototype obstacle as an `"occluder"`, and documents the prototype readability gate in the module doc. The main behavior matches the intended client-local camera scope and the implementation is otherwise straightforward. Risk is still moderate because the fade state is stateful: one cleanup edge case can leave geometry stuck transparent after the follow target disappears, and the review also found two documentation follow-ups so the saved plan/decomposition docs stay aligned with the chosen implementation. + +## Documentation checked + +| Document | Result | +|----------|--------| +| `docs/plans/NEON-27-implementation-plan.md` | **Partially matches** — overall approach, exported resource, scene wiring, tests, and module-doc update all align; however the “Material override strategy” paragraph still says `null` materials are skipped, while the implementation and Decisions table create a transparent `StandardMaterial3D` for that case. | +| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — implementation snapshot and readability gate note align with the code and scene changes. | +| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E1.M2 status remains appropriately **In progress**, but the note still says occlusion policy “remain[s]”; after this merge it should mention occlusion shipped and only integration hardening remains. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — the tracking table still omits `E1.M2` despite NEON-25/26/27 landing meaningful implementation; add or update that row after merge. | +| `docs/decomposition/modules/client_server_authority.md` | **Matches** — the change stays client-local and does not introduce any server use of camera pose. | + +## Blocking issues + +1. ~~**`client/scripts/isometric_follow_camera.gd` — occluders are never restored when the follow target disappears mid-occlusion.** `_process()` returns immediately when `follow_target_path` no longer resolves, but that path does not call `_restore_all_occluders()`. If the player target is freed, renamed, or otherwise temporarily unavailable while an obstacle is faded, the saved surface overrides stay applied until the camera node exits the tree. Because this leaves world geometry stuck semi-transparent in a recoverable runtime failure mode, it should be fixed before merge by restoring all active occluders on the early-return path (and similarly on any other “cannot update camera this frame” exit that can strand state).~~ Done. + +## Suggestions + +1. **`docs/plans/NEON-27-implementation-plan.md` — reconcile the null-material story.** The plan currently says null materials are skipped in the “Material override strategy” section, but the implementation and Decisions table intentionally create a transparent `StandardMaterial3D` for null-material surfaces. Update the technical-approach prose so the plan is a single source of truth. + +2. **`docs/decomposition/modules/module_dependency_register.md` and `docs/decomposition/modules/documentation_and_implementation_alignment.md` — refresh implementation tracking after merge.** `E1.M2` has meaningful shipped work now (follow, zoom, occlusion), so the register note and tracking table should reflect that status explicitly instead of implying occlusion is still pending or leaving the implementation table empty for this module. + +## Nits + +None. + +## Verification + +- From `client/`: Godot 4.6 headless import, then GdUnit: + `godot --headless --import --path . --quit-after 8` + `godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test` +- Manual: run the prototype scene, walk the player behind the `Obstacle`, confirm it fades and restores normally, then test the failure path by making `follow_target_path` unresolved or removing the target while the obstacle is faded; geometry should restore immediately instead of staying transparent. +- Review environment note: I could not run the automated Godot command here because no `godot` binary was available on `PATH`. From 71232c36ef6313e73c5cd1a6570aabc162e3b9b7 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 23:52:46 -0400 Subject: [PATCH 6/8] =?UTF-8?q?NEON-27:=20address=20review=20suggestions?= =?UTF-8?q?=20=E2=80=94=20doc=20alignment=20pass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Plan: reconcile null-material prose in "Material override strategy" to match implementation (null → new StandardMaterial3D, not skipped) - module_dependency_register.md: E1.M2 note updated — occlusion shipped, only integration hardening (NEON-28) remains - documentation_and_implementation_alignment.md: add E1.M2 tracking row (NEON-25–27 shipped, NEON-28 open) - Review: strike through both suggestions as Done --- .../modules/documentation_and_implementation_alignment.md | 1 + docs/decomposition/modules/module_dependency_register.md | 2 +- docs/plans/NEON-27-implementation-plan.md | 7 ++++++- docs/reviews/2026-04-08-NEON-27.md | 4 ++-- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 6a82c9c..dbb0ceb 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -47,6 +47,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | Module | Register status | Snapshot | Plans / pointers | |--------|-----------------|----------|-------------------| | E1.M1 | Ready | Prototype milestone **Done** ([NEON-9](https://neon-sprawl.atlassian.net/browse/NEON-9)). Authoritative `PositionState` + **MoveCommand** over HTTP (JSON v1 snap); **in-memory** store by default, **Postgres** when configured ([NEON-5](../../plans/NEON-5-implementation-plan.md)); shared **NpgsqlDataSource** disposed on host shutdown ([NEON-15](../../plans/NEON-15-implementation-plan.md)); Godot sync + path-follow ([NEON-4](../../plans/NEON-4-implementation-plan.md), [NEON-8](../../plans/NEON-8-implementation-plan.md)); **InteractionRequest** + horizontal range ([NEON-6](../../plans/NEON-6-implementation-plan.md)). Follow-on: prediction/reconciliation, Slice 1 telemetry, Protobuf wire per [contracts.md](contracts.md). | [NEON-3](../../plans/NEON-3-implementation-plan.md), [NEON-4](../../plans/NEON-4-implementation-plan.md), [NEON-5](../../plans/NEON-5-implementation-plan.md), [NEON-6](../../plans/NEON-6-implementation-plan.md), [NEON-7](../../plans/NEON-7-implementation-plan.md), [NEON-8](../../plans/NEON-8-implementation-plan.md), [NEON-15](../../plans/NEON-15-implementation-plan.md); `server/NeonSprawl.Server/Game/PositionState/`, `Game/Interaction/`; [server README](../../../server/README.md) | +| E1.M2 | In progress | **Slice 2 prototype stories shipped:** fixed-yaw isometric follow + `CameraState` seam ([NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25)); discrete zoom bands via `ZoomBandConfig` resource ([NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26)); `OcclusionPolicy` RayCast-based material fade with `"occluder"` group tagging ([NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27)). Open: integration hardening + dependent contract notes ([NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28)). Client-local; no server use of camera pose. | [NEON-25](../../plans/NEON-25-implementation-plan.md), [NEON-26](../../plans/NEON-26-implementation-plan.md), [NEON-27](../../plans/NEON-27-implementation-plan.md); `client/scripts/isometric_follow_camera.gd`, `camera_state.gd`, `zoom_band_config.gd`, `occlusion_policy.gd`; [E1_M2_IsometricCameraController.md](E1_M2_IsometricCameraController.md) | --- diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 1c2b070..c52f230 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -17,7 +17,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen | E1.M3 | InteractionAndTargetingLayer | E1.M1 | TargetState, InteractableDescriptor, SelectionEvent | Prototype | Planned | | E1.M4 | AbilityInputScaffold | E1.M3, E5.M1 | AbilityCastRequest, HotbarLoadout, CooldownSnapshot | Prototype | Planned | -**E1.M2 note:** Client follow rig + per-tick `CameraState` + **discrete zoom bands** (`ZoomBandConfig`, [NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26)) are in repo ([NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25)). Occlusion policy and integration hardening remain ([NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27)–[NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28)). +**E1.M2 note:** Client follow rig + per-tick `CameraState` + **discrete zoom bands** (`ZoomBandConfig`, [NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26)) + **`OcclusionPolicy`** RayCast fade ([NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27)) are in repo ([NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25)–[NEON-27](https://neon-sprawl.atlassian.net/browse/NEON-27)). Integration hardening remains ([NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28)). ### Epic 2 — Skills and Progression Framework diff --git a/docs/plans/NEON-27-implementation-plan.md b/docs/plans/NEON-27-implementation-plan.md index ccc97a3..cc19a58 100644 --- a/docs/plans/NEON-27-implementation-plan.md +++ b/docs/plans/NEON-27-implementation-plan.md @@ -50,7 +50,12 @@ Each `_process` frame, after the camera eye is positioned, cast a ray from the s **Player exclusion:** the `Player` is on collision layer 2 (independent of layer 1); the default `occluder_collision_mask` of `1` already excludes the player body without needing to add it to the ray exclusion list. Confirmed from `main.tscn`: `Player.collision_layer = 2`. -**Material override strategy:** for each `MeshInstance3D` child of a blocking body, iterate surfaces. For each surface, save the current `surface_override_material` (may be null). Resolve the effective material: prefer the surface override; fall back to `mesh.surface_get_material(i)`. If that material is a `StandardMaterial3D`, `duplicate()` it, set `transparency = BaseMaterial3D.TRANSPARENCY_ALPHA` and `albedo_color.a = fade_alpha`, then apply as the surface override. If it is not a `StandardMaterial3D` (e.g. `ShaderMaterial` or null), **skip that surface silently and emit `push_warning`** — do not substitute a plain gray material. Restore by setting `surface_override_material` back to the saved value (or `null`) when the body is no longer blocking. +**Material override strategy:** for each `MeshInstance3D` child of a blocking body, iterate surfaces. For each surface, save the current `surface_override_material` (may be null). Resolve the effective material: prefer the surface override; fall back to `mesh.surface_get_material(i)`. Three cases: +- **`StandardMaterial3D`:** `duplicate()` it, set `transparency = BaseMaterial3D.TRANSPARENCY_ALPHA` and `albedo_color.a = fade_alpha`, apply as the surface override. +- **null (no material assigned):** create a plain `StandardMaterial3D` with `transparency = TRANSPARENCY_ALPHA` and `albedo_color.a = fade_alpha`. This handles the common prototype case where a mesh has no material set (e.g. the `Obstacle` which uses a plain `BoxMesh`); restoring to `null` returns the surface to Godot's default rendering exactly. +- **Anything else (e.g. `ShaderMaterial`):** skip that surface silently and emit `push_warning` — do not substitute a plain gray material. + +Restore by setting `surface_override_material` back to the saved value (or `null`) when the body is no longer blocking. **Fade timing:** instant — set `albedo_color.a` to `fade_alpha` on entry, restore immediately on exit. No Tween; no `fade_duration` export. A smooth-fade pass can be added post-prototype if the pop is distracting during the demo playtest. State: `_occluder_overrides: Dictionary` mapping `Node3D → Array[{mesh, surface, original_mat}]`. diff --git a/docs/reviews/2026-04-08-NEON-27.md b/docs/reviews/2026-04-08-NEON-27.md index 29024c4..96b52e7 100644 --- a/docs/reviews/2026-04-08-NEON-27.md +++ b/docs/reviews/2026-04-08-NEON-27.md @@ -28,9 +28,9 @@ The branch adds a data-driven `OcclusionPolicy` resource, wires it into `Isometr ## Suggestions -1. **`docs/plans/NEON-27-implementation-plan.md` — reconcile the null-material story.** The plan currently says null materials are skipped in the “Material override strategy” section, but the implementation and Decisions table intentionally create a transparent `StandardMaterial3D` for null-material surfaces. Update the technical-approach prose so the plan is a single source of truth. +1. ~~**`docs/plans/NEON-27-implementation-plan.md` — reconcile the null-material story.** The plan currently says null materials are skipped in the “Material override strategy” section, but the implementation and Decisions table intentionally create a transparent `StandardMaterial3D` for null-material surfaces. Update the technical-approach prose so the plan is a single source of truth.~~ Done. -2. **`docs/decomposition/modules/module_dependency_register.md` and `docs/decomposition/modules/documentation_and_implementation_alignment.md` — refresh implementation tracking after merge.** `E1.M2` has meaningful shipped work now (follow, zoom, occlusion), so the register note and tracking table should reflect that status explicitly instead of implying occlusion is still pending or leaving the implementation table empty for this module. +2. ~~**`docs/decomposition/modules/module_dependency_register.md` and `docs/decomposition/modules/documentation_and_implementation_alignment.md` — refresh implementation tracking after merge.** `E1.M2` has meaningful shipped work now (follow, zoom, occlusion), so the register note and tracking table should reflect that status explicitly instead of implying occlusion is still pending or leaving the implementation table empty for this module.~~ Done. ## Nits From f1b4e50729a791c4f43e5fc0fa66ca784cb07f4b Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 23:55:29 -0400 Subject: [PATCH 7/8] NEON-27: finalise review (Approved) + Godot uid files Review verdict updated to Approved; all blocking issues and suggestions resolved and documented. Add auto-generated Godot .uid sidecar files for occlusion_policy.gd and occlusion_policy_test.gd. --- client/scripts/occlusion_policy.gd.uid | 1 + client/test/occlusion_policy_test.gd.uid | 1 + docs/reviews/2026-04-08-NEON-27.md | 19 ++++++++++--------- 3 files changed, 12 insertions(+), 9 deletions(-) create mode 100644 client/scripts/occlusion_policy.gd.uid create mode 100644 client/test/occlusion_policy_test.gd.uid diff --git a/client/scripts/occlusion_policy.gd.uid b/client/scripts/occlusion_policy.gd.uid new file mode 100644 index 0000000..8d927f4 --- /dev/null +++ b/client/scripts/occlusion_policy.gd.uid @@ -0,0 +1 @@ +uid://dtq0253dpm3kg diff --git a/client/test/occlusion_policy_test.gd.uid b/client/test/occlusion_policy_test.gd.uid new file mode 100644 index 0000000..8b1b143 --- /dev/null +++ b/client/test/occlusion_policy_test.gd.uid @@ -0,0 +1 @@ +uid://1ypnjycsj3y5 diff --git a/docs/reviews/2026-04-08-NEON-27.md b/docs/reviews/2026-04-08-NEON-27.md index 96b52e7..5c3463e 100644 --- a/docs/reviews/2026-04-08-NEON-27.md +++ b/docs/reviews/2026-04-08-NEON-27.md @@ -2,35 +2,36 @@ **Date:** 2026-04-08 **Scope:** Branch `NEON-27-occlusion-policy` vs `origin/main` (NEON-27 occlusion policy resource, camera integration, scene wiring, tests, and docs). Issue **NEON-27**; no PR URL supplied. -**Base:** `origin/main` @ `1d891eb8bb42ccc5e8a74b4fcdb349b9165f3454` (merge-base with HEAD at review time). +**Base:** `origin/main` @ `1d891eb8bb42ccc5e8a74b4fcdb349b9165f3454` (merge-base with HEAD at review time). +**Follow-up:** Previously requested changes below are done and were re-reviewed against the updated branch state. ## Verdict -**Request changes** +**Approve** ## Summary -The branch adds a data-driven `OcclusionPolicy` resource, wires it into `IsometricFollowCamera`, tags the prototype obstacle as an `"occluder"`, and documents the prototype readability gate in the module doc. The main behavior matches the intended client-local camera scope and the implementation is otherwise straightforward. Risk is still moderate because the fade state is stateful: one cleanup edge case can leave geometry stuck transparent after the follow target disappears, and the review also found two documentation follow-ups so the saved plan/decomposition docs stay aligned with the chosen implementation. +The branch adds a data-driven `OcclusionPolicy` resource, wires it into `IsometricFollowCamera`, tags the prototype obstacle as an `"occluder"`, and documents the prototype readability gate in the module doc. Re-review confirms the prior cleanup bug is fixed: occluders are now restored on the `_process()` early-return paths as well as on node exit, so transient target-resolution failures no longer strand transparent geometry. The plan and decomposition tracking docs have also been brought back into alignment with the implemented null-material behavior and shipped E1.M2 scope. Residual risk is low and limited to the manual runtime behavior that still needs Godot-side verification. ## Documentation checked | Document | Result | |----------|--------| -| `docs/plans/NEON-27-implementation-plan.md` | **Partially matches** — overall approach, exported resource, scene wiring, tests, and module-doc update all align; however the “Material override strategy” paragraph still says `null` materials are skipped, while the implementation and Decisions table create a transparent `StandardMaterial3D` for that case. | +| `docs/plans/NEON-27-implementation-plan.md` | **Matches** — technical approach now reflects the implemented three-way material handling, including the explicit null-material fallback. | | `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — implementation snapshot and readability gate note align with the code and scene changes. | -| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E1.M2 status remains appropriately **In progress**, but the note still says occlusion policy “remain[s]”; after this merge it should mention occlusion shipped and only integration hardening remains. | -| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — the tracking table still omits `E1.M2` despite NEON-25/26/27 landing meaningful implementation; add or update that row after merge. | +| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E1.M2 still correctly reads **In progress**, and the note now reflects follow + zoom + occlusion shipped with NEON-28 remaining. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — implementation tracking now includes `E1.M2` with current shipped scope and pointers. | | `docs/decomposition/modules/client_server_authority.md` | **Matches** — the change stays client-local and does not introduce any server use of camera pose. | ## Blocking issues -1. ~~**`client/scripts/isometric_follow_camera.gd` — occluders are never restored when the follow target disappears mid-occlusion.** `_process()` returns immediately when `follow_target_path` no longer resolves, but that path does not call `_restore_all_occluders()`. If the player target is freed, renamed, or otherwise temporarily unavailable while an obstacle is faded, the saved surface overrides stay applied until the camera node exits the tree. Because this leaves world geometry stuck semi-transparent in a recoverable runtime failure mode, it should be fixed before merge by restoring all active occluders on the early-return path (and similarly on any other “cannot update camera this frame” exit that can strand state).~~ Done. +1. ~~**`client/scripts/isometric_follow_camera.gd` — occluders are never restored when the follow target disappears mid-occlusion.** `_process()` returns immediately when `follow_target_path` no longer resolves, but that path does not call `_restore_all_occluders()`. If the player target is freed, renamed, or otherwise temporarily unavailable while an obstacle is faded, the saved surface overrides stay applied until the camera node exits the tree. Because this leaves world geometry stuck semi-transparent in a recoverable runtime failure mode, it should be fixed before merge by restoring all active occluders on the early-return path (and similarly on any other “cannot update camera this frame” exit that can strand state).~~ **Done.** `_process()` now restores all occluders before both early returns (`camera/_state` unavailable and unresolved `follow_target_path`), so the fade state is cleared when camera updates cannot proceed. ## Suggestions -1. ~~**`docs/plans/NEON-27-implementation-plan.md` — reconcile the null-material story.** The plan currently says null materials are skipped in the “Material override strategy” section, but the implementation and Decisions table intentionally create a transparent `StandardMaterial3D` for null-material surfaces. Update the technical-approach prose so the plan is a single source of truth.~~ Done. +1. ~~**`docs/plans/NEON-27-implementation-plan.md` — reconcile the null-material story.** The plan currently says null materials are skipped in the “Material override strategy” section, but the implementation and Decisions table intentionally create a transparent `StandardMaterial3D` for null-material surfaces. Update the technical-approach prose so the plan is a single source of truth.~~ **Done.** The “Material override strategy” section now documents the three explicit cases: `StandardMaterial3D`, null material, and non-`StandardMaterial3D`. -2. ~~**`docs/decomposition/modules/module_dependency_register.md` and `docs/decomposition/modules/documentation_and_implementation_alignment.md` — refresh implementation tracking after merge.** `E1.M2` has meaningful shipped work now (follow, zoom, occlusion), so the register note and tracking table should reflect that status explicitly instead of implying occlusion is still pending or leaving the implementation table empty for this module.~~ Done. +2. ~~**`docs/decomposition/modules/module_dependency_register.md` and `docs/decomposition/modules/documentation_and_implementation_alignment.md` — refresh implementation tracking after merge.** `E1.M2` has meaningful shipped work now (follow, zoom, occlusion), so the register note and tracking table should reflect that status explicitly instead of implying occlusion is still pending or leaving the implementation table empty for this module.~~ **Done.** The register note now says NEON-25 through NEON-27 are in repo, and the implementation alignment table now includes an `E1.M2` row with current scope and pointers. ## Nits From 986613fd26c982917786ab1d5eec1fbd55880833 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 23:56:21 -0400 Subject: [PATCH 8/8] NEON-27: apply gdformat to isometric_follow_camera.gd Style-only: flatten short boolean expression in occlusion_policy_is_valid; reformat string interpolation in push_warning calls. --- client/scripts/isometric_follow_camera.gd | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/client/scripts/isometric_follow_camera.gd b/client/scripts/isometric_follow_camera.gd index 74fd654..e548fbd 100644 --- a/client/scripts/isometric_follow_camera.gd +++ b/client/scripts/isometric_follow_camera.gd @@ -220,11 +220,7 @@ func _occlusion_policy_valid() -> bool: ## Static guard — safe to call from tests without a live scene. static func occlusion_policy_is_valid(policy: Resource) -> bool: - return ( - policy != null - and policy.get_script() == OcclusionPolicyScript - and policy.is_valid() - ) + return policy != null and policy.get_script() == OcclusionPolicyScript and policy.is_valid() ## Cast iterative rays from the smoothed eye to the player focus. @@ -258,9 +254,10 @@ func _update_occlusion(focus: Vector3) -> void: and newly_blocking.size() >= policy.occluder_count_log_threshold ): push_warning( - "OcclusionPolicy: %d occluder(s) active (threshold %d)" % [ - newly_blocking.size(), policy.occluder_count_log_threshold - ] + ( + "OcclusionPolicy: %d occluder(s) active (threshold %d)" + % [newly_blocking.size(), policy.occluder_count_log_threshold] + ) ) var to_restore: Array = _occluder_overrides.keys() @@ -295,9 +292,10 @@ func _apply_occluder_fade(body: Node3D) -> void: faded = StandardMaterial3D.new() else: push_warning( - "OcclusionPolicy: %s surface %d is %s — skipping fade." % [ - mi.get_path(), s, effective.get_class() - ] + ( + "OcclusionPolicy: %s surface %d is %s — skipping fade." + % [mi.get_path(), s, effective.get_class()] + ) ) continue faded.transparency = BaseMaterial3D.TRANSPARENCY_ALPHA