jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(client): Sprint 6 Touch — z-layer pipeline, cursor, fog, interactions, inventory, stance #22

Closed
jpmschweitzer wants to merge 0 commits from client into main
pull from: client
merge into: :main
Conversation 2 Commits - Files Changed -
jpmschweitzer commented 2026-02-15 23:06:52 +01:00
Owner
Copy Link
Copy Source

Summary

Sprint 6 client team delivery: movement stances, fog shader rebuild, multi-verb interactions, smuggler inventory, and the three-scope z-layer rendering architecture.

Protocol

  • Client protocol v6 bridge — player_stance + player_inventory decode, stance input actions, 25 tests

Architecture

  • Three-scope z-layer rendering pipeline (D-049): world z:0-900 inside CanvasGroup, insert CanvasLayer 10, UI CanvasLayer 20, modal CanvasLayer 30
  • Y-sort contract enforced (entities z_index=0), reserved VFX/airborne/lower-floor ranges
  • Architecture docs: z-layer gap analysis, fog shader spec, flying taxi feasibility analysis

UI Systems (6 tickets)

  • #429 Cursor state machine — 4 states, 150ms transitions (D-056)
  • #430 Fog shader rebuild — 5-layer fragment shader, animated Perlin noise (D-059)
  • #432 Entity interaction list — vertical multi-verb, insert-styled (D-057)
  • #433 World radial menu — 2 spokes, drag-release + click-click (D-058)
  • #438 Inventory UI — 3x3 grid, 1-9 hotkeys (D-065)
  • #439 Stance indicator — color-coded HUD, C/X keybinds (D-053)

Breaking Changes

  • Scene tree z_index values restructured (was 0-10, now 0-900)
  • fog_renderer.gd deleted, replaced by fog_shader.gd + fog.gdshader
  • Protocol version 5 → 6

Test plan

  • godot --headless --quit — client launches without errors
  • Protocol v6 tests pass (25 tests in test_protocol_v6.gd)
  • Cursor state tests pass (test_cursor_states.gd)
  • Fog shader tests pass (test_fog_shader.gd)
  • Interaction list tests pass (test_interaction_list.gd)
  • Z-layer visual inspection: entities y-sort correctly with furniture (z_index=0 contract)
  • Fog shader <1ms/frame (Godot profiler)
  • No regressions in existing rendering tests

🤖 Generated with Claude Code

## Summary Sprint 6 client team delivery: movement stances, fog shader rebuild, multi-verb interactions, smuggler inventory, and the three-scope z-layer rendering architecture. ### Protocol - Client protocol v6 bridge — player_stance + player_inventory decode, stance input actions, 25 tests ### Architecture - Three-scope z-layer rendering pipeline (D-049): world z:0-900 inside CanvasGroup, insert CanvasLayer 10, UI CanvasLayer 20, modal CanvasLayer 30 - Y-sort contract enforced (entities z_index=0), reserved VFX/airborne/lower-floor ranges - Architecture docs: z-layer gap analysis, fog shader spec, flying taxi feasibility analysis ### UI Systems (6 tickets) - **#429** Cursor state machine — 4 states, 150ms transitions (D-056) - **#430** Fog shader rebuild — 5-layer fragment shader, animated Perlin noise (D-059) - **#432** Entity interaction list — vertical multi-verb, insert-styled (D-057) - **#433** World radial menu — 2 spokes, drag-release + click-click (D-058) - **#438** Inventory UI — 3x3 grid, 1-9 hotkeys (D-065) - **#439** Stance indicator — color-coded HUD, C/X keybinds (D-053) ### Breaking Changes - Scene tree z_index values restructured (was 0-10, now 0-900) - fog_renderer.gd deleted, replaced by fog_shader.gd + fog.gdshader - Protocol version 5 → 6 ## Test plan - [ ] `godot --headless --quit` — client launches without errors - [ ] Protocol v6 tests pass (25 tests in test_protocol_v6.gd) - [ ] Cursor state tests pass (test_cursor_states.gd) - [ ] Fog shader tests pass (test_fog_shader.gd) - [ ] Interaction list tests pass (test_interaction_list.gd) - [ ] Z-layer visual inspection: entities y-sort correctly with furniture (z_index=0 contract) - [ ] Fog shader <1ms/frame (Godot profiler) - [ ] No regressions in existing rendering tests 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer added 4 commits 2026-02-15 23:06:52 +01:00
feat(client): protocol v6 bridge — stance, inventory, input mapping 24cce41379 
Upgrade client protocol bridge from v5 to v6 to match server.
Adds player_stance (4 variants) and player_inventory decode to
ObserverSnapshot. Adds TOGGLE_STANCE_UP/DOWN to InputMapper.
Includes 25 gdUnit4 tests for v6 decode + server serialization
test gap fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(client): Sprint 6 Touch — z-layer pipeline, cursor, fog, interactions, inventory, stance, radial 1fcf08d21f 
Three-scope z-layer rendering pipeline (D-049): world z:0-900 inside
CanvasGroup, insert overlay CanvasLayer 10, UI CanvasLayer 20, modal
CanvasLayer 30. Y-sort contract enforced (entities z_index=0). Reserved
ranges for VFX, airborne, lower floors documented in constants.gd.

