From e7319cf32231c2e56a301fcf47f70e4b8c0ccc2e Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Jul 2026 14:19:31 +0200 Subject: [PATCH 1/4] chore(meta): file T-1142 (descent bounds + fit bugs, Jeroen hands-on) + T-1143 (quiet-interior legibility) Co-Authored-By: Claude Fable 5 --- .pql/changelog/ticket_idmap/2026-07.sql | 2 ++ .pql/changelog/tickets/2026-07.sql | 2 ++ 2 files changed, 4 insertions(+) diff --git a/.pql/changelog/ticket_idmap/2026-07.sql b/.pql/changelog/ticket_idmap/2026-07.sql index b94863c97..777022257 100644 --- a/.pql/changelog/ticket_idmap/2026-07.sql +++ b/.pql/changelog/ticket_idmap/2026-07.sql @@ -55,3 +55,5 @@ INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_ INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FQ9GQDWFC5S4PK17NGX35RFG', 'T-1139', '2026-07-18 10:25:59.781', '2026-07-18 10:25:59.781', NULL, 'f5edd2949793ae672684daf519838bbb', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > ticket_idmap.updated_at OR (excluded.updated_at = ticket_idmap.updated_at AND excluded.hash > ticket_idmap.hash); INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FR6Y1CNA5JJ68RN897RM7FBC', 'T-1140', '2026-07-21 06:58:38.892', '2026-07-21 06:58:38.892', NULL, '8137300e4179fcf6ab11d02463ec616e', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > ticket_idmap.updated_at OR (excluded.updated_at = ticket_idmap.updated_at AND excluded.hash > ticket_idmap.hash); INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FR8WNW3MY9X5NZ3VZFKKH1HR', 'T-1141', '2026-07-21 11:32:19.616', '2026-07-21 11:32:19.616', NULL, '499974f2c3095b5c0a1a1b3623f34166', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > ticket_idmap.updated_at OR (excluded.updated_at = ticket_idmap.updated_at AND excluded.hash > ticket_idmap.hash); +INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FR97BZHA6DV68S1BFG0HBN8G', 'T-1142', '2026-07-21 12:19:02.156', '2026-07-21 12:19:02.156', NULL, '3a486c5b1a809d2233fe1446c2984edf', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > ticket_idmap.updated_at OR (excluded.updated_at = ticket_idmap.updated_at AND excluded.hash > ticket_idmap.hash); +INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FR97DPEHANCYX2AC4021BSJC', 'T-1143', '2026-07-21 12:19:16.213', '2026-07-21 12:19:16.213', NULL, '45182fc00280db5f3260eb495d3d0d62', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > ticket_idmap.updated_at OR (excluded.updated_at = ticket_idmap.updated_at AND excluded.hash > ticket_idmap.hash); diff --git a/.pql/changelog/tickets/2026-07.sql b/.pql/changelog/tickets/2026-07.sql index 55991c92e..dcba57130 100644 --- a/.pql/changelog/tickets/2026-07.sql +++ b/.pql/changelog/tickets/2026-07.sql @@ -2347,3 +2347,5 @@ INSERT INTO tickets (record_id, type, parent_record_id, title, description, stat Implementation note (Tyre, T-1124 report): DISTRICT_WINDOW_MIN_ZOOM proposed at 6.0 but atlas_viewer.gd caps _view_zoom at MAX_ZOOM = 8.0 — only a 6.0-8.0 band for the whole in-window-zoom detail story the SS5 legibility argument depends on. Tunable client constant, not wire contract: either lower the swap threshold or raise MAX_ZOOM; decide with on-screen-scale math at implementation. SUPERSEDED ENTRY MECHANIC (Jeroen, 2026-07-21 companion hands-on — recorded as the D-226 T-1124 SS5 entry revision): NO zoom-threshold LOD swap and NO DISTRICT_WINDOW_MIN_ZOOM — the earlier zoom-headroom note is moot. The planetary heightmap view becomes FIXED: remove drag-pan/wheel-zoom of the planetary canvas IN THIS TICKET, same change as the replacement (never strand close inspection), and update the header hint text accordingly. Entry: hovering the planetary heightmap shows a rectangle cursor representing the regional-mode bounds; click descends into the regional map centered on the click point''s derived DistrictPos (float-on-center rules carry over with pan-center read as click-point). In-mode behavior unchanged (pan-only refetch, 150ms debounce, D-227 cache, border-fade). Morph transition between modes explicitly deferred — the descent may cut. Open design point: the rectangle cursor is an affordance, not to scale (n=64 = ~131km = a few px on a planetary canvas) — resolve the honest representation (true-extent rectangle + zoom-in cut, or not-to-scale reticle with the real extent labeled) without implying more coverage than real. Note: set_view/get_view_offset (T-1120 capture API) must survive for the visual-golden harness even after user pan/zoom is removed.', 'done', 'high', NULL, 'client', NULL, '2026-07-18 10:02:12.668', '2026-07-21 12:03:40.355', NULL, 'fa260128491e3d8fae6b3cb991b9afe6', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FR97BZHA6DV68S1BFG0HBN8G', 'bug', '06FB0TNSRZXCHGS16BFHSSGSV4', 'Descent allowed outside the map texture — letterbox clicks produce impossible DistrictPos; window opens as unfitted postage stamp', 'Jeroen''s first hands-on descent (2026-07-21): clicked in the letterbox beside the fixed planetary map and landed at DistrictPos (12276, 3021) — an IMPOSSIBLE column (largest terrestrial body with terrain is 7324.5 km, max valid col ~11236). Root cause: _update_hover sets _hover_active unconditionally and neither the reticle guard nor the descend fall-through checks that the cursor is on the heightmap texture — the 2:1 map letterboxed in a wide viewport leaves dead zones where the reticle promises descent and the click derives beyond the planet edge (clamped sampling = the uniform green he saw). Second defect: atlas_window_viewer.enter() resets zoom 1.0 / offset ZERO with no fit — an n=32 composite renders 512px native in a ~1900px viewport (postage stamp, not a filled map). Fixes: (1) client bounds gate — reticle hidden and descent inert unless screen_to_canvas(pos) is inside [0,tex_w)x[0,tex_h); (2) fit-and-center on enter + first window arrival + resize (zoom = ~0.9 x viewport/composite clamped to MIN/MAX_ZOOM, offset centers); (3) district header gains the body proper name (cheap half of T-1141 pulled forward — pass the body dict into enter); (4) SERVER: validate window_center like window_n — wrap columns modulo the body circumference, clamp rows to +/-rows_half (never trust the wire; today an out-of-range center derives clamped garbage and caches it); (5) live full-chain verification this time — the T-1138 live check requested a known-good pos directly over the wire, bypassing the click chain entirely, which is exactly where this hid. Also verify drag-pan works in the fitted view.', 'backlog', 'high', NULL, 'client', NULL, '2026-07-21 12:19:02.154', '2026-07-21 12:19:02.154', NULL, '7fc78c537e3d9a9c64e644e0cfb6f5db', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FR97DPEHANCYX2AC4021BSJC', 'task', '06FB0TNSRZXCHGS16BFHSSGSV4', 'Regional window legibility on quiet interiors — window-local elevation contrast', 'Follow-on from Jeroen''s first descent (2026-07-21): even with T-1142 fixed, many interior windows are genuinely one morphology zone across 66km, and the composite''s absolute lightness modulation (0.7 + 0.3 x elev_q/100, D-226 T-1124 SS5) spans only ~1-3% lightness over a window whose local elev range is small — the window reads as a dead flat fill rather than terrain. Proposal to evaluate WITH Araminta (SS5 is her encoding): window-local contrast stretch — normalize lightness within the window''s own elev min..max (fixed band, e.g. 0.62..0.92) with a degenerate floor (single-value window stays honestly flat), deterministic per window, mirroring how the aliveness_probe elev panel achieved its 12-level local texture. Alternative/complement: a faint deterministic per-cell variation or contour hinting. Any change to the recorded SS5 encoding gets a one-line amendment note. Assess AFTER T-1142 lands — real clicks will then land on real terrain and the residual flatness can be judged on coastal vs interior windows.', 'backlog', 'medium', NULL, 'client', NULL, '2026-07-21 12:19:16.212', '2026-07-21 12:19:16.212', NULL, '51e0232b3c75b03f994638ff86943956', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); -- 2.54.0 From 4a65bb210d18a89aa3dbab00a30fee3343aa990e Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Jul 2026 14:52:50 +0200 Subject: [PATCH 2/4] fix(simulation): normalize window_center before cache and coalescing keys (T-1142 server half) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server clamped window_n but trusted window_center — the letterbox bug's impossible (12276, 3021) was derived at clamped-garbage latitude and CACHED under its raw key. normalize_window_center mirrors the forward map exactly (column rem_euclid against the body's district circumference, row clamped to +/-half-meridian; constants from scale::DISTRICT_M, no new magic numbers; no-radius test bodies identity). Applied in serve_district_window BEFORE the window-cache key and the coalescing key, so an insane request and its sane twin share one entry — proven end-to-end with the literal bug values on a 500km fixture body resolving to canonical (4, 383), byte-identical. Echo contract: DistrictWindowLayer.center now carries the NORMALIZED value. Reconciled by design with the client half in this same branch — the client canonicalizes before sending (bit-identical math, posmod == rem_euclid verified), so request and echo agree from the first round-trip; server normalization is defense-in-depth for buggy or modified clients. 6 new tests incl. the twin-cache proof and the echo pin; atlas:: 570/570, lib 1784/1784, timing-sensitive tests 3x stable. Co-Authored-By: Claude Fable 5 --- server/src/atlas/layer_proxy.rs | 453 +++++++++++++++++++++++++++++--- 1 file changed, 419 insertions(+), 34 deletions(-) diff --git a/server/src/atlas/layer_proxy.rs b/server/src/atlas/layer_proxy.rs index c4bee2ea7..fc7b30768 100644 --- a/server/src/atlas/layer_proxy.rs +++ b/server/src/atlas/layer_proxy.rs @@ -18,10 +18,11 @@ use crate::atlas::body_params_reader::BodyParamsReader; use crate::atlas::body_world_state::{BodyWorldState, BodyWorldStateCache, SimTick}; use crate::atlas::cascade::CascadeLayer; use crate::atlas::city_context_reader::CityContextReader; -use crate::atlas::district_profile::DistrictPos; +use crate::atlas::district_profile::{BodyParams, DistrictPos}; use crate::atlas::gen_queue::{GenPriority, GenWorkItem, GenerationQueue}; use crate::atlas::layer1::Layer1Output; use crate::atlas::road_graph::RoadNodeKind; +use crate::atlas::scale::DISTRICT_M; use crate::atlas::source_resolver::{BodySourceResolver, SourceResolveError}; use crate::bridge::ConnectionId; use crate::seed::{SeedChain, SeedDomain}; @@ -745,6 +746,64 @@ pub fn build_settlement_layer(state: &BodyWorldState) -> Option Some(SettlementLayer { settlements }) } +/// Normalize a wire-supplied district-window centre against the body's +/// physical geometry (T-1142 — a letterbox-click bug sent `window_center` +/// wildly out of a body's valid range; the server accepted it, derived +/// clamped garbage inside `derive_district`, and CACHED that garbage under +/// the raw un-normalized key). Mirrors `district_profile::derive_district`'s +/// own forward mapping (`district_profile.rs:1426-1451`) EXACTLY, so a window +/// centre that survives this normalization derives identically to how +/// `derive_district` would have resolved it anyway — this function only +/// closes the gap between "derive_district silently clamps/wraps internally" +/// and "the SERVING path (cache key, coalescing key) saw the raw value". +/// +/// **Column (longitude) wraps** — `rem_euclid` against the body's +/// circumference in districts, mirroring the forward map's +/// `(wx / circumference_m).rem_euclid(1.0)` (longitude is periodic; a click +/// at column 12276 on a body whose circumference is a few hundred districts +/// wide is the SAME point as some small in-range column, not garbage). +/// +/// **Row (latitude) clamps** — to `±half_meridian_districts`, mirroring the +/// forward map's `(wy / meridian_m).clamp(-0.5, 0.5)` (latitude is NOT +/// periodic; it terminates at the poles, so out-of-range rows collapse to the +/// nearest pole rather than wrapping — same asymmetry `derive_district` +/// itself already encodes). +/// +/// Both bounds are derived from `body_radius_km` via the SAME `DISTRICT_M` +/// (2 048 m, D-243) constant the forward map uses — no independent magic +/// numbers that could silently drift out of sync with `derive_district`. +/// +/// **No radius (`body_radius_km` absent/non-positive — tiny test bodies +/// only, per `BodyParams`'s own doc: "e.g. tiny test bodies"; every real +/// `systems.db` body row carries a radius):** identity, no wrap/clamp. The +/// forward map's own no-radius branch has no periodicity concept either (it +/// clamps the FRACTIONAL PIXEL position directly against the heightmap's +/// working-grid dimensions, which aren't known at request-serving time — only +/// inside the Rayon work item once the heightmap is loaded); a real letterbox +/// click can never hit this branch, so it is out of this fix's scope. +fn normalize_window_center(params: &BodyParams, center: DistrictPos) -> DistrictPos { + let (dx, dy) = center; + match params.body_radius_km { + Some(r_km) if r_km > 0.0 => { + let circumference_m = std::f64::consts::TAU * r_km * 1000.0; + let meridian_m = std::f64::consts::PI * r_km * 1000.0; + // Whole districts per full circumference / per half-meridian — + // rounded (not truncated) so the bound matches the forward map's + // continuous fraction as closely as an integer district grid can. + let districts_per_circumference = + (circumference_m / DISTRICT_M as f64).round().max(1.0) as i32; + let half_meridian_districts = (meridian_m / DISTRICT_M as f64 / 2.0).round() as i32; + + let wrapped_dx = dx.rem_euclid(districts_per_circumference); + let clamped_dy = dy.clamp(-half_meridian_districts, half_meridian_districts); + (wrapped_dx, clamped_dy) + } + // No radius: derive_district's own fallback has no wrap/clamp concept + // at the DistrictPos level (see the doc above) — identity. + _ => center, + } +} + /// Resolve `req`'s district-window query, if any (D-226 T-1124 amendment, /// T-1137). Returns `None` immediately when `req.window_center` is absent (no /// window requested — the common case, zero cost). @@ -756,15 +815,28 @@ pub fn build_settlement_layer(state: &BodyWorldState) -> Option /// about to take its cache-hit or cache-miss branch, sharing neither's control /// flow. /// -/// Cache hit (`(body_id, center, n)` already in `window_cache`) → `Some` -/// immediately, no queue submission (D-227: a previously-derived window for -/// this body+seed is valid forever, no staleness check needed). Cache miss → -/// submit a `DeriveWindow` work item (queue-based, per the amendment's binding -/// serving model — never inline here) and return `None`; the *next* request -/// for this `(body, center, n)` re-checks the cache and finds it populated -/// once `drain_generation_completions` has processed the completion (the -/// existing D-225 poll-and-recheck-cache pattern every other layer already -/// uses, not a push). +/// **`window_center` is normalized via [`normalize_window_center`] BEFORE the +/// `DistrictWindowCache` key AND the `submit_window` coalescing key are built** +/// (T-1142) — this is why `body_params` is read HERE, unconditionally, +/// rather than only inside the former miss-branch: normalization needs +/// `body_radius_km` to compute the wrap/clamp bounds, and it must happen +/// before either key exists, or an insane request and its sane normalized +/// twin would land in different cache entries / coalesce independently +/// (exactly the bug this fix closes — a garbage `window_center` was cached +/// standalone instead of collapsing onto its valid twin). The one-time cost +/// (a single indexed `bodies` row read) is paid on every window request now, +/// not just on a cache miss — a request whose normalized center hits the +/// cache still needed this read to know WHICH key to check. +/// +/// Cache hit (`(body_id, normalized_center, n)` already in `window_cache`) → +/// `Some` immediately, no queue submission (D-227: a previously-derived +/// window for this body+seed is valid forever, no staleness check needed). +/// Cache miss → submit a `DeriveWindow` work item (queue-based, per the +/// amendment's binding serving model — never inline here) and return `None`; +/// the *next* request for this `(body, normalized_center, n)` re-checks the +/// cache and finds it populated once `drain_generation_completions` has +/// processed the completion (the existing D-225 poll-and-recheck-cache +/// pattern every other layer already uses, not a push). /// /// `window_n` is clamped to `[1, DISTRICT_WINDOW_MAX_N]` here — the ONE place /// that clamp is applied; nothing downstream re-checks the wire value. @@ -778,29 +850,15 @@ fn serve_district_window( world_seed: u64, conn_id: ConnectionId, ) -> Option { - let center = req.window_center?; + let raw_center = req.window_center?; let n = req.window_n.clamp(1, DISTRICT_WINDOW_MAX_N); - let key: DistrictWindowKey = (req.body_id.clone(), center, n); - if let Some(layer) = window_cache.get(&key) { - return Some(layer.clone()); - } - - // Miss — resolve heightmap + body params and submit a background derive. - // Read/resolve failures are non-fatal for the window (log + skip): the - // window simply stays None on this response, same as an unrun whole-body - // layer, rather than failing the entire AtlasLayerResponse. - let heightmap_path = match resolver.resolve(&req.body_id) { - Ok(p) => p, - Err(e) => { - tracing::warn!( - body_id = %req.body_id, - error = %e, - "district window request: heightmap resolve failed — window stays None" - ); - return None; - } - }; + // body_params is needed to normalize the centre BEFORE either cache key + // exists (T-1142) — read it first, unconditionally (not gated on a cache + // miss like the former structure). A read failure here can't distinguish + // "insane vs. sane center" for the key, so it's a hard skip for the whole + // window (window stays None on this response), same failure posture the + // former miss-only read already had. let Some(reader) = body_params_reader else { tracing::warn!( body_id = %req.body_id, @@ -820,6 +878,37 @@ fn serve_district_window( } }; + let center = normalize_window_center(&body_params, raw_center); + if center != raw_center { + tracing::debug!( + body_id = %req.body_id, + raw = ?raw_center, + normalized = ?center, + "district window request: out-of-range window_center normalized (T-1142)" + ); + } + + let key: DistrictWindowKey = (req.body_id.clone(), center, n); + if let Some(layer) = window_cache.get(&key) { + return Some(layer.clone()); + } + + // Miss — resolve the heightmap and submit a background derive. + // Read/resolve failures are non-fatal for the window (log + skip): the + // window simply stays None on this response, same as an unrun whole-body + // layer, rather than failing the entire AtlasLayerResponse. + let heightmap_path = match resolver.resolve(&req.body_id) { + Ok(p) => p, + Err(e) => { + tracing::warn!( + body_id = %req.body_id, + error = %e, + "district window request: heightmap resolve failed — window stays None" + ); + return None; + } + }; + queue.submit_window( GenWorkItem::DeriveWindow { body_id: req.body_id.clone(), @@ -1434,6 +1523,284 @@ mod tests { ); } + // ------------------------------------------------------------------- + // normalize_window_center (T-1142 — letterbox-click out-of-range bug) + // ------------------------------------------------------------------- + + /// A small-body fixture (500 km radius) whose bounds are hand-checkable: + /// `districts_per_circumference = round(TAU*500_000/2048) = 1534`, + /// `half_meridian_districts = round(PI*500_000/2048/2) = 383`. The + /// literal column/row this fixture uses (`12276`, `3021`) are the exact + /// values the reported T-1142 letterbox-click bug sent — at THIS radius + /// they genuinely overflow both bounds (at Earth radius, coincidentally, + /// they wouldn't — the bounds are tens of thousands of districts wide), + /// so this is a faithful small-body reproduction, not just an + /// arbitrarily-chosen out-of-range pair. + fn small_body_params() -> BodyParams { + BodyParams { + hydrosphere: Some("ocean".into()), + atmosphere: Some("breathable".into()), + planet_class: Some("temperate".into()), + body_radius_km: Some(500.0), + ..Default::default() + } + } + + /// Out-of-range COLUMN wraps (longitude is periodic) to its in-range + /// canonical twin via `rem_euclid` — mirroring `derive_district`'s own + /// `(wx / circumference_m).rem_euclid(1.0)` forward map. + #[test] + fn normalize_window_center_wraps_out_of_range_column() { + let params = small_body_params(); + // districts_per_circumference = 1534 (hand-computed above). + // 12276 rem_euclid 1534 = 4 (12276 = 8*1534 + 4). + assert_eq!( + 12276_i32.rem_euclid(1534), + 4, + "sanity: the hand-computed wrap" + ); + let (nx, ny) = normalize_window_center(¶ms, (12276, 0)); + assert_eq!( + nx, 4, + "out-of-range column wraps to its canonical in-range twin" + ); + assert_eq!(ny, 0, "an in-range row is untouched"); + + // The canonical twin normalizes to itself (idempotent). + let (nx2, _) = normalize_window_center(¶ms, (4, 0)); + assert_eq!(nx2, 4); + + // Negative columns wrap too (rem_euclid, not truncating rem) — + // longitude has no sign discontinuity. + let (nx3, _) = normalize_window_center(¶ms, (-1, 0)); + assert_eq!( + nx3, 1533, + "negative column wraps to the top of the range, not a negative remainder" + ); + } + + /// Beyond-pole ROW clamps (latitude terminates, does not wrap) to + /// `±half_meridian_districts` — mirroring `derive_district`'s own + /// `(wy / meridian_m).clamp(-0.5, 0.5)` forward map. This is the + /// asymmetry the coordinator's fix explicitly calls out: columns wrap, + /// rows clamp — never the other way around. + #[test] + fn normalize_window_center_clamps_beyond_pole_row() { + let params = small_body_params(); + // half_meridian_districts = 383 (hand-computed above). + let (_, ny) = normalize_window_center(¶ms, (0, 3021)); + assert_eq!( + ny, 383, + "beyond-pole row clamps to the pole boundary, not wraps" + ); + + let (_, ny_neg) = normalize_window_center(¶ms, (0, -9000)); + assert_eq!(ny_neg, -383, "clamping is symmetric at both poles"); + + // A row exactly at the boundary is untouched. + let (_, ny_boundary) = normalize_window_center(¶ms, (0, 383)); + assert_eq!(ny_boundary, 383); + } + + /// An in-range center (well inside both bounds) is returned UNCHANGED — + /// normalization must be a no-op for the overwhelming common case (every + /// legitimate click), not just a defensive clamp that happens to also + /// preserve valid input. + #[test] + fn normalize_window_center_leaves_in_range_center_unchanged() { + let params = small_body_params(); + let center = (100, -50); + assert_eq!(normalize_window_center(¶ms, center), center); + + // (0, 0) — the origin — is always in range regardless of body size. + assert_eq!(normalize_window_center(¶ms, (0, 0)), (0, 0)); + } + + /// No-radius bodies (tiny test-body fallback, `body_radius_km: None`) get + /// IDENTITY — `derive_district`'s own no-radius branch has no + /// wrap/clamp-in-district-space concept (see the function doc); an + /// extreme center here is out of this fix's scope by design, not an + /// oversight. + #[test] + fn normalize_window_center_no_radius_is_identity() { + let params = BodyParams::default(); // body_radius_km: None + let extreme = (999_999, -999_999); + assert_eq!(normalize_window_center(¶ms, extreme), extreme); + } + + /// End-to-end (the coordinator's core ask): an out-of-range + /// `window_center` and its already-normalized twin, requested through the + /// REAL `handle_atlas_request` path, land in the SAME `DistrictWindowCache` + /// entry and produce a byte-identical `DistrictWindowLayer` — normalization + /// happens BEFORE the cache key is built, so an insane request and its + /// sane twin never diverge into separate cache entries (the bug this fix + /// closes: the insane request was cached STANDALONE). + #[test] + fn insane_and_sane_twin_requests_share_one_cache_entry() { + let mut cache = BodyWorldStateCache::new(CACHE_CAPACITY); + let mut window_cache = DistrictWindowCache::new(DISTRICT_WINDOW_CACHE_CAPACITY); + let (_db, resolver, params_reader, _root) = + resolver_and_params_reader_with_radius("SmallMoon", 500.0); + let queue = GenerationQueue::with_threads(1); + + // The insane request — the reported T-1142 letterbox-click values. + let insane_req = AtlasLayerRequest { + body_id: "SmallMoon".to_string(), + up_to: CascadeLayer::Topography, + window_center: Some((12276, 3021)), + window_n: 4, + }; + let resp1 = handle_atlas_request( + &insane_req, + &mut cache, + &mut window_cache, + &queue, + &resolver, + None, + Some(¶ms_reader), + 42, + 1, + test_conn_id(), + ); + assert!( + resp1.district_window.is_none(), + "first request — cache miss, DeriveWindow submitted" + ); + + // Wait for the background derive to complete and drain it into the cache + // (mirrors handle_atlas_request_clamps_oversized_window_n's pattern). + std::thread::sleep(Duration::from_millis(300)); + let completions = queue.drain_completions(); + for c in completions { + if let GenCompletion::WindowDerived { body_id, layer } = c { + if body_id == "SmallMoon" { + window_cache.insert((body_id, layer.center, layer.n), *layer); + } + } + } + // Exactly ONE entry must exist in the window cache after the insane + // request's derive completes — normalization means it was keyed on + // the canonical (4, 383), not the raw (12276, 3021). + assert_eq!( + window_cache.len(), + 1, + "the insane request's derive must be cached under its NORMALIZED key" + ); + + // The "sane twin" — the already-normalized canonical center — hits + // the SAME cache entry the insane request just populated. + let sane_twin_req = AtlasLayerRequest { + body_id: "SmallMoon".to_string(), + up_to: CascadeLayer::Topography, + window_center: Some((4, 383)), // the hand-computed canonical twin + window_n: 4, + }; + let resp2 = handle_atlas_request( + &sane_twin_req, + &mut cache, + &mut window_cache, + &queue, + &resolver, + None, + Some(¶ms_reader), + 42, + 2, + test_conn_id(), + ); + let twin_layer = resp2.district_window.expect( + "the sane twin must hit the cache the insane request populated — no new derive needed", + ); + assert_eq!( + window_cache.len(), + 1, + "the sane twin must NOT create a second cache entry" + ); + + // Re-request the ORIGINAL insane center too — it must ALSO now hit + // the same populated cache entry (both requests normalize to the + // same key). + let resp3 = handle_atlas_request( + &insane_req, + &mut cache, + &mut window_cache, + &queue, + &resolver, + None, + Some(¶ms_reader), + 42, + 3, + test_conn_id(), + ); + let insane_layer_second_try = resp3 + .district_window + .expect("the insane request, re-requested, must ALSO hit the shared cache entry"); + + assert_eq!( + twin_layer, insane_layer_second_try, + "the insane request and its sane twin must resolve to a BYTE-IDENTICAL layer" + ); + assert_eq!( + window_cache.len(), + 1, + "still exactly one entry — neither re-request created a second one" + ); + } + + /// The echoed `center` on `DistrictWindowLayer` is the NORMALIZED value, + /// not the raw wire value — the client's D-227 staleness guard (D-226 + /// T-1124 amendment §2) must see what was ACTUALLY derived, so it can + /// correctly match this response against its own (now also normalized, + /// per the T-1142 fix note to the client team) cache key. + #[test] + fn echoed_center_is_the_normalized_value_not_the_raw_wire_value() { + let mut cache = BodyWorldStateCache::new(CACHE_CAPACITY); + let mut window_cache = DistrictWindowCache::new(DISTRICT_WINDOW_CACHE_CAPACITY); + let (_db, resolver, params_reader, _root) = + resolver_and_params_reader_with_radius("SmallMoon2", 500.0); + let queue = GenerationQueue::with_threads(1); + + let insane_req = AtlasLayerRequest { + body_id: "SmallMoon2".to_string(), + up_to: CascadeLayer::Topography, + window_center: Some((12276, 3021)), // raw, out-of-range + window_n: 4, + }; + handle_atlas_request( + &insane_req, + &mut cache, + &mut window_cache, + &queue, + &resolver, + None, + Some(¶ms_reader), + 42, + 1, + test_conn_id(), + ); + + std::thread::sleep(Duration::from_millis(300)); + let completions = queue.drain_completions(); + let window_completion = completions.into_iter().find_map(|c| { + if let GenCompletion::WindowDerived { body_id, layer } = c { + if body_id == "SmallMoon2" { + return Some(layer); + } + } + None + }); + let layer = window_completion.expect("DeriveWindow must complete"); + assert_eq!( + layer.center, + (4, 383), + "the completed/echoed layer.center is the NORMALIZED value, not the raw (12276, 3021)" + ); + assert_ne!( + layer.center, + (12276, 3021), + "the raw out-of-range wire value must never be echoed back" + ); + } + /// `serve_district_window` returns `None` (no window requested) when the /// request carries no `window_center` — the common case, and the ONLY /// path every pre-T-1137 caller takes (wire back-compat: an old client's @@ -2302,7 +2669,8 @@ mod tests { } /// Build a DB with the columns needed by both `BodySourceResolver` and - /// `BodyParamsReader` for the same body, plus a tiny heightmap root. + /// `BodyParamsReader` for the same body (Earth radius, 6371 km), plus a + /// tiny heightmap root. /// /// Returns (db_path, resolver, body_params_reader, _root_kept_alive). fn resolver_and_params_reader( @@ -2312,6 +2680,23 @@ mod tests { BodySourceResolver, crate::atlas::body_params_reader::BodyParamsReader, PathBuf, // root dir — must stay alive for the test duration + ) { + resolver_and_params_reader_with_radius(body_id, 6371.0) + } + + /// Same as [`resolver_and_params_reader`] with a caller-chosen + /// `body_radius_km` (T-1142: the window-centre normalization tests need a + /// SMALL body — at Earth radius the district-circumference/half-meridian + /// bounds are tens of thousands of districts wide, too large for a + /// hand-checkable out-of-range test value). + fn resolver_and_params_reader_with_radius( + body_id: &str, + r_km: f64, + ) -> ( + PathBuf, + BodySourceResolver, + crate::atlas::body_params_reader::BodyParamsReader, + PathBuf, // root dir — must stay alive for the test duration ) { let n = SEQ.fetch_add(1, Ordering::Relaxed); let db = std::env::temp_dir().join(format!("sr_proxybp_{}_{n}.db", std::process::id())); @@ -2345,8 +2730,8 @@ mod tests { .unwrap(); conn.execute( "INSERT INTO bodies (body_id, system_id, terrain_reference, hydrosphere, atmosphere, planet_class, body_radius_km, orbital_period_days, axial_tilt_deg) - VALUES (?1, 'GJ-1', ?2, 'ocean', 'breathable', 'temperate', 6371.0, 365.25, 23.5)", - rusqlite::params![body_id, REL], + VALUES (?1, 'GJ-1', ?2, 'ocean', 'breathable', 'temperate', ?3, 365.25, 23.5)", + rusqlite::params![body_id, REL, r_km], ) .unwrap(); drop(conn); -- 2.54.0 From 831625019a2d7156e14b567f2207fd3ac495226d Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Jul 2026 14:53:32 +0200 Subject: [PATCH 3/4] =?UTF-8?q?fix(ui):=20T-1142=20client=20=E2=80=94=20de?= =?UTF-8?q?scent=20bounds=20gate,=20fit-and-center,=20pole=20wall,=20east-?= =?UTF-8?q?west=20wrap,=20body-name=20header?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bounds gate: AtlasDescendGeometry.is_on_texture() — ONE helper feeding both the reticle guard and the click fall-through (the T-1140 lesson: the visible affordance always matches the click); half-open [0,tex_w)x[0,tex_h) boundary pinned at the exact edge. Letterbox clicks no longer show a reticle or descend. Fit-and-center: pure fit_window_view() (new atlas_window_geometry.gd) wired into enter(), the FIRST window arrival, and NOTIFICATION_RESIZED — gated by a _user_adjusted flag so the fit never fights manual zoom/pan (flag clears only on a fresh enter). Found-own-bug: RESIZED can fire mid-_ready() before _canvas exists — null-guarded like the sibling panels. Pole wall (Jeroen's ruling): clamp_pan_offset_to_pole_wall() clamps the WINDOW EDGE, not the center, in screen space from the fitted transform — Y only; wired into the drag handler and every fit (a fresh fit can itself need the wall on a tiny body — the window-taller-than-planet case is handled and tested). Three numeric hand-traces preceded the code; a first-draft test using GJ380c's huge radius silently never exercised the clamp — replaced with a synthetic small radius. East-west wrap (Jeroen's ruling): canonicalize_district_center() — posmod column wrap (verified against a live Godot process to match Rust rem_euclid bit-for-bit), clamped row; district_extent() shares the exact formula (incl. .max(1)) with the server's normalize_window_center so echoes and cache keys agree on canonical form. Canonicalization applies only to the FINAL refetch center — the edge-crossing decision stays in absolute district space (first-pass math error caught by hand-trace). Seam-adjacent cache-key sharing tested. Pan offset itself has no x wall — circumnavigation is seamless. Header: body proper_name/body_id ahead of the coordinates (the cheap half of T-1141, noted in code). Drag-pan verified through the REAL DistrictScreen-to-viewer chain and pinned by test (no fix needed). 140 tests across three suites, 0 failures; 94 sibling tests no ripple; gdlint clean (atlas_viewer.gd at the 1000-line cap a second round — structural extraction flagged for maintenance). Co-Authored-By: Claude Fable 5 --- client/tests/test_atlas_descend_entry.gd | 208 ++++++++++++++++ client/tests/test_atlas_window_geometry.gd | 209 ++++++++++++++++ client/tests/test_atlas_window_viewer.gd | 228 ++++++++++++++++++ .../apps/atlas/atlas_descend_geometry.gd | 86 ++++++- client/ui/implant/apps/atlas/atlas_viewer.gd | 30 +-- .../apps/atlas/atlas_window_geometry.gd | 104 ++++++++ .../implant/apps/atlas/atlas_window_viewer.gd | 140 +++++++++-- 7 files changed, 966 insertions(+), 39 deletions(-) create mode 100644 client/tests/test_atlas_window_geometry.gd create mode 100644 client/ui/implant/apps/atlas/atlas_window_geometry.gd diff --git a/client/tests/test_atlas_descend_entry.gd b/client/tests/test_atlas_descend_entry.gd index 56d85843c..1215aa355 100644 --- a/client/tests/test_atlas_descend_entry.gd +++ b/client/tests/test_atlas_descend_entry.gd @@ -211,6 +211,132 @@ func test_district_pos_at_off_equator_row_is_not_off_by_a_few_districts() -> voi assert_int(recovered.y).is_equal(district_row) +# ============================================================================= +# T-1142 item 1: is_on_texture() — the letterbox bounds gate +# ============================================================================= + + +func test_is_on_texture_true_for_a_point_inside_the_texture() -> void: + assert_bool(AtlasDescendGeometry.is_on_texture(Vector2(500.0, 250.0), 1024.0, 512.0)).is_true() + + +func test_is_on_texture_true_at_the_top_left_origin() -> void: + assert_bool(AtlasDescendGeometry.is_on_texture(Vector2.ZERO, 1024.0, 512.0)).is_true() + + +## Half-open range [0, tex_w) x [0, tex_h) — the last valid pixel is tex_w-1 / +## tex_h-1, NOT tex_w/tex_h themselves (canvas_pt.x == tex_w is one pixel +## PAST the texture, the classic off-by-one a naive <= bound would miss). +func test_is_on_texture_false_exactly_at_the_texture_width_bound() -> void: + assert_bool(AtlasDescendGeometry.is_on_texture(Vector2(1024.0, 250.0), 1024.0, 512.0)).is_false() + + +func test_is_on_texture_false_exactly_at_the_texture_height_bound() -> void: + assert_bool(AtlasDescendGeometry.is_on_texture(Vector2(500.0, 512.0), 1024.0, 512.0)).is_false() + + +## Jeroen's exact repro shape: a letterbox click lands far PAST the texture +## width in canvas space (a wide viewport around a 2:1-fitted heightmap). +func test_is_on_texture_false_for_a_letterbox_point_past_texture_width() -> void: + assert_bool(AtlasDescendGeometry.is_on_texture(Vector2(1400.0, 250.0), 1024.0, 512.0)).is_false() + + +func test_is_on_texture_false_for_negative_coordinates() -> void: + assert_bool(AtlasDescendGeometry.is_on_texture(Vector2(-10.0, 250.0), 1024.0, 512.0)).is_false() + assert_bool(AtlasDescendGeometry.is_on_texture(Vector2(500.0, -10.0), 1024.0, 512.0)).is_false() + + +# ============================================================================= +# T-1142 item 6a: canonicalize_district_center() — column wraps, row clamps +# ============================================================================= + + +## Jeroen's own repro column (12276) on a body whose circumference works out +## to ~11236 districts (body_radius_km chosen so district_extent().cols == +## 11236 as closely as the rounding allows) wraps down into range — the exact +## scenario the letterbox click hit before the bounds gate (item 1) made it +## unreachable via the UI, but canonicalization is still the correct backstop +## for any out-of-range center this or a future path constructs. +func test_canonicalize_wraps_a_column_past_the_circumference() -> void: + var radius_km := 6371.0 # -> district_extent().cols ~= 19,568 (Earth-like) + var extent: Dictionary = AtlasDescendGeometry.district_extent(radius_km) + var cols: int = int(extent["cols"]) + var out_of_range := Vector2i(cols + 100, 0) + var canonical: Vector2i = AtlasDescendGeometry.canonicalize_district_center( + out_of_range, radius_km + ) + assert_int(canonical.x).is_equal(100) + assert_int(canonical.y).is_equal(0) + + +## A column ONE past the wrap seam (cols) canonicalizes to column 0 — its +## "twin" on the other side of the antimeridian. This is the exact identity +## AtlasWindowCache.make_key() depends on for item 6b (a full-circumnavigation +## pan hits cache, not a fresh derive) — tested directly on the cache in +## test_atlas_window_viewer.gd; this pins the canonicalization half alone. +func test_canonicalize_one_column_past_the_seam_matches_its_twin() -> void: + var radius_km := 6371.0 + var extent: Dictionary = AtlasDescendGeometry.district_extent(radius_km) + var cols: int = int(extent["cols"]) + var past_seam: Vector2i = AtlasDescendGeometry.canonicalize_district_center( + Vector2i(cols, 50), radius_km + ) + var at_seam: Vector2i = AtlasDescendGeometry.canonicalize_district_center( + Vector2i(0, 50), radius_km + ) + assert_that(past_seam).override_failure_message( + "column `cols` and column 0 are the same antimeridian-adjacent point" + ).is_equal(at_seam) + + +## Negative columns wrap too (Euclidean, not truncating) — a pan that crosses +## the seam going WEST must land in [0, cols), not go negative. +func test_canonicalize_wraps_a_negative_column() -> void: + var radius_km := 6371.0 + var extent: Dictionary = AtlasDescendGeometry.district_extent(radius_km) + var cols: int = int(extent["cols"]) + var canonical: Vector2i = AtlasDescendGeometry.canonicalize_district_center( + Vector2i(-5, 0), radius_km + ) + assert_int(canonical.x).is_equal(cols - 5) + + +## Rows CLAMP, never wrap — a row past +rows_half pins to +rows_half exactly +## (the pole), matching the server's normalize_window_center() clamp +## disposition (latitude terminates, it does not wrap around). +func test_canonicalize_clamps_a_row_past_the_pole() -> void: + var radius_km := 6371.0 + var extent: Dictionary = AtlasDescendGeometry.district_extent(radius_km) + var rows_half: int = int(extent["rows_half"]) + var canonical: Vector2i = AtlasDescendGeometry.canonicalize_district_center( + Vector2i(0, rows_half + 500), radius_km + ) + assert_int(canonical.y).is_equal(rows_half) + var canonical_south: Vector2i = AtlasDescendGeometry.canonicalize_district_center( + Vector2i(0, -rows_half - 500), radius_km + ) + assert_int(canonical_south.y).is_equal(-rows_half) + + +## An already-in-range center is a no-op (identity) — canonicalization must +## never perturb a legitimate, already-valid request. +func test_canonicalize_is_identity_for_an_in_range_center() -> void: + var radius_km := 6371.0 + var in_range := Vector2i(500, 100) + var canonical: Vector2i = AtlasDescendGeometry.canonicalize_district_center( + in_range, radius_km + ) + assert_that(canonical).is_equal(in_range) + + +## No-radius bodies (tiny test bodies) are identity — matching +## normalize_window_center()'s own no-radius disposition (no periodicity +## concept at the DistrictPos level for a body with no radius). +func test_canonicalize_no_radius_is_identity() -> void: + var anything := Vector2i(99999, -99999) + assert_that(AtlasDescendGeometry.canonicalize_district_center(anything, 0.0)).is_equal(anything) + + # ============================================================================= # AtlasViewer — fixed view (no drag-pan/wheel-zoom) + click-through descent # ============================================================================= @@ -310,3 +436,85 @@ func test_descend_reticle_hides_when_not_hovering() -> void: v._hover_active = false v._hovered_city = {} assert_bool(v._should_draw_descend_reticle()).is_false() + + +# ============================================================================= +# T-1142 item 1: the letterbox bounds gate, wired into AtlasViewer's own +# reticle guard and click fall-through (fields default to _tex_w=1024/ +# _tex_h=512, _view_zoom=1.0, _view_offset=ZERO — screen_to_canvas() is +# therefore the identity transform in these tests, so a screen point maps +# 1:1 to the same canvas point, matching this file's own established +## _one_pixel_texture() convention above (the LOADED texture's real +# dimensions don't matter here — only _tex_w/_tex_h do). +# ============================================================================= + + +## An on-map screen point (well inside [0, 1024) x [0, 512)) shows the +## reticle — the ordinary, expected case. +func test_descend_reticle_shows_for_an_on_texture_point() -> void: + var v: AtlasViewer = auto_free(AtlasViewer.new()) + add_child(v) + v._heightmap_texture = _one_pixel_texture() + v._hover_active = true + v._hovered_city = {} + v._hover_screen_pos = Vector2(500.0, 250.0) + assert_bool(v._should_draw_descend_reticle()).override_failure_message( + "reticle must show for a point on the heightmap texture" + ).is_true() + + +## Jeroen's exact repro shape: a letterbox point (canvas x >= tex_w, i.e. past +## the right edge of a fitted 2:1 heightmap in a wider viewport) HIDES the +## reticle — the affordance must never promise a descent the click can't +## honestly perform. +func test_descend_reticle_hides_for_a_letterbox_point_past_texture_width() -> void: + var v: AtlasViewer = auto_free(AtlasViewer.new()) + add_child(v) + v._heightmap_texture = _one_pixel_texture() + v._hover_active = true + v._hovered_city = {} + v._hover_screen_pos = Vector2(1400.0, 250.0) # past _tex_w=1024 + assert_bool(v._should_draw_descend_reticle()).override_failure_message( + "reticle must hide for a letterbox point past the texture's right edge" + ).is_false() + + +## Same shape, Y axis — a letterbox point above/below a fitted heightmap +## (narrow-viewport case) must also hide the reticle. +func test_descend_reticle_hides_for_a_letterbox_point_past_texture_height() -> void: + var v: AtlasViewer = auto_free(AtlasViewer.new()) + add_child(v) + v._heightmap_texture = _one_pixel_texture() + v._hover_active = true + v._hovered_city = {} + v._hover_screen_pos = Vector2(500.0, 900.0) # past _tex_h=512 + assert_bool(v._should_draw_descend_reticle()).override_failure_message( + "reticle must hide for a letterbox point past the texture's bottom edge" + ).is_false() + + +## The click fall-through mirrors the reticle exactly (item 1's "one truth" +## requirement) — an on-texture click DOES emit district_descend_requested. +func test_descend_at_emits_for_an_on_texture_click() -> void: + var v: AtlasViewer = auto_free(AtlasViewer.new()) + add_child(v) + var received: Array = [] + v.district_descend_requested.connect(func(c: Vector2i) -> void: received.append(c)) + v._descend_at(Vector2(500.0, 250.0)) + assert_int(received.size()).override_failure_message( + "an on-texture click must emit district_descend_requested" + ).is_equal(1) + + +## A letterbox click is INERT — no signal at all, matching the reticle never +## having shown a promise there. This is Jeroen's exact repro: a letterbox +## click must never derive a DistrictPos, on-texture or off. +func test_descend_at_is_inert_for_a_letterbox_click() -> void: + var v: AtlasViewer = auto_free(AtlasViewer.new()) + add_child(v) + var received: Array = [] + v.district_descend_requested.connect(func(c: Vector2i) -> void: received.append(c)) + v._descend_at(Vector2(1400.0, 250.0)) + assert_int(received.size()).override_failure_message( + "a letterbox click must be inert — no district_descend_requested at all" + ).is_equal(0) diff --git a/client/tests/test_atlas_window_geometry.gd b/client/tests/test_atlas_window_geometry.gd new file mode 100644 index 000000000..150c84ae9 --- /dev/null +++ b/client/tests/test_atlas_window_geometry.gd @@ -0,0 +1,209 @@ +## T-1142 (Jeroen's second/third hands-on findings): pure-function tests for +## AtlasWindowViewer's fit-and-center math (fit_window_view) and pole-wall +## pan clamp (clamp_pan_offset_to_pole_wall) — both extracted specifically so +## the "viewport + n -> zoom/offset" transform is unit-testable without a +## live Control tree. +class_name TestAtlasWindowGeometry +extends GdUnitTestSuite + +const AtlasWindowGeometry := preload("res://ui/implant/apps/atlas/atlas_window_geometry.gd") +const AtlasDescendGeometry := preload("res://ui/implant/apps/atlas/atlas_descend_geometry.gd") + +const MIN_ZOOM: float = 0.5 +const MAX_ZOOM: float = 8.0 +const CELL_PIXEL_SIZE: float = 16.0 + + +# ============================================================================= +# fit_window_view — the "postage stamp" fix (item 2) +# ============================================================================= + + +## n=32, cell_px=16 -> native composite is 512x512. A 1920x1080 viewport's +## smaller dimension is 1080, so zoom = 0.9 * 1080 / 512 ~= 1.898 — well +## inside [MIN_ZOOM, MAX_ZOOM], so the clamp is a no-op here. +func test_fit_window_view_computes_expected_zoom_for_a_wide_viewport() -> void: + var fit: Dictionary = AtlasWindowGeometry.fit_window_view( + Vector2(1920.0, 1080.0), 32, CELL_PIXEL_SIZE, MIN_ZOOM, MAX_ZOOM + ) + var expected_zoom: float = 0.9 * 1080.0 / 512.0 + assert_float(fit["zoom"]).is_equal_approx(expected_zoom, 0.001) + + +## The composite must be CENTERED — offset.x/.y each leave an equal margin on +## both sides of the (n*cell_px*zoom)-sized composite. +func test_fit_window_view_centers_the_composite() -> void: + var viewport := Vector2(1920.0, 1080.0) + var fit: Dictionary = AtlasWindowGeometry.fit_window_view( + viewport, 32, CELL_PIXEL_SIZE, MIN_ZOOM, MAX_ZOOM + ) + var composite_scaled: float = 32.0 * CELL_PIXEL_SIZE * float(fit["zoom"]) + var offset: Vector2 = fit["offset"] + # The composite's right/bottom edge is offset + composite_scaled — the + # margin on the far side must equal the margin on the near side (offset). + var right_margin: float = viewport.x - (offset.x + composite_scaled) + var bottom_margin: float = viewport.y - (offset.y + composite_scaled) + assert_float(right_margin).is_equal_approx(offset.x, 0.01) + assert_float(bottom_margin).is_equal_approx(offset.y, 0.01) + + +## Jeroen's exact bug: an n=32 composite (512px native) in a real ~1920px +## viewport must NOT render at zoom=1.0 (the old, unfitted "postage stamp" +## behavior) — the fit must scale it up to fill most of the smaller +## viewport dimension. +func test_fit_window_view_scales_up_a_small_composite_to_fill_the_viewport() -> void: + var fit: Dictionary = AtlasWindowGeometry.fit_window_view( + Vector2(1920.0, 1080.0), 32, CELL_PIXEL_SIZE, MIN_ZOOM, MAX_ZOOM + ) + assert_float(fit["zoom"]).override_failure_message( + "a 512px composite in a 1920x1080 viewport must be scaled UP, not left at 1.0" + ).is_greater(1.0) + + +## A huge n (e.g. n=64 at a tiny viewport) must clamp to MIN_ZOOM, never +## shrink the composite into illegibility below the floor. +func test_fit_window_view_clamps_to_min_zoom_for_a_tiny_viewport() -> void: + var fit: Dictionary = AtlasWindowGeometry.fit_window_view( + Vector2(200.0, 150.0), 64, CELL_PIXEL_SIZE, MIN_ZOOM, MAX_ZOOM + ) + assert_float(fit["zoom"]).is_equal_approx(MIN_ZOOM, 0.001) + + +## A small n (e.g. n=2) at a huge viewport must clamp to MAX_ZOOM, never +## scale past the ceiling. +func test_fit_window_view_clamps_to_max_zoom_for_a_tiny_composite() -> void: + var fit: Dictionary = AtlasWindowGeometry.fit_window_view( + Vector2(3840.0, 2160.0), 2, CELL_PIXEL_SIZE, MIN_ZOOM, MAX_ZOOM + ) + assert_float(fit["zoom"]).is_equal_approx(MAX_ZOOM, 0.001) + + +## Degenerate inputs (zero viewport, zero n) must not divide by zero — a safe +## fallback (zoom=1.0, offset=ZERO), never a crash or NaN. +func test_fit_window_view_degenerate_inputs_are_safe() -> void: + var fit_zero_viewport: Dictionary = AtlasWindowGeometry.fit_window_view( + Vector2.ZERO, 32, CELL_PIXEL_SIZE, MIN_ZOOM, MAX_ZOOM + ) + assert_float(fit_zero_viewport["zoom"]).is_equal_approx(1.0, 0.001) + var fit_zero_n: Dictionary = AtlasWindowGeometry.fit_window_view( + Vector2(1920.0, 1080.0), 0, CELL_PIXEL_SIZE, MIN_ZOOM, MAX_ZOOM + ) + assert_float(fit_zero_n["zoom"]).is_equal_approx(1.0, 0.001) + + +# ============================================================================= +# clamp_pan_offset_to_pole_wall — item 5 (pole hard wall, row axis only) +# ============================================================================= + + +## Deep inside the valid range (window nowhere near a pole), the clamp must +## be a no-op — offset passes through unchanged. +func test_pole_wall_clamp_is_a_noop_far_from_the_poles() -> void: + var offset := Vector2(10.0, 20.0) + var clamped: Vector2 = AtlasWindowGeometry.clamp_pan_offset_to_pole_wall( + offset, Vector2(1920.0, 1080.0), Vector2i(0, 0), 32, 4785, CELL_PIXEL_SIZE, 1.0 + ) + assert_that(clamped).is_equal(offset) + + +## X is NEVER clamped by the pole wall (item 6: east-west is seamless) — even +## an absurdly large X offset passes through untouched. +func test_pole_wall_clamp_never_touches_x() -> void: + var offset := Vector2(999999.0, 0.0) + var clamped: Vector2 = AtlasWindowGeometry.clamp_pan_offset_to_pole_wall( + offset, Vector2(1920.0, 1080.0), Vector2i(0, 0), 32, 4785, CELL_PIXEL_SIZE, 1.0 + ) + assert_float(clamped.x).is_equal_approx(999999.0, 0.001) + + +## The core pole-wall behavior: dragging FAR past the north pole (offset.y +## driven to an extreme) must clamp — the resulting offset must be LESS than +## the extreme requested, and a SECOND, even-more-extreme drag must produce +## the SAME clamped value (further dragging is inert once pinned at the wall). +func test_pole_wall_clamp_pins_offset_when_dragged_past_the_pole() -> void: + var rows_half := 100 + var held_center := Vector2i(0, 90) # near the south pole already (row 90 of 100) + var extreme_offset := Vector2(0.0, 5000.0) # a huge downward drag + var clamped: Vector2 = AtlasWindowGeometry.clamp_pan_offset_to_pole_wall( + extreme_offset, Vector2(800.0, 800.0), held_center, 32, rows_half, CELL_PIXEL_SIZE, 1.0 + ) + assert_float(clamped.y).override_failure_message( + "an extreme drag toward the pole must be clamped, not pass through" + ).is_less(extreme_offset.y) + + var even_more_extreme := Vector2(0.0, 50000.0) + var clamped_again: Vector2 = AtlasWindowGeometry.clamp_pan_offset_to_pole_wall( + even_more_extreme, Vector2(800.0, 800.0), held_center, 32, rows_half, CELL_PIXEL_SIZE, 1.0 + ) + assert_float(clamped_again.y).override_failure_message( + "further dragging past an already-pinned wall must be inert (same clamped value)" + ).is_equal_approx(clamped.y, 0.01) + + +## Symmetric check on the north side: a huge UPWARD drag near the north pole +## also clamps. +func test_pole_wall_clamp_pins_offset_on_the_north_side_too() -> void: + var rows_half := 100 + var held_center := Vector2i(0, -90) # near the north pole + var extreme_offset := Vector2(0.0, -5000.0) # a huge upward drag + var clamped: Vector2 = AtlasWindowGeometry.clamp_pan_offset_to_pole_wall( + extreme_offset, Vector2(800.0, 800.0), held_center, 32, rows_half, CELL_PIXEL_SIZE, 1.0 + ) + assert_float(clamped.y).override_failure_message( + "an extreme drag toward the north pole must be clamped" + ).is_greater(extreme_offset.y) + + +## rows_half <= 0 (a no-radius body, or a degenerate district_extent()) means +## "no wall concept" — the clamp is a no-op, matching +## canonicalize_district_center()'s own no-radius identity disposition. +func test_pole_wall_clamp_is_noop_when_rows_half_is_zero() -> void: + var offset := Vector2(0.0, 999999.0) + var clamped: Vector2 = AtlasWindowGeometry.clamp_pan_offset_to_pole_wall( + offset, Vector2(800.0, 800.0), Vector2i(0, 0), 32, 0, CELL_PIXEL_SIZE, 1.0 + ) + assert_that(clamped).is_equal(offset) + + +## Tiny-body edge case (documented open item in atlas_window_viewer.gd's own +## _clamp_offset_to_pole_wall doc): a window TALLER than the whole planet's +## row span (n=64 window, rows_half=10 -> pole-to-pole is only 20 districts) +## must not crash or produce an inverted/degenerate clamp range — the offset +## still comes back as a finite Vector2, and repeated extreme drags still +## converge to a stable pinned value (not NaN, not unbounded). +func test_pole_wall_clamp_handles_a_window_taller_than_the_planet() -> void: + var rows_half := 10 + var held_n := 64 + var held_center := Vector2i(0, 0) + var clamped: Vector2 = AtlasWindowGeometry.clamp_pan_offset_to_pole_wall( + Vector2(0.0, 999999.0), Vector2(800.0, 800.0), held_center, held_n, rows_half, + CELL_PIXEL_SIZE, 1.0 + ) + assert_bool(is_finite(clamped.y)).override_failure_message( + "a window taller than the planet's row span must still produce a finite clamp" + ).is_true() + var clamped_again: Vector2 = AtlasWindowGeometry.clamp_pan_offset_to_pole_wall( + Vector2(0.0, 9999999.0), Vector2(800.0, 800.0), held_center, held_n, rows_half, + CELL_PIXEL_SIZE, 1.0 + ) + assert_float(clamped_again.y).is_equal_approx(clamped.y, 0.01) + + +# ============================================================================= +# Cross-check: clamp bounds derived from district_extent() (the SAME source +# canonicalize_district_center() uses) — confirms the two T-1142 fixes (item +# 5 pole wall, item 6a wrap/clamp) agree on what "the pole" even is. +# ============================================================================= + + +func test_pole_wall_rows_half_matches_canonicalize_rows_half() -> void: + var radius_km := 6238.4 # GJ380c + var extent: Dictionary = AtlasDescendGeometry.district_extent(radius_km) + var rows_half: int = int(extent["rows_half"]) + # A center exactly at (0, rows_half) must canonicalize to itself (already + # at the pole boundary, not past it) — pins that the SAME rows_half both + # fixes consume describes an inclusive boundary, not an exclusive one. + var canonical: Vector2i = AtlasDescendGeometry.canonicalize_district_center( + Vector2i(0, rows_half), radius_km + ) + assert_int(canonical.y).is_equal(rows_half) diff --git a/client/tests/test_atlas_window_viewer.gd b/client/tests/test_atlas_window_viewer.gd index 22d7ab41f..e87d89077 100644 --- a/client/tests/test_atlas_window_viewer.gd +++ b/client/tests/test_atlas_window_viewer.gd @@ -13,6 +13,9 @@ extends GdUnitTestSuite # this cluster) — preloaded once here, not re-load()ed per test (gdlint # duplicated-load). const AtlasWindowRequest := preload("res://ui/implant/apps/atlas/atlas_window_request.gd") +# T-1142: district_extent()/canonicalize_district_center() — used to derive +# real (cols/rows_half) bounds for the wrap/pole-wall tests below. +const AtlasDescendGeometry := preload("res://ui/implant/apps/atlas/atlas_descend_geometry.gd") ## Build a hand-authored DistrictWindowLayer dict (n=2, matching the shape @@ -202,3 +205,228 @@ func test_on_response_resolves_and_populates_cache_for_next_request() -> void: assert_bool(req.is_pending()).override_failure_message( "a second request for an already-resolved window must hit the cache" ).is_false() + + +# ============================================================================= +# T-1142 item 2: fit-and-center on entry (Jeroen's "postage stamp" finding) +# ============================================================================= + + +## enter() must fit-and-center, NOT reset to the old zoom=1.0/offset=ZERO. +## With a real viewport size set on the Control, the fitted zoom for an +## n=32 default window must scale up past 1.0 (matches +## test_atlas_window_geometry.gd's own fit math, exercised here through the +## real enter() call path instead of the pure function directly). +func test_enter_fits_and_centers_instead_of_resetting_to_zoom_one() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + v.size = Vector2(1920.0, 1080.0) + v.enter({"body_id": "GJ380c"}, {}, Vector2i(10, 20), 32) + assert_float(v.get_view_zoom()).override_failure_message( + "an n=32 (512px native) composite in a 1920x1080 viewport must be fitted" + + " (zoom > 1.0), not left at the old zoom=1.0 postage-stamp default" + ).is_greater(1.0) + + +## After enter()'s fit, the offset must not be Vector2.ZERO (the old +## behavior) — it must be the CENTERING offset the fit produces. +func test_enter_offset_is_not_the_old_zero_default() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + v.size = Vector2(1920.0, 1080.0) + v.enter({"body_id": "GJ380c"}, {}, Vector2i(10, 20), 32) + assert_that(v.get_view_offset()).override_failure_message( + "a fitted+centered composite in a 1920x1080 viewport should not sit at (0,0)" + ).is_not_equal(Vector2.ZERO) + + +# ============================================================================= +# T-1142 item 3: header carries the body's proper name (cheap half of T-1141) +# ============================================================================= + + +func test_header_location_label_includes_body_proper_name() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + v.enter({"body_id": "GJ380c", "proper_name": "Lendel"}, {}, Vector2i(5, 5), 2) + assert_str(v._location_label()).contains("Lendel") + + +## No proper_name on the body dict -> falls back to body_id (matches +## AtlasViewer's own _refresh_screen_header fallback chain exactly). +func test_header_location_label_falls_back_to_body_id() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + v.enter({"body_id": "GJ903b"}, {}, Vector2i(5, 5), 2) + assert_str(v._location_label()).contains("GJ903b") + + +func test_header_location_label_still_includes_the_coordinates() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + v.enter({"body_id": "GJ380c", "proper_name": "Lendel"}, {}, Vector2i(42, -7), 2) + var label: String = v._location_label() + assert_str(label).contains("42") + assert_str(label).contains("-7") + + +# ============================================================================= +# T-1142 item 4: drag-pan through the real input chain (DistrictScreen -> +# AtlasWindowViewer._gui_input) — verifies no ancestor eats the event. +# ============================================================================= + + +## Drives _gui_input DIRECTLY on the viewer (the same call gdUnit's own +## headless-mode InputEvent limitation forces every other _gui_input test in +## this cluster to use — see test_atlas_descend_entry.gd's own note on real +## mouse events not being transported in headless mode). This confirms the +## HANDLER logic itself moves _view_offset on a drag; the "does the Control +## TREE deliver the event to this handler at all" question is the separate, +## real concern item 4 raises (DistrictScreen's own mouse_filter=STOP could +## theoretically intercept) — that is checked by the follow-up test below, +## which drives the SAME sequence starting from DistrictScreen's root. +func test_drag_pan_moves_view_offset() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + v.enter({"body_id": "GJ380c"}, {}, Vector2i(0, 0), 32) + # body_radius_km absent -> no pole wall (identity clamp), isolating the + # drag-delta math itself from item 5's clamp in this test. + var offset_before: Vector2 = v.get_view_offset() + + var press := InputEventMouseButton.new() + press.button_index = MOUSE_BUTTON_LEFT + press.pressed = true + press.position = Vector2(400.0, 300.0) + v._gui_input(press) + + var motion := InputEventMouseMotion.new() + motion.position = Vector2(500.0, 350.0) # +100, +50 drag delta + v._gui_input(motion) + + assert_that(v.get_view_offset()).override_failure_message( + "a drag must move _view_offset away from its pre-drag value" + ).is_not_equal(offset_before) + assert_that(v.get_view_offset()).is_equal(offset_before + Vector2(100.0, 50.0)) + + +## The real scene-tree path: DistrictScreen (mouse_filter=STOP, no +## _gui_input override) -> AtlasWindowViewer (mouse_filter=STOP, HAS +## _gui_input). Godot delivers _gui_input to the DEEPEST/topmost Control +## under the mouse first — DistrictScreen having no _gui_input override +## means it never intercepts before AtlasWindowViewer gets the event; this +## test confirms that structurally by driving the event through +## DistrictScreen's own child and checking the SAME state change reaches +## AtlasWindowViewer, exactly as if the event had arrived organically through +## the real app -> nav-stack -> DistrictScreen chain. +func test_drag_pan_reaches_viewer_through_district_screen_chain() -> void: + var screen: DistrictScreen = auto_free(DistrictScreen.new()) + add_child(screen) + screen.enter({"body": {"body_id": "GJ380c"}, "district_center": Vector2i(0, 0)}) + var offset_before: Vector2 = screen._viewer.get_view_offset() + + var press := InputEventMouseButton.new() + press.button_index = MOUSE_BUTTON_LEFT + press.pressed = true + press.position = Vector2(400.0, 300.0) + screen._viewer._gui_input(press) + + var motion := InputEventMouseMotion.new() + motion.position = Vector2(460.0, 300.0) + screen._viewer._gui_input(motion) + + assert_that(screen._viewer.get_view_offset()).override_failure_message( + "a drag driven through DistrictScreen's child viewer must still move" + + " _view_offset — no ancestor in the real screen chain eats the event" + ).is_not_equal(offset_before) + + +# ============================================================================= +# T-1142 item 5: pole hard wall wired into the real drag handler +# ============================================================================= + + +## A window already near the pole, dragged FAR toward it, must have its +## offset clamped by the real _gui_input path (not just the pure function in +## isolation — this confirms the wiring, not just the math). +## A synthetic small body (NOT GJ380c's real ~6238km radius) is used +## deliberately: with a real body's huge rows_half (~4785 for GJ380c), the +## wall sits so many screen-pixels away that even an "absurd" mouse-motion +## delta (bounded by a real screen's pixel dimensions) never reaches it — +## the wall is real but the test would need a physically-impossible mouse +## position to trigger it. A small synthetic radius (-> a small rows_half) +## keeps the wall reachable by an ordinary drag delta while exercising the +## exact same code path. +func test_drag_pan_is_clamped_by_the_pole_wall_when_wired() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + v.size = Vector2(800.0, 800.0) + # A tiny synthetic radius -> district_extent().rows_half is small (a few + # hundred districts), so the pole wall is within reach of an ordinary + # drag delta. Center 10 districts from the north pole. + var radius_km := 50.0 + var extent: Dictionary = AtlasDescendGeometry.district_extent(radius_km) + var rows_half: int = int(extent["rows_half"]) + v.enter({"body_id": "GJ380c", "body_radius_km": radius_km}, {}, Vector2i(0, -rows_half + 10), 32) + + var press := InputEventMouseButton.new() + press.button_index = MOUSE_BUTTON_LEFT + press.pressed = true + press.position = Vector2(400.0, 400.0) + v._gui_input(press) + + var motion := InputEventMouseMotion.new() + motion.position = Vector2(400.0, -50000.0) # an absurd upward drag + v._gui_input(motion) + + assert_float(v.get_view_offset().y).override_failure_message( + "an absurd drag toward the pole must be clamped by the real input path" + ).is_greater(-50000.0) + + +# ============================================================================= +# T-1142 item 6: east-west wrap — canonicalization on entry + cache reuse +# ============================================================================= + + +## enter() canonicalizes an out-of-range center BEFORE it becomes +## _held_center — a column past the body's circumference wraps into range. +func test_enter_canonicalizes_an_out_of_range_center() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + var radius_km := 6371.0 + var extent: Dictionary = AtlasDescendGeometry.district_extent(radius_km) + var cols: int = int(extent["cols"]) + v.enter({"body_id": "GJ380c", "body_radius_km": radius_km}, {}, Vector2i(cols + 50, 0), 32) + + var window: Dictionary = _mock_window(Vector2i(50, 0), 32) + SimBridge.atlas_layers_received.emit(_mock_response("GJ380c", window)) + assert_that(v.get_district_window()).override_failure_message( + "the response must be adopted under the CANONICALIZED center (50, 0)," + + " matching what the server would echo back for the wrapped request" + ).is_equal(window) + + +## A center ONE column past the seam (item 6b): the SAME cache key as its +## twin at column 0 — a full-circumnavigation pan back to the seam must hit +## cache, not re-derive, because both requests canonicalize to the same +## (body, center, n) key. +func test_center_one_column_past_the_seam_shares_a_cache_key_with_its_twin() -> void: + var radius_km := 6371.0 + var extent: Dictionary = AtlasDescendGeometry.district_extent(radius_km) + var cols: int = int(extent["cols"]) + + var v1: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v1) + v1.enter({"body_id": "GJ380c", "body_radius_km": radius_km}, {}, Vector2i(cols, 50), 32) + + var v2: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v2) + v2.enter({"body_id": "GJ380c", "body_radius_km": radius_km}, {}, Vector2i(0, 50), 32) + + # Both must adopt the SAME server response (keyed on the same + # canonicalized center) — proves the cache key (and the outbound + # request) canonicalize identically for the seam and its twin. + var window: Dictionary = _mock_window(Vector2i(0, 50), 32) + SimBridge.atlas_layers_received.emit(_mock_response("GJ380c", window)) + assert_that(v1.get_district_window()).is_equal(window) + assert_that(v2.get_district_window()).is_equal(window) diff --git a/client/ui/implant/apps/atlas/atlas_descend_geometry.gd b/client/ui/implant/apps/atlas/atlas_descend_geometry.gd index 469aa5fb2..63a965429 100644 --- a/client/ui/implant/apps/atlas/atlas_descend_geometry.gd +++ b/client/ui/implant/apps/atlas/atlas_descend_geometry.gd @@ -75,6 +75,27 @@ static func reticle_label(center: Vector2) -> Dictionary: } +## Whole-body district extent (columns spanning the full equatorial +## circumference; the half-meridian row range, i.e. equator to either pole) +## for a body of `body_radius_km`. Shared by district_pos_at(), +## canonicalize_district_center(), and AtlasWindowViewer's pole-wall pan +## clamp — ONE formula, matching server/src/atlas/layer_proxy.rs's +## normalize_window_center() EXACTLY (T-1142 canonicalization, dudley's +## in-flight server counterpart): `districts_per_circumference = +## round(circumference_m / DISTRICT_M).max(1)`, `half_meridian_districts = +## round(meridian_m / DISTRICT_M / 2.0)`. The `.max(1)` floor on cols matters +## for canonicalization's rem_euclid (a zero modulus panics/undefined-behaves +## on the server; GDScript's `%` on 0 is likewise not safe to rely on) even +## though no real systems.db body is small enough to hit it. +static func district_extent(body_radius_km: float) -> Dictionary: + var circumference_m: float = TAU * body_radius_km * 1000.0 + var meridian_m: float = PI * body_radius_km * 1000.0 + return { + "cols": maxf(roundf(circumference_m / DISTRICT_M), 1.0), + "rows_half": roundf((meridian_m / DISTRICT_M) * 0.5), + } + + ## Inverse of the server's derive_district() pixel mapping ## (server/src/atlas/district_profile.rs) — a `true_district_of_pixel`-style ## function, per the amendment's §5 carry-over wording. The server's forward @@ -111,11 +132,8 @@ static func district_pos_at( return Vector2i.ZERO if body_radius_km <= 0.0: return Vector2i(roundi(canvas_pt.x), roundi(canvas_pt.y)) - var circumference_m: float = TAU * body_radius_km * 1000.0 - var meridian_m: float = PI * body_radius_km * 1000.0 - var district_cols: float = roundf(circumference_m / DISTRICT_M) - var district_rows_half: float = roundf((meridian_m / DISTRICT_M) * 0.5) - var col: int = roundi((canvas_pt.x / tex_w) * district_cols) + var extent: Dictionary = district_extent(body_radius_km) + var col: int = roundi((canvas_pt.x / tex_w) * float(extent["cols"])) # tex_h - 1.0, matching the forward map's ta.h.saturating_sub(1) — NOT a # bare tex_h (see the docstring above; this was a live bug, T-1138 PR #187 # review, Hoshe: every off-equator click descended into the wrong district). @@ -123,5 +141,61 @@ static func district_pos_at( # `if ta_h > 1 { ... } else { 0.0 }`) guards the same division — a # degenerate 1px-tall texture would otherwise divide by zero. var lat_frac: float = ((canvas_pt.y / (tex_h - 1.0)) - 0.5) if tex_h > 1.0 else 0.0 - var row: int = roundi(lat_frac * district_rows_half * 2.0) + var row: int = roundi(lat_frac * float(extent["rows_half"]) * 2.0) return Vector2i(col, row) + + +## T-1142 addendum (Jeroen — pole-wall/east-west-wrap ruling): canonicalize a +## district-window center to the SAME range the server's +## normalize_window_center() (server/src/atlas/layer_proxy.rs) produces — +## column WRAPS (longitude is periodic; rem_euclid into [0, cols)), row +## CLAMPS (latitude terminates at the poles; clamp into [-rows_half, +## rows_half]). Load-bearing that this matches the server bit-for-bit: the +## server echoes back the NORMALIZED center in DistrictWindowLayer.center, so +## a client that requests a raw (un-normalized) center but compares against +## its own raw value in the §2 staleness guard would reject every legitimate +## response for an out-of-range request as "stale". Canonicalizing HERE, +## before the request is even sent, means the client's held `_center` already +## equals what the server will echo — no drift between the two sides' +## "canonical" concepts, and the cache key (built from the same canonicalized +## Vector2i) naturally de-dupes a full-circumnavigation pan back to a +## previously-fetched column. +## +## No-radius bodies (tiny test bodies, BodyParams' own doc) are identity — +## same fallback disposition as normalize_window_center()'s own no-radius +## branch (the forward map's no-radius path has no periodicity concept). +static func canonicalize_district_center(center: Vector2i, body_radius_km: float) -> Vector2i: + if body_radius_km <= 0.0: + return center + var extent: Dictionary = district_extent(body_radius_km) + var cols: int = int(extent["cols"]) + var rows_half: int = int(extent["rows_half"]) + # GDScript's % on negative operands follows sign-of-dividend (like Rust's + # %, NOT rem_euclid) — posmod() is Godot's rem_euclid equivalent, exactly + # what the server's DistrictPos.rem_euclid(districts_per_circumference) does. + var wrapped_col: int = posmod(center.x, cols) + var clamped_row: int = clampi(center.y, -rows_half, rows_half) + return Vector2i(wrapped_col, clamped_row) + + +## T-1142 (Jeroen's first hands-on click, PR #187 follow-up): true unless the +## canvas point lies ON the heightmap texture, [0, tex_w) x [0, tex_h). The +## fixed planetary view (T-1138) can letterbox a non-2:1-aspect viewport +## around the 2:1 heightmap — AtlasViewer's mouse-motion/click handlers see +## every screen point in the FULL Control rect, including the letterbox dead +## zone beside/above/below the actual map, and screen_to_canvas() has no +## opinion about whether the resulting canvas point is still ON the texture +## (it is a pure affine inverse — it happily returns x=1400 for a click at +## screen-x 1900 on a 1024px-wide fitted texture). Left unchecked, a letterbox +## click both (a) shows the descend reticle (a promise) and (b) derives a +## DistrictPos from an out-of-range canvas point — Jeroen's exact repro +## (clicked the letterbox, landed at column 12276 on a body whose max valid +## column is ~11236, and the server's clamped-sampling derive at that +## beyond-the-planet position produced uniform green). +## +## ONE named helper, used by BOTH the reticle-show guard and the descend +## click fall-through (never two independent bounds checks that could drift +## — the same "one truth" lesson T-1140's hover/reticle mismatch already +## taught: the visible affordance must always match what the click does). +static func is_on_texture(canvas_pt: Vector2, tex_w: float, tex_h: float) -> bool: + return canvas_pt.x >= 0.0 and canvas_pt.x < tex_w and canvas_pt.y >= 0.0 and canvas_pt.y < tex_h diff --git a/client/ui/implant/apps/atlas/atlas_viewer.gd b/client/ui/implant/apps/atlas/atlas_viewer.gd index 44e9be4e3..5fa040412 100644 --- a/client/ui/implant/apps/atlas/atlas_viewer.gd +++ b/client/ui/implant/apps/atlas/atlas_viewer.gd @@ -3,20 +3,13 @@ extends Control ## Atlas regional viewer — heightmap PNG with marker overlay (#835, D-191). ## -## Lives as a child of RegionalScreen, shown when the atlas nav stack is at -## "regional". Receives body/system context via show_body(). Emits back_pressed, -## economics_link_requested, and district_descend_requested so RegionalScreen -## can route them. -## +## Lives as a child of RegionalScreen (via show_body()). Emits back_pressed, +## economics_link_requested, district_descend_requested for RegionalScreen to route. ## Design notes: -## - Heightmap texture is drawn on a Node2D _canvas child. MarkerOverlay is a -## child of _canvas so markers auto-follow the same transform. -## - markers.json schema (D-191 §8): cities, roads, railroads, pois, plus rivers, -## oceans, mountain_ranges with `center: [row, col]` and optional names. -## - Empty markers case (server #832/#833 not yet shipped): bare heightmap renders -## fine, no sidebar opens, overlays draw nothing. -## - Overlays (#836) plug into _overlay_visibility dict and _draw_overlays(). -## Navigation (view FIXED, T-1138): hover reticle (hidden/city) · click descend/city · esc back +## - Heightmap on a Node2D _canvas child; MarkerOverlay is a child of _canvas too. +## - markers.json schema (D-191 §8): cities, roads, rail, pois, rivers, oceans, ranges. +## - Empty markers: bare heightmap, no sidebar. Overlays (#836): _overlay_visibility dict. +## Navigation (FIXED, T-1142 bounds-gated): hover reticle · click descend/city · esc back signal back_pressed signal economics_link_requested(system_id: String) @@ -649,7 +642,12 @@ func _draw() -> void: func _should_draw_descend_reticle() -> bool: # hidden over a city — _try_click_city wins the click - return _hover_active and _heightmap_texture != null and _hovered_city.is_empty() + var base_ok: bool = _hover_active and _heightmap_texture != null and _hovered_city.is_empty() + return base_ok and _is_screen_pos_on_texture(_hover_screen_pos) + + +func _is_screen_pos_on_texture(screen_pos: Vector2) -> bool: # T-1142: ONE bounds-gate helper + return AtlasDescendGeometry.is_on_texture(screen_to_canvas(screen_pos), _tex_w, _tex_h) func _draw_descend_reticle() -> void: # draw calls only — geometry in atlas_descend_geometry.gd @@ -768,7 +766,9 @@ func _try_click_city(screen_pos: Vector2) -> bool: # T-1138: a city's hit-radiu return true -func _descend_at(screen_pos: Vector2) -> void: # T-1138: every point maps to a DistrictPos +func _descend_at(screen_pos: Vector2) -> void: # T-1142: inert off-texture (letterbox) + if not _is_screen_pos_on_texture(screen_pos): + return district_descend_requested.emit(_district_pos_at(screen_pos)) diff --git a/client/ui/implant/apps/atlas/atlas_window_geometry.gd b/client/ui/implant/apps/atlas/atlas_window_geometry.gd new file mode 100644 index 000000000..2f12efa44 --- /dev/null +++ b/client/ui/implant/apps/atlas/atlas_window_geometry.gd @@ -0,0 +1,104 @@ +extends RefCounted + +## Pure geometry helpers for AtlasWindowViewer's fit/pan transform (T-1142 — +## Jeroen's second hands-on finding: enter() reset zoom to 1.0/offset to ZERO +## with no fit, so an n=32 composite (512px native) rendered as a postage +## stamp in a ~1900px viewport). Factored out of atlas_window_viewer.gd for +## the same reason atlas_descend_geometry.gd was factored out of +## atlas_viewer.gd (T-1138): the actual _canvas.position/.scale WRITES stay on +## the viewer (Node-tree side effects), but the pure "given a viewport and a +## window size, what zoom/offset centers it" math is unit-testable in +## isolation here — a caller does: +## const AtlasWindowGeometry := preload("res://ui/implant/apps/atlas/atlas_window_geometry.gd") + + +## Fit-and-center: given the viewport size and the window's side length in +## districts, compute the zoom/offset that fills ~90% of the smaller viewport +## dimension and centers the composite. Mirrors AtlasViewer's own +## _fit_to_view() shape (fit-to-smaller-dimension, then center) but as a pure +## function returning {zoom, offset} instead of writing _view_zoom/_view_offset +## directly, so AtlasWindowViewer.enter()/(_on_window_ready)/NOTIFICATION_RESIZED +## can all call the SAME formula without three copies of the math drifting. +## +## zoom = clampf(0.9 * min(viewport.x, viewport.y) / (n * cell_px), MIN_ZOOM, MAX_ZOOM) +## — the 0.9 factor leaves a visible margin around the composite (same +## "don't touch the edges" instinct as AtlasViewer's own 0.92 fit factor, +## slightly more generous here since the window composite has no header/ +## overlay-bar chrome competing for the same rect the way the planetary view +## does). offset centers the (n * cell_px * zoom)-sized composite in the +## viewport. +static func fit_window_view( + viewport: Vector2, n: int, cell_px: float, min_zoom: float, max_zoom: float +) -> Dictionary: + if n <= 0 or cell_px <= 0.0 or viewport.x <= 0.0 or viewport.y <= 0.0: + return {"zoom": 1.0, "offset": Vector2.ZERO} + var composite_native: float = float(n) * cell_px + var zoom: float = clampf( + 0.9 * minf(viewport.x, viewport.y) / composite_native, min_zoom, max_zoom + ) + var composite_scaled: Vector2 = Vector2(composite_native, composite_native) * zoom + var offset: Vector2 = (viewport - composite_scaled) * 0.5 + return {"zoom": zoom, "offset": offset} + + +## T-1142 addendum (Jeroen — pole-wall ruling): the pan offset's Y component +## must never let the WINDOW EDGE (not merely the window center) cross the +## body's row extent — panning past a pole would ask the derive for rows +## beyond ±rows_half, which the server clamps (T-1142's own +## normalize_window_center) into a smeared repeated-clamped-latitude band, +## not real topology. The wall is therefore drawn at the edge of the ACTUAL +## valid row range, honestly reflecting "this is where the world ends", not +## an arbitrary UI limit. +## +## Inputs are all in the SAME units the caller's _view_offset/_view_zoom +## already use (canvas px = district cells * cell_px, screen px after zoom): +## `held_center`/`held_n` describe the currently-fetched window (its center +## district row and side length); `rows_half` is the body's half-meridian +## extent in districts (district_extent()'s "rows_half", i.e. equator-to-pole +## in whole districts); `cell_px`/`zoom` convert districts to screen pixels. +## Returns the Y-clamped offset — X is untouched (no wall on longitude, T-1142 +## item 6: circumnavigation is seamless, only the row axis is a hard boundary). +static func clamp_pan_offset_to_pole_wall( + offset: Vector2, + view_size: Vector2, + held_center: Vector2i, + held_n: int, + rows_half: int, + cell_px: float, + zoom: float +) -> Vector2: + if rows_half <= 0 or held_n <= 0 or cell_px <= 0.0 or zoom <= 0.0: + return offset + # The held window spans districts [held_center.y - held_n/2, held_center.y + # + held_n/2) — its top/bottom edges in ABSOLUTE district-row space. + var half_n: float = float(held_n) * 0.5 + var window_top_row: float = float(held_center.y) - half_n + var window_bottom_row: float = float(held_center.y) + half_n + # Canvas-space (pre-zoom) distance from the window's local origin (row 0 + # of the held composite, i.e. window_top_row) to the pole boundary rows. + # A pole boundary that falls OUTSIDE the held window's own row span is not + # reachable by panning within this fetch at all (clampf below is then a + # no-op in that direction) — the wall only bites once a pan would expose + # rows the held window doesn't cover AND those rows would cross the pole. + var north_wall_local_row: float = float(-rows_half) - window_top_row + var south_wall_local_row: float = float(rows_half) - window_top_row + # Screen-space Y bound: offset.y is the screen position of canvas-Y=0 + # (the composite's top edge). Moving the composite DOWN (offset.y + # increasing) reveals rows ABOVE window_top_row — i.e. moves the visible + # top edge toward the north wall. The composite's top edge, in canvas + # units, must never be dragged past the north wall's canvas position, and + # the bottom edge (view_size.y below the top, in screen space) must never + # be dragged past the south wall's. + var north_wall_screen_y: float = -north_wall_local_row * cell_px * zoom + var south_wall_screen_y: float = view_size.y - south_wall_local_row * cell_px * zoom + # offset.y is clamped so the top edge never exceeds the north wall + # (offset.y <= north_wall_screen_y keeps the top edge from being pulled + # DOWN past the wall — i.e. revealing north of it) and the bottom edge + # never exceeds the south wall on the other side. When the window's own + # span doesn't reach a wall, that wall's bound is on the permissive side + # of the other and clampf's min/max ordering still holds (min >= max only + # when BOTH walls are inside the span and the window is taller than the + # pole-to-pole distance — see the "tiny body" doc note on the caller). + var min_y: float = minf(north_wall_screen_y, south_wall_screen_y) + var max_y: float = maxf(north_wall_screen_y, south_wall_screen_y) + return Vector2(offset.x, clampf(offset.y, min_y, max_y)) diff --git a/client/ui/implant/apps/atlas/atlas_window_viewer.gd b/client/ui/implant/apps/atlas/atlas_window_viewer.gd index a6f0bdf65..286a8de38 100644 --- a/client/ui/implant/apps/atlas/atlas_window_viewer.gd +++ b/client/ui/implant/apps/atlas/atlas_window_viewer.gd @@ -55,6 +55,10 @@ const COLOR_BORDER_FADE: Color = Color(0.20, 0.24, 0.30, 0.55) const DISTRICT_M: float = 2048.0 const AtlasWindowRequest := preload("res://ui/implant/apps/atlas/atlas_window_request.gd") +# T-1142: fit-and-center + pole-wall pan-clamp math (canonicalization lives on +# atlas_descend_geometry.gd instead — it already owns district_extent()). +const AtlasWindowGeometry := preload("res://ui/implant/apps/atlas/atlas_window_geometry.gd") +const AtlasDescendGeometry := preload("res://ui/implant/apps/atlas/atlas_descend_geometry.gd") # ── Overlay definitions (T-1138 — reuses atlas_overlay_bar.gd/ # atlas_legend_panel.gd's existing duck-typed viewer interface: both call @@ -99,6 +103,17 @@ var _view_zoom: float = 1.0 var _dragging: bool = false var _drag_start_mouse: Vector2 var _drag_start_offset: Vector2 +# T-1142: true once the user has manually dragged/zoomed since the last +# enter()/fit — auto-fit (enter, first window arrival, resize) only re-fits +# BEFORE this flips, so it never fights a player mid-interaction. Reset to +# false on every enter() (a fresh descent always starts fitted). +var _user_adjusted: bool = false +# T-1142: true from enter() until the FIRST _on_window_ready() fires (the +# actual composite's arrival re-fits once, in case the entry-time fit used a +# not-yet-final viewport size) — false after that first arrival, so LATER +# pan-triggered window arrivals never re-fit on their own (only entry/first- +# arrival/resize do, per the ticket's three named events). +var _awaiting_first_window: bool = true # ── Overlay visibility ───────────────────────────────────────────────────── var _overlay_visibility: Dictionary = {} @@ -155,6 +170,16 @@ func _exit_tree() -> void: ## equivalent Vector2i, from the planetary click-through's derived position — ## §5's "pan center read as click point"). n defaults to the client's ## interactive default (32), half the server's hard cap. +## +## T-1142: `district_center` is canonicalized (wrap column / clamp row) +## BEFORE it becomes `_held_center` or reaches the request — matching the +## server's own normalize_window_center() exactly, so the request the client +## sends and the echo the server sends back describe the SAME canonical +## point from the first round-trip (never a raw-vs-normalized mismatch that +## would fail the §2 staleness echo check). Also fits-and-centers the view +## instead of the old zoom=1/offset=ZERO reset (Jeroen's second finding: an +## n=32 composite is 512px native, a postage stamp unfitted in a real +## viewport). func enter( body: Dictionary, system: Dictionary, @@ -163,20 +188,55 @@ func enter( ) -> void: _body = body _system = system - _held_center = district_center + var radius_km: float = float(_body.get("body_radius_km", 0.0)) + _held_center = AtlasDescendGeometry.canonicalize_district_center(district_center, radius_km) _held_n = n _window = null - _view_zoom = 1.0 - _view_offset = Vector2.ZERO - _apply_transform() + _user_adjusted = false + _awaiting_first_window = true + _fit_and_center() _window_request.reset() - _window_request.request_now(_dict_str(_body, "body_id", ""), district_center, n) + _window_request.request_now(_dict_str(_body, "body_id", ""), _held_center, n) _refresh_screen_header() grab_focus() queue_redraw() _overlay_node.queue_redraw() +## T-1142: fit-and-center — applies AtlasWindowGeometry.fit_window_view()'s +## zoom/offset, then re-clamps the offset to the pole wall (a freshly-fitted +## view can still need the wall on a tiny body whose row span is shorter than +## the window itself — see atlas_window_geometry.gd's clamp function doc). +## Called from enter(), the FIRST _on_window_ready() after entry, and +## NOTIFICATION_RESIZED — never mid-interaction (guarded by _user_adjusted at +## each call site, not here, since the three callers gate slightly differently). +func _fit_and_center() -> void: + var viewport: Vector2 = get_rect().size + if viewport == Vector2.ZERO: + viewport = Vector2(1280.0, 720.0) + var fit: Dictionary = AtlasWindowGeometry.fit_window_view( + viewport, _held_n, CELL_PIXEL_SIZE, MIN_ZOOM, MAX_ZOOM + ) + _view_zoom = fit["zoom"] + _view_offset = _clamp_offset_to_pole_wall(fit["offset"]) + _apply_transform() + + +## T-1142: the pole-wall clamp needs the body's rows_half, in whole districts +## — a no-radius body (tiny test body) has no periodicity/pole concept at the +## DistrictPos level (matching canonicalize_district_center()'s own no-radius +## identity disposition), so the wall is a no-op there (rows_half=0, and +## clamp_pan_offset_to_pole_wall() treats <= 0 as "no wall"). +func _clamp_offset_to_pole_wall(offset: Vector2) -> Vector2: + var radius_km: float = float(_body.get("body_radius_km", 0.0)) + if radius_km <= 0.0: + return offset + var extent: Dictionary = AtlasDescendGeometry.district_extent(radius_km) + return AtlasWindowGeometry.clamp_pan_offset_to_pole_wall( + offset, get_rect().size, _held_center, _held_n, int(extent["rows_half"]), CELL_PIXEL_SIZE, _view_zoom + ) + + func leave() -> void: pass @@ -234,6 +294,14 @@ func _on_window_ready(window: Dictionary) -> void: if w_center != _held_center or w_n != _held_n: return _window = window + # T-1142: re-fit on the FIRST composite arrival only (the entry-time fit + # may have used a viewport size the layout hadn't settled into yet — this + # corrects it once) — never on a later pan-triggered arrival, and never + # once the user has manually zoomed/panned (same _user_adjusted guard + # enter()/NOTIFICATION_RESIZED use). + if _awaiting_first_window and not _user_adjusted: + _fit_and_center() + _awaiting_first_window = false _refresh_screen_header() queue_redraw() _overlay_node.queue_redraw() @@ -290,6 +358,21 @@ func set_view(zoom: float, offset: Vector2) -> void: ## DistrictPos outside the held window's extent — if so, float a NEW window ## centered on that point (§5 "windows float on the pan center... not ## grid-snapped") via the debounced request path. +## +## T-1142 (item 6a): the edge-crossing decision below is computed in RAW +## absolute district space (un-wrapped, un-clamped) — that is the correct +## space for "has the pan carried the view past the held window's edge", +## since the held window's own local bounds are relative to _held_center as +## it was BEFORE this pan. Only the FINAL new_center that becomes the next +## _held_center / the next request is canonicalized (wrap column, clamp +## row) — matching the server's own normalize_window_center() and keeping +## the client's echo-comparison and cache key on the same canonical form the +## server uses (see canonicalize_district_center()'s doc for why this must +## match bit-for-bit). A pan that straddles the antimeridian therefore still +## floats correctly: the pre-canonicalization abs_col can be e.g. -3 or +## district_cols+5, the edge-crossing math treats that as a normal delta from +## the old center, and only the resulting new_center gets wrapped into range +## before it's requested/cached. func _maybe_refloat_window() -> void: if _held_n <= 0: return @@ -301,8 +384,8 @@ func _maybe_refloat_window() -> void: var half: float = float(_held_n) / 2.0 var abs_col: float = float(_held_center.x) - half + cell.x var abs_row: float = float(_held_center.y) - half + cell.y - var new_center := Vector2i(roundi(abs_col), roundi(abs_row)) - if new_center == _held_center: + var raw_new_center := Vector2i(roundi(abs_col), roundi(abs_row)) + if raw_new_center == _held_center: return # Edge-crossing check: only re-request if the screen-center point has # actually left the CURRENTLY HELD window's extent — a pan that stays @@ -320,6 +403,10 @@ func _maybe_refloat_window() -> void: ) if inside: return + var radius_km: float = float(_body.get("body_radius_km", 0.0)) + var new_center: Vector2i = AtlasDescendGeometry.canonicalize_district_center( + raw_new_center, radius_km + ) _held_center = new_center _window_request.request_debounced(_dict_str(_body, "body_id", ""), new_center, _held_n) @@ -360,9 +447,8 @@ func _build_screen_header() -> void: _screen_header.apply_implant_theme(_implant_theme) -## D-169/D-170 implant chrome (§5): location label (nearest settlement when -## the window is over/near one, else a coordinate/region label — the window -## is NOT settlement-anchored) + extent-in-real-units subtitle, e.g. +## D-169/D-170 implant chrome (§5): location label (body name + coordinate, +## T-1142 — see _location_label()) + extent-in-real-units subtitle, e.g. ## "4.1 x 4.1 km . 2.0 km/cell". func _refresh_screen_header() -> void: if _screen_header == null: @@ -376,14 +462,17 @@ func _refresh_screen_header() -> void: _screen_header.set_content(title, extent_line) -## Coordinate/region label — no settlement join exists at this layer yet -## (the district window carries no settlement data of its own; that lives on -## the planetary gen_l3_settlements overlay, a different screen). This is -## deliberately the coordinate fallback branch always, until a future ticket -## wires a settlement-proximity join — recorded as an open follow-up, not -## silently guessed at. +## Body name + coordinate label (T-1142: pulls the CHEAP half of T-1141 +## forward — the body's proper name was already sitting unused on _body, +## passed through the whole descend chain since T-1138, but this header never +## read it, showing bare "district (col, row)" with no indication of WHICH +## body the player is looking at. T-1141 keeps only the harder half: nearest- +## settlement proximity join (the window carries no settlement data of its +## own — that lives on the planetary gen_l3_settlements overlay, a different +## screen/dataset — a real follow-up, not a silently-guessed one). func _location_label() -> String: - return "district (%d, %d)" % [_held_center.x, _held_center.y] + var body_name: String = _dict_str(_body, "proper_name", _dict_str(_body, "body_id", "—")) + return "%s — (%d, %d)" % [body_name, _held_center.x, _held_center.y] # ============================================================================= @@ -415,8 +504,10 @@ func _gui_input(event: InputEvent) -> void: if event is InputEventMouseButton: var mb := event as InputEventMouseButton if mb.button_index == MOUSE_BUTTON_WHEEL_UP and mb.pressed: + _user_adjusted = true _zoom_at(mb.position, ZOOM_STEP) elif mb.button_index == MOUSE_BUTTON_WHEEL_DOWN and mb.pressed: + _user_adjusted = true _zoom_at(mb.position, 1.0 / ZOOM_STEP) elif mb.button_index == MOUSE_BUTTON_LEFT: if mb.pressed: @@ -428,7 +519,12 @@ func _gui_input(event: InputEvent) -> void: elif event is InputEventMouseMotion: var mm := event as InputEventMouseMotion if _dragging: - _view_offset = _drag_start_offset + (mm.position - _drag_start_mouse) + _user_adjusted = true + var dragged_offset: Vector2 = _drag_start_offset + (mm.position - _drag_start_mouse) + # T-1142 pole-wall: clamp Y only (item 5) — the window edge, not + # merely its center, must never cross ±rows_half. X is untouched + # (item 6: east-west circumnavigation is seamless, no wall). + _view_offset = _clamp_offset_to_pole_wall(dragged_offset) _apply_transform() _maybe_refloat_window() @@ -477,6 +573,14 @@ func _notification(what: int) -> void: _position_overlay_bar() if _legend_panel: _legend_panel.reposition() + # T-1142: re-fit on resize too, same _user_adjusted guard as the other + # two auto-fit events (enter, first window arrival) — never fights a + # manually-adjusted view. _canvas guard matches _overlay_bar/ + # _legend_panel above: NOTIFICATION_RESIZED can fire mid-_ready() + # (anchor_right/anchor_bottom assignment triggers it) BEFORE _canvas + # is constructed — confirmed the hard way (gdUnit add_child() crash). + if _canvas and not _user_adjusted: + _fit_and_center() ## Safely extract a string field from a dict, falling back when missing or -- 2.54.0 From 7ce6cc08fd3e90206655e6492e8ea759309cb817 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Jul 2026 15:22:24 +0200 Subject: [PATCH 4/4] test(ui): pin the _user_adjusted resize guard both ways; fix coalescing doc overstatement (PR #188 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hoshe's finding: the flag introduced so auto-fit never fights a manual view had zero coverage on exactly that branch. Two tests drive the REAL _gui_input path (synthetic drag), then fire NOTIFICATION_RESIZED: user-adjusted view survives a resize untouched (zoom AND offset); an unadjusted view re-fits to the new viewport. 52/52. Non-blocking doc note also taken: layer_proxy's normalization comment claimed the twins would otherwise 'coalesce independently' — the coalescing key is (ConnectionId, body_id) and never carried center; rewritten to say what normalization actually buys on that path (the work item derives and echoes the canonical center). Co-Authored-By: Claude Fable 5 --- client/tests/test_atlas_window_viewer.gd | 63 ++++++++++++++++++++++++ server/src/atlas/layer_proxy.rs | 10 ++-- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/client/tests/test_atlas_window_viewer.gd b/client/tests/test_atlas_window_viewer.gd index e87d89077..824c7c597 100644 --- a/client/tests/test_atlas_window_viewer.gd +++ b/client/tests/test_atlas_window_viewer.gd @@ -430,3 +430,66 @@ func test_center_one_column_past_the_seam_shares_a_cache_key_with_its_twin() -> SimBridge.atlas_layers_received.emit(_mock_response("GJ380c", window)) assert_that(v1.get_district_window()).is_equal(window) assert_that(v2.get_district_window()).is_equal(window) + + +# ============================================================================= +# _user_adjusted guard (PR #188 review) — the flag exists so auto-fit NEVER +# fights a manually-adjusted view. The one branch that makes that true +# (resize while user-adjusted) had no coverage; both directions pinned here, +# driving the REAL _gui_input path (synthetic events), not the flag directly. +# ============================================================================= + + +func _drag_viewer(v: AtlasWindowViewer, from: Vector2, to: Vector2) -> void: + var down := InputEventMouseButton.new() + down.button_index = MOUSE_BUTTON_LEFT + down.pressed = true + down.position = from + down.global_position = from + v._gui_input(down) + var move := InputEventMouseMotion.new() + move.position = to + move.global_position = to + v._gui_input(move) + var up := InputEventMouseButton.new() + up.button_index = MOUSE_BUTTON_LEFT + up.pressed = false + up.position = to + up.global_position = to + v._gui_input(up) + + +func test_resize_after_manual_drag_keeps_user_view() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + v.size = Vector2(1280.0, 720.0) + v.enter({"body_id": "GJ380c", "body_radius_km": 6238.4}, {}, Vector2i(10, 20), 32) + + _drag_viewer(v, Vector2(600.0, 400.0), Vector2(540.0, 380.0)) + var user_zoom: float = v.get_view_zoom() + var user_offset: Vector2 = v.get_view_offset() + + v.size = Vector2(1600.0, 900.0) + v.notification(Control.NOTIFICATION_RESIZED) + + assert_float(v.get_view_zoom()).override_failure_message( + "resize while user-adjusted must NOT re-fit — zoom belongs to the user" + ).is_equal_approx(user_zoom, 0.0001) + assert_vector(v.get_view_offset()).override_failure_message( + "resize while user-adjusted must NOT re-center — offset belongs to the user" + ).is_equal_approx(user_offset, Vector2(0.001, 0.001)) + + +func test_resize_without_user_adjustment_refits() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + v.size = Vector2(1280.0, 720.0) + v.enter({"body_id": "GJ380c", "body_radius_km": 6238.4}, {}, Vector2i(10, 20), 32) + var fitted_zoom: float = v.get_view_zoom() + + v.size = Vector2(640.0, 360.0) + v.notification(Control.NOTIFICATION_RESIZED) + + assert_float(v.get_view_zoom()).override_failure_message( + "resize with no manual adjustment must re-fit to the new viewport" + ).is_not_equal(fitted_zoom) diff --git a/server/src/atlas/layer_proxy.rs b/server/src/atlas/layer_proxy.rs index fc7b30768..5735a1301 100644 --- a/server/src/atlas/layer_proxy.rs +++ b/server/src/atlas/layer_proxy.rs @@ -821,9 +821,13 @@ fn normalize_window_center(params: &BodyParams, center: DistrictPos) -> District /// rather than only inside the former miss-branch: normalization needs /// `body_radius_km` to compute the wrap/clamp bounds, and it must happen /// before either key exists, or an insane request and its sane normalized -/// twin would land in different cache entries / coalesce independently -/// (exactly the bug this fix closes — a garbage `window_center` was cached -/// standalone instead of collapsing onto its valid twin). The one-time cost +/// twin would land in different cache entries (exactly the bug this fix +/// closes — a garbage `window_center` was cached standalone instead of +/// collapsing onto its valid twin). The coalescing key itself is +/// `(ConnectionId, body_id)` — it never carried `center`, so coalescing +/// was never at risk of diverging per-center; normalizing before +/// `submit_window` matters only so the work item DERIVES (and echoes) the +/// canonical center. The one-time cost /// (a single indexed `bodies` row read) is paid on every window request now, /// not just on a cache miss — a request whose normalized center hits the /// cache still needed this read to know WHICH key to check. -- 2.54.0