diff --git a/client/tests/golden/visual/atlas_GJ144e_Global.png b/client/tests/golden/visual/atlas_GJ144e_Global.png index c6b4acc89..83be3de7c 100644 Binary files a/client/tests/golden/visual/atlas_GJ144e_Global.png and b/client/tests/golden/visual/atlas_GJ144e_Global.png differ diff --git a/client/tests/golden/visual/atlas_GJ144e_Quarter.png b/client/tests/golden/visual/atlas_GJ144e_Quarter.png index 7f423925b..f13aa1eea 100644 Binary files a/client/tests/golden/visual/atlas_GJ144e_Quarter.png and b/client/tests/golden/visual/atlas_GJ144e_Quarter.png differ diff --git a/client/tests/golden/visual/atlas_GJ244Ad_Global.png b/client/tests/golden/visual/atlas_GJ244Ad_Global.png index 091555a74..5b53c209a 100644 Binary files a/client/tests/golden/visual/atlas_GJ244Ad_Global.png and b/client/tests/golden/visual/atlas_GJ244Ad_Global.png differ diff --git a/client/tests/golden/visual/atlas_GJ251c_Global.png b/client/tests/golden/visual/atlas_GJ251c_Global.png index 79e0b473d..0f35f40c3 100644 Binary files a/client/tests/golden/visual/atlas_GJ251c_Global.png and b/client/tests/golden/visual/atlas_GJ251c_Global.png differ diff --git a/client/tests/golden/visual/atlas_GJ251c_Region.png b/client/tests/golden/visual/atlas_GJ251c_Region.png index 2a7b8206a..a1eb0157f 100644 Binary files a/client/tests/golden/visual/atlas_GJ251c_Region.png and b/client/tests/golden/visual/atlas_GJ251c_Region.png differ diff --git a/client/tests/golden/visual/atlas_GJ338Bd_Block.png b/client/tests/golden/visual/atlas_GJ338Bd_Block.png index 0304ad77e..7f7e1f169 100644 Binary files a/client/tests/golden/visual/atlas_GJ338Bd_Block.png and b/client/tests/golden/visual/atlas_GJ338Bd_Block.png differ diff --git a/client/tests/golden/visual/atlas_GJ338Bd_Global.png b/client/tests/golden/visual/atlas_GJ338Bd_Global.png index a90ec90d8..5fcee8e33 100644 Binary files a/client/tests/golden/visual/atlas_GJ338Bd_Global.png and b/client/tests/golden/visual/atlas_GJ338Bd_Global.png differ diff --git a/client/tests/golden/visual/atlas_GJ380c_District.png b/client/tests/golden/visual/atlas_GJ380c_District.png index 8adee583f..bc7c7688e 100644 Binary files a/client/tests/golden/visual/atlas_GJ380c_District.png and b/client/tests/golden/visual/atlas_GJ380c_District.png differ diff --git a/client/tests/golden/visual/atlas_GJ380c_Global.png b/client/tests/golden/visual/atlas_GJ380c_Global.png index 422a6e699..a514d8fe3 100644 Binary files a/client/tests/golden/visual/atlas_GJ380c_Global.png and b/client/tests/golden/visual/atlas_GJ380c_Global.png differ diff --git a/client/tests/golden/visual/atlas_GJ380c_Region.png b/client/tests/golden/visual/atlas_GJ380c_Region.png index 93ffa363d..986e86fe7 100644 Binary files a/client/tests/golden/visual/atlas_GJ380c_Region.png and b/client/tests/golden/visual/atlas_GJ380c_Region.png differ diff --git a/client/tests/golden/visual/atlas_GJ445c-m1_Chunk.png b/client/tests/golden/visual/atlas_GJ445c-m1_Chunk.png index eb9f13c88..11c372be2 100644 Binary files a/client/tests/golden/visual/atlas_GJ445c-m1_Chunk.png and b/client/tests/golden/visual/atlas_GJ445c-m1_Chunk.png differ diff --git a/client/tests/golden/visual/atlas_GJ820Bc_District.png b/client/tests/golden/visual/atlas_GJ820Bc_District.png index bf9fd0dd4..ed47e134d 100644 Binary files a/client/tests/golden/visual/atlas_GJ820Bc_District.png and b/client/tests/golden/visual/atlas_GJ820Bc_District.png differ diff --git a/client/tests/golden/visual/atlas_GJ820Bc_Global.png b/client/tests/golden/visual/atlas_GJ820Bc_Global.png index 076897efb..608b89f0a 100644 Binary files a/client/tests/golden/visual/atlas_GJ820Bc_Global.png and b/client/tests/golden/visual/atlas_GJ820Bc_Global.png differ diff --git a/client/tests/test_step_canvas_legend.gd b/client/tests/test_step_canvas_legend.gd index fb182efa9..1f9e8e4c5 100644 --- a/client/tests/test_step_canvas_legend.gd +++ b/client/tests/test_step_canvas_legend.gd @@ -73,13 +73,30 @@ func test_refresh_reflects_the_active_overlay_toggle() -> void: ).is_greater(rows_before) -func test_reposition_sets_a_fixed_panel_margin_position() -> void: +## T-1197 PR #217 review (both reviewers, pixel-proven): this used to assert +## a hardcoded Vector2(PANEL_MARGIN, 60.0) — the same "60.0" constant whose +## drift out of sync with the header panel's REAL grown footprint (once +## step_canvas_viewer.gd wrapped the header in its own ImplantPanel) caused +## the two panels to fuse into one double-height box. The fix removed that +## hardcoded Y from production code (step_canvas_legend.gd's reposition() now +## derives it from StepCanvasViewer.get_header_panel_bottom_y() + +## get_header_legend_gap_px()) — this test must assert the SAME derived +## relationship, not a second copy of the old magic number, or it would go on +## silently enforcing the exact drift-prone shape production code just +## stopped doing (test_step_canvas_viewer.gd's +## test_legend_panel_never_overlaps_the_header_panel_vertically/ +## test_legend_panel_leaves_a_real_gap_below_the_header_panel are the fuller +## non-overlap/gap proof against the REAL viewer-owned legend; this one stays +## a narrow smoke check on a standalone legend instance, matching this file's +## own "smoke coverage" scope note at the top). +func test_reposition_derives_y_from_the_headers_measured_bottom_plus_gap() -> void: var v: StepCanvasViewer = _make_viewer() add_child(v) var legend = LegendScript.new(v) auto_free(legend) legend.reposition() - assert_that(legend.position).is_equal(Vector2(LegendScript.PANEL_MARGIN, 60.0)) + var expected_y: float = v.get_header_panel_bottom_y() + v.get_header_legend_gap_px() + assert_that(legend.position).is_equal(Vector2(LegendScript.PANEL_MARGIN, expected_y)) ## T-1192 review fix: RESERVED_COLUMN_PX is now a direct read of diff --git a/client/tests/test_step_canvas_viewer.gd b/client/tests/test_step_canvas_viewer.gd index a81a1cfea..afa0bac1c 100644 --- a/client/tests/test_step_canvas_viewer.gd +++ b/client/tests/test_step_canvas_viewer.gd @@ -837,3 +837,81 @@ func test_get_current_canvas_summary_counts_match_a_fixture_canvas() -> void: var by_class: Dictionary = summary.get("course_count_by_class", {}) assert_int(int(by_class.get(0, 0))).is_equal(2) assert_int(int(by_class.get(2, 0))).is_equal(1) + + +# ============================================================================= +# T-1197 PR #217 review (Hoshe): header-panel-vs-legend-panel vertical +# non-overlap — the exact regression this review round caught. Mirrors the +# T-1192 precedent above (test_global_canvas_left_edge_never_overlaps_the_legend_column, +# line ~674): a geometric non-overlap invariant against the REAL viewer +# wiring, not a hand-computed expected pixel value that could silently drift +# out of sync with the production layout the same way the old hardcoded +# Vector2(PANEL_MARGIN, 60.0) drifted out of sync with the header's real +# grown footprint. +# ============================================================================= + + +## The screen header panel and the legend panel must never vertically +## overlap: the legend's TOP edge (position.y) must be at or below the +## header's BOTTOM edge (position.y + size.y). Before the T-1197 PR #217 fix, +## step_canvas_legend.gd's reposition() hardcoded Y=60.0 — a constant tuned +## for the OLD bare-ImplantHeader footprint — so once the header grew its own +## ImplantPanel wrapper (border + content margins), the legend's fixed Y sat +## INSIDE the header panel's new, taller footprint: the two fused into one +## unbroken double-height box with zero terrain gap between them (pixel- +## proven independently by both PR #217 reviewers). This test pins the +## invariant directly against the real _screen_header_panel/_legend_panel +## Controls the production layout builds, not a copy of the geometry math. +func test_legend_panel_never_overlaps_the_header_panel_vertically() -> void: + var v: StepCanvasViewer = _make_viewer() + add_child(v) + v.enter({"body_id": "GJ380c", "body_radius_km": 6238.4}, {}) + + # Both panels are manually positioned (implant_panel.gd's own doc: "not + # itself inside a parent Container, so nothing else forces a re-measure"), + # and reset_to_content_size()/reposition() are deferred — award one idle + # frame so the REAL settled sizes are in place before asserting, exactly + # like the deferred-resize idiom both panels already rely on in production + # (see _build_screen_header()'s own call to reset_to_content_size(), and + # _ready()'s own deferred reposition() call added alongside this test). + await get_tree().process_frame + await get_tree().process_frame + + var header_top: float = v._screen_header_panel.position.y + var header_bottom: float = header_top + v._screen_header_panel.size.y + var legend_top: float = v._legend_panel.position.y + + assert_float(legend_top).override_failure_message( + ( + "the legend panel's top edge (y=%.1f) must be AT OR BELOW the header" + + " panel's measured bottom edge (y=%.1f) — a smaller value means the" + + " two panels overlap/fuse into one box, the exact PR #217 regression" + ) + % [legend_top, header_bottom] + ).is_greater_equal(header_bottom - 0.01) + + +## The gap must be a REAL, visible gap — not just "touching at exactly the +## same pixel" (which would still satisfy >= but reads as fused on screen). +## Pins the fixed HEADER_LEGEND_GAP_PX constant is actually being applied, +## not merely that overlap happens to be avoided by coincidence of content +## size on this particular test body. +func test_legend_panel_leaves_a_real_gap_below_the_header_panel() -> void: + var v: StepCanvasViewer = _make_viewer() + add_child(v) + v.enter({"body_id": "GJ380c", "body_radius_km": 6238.4}, {}) + await get_tree().process_frame + await get_tree().process_frame + + var header_bottom: float = v._screen_header_panel.position.y + v._screen_header_panel.size.y + var legend_top: float = v._legend_panel.position.y + var gap: float = legend_top - header_bottom + + assert_float(gap).override_failure_message( + ( + "expected a visible gap of at least %.1fpx between the header panel's" + + " bottom (y=%.1f) and the legend panel's top (y=%.1f), got %.1fpx —" + + " panels that merely touch still read as one fused box on screen" + ) + % [v.get_header_legend_gap_px(), header_bottom, legend_top, gap] + ).is_greater_equal(v.get_header_legend_gap_px() - 0.01) diff --git a/client/ui/implant/apps/atlas/step_canvas/step_canvas_legend.gd b/client/ui/implant/apps/atlas/step_canvas/step_canvas_legend.gd index 659fa12c0..5b6bd109c 100644 --- a/client/ui/implant/apps/atlas/step_canvas/step_canvas_legend.gd +++ b/client/ui/implant/apps/atlas/step_canvas/step_canvas_legend.gd @@ -52,8 +52,21 @@ func _init(viewer_ref = null) -> void: visible = false +## T-1197 PR #217 review (both reviewers, pixel-proven): this used to hardcode +## Vector2(PANEL_MARGIN, 60.0) — a constant tuned for the bare-ImplantHeader +## era, before T-1197 wrapped the header in its own ImplantPanel (border + +## content margins grew its real footprint). The two panels fused into one +## unbroken double-height box with zero terrain gap. Now DERIVED from the +## header panel's own MEASURED height (StepCanvasViewer.get_header_panel_bottom_y(), +## which reads `_screen_header_panel.size.y` after that panel's own +## reset_to_content_size() has settled) plus a fixed gap constant — never a +## second hardcoded Y that can drift out of sync with the header again. func reposition() -> void: - position = Vector2(PANEL_MARGIN, 60.0) + if _viewer == null: + position = Vector2(PANEL_MARGIN, 60.0) # pre-viewer fallback, unreachable in practice (_init always takes a viewer) + return + var header_bottom_y: float = _viewer.get_header_panel_bottom_y() + position = Vector2(PANEL_MARGIN, header_bottom_y + _viewer.get_header_legend_gap_px()) func refresh() -> void: diff --git a/client/ui/implant/apps/atlas/step_canvas/step_canvas_viewer.gd b/client/ui/implant/apps/atlas/step_canvas/step_canvas_viewer.gd index e5801611e..53afdd96e 100644 --- a/client/ui/implant/apps/atlas/step_canvas/step_canvas_viewer.gd +++ b/client/ui/implant/apps/atlas/step_canvas/step_canvas_viewer.gd @@ -145,6 +145,7 @@ var _canvas: Node2D = null var _terrain_layer: StepCanvasTerrainLayer = null var _annotation_layer: StepCanvasAnnotationLayer = null var _screen_header: ImplantHeader = null +var _screen_header_panel = null # ImplantPanel — T-1197 backing scrim (untyped: no class_name cycle needed) var _overlay_bar = null var _legend_panel = null var _request = null # StepCanvasRequest @@ -193,6 +194,19 @@ func _ready() -> void: _build_overlay_bar() _build_legend_panel() + # T-1197 PR #217 review: the legend's FIRST reposition() (called + # synchronously above, inside _build_legend_panel()'s refresh()) reads + # _screen_header_panel.size.y before that panel's own deferred + # reset_to_content_size() has fired (call_deferred runs at end-of-frame, + # per implant_panel.gd's own doc) — so it can still see a pre-layout + # height on the very first frame. Deferring ONE more reposition() call + # after this _ready() returns guarantees it re-reads the header panel's + # real settled height at least once, the same "defer past the transient + # wrong value" idiom implant_panel.gd already uses for exactly this class + # of problem. + if _legend_panel: + _legend_panel.reposition.call_deferred() + _disk_sweep_timer = Timer.new() _disk_sweep_timer.name = "DiskSweepTimer" _disk_sweep_timer.wait_time = DISK_SWEEP_INTERVAL_SEC @@ -691,14 +705,64 @@ func _apply_transform() -> void: # ============================================================================= +## T-1197: the header used to be a bare ImplantHeader added straight to this +## Control, drawn directly over the terrain canvas with no backing — every +## OTHER implant chrome element (the legend below it) gets its scrim from +## ImplantPanel's `panel_bg` StyleBoxFlat, but a lone ImplantHeader has no +## panel of its own (see implant_header.gd — just two Labels). Its fixed +## opaque text color reads fine against the near-black viewer background or +## dark ocean terrain (the common case at rung 0's letterboxed corner and most +## fixed-rung captures), but against pale/light terrain — Quarter's white +## upland scatter, District's olive/tan dry terrain — the contrast collapses +## to the point of unreadability (Araminta's T-1196 observation, +## scratchpad t1196/gj1002b_quarter.png; reproduced baked into the committed +## golden atlas_GJ820Bc_District.png). This is a static contrast defect, not a +## timing race — there is no fade/tween anywhere in this cluster (confirmed by +## reading every script under client/ui/implant), so no harness settle change +## could fix it. Wrapping in an ImplantPanel (same component every other +## header-bearing screen in the implant UI already uses — index_screen.gd, +## planet_screen.gd, system_screen.gd, reach_screen.gd, kind_menu_screen.gd, +## detail_screen.gd, overview_screen.gd, character_creation.gd, and the +## legend right below THIS header — step_canvas_viewer.gd's bare add_child() +## was the only outlier) gives the header the same opaque scrim as every +## sibling chrome element, guaranteeing contrast regardless of what terrain +## renders underneath. +## T-1197 PR #217 review (both reviewers, pixel-proven): the ImplantPanel wrap +## below grew the header's on-screen footprint (border + content_margin_* from +## the theme's panel stylebox) beyond the bare ImplantHeader's old, smaller +## size — but step_canvas_legend.gd's own reposition() still hardcoded its Y +## at PANEL_MARGIN + 60.0, a constant tuned for the OLD bare-header height. +## Result: the two panels fused into one unbroken double-height box, zero +## terrain gap, in every capture and all 13 goldens. Fixed via +## HEADER_PANEL_TOP_Y/HEADER_LEGEND_GAP_PX below + get_header_panel_bottom_y() +## — the legend now DERIVES its Y from the header panel's own MEASURED height +## (never a second hardcoded constant that would drift again the next time +## either panel's content changes shape). +const HEADER_PANEL_TOP_Y: float = 16.0 +const HEADER_LEGEND_GAP_PX: float = 12.0 + + func _build_screen_header() -> void: + var PanelScript := load("res://ui/implant/implant_panel.gd") + _screen_header_panel = PanelScript.new() + _screen_header_panel.name = "ScreenHeaderPanel" + _screen_header_panel.position = Vector2(PANEL_MARGIN, HEADER_PANEL_TOP_Y) + _screen_header_panel.mouse_filter = Control.MOUSE_FILTER_IGNORE + _screen_header_panel.custom_minimum_size.x = 320.0 + _screen_header_panel.theme_resource = _implant_theme # set BEFORE add_child(), matching _build_legend_panel()'s own ordering + add_child(_screen_header_panel) + _screen_header = ImplantHeader.new() - _screen_header.position = Vector2(PANEL_MARGIN, 16.0) _screen_header.custom_minimum_size.x = 320.0 _screen_header.mouse_filter = Control.MOUSE_FILTER_IGNORE - add_child(_screen_header) - if _implant_theme: - _screen_header.apply_implant_theme(_implant_theme) + _screen_header_panel.add_component(_screen_header) + + # Same "manually positioned, not inside a parent Container, so nothing else + # forces a re-measure" situation implant_panel.gd's own + # reset_to_content_size() doc describes for the legend — this panel needs + # the identical deferred re-measure so get_header_panel_bottom_y() reads a + # settled `size.y`, not a transient pre-layout value. + _screen_header_panel.reset_to_content_size() func _refresh_screen_header() -> void: @@ -709,6 +773,35 @@ func _refresh_screen_header() -> void: var title := "ATLAS — %s" % name_label.to_upper() var subtitle := "%s · %.3f km/gridunit" % [_held_rung.to_upper(), spacing_km] _screen_header.set_content(title, subtitle) + # Title/subtitle text length never changes the panel's HEIGHT (autowrap is + # word-smart but both lines are short, fixed-shape content — only the + # legend's row COUNT genuinely varies), but re-measuring here is free + # insurance against exactly the class of drift this review round caught, + # and keeps the legend's derived Y correct even if header content ever + # grows a line. + _screen_header_panel.reset_to_content_size() + if _legend_panel: + _legend_panel.reposition() + + +## T-1197 PR #217 review: the ONE place step_canvas_legend.gd's reposition() +## reads the header panel's footprint from — never a second hardcoded +## constant. Falls back to HEADER_PANEL_TOP_Y-only (as if the header panel had +## zero height) before the header panel exists, mirroring +## _centered_view_offset()'s own "nothing to measure yet" pre-arrival pattern. +func get_header_panel_bottom_y() -> float: + if _screen_header_panel == null: + return HEADER_PANEL_TOP_Y + return _screen_header_panel.position.y + _screen_header_panel.size.y + + +## Plain-method accessor for HEADER_LEGEND_GAP_PX (not a bare const read) — +## step_canvas_legend.gd holds `_viewer` UNTYPED (its own doc: "avoid cyclic +## ref"), and this cluster's convention is a method call across that boundary, +## matching get_header_panel_bottom_y()/get_held_rung()/is_overlay_visible() +## rather than relying on GDScript's duck-typed const-through-Variant access. +func get_header_legend_gap_px() -> float: + return HEADER_LEGEND_GAP_PX func _build_overlay_bar() -> void: