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

12 KiB
Raw Blame History

NEO-18 — Implementation plan

Story reference

Field Value
Key NEO-18
Title E1.M2: Camera integration hardening + dependent contract notes
Linear NEO-18
Parent Epic 1 — Core Player Runtime
Module E1.M2 — IsometricCameraController; umbrella E1.M2

Linear source: Description and acceptance criteria were pulled from Linear (issue NEO-18). This plan is the working implementation breakdown; if the Linear issue edits after this date, re-sync the checklist.

Board: Transitioned To Do → In Progress on kickoff (Linear MCP save_issue with state In Progress).

Goal, scope, and out-of-scope

Goal (from Linear): 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 Linear)

  • 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 E1.M2 / module backlog for traceability.
  • Client-local authority and fixed pitch/roll for isometric presentation — align 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; erase stale dict keys without calling set_surface_override_material on a freed subtree (that errors); live bodies still restore when they leave the ray.

Out of scope (from Linear)

  • 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

Acceptance criteria (verbatim intent):

  • Module dependency register / module doc: E1.M2 Ready (or agreed status) with pointers to scripts/scenes.
  • CameraState contract summarized in one place and matches module doc (includes yaw).
  • E6.M2 note: what consumers may read vs undefined; how yaw affects camera-adjacent UX if non-zero later.
  • 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 written acceptance criteria):

  • Occluder invalid-key purge covered by tests or documented manual steps (see Tests).
  • 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 _physics_process tick on the rig (process_physics_priority after Player — see Decisions).
    • 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 E1.M2, NEO-15NEO-17, NEO-20 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 “NEO-18 remains” narrative with shipped wording + script/scene pointers.
    • documentation_and_implementation_alignment.md: E1.M2 row matches register + NEO-18 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): erase dictionary keys whose body is not is_instance_valid (do not restore materials on that path — freed occluders meshes are gone). Harden _restore_occluder_overrides_list with is_instance_valid on mesh and non-null original for the live-body restore path.
  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 E6.M2 contract text requires a snapshot; avoid speculative API.
Invalid occluder keys Erase only Freed body ⇒ subtree is gone; restoring overrides hits freed MeshInstance3D / materials. Drop dict entry; live bodies restore when they leave the ray.
Epic Slice 2 wording Align with rotation policy Satisfies acceptance criteria on “yaw not exposed / default in UX” vs misleading “no rotation.”
Rig tick (jitter follow-up) _physics_process + process_physics_priority = 1 Player default 0 runs before 1; camera reads postmove_and_slide. Avoids _process vs 120Hz physics jitter (camera was above Player in scene order).
Runtime “remove obstacle” + NavigationAgent3D Reparent out of NavigationRegion3D, then rebake + sync agent Default NavigationMesh.geometry_parsed_geometry_type is PARSED_GEOMETRY_BOTH: mesh instances under the region are baked as source geometry alongside static colliders. Disabling collision (or rebaking alone) does not reopen nav if the bodys MeshInstance3D remains a child of the region — the hole persists. For any gameplay mechanic that removes or opens geometry while using baked nav, either exclude that mesh from source (project/settings), reparent the subtree outside the region before rebake, queue_free (if safe), or switch to an explicit geometry pipeline. Prototype dev toggle: main.gd reparents Obstacle to World when hidden, rebakes, player.sync_navigation_agent_after_map_rebuild.

Runtime navigation (lesson for future mechanics)

If the client uses NavigationRegion3D with default parsing (colliders + meshes), hiding or disabling collision on a prop is not enough for click-to-move to cross that tile: bake_navigation_mesh will still see MeshInstance3D children of the region. Rebake + NavigationAgent3D map sync only fixes the path once that geometry is no longer under the region (or no longer parsed). Plan any destructible door, removed barricade, or temporary wall with this in mind — same pitfall as NEO-18 dev smoke.

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 Linear optional telemetry note.
docs/decomposition/modules/E1_M2_IsometricCameraController.md Consumer contract, NEO-18 snapshot, E1.M2 / 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; spot-check Slice 2 behaviors (readable motion, occlusion active).

Occluder smoke (embedded editor): Input action dev_toggle_occluder_obstacle (default Ctrl+Shift+K in project.godot) toggles Obstacle visibility + collision in main.gd (does not queue_free). F9 / F10 are debugger shortcuts (Suspend/Resume, next frame) when the embedded game has focus. Position the player so the obstacle is on the occlusion ray to test fade + restore_all_occluder_materials_now before hide. After hide, confirm click-to-move through the former cell: the implementation reparents the obstacle under World, rebakes the region mesh, and sync_navigation_agent_after_map_rebuild — see Decisions table and Runtime navigation note (PARSED_GEOMETRY_BOTH). Complements static occluder_override_key_is_valid tests, which do not execute the full purge path in CI.

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, NEO-18 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.
  • Follow-up: rig _physics_process + priority (see Decisions).
  • Client dev smoke (main.gd): obstacle toggle + nav: reparent out of NavigationRegion3D, rebake, agent sync — documents the PARSED_GEOMETRY_BOTH / mesh-as-source pitfall for future runtime geometry removal (see Decisions and Runtime navigation).