fix(client): address PR #65 round 2 — broken tests, BBCode escape, expiry loop
- 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>
This commit is contained in:
@@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user