From 776ba2e55f74ea4aa2d6ac1b2b2f5cfdf2f46b58 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 25 Feb 2026 09:44:42 +0100 Subject: [PATCH] =?UTF-8?q?fix(client):=20address=20PR=20#65=20round=202?= =?UTF-8?q?=20=E2=80=94=20broken=20tests,=20BBCode=20escape,=20expiry=20lo?= =?UTF-8?q?op?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- client/tests/test_journal_sprint18.gd | 130 ++++++++++++++++---------- client/ui/dialogue_box.gd | 15 +-- 2 files changed, 92 insertions(+), 53 deletions(-) diff --git a/client/tests/test_journal_sprint18.gd b/client/tests/test_journal_sprint18.gd index c5fefddfd..ad2a5dd13 100644 --- a/client/tests/test_journal_sprint18.gd +++ b/client/tests/test_journal_sprint18.gd @@ -399,43 +399,67 @@ func test_update_from_state_does_not_close_when_dialogue_inactive() -> void: # --------------------------------------------------------------------------- -# Journal panel: CONFIDENCE_LABELS and SOURCE_LABELS constants +# UIStrings: confidence and source label keys (D-042 — now via UIStrings) +## CONFIDENCE_LABELS and SOURCE_LABELS dicts were removed from journal_panel.gd. +## Labels now come from UIStrings: knowledge_panel.confidence_* / knowledge_panel.source_* # --------------------------------------------------------------------------- -func test_confidence_labels_covers_all_four_levels() -> void: - ## D-041: Four confidence tiers. CONFIDENCE_LABELS must map all four. - var panel := _make_journal_panel() - if panel == null: return - var labels: Dictionary = panel.CONFIDENCE_LABELS - for level in ["Direct", "KnowsDetails", "KnowsOf", "Suspects"]: - assert_bool(labels.has(level)).override_failure_message( - "CONFIDENCE_LABELS must include '%s'" % level +func test_ui_strings_confidence_direct_exists() -> void: + ## D-042: confidence label for "Direct" tier must be in UIStrings. + assert_bool(UIStrings.has_key("knowledge_panel.confidence_direct")).override_failure_message( + "UIStrings must have 'knowledge_panel.confidence_direct' (D-042)" + ).is_true() + + +func test_ui_strings_confidence_knowsdetails_exists() -> void: + assert_bool(UIStrings.has_key("knowledge_panel.confidence_knowsdetails")).override_failure_message( + "UIStrings must have 'knowledge_panel.confidence_knowsdetails' (D-042)" + ).is_true() + + +func test_ui_strings_confidence_knowsof_exists() -> void: + assert_bool(UIStrings.has_key("knowledge_panel.confidence_knowsof")).override_failure_message( + "UIStrings must have 'knowledge_panel.confidence_knowsof' (D-042)" + ).is_true() + + +func test_ui_strings_confidence_suspects_exists() -> void: + assert_bool(UIStrings.has_key("knowledge_panel.confidence_suspects")).override_failure_message( + "UIStrings must have 'knowledge_panel.confidence_suspects' (D-042)" + ).is_true() + + +func test_ui_strings_all_confidence_keys_non_empty() -> void: + ## All four confidence label values must be non-empty strings. + var keys := [ + "knowledge_panel.confidence_direct", + "knowledge_panel.confidence_knowsdetails", + "knowledge_panel.confidence_knowsof", + "knowledge_panel.confidence_suspects", + ] + for key in keys: + if not UIStrings.has_key(key): continue + assert_bool(UIStrings.get_text(key).length() > 0).override_failure_message( + "UIStrings key '%s' must be non-empty" % key ).is_true() - panel.queue_free() -func test_source_labels_covers_known_sources() -> void: - ## D-041: Source types. SOURCE_LABELS must map core sources. - var panel := _make_journal_panel() - if panel == null: return - var labels: Dictionary = panel.SOURCE_LABELS - for source in ["DirectObservation", "ToldBy", "Heard"]: - assert_bool(labels.has(source)).override_failure_message( - "SOURCE_LABELS must include '%s'" % source - ).is_true() - panel.queue_free() +func test_ui_strings_source_directobservation_exists() -> void: + assert_bool(UIStrings.has_key("knowledge_panel.source_directobservation")).override_failure_message( + "UIStrings must have 'knowledge_panel.source_directobservation' (D-042)" + ).is_true() -func test_confidence_labels_all_non_empty() -> void: - ## All CONFIDENCE_LABELS values must be non-empty strings. - var panel := _make_journal_panel() - if panel == null: return - for key in panel.CONFIDENCE_LABELS: - var val: String = panel.CONFIDENCE_LABELS[key] - assert_bool(val.length() > 0).override_failure_message( - "CONFIDENCE_LABELS['%s'] must be non-empty" % key - ).is_true() - panel.queue_free() +func test_ui_strings_source_toldby_exists() -> void: + assert_bool(UIStrings.has_key("knowledge_panel.source_toldby")).override_failure_message( + "UIStrings must have 'knowledge_panel.source_toldby' (D-042)" + ).is_true() + + +func test_ui_strings_source_heard_exists() -> void: + assert_bool(UIStrings.has_key("knowledge_panel.source_heard")).override_failure_message( + "UIStrings must have 'knowledge_panel.source_heard' (D-042)" + ).is_true() # --------------------------------------------------------------------------- @@ -524,30 +548,42 @@ func test_ui_strings_knowledge_panel_tab_contacts_non_empty() -> void: # --------------------------------------------------------------------------- -# D-042 finding: CONFIDENCE_LABELS is hardcoded, not UIStrings -## The journal panel uses a hardcoded CONFIDENCE_LABELS dict rather than UIStrings. -## D-042 requires all labels via UIStrings YAML. -## Filed as a finding — not a blocking test, but documents the gap. +# D-042: CONFIDENCE_LABELS/SOURCE_LABELS now via UIStrings — FIXED (2026-02-25) +## Previously filed as a gap: journal_panel.gd had hardcoded CONFIDENCE_LABELS dict. +## Fixed by Stig: dicts removed, all labels now use UIStrings.get_text("knowledge_panel.*"). +## Regression guard: verify the dicts are gone and UIStrings fallback works. # --------------------------------------------------------------------------- -func test_d042_finding_confidence_labels_not_in_uistrings() -> void: - ## D-042 gap: journal_panel.gd uses hardcoded CONFIDENCE_LABELS dict. - ## Expected per D-042: UIStrings should drive confidence display labels. - ## ui-strings.yaml has confidence_high/medium/low/rumor but not Direct/KnowsDetails/etc. - ## These keys are not mapped to UIStrings. Flagging as technical debt. - ## - ## This test PASSES because we're documenting the gap, not requiring it to be fixed now. - ## See: journal_panel.gd CONFIDENCE_LABELS vs data/ui-strings.yaml knowledge_panel section. +func test_d042_fixed_panel_has_no_confidence_labels_dict() -> void: + ## Regression: CONFIDENCE_LABELS dict must NOT exist on journal_panel — it was removed. + ## If this test fails, the hardcoded dict was accidentally re-introduced. var panel := _make_journal_panel() if panel == null: return - var has_confidence_labels: bool = panel.CONFIDENCE_LABELS.size() > 0 - assert_bool(has_confidence_labels).is_true() # Hardcoded dict exists - # D-042 finding: confidence labels should use UIStrings.get_text("knowledge_panel.confidence_*") - # Current state: CONFIDENCE_LABELS = {"Direct": "Confirmed", "KnowsDetails": "Detailed", ...} - # Recommendation: replace with UIStrings lookup in a future sprint + assert_bool(panel.get("CONFIDENCE_LABELS") == null).override_failure_message( + "D-042 regression: CONFIDENCE_LABELS dict must be removed from journal_panel.gd" + ).is_true() panel.queue_free() +func test_d042_fixed_panel_has_no_source_labels_dict() -> void: + ## Regression: SOURCE_LABELS dict must NOT exist on journal_panel — it was removed. + var panel := _make_journal_panel() + if panel == null: return + assert_bool(panel.get("SOURCE_LABELS") == null).override_failure_message( + "D-042 regression: SOURCE_LABELS dict must be removed from journal_panel.gd" + ).is_true() + panel.queue_free() + + +func test_d042_uistrings_fallback_for_unknown_confidence() -> void: + ## UIStrings falls back to the key string itself for missing keys. + ## journal_panel.gd relies on this for graceful degradation. + var fallback := UIStrings.get_text("knowledge_panel.confidence_nonexistent_level") + assert_that(fallback).override_failure_message( + "UIStrings fallback must return the key string itself for unknown keys" + ).is_equal("knowledge_panel.confidence_nonexistent_level") + + # --------------------------------------------------------------------------- # Constants # --------------------------------------------------------------------------- diff --git a/client/ui/dialogue_box.gd b/client/ui/dialogue_box.gd index 90d7885cf..54a0f5b87 100644 --- a/client/ui/dialogue_box.gd +++ b/client/ui/dialogue_box.gd @@ -538,7 +538,7 @@ func _format_entry(entry: Dictionary, alpha: float) -> String: ## Escape BBCode bracket characters in server-sourced text (Hoshe #2). static func _escape_bbcode(text: String) -> String: - return text.replace("[", "[lb]") + return text.replace("[", "[lb]").replace("]", "[rb]") ## Get a stable color for a character name, with contrast floor enforcement. @@ -594,19 +594,22 @@ func _expire_entries() -> void: var removed := false var has_fading := false - # Remove expired non-pinned entries from the front (oldest first) - while _log_entries.size() > 0: - var entry: Dictionary = _log_entries[0] + # Remove expired non-pinned entries (oldest first, skipping pinned) + var i := 0 + while i < _log_entries.size(): + var entry: Dictionary = _log_entries[i] if entry.pinned: - break # Pinned entries never expire + i += 1 + continue # Pinned entries never expire — keep scanning var age: int = now - entry.timestamp_msec if age < total_lifetime_msec: # Check if this entry is in the fading phase if age > int(_entry_lifetime * 1000.0): has_fading = true break - _log_entries.remove_at(0) + _log_entries.remove_at(i) removed = true + # Don't increment i — element at i is now the next entry # Check remaining entries for fading state if not has_fading: