fix(theme-guard): close the drift guard's third miss — the meta-guard's own continue whitelisted the gap
test_committed_tres_matches_builder has now been "fixed" three times (traps.md #12). v1 hardcoded 3 checks; v2 iterated ThemeKeys.ALL but missed the six font roles; v3 added a meta-guard specifically to catch "the builder styles a thing guarded by nothing" — but that meta-guard contained `if fresh.get_type_variation_base(variation) == &"": continue`, which whitelisted exactly the case it was built to catch. The builder styles RichTextLabel directly (build_game_theme.gd's _rich_text()), with no set_type_variation, so it reports base "" and was skipped unconditionally — and RichTextLabel is what renders every line of DM prose the creation screen shows (_origin_text, _detail_blurb). Nothing compared its fonts, font size, or colour, nor the theme-level default_font/default_font_size, against the committed .tres. Adds ThemeKeys.BASE_TYPES for base Control types the builder styles directly, folds it into the drift guard's coverage, replaces the meta-guard's continue with an assertion that names the offending type, and adds the missing theme-level default font/size comparison. Proved with three reverted breaks: a RichTextLabel font-size edit, a theme-level default_font_size edit, and an unregistered "GhostRole" variation — all three now go red and name the drifted property. Also: strengthens test_creation_copy's error-copy sweep to assert a mapped error actually reaches its authored line rather than silently falling through to the generic UNSPOKEN fallback (CreationCopy.UNSPOKEN satisfied all four prior checks, so a renamed validator string could regress silently); and removes two assertions that were true by construction / already true before their test's action ran. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -47,6 +47,19 @@ const FONT_ROLES := {
|
|||||||
SECTION_LABEL: "Label",
|
SECTION_LABEL: "Label",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
## Base Control types the builder styles DIRECTLY — no set_type_variation is
|
||||||
|
## ever called for these, so a fresh Theme reports get_type_variation_base("")
|
||||||
|
## for them. Kept as its own set (not folded into ALL or FONT_ROLES) because a
|
||||||
|
## base type is not a variation: nothing sets `theme_type_variation` to
|
||||||
|
## "RichTextLabel" anywhere, the node's own class name is the lookup key.
|
||||||
|
## Today: RichTextLabel, the builder's prose voice (mock README Typography:
|
||||||
|
## serif body/italic emphasis) — the creation screen's origin panel and
|
||||||
|
## calling detail panel are RichTextLabels rendering this styling directly,
|
||||||
|
## i.e. every line of DM prose the screen shows.
|
||||||
|
const BASE_TYPES := {
|
||||||
|
&"RichTextLabel": true,
|
||||||
|
}
|
||||||
|
|
||||||
## Every variation name + the base Control type it decorates. The builder and the
|
## Every variation name + the base Control type it decorates. The builder and the
|
||||||
## test both iterate this so they can never drift apart.
|
## test both iterate this so they can never drift apart.
|
||||||
const ALL := {
|
const ALL := {
|
||||||
|
|||||||
@@ -130,7 +130,6 @@ func test_no_node_on_the_screen_says_luck():
|
|||||||
var forbidden: Array = ["luck", "lck"]
|
var forbidden: Array = ["luck", "lck"]
|
||||||
for band in Luck.BANDS:
|
for band in Luck.BANDS:
|
||||||
forbidden.append(str(band["text"]).to_lower())
|
forbidden.append(str(band["text"]).to_lower())
|
||||||
assert_eq(forbidden.size(), Luck.BANDS.size() + 2, "every band's text must be swept, not just the word")
|
|
||||||
|
|
||||||
for race_i in range(Races.IDS.size()):
|
for race_i in range(Races.IDS.size()):
|
||||||
s._race_cards[race_i].pressed.emit()
|
s._race_cards[race_i].pressed.emit()
|
||||||
|
|||||||
@@ -141,10 +141,35 @@ func test_an_unmapped_error_still_speaks_in_voice():
|
|||||||
assert_false(CreationCopy.UNSPOKEN.contains("_"))
|
assert_false(CreationCopy.UNSPOKEN.contains("_"))
|
||||||
|
|
||||||
|
|
||||||
|
func _is_deliberately_unmapped(raw: String) -> bool:
|
||||||
|
# The shapes error_line has NO authored line for, on purpose, because the
|
||||||
|
# screen structurally cannot produce them (CreationDraft always emits an
|
||||||
|
# int seed and a spend built only from increment/decrement, which can
|
||||||
|
# never exceed the pool or name a non-attribute) — a malformed seed or
|
||||||
|
# spend, or a broken content reference, is not a player mistake to voice
|
||||||
|
# in character; test_an_unmapped_error_still_speaks_in_voice already pins
|
||||||
|
# the fallback for exactly this set. Named by the validator's own prefixes
|
||||||
|
# so this stays independent of error_line's internals.
|
||||||
|
return (raw.begins_with("unresolved ref:")
|
||||||
|
or raw.begins_with("seed is required")
|
||||||
|
or raw.begins_with("spend")
|
||||||
|
or raw.begins_with("cannot spend on")
|
||||||
|
or raw == "skills must be an array")
|
||||||
|
|
||||||
|
|
||||||
func test_no_error_the_validator_can_raise_reaches_the_player_raw():
|
func test_no_error_the_validator_can_raise_reaches_the_player_raw():
|
||||||
# Sweep EVERY error NewGame.validate actually produces for a draft the screen can
|
# Sweep EVERY error NewGame.validate actually produces for a draft the screen can
|
||||||
# be driven into (plus the hostile shapes only a saga could hand it), and assert
|
# be driven into (plus the hostile shapes only a saga could hand it), and assert
|
||||||
# the player never sees the id, the underscore, or the validator's grammar.
|
# the player never sees the id, the underscore, or the validator's grammar — AND,
|
||||||
|
# for every shape error_line actually has an authored line for, that the line
|
||||||
|
# fired rather than silently falling through to the generic UNSPOKEN fallback.
|
||||||
|
#
|
||||||
|
# CreationCopy.UNSPOKEN ("something in this does not hold — look again") is
|
||||||
|
# itself non-blank, isn't the raw string, has no underscore, and has no "got " —
|
||||||
|
# so the four checks below cannot tell "correctly mapped to its authored line"
|
||||||
|
# from "silently fell through." Without the UNSPOKEN check, renaming a validator
|
||||||
|
# string in new_game.gd so an error_line branch stops matching would leave every
|
||||||
|
# error line reading the generic fallback, suite green.
|
||||||
var d := _draft()
|
var d := _draft()
|
||||||
var hostiles: Array = [
|
var hostiles: Array = [
|
||||||
{"name": "", "skills": [], "bonus_skill": ""}, # nameless, unpicked
|
{"name": "", "skills": [], "bonus_skill": ""}, # nameless, unpicked
|
||||||
@@ -165,6 +190,7 @@ func test_no_error_the_validator_can_raise_reaches_the_player_raw():
|
|||||||
}
|
}
|
||||||
|
|
||||||
var seen := 0
|
var seen := 0
|
||||||
|
var mapped_seen := 0
|
||||||
for h in hostiles:
|
for h in hostiles:
|
||||||
var creation := base.duplicate(true)
|
var creation := base.duplicate(true)
|
||||||
for k in h:
|
for k in h:
|
||||||
@@ -173,9 +199,16 @@ func test_no_error_the_validator_can_raise_reaches_the_player_raw():
|
|||||||
assert_gt(errors.size(), 0, "hostile creation %s must actually be invalid, or it guards nothing" % str(h))
|
assert_gt(errors.size(), 0, "hostile creation %s must actually be invalid, or it guards nothing" % str(h))
|
||||||
for e in errors:
|
for e in errors:
|
||||||
seen += 1
|
seen += 1
|
||||||
var line: String = CreationCopy.error_line(str(e), d)
|
var raw := str(e)
|
||||||
assert_ne(line.strip_edges(), "", "'%s' put a BLANK line on the label" % e)
|
var line: String = CreationCopy.error_line(raw, d)
|
||||||
assert_ne(line, str(e), "'%s' reached the player raw" % e)
|
assert_ne(line.strip_edges(), "", "'%s' put a BLANK line on the label" % raw)
|
||||||
assert_false(line.contains("_"), "'%s' put a snake_case id on the label: %s" % [e, line])
|
assert_ne(line, raw, "'%s' reached the player raw" % raw)
|
||||||
assert_false(line.contains("got "), "'%s' put the validator's grammar on the label: %s" % [e, line])
|
assert_false(line.contains("_"), "'%s' put a snake_case id on the label: %s" % [raw, line])
|
||||||
|
assert_false(line.contains("got "), "'%s' put the validator's grammar on the label: %s" % [raw, line])
|
||||||
|
|
||||||
|
if not _is_deliberately_unmapped(raw):
|
||||||
|
mapped_seen += 1
|
||||||
|
assert_ne(line, CreationCopy.UNSPOKEN,
|
||||||
|
"'%s' has an authored error_line branch but silently fell through to the generic fallback" % raw)
|
||||||
assert_gt(seen, 12, "the sweep must actually reach a spread of errors, not one or two")
|
assert_gt(seen, 12, "the sweep must actually reach a spread of errors, not one or two")
|
||||||
|
assert_gt(mapped_seen, 8, "the sweep must actually exercise error_line's mapped branches, not just the deliberately-unmapped ones")
|
||||||
|
|||||||
@@ -363,8 +363,17 @@ func test_construct_ignores_an_attribute_block_handed_to_it():
|
|||||||
"construct re-rolls from the SEED — a stat block handed to it is not state, it is noise (§2)")
|
"construct re-rolls from the SEED — a stat block handed to it is not state, it is noise (§2)")
|
||||||
|
|
||||||
# And the hostile block is not merely ignored on the sheet — it never becomes state
|
# And the hostile block is not merely ignored on the sheet — it never becomes state
|
||||||
# anywhere. (A `sheet.attributes` the caller supplied would be the exact §2 failure.)
|
# anywhere else either. `res["log"].to_dict().has("attributes")` (the old check here)
|
||||||
assert_false(res["log"].to_dict().has("attributes"), "no attribute ever reaches the canon log")
|
# was already false before this test's action ran: LogPlayer.to_dict() has no such
|
||||||
|
# TOP-LEVEL key on any code path, hostile block or not, so it could never fail — delete
|
||||||
|
# construct's whole §2 guard and this stayed green. Assert the player row's exact key
|
||||||
|
# SET instead (sorted — insertion order in LogPlayer.to_dict() isn't the contract):
|
||||||
|
# it feeds the AI (§2/§7), so a stray "attributes", "str", or anything else reaching it
|
||||||
|
# is a real leak, and this is what actually fails if one does.
|
||||||
|
var player_keys: Array = res["log"].to_dict()["player"].keys()
|
||||||
|
player_keys.sort()
|
||||||
|
assert_eq(player_keys, ["calling_id", "luck_descriptor", "name", "race_id"],
|
||||||
|
"the canon-log player row carries a key beyond name/race_id/calling_id/luck_descriptor — numeric state reached the AI-facing log (§2/§7)")
|
||||||
|
|
||||||
|
|
||||||
func test_validate_is_public_and_agrees_with_construct():
|
func test_validate_is_public_and_agrees_with_construct():
|
||||||
|
|||||||
@@ -30,14 +30,19 @@ func test_default_font_is_set():
|
|||||||
|
|
||||||
|
|
||||||
func _guarded_variations() -> Array:
|
func _guarded_variations() -> Array:
|
||||||
# EVERY variation the builder touches — the stylebox variations (ThemeKeys.ALL)
|
# EVERY variation OR BASE TYPE the builder touches — the stylebox variations
|
||||||
# AND the six font roles (ThemeKeys.FONT_ROLES), which ALL deliberately excludes
|
# (ThemeKeys.ALL), the six font roles (ThemeKeys.FONT_ROLES), AND the base
|
||||||
# by its own docstring. Iterating ALL alone left every font, font size and font
|
# types the builder styles directly with no variation on top
|
||||||
# colour in the theme unguarded: change Palette.CREAM (TitleLogo's font colour,
|
# (ThemeKeys.BASE_TYPES — today just RichTextLabel). ALL and FONT_ROLES
|
||||||
# and nothing else's), skip the rebuild, and the suite stayed green with a stale
|
# deliberately exclude each other's half by their own docstrings; missing
|
||||||
# .tres shipped. traps.md #12 — "covers everything" must iterate the full set.
|
# BASE_TYPES here left RichTextLabel's two fonts, its font size and its
|
||||||
|
# font colour — the ONLY things rendering DM prose on the creation screen —
|
||||||
|
# unguarded against a stale .tres. traps.md #12, the third miss: iterating
|
||||||
|
# ALL alone, then ALL+FONT_ROLES, both still left a gap "covers everything"
|
||||||
|
# did not actually cover.
|
||||||
var out: Array = ThemeKeys.ALL.keys()
|
var out: Array = ThemeKeys.ALL.keys()
|
||||||
out.append_array(ThemeKeys.FONT_ROLES.keys())
|
out.append_array(ThemeKeys.FONT_ROLES.keys())
|
||||||
|
out.append_array(ThemeKeys.BASE_TYPES.keys())
|
||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
@@ -57,7 +62,22 @@ func test_the_drift_guard_covers_every_variation_the_builder_touches():
|
|||||||
if not touched:
|
if not touched:
|
||||||
continue
|
continue
|
||||||
if fresh.get_type_variation_base(variation) == &"":
|
if fresh.get_type_variation_base(variation) == &"":
|
||||||
continue # a BASE type (the builder styles RichTextLabel directly), not a variation
|
# "" means one of two things, and they must be told apart, not both
|
||||||
|
# waved through: (a) a KNOWN base type the builder styles directly
|
||||||
|
# with no set_type_variation (RichTextLabel today — registered in
|
||||||
|
# ThemeKeys.BASE_TYPES), or (b) a type nobody ever registered at
|
||||||
|
# all — styled with set_stylebox/set_font/set_color but never
|
||||||
|
# given a set_type_variation, which reports the same "" and used
|
||||||
|
# to slip through this `continue` forever. The old guard could not
|
||||||
|
# tell these apart because it treated "" as "definitely (a)" and
|
||||||
|
# skipped it unconditionally — the exact whitelist that let
|
||||||
|
# RichTextLabel through in the first place (traps.md #12, third
|
||||||
|
# miss). Assert (a); anything else IS (b) and must be named.
|
||||||
|
assert_true(variation in ThemeKeys.BASE_TYPES,
|
||||||
|
("the builder styles %s directly (no set_type_variation) and it is not in " +
|
||||||
|
"ThemeKeys.BASE_TYPES — add it there, or if it should be a real variation, " +
|
||||||
|
"give it a set_type_variation(...) base so ALL/FONT_ROLES can guard it") % variation)
|
||||||
|
continue
|
||||||
assert_true(variation in guarded,
|
assert_true(variation in guarded,
|
||||||
"the builder styles the variation %s but no drift guard walks it — add it to ThemeKeys.ALL or ThemeKeys.FONT_ROLES" % variation)
|
"the builder styles the variation %s but no drift guard walks it — add it to ThemeKeys.ALL or ThemeKeys.FONT_ROLES" % variation)
|
||||||
|
|
||||||
@@ -75,6 +95,18 @@ func test_committed_tres_matches_builder():
|
|||||||
var fresh: Theme = Builder.build_theme()
|
var fresh: Theme = Builder.build_theme()
|
||||||
var committed: Theme = load(THEME_PATH)
|
var committed: Theme = load(THEME_PATH)
|
||||||
|
|
||||||
|
# Theme-LEVEL defaults (build_game_theme.gd:69-70) live on the Theme object
|
||||||
|
# itself, not under any type or variation, so no per-variation loop below
|
||||||
|
# ever reaches them — they need their own comparison or a changed
|
||||||
|
# default_font_size (18 -> anything) ships silently. Font by resource_path,
|
||||||
|
# not the Resource: load() caches, so two references to the same .ttf are
|
||||||
|
# `==` even when the builder swapped fonts, unless the .tres happens to
|
||||||
|
# still point at the same file.
|
||||||
|
assert_eq(committed.default_font.resource_path, fresh.default_font.resource_path,
|
||||||
|
"stale game_theme.tres — theme.default_font drifted from the builder; re-run build_game_theme.gd")
|
||||||
|
assert_eq(committed.default_font_size, fresh.default_font_size,
|
||||||
|
"stale game_theme.tres — theme.default_font_size drifted from the builder; re-run build_game_theme.gd")
|
||||||
|
|
||||||
for variation in _guarded_variations():
|
for variation in _guarded_variations():
|
||||||
for state in fresh.get_stylebox_list(variation):
|
for state in fresh.get_stylebox_list(variation):
|
||||||
assert_true(committed.has_stylebox(state, variation),
|
assert_true(committed.has_stylebox(state, variation),
|
||||||
|
|||||||
Reference in New Issue
Block a user