From f4a30b1b9dbd4c5f4ab4ebfd9f4e7507e577ce4f Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 22:17:36 -0400 Subject: [PATCH 1/8] NEON-26: add implementation plan for zoom bands --- docs/plans/NEON-26-implementation-plan.md | 84 +++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 docs/plans/NEON-26-implementation-plan.md diff --git a/docs/plans/NEON-26-implementation-plan.md b/docs/plans/NEON-26-implementation-plan.md new file mode 100644 index 0000000..3bf479b --- /dev/null +++ b/docs/plans/NEON-26-implementation-plan.md @@ -0,0 +1,84 @@ +# NEON-26 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEON-26 | +| **Title** | E1.M2: ZoomBandConfig + discrete zoom input (clamped) | +| **Jira** | [NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26) | +| **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:** Add **data-driven discrete zoom bands** so the player can adjust framing within limits; **`CameraState`** reports the active band and **effective distance** while **preserving yaw** and the NEON-25 orbit seam. + +**In scope** + +- **`ZoomBandConfig`** — `Resource` (no `class_name`; match [godot-client-script-organization](../../.cursor/rules/godot-client-script-organization.md)) holding an ordered list of **follow distances** (meters per band), plus a **default band index** (clamped to valid range when applied). +- **`isometric_follow_camera.gd`** — `@export` reference to `ZoomBandConfig`; **internal active band index** clamped to `[0, band_count - 1]`; each frame use **distance for the active band** in `desired_eye_world` (same pitch / presentation yaw as today). +- **Input:** cycle bands **in** / **out** (e.g. mouse wheel up/down and/or `+`/`-` keys) via **`InputMap` actions** in `project.godot`; handle in **`_unhandled_input`** (or equivalent) on the rig so UI can steal focus later if needed. +- **`CameraState`:** set **`zoom_band_index`** to the active index and **`distance`** to the **effective** distance used for framing (not the legacy single export alone when bands are active). +- **Designer tuning:** one **Resource** asset (e.g. `res://resources/...tres` or inline default) editable in the inspector; rig points at it. +- **Optional:** stub signal / throttled `print` / `TODO(E9.M1)` for `camera_zoom_changed` telemetry when a schema exists. + +**Out of scope** (per Jira) + +- Smooth analog zoom between bands (unless a one-line lerp is explicitly chosen later — default **no**). +- Occlusion policy (**NEON-27**). +- Yaw orbit input (**NEON-25** seam stays; do not bind rotate). + +## Acceptance criteria checklist + +- [ ] Zoom input only moves between **defined bands** (clamped; no out-of-range index). +- [ ] At each band, player remains **readably framed** during motion (manual playtest + distances chosen around current NEON-25 baseline). +- [ ] `CameraState` reflects **active band** and **effective distance**, and still exposes **yaw** unchanged in meaning. +- [ ] Config lives in **one tunable place** for designers (`ZoomBandConfig` resource referenced by the rig). + +## Technical approach + +1. **`zoom_band_config.gd`** — `extends Resource` with e.g. `@export var band_distances: PackedFloat32Array` (length ≥ 1) and `@export var default_band_index: int`. Provide **`band_count`**, **`clamp_index(i: int) -> int`**, **`distance_at(index: int) -> float`** (uses clamped index). Validate in `_validate_property` or getter: if empty at runtime, treat as single band using rig fallback (see below). +2. **`isometric_follow_camera.gd`** — Add `@export var zoom_band_config: ZoomBandConfig`. Keep **`follow_distance`** as **fallback** when config is null or has no bands (preserves NEON-25 scenes/tests). Track **`_zoom_band_index`** initialized from config default (clamped). On zoom-in/out actions: adjust index with `clamp_index`, no op at ends (or wrap — **clamp** per story). **`_process`:** compute `var d := effective_follow_distance()` from config + index or fallback export. Replace uses of raw `follow_distance` in `desired_eye_world` with `d`. **`_sync_camera_state`:** assign `_state.zoom_band_index` and `_state.distance = d` (and existing yaw / path / focus). +3. **`project.godot`** — Add **`camera_zoom_in`** and **`camera_zoom_out`** (or one action with axis — prefer two actions for wheel + keys) with mouse wheel and key events as in Jira examples. +4. **Scene / resource** — Add a default **`ZoomBandConfig`** `.tres` under `res://resources/` (or co-located under `client/resources/`) with **≥ 3** bands bracketing **~25.709** m so readability can be checked; assign on **`IsometricFollowCamera`** in `main.tscn`. +5. **Telemetry stub** — Optional `signal zoom_band_changed(new_index: int, distance: float)` or `TODO` comment referencing E9.M1; no external schema required for this story. + +**Decisions (pre-implementation)** + +| Topic | Choice | +|--------|--------| +| **Band parameter** | **Distance-only** per step (same pitch / FOV as NEON-25); document in module snapshot. FOV-per-band deferred unless trivial. | +| **Bounds at ends** | **Clamp** (no wrap): input at min/max does nothing extra. | +| **Config null / empty** | Use **`follow_distance`** export only; **`zoom_band_index`** in state = `0`. | + +## Files to add + +| Path | Purpose | +|------|---------| +| `client/scripts/zoom_band_config.gd` | `Resource` type for ordered band distances + default index; clamp helpers (no `class_name`). | +| `client/resources/isometric_zoom_bands.tres` | Default `ZoomBandConfig` instance for designers (≥ 3 bands around current baseline distance). | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `client/scripts/isometric_follow_camera.gd` | Wire config, band index, input handling, effective distance for eye position, correct `CameraState` sync (replace hard-coded `zoom_band_index = 0`). | +| `client/scripts/camera_state.gd` | Tighten comments only if needed so **`distance`** = effective follow distance and **`zoom_band_index`** = active band are unambiguous (fields already exist). | +| `client/project.godot` | Register **`camera_zoom_in`** / **`camera_zoom_out`** input actions. | +| `client/scenes/main.tscn` | Assign **`zoom_band_config`** on the follow rig to the new `.tres` (or leave null to exercise fallback — prefer assigned for acceptance “designer tunable”). | +| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | Update **implementation snapshot** for NEON-26: `ZoomBandConfig` path, input actions, discrete bands replace “single scalar until NEON-26”. | + +## Tests + +| File | Coverage | +|------|----------| +| `client/test/zoom_band_config_test.gd` | **New:** `clamp_index` for empty / single / multi band; `distance_at` matches configured values; default index clamped when out of range. | +| `client/test/isometric_follow_camera_test.gd` | **Change:** add a test for a **small static helper** on the rig script (e.g. resolve effective follow distance from optional config + band index + fallback export) so headless CI covers band selection **without** simulating `InputEvent`. | + +**Manual verification:** in running client, wheel and keys cycle bands; confirm framing stays readable at each step and `CameraState` matches (debugger or brief log). + +## Open questions / risks + +- **Band count = 1:** Still valid; zoom input should no-op (clamped range size 1). +- **Readability AC** is partly subjective; pick band spread conservatively and adjust `.tres` after first playtest. From 753ce84861bcaf76b6bd4b1f20df121264fca827 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 22:32:45 -0400 Subject: [PATCH 2/8] NEON-26: ZoomBandConfig, discrete zoom input, CameraState sync --- client/project.godot | 14 ++++ client/resources/isometric_zoom_bands.tres | 8 +++ client/scenes/main.tscn | 2 + client/scripts/camera_state.gd | 4 +- client/scripts/isometric_follow_camera.gd | 72 +++++++++++++++++-- client/scripts/zoom_band_config.gd | 25 +++++++ client/scripts/zoom_band_config.gd.uid | 1 + client/test/isometric_follow_camera_test.gd | 24 +++++++ client/test/zoom_band_config_test.gd | 48 +++++++++++++ client/test/zoom_band_config_test.gd.uid | 1 + .../E1_M2_IsometricCameraController.md | 8 +-- .../modules/module_dependency_register.md | 2 +- docs/plans/NEON-26-implementation-plan.md | 8 +-- 13 files changed, 202 insertions(+), 15 deletions(-) create mode 100644 client/resources/isometric_zoom_bands.tres create mode 100644 client/scripts/zoom_band_config.gd create mode 100644 client/scripts/zoom_band_config.gd.uid create mode 100644 client/test/zoom_band_config_test.gd create mode 100644 client/test/zoom_band_config_test.gd.uid diff --git a/client/project.godot b/client/project.godot index 19f828a..054fc3c 100644 --- a/client/project.godot +++ b/client/project.godot @@ -44,6 +44,20 @@ interact={ "events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":69,"physical_keycode":69,"key_label":0,"unicode":101,"location":0,"echo":false,"script":null) ] } +camera_zoom_in={ +"deadzone": 0.5, +"events": [Object(InputEventMouseButton,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"button_mask":0,"position":Vector2(0, 0),"global_position":Vector2(0, 0),"button_index":4,"factor":1.0,"pressed":false,"canceled":false,"double_click":false,"script":null) +, Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":61,"physical_keycode":61,"key_label":0,"unicode":0,"location":0,"echo":false,"script":null) +, Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":4194437,"physical_keycode":4194437,"key_label":0,"unicode":0,"location":0,"echo":false,"script":null) +] +} +camera_zoom_out={ +"deadzone": 0.5, +"events": [Object(InputEventMouseButton,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"button_mask":0,"position":Vector2(0, 0),"global_position":Vector2(0, 0),"button_index":5,"factor":1.0,"pressed":false,"canceled":false,"double_click":false,"script":null) +, Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":45,"physical_keycode":45,"key_label":0,"unicode":0,"location":0,"echo":false,"script":null) +, Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":4194435,"physical_keycode":4194435,"key_label":0,"unicode":0,"location":0,"echo":false,"script":null) +] +} [physics] diff --git a/client/resources/isometric_zoom_bands.tres b/client/resources/isometric_zoom_bands.tres new file mode 100644 index 0000000..187e913 --- /dev/null +++ b/client/resources/isometric_zoom_bands.tres @@ -0,0 +1,8 @@ +[gd_resource type="Resource" load_steps=2 format=3] + +[ext_resource type="Script" path="res://scripts/zoom_band_config.gd" id="1_zoom_cfg"] + +[resource] +script = ExtResource("1_zoom_cfg") +band_distances = PackedFloat32Array(18, 25.709, 32) +default_band_index = 1 diff --git a/client/scenes/main.tscn b/client/scenes/main.tscn index 17e10ee..1e98450 100644 --- a/client/scenes/main.tscn +++ b/client/scenes/main.tscn @@ -7,6 +7,7 @@ [ext_resource type="Script" uid="uid://bv0xprp660hib" path="res://scripts/interaction_request_client.gd" id="5_ix"] [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"] [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) @@ -89,6 +90,7 @@ shadow_enabled = true [node name="IsometricFollowCamera" type="Node3D" parent="World" unique_id=1124088856] 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") [node name="Camera3D" type="Camera3D" parent="World/IsometricFollowCamera" unique_id=1124088857] current = true diff --git a/client/scripts/camera_state.gd b/client/scripts/camera_state.gd index 709a772..b71f310 100644 --- a/client/scripts/camera_state.gd +++ b/client/scripts/camera_state.gd @@ -12,10 +12,10 @@ var yaw: float = 0.0 ## Scene path to the follow anchor (typically `CharacterBody3D` / `Node3D`). var follow_target_path: NodePath = NodePath() -## Single-band follow distance until NEON-26 zoom bands (meters). +## Effective follow distance used for framing this tick (meters) — active zoom band or fallback. var distance: float = 0.0 -## Placeholder for discrete zoom steps (NEON-26). +## Active discrete zoom band index when the rig uses a zoom-band resource; otherwise **0**. var zoom_band_index: int = 0 ## World-space point the rig used as look-at focus last tick. diff --git a/client/scripts/isometric_follow_camera.gd b/client/scripts/isometric_follow_camera.gd index b7cb94a..fbef68b 100644 --- a/client/scripts/isometric_follow_camera.gd +++ b/client/scripts/isometric_follow_camera.gd @@ -1,19 +1,28 @@ extends Node3D ## NEON-25: client-local isometric follow; updates [CameraState] each `_process`. +## NEON-26: discrete zoom bands via [member zoom_band_config]; wheel / `camera_zoom_*` actions. ## Pitch / roll are fixed by presentation exports; **orbit yaw** in state stays **0** while ## [member allow_yaw] is false (no rotate input bound). Future orbit: read relative input here, ## add to `_orbit_yaw_rad`, clamp with [member max_yaw_deg]. const CameraStateScript := preload("res://scripts/camera_state.gd") +const ZoomBandConfigScript := preload("res://scripts/zoom_band_config.gd") + +## TODO(E9.M1): throttled `camera_zoom_changed` telemetry when schema exists. +signal zoom_band_changed(new_index: int, distance: float) ## 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. @export var follow_target_path: NodePath = NodePath("../Player") @export var follow_distance: float = 25.709 @export var pitch_elevation_deg: float = 20.693 @export var presentation_yaw_deg: float = 45.0 @export var focus_vertical_offset: float = 0.0 +## Designer-tunable discrete bands (`res://resources/isometric_zoom_bands.tres` on main rig). +@export var zoom_band_config: Resource + @export var allow_yaw: bool = false @export var max_yaw_deg: float = 45.0 @@ -30,6 +39,7 @@ var camera_state: var _state = null 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 @onready var camera: Camera3D = $Camera3D @@ -37,6 +47,7 @@ var _warned_missing_follow_target: bool = false func _ready() -> void: _state = CameraStateScript.new() + _init_zoom_band_index() if camera == null: push_error("IsometricFollowCamera: expected child Camera3D") return @@ -50,8 +61,9 @@ func _ready() -> void: ) var focus0: Vector3 = t.global_position + Vector3(0.0, focus_vertical_offset, 0.0) var yaw0: float = deg_to_rad(presentation_yaw_deg) + _orbit_yaw_rad + var dist0: float = _current_follow_distance() _smoothed_eye = desired_eye_world( - focus0, follow_distance, deg_to_rad(pitch_elevation_deg), yaw0 + focus0, dist0, deg_to_rad(pitch_elevation_deg), yaw0 ) global_position = _smoothed_eye camera.look_at(focus0, Vector3.UP) @@ -78,8 +90,9 @@ func _process(delta: float) -> void: var focus: Vector3 = target.global_position + Vector3(0.0, focus_vertical_offset, 0.0) var yaw_total: float = deg_to_rad(presentation_yaw_deg) + _orbit_yaw_rad + var dist: float = _current_follow_distance() var desired: Vector3 = desired_eye_world( - focus, follow_distance, deg_to_rad(pitch_elevation_deg), yaw_total + focus, dist, deg_to_rad(pitch_elevation_deg), yaw_total ) if _smoothed_eye.distance_to(desired) >= snap_distance: @@ -93,12 +106,63 @@ func _process(delta: float) -> void: _sync_camera_state(focus) +func _unhandled_input(event: InputEvent) -> void: + if event.is_echo(): + return + if event.is_action_pressed("camera_zoom_in"): + _apply_zoom_step(-1) + elif event.is_action_pressed("camera_zoom_out"): + _apply_zoom_step(1) + + +## **Near** = lower band index. Zoom in → closer → decrease index (clamped). +func _apply_zoom_step(delta_bands: int) -> void: + if not _zoom_config_valid(): + return + var cfg: Resource = zoom_band_config + var new_idx: int = cfg.clamp_index(_zoom_band_index + delta_bands) + if new_idx == _zoom_band_index: + return + _zoom_band_index = new_idx + zoom_band_changed.emit(_zoom_band_index, _current_follow_distance()) + + +func _init_zoom_band_index() -> void: + _zoom_band_index = 0 + if not _zoom_config_valid(): + return + var cfg: Resource = zoom_band_config + _zoom_band_index = cfg.clamp_index(cfg.default_band_index) + + +func _zoom_config_valid() -> bool: + return ( + zoom_band_config != null + and zoom_band_config.get_script() == ZoomBandConfigScript + and zoom_band_config.band_count() > 0 + ) + + +func _current_follow_distance() -> float: + return effective_follow_distance(zoom_band_config, _zoom_band_index, follow_distance) + + +static func effective_follow_distance( + zoom_cfg: Resource, band_index: int, fallback_distance: float +) -> float: + if zoom_cfg == null or zoom_cfg.get_script() != ZoomBandConfigScript: + return fallback_distance + if zoom_cfg.band_count() == 0: + return fallback_distance + return zoom_cfg.distance_at(band_index) + + func _sync_camera_state(focus: Vector3) -> void: # `CameraState.yaw` = orbit delta only; world-fixed diagonal framing = # `presentation_yaw_deg`. _state.follow_target_path = follow_target_path - _state.distance = follow_distance - _state.zoom_band_index = 0 + _state.distance = _current_follow_distance() + _state.zoom_band_index = _zoom_band_index if _zoom_config_valid() else 0 _state.focus_world = focus _state.yaw = _orbit_yaw_rad diff --git a/client/scripts/zoom_band_config.gd b/client/scripts/zoom_band_config.gd new file mode 100644 index 0000000..a83dbe7 --- /dev/null +++ b/client/scripts/zoom_band_config.gd @@ -0,0 +1,25 @@ +extends Resource + +## Discrete follow distances (meters), **near → far** (ascending). Index **0** = closest. +## No `class_name` — see repo Godot headless / CI notes. +@export var band_distances: PackedFloat32Array = PackedFloat32Array([25.709]) +@export var default_band_index: int = 0 + + +func band_count() -> int: + return band_distances.size() + + +func clamp_index(i: int) -> int: + var n := band_count() + if n == 0: + return 0 + return clampi(i, 0, n - 1) + + +func distance_at(index: int) -> float: + var n := band_count() + if n == 0: + return 0.0 + var ci := clamp_index(index) + return band_distances[ci] diff --git a/client/scripts/zoom_band_config.gd.uid b/client/scripts/zoom_band_config.gd.uid new file mode 100644 index 0000000..79ebab0 --- /dev/null +++ b/client/scripts/zoom_band_config.gd.uid @@ -0,0 +1 @@ +uid://b6lpmuv4rj0vp diff --git a/client/test/isometric_follow_camera_test.gd b/client/test/isometric_follow_camera_test.gd index da43497..fbe0666 100644 --- a/client/test/isometric_follow_camera_test.gd +++ b/client/test/isometric_follow_camera_test.gd @@ -2,6 +2,7 @@ extends GdUnitTestSuite const IsoScript := preload("res://scripts/isometric_follow_camera.gd") +const ZoomBandConfigScript := preload("res://scripts/zoom_band_config.gd") func test_desired_eye_matches_previous_static_camera_frame() -> void: @@ -11,3 +12,26 @@ func test_desired_eye_matches_previous_static_camera_frame() -> void: ) var want := Vector3(12.0, 10.0, 12.0) assert_that(eye.distance_to(want)).is_less(0.06) + + +func test_effective_follow_distance_fallback_when_config_null() -> void: + assert_that(IsoScript.effective_follow_distance(null, 3, 12.5)).is_equal(12.5) + + +func test_effective_follow_distance_uses_config_when_valid() -> void: + var cfg = ZoomBandConfigScript.new() + cfg.band_distances = PackedFloat32Array([10.0, 20.0, 30.0]) + assert_that(IsoScript.effective_follow_distance(cfg, 1, 99.0)).is_equal(20.0) + + +func test_effective_follow_distance_clamps_band_index() -> void: + var cfg = ZoomBandConfigScript.new() + cfg.band_distances = PackedFloat32Array([10.0, 20.0]) + assert_that(IsoScript.effective_follow_distance(cfg, 99, 1.0)).is_equal(20.0) + assert_that(IsoScript.effective_follow_distance(cfg, -3, 1.0)).is_equal(10.0) + + +func test_effective_follow_distance_empty_config_bands_falls_back() -> void: + var cfg = ZoomBandConfigScript.new() + cfg.band_distances = PackedFloat32Array() + assert_that(IsoScript.effective_follow_distance(cfg, 0, 7.5)).is_equal(7.5) diff --git a/client/test/zoom_band_config_test.gd b/client/test/zoom_band_config_test.gd new file mode 100644 index 0000000..901b72b --- /dev/null +++ b/client/test/zoom_band_config_test.gd @@ -0,0 +1,48 @@ +# Tests res://scripts/zoom_band_config.gd (NEON-26 discrete bands). +extends GdUnitTestSuite + +const ZoomBandConfigScript := preload("res://scripts/zoom_band_config.gd") + + +func test_empty_bands_band_count_zero() -> void: + var c = ZoomBandConfigScript.new() + c.band_distances = PackedFloat32Array() + assert_that(c.band_count()).is_equal(0) + + +func test_empty_bands_clamp_index_zero() -> void: + var c = ZoomBandConfigScript.new() + c.band_distances = PackedFloat32Array() + assert_that(c.clamp_index(0)).is_equal(0) + assert_that(c.clamp_index(99)).is_equal(0) + + +func test_empty_bands_distance_at_zero() -> void: + var c = ZoomBandConfigScript.new() + c.band_distances = PackedFloat32Array() + assert_that(c.distance_at(0)).is_equal(0.0) + + +func test_single_band_clamp_always_zero() -> void: + var c = ZoomBandConfigScript.new() + c.band_distances = PackedFloat32Array([42.0]) + assert_that(c.clamp_index(-1)).is_equal(0) + assert_that(c.clamp_index(0)).is_equal(0) + assert_that(c.clamp_index(9)).is_equal(0) + + +func test_multi_band_clamp_and_distance_at() -> void: + var c = ZoomBandConfigScript.new() + c.band_distances = PackedFloat32Array([10.0, 20.0, 30.0]) + assert_that(c.clamp_index(-5)).is_equal(0) + assert_that(c.clamp_index(1)).is_equal(1) + assert_that(c.clamp_index(99)).is_equal(2) + assert_that(c.distance_at(1)).is_equal(20.0) + assert_that(c.distance_at(100)).is_equal(30.0) + + +func test_default_index_clamped_via_clamp_index() -> void: + var c = ZoomBandConfigScript.new() + c.band_distances = PackedFloat32Array([1.0, 2.0, 3.0]) + c.default_band_index = 99 + assert_that(c.clamp_index(c.default_band_index)).is_equal(2) diff --git a/client/test/zoom_band_config_test.gd.uid b/client/test/zoom_band_config_test.gd.uid new file mode 100644 index 0000000..b8b383b --- /dev/null +++ b/client/test/zoom_band_config_test.gd.uid @@ -0,0 +1 @@ +uid://cr1ot54crlhwn diff --git a/docs/decomposition/modules/E1_M2_IsometricCameraController.md b/docs/decomposition/modules/E1_M2_IsometricCameraController.md index 718f4bf..e466ee4 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` baseline shipped ([NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25)); zoom, occlusion, hardening open ([NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26)–[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** 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) | | **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,11 @@ 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, 2026-04-07) +## Implementation snapshot (NEON-25 + NEON-26, 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`**, **`zoom_band_index`**, **`focus_world`**, **`follow_target_path`**). +- **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:** single scalar **`follow_distance`** until [NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26); **`zoom_band_index`** reserved on **`CameraState`**. +- **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. ## Jira backlog diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index a4335b3..1c2b070 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` baseline is in repo ([NEON-25](https://neon-sprawl.atlassian.net/browse/NEON-25)). Discrete zoom bands, occlusion policy, and integration hardening remain ([NEON-26](https://neon-sprawl.atlassian.net/browse/NEON-26)–[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)) 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)). ### Epic 2 — Skills and Progression Framework diff --git a/docs/plans/NEON-26-implementation-plan.md b/docs/plans/NEON-26-implementation-plan.md index 3bf479b..6c55701 100644 --- a/docs/plans/NEON-26-implementation-plan.md +++ b/docs/plans/NEON-26-implementation-plan.md @@ -31,10 +31,10 @@ ## Acceptance criteria checklist -- [ ] Zoom input only moves between **defined bands** (clamped; no out-of-range index). -- [ ] At each band, player remains **readably framed** during motion (manual playtest + distances chosen around current NEON-25 baseline). -- [ ] `CameraState` reflects **active band** and **effective distance**, and still exposes **yaw** unchanged in meaning. -- [ ] Config lives in **one tunable place** for designers (`ZoomBandConfig` resource referenced by the rig). +- [x] Zoom input only moves between **defined bands** (clamped; no out-of-range index). +- [x] At each band, player remains **readably framed** during motion (manual playtest + distances chosen around current NEON-25 baseline). +- [x] `CameraState` reflects **active band** and **effective distance**, and still exposes **yaw** unchanged in meaning. +- [x] Config lives in **one tunable place** for designers (`ZoomBandConfig` resource referenced by the rig). ## Technical approach From 806777b6d5de75db28eba1d9966bc86c3c4524ca Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 22:35:54 -0400 Subject: [PATCH 3/8] NEON-26: widen default zoom bands, stronger zoom-out --- client/resources/isometric_zoom_bands.tres | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/resources/isometric_zoom_bands.tres b/client/resources/isometric_zoom_bands.tres index 187e913..575b7e4 100644 --- a/client/resources/isometric_zoom_bands.tres +++ b/client/resources/isometric_zoom_bands.tres @@ -4,5 +4,5 @@ [resource] script = ExtResource("1_zoom_cfg") -band_distances = PackedFloat32Array(18, 25.709, 32) +band_distances = PackedFloat32Array(17, 25.709, 42, 72) default_band_index = 1 From 0d71b873af53eed58e5f51838fca3be221ecd522 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 22:37:33 -0400 Subject: [PATCH 4/8] NEON-26: add finer zoom band ladder (11 steps) --- client/resources/isometric_zoom_bands.tres | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/resources/isometric_zoom_bands.tres b/client/resources/isometric_zoom_bands.tres index 575b7e4..0eabf96 100644 --- a/client/resources/isometric_zoom_bands.tres +++ b/client/resources/isometric_zoom_bands.tres @@ -4,5 +4,5 @@ [resource] script = ExtResource("1_zoom_cfg") -band_distances = PackedFloat32Array(17, 25.709, 42, 72) -default_band_index = 1 +band_distances = PackedFloat32Array(14, 17, 19.5, 22.5, 25.709, 29, 34, 41, 50, 62, 78) +default_band_index = 4 From b9ec6ccb0f0c6087b7c8e57f8495860b17343a40 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 22:55:37 -0400 Subject: [PATCH 5/8] NEON-26: add code review notes for zoom bands --- docs/reviews/2026-04-08-NEON-26.md | 50 ++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 docs/reviews/2026-04-08-NEON-26.md diff --git a/docs/reviews/2026-04-08-NEON-26.md b/docs/reviews/2026-04-08-NEON-26.md new file mode 100644 index 0000000..cf15c17 --- /dev/null +++ b/docs/reviews/2026-04-08-NEON-26.md @@ -0,0 +1,50 @@ +# Code review — NEON-26 (ZoomBandConfig + discrete zoom) + +**Date:** 2026-04-08 +**Scope:** Branch `NEON-26-zoom-band-config-discrete-input` vs `origin/main` (NEON-26 zoom bands + follow-up `.tres` tuning commits). Issue **NEON-26**; no PR URL supplied. +**Base:** `origin/main` @ `88653292104eddfc1db702f97b02a088fb80c1c7` (merge-base with HEAD at review time). + +## Verdict + +**Approve with nits** + +## Summary + +The branch delivers **data-driven discrete zoom** via `ZoomBandConfig`, **InputMap** actions, **`_unhandled_input`** on the follow rig, and **`CameraState`** updates for effective distance and band index, with **clamp** semantics and **`follow_distance`** fallback when config is missing or empty. **GdUnit** coverage was added for the resource and **`effective_follow_distance`**. Module and dependency-register docs were updated. Overall risk is **low**: client-local camera only, no server coupling; logic is straightforward. Follow-up tuning of **`isometric_zoom_bands.tres`** (band count and distances) is data-only and acceptable on the same story. + +## Documentation checked + +| Document | Result | +|----------|--------| +| `docs/plans/NEON-26-implementation-plan.md` | **Matches** intent and acceptance criteria. Minor **partial match**: the plan’s technical approach mentions `@export var zoom_band_config: ZoomBandConfig`; the implementation correctly uses **`Resource` + `get_script()`** comparison to avoid `class_name`, consistent with `godot-client-script-organization` and NEON-14 headless notes. Consider a one-line clarification in the plan that the export is untyped/`Resource` by design. | +| `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — implementation snapshot updated for NEON-26 (config path, input actions, distance-only bands, fallback, signal + E9.M1 TODO). | +| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E1.M2 note reflects zoom bands shipped. | +| `docs/decomposition/modules/client_server_authority.md` (camera client-local) | **Matches** — no server use of camera pose introduced. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **N/A** for mandatory table edits; E1.M2 status remains “In progress” appropriately (NEON-27/28 open). | + +## Blocking issues + +None. + +## Suggestions + +1. **`client/README.md` (testing section)** — The bullet that lists covered scripts is now slightly stale: it does not mention **`isometric_follow_camera.gd`**, **`camera_state.gd`**, or **`zoom_band_config.gd`**, though **`client/test/`** includes suites for them. Per [testing-expectations](../../.cursor/rules/testing-expectations.md), consider extending that sentence so contributors know camera/zoom logic is under GdUnit. + +2. **`ZoomBandConfig` validation** — Designer-editable **`band_distances`** could theoretically include **non-positive** values, which would produce odd framing. Optional: `_validate_property` or a short runtime guard in the rig when loading config (non-blocking for prototype). + +3. **Plan vs implementation (typing)** — As above, add a brief note to **`NEON-26-implementation-plan.md`** that **`zoom_band_config` is exported as `Resource`** (or untyped) with script identity checked at runtime, so future readers are not confused by the plan’s “ZoomBandConfig” type mention. + +## Nits + +- **`_current_follow_distance()`** is invoked multiple times per **`_process`** / **`_sync_camera_state`**; a single local **`var d`** per frame would avoid redundant calls (micro-optimization, clarity). + +- **Keyboard zoom:** **`=`** without **Shift** may not match all layouts’ “+” expectations; wheel and keypad bindings mitigate this. Document in README or input remap note if QA reports gaps. + +- **Signal name vs telemetry TODO:** Comment mentions **`camera_zoom_changed`**; signal is **`zoom_band_changed`** — harmless but slightly inconsistent wording for whoever wires E9.M1 later. + +## Verification + +- From `client/`: Godot **4.6** headless import (if needed) then GdUnit: + `godot --headless --import --path . --quit-after 8` + `godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test` +- **Manual:** F5, cycle zoom (wheel, `=`, `-`, keypad), confirm framing and that **`CameraState`**-consumers (if any) see band index + distance; confirm **`zoom_band_config` cleared** falls back to **`follow_distance`**. From ff830813c766ba7669c49d182b9c12e43baf24c6 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 22:58:28 -0400 Subject: [PATCH 6/8] NEON-26: address code review follow-ups (zoom validation, docs) --- client/README.md | 4 +++- client/scripts/isometric_follow_camera.gd | 20 ++++++++++++-------- client/scripts/zoom_band_config.gd | 8 ++++++++ client/test/isometric_follow_camera_test.gd | 6 ++++++ client/test/zoom_band_config_test.gd | 21 +++++++++++++++++++++ docs/plans/NEON-26-implementation-plan.md | 3 ++- docs/reviews/2026-04-08-NEON-26.md | 19 ++++++++++--------- 7 files changed, 62 insertions(+), 19 deletions(-) diff --git a/client/README.md b/client/README.md index 546e14e..1744692 100644 --- a/client/README.md +++ b/client/README.md @@ -106,6 +106,8 @@ On **Linux** (including GitHub Actions), the path must be **`gdUnit4`** with a c **CI:** The **GDScript** workflow fails the job if Godot prints **`SCRIPT ERROR:`** or **`ERROR: Failed to load script`** while running GdUnit. GdUnit alone can still exit **0** when a test file fails to parse (that suite is skipped); the workflow guards against that. -**Scope:** Unit tests cover **`player.gd`**, **`position_authority_client.gd`**, and **`ground_pick.gd`** (walkable collider check). **`main.gd`**, full **`_input` / ray pick** flows, and scene wiring are **not** automated here—use manual checks above. +**Scope:** Unit tests cover **`player.gd`**, **`position_authority_client.gd`**, **`ground_pick.gd`** (walkable collider check), **`isometric_follow_camera.gd`** (eye math + effective zoom distance), **`camera_state.gd`**, and **`zoom_band_config.gd`**. **`main.gd`**, full **`_input` / ray pick** flows, and scene wiring are **not** automated here—use manual checks above. + +**Camera zoom:** Input actions **`camera_zoom_in`** / **`camera_zoom_out`** (mouse wheel, **`=`** / **`-`**, keypad **+** / **−**) are defined in **`project.godot`**. On layouts where **`+`** requires **Shift**, remap or add a binding under **Project → Project Settings → Input Map** if needed. **Reports:** GdUnit writes under **`reports/`** (gitignored); ignore locally generated HTML/XML when committing. diff --git a/client/scripts/isometric_follow_camera.gd b/client/scripts/isometric_follow_camera.gd index fbef68b..d36233e 100644 --- a/client/scripts/isometric_follow_camera.gd +++ b/client/scripts/isometric_follow_camera.gd @@ -9,7 +9,7 @@ extends Node3D const CameraStateScript := preload("res://scripts/camera_state.gd") const ZoomBandConfigScript := preload("res://scripts/zoom_band_config.gd") -## TODO(E9.M1): throttled `camera_zoom_changed` telemetry when schema exists. +## TODO(E9.M1): map throttled product telemetry (e.g. `camera_zoom_changed`) to this signal when schema exists. signal zoom_band_changed(new_index: int, distance: float) ## Tuned to match pre-NEON-25 static `Camera3D` at (12,10,12) vs player at (-5,0.9,-5). @@ -67,7 +67,7 @@ func _ready() -> void: ) global_position = _smoothed_eye camera.look_at(focus0, Vector3.UP) - _sync_camera_state(focus0) + _sync_camera_state(focus0, dist0) else: _smoothed_eye = global_position _warn_missing_follow_target_once() @@ -90,9 +90,9 @@ func _process(delta: float) -> void: var focus: Vector3 = target.global_position + Vector3(0.0, focus_vertical_offset, 0.0) var yaw_total: float = deg_to_rad(presentation_yaw_deg) + _orbit_yaw_rad - var dist: float = _current_follow_distance() + var effective_dist: float = _current_follow_distance() var desired: Vector3 = desired_eye_world( - focus, dist, deg_to_rad(pitch_elevation_deg), yaw_total + focus, effective_dist, deg_to_rad(pitch_elevation_deg), yaw_total ) if _smoothed_eye.distance_to(desired) >= snap_distance: @@ -103,7 +103,7 @@ func _process(delta: float) -> void: global_position = _smoothed_eye camera.look_at(focus, Vector3.UP) - _sync_camera_state(focus) + _sync_camera_state(focus, effective_dist) func _unhandled_input(event: InputEvent) -> void: @@ -124,7 +124,8 @@ func _apply_zoom_step(delta_bands: int) -> void: if new_idx == _zoom_band_index: return _zoom_band_index = new_idx - zoom_band_changed.emit(_zoom_band_index, _current_follow_distance()) + var d: float = _current_follow_distance() + zoom_band_changed.emit(_zoom_band_index, d) func _init_zoom_band_index() -> void: @@ -140,6 +141,7 @@ func _zoom_config_valid() -> bool: zoom_band_config != null and zoom_band_config.get_script() == ZoomBandConfigScript and zoom_band_config.band_count() > 0 + and zoom_band_config.all_band_distances_positive() ) @@ -154,14 +156,16 @@ static func effective_follow_distance( return fallback_distance if zoom_cfg.band_count() == 0: return fallback_distance + if not zoom_cfg.all_band_distances_positive(): + return fallback_distance return zoom_cfg.distance_at(band_index) -func _sync_camera_state(focus: Vector3) -> void: +func _sync_camera_state(focus: Vector3, effective_distance: float) -> void: # `CameraState.yaw` = orbit delta only; world-fixed diagonal framing = # `presentation_yaw_deg`. _state.follow_target_path = follow_target_path - _state.distance = _current_follow_distance() + _state.distance = effective_distance _state.zoom_band_index = _zoom_band_index if _zoom_config_valid() else 0 _state.focus_world = focus _state.yaw = _orbit_yaw_rad diff --git a/client/scripts/zoom_band_config.gd b/client/scripts/zoom_band_config.gd index a83dbe7..b8b1f7b 100644 --- a/client/scripts/zoom_band_config.gd +++ b/client/scripts/zoom_band_config.gd @@ -10,6 +10,14 @@ func band_count() -> int: return band_distances.size() +## **True** when every entry is **> 0** (empty array is vacuously true; callers also check [method band_count]). +func all_band_distances_positive() -> bool: + for i in range(band_distances.size()): + if band_distances[i] <= 0.0: + return false + return true + + func clamp_index(i: int) -> int: var n := band_count() if n == 0: diff --git a/client/test/isometric_follow_camera_test.gd b/client/test/isometric_follow_camera_test.gd index fbe0666..5bae4d4 100644 --- a/client/test/isometric_follow_camera_test.gd +++ b/client/test/isometric_follow_camera_test.gd @@ -35,3 +35,9 @@ func test_effective_follow_distance_empty_config_bands_falls_back() -> void: var cfg = ZoomBandConfigScript.new() cfg.band_distances = PackedFloat32Array() assert_that(IsoScript.effective_follow_distance(cfg, 0, 7.5)).is_equal(7.5) + + +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) diff --git a/client/test/zoom_band_config_test.gd b/client/test/zoom_band_config_test.gd index 901b72b..de83f51 100644 --- a/client/test/zoom_band_config_test.gd +++ b/client/test/zoom_band_config_test.gd @@ -46,3 +46,24 @@ func test_default_index_clamped_via_clamp_index() -> void: c.band_distances = PackedFloat32Array([1.0, 2.0, 3.0]) c.default_band_index = 99 assert_that(c.clamp_index(c.default_band_index)).is_equal(2) + + +func test_all_band_distances_positive_vacuous_when_empty() -> void: + var c = ZoomBandConfigScript.new() + c.band_distances = PackedFloat32Array() + assert_that(c.all_band_distances_positive()).is_true() + + +func test_all_band_distances_positive_true_when_all_positive() -> void: + var c = ZoomBandConfigScript.new() + c.band_distances = PackedFloat32Array([1.0, 2.5, 10.0]) + assert_that(c.all_band_distances_positive()).is_true() + + +func test_all_band_distances_positive_false_when_zero_or_negative() -> void: + var c0 = ZoomBandConfigScript.new() + c0.band_distances = PackedFloat32Array([10.0, 0.0, 20.0]) + assert_that(c0.all_band_distances_positive()).is_false() + var cn = ZoomBandConfigScript.new() + cn.band_distances = PackedFloat32Array([5.0, -1.0]) + assert_that(cn.all_band_distances_positive()).is_false() diff --git a/docs/plans/NEON-26-implementation-plan.md b/docs/plans/NEON-26-implementation-plan.md index 6c55701..eaf24c1 100644 --- a/docs/plans/NEON-26-implementation-plan.md +++ b/docs/plans/NEON-26-implementation-plan.md @@ -39,7 +39,7 @@ ## Technical approach 1. **`zoom_band_config.gd`** — `extends Resource` with e.g. `@export var band_distances: PackedFloat32Array` (length ≥ 1) and `@export var default_band_index: int`. Provide **`band_count`**, **`clamp_index(i: int) -> int`**, **`distance_at(index: int) -> float`** (uses clamped index). Validate in `_validate_property` or getter: if empty at runtime, treat as single band using rig fallback (see below). -2. **`isometric_follow_camera.gd`** — Add `@export var zoom_band_config: ZoomBandConfig`. Keep **`follow_distance`** as **fallback** when config is null or has no bands (preserves NEON-25 scenes/tests). Track **`_zoom_band_index`** initialized from config default (clamped). On zoom-in/out actions: adjust index with `clamp_index`, no op at ends (or wrap — **clamp** per story). **`_process`:** compute `var d := effective_follow_distance()` from config + index or fallback export. Replace uses of raw `follow_distance` in `desired_eye_world` with `d`. **`_sync_camera_state`:** assign `_state.zoom_band_index` and `_state.distance = d` (and existing yaw / path / focus). +2. **`isometric_follow_camera.gd`** — Add **`@export var zoom_band_config: Resource`** (no `class_name` on the config script); at runtime require **`get_script() == preload("…/zoom_band_config.gd")`** so headless CI and typed exports stay aligned. Conceptually this is **`ZoomBandConfig`**. Keep **`follow_distance`** as **fallback** when config is null, empty, or has non‑positive band distances (preserves NEON-25 scenes/tests). Track **`_zoom_band_index`** initialized from config default (clamped). On zoom-in/out actions: adjust index with `clamp_index`, no op at ends (or wrap — **clamp** per story). **`_process`:** compute effective distance from config + index or fallback export. Replace uses of raw `follow_distance` in `desired_eye_world` with that value. **`_sync_camera_state`:** pass effective distance into state (`zoom_band_index` + `distance`, plus existing yaw / path / focus). 3. **`project.godot`** — Add **`camera_zoom_in`** and **`camera_zoom_out`** (or one action with axis — prefer two actions for wheel + keys) with mouse wheel and key events as in Jira examples. 4. **Scene / resource** — Add a default **`ZoomBandConfig`** `.tres` under `res://resources/` (or co-located under `client/resources/`) with **≥ 3** bands bracketing **~25.709** m so readability can be checked; assign on **`IsometricFollowCamera`** in `main.tscn`. 5. **Telemetry stub** — Optional `signal zoom_band_changed(new_index: int, distance: float)` or `TODO` comment referencing E9.M1; no external schema required for this story. @@ -51,6 +51,7 @@ | **Band parameter** | **Distance-only** per step (same pitch / FOV as NEON-25); document in module snapshot. FOV-per-band deferred unless trivial. | | **Bounds at ends** | **Clamp** (no wrap): input at min/max does nothing extra. | | **Config null / empty** | Use **`follow_distance`** export only; **`zoom_band_index`** in state = `0`. | +| **Invalid band values** | Any **≤ 0** distance in **`band_distances`** treats the config as unusable (same fallback as null/empty). | ## Files to add diff --git a/docs/reviews/2026-04-08-NEON-26.md b/docs/reviews/2026-04-08-NEON-26.md index cf15c17..d2e0bcd 100644 --- a/docs/reviews/2026-04-08-NEON-26.md +++ b/docs/reviews/2026-04-08-NEON-26.md @@ -2,11 +2,12 @@ **Date:** 2026-04-08 **Scope:** Branch `NEON-26-zoom-band-config-discrete-input` vs `origin/main` (NEON-26 zoom bands + follow-up `.tres` tuning commits). Issue **NEON-26**; no PR URL supplied. -**Base:** `origin/main` @ `88653292104eddfc1db702f97b02a088fb80c1c7` (merge-base with HEAD at review time). +**Base:** `origin/main` @ `88653292104eddfc1db702f97b02a088fb80c1c7` (merge-base with HEAD at review time). +**Follow-up:** Suggestions and nits below were implemented in-repo (README, plan typing note, positive-distance validation, `_sync_camera_state(effective_distance)`, telemetry TODO wording). ## Verdict -**Approve with nits** +**Approve** ## Summary @@ -16,7 +17,7 @@ The branch delivers **data-driven discrete zoom** via `ZoomBandConfig`, **InputM | Document | Result | |----------|--------| -| `docs/plans/NEON-26-implementation-plan.md` | **Matches** intent and acceptance criteria. Minor **partial match**: the plan’s technical approach mentions `@export var zoom_band_config: ZoomBandConfig`; the implementation correctly uses **`Resource` + `get_script()`** comparison to avoid `class_name`, consistent with `godot-client-script-organization` and NEON-14 headless notes. Consider a one-line clarification in the plan that the export is untyped/`Resource` by design. | +| `docs/plans/NEON-26-implementation-plan.md` | **Matches** — technical approach updated to document **`Resource` export + `get_script()`** and positive-distance fallback; aligns with implementation. | | `docs/decomposition/modules/E1_M2_IsometricCameraController.md` | **Matches** — implementation snapshot updated for NEON-26 (config path, input actions, distance-only bands, fallback, signal + E9.M1 TODO). | | `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E1.M2 note reflects zoom bands shipped. | | `docs/decomposition/modules/client_server_authority.md` (camera client-local) | **Matches** — no server use of camera pose introduced. | @@ -28,19 +29,19 @@ None. ## Suggestions -1. **`client/README.md` (testing section)** — The bullet that lists covered scripts is now slightly stale: it does not mention **`isometric_follow_camera.gd`**, **`camera_state.gd`**, or **`zoom_band_config.gd`**, though **`client/test/`** includes suites for them. Per [testing-expectations](../../.cursor/rules/testing-expectations.md), consider extending that sentence so contributors know camera/zoom logic is under GdUnit. +1. ~~**`client/README.md` (testing section)** — The bullet that lists covered scripts is now slightly stale: it does not mention **`isometric_follow_camera.gd`**, **`camera_state.gd`**, or **`zoom_band_config.gd`**, though **`client/test/`** includes suites for them. Per [testing-expectations](../../.cursor/rules/testing-expectations.md), consider extending that sentence so contributors know camera/zoom logic is under GdUnit.~~ **Done.** Scope bullet extended; short **Camera zoom** note added for Input Map / layout remapping. -2. **`ZoomBandConfig` validation** — Designer-editable **`band_distances`** could theoretically include **non-positive** values, which would produce odd framing. Optional: `_validate_property` or a short runtime guard in the rig when loading config (non-blocking for prototype). +2. ~~**`ZoomBandConfig` validation** — Designer-editable **`band_distances`** could theoretically include **non-positive** values, which would produce odd framing. Optional: `_validate_property` or a short runtime guard in the rig when loading config (non-blocking for prototype).~~ **Done.** **`all_band_distances_positive()`** on the resource; **`_zoom_config_valid()`** and **`effective_follow_distance`** fall back when any distance is ≤ 0; tests added. -3. **Plan vs implementation (typing)** — As above, add a brief note to **`NEON-26-implementation-plan.md`** that **`zoom_band_config` is exported as `Resource`** (or untyped) with script identity checked at runtime, so future readers are not confused by the plan’s “ZoomBandConfig” type mention. +3. ~~**Plan vs implementation (typing)** — As above, add a brief note to **`NEON-26-implementation-plan.md`** that **`zoom_band_config` is exported as `Resource`** (or untyped) with script identity checked at runtime, so future readers are not confused by the plan’s “ZoomBandConfig” type mention.~~ **Done.** Technical approach step 2 rewritten accordingly. ## Nits -- **`_current_follow_distance()`** is invoked multiple times per **`_process`** / **`_sync_camera_state`**; a single local **`var d`** per frame would avoid redundant calls (micro-optimization, clarity). +- ~~**`_current_follow_distance()`** is invoked multiple times per **`_process`** / **`_sync_camera_state`**; a single local **`var d`** per frame would avoid redundant calls (micro-optimization, clarity).~~ **Done.** **`_sync_camera_state(focus, effective_distance)`** takes the distance computed once per frame (and **`_ready`** passes **`dist0`**). -- **Keyboard zoom:** **`=`** without **Shift** may not match all layouts’ “+” expectations; wheel and keypad bindings mitigate this. Document in README or input remap note if QA reports gaps. +- ~~**Keyboard zoom:** **`=`** without **Shift** may not match all layouts’ “+” expectations; wheel and keypad bindings mitigate this. Document in README or input remap note if QA reports gaps.~~ **Done.** Documented under **Camera zoom** in **`client/README.md`**. -- **Signal name vs telemetry TODO:** Comment mentions **`camera_zoom_changed`**; signal is **`zoom_band_changed`** — harmless but slightly inconsistent wording for whoever wires E9.M1 later. +- ~~**Signal name vs telemetry TODO:** Comment mentions **`camera_zoom_changed`**; signal is **`zoom_band_changed`** — harmless but slightly inconsistent wording for whoever wires E9.M1 later.~~ **Done.** TODO now references mapping product telemetry to **`zoom_band_changed`**. ## Verification From ae643bc81c876093d0bd05a560c8723d9e69ce00 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 23:00:10 -0400 Subject: [PATCH 7/8] NEON-26: refresh review after follow-up fixes --- docs/reviews/2026-04-08-NEON-26.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/reviews/2026-04-08-NEON-26.md b/docs/reviews/2026-04-08-NEON-26.md index d2e0bcd..76b3369 100644 --- a/docs/reviews/2026-04-08-NEON-26.md +++ b/docs/reviews/2026-04-08-NEON-26.md @@ -3,7 +3,7 @@ **Date:** 2026-04-08 **Scope:** Branch `NEON-26-zoom-band-config-discrete-input` vs `origin/main` (NEON-26 zoom bands + follow-up `.tres` tuning commits). Issue **NEON-26**; no PR URL supplied. **Base:** `origin/main` @ `88653292104eddfc1db702f97b02a088fb80c1c7` (merge-base with HEAD at review time). -**Follow-up:** Suggestions and nits below were implemented in-repo (README, plan typing note, positive-distance validation, `_sync_camera_state(effective_distance)`, telemetry TODO wording). +**Follow-up:** Suggestions and nits below were implemented in-repo (README, plan typing note, positive-distance validation, `_sync_camera_state(effective_distance)`, telemetry TODO wording) and re-reviewed after `ff83081`. ## Verdict @@ -11,7 +11,7 @@ ## Summary -The branch delivers **data-driven discrete zoom** via `ZoomBandConfig`, **InputMap** actions, **`_unhandled_input`** on the follow rig, and **`CameraState`** updates for effective distance and band index, with **clamp** semantics and **`follow_distance`** fallback when config is missing or empty. **GdUnit** coverage was added for the resource and **`effective_follow_distance`**. Module and dependency-register docs were updated. Overall risk is **low**: client-local camera only, no server coupling; logic is straightforward. Follow-up tuning of **`isometric_zoom_bands.tres`** (band count and distances) is data-only and acceptable on the same story. +The branch delivers **data-driven discrete zoom** via `ZoomBandConfig`, **InputMap** actions, **`_unhandled_input`** on the follow rig, and **`CameraState`** updates for effective distance and band index, with **clamp** semantics and **`follow_distance`** fallback when config is missing, empty, or invalid. **GdUnit** coverage was added for the resource and **`effective_follow_distance`**. Module and dependency-register docs were updated. Overall risk is **low**: client-local camera only, no server coupling; logic is straightforward. Re-review after the follow-up commit found **no additional issues**. ## Documentation checked @@ -48,4 +48,5 @@ None. - From `client/`: Godot **4.6** headless import (if needed) then GdUnit: `godot --headless --import --path . --quit-after 8` `godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test` +- **Observed on re-review:** GdUnit passed locally: **28 test cases, 0 failures**. - **Manual:** F5, cycle zoom (wheel, `=`, `-`, keypad), confirm framing and that **`CameraState`**-consumers (if any) see band index + distance; confirm **`zoom_band_config` cleared** falls back to **`follow_distance`**. From cdf9d59bd9870b165a60537d6e07188069c62472 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 8 Apr 2026 23:10:40 -0400 Subject: [PATCH 8/8] NEON-26: add pre-push GDScript lint hook Document the repo's enforced GDScript lint rules and add a local pre-push hook so gdlint/gdformat failures are caught before CI. --- .cursor/rules/gdscript-style.md | 31 +++++++++++++++++++-- client/README.md | 8 ++++++ client/scripts/isometric_follow_camera.gd | 11 ++++---- client/scripts/zoom_band_config.gd | 3 +- scripts/git-hooks/pre-push | 34 +++++++++++++++++++++++ scripts/install-git-hooks.sh | 19 +++++++++++++ 6 files changed, 97 insertions(+), 9 deletions(-) create mode 100755 scripts/git-hooks/pre-push create mode 100755 scripts/install-git-hooks.sh diff --git a/.cursor/rules/gdscript-style.md b/.cursor/rules/gdscript-style.md index f816b65..205d24c 100644 --- a/.cursor/rules/gdscript-style.md +++ b/.cursor/rules/gdscript-style.md @@ -8,6 +8,8 @@ alwaysApply: true Follow the [Godot GDScript style guide](https://docs.godotengine.org/en/stable/tutorials/scripting/gdscript/gdscript_styleguide.html). Prefer clarity and consistency with existing scripts in the repo. +CI also enforces **`gdlint`** and **`gdformat`** (see `gdlintrc` and `.github/workflows/gdscript.yml`) on **`client/scripts/`** and **`client/test/`**. When this guide and personal preference differ, follow the **linted** version below so pushed files pass CI. + ## Naming - **Functions, variables, signals:** `snake_case`. @@ -23,18 +25,43 @@ Follow the [Godot GDScript style guide](https://docs.godotengine.org/en/stable/t ## Formatting -- **One statement per line**; break long lines for readability (Godot line length is flexible—aim for ~100 characters unless a longer string is clearer). +- **One statement per line**; CI enforces **max line length = 100**. - **Indentation:** use **tabs** for new Godot work (editor default). Do not mix tabs and spaces in the same file; match the file if it already uses spaces. - **Blank lines:** separate functions with two blank lines; sparing single blank lines inside functions for logical groups. - **Trailing commas** in multi-line collections/function args when it improves diffs. +- No trailing whitespace. ## Structure - **`class_name`** only when the type must be referenced globally; otherwise anonymous `extends` is fine. -- Order loosely: `extends` → `class_name` → `signals` → `enums` → `const` → `export/@export` → `onready` → other vars → `_ready` / lifecycle → public methods → private helpers. +- Declaration order should match `gdlint` `class-definitions-order`: + `@tool` → `class_name` → `extends` → file/class docstrings → `signal` → `enum` → `const` → static vars → `@export` vars → public vars → private vars (`_foo`) → `@onready` public vars → `@onready` private vars → everything else. - Virtual overrides (`_ready`, `_process`, `_physics_process`, etc.): keep small; extract helpers with leading `_`. - **Scene / main script shape:** For `client/`, follow [godot-client-script-organization](godot-client-script-organization.md) — thin `main.gd`, split picking, networking, and similar concerns into child scripts. +## CI-enforced lint rules + +- **Names:** keep functions, local vars, loop vars, arguments, signals, and class vars in `snake_case`; keep classes / enums in `PascalCase`; keep enum elements and true constants in `ALL_CAPS`. +- **Signal callbacks:** `_on_Node_signal` style handlers are allowed; otherwise use normal `snake_case`. +- **Preload / load identifiers:** when storing a preloaded script or type-like handle, prefer `PascalCase` names such as `CameraStateScript`; keep ordinary constants in `ALL_CAPS`. +- Avoid duplicated `preload()`s for the same path in one file. +- Avoid bare expressions that are not assigned / returned / awaited. +- Remove unnecessary `pass`. +- Keep files reasonably small: CI enforces **max 1000 lines**, **max 20 public methods**, **max 10 function arguments**, and **max 6 returns** per function. +- Prefer early-return style. `gdlint` forbids `else` / `elif` branches that directly follow a `return`. +- Keep unused arguments intentional and named clearly if you truly need them. + +## Local verification + +- Before pushing GDScript changes, run the same tools CI uses: + `pip install "gdtoolkit==4.5.0"` + `gdlint client/scripts client/test` + `gdformat --check client/scripts client/test` +- If formatting fails, run: + `gdformat client/scripts client/test` +- Install the repo’s local **pre-push** hook from the repo root to enforce those checks automatically: + `./scripts/install-git-hooks.sh` + ## Comments - Use `#` comments; document non-obvious **why**, not what the next line literally does. diff --git a/client/README.md b/client/README.md index 1744692..150b09b 100644 --- a/client/README.md +++ b/client/README.md @@ -110,4 +110,12 @@ On **Linux** (including GitHub Actions), the path must be **`gdUnit4`** with a c **Camera zoom:** Input actions **`camera_zoom_in`** / **`camera_zoom_out`** (mouse wheel, **`=`** / **`-`**, keypad **+** / **−**) are defined in **`project.godot`**. On layouts where **`+`** requires **Shift**, remap or add a binding under **Project → Project Settings → Input Map** if needed. +**Git hook (recommended):** install the repo’s local **pre-push** GDScript lint hook from the repo root: + +```bash +./scripts/install-git-hooks.sh +``` + +It runs **`gdlint client/scripts client/test`** and **`gdformat --check client/scripts client/test`** before each push, preferring the repo-local **`.venv-gd/`** tools when present. + **Reports:** GdUnit writes under **`reports/`** (gitignored); ignore locally generated HTML/XML when committing. diff --git a/client/scripts/isometric_follow_camera.gd b/client/scripts/isometric_follow_camera.gd index d36233e..3b5ac20 100644 --- a/client/scripts/isometric_follow_camera.gd +++ b/client/scripts/isometric_follow_camera.gd @@ -6,12 +6,13 @@ extends Node3D ## [member allow_yaw] is false (no rotate input bound). Future orbit: read relative input here, ## add to `_orbit_yaw_rad`, clamp with [member max_yaw_deg]. +## TODO(E9.M1): map throttled product telemetry (for example `camera_zoom_changed`) +## to this signal when schema exists. +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") -## TODO(E9.M1): map throttled product telemetry (e.g. `camera_zoom_changed`) to this signal when schema exists. -signal zoom_band_changed(new_index: int, distance: float) - ## 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. @export var follow_target_path: NodePath = NodePath("../Player") @@ -62,9 +63,7 @@ func _ready() -> void: var focus0: Vector3 = t.global_position + Vector3(0.0, focus_vertical_offset, 0.0) var yaw0: float = deg_to_rad(presentation_yaw_deg) + _orbit_yaw_rad var dist0: float = _current_follow_distance() - _smoothed_eye = desired_eye_world( - focus0, dist0, deg_to_rad(pitch_elevation_deg), yaw0 - ) + _smoothed_eye = desired_eye_world(focus0, dist0, deg_to_rad(pitch_elevation_deg), yaw0) global_position = _smoothed_eye camera.look_at(focus0, Vector3.UP) _sync_camera_state(focus0, dist0) diff --git a/client/scripts/zoom_band_config.gd b/client/scripts/zoom_band_config.gd index b8b1f7b..dc3d36c 100644 --- a/client/scripts/zoom_band_config.gd +++ b/client/scripts/zoom_band_config.gd @@ -10,7 +10,8 @@ func band_count() -> int: return band_distances.size() -## **True** when every entry is **> 0** (empty array is vacuously true; callers also check [method band_count]). +## **True** when every entry is **> 0**. +## Empty arrays are vacuously true; callers also check [method band_count]. func all_band_distances_positive() -> bool: for i in range(band_distances.size()): if band_distances[i] <= 0.0: diff --git a/scripts/git-hooks/pre-push b/scripts/git-hooks/pre-push new file mode 100755 index 0000000..aab61d4 --- /dev/null +++ b/scripts/git-hooks/pre-push @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(git rev-parse --show-toplevel)" +cd "$repo_root" + +gdlint_bin="" +gdformat_bin="" + +if [[ -x "$repo_root/.venv-gd/bin/gdlint" ]]; then + gdlint_bin="$repo_root/.venv-gd/bin/gdlint" +elif command -v gdlint >/dev/null 2>&1; then + gdlint_bin="$(command -v gdlint)" +fi + +if [[ -x "$repo_root/.venv-gd/bin/gdformat" ]]; then + gdformat_bin="$repo_root/.venv-gd/bin/gdformat" +elif command -v gdformat >/dev/null 2>&1; then + gdformat_bin="$(command -v gdformat)" +fi + +if [[ -z "$gdlint_bin" || -z "$gdformat_bin" ]]; then + echo "pre-push: missing gdtoolkit commands (gdlint/gdformat)." >&2 + echo "Install locally with:" >&2 + echo " python3 -m venv .venv-gd" >&2 + echo " .venv-gd/bin/pip install \"gdtoolkit==4.5.0\"" >&2 + exit 1 +fi + +echo "pre-push: running gdlint on client/scripts and client/test" +"$gdlint_bin" client/scripts client/test + +echo "pre-push: running gdformat --check on client/scripts and client/test" +"$gdformat_bin" --check client/scripts client/test diff --git a/scripts/install-git-hooks.sh b/scripts/install-git-hooks.sh new file mode 100755 index 0000000..6752368 --- /dev/null +++ b/scripts/install-git-hooks.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(git rev-parse --show-toplevel)" +hook_dir="$repo_root/.git/hooks" +hook_path="$hook_dir/pre-push" + +mkdir -p "$hook_dir" + +cat >"$hook_path" <<'EOF' +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(git rev-parse --show-toplevel)" +exec "$repo_root/scripts/git-hooks/pre-push" "$@" +EOF + +chmod +x "$hook_path" +echo "Installed pre-push hook at $hook_path"