fix(client): cold-start black screen — self-healing tile repaint, legend re-entry root cause, pending-tile wash
Jeroen hit a black mosaic zooming into a body from a fresh make-atlas spawn. Live diagnosis showed all six tiles held with colored textures and no repaint; the original line-specific diagnosis (unpaired queue_redraw in _on_tile_ready) turned out WRONG — the pairing already existed (lead's truncated grep misread the function; Stig verified via git log -p before acting). Rather than chase the exact dropped signal edge, _process() now self-heals: both viewer and overlay redraw every frame while the tile set has pending tiles (has_pending_tiles(), new) — a strict superset that closes the black regardless of which edge drops, pinned by a _draw()-counting real-subclass spy test. Legend stacking root cause found by trace, not guess: ImplantApp. _on_screen_changed() re-runs enter() unconditionally on repeat same-screen pushes — each re-entry tore down and rebuilt all six tile requests (the round-6 orphaning fingerprint via a new trigger) and stacked another legend (~10 deep, full-height dark panel). Fixed both ends: ImplantPanel.clear() frees immediately (same-frame re-entrant refresh can never observe stale children — protects every implant app), and RegionalScreen.enter() no-ops for the same body (different body still re-enters fresh). The load-bearing regression asserts an ARRIVED TILE'S DATA survives a repeat push — node identity would not catch the teardown (the tile-set Node is a fixed field; only its internals reset). Cold-start UX: pending tiles now draw the single-window path's COLOR_BORDER_FADE wash instead of raw background — a deriving mosaic reads as loading, not broken. Suites green (zoom_ladder 48, viewer 74, tile_set 20, overlay 30, overlays 46 + 2 new files), gdlint clean, revert-verified throughout.
This commit is contained in:
@@ -0,0 +1,161 @@
|
||||
## PR #192 cold-start dossier — coordinator's live repro against a freshly-
|
||||
## spawned (cold) server (`make atlas` shape, first AnalyzeBody taking
|
||||
## seconds): BUG 1 (tile-mosaic paint never resolving) and the legend-
|
||||
## stacking half of BUG 2. Split out of test_atlas_zoom_ladder.gd purely for
|
||||
## file-length reasons (gdlint max-file-lines) — same instantiation/mock-
|
||||
## response conventions as that file, not a different testing philosophy.
|
||||
## RegionalScreen's own re-entry-guard half of BUG 2 is covered separately
|
||||
## in test_regional_screen.gd (a different layer — nav, not the viewer).
|
||||
class_name TestAtlasColdStart
|
||||
extends GdUnitTestSuite
|
||||
|
||||
const AtlasWindowRequest := preload("res://ui/implant/apps/atlas/atlas_window_request.gd")
|
||||
|
||||
## Dudley's WINDOW_GRANULARITY_REGION_KEY sentinel — mirrors
|
||||
## test_atlas_zoom_ladder.gd's own constant (see that file's doc for why the
|
||||
## real wire value matters, not a convenient placeholder).
|
||||
const SERVER_LEGACY_GRANULARITY_REGION_SENTINEL: int = 4294967295
|
||||
|
||||
|
||||
## Build a hand-authored DistrictWindowLayer dict (n=2 by default) — mirrors
|
||||
## test_atlas_zoom_ladder.gd's own _mock_window().
|
||||
static func _mock_window(center: Vector2i, n: int = 2) -> Dictionary:
|
||||
return {
|
||||
"center": [center.x, center.y],
|
||||
"n": n,
|
||||
"morphology": PackedByteArray([8, 14, 0, 1]),
|
||||
"elev_q": PackedByteArray([40, 90, 5, 60]),
|
||||
"temp_dc": [120, 95, -32768, 60],
|
||||
"moisture_q": PackedByteArray([50, 30, 90, 20]),
|
||||
"vegetation": PackedByteArray([2, 1, 6, 3]),
|
||||
"glaciation": PackedByteArray([0, 0, 1, 2]),
|
||||
}
|
||||
|
||||
|
||||
static func _mock_response(body_id: String, window: Variant) -> Dictionary:
|
||||
return {"body_id": body_id, "status": "Ready", "district_window": window}
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# BUG 1 — tile-mosaic paint self-heal.
|
||||
# =============================================================================
|
||||
|
||||
|
||||
## Counts real _draw() invocations — CanvasItem exposes no public
|
||||
## "is a redraw pending" query in this Godot version, so the only reliable
|
||||
## signal that queue_redraw() actually had an effect is the engine calling
|
||||
## _draw() again on a subsequent frame. Subclasses the REAL AtlasWindowOverlay
|
||||
## (not a duck-typed stub) so drawing still runs through the genuine
|
||||
## production code path — this spy only adds counting, nothing else.
|
||||
class _CountingOverlay extends AtlasWindowOverlay:
|
||||
var draw_count := 0
|
||||
|
||||
func _draw() -> void:
|
||||
draw_count += 1
|
||||
super._draw()
|
||||
|
||||
|
||||
## On a cold server, a tile's window_ready can land well after entry's own
|
||||
## paint window, and a live repro showed the mosaic staying black even with
|
||||
## every tile held/textured — only resolving on an unrelated gesture.
|
||||
## _process() must therefore queue a redraw on BOTH the viewer and the
|
||||
## overlay every frame while any tile is still pending, regardless of
|
||||
## whether the tile-arrival signal path painted correctly on its own. Proven
|
||||
## here by swapping the REAL overlay for a _draw()-counting subclass right
|
||||
## after entry (once the entry-time redraw has already resolved via a real
|
||||
## frame), then calling _process() directly with NO input/gesture and
|
||||
## confirming a further frame actually invokes _draw() again — revert-
|
||||
## verified against a version of _process() with the self-heal removed
|
||||
## (fails without it, since nothing else re-queues while idle).
|
||||
func test_process_self_heals_the_overlay_redraw_while_tiles_are_pending() -> void:
|
||||
var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new())
|
||||
add_child(v)
|
||||
var radius_km := 6238.4 # GJ380c (Lendel) — needs tiling
|
||||
v.enter_orbital({"body_id": "GJ380c", "body_radius_km": radius_km}, {})
|
||||
assert_bool(v.is_tile_mode()).is_true()
|
||||
assert_bool(v.get_tile_set().has_pending_tiles()).override_failure_message(
|
||||
"sanity: entry must leave every tile pending before any response arrives"
|
||||
).is_true()
|
||||
|
||||
# Swap in the counting spy AFTER entry (so entry's own queue_redraw()
|
||||
# calls don't pollute the baseline) but the OLD overlay is freed and the
|
||||
# spy re-added under the same _canvas parent, matching _ready()'s own
|
||||
# construction shape exactly.
|
||||
var spy := _CountingOverlay.new()
|
||||
spy.viewer = v
|
||||
v._overlay_node.queue_free()
|
||||
v._overlay_node = spy
|
||||
v._canvas.add_child(spy)
|
||||
|
||||
await get_tree().process_frame # let this frame settle with the spy in place
|
||||
await get_tree().process_frame
|
||||
var baseline: int = spy.draw_count
|
||||
assert_int(baseline).override_failure_message(
|
||||
"sanity: the spy must have been drawn at least once before the no-input"
|
||||
+ " frame below, or this test can't distinguish self-heal from a first draw"
|
||||
).is_greater(0)
|
||||
|
||||
# No pan/zoom/gesture — the ONLY thing that should cause another _draw()
|
||||
# is _process()'s own self-heal, since has_pending_tiles() is still true
|
||||
# (no response has been delivered).
|
||||
v._process(0.016)
|
||||
await get_tree().process_frame
|
||||
|
||||
assert_int(spy.draw_count).override_failure_message(
|
||||
"_process() must queue a redraw every frame while has_pending_tiles()"
|
||||
+ " is true, with NO input/gesture — draw_count must have advanced past"
|
||||
+ " the baseline (%d), the cold-start self-heal" % baseline
|
||||
).is_greater(baseline)
|
||||
|
||||
|
||||
## The self-heal must STOP once every tile has arrived — a redraw queued
|
||||
## forever regardless of state would just be a disguised always-redraw, not
|
||||
## a targeted fix for the pending window.
|
||||
func test_process_stops_self_healing_once_every_tile_has_arrived() -> void:
|
||||
var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new())
|
||||
add_child(v)
|
||||
var radius_km := 6238.4 # GJ380c (Lendel)
|
||||
v.enter_orbital({"body_id": "GJ380c", "body_radius_km": radius_km}, {})
|
||||
var tile_set = v.get_tile_set()
|
||||
for tile: Dictionary in tile_set.get_tiles():
|
||||
var window: Dictionary = _mock_window(tile["center"])
|
||||
window["granularity_v2"] = "Region"
|
||||
window["granularity"] = SERVER_LEGACY_GRANULARITY_REGION_SENTINEL
|
||||
window["n"] = AtlasWindowRequest.SERVER_DISTRICT_WINDOW_MAX_N_REGION
|
||||
SimBridge.atlas_layers_received.emit(_mock_response("GJ380c", window))
|
||||
assert_bool(tile_set.has_pending_tiles()).override_failure_message(
|
||||
"sanity: every tile must have arrived after this loop"
|
||||
).is_false()
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# BUG 2 — legend stacking (the viewer-level half; see test_regional_screen.gd
|
||||
# for the nav-layer re-entry guard).
|
||||
# =============================================================================
|
||||
|
||||
|
||||
## Coordinator's live scene dump: WindowLegend measured 260x2343 px, ~10
|
||||
## legends stacked — ImplantPanel.clear() used deferred queue_free(), so
|
||||
## same-frame repeat refresh() calls piled new content onto STALE not-yet-
|
||||
## freed children instead of replacing them. N refresh() calls in the SAME
|
||||
## frame (no process_frame between them, matching how the actual trigger —
|
||||
## RegionalScreen.enter() previously lacking its own re-entry guard — landed
|
||||
## repeat enter_orbital() calls back to back) must leave exactly ONE legend's
|
||||
## worth of children, not N stacked copies.
|
||||
func test_legend_refresh_is_idempotent_against_same_frame_re_entry() -> void:
|
||||
var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new())
|
||||
add_child(v)
|
||||
v.enter({"body_id": "GJ380c"}, {}, Vector2i(10, 20), 2)
|
||||
|
||||
var baseline_count: int = v._legend_panel.get_implant_children().size()
|
||||
for _i in range(10):
|
||||
v._legend_panel.refresh()
|
||||
var after_count: int = v._legend_panel.get_implant_children().size()
|
||||
|
||||
assert_int(after_count).override_failure_message(
|
||||
(
|
||||
"10 same-frame refresh() calls must leave exactly ONE legend's worth of"
|
||||
+ " children (%d), not %d stacked copies — ImplantPanel.clear() must"
|
||||
+ " free immediately, not defer via queue_free()"
|
||||
) % [baseline_count, after_count]
|
||||
).is_equal(baseline_count)
|
||||
@@ -87,6 +87,12 @@ class _SingleWindowViewerStub:
|
||||
## contract, matching AtlasWindowTileSet.get_tiles()'s public shape exactly:
|
||||
## Array of {"center": Vector2i, "window": Variant}).
|
||||
class _TileModeViewerStub:
|
||||
# PR #192 cold-start dossier: AtlasWindowOverlay reads viewer.COLOR_BORDER_FADE
|
||||
# directly for a pending tile's wash (avoids a cyclic preload of the
|
||||
# viewer's own script — see that read site's own doc) — mirrored here
|
||||
# byte-for-byte (AtlasWindowViewer.COLOR_BORDER_FADE, private const).
|
||||
const COLOR_BORDER_FADE: Color = Color(0.20, 0.24, 0.30, 0.55)
|
||||
|
||||
var tiles: Array = []
|
||||
var held_center: Vector2i = Vector2i.ZERO
|
||||
var held_n: int = 0
|
||||
@@ -357,3 +363,45 @@ func test_tile_mosaic_draw_produces_visible_pixels() -> void:
|
||||
)
|
||||
% (fraction * 100.0)
|
||||
).is_greater(MIN_NON_BACKGROUND_FRACTION)
|
||||
|
||||
|
||||
## (c) PR #192 cold-start dossier, BUG 3: a mosaic tile with NO window yet
|
||||
## (the cold-server "still working" state, every tile in the mosaic at once
|
||||
## right after enter_orbital() on a real cold server) must render the SAME
|
||||
## border-fade wash the single-window path already gives its own
|
||||
## no-composite-yet wait — not a bare COLOR_BG gap that reads as broken.
|
||||
## Same real-render infrastructure as (a)/(b): a pending tile (`window: null`)
|
||||
## must still produce visible non-background pixels, proving the wash
|
||||
## genuinely draws rather than the loop just `continue`-ing past it silently.
|
||||
func test_pending_tile_gets_a_visible_border_fade_wash() -> void:
|
||||
if _dummy_renderer_active():
|
||||
print(SKIP_REASON)
|
||||
return
|
||||
var overlay: AtlasWindowOverlay = AtlasWindowOverlay.new()
|
||||
var stub := _TileModeViewerStub.new()
|
||||
var tile_n: int = AtlasWindowGeometry.TILE_N
|
||||
var cell_px: float = stub.get_cell_pixel_size()
|
||||
stub.held_center = Vector2i.ZERO
|
||||
stub.held_n = 19139 # GJ380c/Lendel's own raw circumference (live round 4)
|
||||
# Same centered-tile placement as (b) above, but with `window: null` —
|
||||
# the pending state this fix targets, instead of a real response.
|
||||
var half_tile: float = float(tile_n) * 0.5
|
||||
var half_body: float = float(stub.held_n) * 0.5
|
||||
var lone_tile_center := Vector2i(roundi(half_tile - half_body), roundi(half_tile - half_body))
|
||||
stub.tiles = [{"center": lone_tile_center, "window": null}]
|
||||
overlay.viewer = stub
|
||||
|
||||
var zoom: float = float(VIEWPORT_SIZE.x) * 1.5 / (half_body * cell_px)
|
||||
var image: Image = await _render_to_image(overlay, zoom)
|
||||
var fraction: float = _non_background_fraction(image)
|
||||
|
||||
assert_float(fraction).override_failure_message(
|
||||
(
|
||||
"a pending tile (window == null) must still render a visible border-fade"
|
||||
+ " wash — got only %.2f%% of the frame differing from COLOR_BG, meaning"
|
||||
+ " the tile is a bare background gap during the cold-server wait, which"
|
||||
+ " reads as broken rather than 'still working' (coordinator's closing"
|
||||
+ " question, PR #192 cold-start dossier)."
|
||||
)
|
||||
% (fraction * 100.0)
|
||||
).is_greater(MIN_NON_BACKGROUND_FRACTION)
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
## PR #192 cold-start dossier (BUG 2): RegionalScreen.enter() tests. Split
|
||||
## into its own file rather than folded into test_atlas_zoom_ladder.gd —
|
||||
## these exercise the NAV-LAYER re-entry guard (RegionalScreen itself), not
|
||||
## AtlasWindowViewer's own zoom-ladder mechanics that suite already owns.
|
||||
class_name TestRegionalScreen
|
||||
extends GdUnitTestSuite
|
||||
|
||||
const AtlasWindowRequest := preload("res://ui/implant/apps/atlas/atlas_window_request.gd")
|
||||
|
||||
## Dudley's WINDOW_GRANULARITY_REGION_KEY sentinel — mirrors
|
||||
## test_atlas_zoom_ladder.gd's own constant (see that file's doc for why the
|
||||
## real wire value matters, not a convenient placeholder).
|
||||
const SERVER_LEGACY_GRANULARITY_REGION_SENTINEL: int = 4294967295
|
||||
|
||||
|
||||
static func _mock_response(body_id: String, window: Variant) -> Dictionary:
|
||||
return {"body_id": body_id, "status": "Ready", "district_window": window}
|
||||
|
||||
|
||||
## BUG 2 root cause: ImplantApp._on_screen_changed() calls enter()
|
||||
## UNCONDITIONALLY on every screen_changed, including a repeat
|
||||
## nav.push("regional", ...) landing on the SAME screen already showing
|
||||
## (reachable from more than one input path — body-click, panel+Enter — and
|
||||
## plausible for a player to trigger twice on a slow cold server before the
|
||||
## first descent settles). Without RegionalScreen's own guard, a repeat
|
||||
## entry re-ran the FULL enter_orbital() teardown/rebuild — tearing down
|
||||
## every in-flight tile request node and rebuilding fresh (null-window) ones
|
||||
## — orphaning whatever had already arrived, plus refreshing the legend
|
||||
## from scratch each time (the confirmed ~10x legend stack). Proven here by
|
||||
## an ARRIVED tile's data: a teardown+rebuild resets it to null; a genuine
|
||||
## no-op leaves it exactly as it was — `is_same()` on `_tile_set` itself
|
||||
## can't tell (that Node is a fixed field, never reassigned — only its
|
||||
## INTERNAL request children get torn down and rebuilt).
|
||||
func test_repeat_enter_for_the_same_body_does_not_orphan_an_arrived_tile() -> void:
|
||||
var screen: RegionalScreen = auto_free(RegionalScreen.new())
|
||||
add_child(screen)
|
||||
var body: Dictionary = {"body_id": "GJ380c", "body_radius_km": 6238.4}
|
||||
screen.enter({"body": body, "system": {}})
|
||||
|
||||
var tile_set = screen._viewer.get_tile_set()
|
||||
var first_tile_center: Vector2i = tile_set.get_tiles()[0]["center"]
|
||||
var arrived_window: Dictionary = {
|
||||
"center": [first_tile_center.x, first_tile_center.y],
|
||||
"n": AtlasWindowRequest.SERVER_DISTRICT_WINDOW_MAX_N_REGION,
|
||||
"granularity": SERVER_LEGACY_GRANULARITY_REGION_SENTINEL,
|
||||
"granularity_v2": "Region",
|
||||
"morphology": PackedByteArray([8, 14, 0, 1]),
|
||||
"elev_q": PackedByteArray([40, 90, 5, 60]),
|
||||
"temp_dc": [120, 95, -32768, 60],
|
||||
"moisture_q": PackedByteArray([50, 30, 90, 20]),
|
||||
"vegetation": PackedByteArray([2, 1, 6, 3]),
|
||||
"glaciation": PackedByteArray([0, 0, 1, 2]),
|
||||
}
|
||||
SimBridge.atlas_layers_received.emit(_mock_response("GJ380c", arrived_window))
|
||||
assert_that(tile_set.get_tiles()[0]["window"]).override_failure_message(
|
||||
"sanity: the first tile's response must have been adopted before the repeat enter()"
|
||||
).is_equal(arrived_window)
|
||||
|
||||
screen.enter({"body": body, "system": {}})
|
||||
|
||||
assert_that(screen._viewer.get_tile_set().get_tiles()[0]["window"]).override_failure_message(
|
||||
"a repeat enter() for the SAME body must not orphan an already-arrived"
|
||||
+ " tile — a teardown+rebuild resets every tile's window back to null,"
|
||||
+ " which is the confirmed source of the cold-start legend stacking bug"
|
||||
).is_equal(arrived_window)
|
||||
|
||||
|
||||
## A GENUINE body change (different body_id) must still enter fresh — the
|
||||
## guard is scoped to "the same body, re-entered", never a blanket "ignore
|
||||
## the second enter() call ever".
|
||||
func test_enter_for_a_different_body_still_re_enters() -> void:
|
||||
var screen: RegionalScreen = auto_free(RegionalScreen.new())
|
||||
add_child(screen)
|
||||
screen.enter({"body": {"body_id": "GJ380c", "body_radius_km": 6238.4}, "system": {}})
|
||||
|
||||
screen.enter({"body": {"body_id": "OtherBody", "body_radius_km": 100.0}, "system": {}})
|
||||
|
||||
assert_str(screen._viewer.get_body_id()).override_failure_message(
|
||||
"a genuinely different body must still re-enter — not be swallowed by"
|
||||
+ " the same-body guard"
|
||||
).is_equal("OtherBody")
|
||||
|
||||
|
||||
## get_body_id() itself: empty before any entry, the entered body's id after.
|
||||
func test_get_body_id_reflects_the_currently_held_body() -> void:
|
||||
var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new())
|
||||
add_child(v)
|
||||
assert_str(v.get_body_id()).is_equal("")
|
||||
v.enter_orbital({"body_id": "GJ380c", "body_radius_km": 6238.4}, {})
|
||||
assert_str(v.get_body_id()).is_equal("GJ380c")
|
||||
Reference in New Issue
Block a user