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

feat(client): Sprint 24 Signal — character select, triangle activation, news ticker #86

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-03-05 11:53:20 +01:00
Owner
Copy Link
Copy Source

Summary

Sprint 24 capstone client delivery — three tickets wiring the storyteller's activation event into player-visible consequences:

  • #588 Character archetype select screen: two-card UI (Smuggler/Detective) between New Game and session start. Keyboard+mouse selection, ESC cancels. GameState.character_archetype persisted and sent in StartupMessage. PROTOCOL_VERSION bumped to 19.
  • #590 Triangle activation consumer: decodes triangle_crisis_events from snapshot, fires sfx_monologue_chime_urgent once per triangle per session. No overlay UI — the monologue system handles the narrative response.
  • #592 News ticker HUD: scrolling marquee on UILayer, visible only when current_ticker is present in snapshot (The Last Shift zone). Zero-arg update_from_state reads from GameState.current_snapshot.

Coordination

⚠️ PROTOCOL_VERSION = 19 — hard coordination point with server PR (already merged to main). Client and server must land together or handshake will reject.

Test plan

  • 16 new tests in test_signal_sprint24.gd (all passing)
  • test_protocol_bridge.gd updated for v19 (passing)
  • Character select: New Game → card selection → archetype in GameState → correct StartupMessage
  • Triangle chime: fires once on first triangle_crisis_events, does not re-fire on subsequent ticks
  • News ticker: visible with text when current_ticker present, hidden when null
  • No regressions in pre-existing test suite

🤖 Generated with Claude Code

## Summary Sprint 24 capstone client delivery — three tickets wiring the storyteller's activation event into player-visible consequences: - **#588** Character archetype select screen: two-card UI (Smuggler/Detective) between New Game and session start. Keyboard+mouse selection, ESC cancels. `GameState.character_archetype` persisted and sent in `StartupMessage`. `PROTOCOL_VERSION` bumped to 19. - **#590** Triangle activation consumer: decodes `triangle_crisis_events` from snapshot, fires `sfx_monologue_chime_urgent` once per triangle per session. No overlay UI — the monologue system handles the narrative response. - **#592** News ticker HUD: scrolling marquee on UILayer, visible only when `current_ticker` is present in snapshot (The Last Shift zone). Zero-arg `update_from_state` reads from `GameState.current_snapshot`. ## Coordination ⚠️ **PROTOCOL_VERSION = 19** — hard coordination point with server PR (already merged to main). Client and server must land together or handshake will reject. ## Test plan - [ ] 16 new tests in `test_signal_sprint24.gd` (all passing) - [ ] `test_protocol_bridge.gd` updated for v19 (passing) - [ ] Character select: New Game → card selection → archetype in GameState → correct StartupMessage - [ ] Triangle chime: fires once on first `triangle_crisis_events`, does not re-fire on subsequent ticks - [ ] News ticker: visible with text when `current_ticker` present, hidden when null - [ ] No regressions in pre-existing test suite 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer added 6 commits 2026-03-05 11:53:20 +01:00
chore(docs): add git command chaining rule to CLAUDE.md 3e1bcd90b2 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merge remote-tracking branch 'origin/main' into client da0fd7c16c 
# Conflicts:
#	CLAUDE.md
Merge remote-tracking branch 'origin/main' into client 7ba2be0652
feat(client): character select, triangle activation consumer, news ticker 61d454228d 
Sprint 24 Signal — three client tickets delivering the player-facing
storyteller feedback loop:

- #588: Character archetype select screen between New Game and session
  start. Two-card UI (Smuggler/Detective), keyboard+mouse, ESC cancels.
  GameState.character_archetype persisted and sent in StartupMessage.
  PROTOCOL_VERSION bumped to 19.
- #590: Triangle crisis event consumer. Decodes triangle_crisis_events
  from snapshot, fires sfx_monologue_chime_urgent once per triangle per
  session via AudioManager.CHIME_ACTIVATION.
