neon-sprawl/docs/plans/NEON-28-implementation-plan.md

117 lines
9.1 KiB
Markdown
Raw 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.

# NEON-28 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEON-28 |
| **Title** | E1.M2: Camera integration hardening + dependent contract notes |
| **Jira** | [NEON-28](https://neon-sprawl.atlassian.net/browse/NEON-28) |
| **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) |
**Jira source:** Description and acceptance criteria were pulled from Jira via Atlassian MCP (cloud `neon-sprawl`, issue **NEON-28**). This plan is the working implementation breakdown; if Jira edits after this date, re-sync the checklist.
**Board:** Transitioned **To Do → In Progress** on kickoff (Atlassian MCP `transitionJiraIssue`, transition id `2` / name `prog`).
## Goal, scope, and out-of-scope
**Goal (from Jira):** Close the **E1.M2 prototype slice** with stable contracts, documentation, and hooks so **E6.M2** (and other dependents) can rely on `CameraState` and presentation assumptions without spelunking implementation.
**In scope (from Jira)**
- **Finalize `CameraState`:** follow target, **yaw** (prototype `0`; document semantics if orbit is enabled mid-project), zoom band / effective framing, **policy flags as needed** (add fields only when the documented consumer contract requires them; otherwise state explicitly what is **not** in state and where to read it on the rig).
- **Implementation snapshot in module doc:** **Mid-project rotation policy** and **post-release freeze** from `E1_M2_IsometricCameraController.md` (prototype **fixed yaw for players**; optional `allow_yaw` / limits seam; world-anchored gameplay preferred; telegraph readability). **Cross-link [NEON-10](https://neon-sprawl.atlassian.net/browse/NEON-10)** / module backlog for traceability.
- **Client-local authority** and **fixed pitch/roll** for isometric presentation — align [client_server_authority.md](../decomposition/modules/client_server_authority.md) references in E1.M2 / E6.M2 notes.
- **Optional telemetry:** document hooks for throttled `camera_zoom_changed` and occlusion/perf counters **when E9.M1** schema exists (no requirement to implement telemetry in this story unless already trivial TODO cleanup).
- **Dependency register + alignment:** E1.M2 **Ready** (or agreed status) with pointers to scripts/scenes per acceptance criteria.
**Additional in-repo hardening (engineering, supports “stable contracts”)**
- **`IsometricFollowCamera` occluder lifecycle:** `_occluder_overrides` must not **leak or fault** when occluder bodies are **freed** or invalid; purge invalid keys and restore surviving `MeshInstance3D` overrides.
**Out of scope (from Jira)**
- New gameplay features; **E1.M3 / E1.M4** work.
- Committing to **yaw orbit in UX** without product sign-off (policy allows mid-project enablement only).
## Acceptance criteria checklist
Jira acceptance criteria (verbatim intent):
- [x] **Module dependency register / module doc:** E1.M2 **Ready** (or agreed status) with pointers to scripts/scenes.
- [x] **`CameraState` contract** summarized in one place and matches module doc (**includes yaw**).
- [x] **E6.M2 note:** what consumers may read vs undefined; how **yaw** affects camera-adjacent UX if non-zero later.
- [x] **Epic 1 Slice 2** acceptance satisfied: prototype keeps **yaw fixed for players**, zoom within bands, readable motion, occlusion policy active; aligns with updated module wording (not “camera has no yaw,” but **yaw not exposed / remains default in UX**).
**Supporting engineering checks (not a substitute for Jira AC):**
- [x] Occluder invalid-key purge covered by tests or documented manual steps (see [Tests](#tests)).
- [x] [epic_01_core_player_runtime.md](../decomposition/epics/epic_01_core_player_runtime.md) **Slice 2** bullets use the same **yaw / presentation** language as the module (replace over-simplified “no rotation” if it contradicts the rotation policy).
## Technical approach
1. **Contract documentation (primary)**
- **`camera_state.gd`:** Expand file header (and/or class-level comments) so it is a **concise, accurate** summary of fields: `yaw` = orbit delta vs presentation compass; `distance`; `zoom_band_index`; `focus_world`; `follow_target_path`; refresh **every `_process`** on the rig.
- **`E1_M2_IsometricCameraController.md`:** Single **Consumer contract** subsection: table linking each `CameraState` field + rig exports (`allow_yaw`, `presentation_yaw_deg`, resources). Include **mid-project rotation** + **post-release freeze** pointers; link **NEON-10**, **NEON-25NEON-27**, **NEON-30** as appropriate.
- **`E6_M2_ConsentAndRiskUxSignals.md`:** **E1.M2 camera adjacency** — allowed reads (`CameraState` fields), **undefined** (what stays on rig only), **yaw non-zero later** (presentation / HUD implications in one short paragraph); link authority (server must not use camera pose for gameplay).
- **`module_dependency_register.md`:** Set E1.M2 **Status** to **Ready** (or agreed label) and replace “NEON-28 remains” narrative with **shipped** wording + script/scene pointers.
- **`documentation_and_implementation_alignment.md`:** E1.M2 row matches register + NEON-28 completion.
- **`epic_01_core_player_runtime.md`:** Adjust **Slice 2** scope/acceptance lines so they reflect **fixed yaw in UX** and documented orbit seam, not flat “camera never rotates” if that misleads.
2. **`CameraState` “finalize”**
- Reconcile code and docs. Add **policy flags** to `CameraState` only if the E6.M2 / E1.M2 contract text commits to them; otherwise explicitly document **“no policy flags in state yet; read rig exports.”**
3. **`isometric_follow_camera.gd` hardening**
- At start of `_update_occlusion` (and/or restore passes): drop dictionary keys whose body is **`not is_instance_valid`**; restore materials where `MeshInstance3D` is still valid; then erase entries.
4. **Optional telemetry**
- Keep or add **TODO(E9.M1)** pointers in rig / policy scripts; no new telemetry pipeline unless out of scope exception is agreed.
5. **Tests**
- Static helper tests for **occluder key validity** / purge behavior where headless-safe; manual **queue_free** occluder check in editor for full confidence.
## Decisions
| Topic | Choice | Rationale |
|-------|--------|-----------|
| **`CameraState` policy flags** | **Doc-first** | Add fields only if Jira/E6.M2 contract text requires a snapshot; avoid speculative API. |
| **Invalid occluder keys** | **Purge + restore** | Prevents stuck materials and leaks when occluders despawn. |
| **Epic Slice 2 wording** | **Align with rotation policy** | Satisfies Jira AC on “yaw not exposed / default in UX” vs misleading “no rotation.” |
## Files to add
**None** unless occlusion tests are split out (optional `client/test/isometric_follow_camera_occlusion_hardening_test.gd`).
## Files to modify
| Path | Rationale |
|------|-----------|
| `client/scripts/camera_state.gd` | Finalize field documentation; optional policy flags if contract requires. |
| `client/scripts/isometric_follow_camera.gd` | Occluder invalid-key purge; keep TODO(E9.M1) aligned with Jira optional telemetry note. |
| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Consumer contract, NEON-28 snapshot, NEON-10 / rotation policy cross-links. |
| `docs/decomposition/modules/E6_M2_ConsentAndRiskUxSignals.md` | E1.M2 adjacency + yaw UX note + authority. |
| `docs/decomposition/modules/module_dependency_register.md` | E1.M2 Ready + pointers; remove “hardening remains.” |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E1.M2 row updated. |
| `docs/decomposition/epics/epic_01_core_player_runtime.md` | Slice 2 wording vs yaw/presentation policy. |
| `client/test/isometric_follow_camera_test.gd` | Tests for new static guards / occluder key validity if added. |
## Tests
| File | Coverage |
|------|----------|
| `client/test/isometric_follow_camera_test.gd` | **Change.** Guards for occluder override dictionary key validity / purge helper if extracted. |
| `client/test/camera_state_test.gd` | **Change only if** new `CameraState` fields ship — round-trip defaults. |
**Manual verification:** Main scene — zoom bands, occlusion fade, fixed yaw UX; free an occluder while faded and confirm no errors / stuck materials; spot-check Slice 2 behaviors (readable motion, occlusion active).
## Open questions / risks
- **`CameraState` policy flags:** Product may later require explicit booleans for risk UX; if E6.M2 text stays non-committal, keep flags out and document **rig as source** until a follow-on story.
- None otherwise.
## Shipped (2026-04-10)
- `camera_state.gd` consumer contract header; `isometric_follow_camera.gd` invalid occluder purge + `occluder_override_key_is_valid`; tests in `isometric_follow_camera_test.gd`.
- Docs: `E1_M2_IsometricCameraController.md` (Ready, consumer contract, NEON-28 snapshot), `E6_M2_ConsentAndRiskUxSignals.md` (E1.M2 adjacency), `module_dependency_register.md`, `documentation_and_implementation_alignment.md`, `epic_01_core_player_runtime.md` Slice 2.