Sprint 6 client tickets:
- #429: Cursor state machine — 4 states, 150ms transitions (D-056)
- #430: Fog shader rebuild — 5-layer fragment shader, animated noise (D-059)
- #432: Entity interaction list — vertical multi-verb, insert-styled (D-057)
- #433: World radial menu — 2 spokes, drag-release + click-click (D-058)
- #438: Inventory UI — 3x3 grid, 40x40px, 1-9 hotkeys (D-065)
- #439: Stance indicator — color-coded HUD, C/X keybinds (D-053)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
docs(architecture): z-layer gap analysis, fog shader spec, flying taxi analysis f08710fa61 
Three architecture documents from Sprint 6 design work:
- z-layer-gap-analysis.md: full three-scope rendering pipeline with
  reserved z-ranges, airborne treatment, cross-floor VFX, liquid depth
- fog-shader-spec.md: 5-layer fog shader architecture (D-059)
- flying-taxi-analysis.md: validates z-layer architecture supports
  flight scenarios with LOD tiers and performance budget

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(meta): update changelog 1b0953db25 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-15 23:16:17 +01:00
Author
Owner
Copy Link
Copy Source

Review: client -> main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Well-architected Sprint 6 implementation with 1,361 lines of test code across 4 test files. Fog shader rebuild, cursor state machine, and UI components are cleanly separated and follow D-049 z-layer architecture. Three critical bugs must be fixed before merge.

# File Severity Issue
1 fog_state.gd:99-108 critical _compute_bounds() crashes on all-invalid tile arrays — min/max sentinels produce negative-sized Rect2i, crashing _resize() with negative image dimensions. Add guard: if min_x > max_x: return Rect2i(0,0,1,1)
2 fog_shader.gd:46-50 critical Hardcoded zoom := Vector2(2.0, 2.0) with "must match Camera2D" comment — hidden coupling breaks silently if zoom ever changes. Read zoom from actual Camera2D node instead.
3 world_radial.gd:150-153 critical global_position = _origin - size / 2.0 relies on size being initialized, but no explicit size is set on this Control. First open will position incorrectly. Set custom_minimum_size in _ready() based on radial geometry.
4 cursor_renderer.gd:118-125 warning EntityRenderer._color_for_kind() called as static but not declared static — fragile if EntityRenderer is refactored. Extract to Constants.gd or mark explicitly static.
5 fog.gdshader:27-28 warning PERIPHERAL_LOW = 0.15 is far below actual peripheral value (0.706). Peripheral tiles will render as "light fog" in a very narrow band — may not match D-059 visual intent. Review thresholds.
6 main.gd:39-44 warning Monologue consumption has no tick-based deduplication — if snapshot polling has edge cases, same monologue could show twice or be lost.
7 fog_state.gd:79-95 suggestion _prev_visible.duplicate() is shallow copy — correct for Dictionary<Vector2i, bool> but document the assumption.
8 interaction_list.gd:79,119 suggestion is_instance_valid(lbl) checks before queue_free() suggest ownership ambiguity — document why or remove if unnecessary.

Coverage gaps: no tests for _compute_bounds() with invalid input, no tests for radial positioning, no fog shader performance test at 1080p.

Tyre (Architecture): APPROVE

Exemplary architectural work. The three-scope z-layer pipeline (D-049) maps 1:1 to the spec. D-010/D-012 baselines fully respected — GameState is pure data store, protocol v6 maintains strict wire format discipline, information boundaries honored (cursor LOS gating, fog knowledge-graph-driven). Coupling is unidirectional: FogShader → FogState → GameState. All 1,361 test lines validate decision compliance. Production-ready.

