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

feat(ui): minimap, dialogue hardening, journal panel (#151, #174, #264) #65

Closed
jpmschweitzer wants to merge 0 commits from client into main
pull from: client
merge into: :main
Conversation 3 Commits - Files Changed -
jpmschweitzer commented 2026-02-25 02:31:25 +01:00
Owner
Copy Link
Copy Source

Summary

Sprint 18: Touch — Client team deliverables.

  • Protocol v14 — Three new ObserverSnapshot fields: poi_list (minimap), examine_result (examine overlay), player_knowledge (journal). Server populates live from KG and PointOfInterest components.
  • #151 Minimap rendering — Circular 160px diegetic insert overlay with POI dots (colored by category: diamond/danger, square/evidence, circle/default), border arrows for distant POIs, player-centered fixed-north, on InsertOverlay CanvasLayer 10.
  • #174 Dialogue UI hardening — Confrontation italic voice (D-063), examine result overlay with 5s auto-dismiss and confidence coloring. D-062 invisible locked options confirmed correct.
  • #264 Knowledge/journal panel — Right-side insert panel (J key), facts grouped by entity with confidence/source/state metadata, contradicted entries in amber+strikethrough (THE FRIEND arc), stale entries dimmed, mutual exclusion with dialogue.
  • 50 gdUnit4 tests — 30 dialogue tests (D-062, D-063, D-064, BBCode guard) + 20 journal tests (KG parsing, scene structure, UIStrings). Test plan document with manual procedures.

Decision compliance

D-013 (diegetic insert), D-020 (client pure renderer), D-041 (KG data model), D-042 (UIStrings YAML), D-049 (z-layer 6), D-061 (dialogue box), D-062 (invisible locked), D-063 (confrontation), D-064 (walk-away)

Test plan

  • Minimap: POI discovered → dot appears. Distant POI → border arrow. Empty state → frame only.
  • Examine: result overlay appears, auto-dismisses after 5s. Dismisses when dialogue opens.
  • Dialogue: no locked/grayed options visible. Confrontation in italic voice. Walk-away fades 300ms.
  • Journal: J key toggles panel. Facts grouped by entity. Contradicted = amber+strikethrough. Stale = dimmed. Closes when dialogue opens.
  • All 50 gdUnit4 tests pass.
## Summary Sprint 18: Touch — Client team deliverables. - **Protocol v14** — Three new ObserverSnapshot fields: `poi_list` (minimap), `examine_result` (examine overlay), `player_knowledge` (journal). Server populates live from KG and PointOfInterest components. - **#151 Minimap rendering** — Circular 160px diegetic insert overlay with POI dots (colored by category: diamond/danger, square/evidence, circle/default), border arrows for distant POIs, player-centered fixed-north, on InsertOverlay CanvasLayer 10. - **#174 Dialogue UI hardening** — Confrontation italic voice (D-063), examine result overlay with 5s auto-dismiss and confidence coloring. D-062 invisible locked options confirmed correct. - **#264 Knowledge/journal panel** — Right-side insert panel (J key), facts grouped by entity with confidence/source/state metadata, contradicted entries in amber+strikethrough (THE FRIEND arc), stale entries dimmed, mutual exclusion with dialogue. - **50 gdUnit4 tests** — 30 dialogue tests (D-062, D-063, D-064, BBCode guard) + 20 journal tests (KG parsing, scene structure, UIStrings). Test plan document with manual procedures. ## Decision compliance D-013 (diegetic insert), D-020 (client pure renderer), D-041 (KG data model), D-042 (UIStrings YAML), D-049 (z-layer 6), D-061 (dialogue box), D-062 (invisible locked), D-063 (confrontation), D-064 (walk-away) ## Test plan - [ ] Minimap: POI discovered → dot appears. Distant POI → border arrow. Empty state → frame only. - [ ] Examine: result overlay appears, auto-dismisses after 5s. Dismisses when dialogue opens. - [ ] Dialogue: no locked/grayed options visible. Confrontation in italic voice. Walk-away fades 300ms. - [ ] Journal: J key toggles panel. Facts grouped by entity. Contradicted = amber+strikethrough. Stale = dimmed. Closes when dialogue opens. - [ ] All 50 gdUnit4 tests pass.
jpmschweitzer added 7 commits 2026-02-25 02:31:25 +01:00
feat(simulation): protocol v14 — POI list, examine result, player knowledge wire types dd3ead7a1b 
Add three new ObserverSnapshot fields for client Sprint 18 tickets:
poi_list (Vec<PoiWire>) for minimap #151, examine_result
(Option<ExamineResultWire>) for #174, player_knowledge
(Option<PlayerKnowledgeWire>) for journal #264. POI list populated
live from KG-discovered PointOfInterest components. KG dump
serializes entity/fact knowledge with confidence, source, and state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(client): protocol v14 decoders and game state fields 80064196f7 
Decode poi_list, examine_result, and player_knowledge from
ObserverSnapshot. Add GameState.discovered_pois,
current_examine_result, and player_knowledge fields populated
from snapshot apply handlers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(ui): minimap rendering with POI dots and border arrows (#151) eeb82535f6 
Circular 160px diegetic insert overlay on CanvasLayer 10. Nearby
POIs as colored dots (diamond=danger, square=evidence, circle=default),
distant POIs as directional border arrows. Player centered, fixed-north.
Moved from UILayer to InsertOverlay per D-013/D-049.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(ui): dialogue UI hardening and examine result overlay (#174) 9b80ff69c3 
Confrontation options use RichTextLabel with italic tags for
first-person voice (D-063). Examine result overlay auto-dismisses
after 5s with confidence-based color tinting. Dismisses when
dialogue opens. D-062 invisible locked options confirmed correct.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(ui): knowledge/journal display panel (#264) 86ff83d927 
Right-side insert panel toggled with J key. Facts grouped by entity
with confidence, source, and state metadata. Contradicted entries
in amber with strikethrough (THE FRIEND arc surface). Stale entries
dimmed. Mutual exclusion with dialogue box. All labels via UIStrings.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
test(client): sprint 18 test suite — dialogue, journal, minimap (#151, #174, #264) dae16326fe 
50 tests across two files: 30 dialogue tests (D-062 compliance,
D-063 confrontation beat, D-064 walk-away, BBCode guard, size
constraints) and 20 journal tests (KG parsing, confidence/source/
state enums, scene structure, UIStrings keys, POI list). Test plan
document with manual procedures and sprint completion checklist.

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

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

Hoshe (Code Quality): REQUEST_CHANGES

Good test coverage and clean feature implementation, but two runtime defects.

# File Severity Issue
1 client/ui/minimap.gd:94 critical POI field key mismatch — reads "poi_category" but decoder stores "category". All POIs render as default white circles.
2 client/ui/journal_panel.gd:32-37 warning SOURCE_LABELS["ToldBy"] never matches server's "ToldBy(12)" format. Raw entity IDs shown in journal.
3 server/src/bridge/types.rs:45 suggestion Stale doc comment says "Current: 13", should be 14.
4 client/ui/dialogue_box.gd:598-608 suggestion Expiry loop stops at first pinned entry — non-pinned entries after it accumulate silently.
5 client/ui/dialogue_box.gd:654-659 suggestion Confrontation option hover-off restores wrong base color (INSERT_COLOR_TEXT vs warm tint).

Tyre (Architecture): REQUEST_CHANGES

Architecture is sound — D-010/D-012 respected throughout. Two issues need fixing.

# File Severity Issue
1 client/ui/minimap.gd:94 critical Same POI category key mismatch as Hoshe #1.
2 server/src/perception/observer/mod.rs:292-366 warning player_knowledge serialized every tick with no dirty flag — contradicts design comment, will cause per-tick allocation spikes as KG grows.
3 server/src/bridge/types.rs:45 warning Stale version doc comment (same as Hoshe #3).
4 client/ui/dialogue_box.gd:142 suggestion Calls private UIStrings._parse_yaml() — couples to internal API.
5 client/ui/journal_panel.gd:88-106 suggestion Full entry rebuild every tick with no diffing — fine for v0.1 counts, flag for fact display sprint.

Verdict: CHANGES REQUESTED

Deduplicated blockers:

  1. Critical — POI "category" vs "poi_category" key mismatch (one-line fix in minimap.gd)
  2. Warning — ToldBy(N) source label never matches SOURCE_LABELS dict
  3. Warning — player_knowledge serialized every tick (no dirty flag)
  4. Warning — Stale version comment in types.rs
## Review: client → main (PR #65, type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Good test coverage and clean feature implementation, but two runtime defects. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `client/ui/minimap.gd:94` | critical | POI field key mismatch — reads `"poi_category"` but decoder stores `"category"`. All POIs render as default white circles. | | 2 | `client/ui/journal_panel.gd:32-37` | warning | `SOURCE_LABELS["ToldBy"]` never matches server's `"ToldBy(12)"` format. Raw entity IDs shown in journal. | | 3 | `server/src/bridge/types.rs:45` | suggestion | Stale doc comment says "Current: 13", should be 14. | | 4 | `client/ui/dialogue_box.gd:598-608` | suggestion | Expiry loop stops at first pinned entry — non-pinned entries after it accumulate silently. | | 5 | `client/ui/dialogue_box.gd:654-659` | suggestion | Confrontation option hover-off restores wrong base color (INSERT_COLOR_TEXT vs warm tint). | ### Tyre (Architecture): REQUEST_CHANGES Architecture is sound — D-010/D-012 respected throughout. Two issues need fixing. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `client/ui/minimap.gd:94` | critical | Same POI category key mismatch as Hoshe #1. | | 2 | `server/src/perception/observer/mod.rs:292-366` | warning | `player_knowledge` serialized every tick with no dirty flag — contradicts design comment, will cause per-tick allocation spikes as KG grows. | | 3 | `server/src/bridge/types.rs:45` | warning | Stale version doc comment (same as Hoshe #3). | | 4 | `client/ui/dialogue_box.gd:142` | suggestion | Calls private `UIStrings._parse_yaml()` — couples to internal API. | | 5 | `client/ui/journal_panel.gd:88-106` | suggestion | Full entry rebuild every tick with no diffing — fine for v0.1 counts, flag for fact display sprint. | ### Verdict: CHANGES REQUESTED Deduplicated blockers: 1. **Critical** — POI `"category"` vs `"poi_category"` key mismatch (one-line fix in minimap.gd) 2. **Warning** — `ToldBy(N)` source label never matches `SOURCE_LABELS` dict 3. **Warning** — player_knowledge serialized every tick (no dirty flag) 4. **Warning** — Stale version comment in types.rs
jpmschweitzer added 1 commit 2026-02-25 02:41:36 +01:00
fix(client): address PR #65 review — POI key, ToldBy parsing, KG dirty flag a82eb0748a 
- minimap.gd: fix "poi_category" → "category" key mismatch so POI
  colors and shapes render correctly
- journal_panel.gd: parse ToldBy(N) source format, resolve entity
  names from player_knowledge; move confidence/source labels to
  UIStrings per D-042
- observer/mod.rs: add Changed<KnowledgeGraph> dirty flag to skip
  per-tick KG serialization when unchanged
- types.rs: fix stale version doc comment (13 → 14)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-25 09:42:26 +01:00
Author
Owner
Copy Link
Copy Source

Review: client -> main (type: code) — PR #65

Hoshe (Code Quality): REQUEST_CHANGES

Well-structured features with sound core logic. The a82eb07 fix commit correctly addresses POI key, ToldBy parsing, and KG dirty flag. However, two issues remain.

# File Severity Issue
1 client/tests/test_journal_sprint18.gd:409,421,433,543 critical Tests access panel.CONFIDENCE_LABELS and panel.SOURCE_LABELS — constants that don't exist on JournalPanel. Five tests will fail at runtime. Either add the constants to journal_panel.gd or rewrite tests to match the UIStrings-based implementation.
2 client/ui/dialogue_box.gd:541 warning _escape_bbcode only escapes [ but not ]. Should also .replace("]", "[rb]") for complete BBCode protection. This was flagged as "Hoshe #2" fix but is incomplete.
3 client/ui/dialogue_box.gd:598-609 warning _expire_entries breaks at first pinned entry — expired entries behind a pinned entry are never cleaned until unpin. Bounded leak but violates the stated contract.
4 client/ui/dialogue_box.gd:349 suggestion Sort comparator is correct but a comment clarifying "lower value = higher priority" would aid maintainability.
5 server/src/perception/observer/mod.rs:397 suggestion examine_result: None always — known stub for #242, acceptable but track as regression risk.
6 client/ui/journal_panel.gd:208-214 suggestion _last_rendered_tick not updated on dialogue-close, causing unnecessary rebuilds after reopen. No functional bug.

Tyre (Architecture): APPROVE

Architecture is sound throughout. D-010/D-012 baselines respected, information boundary honored at every layer, ObserverSnapshot remains the sole client-server data boundary. V14 protocol additions follow established versioning strategy (flat struct, #[serde(default)], additive-only). The a82eb07 fixes are architecturally correct.

# File Severity Issue
1 client/ui/journal_panel.gd:167-183 suggestion ToldBy parsing fix is correct. to_int() on malformed input degrades gracefully to "#0".
2 server/src/perception/observer/mod.rs:123-124 suggestion KG dirty flag capture before reborrow is idiomatic bevy_ecs. Empty KG guard handles first-tick edge case.
3 client/ui/journal_panel.gd:212-214 suggestion Tick-gated rebuild triggers every tick journal is open even if KG unchanged. Acceptable at current entity counts.
4 client/ui/dialogue_box.gd:142 suggestion Direct call to UIStrings._parse_yaml() crosses private method convention. Future refactor candidate.

Verdict: CHANGES REQUESTED

Hoshe flagged 1 critical (missing constants causing test failures) and 2 warnings (incomplete BBCode escape, pinned entry expiry gap). Tyre approved with suggestions only.

## Review: client -> main (type: code) — PR #65 ### Hoshe (Code Quality): REQUEST_CHANGES Well-structured features with sound core logic. The `a82eb07` fix commit correctly addresses POI key, ToldBy parsing, and KG dirty flag. However, two issues remain. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `client/tests/test_journal_sprint18.gd:409,421,433,543` | critical | Tests access `panel.CONFIDENCE_LABELS` and `panel.SOURCE_LABELS` — constants that don't exist on `JournalPanel`. Five tests will fail at runtime. Either add the constants to `journal_panel.gd` or rewrite tests to match the UIStrings-based implementation. | | 2 | `client/ui/dialogue_box.gd:541` | warning | `_escape_bbcode` only escapes `[` but not `]`. Should also `.replace("]", "[rb]")` for complete BBCode protection. This was flagged as "Hoshe #2" fix but is incomplete. | | 3 | `client/ui/dialogue_box.gd:598-609` | warning | `_expire_entries` breaks at first pinned entry — expired entries behind a pinned entry are never cleaned until unpin. Bounded leak but violates the stated contract. | | 4 | `client/ui/dialogue_box.gd:349` | suggestion | Sort comparator is correct but a comment clarifying "lower value = higher priority" would aid maintainability. | | 5 | `server/src/perception/observer/mod.rs:397` | suggestion | `examine_result: None` always — known stub for #242, acceptable but track as regression risk. | | 6 | `client/ui/journal_panel.gd:208-214` | suggestion | `_last_rendered_tick` not updated on dialogue-close, causing unnecessary rebuilds after reopen. No functional bug. | ### Tyre (Architecture): APPROVE Architecture is sound throughout. D-010/D-012 baselines respected, information boundary honored at every layer, `ObserverSnapshot` remains the sole client-server data boundary. V14 protocol additions follow established versioning strategy (flat struct, `#[serde(default)]`, additive-only). The `a82eb07` fixes are architecturally correct. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `client/ui/journal_panel.gd:167-183` | suggestion | ToldBy parsing fix is correct. `to_int()` on malformed input degrades gracefully to `"#0"`. | | 2 | `server/src/perception/observer/mod.rs:123-124` | suggestion | KG dirty flag capture before reborrow is idiomatic bevy_ecs. Empty KG guard handles first-tick edge case. | | 3 | `client/ui/journal_panel.gd:212-214` | suggestion | Tick-gated rebuild triggers every tick journal is open even if KG unchanged. Acceptable at current entity counts. | | 4 | `client/ui/dialogue_box.gd:142` | suggestion | Direct call to `UIStrings._parse_yaml()` crosses private method convention. Future refactor candidate. | ### Verdict: CHANGES REQUESTED Hoshe flagged 1 critical (missing constants causing test failures) and 2 warnings (incomplete BBCode escape, pinned entry expiry gap). Tyre approved with suggestions only.
jpmschweitzer added 1 commit 2026-02-25 09:44:50 +01:00
fix(client): address PR #65 round 2 — broken tests, BBCode escape, expiry loop 776ba2e55f 
- test_journal_sprint18.gd: replace references to removed
  CONFIDENCE_LABELS/SOURCE_LABELS with UIStrings key tests and
  regression guards
- dialogue_box.gd: escape ] as [rb] in _escape_bbcode for complete
  BBCode injection protection
- dialogue_box.gd: fix _expire_entries to skip pinned entries with
  continue instead of break, cleaning expired entries behind pins

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-25 09:47:41 +01:00
Author
Owner
Copy Link
Copy Source

Review Round 3: client -> main — PR #65

Hoshe (Code Quality): APPROVE

All three round 2 issues confirmed fixed:

  1. Missing constants — Tests rewritten to use UIStrings lookups. Regression guards added.
  2. BBCode escape — Now escapes both [ and ].
  3. Expiry loop — Rewritten to continue past pinned entries instead of break.
# File Severity Issue
1 client/ui/dialogue_box.gd:615-621 suggestion Second fading pass re-examines entries already evaluated. Harmless.
2 client/tests/test_dialogue_sprint18.gd:432-440 suggestion BBCode tests don't explicitly assert [rb] for ] escaping.

Tyre (Architecture): APPROVE

Fix commit is narrow and correct. D-042 boundary respected. D-010 information boundary intact. No new concerns.

Verdict: APPROVED

Ready to merge.

## Review Round 3: client -> main — PR #65 ### Hoshe (Code Quality): APPROVE All three round 2 issues confirmed fixed: 1. **Missing constants** — Tests rewritten to use UIStrings lookups. Regression guards added. 2. **BBCode escape** — Now escapes both `[` and `]`. 3. **Expiry loop** — Rewritten to `continue` past pinned entries instead of `break`. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `client/ui/dialogue_box.gd:615-621` | suggestion | Second fading pass re-examines entries already evaluated. Harmless. | | 2 | `client/tests/test_dialogue_sprint18.gd:432-440` | suggestion | BBCode tests don't explicitly assert `[rb]` for `]` escaping. | ### Tyre (Architecture): APPROVE Fix commit is narrow and correct. D-042 boundary respected. D-010 information boundary intact. No new concerns. ### Verdict: APPROVED Ready to merge.
jpmschweitzer closed this pull request 2026-02-25 09:59:28 +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#65