fix(client): T-1153 — v2 granularity is authoritative in the staleness guard; legacy compared only when v2 absent
Second live-round blocker: the T-1150 legacy granularity comparison stayed armed alongside the v2 check, and the server ALWAYS sends the u32::MAX Region sentinel in the legacy slot — which can never equal the client's pinned legacy value, so every Region response was stale-dropped after the v2 check passed. _echoed_granularity_matches() now branches on PRESENCE of granularity_v2: present -> v2 is the only comparison; absent (old server) -> legacy fallback. Mock-fleet audit while fixing: three tests had responses diverging from the real wire — the oversized-orbital round-trip omitted the legacy sentinel (passed for the wrong reason), the Region-accept test used legacy=1, and the mismatch-drop test left v2 at a masking default that would have inverted under the new rule. All wire-accurate now with a named SERVER_LEGACY_GRANULARITY_REGION_SENTINEL const; +2 tests (legacy- only old-server acceptance; v2-wins-regardless-of-legacy precedence proof). All three verified to fail against the reverted fix. Full suite 3524/3524.
This commit is contained in:
@@ -259,12 +259,35 @@ func _on_debounce_timeout() -> void:
|
||||
## Handle an AtlasLayerResponse (routed by the owning viewer from its own
|
||||
## SimBridge.atlas_layers_received subscription — this object has no signal
|
||||
## connection of its own, matching atlas_generation_proxy.gd's on_response()
|
||||
## shape). Ignores responses for a stale body/center/n/granularity/min_wl_m/
|
||||
## granularity_v2 (the player panned, zoomed across a rung boundary, or
|
||||
## navigated away while a request was in flight, or a different rung's derive
|
||||
## answers a request for a different rung, T-1150/T-1152) — the echoed fields
|
||||
## ARE the staleness guard (§2, extended T-1150/T-1152), compared here
|
||||
## against what THIS object most recently asked for.
|
||||
## shape). Ignores responses for a stale body/center/n/min_wl_m/granularity
|
||||
## (legacy OR v2, see below) — the player panned, zoomed across a rung
|
||||
## boundary, or navigated away while a request was in flight, or a different
|
||||
## rung's derive answers a request for a different rung, T-1150/T-1152 — the
|
||||
## echoed fields ARE the staleness guard (§2, extended T-1150/T-1152),
|
||||
## compared here against what THIS object most recently asked for.
|
||||
##
|
||||
## **Live-round finding (the second C1-shaped bug): v2 is AUTHORITATIVE over
|
||||
## the legacy field whenever v2 is present — the legacy comparison is
|
||||
## SKIPPED entirely, not run alongside it.** A T-1152-aware server (this
|
||||
## codebase's) ALWAYS populates `granularity_v2` on the wire (Dudley's
|
||||
## contract, `DistrictWindowLayer.granularity_v2`'s own doc: "Always
|
||||
## populated (never `None`)"), and for `Region` responses specifically the
|
||||
## LEGACY `granularity` slot carries `WINDOW_GRANULARITY_REGION_KEY`
|
||||
## (`u32::MAX` = 4294967295) — a reserved KEY-SPACE TAG, not a real
|
||||
## multiplier, that can never equal this object's own stored `_granularity`
|
||||
## (which stays pinned at `DEFAULT_GRANULARITY`=1 for every rung this object
|
||||
## requests, per that field's own doc — the legacy slot has no concept of
|
||||
## Region at all). Comparing the legacy field UNCONDITIONALLY alongside v2
|
||||
## therefore drops EVERY Region response as stale forever, even though the
|
||||
## v2 comparison alone would have correctly accepted it — exactly the live
|
||||
## bug (`_held_n` fixed; this is the same "old comparison still active
|
||||
## alongside the new one" class of bug, one layer up in the staleness
|
||||
## checks). Fix: branch on whether `granularity_v2` is actually PRESENT in
|
||||
## the response dict (`w.has(...)`, not `w.get(..., default)` — the
|
||||
## presence/absence distinction is the whole point here) — present (every
|
||||
## real server, always) -> v2 is the ONLY granularity comparison; absent (a
|
||||
## hypothetically old, pre-T-1152 server) -> fall back to the legacy
|
||||
## comparison alone, matching this object's own pre-T-1152 behavior exactly.
|
||||
func on_response(response: Dictionary) -> void:
|
||||
if str(response.get("body_id", "")) != _body_id:
|
||||
return
|
||||
@@ -289,21 +312,13 @@ func on_response(response: Dictionary) -> void:
|
||||
var w: Dictionary = window
|
||||
var echoed_center := _vec_from_center(w.get("center", [0, 0]))
|
||||
var echoed_n := int(w.get("n", 0))
|
||||
var echoed_granularity := int(w.get("granularity", AtlasWindowCache.DISTRICT_GRANULARITY))
|
||||
var echoed_min_wl_m := int(w.get("min_wl_m", 0))
|
||||
# T-1152: granularity_v2 is ALWAYS populated on a real server response
|
||||
# (resolve_window_granularity_v2() always resolves to a concrete rung —
|
||||
# see DistrictWindowLayer.granularity_v2's own doc), but the mock/old-shape
|
||||
# response fixtures this suite's own tests build predate the field —
|
||||
# default to "District" so an old-shape mock keeps matching a
|
||||
# district-granularity request exactly as it did before this field existed.
|
||||
var echoed_granularity_v2 := str(w.get("granularity_v2", AtlasWindowCache.DEFAULT_GRANULARITY_V2))
|
||||
var granularity_matches: bool = _echoed_granularity_matches(w)
|
||||
if (
|
||||
echoed_center != _center
|
||||
or echoed_n != _n
|
||||
or echoed_granularity != _granularity
|
||||
or echoed_min_wl_m != _min_wl_m
|
||||
or echoed_granularity_v2 != _granularity_v2
|
||||
or not granularity_matches
|
||||
):
|
||||
return # stale — answers a window we've since panned/zoomed away from, or a different rung
|
||||
|
||||
@@ -313,6 +328,21 @@ func on_response(response: Dictionary) -> void:
|
||||
window_ready.emit(w)
|
||||
|
||||
|
||||
## The granularity half of on_response()'s staleness check, split out for the
|
||||
## v2-authoritative-when-present precedence rule (see on_response()'s own
|
||||
## doc for the full live-round rationale). Presence, not value, is the
|
||||
## branch: `w.has("granularity_v2")` — a real server ALWAYS sets this key
|
||||
## (even if its value happened to coincidentally equal a default), so
|
||||
## checking presence rather than "is it the default value" is the only
|
||||
## correct way to distinguish "an old server that never heard of this field"
|
||||
## from "a new server whose value happens to match."
|
||||
func _echoed_granularity_matches(w: Dictionary) -> bool:
|
||||
if w.has("granularity_v2"):
|
||||
return str(w.get("granularity_v2")) == _granularity_v2
|
||||
var echoed_granularity := int(w.get("granularity", AtlasWindowCache.DISTRICT_GRANULARITY))
|
||||
return echoed_granularity == _granularity
|
||||
|
||||
|
||||
func _schedule_retry() -> void:
|
||||
var timer := get_tree().create_timer(RETRY_DELAY)
|
||||
timer.timeout.connect(
|
||||
|
||||
Reference in New Issue
Block a user