neon-sprawl/docs/plans/NEO-17-implementation-plan.md

131 lines
12 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# NEO-17 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-17 |
| **Title** | E1.M2: OcclusionPolicy — keep player readable through geometry |
| **Linear** | [NEO-17](https://linear.app/neon-sprawl/issue/NEO-17) |
| **Parent** | [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/project/epic-1-core-player-runtime-client-controls-character-loop-66bd590cd016) |
| **Module** | [E1.M2 — IsometricCameraController](../decomposition/modules/E1_M2_IsometricCameraController.md); umbrella [E1.M2](https://linear.app/neon-sprawl/project/e1m2-isometriccameracontroller-deac8ef10395) |
## 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 Linear)
- 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
- [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
### 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). 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}]`.
### 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).
- `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 `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, 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 NEO-17 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; 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
| 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 NEO-17 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
- **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 (NEO-19):** when the prototype district expands (NEO-19), 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.