- #592: News ticker HUD element. Scrolling marquee on UILayer, visible
  only when current_ticker is present in snapshot (Last Shift zone).
  Zero-arg update_from_state reads from GameState.current_snapshot.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
test(client): Sprint 24 signal tests 7dbd3247d4 
16 tests covering character select, triangle activation consumer,
news ticker, and protocol v19 bridge. Includes show/hide behavior
for ticker on null current_ticker.

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

Review: client → main (PR #86, type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Three features well-structured with solid test coverage. Two correctness bugs need fixing.

# File:line Severity Issue
1 protocol.gd:473 critical String.capitalize() silently passes unknown archetype strings to server with no validation. Should be explicit match with push_error fallback.
2 main.gd:306–314 critical _known_triangle_ids never cleared on teleport — _teleport_transition() clears _known_recognition_ids but not triangle IDs. Chime won't re-fire after room change.
3 news_ticker.gd:40–41 warning get_minimum_size() called before layout pass — first headline scroll distance will be wrong. Defer width read by one frame.
4 test_signal_sprint24.gd warning No test for unknown archetype validation path in encode_startup_message.
5 test_signal_sprint24.gd warning Triangle chime dedup logic in main.gd untested — only Protocol decode is covered.
6 session_manager.gd:151–157 suggestion save_character_archetype() not called inside new_game() — callers must pair them manually.
7 test_protocol_bridge.gd:285–288 suggestion Hardcoded version 8 in harness test is misleading vs protocol v19.
8 character_select.gd:86–87 suggestion String node paths ("CardBorder", "CardInner") not validated at parse time.
9 main_menu.gd:57–64 suggestion Synchronous load() of character select scene — inconsistent with threaded pattern used elsewhere.

Tyre (Architecture): APPROVE

Protocol v19 contract matches server. Character select flow correctly threads archetype through GameState → SessionManager → StartupMessage. D-012 respected — triangle events carry only opaque IDs. Zone-gating is server-authoritative per D-020.

# File:line Severity Issue
1 main.gd:306–314 warning _known_triangle_ids not cleared on teleport (same as Hoshe #2).
2 protocol.gd:259–268 suggestion Triangle event decode silently drops role_assignments, trigger_npc_id, tick from wire. Add comment noting intentional v0.1 omission.
3 main.gd:307, news_ticker.gd:28 suggestion Triangle events and ticker read from raw snapshot dict instead of typed GameState fields — pattern divergence from other consumers.

Verdict: CHANGES REQUESTED

Must-fix items:

  1. _known_triangle_ids.clear() in _teleport_transition() (both reviewers flagged)
  2. Archetype validation in encode_startup_message — explicit match, not capitalize()
  3. Ticker _content_width — defer measurement by one frame
## Review: `client` → `main` (PR #86, type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Three features well-structured with solid test coverage. Two correctness bugs need fixing. | # | File:line | Severity | Issue | |---|-----------|----------|-------| | 1 | `protocol.gd:473` | critical | `String.capitalize()` silently passes unknown archetype strings to server with no validation. Should be explicit match with `push_error` fallback. | | 2 | `main.gd:306–314` | critical | `_known_triangle_ids` never cleared on teleport — `_teleport_transition()` clears `_known_recognition_ids` but not triangle IDs. Chime won't re-fire after room change. | | 3 | `news_ticker.gd:40–41` | warning | `get_minimum_size()` called before layout pass — first headline scroll distance will be wrong. Defer width read by one frame. | | 4 | `test_signal_sprint24.gd` | warning | No test for unknown archetype validation path in `encode_startup_message`. | | 5 | `test_signal_sprint24.gd` | warning | Triangle chime dedup logic in `main.gd` untested — only Protocol decode is covered. | | 6 | `session_manager.gd:151–157` | suggestion | `save_character_archetype()` not called inside `new_game()` — callers must pair them manually. | | 7 | `test_protocol_bridge.gd:285–288` | suggestion | Hardcoded version `8` in harness test is misleading vs protocol v19. | | 8 | `character_select.gd:86–87` | suggestion | String node paths (`"CardBorder"`, `"CardInner"`) not validated at parse time. | | 9 | `main_menu.gd:57–64` | suggestion | Synchronous `load()` of character select scene — inconsistent with threaded pattern used elsewhere. | ### Tyre (Architecture): APPROVE Protocol v19 contract matches server. Character select flow correctly threads archetype through GameState → SessionManager → StartupMessage. D-012 respected — triangle events carry only opaque IDs. Zone-gating is server-authoritative per D-020. | # | File:line | Severity | Issue | |---|-----------|----------|-------| | 1 | `main.gd:306–314` | warning | `_known_triangle_ids` not cleared on teleport (same as Hoshe #2). | | 2 | `protocol.gd:259–268` | suggestion | Triangle event decode silently drops `role_assignments`, `trigger_npc_id`, `tick` from wire. Add comment noting intentional v0.1 omission. | | 3 | `main.gd:307`, `news_ticker.gd:28` | suggestion | Triangle events and ticker read from raw snapshot dict instead of typed GameState fields — pattern divergence from other consumers. | --- ### Verdict: CHANGES REQUESTED **Must-fix items:** 1. `_known_triangle_ids.clear()` in `_teleport_transition()` (both reviewers flagged) 2. Archetype validation in `encode_startup_message` — explicit match, not `capitalize()` 3. Ticker `_content_width` — defer measurement by one frame
jpmschweitzer added 1 commit 2026-03-05 16:28:17 +01:00
fix(client): address PR #86 review — archetype validation, teleport clear, ticker layout e0eb3cd35e 
- protocol.gd: replace capitalize() with explicit match for archetype
  string mapping, push_error on unknown input with Detective fallback
- main.gd: clear _known_triangle_ids in _teleport_transition() alongside
  _known_recognition_ids so chime re-fires after room change
- news_ticker.gd: defer get_minimum_size() via call_deferred to run
  after layout pass, fixing first-frame scroll distance
- 3 new tests: unknown archetype fallback, triangle dedup per-id,
  independent triangle ID firing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-03-05 16:43:51 +01:00
Author
Owner
Copy Link
Copy Source

Re-review: client → main (PR #86, round 2)

Hoshe (Code Quality): APPROVE

All 5 round-1 items verified resolved:

  1. ✅ _known_triangle_ids.clear() in _teleport_transition()
  2. ✅ Explicit match replaces capitalize() with push_error fallback
  3. ✅ Ticker width deferred via call_deferred("_update_content_width")
  4. ✅ Unknown archetype test added
  5. ✅ Triangle dedup tests added (2 tests)
  6. ✅ Intentional v0.1 field omission comment on triangle decode

Tyre (Architecture): APPROVE

Teleport clear correctly placed alongside recognition ID clear. Archetype match mirrors server #[serde(default)] — both sides default to Detective on bad input. Wire field omission comment accurate.

Verdict: APPROVED ✅

Ready for merge.

## Re-review: `client` → `main` (PR #86, round 2) ### Hoshe (Code Quality): APPROVE All 5 round-1 items verified resolved: 1. ✅ `_known_triangle_ids.clear()` in `_teleport_transition()` 2. ✅ Explicit `match` replaces `capitalize()` with `push_error` fallback 3. ✅ Ticker width deferred via `call_deferred("_update_content_width")` 4. ✅ Unknown archetype test added 5. ✅ Triangle dedup tests added (2 tests) 6. ✅ Intentional v0.1 field omission comment on triangle decode ### Tyre (Architecture): APPROVE Teleport clear correctly placed alongside recognition ID clear. Archetype match mirrors server `#[serde(default)]` — both sides default to Detective on bad input. Wire field omission comment accurate. ### Verdict: APPROVED ✅ Ready for merge.
jpmschweitzer closed this pull request 2026-03-05 16:44:43 +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#86