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

feat(client): Sprint 7 — dialogue box + fog entity visualization (#434, #431) #24

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

Summary

  • Dialogue box (#434, D-061): Bottom-screen UI skeleton — NPC speech + max 3 response options, insert-styled colors, WASD walk-away with 300ms fade, no close button. Diegetic on InsertOverlay (CanvasLayer 10, z-layer 6). Mock test data for Sprint 7; server dialogue system (#305) wires in Sprint 8+.
  • Fog entity visualization (#431, D-059/D-060): Cognitive delay rendering — sonar-style sound pings (3 concentric rings, 1.5s fade), unrecognized grey blobs with 0.8s breathing pulse, D-033 color transition at 50% recognition progress, ±0.5 tile position drift. FogEntities node at z:950 between fog shader and InsertOverlay.
  • Protocol v7 bump: Client protocol version 6→7 to match server PR #23 (pending_recognitions field in ObserverSnapshot).

Completes Sprint 7 client: 8/8 tickets done.

Files changed

New:

  • client/ui/dialogue_box.gd + .tscn — dialogue box Control
  • client/scripts/rendering/fog_entities.gd — fog entity renderer

Modified:

  • client/scenes/main.tscn — DialogueBox in InsertOverlay, FogEntities at z:950
  • client/scripts/main.gd — wiring for both systems
  • client/scripts/autoloads/game_state.gd — current_dialogue + pending_recognitions fields
  • client/scripts/autoloads/sim_bridge.gd — mock dialogue + mock cognitive delay data
  • client/scripts/protocol/protocol.gd — v7 bump + pending_recognitions decode
  • client/scripts/constants.gd — Z_FOG_ENTITIES constant

Test plan

  • Launch in test mode: fog entity grey blob appears at (13.5, 12.5), cycles through 6-tick cognitive delay, transitions to teal with silhouette
  • Sound pings: 3 concentric rings expand and fade on entity appearance
  • Press E near NPC: dialogue box appears bottom-center with Kael speech + 3 options
  • WASD during dialogue: box fades over 300ms (walk-away)
  • Click response option: option_selected signal fires, box hides
  • Monologue + dialogue visible simultaneously (different z-layers)
  • No performance regression: fog entities + dialogue box < 1ms combined
  • Protocol v7: client accepts server snapshots with pending_recognitions field

🤖 Generated with Claude Code

## Summary - **Dialogue box (#434, D-061):** Bottom-screen UI skeleton — NPC speech + max 3 response options, insert-styled colors, WASD walk-away with 300ms fade, no close button. Diegetic on InsertOverlay (CanvasLayer 10, z-layer 6). Mock test data for Sprint 7; server dialogue system (#305) wires in Sprint 8+. - **Fog entity visualization (#431, D-059/D-060):** Cognitive delay rendering — sonar-style sound pings (3 concentric rings, 1.5s fade), unrecognized grey blobs with 0.8s breathing pulse, D-033 color transition at 50% recognition progress, ±0.5 tile position drift. FogEntities node at z:950 between fog shader and InsertOverlay. - **Protocol v7 bump:** Client protocol version 6→7 to match server PR #23 (pending_recognitions field in ObserverSnapshot). Completes Sprint 7 client: 8/8 tickets done. ## Files changed **New:** - `client/ui/dialogue_box.gd` + `.tscn` — dialogue box Control - `client/scripts/rendering/fog_entities.gd` — fog entity renderer **Modified:** - `client/scenes/main.tscn` — DialogueBox in InsertOverlay, FogEntities at z:950 - `client/scripts/main.gd` — wiring for both systems - `client/scripts/autoloads/game_state.gd` — current_dialogue + pending_recognitions fields - `client/scripts/autoloads/sim_bridge.gd` — mock dialogue + mock cognitive delay data - `client/scripts/protocol/protocol.gd` — v7 bump + pending_recognitions decode - `client/scripts/constants.gd` — Z_FOG_ENTITIES constant ## Test plan - [ ] Launch in test mode: fog entity grey blob appears at (13.5, 12.5), cycles through 6-tick cognitive delay, transitions to teal with silhouette - [ ] Sound pings: 3 concentric rings expand and fade on entity appearance - [ ] Press E near NPC: dialogue box appears bottom-center with Kael speech + 3 options - [ ] WASD during dialogue: box fades over 300ms (walk-away) - [ ] Click response option: option_selected signal fires, box hides - [ ] Monologue + dialogue visible simultaneously (different z-layers) - [ ] No performance regression: fog entities + dialogue box < 1ms combined - [ ] Protocol v7: client accepts server snapshots with pending_recognitions field 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer added 3 commits 2026-02-16 01:20:06 +01:00
feat(ui): add dialogue box — bottom screen, max 20% height, walk-away (#434) 2d56131221 
D-061 compliant UI skeleton: NPC speech top, max 3 response options
below, left-aligned. Insert-styled colors (white-blue, amber hover,
green pressed). WASD walk-away fades over 300ms, no close button.
InsertOverlay (CanvasLayer 10, z-layer 6). Mock data in next commit.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(client): add fog entity visualization and wire Sprint 7 UI (#431, #434) 2659de0c28 
Fog entity cognitive delay rendering (D-059/D-060): sonar-style sound
pings (3 concentric rings, 1.5s fade), unrecognized grey blobs with
breathing pulse, D-033 color transition at 50% recognition progress,
±0.5 tile position drift. FogEntities node at z:950 between fog
shader and InsertOverlay.

Protocol v7 bump to match server PR #23 (pending_recognitions field).
Wires dialogue box and fog entities into game loop with mock test data.

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

Review: client → main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Clean implementation of dialogue box and fog entity visualization with proper spec adherence. However, 3 critical bugs need fixing: a stale protocol test, missing signal connections, and a consume-once race condition.

# File Severity Issue
1 client/tests/test_protocol_v6.gd:30 critical Protocol version test still checks for v6 but PROTOCOL_VERSION is now 7. Test will fail.
2 client/scripts/main.gd:53-63 critical Dialogue box signals (option_selected, dialogue_dismissed) never connected. Selecting options or walking away has no effect on simulation.
3 client/scripts/main.gd:56-63 critical Consume-once pattern broken — current_dialogue set to null after showing, but if new snapshot arrives during 200ms fade-in with null, client sees "dismiss". Need dialogue ID tracking or move consume logic to dialogue_box internal state.
4 client/tests/ warning Zero test coverage for fog_entities.gd (165 lines) and dialogue_box.gd (127 lines).
5 client/scripts/rendering/fog_entities.gd:40 warning queue_redraw() every frame unconditionally. Add early-return when no entities and no pings.
6 client/ui/dialogue_box.gd:33-47 suggestion 8 separate is_action_pressed() calls — use action list loop.
7 client/ui/dialogue_box.gd:82-92 suggestion Hardcoded button colors — reference Constants for consistency with other insert-styled UI.
8 client/scripts/rendering/fog_entities.gd:96 suggestion Magic number 0.01 for color transition threshold — use named constant.
9 client/scripts/rendering/fog_entities.gd:111 suggestion Silhouette threshold 0.3 and size Vector2(6, 10) lack explanation.
10 client/scripts/protocol/protocol.gd:126 suggestion No bounds check on pending_recognitions array size.
11 client/scripts/autoloads/sim_bridge.gd:342-355 suggestion Test mode dialogue appears for 1 tick then vanishes — can't test sustained dialogue.

Tyre (Architecture): REQUEST_CHANGES

Strong z-layer adherence and correct protocol v7 bump. Cognitive delay visualization correctly implements D-059/D-060. Two architectural violations against D-061: fixed-pixel width instead of max-width, and missing auto-pause.

# File Severity Issue
1 client/ui/dialogue_box.tscn:27-29 critical D-061 specifies max-width (not percentage) but implementation uses fixed pixel offsets (±416px = 832px). Breaks at non-standard resolutions. Use anchor_right = 1.0 with margin constants.
2 client/ui/dialogue_box.gd critical D-061 specifies auto-pause in single-player when dialogue open. No pause/unpause logic present.
3 client/ui/dialogue_box.tscn:8-10 warning Fixed 200px height = 28% at 720p, violating D-061's "max 20% height". Height should use viewport percentage.
4 client/scripts/rendering/fog_entities.gd:75-82 warning Drift animation not deterministic — fine for v0.1 single-player but flag for multiplayer.
5 client/ui/dialogue_box.gd:28 suggestion FADE_OUT = 0.3 — consider millisecond constant matching D-064's "300ms" wording.
6 client/scripts/rendering/fog_entities.gd:13-18 suggestion COLOR_PING reuses cursor color — document intentional coupling or decouple.
7 client/scripts/main.gd:56-63 suggestion Dialogue consume pattern duplicates monologue pattern — extract helper.
8 client/ui/dialogue_box.gd:59-72 suggestion Insert-styled button colors should live in theme resource or Constants.

Decision compliance: D-049 (z-layers) ✅ | D-059 (fog) ✅ | D-060 (cognitive delay) ✅ | D-061 (dialogue box) ❌ width/pause | D-064 (walk-away) ✅

Verdict: CHANGES REQUESTED

Must fix (5 critical):

  1. Protocol version test still checks v6
  2. Dialogue signal connections missing — options do nothing
  3. Consume-once race condition during fade-in
  4. Dialogue box width: fixed pixels instead of D-061 max-width
  5. Missing auto-pause when dialogue open

Should fix (3 warnings):

  • Zero test coverage for new features
  • queue_redraw() every frame when idle
  • Fixed 200px height violates D-061 max 20% at 720p
## Review: client → main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Clean implementation of dialogue box and fog entity visualization with proper spec adherence. However, 3 critical bugs need fixing: a stale protocol test, missing signal connections, and a consume-once race condition. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | client/tests/test_protocol_v6.gd:30 | critical | Protocol version test still checks for v6 but PROTOCOL_VERSION is now 7. Test will fail. | | 2 | client/scripts/main.gd:53-63 | critical | Dialogue box signals (`option_selected`, `dialogue_dismissed`) never connected. Selecting options or walking away has no effect on simulation. | | 3 | client/scripts/main.gd:56-63 | critical | Consume-once pattern broken — `current_dialogue` set to null after showing, but if new snapshot arrives during 200ms fade-in with null, client sees "dismiss". Need dialogue ID tracking or move consume logic to dialogue_box internal state. | | 4 | client/tests/ | warning | Zero test coverage for fog_entities.gd (165 lines) and dialogue_box.gd (127 lines). | | 5 | client/scripts/rendering/fog_entities.gd:40 | warning | `queue_redraw()` every frame unconditionally. Add early-return when no entities and no pings. | | 6 | client/ui/dialogue_box.gd:33-47 | suggestion | 8 separate `is_action_pressed()` calls — use action list loop. | | 7 | client/ui/dialogue_box.gd:82-92 | suggestion | Hardcoded button colors — reference Constants for consistency with other insert-styled UI. | | 8 | client/scripts/rendering/fog_entities.gd:96 | suggestion | Magic number `0.01` for color transition threshold — use named constant. | | 9 | client/scripts/rendering/fog_entities.gd:111 | suggestion | Silhouette threshold `0.3` and size `Vector2(6, 10)` lack explanation. | | 10 | client/scripts/protocol/protocol.gd:126 | suggestion | No bounds check on `pending_recognitions` array size. | | 11 | client/scripts/autoloads/sim_bridge.gd:342-355 | suggestion | Test mode dialogue appears for 1 tick then vanishes — can't test sustained dialogue. | ### Tyre (Architecture): REQUEST_CHANGES Strong z-layer adherence and correct protocol v7 bump. Cognitive delay visualization correctly implements D-059/D-060. Two architectural violations against D-061: fixed-pixel width instead of max-width, and missing auto-pause. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | client/ui/dialogue_box.tscn:27-29 | critical | D-061 specifies max-width (not percentage) but implementation uses fixed pixel offsets (±416px = 832px). Breaks at non-standard resolutions. Use `anchor_right = 1.0` with margin constants. | | 2 | client/ui/dialogue_box.gd | critical | D-061 specifies auto-pause in single-player when dialogue open. No pause/unpause logic present. | | 3 | client/ui/dialogue_box.tscn:8-10 | warning | Fixed 200px height = 28% at 720p, violating D-061's "max 20% height". Height should use viewport percentage. | | 4 | client/scripts/rendering/fog_entities.gd:75-82 | warning | Drift animation not deterministic — fine for v0.1 single-player but flag for multiplayer. | | 5 | client/ui/dialogue_box.gd:28 | suggestion | `FADE_OUT = 0.3` — consider millisecond constant matching D-064's "300ms" wording. | | 6 | client/scripts/rendering/fog_entities.gd:13-18 | suggestion | `COLOR_PING` reuses cursor color — document intentional coupling or decouple. | | 7 | client/scripts/main.gd:56-63 | suggestion | Dialogue consume pattern duplicates monologue pattern — extract helper. | | 8 | client/ui/dialogue_box.gd:59-72 | suggestion | Insert-styled button colors should live in theme resource or Constants. | **Decision compliance:** D-049 (z-layers) ✅ | D-059 (fog) ✅ | D-060 (cognitive delay) ✅ | D-061 (dialogue box) ❌ width/pause | D-064 (walk-away) ✅ --- ### Verdict: CHANGES REQUESTED **Must fix (5 critical):** 1. Protocol version test still checks v6 2. Dialogue signal connections missing — options do nothing 3. Consume-once race condition during fade-in 4. Dialogue box width: fixed pixels instead of D-061 max-width 5. Missing auto-pause when dialogue open **Should fix (3 warnings):** - Zero test coverage for new features - `queue_redraw()` every frame when idle - Fixed 200px height violates D-061 max 20% at 720p
jpmschweitzer added 2 commits 2026-02-16 01:30:19 +01:00
Merge remote-tracking branch 'origin/main' into client 90a80e73df
fix(client): address PR #24 review — 5 critical bugs, 3 warnings, 8 suggestions 6d6b59c17b 
Critical:
- Protocol test assertions updated v6→v7 (test_protocol_v6.gd)
- Dialogue signal connections wired (option_selected→DialogueResponse,
  dialogue_dismissed→DialogueEnd sent to server via SimBridge)
- Consume-once race fixed: _consume_dialogue() checks is_dialogue_active()
  before re-showing; dialogue_id tracking prevents re-trigger during fade
- Dialogue box responsive: _update_layout() clamps width to MAX_WIDTH_PX
  (832px) or 65% viewport, height to 20% viewport (MAX_HEIGHT_RATIO)
- Auto-pause added: SimBridge.send_input(PAUSE) on dialogue open/close

Warnings:
- Test coverage: 16 new tests in test_protocol_v7.gd (pending_recognitions
  decode, current_dialogue, GameState, SimBridge mock data, insert colors)
- queue_redraw() optimization: early return when no entities and no pings
- Fixed 200px height → responsive 20% viewport via _update_layout()

Suggestions:
- WASD detection refactored to _WALK_AWAY_ACTIONS array loop
- Button colors reference Constants.INSERT_COLOR_TEXT/HOVER/ACTIVE
- Named constants: COLOR_TRANSITION_START, SILHOUETTE_APPEAR_THRESHOLD,
  SILHOUETTE_SIZE with explanatory comments
- Bounds check: MAX_PENDING_RECOGNITIONS=64 with truncation warning
- Mock dialogue sustained across ticks (not 1-tick flash)
- COLOR_PING coupling with cursor documented as intentional
- Consume helpers extracted: _consume_monologue(), _consume_dialogue()

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

Re-review: fix commit 6d6b59c

All 5 criticals resolved, all 3 warnings fixed, all 8 suggestions addressed.

Verdict: APPROVED — merged to main.

Follow-up: test_protocol_v6.gd should be renamed — version numbers in test filenames go stale every sprint. Will rename on main.

## Re-review: fix commit 6d6b59c All 5 criticals resolved, all 3 warnings fixed, all 8 suggestions addressed. ### Verdict: **APPROVED** — merged to main. **Follow-up:** `test_protocol_v6.gd` should be renamed — version numbers in test filenames go stale every sprint. Will rename on main.
jpmschweitzer closed this pull request 2026-02-16 01:33:15 +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#24