fix(ui): PR #203 review round — total retirement + bin decode + wired reset
Tyre finding: the orphaned AtlasViewer cluster is now actually deleted (atlas_viewer, atlas_marker_overlay, atlas_descend_geometry, atlas_legend_panel — a sixth orphan found beyond the review list — atlas_generation_proxy, atlas_generation_state; ~2,497 lines), with reachability re-verified across preload/class_name/res:// strings, every .tscn, and the standalone companion app. Test suites triaged, not blanket-deleted: 5 pure AtlasOverlayColors tests relocated into test_atlas_window_colors, the live type-identity regression guard relocated into test_step_canvas_viewer, dead coverage deleted. A real harness gap surfaced during diligence and RULED, not patched: visual_scenarios/visual_capture golden shots call retired continuous-zoom API — no shim (would resurrect what D-255 kills); inventory recorded on re-scoped T-1157 (gate-invisible, manual targets only). Hoshe finding 1: decode_png_field now detects the [Error, PackedByteArray] bin-shape from messagepack.gd explicitly — a genuine msgpack bin payload decodes correctly instead of silently collapsing to [0,0]; test built from a real round-tripped bin decode. Hoshe finding 2: the hard zoom-out reset is wired — ascend at rung 0 with a drifted view triggers _reset_to_global (the restored HARD condition), behavioral tests through the real input path. Notes folded: refloat + edge-scroll test coverage, legend smoke suite, Vector2i narrowing-safety comment with computed headroom. gdlint clean on touched files; full client suite 3364/3364. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -97,19 +97,52 @@ static func _decode_status_field(status_raw: Variant) -> Dictionary:
|
||||
return {"status": "", "error": ""}
|
||||
|
||||
|
||||
## Repack a decoded {"png_bytes": [...]} field into a PackedByteArray ready
|
||||
## for Image.load_png_from_buffer(). `field_raw` is the raw decoded Variant
|
||||
## for one of the six PNG-per-field planes — null/malformed input returns an
|
||||
## empty PackedByteArray (the caller's Image decode then fails gracefully,
|
||||
## same "skip the bad entry" posture the rest of this cluster uses rather
|
||||
## than crashing on a malformed wire payload).
|
||||
## Repack a decoded {"png_bytes": ...} field into a PackedByteArray ready for
|
||||
## Image.load_png_from_buffer(). `field_raw` is the raw decoded Variant for
|
||||
## one of the six PNG-per-field planes.
|
||||
##
|
||||
## **Two possible decoded shapes for `png_bytes`, both handled (PR #203
|
||||
## review, Hoshe finding 1):**
|
||||
## 1. **Array of ints** — the shape rmp_serde ACTUALLY produces today (no
|
||||
## `serde_bytes` anywhere in server/src, confirmed by grep — see this
|
||||
## file's own header doc): serde's blanket `Vec<u8>` impl serializes via
|
||||
## `serialize_seq`, a msgpack ARRAY, which messagepack.gd's decoder turns
|
||||
## into a plain GDScript `Array` of small ints. `PackedByteArray(array)`
|
||||
## repacks this correctly.
|
||||
## 2. **`[error: int, data: PackedByteArray]`** — the shape messagepack.gd's
|
||||
## OWN `bin_8`/`bin_16`/`bin_32` decode path produces
|
||||
## (`StreamPeerBuffer.get_partial_data()`'s documented `[Error,
|
||||
## PackedByteArray]` pair), reachable the day the server adds
|
||||
## `serde_bytes` to `png_bytes` (a `bin`-typed field is a strictly-smaller
|
||||
## wire encoding for the same bytes, a plausible future optimization this
|
||||
## codec must not silently mis-decode). **This shape is ALSO `is Array`**
|
||||
## — it passed the old guard silently. Feeding it straight to
|
||||
## `PackedByteArray(array)` does NOT error; it coerces each Array element
|
||||
## to a byte, and a `PackedByteArray` element coerces to `0` (verified
|
||||
## live: `PackedByteArray([0, PackedByteArray([1,2,3])])` ==
|
||||
## `PackedByteArray([0, 0])`) — a silent, undetectable byte-loss that
|
||||
## would corrupt every PNG plane the day this shape appears, with no
|
||||
## error anywhere in the path (the corrupted bytes still often decode as
|
||||
## SOME image, just the wrong one).
|
||||
##
|
||||
## The fix: detect shape 2 explicitly (`size() == 2`, first element an
|
||||
## `int`, second a `PackedByteArray`) and return the inner bytes directly —
|
||||
## checked BEFORE the generic Array repack, so shape 2 never reaches the
|
||||
## byte-coercion path at all. Any other/malformed input (not a Dictionary,
|
||||
## no `png_bytes` key, an Array that matches neither shape) returns an empty
|
||||
## PackedByteArray — the caller's Image decode then fails gracefully, same
|
||||
## "skip the bad entry" posture the rest of this cluster uses rather than
|
||||
## crashing on a malformed wire payload.
|
||||
static func decode_png_field(field_raw: Variant) -> PackedByteArray:
|
||||
if not field_raw is Dictionary:
|
||||
return PackedByteArray()
|
||||
var bytes_raw: Variant = (field_raw as Dictionary).get("png_bytes")
|
||||
if not bytes_raw is Array:
|
||||
return PackedByteArray()
|
||||
return PackedByteArray(bytes_raw)
|
||||
var arr: Array = bytes_raw
|
||||
if arr.size() == 2 and arr[0] is int and arr[1] is PackedByteArray:
|
||||
return arr[1] # messagepack.gd's bin_8/16/32 [error, data] pair
|
||||
return PackedByteArray(arr)
|
||||
|
||||
|
||||
## Build an EncodedStepCanvas Dictionary (GDScript-side shape) from an
|
||||
@@ -171,6 +204,29 @@ static func step_canvas_response_from_raw(raw: Variant) -> Variant:
|
||||
}
|
||||
|
||||
|
||||
## Narrows the wire's `center: (i64, i64)` / `extent: (u32, u32)` pairs (both
|
||||
## StepCanvasResponse fields, step_canvas.rs) into Godot's `Vector2i`, whose
|
||||
## components are 32-bit signed ints (`int32_t`, range ±2,147,483,647 —
|
||||
## confirmed against Godot's own `Vector2i` type, not assumed). This
|
||||
## narrowing is safe under the two invariants both fields actually carry on
|
||||
## this wire, neither of them the full i64/u32 range in practice:
|
||||
## - `center` — world-metres. Bounded by the body's own circumference (a
|
||||
## center coordinate is never meaningfully larger than the body's half
|
||||
## circumference, `pi * body_radius_km * 1000`): even a Jupiter-class
|
||||
## body (~70,000 km radius) caps out around 2.2e8 m, three orders of
|
||||
## magnitude under the int32 ceiling. No real or plausible body in this
|
||||
## project's scope (`server/data/systems.db`'s body_radius_km column)
|
||||
## approaches the ~682,000 km radius that WOULD overflow int32 metres.
|
||||
## - `extent` — canvas cells. Hard-clamped server-side to
|
||||
## STEP_CANVAS_MAX_EXTENT_AXIS (3,840 per axis, step_canvas.rs's
|
||||
## `clamp_step_canvas_extent()`) before it ever reaches the wire —
|
||||
## trivially in range regardless of what a malicious/buggy sender might
|
||||
## have originally requested (the CLAMPED value is what's echoed, per
|
||||
## this file's own "extent ECHO rule" doc on StepCanvasResponse).
|
||||
## A value outside either invariant (a malformed/adversarial frame) still
|
||||
## narrows via GDScript's own int() truncation rather than erroring — no
|
||||
## worse than any other wire field this codec trusts once decoded, and no
|
||||
## production code path can produce one.
|
||||
static func _vec_from_pair(pair: Variant) -> Vector2i:
|
||||
if pair is Array and pair.size() >= 2:
|
||||
return Vector2i(int(pair[0]), int(pair[1]))
|
||||
|
||||
Reference in New Issue
Block a user