# File Severity Issue
1 fog_state.gd:66 suggestion Exploration decay is immediate (255→128) — gradual decay over game-time would better represent fading memory. v0.2+ enhancement.
2 cursor_renderer.gd:90 suggestion Bracket _hover_offset cached from _detect_hover() — recalculate in _draw() from current canvas transform to avoid wobble during camera movement.
3 interaction_list.gd:19 suggestion FADE_IN=0.12s / FADE_OUT=0.10s differs from cursor's 150ms — align or document rationale for mismatch.
4 world_radial.gd:119 suggestion Pause action reused for both game freeze and insert toggle — consider separate ToggleInsert action in protocol v7+.
5 — suggestion No .msgpack fixtures for v6 stance/inventory variations — add snapshot_v6_sprint.msgpack etc. for regression protection.

Verdict: CHANGES REQUESTED

Tyre approves architecture. Hoshe flags 3 critical bugs that need fixing before merge: (1) fog_state _compute_bounds crash on edge-case input, (2) fog_shader hardcoded zoom — hidden coupling, (3) world_radial sizing — uninitialized Control size.

## Review: client -> main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Well-architected Sprint 6 implementation with 1,361 lines of test code across 4 test files. Fog shader rebuild, cursor state machine, and UI components are cleanly separated and follow D-049 z-layer architecture. Three critical bugs must be fixed before merge. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | fog_state.gd:99-108 | critical | `_compute_bounds()` crashes on all-invalid tile arrays — min/max sentinels produce negative-sized Rect2i, crashing `_resize()` with negative image dimensions. Add guard: `if min_x > max_x: return Rect2i(0,0,1,1)` | | 2 | fog_shader.gd:46-50 | critical | Hardcoded `zoom := Vector2(2.0, 2.0)` with "must match Camera2D" comment — hidden coupling breaks silently if zoom ever changes. Read zoom from actual Camera2D node instead. | | 3 | world_radial.gd:150-153 | critical | `global_position = _origin - size / 2.0` relies on `size` being initialized, but no explicit size is set on this Control. First open will position incorrectly. Set `custom_minimum_size` in `_ready()` based on radial geometry. | | 4 | cursor_renderer.gd:118-125 | warning | `EntityRenderer._color_for_kind()` called as static but not declared static — fragile if EntityRenderer is refactored. Extract to Constants.gd or mark explicitly static. | | 5 | fog.gdshader:27-28 | warning | `PERIPHERAL_LOW = 0.15` is far below actual peripheral value (0.706). Peripheral tiles will render as "light fog" in a very narrow band — may not match D-059 visual intent. Review thresholds. | | 6 | main.gd:39-44 | warning | Monologue consumption has no tick-based deduplication — if snapshot polling has edge cases, same monologue could show twice or be lost. | | 7 | fog_state.gd:79-95 | suggestion | `_prev_visible.duplicate()` is shallow copy — correct for Dictionary<Vector2i, bool> but document the assumption. | | 8 | interaction_list.gd:79,119 | suggestion | `is_instance_valid(lbl)` checks before `queue_free()` suggest ownership ambiguity — document why or remove if unnecessary. | Coverage gaps: no tests for `_compute_bounds()` with invalid input, no tests for radial positioning, no fog shader performance test at 1080p. ### Tyre (Architecture): APPROVE Exemplary architectural work. The three-scope z-layer pipeline (D-049) maps 1:1 to the spec. D-010/D-012 baselines fully respected — GameState is pure data store, protocol v6 maintains strict wire format discipline, information boundaries honored (cursor LOS gating, fog knowledge-graph-driven). Coupling is unidirectional: FogShader → FogState → GameState. All 1,361 test lines validate decision compliance. Production-ready. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | fog_state.gd:66 | suggestion | Exploration decay is immediate (255→128) — gradual decay over game-time would better represent fading memory. v0.2+ enhancement. | | 2 | cursor_renderer.gd:90 | suggestion | Bracket `_hover_offset` cached from `_detect_hover()` — recalculate in `_draw()` from current canvas transform to avoid wobble during camera movement. | | 3 | interaction_list.gd:19 | suggestion | FADE_IN=0.12s / FADE_OUT=0.10s differs from cursor's 150ms — align or document rationale for mismatch. | | 4 | world_radial.gd:119 | suggestion | Pause action reused for both game freeze and insert toggle — consider separate `ToggleInsert` action in protocol v7+. | | 5 | — | suggestion | No `.msgpack` fixtures for v6 stance/inventory variations — add `snapshot_v6_sprint.msgpack` etc. for regression protection. | ### Verdict: CHANGES REQUESTED Tyre approves architecture. Hoshe flags 3 critical bugs that need fixing before merge: (1) fog_state `_compute_bounds` crash on edge-case input, (2) fog_shader hardcoded zoom — hidden coupling, (3) world_radial sizing — uninitialized Control size.
jpmschweitzer added 1 commit 2026-02-15 23:23:17 +01:00
fix(client): address PR #22 review — 3 critical bugs, 2 warnings, 8 suggestions 9e8ccb13bf 
Critical fixes:
- fog_state: guard _compute_bounds() against all-invalid tiles (negative Rect2i crash)
- fog_shader: read Camera2D zoom dynamically instead of hardcoded Vector2(2,2)
- world_radial: set custom_minimum_size in _ready() from spoke geometry

Warnings:
- fog.gdshader: tighten PERIPHERAL_LOW 0.15→0.55 to match D-059 peripheral band
- Extract color_for_entity_kind() to Constants.gd, decouple CursorRenderer from EntityRenderer

Documentation: shallow copy assumption, gradual decay TODO, tween guard rationale,
fade timing rationale, ToggleInsert TODO, monologue consume-once, hover offset safety,
v6 fixture gap TODO.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-15 23:27:49 +01:00
Author
Owner
Copy Link
Copy Source

Re-review: client -> main (round 2)

Fix commit 9e8ccb1 addresses all issues from round 1.

Hoshe (Code Quality): APPROVE

All 3 critical bugs fixed, both warnings addressed, all suggestions documented.

# Original Issue Status
1 _compute_bounds crash Fixed — guard for all-invalid tiles
2 Hardcoded camera zoom Fixed — get_viewport().get_camera_2d() with fallback
3 Radial menu sizing Fixed — explicit custom_minimum_size in _ready()
4 EntityRenderer static call Fixed — extracted to Constants.color_for_entity_kind()
5 PERIPHERAL_LOW threshold Fixed — 0.15 → 0.55
6 Monologue dedup Documented — server contract guarantee
7 Shallow copy docs Documented
8 is_instance_valid docs Documented — tween race condition

Tyre (Architecture): APPROVE

All 5 suggestions handled. Constants extraction improves Phase 2 (#361) path. No new coupling.

# Suggestion Status
1 Exploration decay TODO(v0.2)
2 Bracket wobble Safety documented
3 Fade timing Rationale documented
4 Insert/Pause TODO(v7)
5 v6 fixtures TODO documented

Verdict: APPROVED — merging.

## Re-review: client -> main (round 2) Fix commit `9e8ccb1` addresses all issues from round 1. ### Hoshe (Code Quality): APPROVE All 3 critical bugs fixed, both warnings addressed, all suggestions documented. | # | Original Issue | Status | |---|---------------|--------| | 1 | `_compute_bounds` crash | **Fixed** — guard for all-invalid tiles | | 2 | Hardcoded camera zoom | **Fixed** — `get_viewport().get_camera_2d()` with fallback | | 3 | Radial menu sizing | **Fixed** — explicit `custom_minimum_size` in `_ready()` | | 4 | EntityRenderer static call | **Fixed** — extracted to `Constants.color_for_entity_kind()` | | 5 | PERIPHERAL_LOW threshold | **Fixed** — 0.15 → 0.55 | | 6 | Monologue dedup | **Documented** — server contract guarantee | | 7 | Shallow copy docs | **Documented** | | 8 | is_instance_valid docs | **Documented** — tween race condition | ### Tyre (Architecture): APPROVE All 5 suggestions handled. Constants extraction improves Phase 2 (#361) path. No new coupling. | # | Suggestion | Status | |---|-----------|--------| | 1 | Exploration decay | `TODO(v0.2)` | | 2 | Bracket wobble | Safety documented | | 3 | Fade timing | Rationale documented | | 4 | Insert/Pause | `TODO(v7)` | | 5 | v6 fixtures | TODO documented | ### Verdict: APPROVED — merging.
jpmschweitzer closed this pull request 2026-02-15 23:28:01 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#22