diff --git a/client/scripts/autoloads/sim_bridge.gd b/client/scripts/autoloads/sim_bridge.gd index d8cb8a641..649783e6c 100644 --- a/client/scripts/autoloads/sim_bridge.gd +++ b/client/scripts/autoloads/sim_bridge.gd @@ -434,21 +434,22 @@ func send_named_action(action_name: String, action_data: Variant = null) -> void ## whole-body-layer caller (show_body()'s existing request), so their wire ## traffic is byte-unchanged. ## -## window_granularity/window_min_wl_m (T-1150): struct/key plumbing for the -## zoom-ladder quarter rung — district (0/omitted) stays the default for -## every caller in this codebase today. +## window_min_wl_m (T-1150): struct/key plumbing for the octave cutoff — 0 +## (omitted) stays the default for every caller in this codebase today. ## -## window_granularity_v2 (T-1152/T-1153): the R5-redesigned string-tag -## granularity ("Quarter"/"District"/"Region") — the ONLY way to request the -## coarser-than-district Region rung the legacy u32 field cannot express. -## Empty string (omitted) is the default for every caller that doesn't pass -## it, byte-compatible with every pre-T-1152 request. +## window_granularity_v2 (T-1152/T-1153): the string-tag granularity +## ("Quarter"/"District"/"Region") — the way to request the coarser-than- +## district Region rung. Empty string (omitted) is the default for every +## caller that doesn't pass it, resolving to District server-side. +## +## T-1159: the legacy `window_granularity: int` parameter this function used +## to also forward is retired — see atlas_map_protocol.gd's +## encode_atlas_layer_request doc for the full rationale. func request_atlas_layers( body_id: String, up_to: String = "Topography", window_center: Variant = null, window_n: int = 0, - window_granularity: int = 0, window_min_wl_m: int = 0, window_granularity_v2: String = "" ) -> void: @@ -459,7 +460,6 @@ func request_atlas_layers( up_to, window_center, window_n, - window_granularity, window_min_wl_m, window_granularity_v2 ) diff --git a/client/scripts/protocol/atlas_map_protocol.gd b/client/scripts/protocol/atlas_map_protocol.gd index 43077a933..045f01b4f 100644 --- a/client/scripts/protocol/atlas_map_protocol.gd +++ b/client/scripts/protocol/atlas_map_protocol.gd @@ -32,29 +32,27 @@ class_name AtlasMapProtocol ## client-side default/cap constants (DISTRICT_WINDOW_DEFAULT_N/MAX_N) live on ## the regional-window viewer, not duplicated into the codec. ## -## `window_granularity`/`window_min_wl_m` (T-1150): the derivation-granularity -## axis (district=1/omitted vs. quarter=4) and the octave cutoff, in whole -## metres. Both OMITTED (not sent as 0) when at their default — this is -## struct/key plumbing only (T-1150 scope): no caller in this codebase -## requests quarter granularity yet (that's T-1153); this function just makes -## it possible to ask, byte-compatible with every existing caller that -## doesn't pass them. +## `window_min_wl_m` (T-1150): the octave cutoff, in whole metres. OMITTED +## (not sent as 0) when at its default — struct/key plumbing, byte-compatible +## with every existing caller that doesn't pass it. ## ## `window_granularity_v2` (T-1152, R5 redesign — see -## server/src/atlas/layer_proxy.rs's `WindowGranularity` doc): the ONLY way to -## express a coarser-than-district rung (`"Region"`) the legacy `u32` field -## cannot encode. A plain STRING variant tag ("Quarter" | "District" | +## server/src/atlas/layer_proxy.rs's `WindowGranularity` doc): the derivation +## granularity, a plain STRING variant tag ("Quarter" | "District" | ## "Region"), matching `RoadNodeKind`'s existing wire precedent on this same ## carrier (a bare `#[derive(Serialize, Deserialize)]` enum with no ## `#[serde(rename_all)]` — rmp_serde encodes the Rust variant NAME verbatim, ## not an integer discriminant). OMITTED (not sent as "") when ## `window_granularity_v2` is the empty string — `#[serde(default)]` on the -## Rust side decodes absence as `None`, falling back to the legacy `u32` -## field's `resolve_window_granularity_v2()` precedence rule (that field wins -## over the legacy one whenever present — see that Rust doc for the full -## precedence contract). Every pre-T-1152 caller (and every T-1150 caller that -## only ever sends `window_granularity`) omits this field entirely and stays -## byte-compatible. +## Rust side decodes absence as `None`, resolving to District (the default +## rung). +## +## **T-1159:** the legacy `window_granularity: int` field this codec used to +## also encode (T-1150's finer-than-district-only `u32` wire encoding) is +## retired — `window_granularity_v2` fully shadowed it since T-1152, and no +## caller in this codebase (nor any external client — single-repo +## client/server pair) ever sent it as anything but the byte-compatible +## default. ## ## **Quantization split (PR #191 review, Hoshe 1 / Tyre C3):** `window_min_wl_m` ## is sent HERE as a raw, unquantized value — this codec does NOT snap it to @@ -71,7 +69,6 @@ static func encode_atlas_layer_request( up_to: String = "Topography", window_center: Variant = null, window_n: int = 0, - window_granularity: int = 0, window_min_wl_m: int = 0, window_granularity_v2: String = "" ) -> PackedByteArray: @@ -80,8 +77,6 @@ static func encode_atlas_layer_request( var center: Vector2i = window_center msg["window_center"] = [center.x, center.y] msg["window_n"] = window_n - if window_granularity != 0: - msg["window_granularity"] = window_granularity if window_min_wl_m != 0: msg["window_min_wl_m"] = window_min_wl_m if not window_granularity_v2.is_empty(): diff --git a/client/scripts/protocol/protocol.gd b/client/scripts/protocol/protocol.gd index cfaed9a65..f84f54c11 100644 --- a/client/scripts/protocol/protocol.gd +++ b/client/scripts/protocol/protocol.gd @@ -786,17 +786,18 @@ static func encode_request_bookmark_catalog() -> PackedByteArray: ## windowed district-resolution regional-map query — see ## atlas_map_protocol.gd's encode_atlas_layer_request doc for the wire shape. ## Omitted callers (every whole-body-layer call site predating T-1138) are -## byte-unchanged. window_granularity/window_min_wl_m (T-1150): same -## byte-compatibility contract, see atlas_map_protocol.gd. -## window_granularity_v2 (T-1152): the R5-redesigned string-tag granularity -## ("Quarter"/"District"/"Region") — the only way to request the Region rung. -## Omitted (empty string) by every caller that doesn't pass it. +## byte-unchanged. window_min_wl_m (T-1150): same byte-compatibility +## contract, see atlas_map_protocol.gd. +## window_granularity_v2 (T-1152): the string-tag granularity +## ("Quarter"/"District"/"Region"). Omitted (empty string) by every caller +## that doesn't pass it, resolving to District server-side. T-1159: the +## legacy `window_granularity: int` parameter this used to also forward is +## retired — see atlas_map_protocol.gd's encode_atlas_layer_request doc. static func encode_atlas_layer_request( body_id: String, up_to: String = "Topography", window_center: Variant = null, window_n: int = 0, - window_granularity: int = 0, window_min_wl_m: int = 0, window_granularity_v2: String = "" ) -> PackedByteArray: @@ -806,7 +807,6 @@ static func encode_atlas_layer_request( up_to, window_center, window_n, - window_granularity, window_min_wl_m, window_granularity_v2 ) diff --git a/client/tests/fixtures/msgpack/atlas_response_ready.msgpack b/client/tests/fixtures/msgpack/atlas_response_ready.msgpack index e54a67dfa..0f90b6f70 100644 Binary files a/client/tests/fixtures/msgpack/atlas_response_ready.msgpack and b/client/tests/fixtures/msgpack/atlas_response_ready.msgpack differ diff --git a/client/tests/fixtures/msgpack/atlas_response_ready_with_window.msgpack b/client/tests/fixtures/msgpack/atlas_response_ready_with_window.msgpack index 90ecca37d..b3b04bc84 100644 Binary files a/client/tests/fixtures/msgpack/atlas_response_ready_with_window.msgpack and b/client/tests/fixtures/msgpack/atlas_response_ready_with_window.msgpack differ diff --git a/client/tests/test_atlas_data_delivery.gd b/client/tests/test_atlas_data_delivery.gd index e1dd4fb15..017841116 100644 --- a/client/tests/test_atlas_data_delivery.gd +++ b/client/tests/test_atlas_data_delivery.gd @@ -214,35 +214,32 @@ func test_encode_atlas_layer_request_carries_window_params() -> void: assert_that(decoded.value.get("window_n")).is_equal(32) -## T-1150: window_granularity/window_min_wl_m are OMITTED (not sent as 0) -## when at their default — a windowed request that doesn't pass them (every -## pre-T-1150 window caller) is byte-identical to pre-T-1150 wire traffic, -## same contract as window_center/window_n's own default-omission above. -func test_encode_atlas_layer_request_omits_granularity_and_min_wl_by_default() -> void: +## T-1150: window_min_wl_m is OMITTED (not sent as 0) when at its default — +## a windowed request that doesn't pass it (every pre-T-1150 window caller) +## is byte-identical to pre-T-1150 wire traffic, same contract as +## window_center/window_n's own default-omission above. +func test_encode_atlas_layer_request_omits_min_wl_by_default() -> void: var bytes := Protocol.encode_atlas_layer_request( "GJ1c", "Topography", Vector2i(140, 260), 32 ) var decoded = Messagepack.decode(bytes) - assert_bool(decoded.value.has("window_granularity")).is_false() assert_bool(decoded.value.has("window_min_wl_m")).is_false() -## T-1150: a quarter-granularity request with an octave cutoff carries both -## new fields verbatim, unclamped (the server owns -## resolve_window_granularity()/clamp_window_n() — never trusted from the -## wire, same posture as window_n). -func test_encode_atlas_layer_request_carries_granularity_and_min_wl() -> void: +## T-1150: a request with an octave cutoff carries it verbatim, unclamped +## (the server owns quantize_min_wl_m() — never trusted from the wire, same +## posture as window_n). +func test_encode_atlas_layer_request_carries_min_wl() -> void: var bytes := Protocol.encode_atlas_layer_request( - "GJ1c", "Topography", Vector2i(140, 260), 32, 4, 512 + "GJ1c", "Topography", Vector2i(140, 260), 32, 512 ) var decoded = Messagepack.decode(bytes) - assert_that(decoded.value.get("window_granularity")).is_equal(4) assert_that(decoded.value.get("window_min_wl_m")).is_equal(512) ## T-1152/T-1153: window_granularity_v2 is OMITTED (not sent as "") when at ## its empty-string default — same byte-compatibility contract as -## window_granularity/window_min_wl_m's own default-omission. +## window_min_wl_m's own default-omission. func test_encode_atlas_layer_request_omits_granularity_v2_by_default() -> void: var bytes := Protocol.encode_atlas_layer_request( "GJ1c", "Topography", Vector2i(140, 260), 32 @@ -252,22 +249,20 @@ func test_encode_atlas_layer_request_omits_granularity_v2_by_default() -> void: ## T-1152/T-1153: a Region-rung request carries window_granularity_v2 as the -## bare string "Region" — the ONLY way to express the coarser-than-district -## rung (WindowGranularity's Rust doc: "the ONLY way to actually request -## Region is window_granularity_v2 = Some(WindowGranularity::Region)"), a -## plain rmp_serde variant-name encoding matching RoadNodeKind's existing -## wire precedent, NOT an integer discriminant. +## bare string "Region" — the way to express the coarser-than-district rung +## (WindowGranularity's Rust doc), a plain rmp_serde variant-name encoding +## matching RoadNodeKind's existing wire precedent, NOT an integer +## discriminant. +## +## T-1159: the legacy `window_granularity: int` param this call used to also +## pass positionally (between window_n and window_min_wl_m) is retired — see +## atlas_map_protocol.gd's encode_atlas_layer_request doc. func test_encode_atlas_layer_request_carries_granularity_v2_region() -> void: var bytes := Protocol.encode_atlas_layer_request( - "GJ1c", "Topography", Vector2i(0, 0), 6400, 0, 0, "Region" + "GJ1c", "Topography", Vector2i(0, 0), 6400, 0, "Region" ) var decoded = Messagepack.decode(bytes) assert_that(decoded.value.get("window_granularity_v2")).is_equal("Region") - # The legacy window_granularity field is independently omittable — a - # Region request sends ONLY the v2 tag, never a legacy value pretending - # to mean something for Region (WINDOW_GRANULARITY_REGION_KEY is a - # key-space tag the SERVER echoes, never a legal wire INPUT). - assert_bool(decoded.value.has("window_granularity")).is_false() ## §2: district_window is a distinct payload (echoes center/n for the @@ -280,7 +275,7 @@ func test_atlas_response_district_window_passthrough() -> void: var window := { "center": [140, 260], "n": 32, - "granularity": 4, # T-1150: quarter granularity, passed through same as every other field + "granularity_v2": "Quarter", "min_wl_m": 512, "morphology": PackedByteArray([8, 14, 0, 5]), "elev_q": PackedByteArray([40, 62, 5, 88]), @@ -307,7 +302,6 @@ func test_atlas_response_district_window_passthrough_region_rung() -> void: var window := { "center": [0, 0], "n": 6400, - "granularity": 4294967295, # WINDOW_GRANULARITY_REGION_KEY (u32::MAX) — key-space tag, not a real multiplier "granularity_v2": "Region", "min_wl_m": 0, "morphology": PackedByteArray([8, 14, 0, 5]), diff --git a/client/tests/test_atlas_window_cache.gd b/client/tests/test_atlas_window_cache.gd index af5294d7c..bd8425b8a 100644 --- a/client/tests/test_atlas_window_cache.gd +++ b/client/tests/test_atlas_window_cache.gd @@ -122,100 +122,58 @@ func test_clear_empties_the_cache() -> void: # ============================================================================= -# granularity / min_wl_m (T-1150, zoom ladder design doc §3 aliasing risk) +# min_wl_m (T-1150, zoom ladder design doc §3 aliasing risk) # ============================================================================= -## **MANDATORY aliasing regression (client half, T-1150):** a granularity-4 -## (quarter) key and a granularity-1 (district) key at the IDENTICAL -## (body_id, center, n) must be DISTINCT cache keys — this is what prevents a -## quarter-spacing request from silently reading (or overwriting) a -## district-spacing window's cache entry, and vice versa. -func test_make_key_distinguishes_granularity_at_identical_body_center_n() -> void: - var k_district := AtlasWindowCache.make_key( - "GJ1c", Vector2i(10, 20), 32, AtlasWindowCache.DISTRICT_GRANULARITY - ) - var k_quarter := AtlasWindowCache.make_key("GJ1c", Vector2i(10, 20), 32, 4) - assert_str(k_district).is_not_equal(k_quarter) - - -## Same aliasing risk, the other new axis: two requests identical except for -## `min_wl_m` (the octave cutoff) must not collide either — different cutoffs -## are different derived payloads (T-1149/T-1150). -func test_make_key_distinguishes_min_wl_m_at_identical_body_center_n_granularity() -> void: - var k_uncut := AtlasWindowCache.make_key( - "GJ1c", Vector2i(10, 20), 32, AtlasWindowCache.DISTRICT_GRANULARITY, 0 - ) - var k_cut := AtlasWindowCache.make_key( - "GJ1c", Vector2i(10, 20), 32, AtlasWindowCache.DISTRICT_GRANULARITY, 512 - ) +## **MANDATORY aliasing regression (client half, T-1150):** two requests +## identical except for `min_wl_m` (the octave cutoff) must not collide — +## different cutoffs are different derived payloads (T-1149/T-1150). +func test_make_key_distinguishes_min_wl_m_at_identical_body_center_n() -> void: + var k_uncut := AtlasWindowCache.make_key("GJ1c", Vector2i(10, 20), 32, 0) + var k_cut := AtlasWindowCache.make_key("GJ1c", Vector2i(10, 20), 32, 512) assert_str(k_uncut).is_not_equal(k_cut) -## Omitting granularity/min_wl_m (every pre-T-1150 call site) must produce the -## SAME key as passing the explicit district/no-cutoff defaults — byte/string -## compatibility for existing callers, not just "doesn't crash". -func test_omitted_granularity_and_min_wl_m_match_explicit_district_defaults() -> void: +## Omitting min_wl_m (every pre-T-1150 call site) must produce the SAME key +## as passing the explicit no-cutoff default — byte/string compatibility for +## existing callers, not just "doesn't crash". +func test_omitted_min_wl_m_matches_explicit_default() -> void: var k_omitted := AtlasWindowCache.make_key("GJ1c", Vector2i(10, 20), 32) - var k_explicit := AtlasWindowCache.make_key( - "GJ1c", Vector2i(10, 20), 32, AtlasWindowCache.DISTRICT_GRANULARITY, 0 - ) + var k_explicit := AtlasWindowCache.make_key("GJ1c", Vector2i(10, 20), 32, 0) assert_str(k_omitted).is_equal(k_explicit) -## End-to-end through put()/get_window()/has() (not just make_key() in -## isolation): a quarter-granularity window and a district-granularity window -## at the identical (body, center, n) must both be independently retrievable, -## neither one clobbering or masking the other. -func test_district_and_quarter_windows_coexist_at_identical_body_center_n() -> void: - var cache := AtlasWindowCache.new() - var district_window := {"granularity": AtlasWindowCache.DISTRICT_GRANULARITY, "id": "district"} - var quarter_window := {"granularity": 4, "id": "quarter"} - - cache.put("GJ1c", Vector2i(10, 20), 32, district_window, AtlasWindowCache.DISTRICT_GRANULARITY) - cache.put("GJ1c", Vector2i(10, 20), 32, quarter_window, 4) - - assert_int(cache.size()).is_equal(2) - assert_that( - cache.get_window("GJ1c", Vector2i(10, 20), 32, AtlasWindowCache.DISTRICT_GRANULARITY) - ).is_equal(district_window) - assert_that(cache.get_window("GJ1c", Vector2i(10, 20), 32, 4)).is_equal(quarter_window) - - # ============================================================================= # granularity_v2 (T-1152/T-1153): the string-tag axis — the ONLY thing that -# distinguishes Region from District/Quarter, since Region has no legal -# legacy-int representation (WindowGranularity::legacy_u32() returns None for -# Region — see the server's own doc). This is the SAME mandatory-aliasing -# regression class as the granularity/min_wl_m tests above, extended to the -# new axis. +# distinguishes Region from District/Quarter (WindowGranularity has no +# integer representation at all — see the server's own doc). This is the +# SAME mandatory-aliasing regression class as the min_wl_m tests above, +# extended to the new axis. +# +# T-1159: these tests used to also carry a legacy int `granularity` +# positional argument (mirroring the server's now-retired +# `window_granularity: u32` wire field) — removed along with the cache's own +# legacy key component (see atlas_window_cache.gd's doc). # ============================================================================= ## **MANDATORY aliasing regression (T-1152/T-1153):** a Region-rung key and a -## District-rung key at the IDENTICAL (body_id, center, n, legacy -## granularity, min_wl_m) must be DISTINCT cache keys — the legacy int slot -## alone (both "District" and default-omitted callers pass -## DISTRICT_GRANULARITY=1) cannot tell them apart; granularity_v2 is what -## does. +## District-rung key at the IDENTICAL (body_id, center, n, min_wl_m) must be +## DISTINCT cache keys — granularity_v2 is the only axis that can tell them +## apart. func test_make_key_distinguishes_granularity_v2_region_from_district() -> void: - var k_district := AtlasWindowCache.make_key( - "GJ1c", Vector2i(10, 20), 32, AtlasWindowCache.DISTRICT_GRANULARITY, 0, "District" - ) - var k_region := AtlasWindowCache.make_key( - "GJ1c", Vector2i(10, 20), 32, AtlasWindowCache.DISTRICT_GRANULARITY, 0, "Region" - ) + var k_district := AtlasWindowCache.make_key("GJ1c", Vector2i(10, 20), 32, 0, "District") + var k_region := AtlasWindowCache.make_key("GJ1c", Vector2i(10, 20), 32, 0, "Region") assert_str(k_district).is_not_equal(k_region) ## Omitting granularity_v2 (every pre-T-1152 call site) must produce the SAME ## key as passing the explicit "District" default — byte/string -## compatibility, same contract as the legacy granularity/min_wl_m defaults. +## compatibility, same contract as the min_wl_m default above. func test_omitted_granularity_v2_matches_explicit_district_default() -> void: var k_omitted := AtlasWindowCache.make_key("GJ1c", Vector2i(10, 20), 32) - var k_explicit := AtlasWindowCache.make_key( - "GJ1c", Vector2i(10, 20), 32, AtlasWindowCache.DISTRICT_GRANULARITY, 0, "District" - ) + var k_explicit := AtlasWindowCache.make_key("GJ1c", Vector2i(10, 20), 32, 0, "District") assert_str(k_omitted).is_equal(k_explicit) @@ -229,23 +187,13 @@ func test_region_and_district_windows_coexist_at_identical_body_center_n() -> vo var district_window := {"granularity_v2": "District", "id": "district"} var region_window := {"granularity_v2": "Region", "id": "region"} - cache.put( - "GJ1c", Vector2i(10, 20), 32, district_window, AtlasWindowCache.DISTRICT_GRANULARITY, 0, - "District" - ) - cache.put( - "GJ1c", Vector2i(10, 20), 32, region_window, AtlasWindowCache.DISTRICT_GRANULARITY, 0, - "Region" - ) + cache.put("GJ1c", Vector2i(10, 20), 32, district_window, 0, "District") + cache.put("GJ1c", Vector2i(10, 20), 32, region_window, 0, "Region") assert_int(cache.size()).is_equal(2) assert_that( - cache.get_window( - "GJ1c", Vector2i(10, 20), 32, AtlasWindowCache.DISTRICT_GRANULARITY, 0, "District" - ) + cache.get_window("GJ1c", Vector2i(10, 20), 32, 0, "District") ).is_equal(district_window) assert_that( - cache.get_window( - "GJ1c", Vector2i(10, 20), 32, AtlasWindowCache.DISTRICT_GRANULARITY, 0, "Region" - ) + cache.get_window("GJ1c", Vector2i(10, 20), 32, 0, "Region") ).is_equal(region_window) diff --git a/client/tests/test_protocol.gd b/client/tests/test_protocol.gd index c8d3e8d59..014d1e85b 100644 --- a/client/tests/test_protocol.gd +++ b/client/tests/test_protocol.gd @@ -512,13 +512,17 @@ func test_decode_atlas_response_not_found() -> void: ## PR #191 review, Hoshe 3: `atlas_response_ready_with_window.msgpack` had NO -## consumer anywhere in client/tests — regenerated by the T-1150 `granularity`/ -## `min_wl_m` field additions but nothing decoded it through the real IPC path. -## This is that consumer, matching the sibling `test_decode_atlas_response_*` -## tests' style/fixture-dir convention above: full decode_atlas_layer_response() +## consumer anywhere in client/tests — regenerated by the T-1150 `min_wl_m` +## field addition but nothing decoded it through the real IPC path. This is +## that consumer, matching the sibling `test_decode_atlas_response_*` tests' +## style/fixture-dir convention above: full decode_atlas_layer_response() ## round trip (not a hand-built Dictionary like test_atlas_data_delivery.gd's -## passthrough tests), confirming `district_window.granularity`/`.min_wl_m` -## (T-1150's two new echo fields) survive the real client decode path. +## passthrough tests), confirming `district_window.granularity_v2`/`.min_wl_m` +## survive the real client decode path. +## +## T-1159: the legacy `district_window.granularity` int echo this test used +## to also assert is retired server-side — see +## server/src/atlas/layer_proxy.rs's `DistrictWindowLayer` doc. func test_decode_atlas_response_ready_with_window() -> void: var bytes := _load_fixture("atlas_response_ready_with_window") var resp = Protocol.decode_atlas_layer_response(bytes) @@ -528,7 +532,7 @@ func test_decode_atlas_response_ready_with_window() -> void: var window: Dictionary = resp.district_window assert_that(window.get("center")).is_equal([10, -5]) assert_that(int(window.get("n"))).is_equal(2) - assert_that(int(window.get("granularity"))).is_equal(1) + assert_that(str(window.get("granularity_v2"))).is_equal("District") assert_that(int(window.get("min_wl_m"))).is_equal(0) diff --git a/client/ui/implant/apps/atlas/atlas_window_cache.gd b/client/ui/implant/apps/atlas/atlas_window_cache.gd index 9c379b98f..b523e9b5f 100644 --- a/client/ui/implant/apps/atlas/atlas_window_cache.gd +++ b/client/ui/implant/apps/atlas/atlas_window_cache.gd @@ -2,27 +2,30 @@ extends RefCounted ## Client-side LRU cache for DistrictWindowLayer responses (T-1138, D-226 ## T-1124 amendment §4 "Client cache policy"; extended T-1150 for the -## granularity/min_wl axes; extended T-1152/T-1153 for the granularity_v2 -## string-tag axis that makes Region representable at all). +## min_wl axis; extended T-1152/T-1153 for the granularity_v2 string-tag +## axis that makes Region representable at all). ## -## Keyed on (body_id, center, n, granularity, min_wl_m, granularity_v2) — -## D-227's determinism guarantee (same seed + body + position + derivation -## params -> same derived output, always) means a previously-fetched window is -## valid FOREVER for that body+seed. This is an LRU-evict-only cache: no -## freshness check, no TTL, no invalidation path at all. The only reason an -## entry ever leaves is capacity pressure. +## Keyed on (body_id, center, n, min_wl_m, granularity_v2) — D-227's +## determinism guarantee (same seed + body + position + derivation params -> +## same derived output, always) means a previously-fetched window is valid +## FOREVER for that body+seed. This is an LRU-evict-only cache: no freshness +## check, no TTL, no invalidation path at all. The only reason an entry ever +## leaves is capacity pressure. ## -## granularity/min_wl_m/granularity_v2 default to -## DISTRICT_GRANULARITY/0/DEFAULT_GRANULARITY_V2 ("District", district -## spacing, no octave cutoff) — every pre-T-1150 caller that doesn't pass them -## keeps its existing key shape and cache behavior unchanged. This is the -## client half of the mandatory aliasing fix (T-1150 design doc §3, extended +## min_wl_m/granularity_v2 default to 0/DEFAULT_GRANULARITY_V2 ("District", +## no octave cutoff) — every pre-T-1150 caller that doesn't pass them keeps +## its existing key shape and cache behavior unchanged. This is the client +## half of the mandatory aliasing fix (T-1150 design doc §3, extended ## T-1152): a quarter-granularity request, a district-granularity request, ## and a REGION-granularity request all at the identical (body, center, n) -## MUST NOT collide on the same cache slot — the legacy int alone cannot -## distinguish Region (it has no legal legacy-int value, see -## GRANULARITY_V2_REGION's doc), which is exactly why granularity_v2 is a -## SEPARATE key component rather than a replacement for the legacy one. +## MUST NOT collide on the same cache slot — granularity_v2 is what +## distinguishes them. +## +## **T-1159:** the legacy int `granularity` key component (district=1 / +## quarter=4, mirroring the server's now-retired `window_granularity: u32` +## wire field) is retired — `granularity_v2` alone has always been sufficient +## to distinguish every rung (it's the ONLY axis that can express Region at +## all), so carrying both was redundant. ## ## Godot's Dictionary preserves insertion order, so "move to the end on ## touch, evict from the front on overflow" is the whole LRU implementation — @@ -35,26 +38,16 @@ extends RefCounted const DEFAULT_MAX_ENTRIES: int = 24 -## Mirrors the server's WINDOW_GRANULARITY_DISTRICT (layer_proxy.rs) — the -## default granularity every pre-T-1150 caller implicitly requests. -const DISTRICT_GRANULARITY: int = 1 - ## Mirrors the server's WindowGranularity enum (T-1152, R5 redesign, ## layer_proxy.rs) — the string-tag vocabulary rmp_serde encodes a bare ## `#[derive(Serialize, Deserialize)]` enum's variant name as, verbatim (same -## wire convention `RoadNodeKind` already established on this carrier). This -## is the KEY-SPACE axis (T-1153): the legacy int `granularity` param below -## stays wired for every existing District/Quarter caller (byte/behavior -## compatible), but a cache slot is now ALSO qualified by this string so a -## Region-rung window can never alias onto a District/Quarter slot at the -## identical (body, center, n, legacy_granularity, min_wl_m) — the exact -## aliasing risk the T-1150 design doc §3 flagged, extended to the new axis. +## wire convention `RoadNodeKind` already established on this carrier). const GRANULARITY_V2_QUARTER: String = "Quarter" const GRANULARITY_V2_DISTRICT: String = "District" const GRANULARITY_V2_REGION: String = "Region" -## Default v2 tag for every caller that doesn't pass one — matches -## DISTRICT_GRANULARITY's own "district is the implicit default" contract, so -## an omitted v2 tag and an explicit "District" tag key identically. +## Default v2 tag for every caller that doesn't pass one — district spacing +## is the implicit default, so an omitted v2 tag and an explicit "District" +## tag key identically. const DEFAULT_GRANULARITY_V2: String = GRANULARITY_V2_DISTRICT var _max_entries: int = DEFAULT_MAX_ENTRIES @@ -65,42 +58,37 @@ func _init(max_entries: int = DEFAULT_MAX_ENTRIES) -> void: _max_entries = maxi(1, max_entries) -## Build the cache key from the six fields D-227 + T-1150/T-1152 make +## Build the cache key from the five fields D-227 + T-1150/T-1152 make ## sufficient: body_id (which world+body), center (a [row, col] pair or -## Vector2i), n (window extent in districts), granularity (the legacy int: -## district=1 / quarter=4), min_wl_m (the octave cutoff, 0 = none), and -## granularity_v2 (the T-1152 string tag: "Quarter"/"District"/"Region" — the -## axis that actually distinguishes Region from every finer rung, since -## Region has no legal legacy-int representation and the legacy slot alone -## cannot tell a Region window's cache entry apart from a District one at the -## same (center, n)). String-keyed rather than a nested Dictionary/Array key -## — Godot Dictionary keys compare by value for primitives but a consistent -## stringification sidesteps any Vector2i-vs-Array identity mismatch between -## what a caller happens to hand in. +## Vector2i), n (window extent in districts), min_wl_m (the octave cutoff, +## 0 = none), and granularity_v2 (the T-1152 string tag: +## "Quarter"/"District"/"Region"). String-keyed rather than a nested +## Dictionary/Array key — Godot Dictionary keys compare by value for +## primitives but a consistent stringification sidesteps any +## Vector2i-vs-Array identity mismatch between what a caller happens to hand +## in. static func make_key( body_id: String, center: Vector2i, n: int, - granularity: int = DISTRICT_GRANULARITY, min_wl_m: int = 0, granularity_v2: String = DEFAULT_GRANULARITY_V2 ) -> String: - return "%s:%d,%d:%d:%d:%d:%s" % [ - body_id, center.x, center.y, n, granularity, min_wl_m, granularity_v2 + return "%s:%d,%d:%d:%d:%s" % [ + body_id, center.x, center.y, n, min_wl_m, granularity_v2 ] ## True if a window is already cached for this exact (body, center, n, -## granularity, min_wl_m, granularity_v2). +## min_wl_m, granularity_v2). func has( body_id: String, center: Vector2i, n: int, - granularity: int = DISTRICT_GRANULARITY, min_wl_m: int = 0, granularity_v2: String = DEFAULT_GRANULARITY_V2 ) -> bool: - return _entries.has(make_key(body_id, center, n, granularity, min_wl_m, granularity_v2)) + return _entries.has(make_key(body_id, center, n, min_wl_m, granularity_v2)) ## Fetch a cached window, touching it (move-to-most-recently-used). Returns @@ -112,11 +100,10 @@ func get_window( body_id: String, center: Vector2i, n: int, - granularity: int = DISTRICT_GRANULARITY, min_wl_m: int = 0, granularity_v2: String = DEFAULT_GRANULARITY_V2 ) -> Variant: - var key := make_key(body_id, center, n, granularity, min_wl_m, granularity_v2) + var key := make_key(body_id, center, n, min_wl_m, granularity_v2) if not _entries.has(key): return null var value: Variant = _entries[key] @@ -133,11 +120,10 @@ func put( center: Vector2i, n: int, window: Dictionary, - granularity: int = DISTRICT_GRANULARITY, min_wl_m: int = 0, granularity_v2: String = DEFAULT_GRANULARITY_V2 ) -> void: - var key := make_key(body_id, center, n, granularity, min_wl_m, granularity_v2) + var key := make_key(body_id, center, n, min_wl_m, granularity_v2) if _entries.has(key): _entries.erase(key) _entries[key] = window diff --git a/server/data/systems.db b/server/data/systems.db index 22523070f..5ff468067 100644 Binary files a/server/data/systems.db and b/server/data/systems.db differ diff --git a/server/src/atlas/atlas_data_proxy.rs b/server/src/atlas/atlas_data_proxy.rs index 87a72b039..01222abe8 100644 --- a/server/src/atlas/atlas_data_proxy.rs +++ b/server/src/atlas/atlas_data_proxy.rs @@ -215,6 +215,120 @@ pub fn handle_city_names_request( } } +// --------------------------------------------------------------------------- +// Feature names proxy (T-1169) +// --------------------------------------------------------------------------- + +/// A client request for a body's reserved geographic feature names (T-1169) — +/// mirrors [`CityNamesRequest`] exactly (D-236 pattern), its own message type +/// per the same Inbound-demux discriminator discipline the module doc +/// describes: outside the [`crate::atlas::layer_proxy::AtlasLayerResponse`] +/// ceilings, since a name pool is unrelated to the dense per-cell wire +/// arrays those ceilings budget for. +/// +/// `feature_names` is the Inbound discriminator (see module doc): always `true`. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct FeatureNamesRequest { + pub feature_names: bool, + pub body_id: String, +} + +/// Status of a [`FeatureNamesResponse`] — mirrors [`CityNamesStatus`] exactly. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum FeatureNamesStatus { + /// Names are ready (`features` is populated; legitimately empty for a + /// body with no reserved feature names). + Ready, + /// D-236: `body_id` is a Sol body. Sol is permanently out of the + /// generation cascade and every DB-derived Atlas path — the client must + /// keep its legacy authored `markers.json` read for Sol. `features` is empty. + SolExcluded, + /// DB/IO failure reading `atlas_feature_names` (message for the client log). + Error(String), +} + +/// One reserved feature-name entry (T-1169) — see +/// [`crate::atlas::city_context_reader::FeatureNameRow`] for the reader-side +/// row this is built from. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct FeatureNameEntry { + pub feature_id: u64, + pub name: String, + /// `"river"` | `"mountain"` (T-1169 scope — see + /// [`crate::atlas::city_context_reader::FeatureNameRow::feature_type`]). + pub feature_type: String, +} + +/// A feature-names response: the body's reserved geographic feature names, or +/// a non-ready status (T-1169). Mirrors [`CityNamesResponse`] exactly. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct FeatureNamesResponse { + pub body_id: String, + pub status: FeatureNamesStatus, + pub features: Vec, +} + +/// Serve one feature-names request (T-1169): D-236 Sol check first (same +/// reader method [`handle_city_names_request`] uses), then the names-only +/// `atlas_feature_names` read. `city_reader` absent (no DB opened at startup) +/// is reported as `Error`, matching `handle_city_names_request`'s convention. +/// +/// This is a POOL read, not a position-assignment lookup — it does not read +/// `layer1::attach_feature_names`'s cascade output (which attaches these +/// names to computed river-mouth/alpine-peak positions at generation time, +/// not at DB-read time). The client pairs this name list with position data +/// it already has from the cascade layer response. +pub fn handle_feature_names_request( + req: &FeatureNamesRequest, + city_reader: Option<&CityContextReader>, +) -> FeatureNamesResponse { + let Some(reader) = city_reader else { + return FeatureNamesResponse { + body_id: req.body_id.clone(), + status: FeatureNamesStatus::Error("city context reader unavailable".to_string()), + features: Vec::new(), + }; + }; + + match reader.is_sol_body(&req.body_id) { + Ok(true) => { + return FeatureNamesResponse { + body_id: req.body_id.clone(), + status: FeatureNamesStatus::SolExcluded, + features: Vec::new(), + }; + } + Ok(false) => {} + Err(e) => { + return FeatureNamesResponse { + body_id: req.body_id.clone(), + status: FeatureNamesStatus::Error(e.to_string()), + features: Vec::new(), + }; + } + } + + match reader.read_body_feature_names(&req.body_id) { + Ok(rows) => FeatureNamesResponse { + body_id: req.body_id.clone(), + status: FeatureNamesStatus::Ready, + features: rows + .into_iter() + .map(|r| FeatureNameEntry { + feature_id: r.feature_id, + name: r.name, + feature_type: r.feature_type, + }) + .collect(), + }, + Err(e) => FeatureNamesResponse { + body_id: req.body_id.clone(), + status: FeatureNamesStatus::Error(e.to_string()), + features: Vec::new(), + }, + } +} + #[cfg(test)] mod tests { use super::*; @@ -312,8 +426,9 @@ mod tests { assert!(decoded.cities[0].is_capital); } - /// Minimal db mirroring what `is_sol_body` + `read_body_city_names` need: - /// `bodies`, `system_history`, `atlas_city_names`. + /// Minimal db mirroring what `is_sol_body` + `read_body_city_names` + + /// `read_body_feature_names` need: `bodies`, `system_history`, + /// `atlas_city_names`, `atlas_feature_names`. fn make_db(body_id: &str, system_id: &str, settlement_wave: Option<&str>) -> PathBuf { let n = SEQ.fetch_add(1, Ordering::Relaxed); let path = std::env::temp_dir().join(format!("sr_adp_{}_{n}.db", std::process::id())); @@ -330,6 +445,12 @@ mod tests { body_id TEXT NOT NULL, name TEXT NOT NULL, kind TEXT NOT NULL DEFAULT 'city' + ); + CREATE TABLE atlas_feature_names ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + body_id TEXT NOT NULL, + name TEXT NOT NULL, + feature_type TEXT NOT NULL );", ) .expect("create tables"); @@ -358,6 +479,15 @@ mod tests { .expect("insert city"); } + fn insert_feature(db: &Path, body_id: &str, name: &str, feature_type: &str) { + let conn = Connection::open(db).expect("reopen"); + conn.execute( + "INSERT INTO atlas_feature_names (body_id, name, feature_type) VALUES (?1, ?2, ?3)", + rusqlite::params![body_id, name, feature_type], + ) + .expect("insert feature"); + } + #[test] fn handle_city_names_request_no_reader_is_error() { let resp = handle_city_names_request( @@ -448,4 +578,127 @@ mod tests { assert_eq!(resp.status, CityNamesStatus::Ready); assert!(resp.cities.is_empty()); } + + // ─── FeatureNamesRequest/Response (T-1169) ─────────────────────────────── + + #[test] + fn feature_names_request_round_trips_msgpack() { + let req = FeatureNamesRequest { + feature_names: true, + body_id: "GJ1c".into(), + }; + let bytes = rmp_serde::to_vec_named(&req).expect("encode"); + let decoded: FeatureNamesRequest = rmp_serde::from_slice(&bytes).expect("decode"); + assert!(decoded.feature_names); + assert_eq!(decoded.body_id, "GJ1c"); + } + + #[test] + fn feature_names_response_round_trips_msgpack() { + let resp = FeatureNamesResponse { + body_id: "GJ1c".into(), + status: FeatureNamesStatus::Ready, + features: vec![FeatureNameEntry { + feature_id: 1, + name: "Serra Verde".into(), + feature_type: "mountain".into(), + }], + }; + let bytes = rmp_serde::to_vec_named(&resp).expect("encode"); + let decoded: FeatureNamesResponse = rmp_serde::from_slice(&bytes).expect("decode"); + assert_eq!(decoded.status, FeatureNamesStatus::Ready); + assert_eq!(decoded.features[0].name, "Serra Verde"); + assert_eq!(decoded.features[0].feature_type, "mountain"); + } + + #[test] + fn handle_feature_names_request_no_reader_is_error() { + let resp = handle_feature_names_request( + &FeatureNamesRequest { + feature_names: true, + body_id: "GJ1c".into(), + }, + None, + ); + assert!(matches!(resp.status, FeatureNamesStatus::Error(_))); + assert!(resp.features.is_empty()); + } + + #[test] + fn handle_feature_names_request_sol_body_is_excluded() { + // D-236: system_id = GJ-0 → SolExcluded, no DB row read for features. + let db = make_db("Earth", "GJ-0", None); + insert_feature(&db, "Earth", "Thames", "river"); + let reader = CityContextReader::open(&db).expect("open"); + + let resp = handle_feature_names_request( + &FeatureNamesRequest { + feature_names: true, + body_id: "Earth".into(), + }, + Some(&reader), + ); + assert_eq!(resp.status, FeatureNamesStatus::SolExcluded); + assert!( + resp.features.is_empty(), + "Sol-excluded response must carry no features even though the row exists" + ); + } + + #[test] + fn handle_feature_names_request_sol_body_via_settlement_wave_is_excluded() { + // D-236's second signal: settlement_wave = 'origin' excludes even a + // non-GJ-0 system_id. + let db = make_db("Weirdbody", "GJ-999", Some("origin")); + let reader = CityContextReader::open(&db).expect("open"); + + let resp = handle_feature_names_request( + &FeatureNamesRequest { + feature_names: true, + body_id: "Weirdbody".into(), + }, + Some(&reader), + ); + assert_eq!(resp.status, FeatureNamesStatus::SolExcluded); + } + + #[test] + fn handle_feature_names_request_ordinary_body_returns_names() { + let db = make_db("GJ1c", "GJ-1", Some("first_wave")); + insert_feature(&db, "GJ1c", "Wiesenbach", "mountain"); + insert_feature(&db, "GJ1c", "Kaltfluss", "river"); + let reader = CityContextReader::open(&db).expect("open"); + + let resp = handle_feature_names_request( + &FeatureNamesRequest { + feature_names: true, + body_id: "GJ1c".into(), + }, + Some(&reader), + ); + assert_eq!(resp.status, FeatureNamesStatus::Ready); + assert_eq!(resp.features.len(), 2); + assert_eq!(resp.features[0].name, "Wiesenbach"); + assert_eq!(resp.features[0].feature_type, "mountain"); + assert_eq!(resp.features[1].name, "Kaltfluss"); + assert_eq!(resp.features[1].feature_type, "river"); + } + + #[test] + fn handle_feature_names_request_unknown_body_is_ready_with_empty_list() { + // Matches handle_city_names_request's existing convention: unknown + // body → empty list under Ready, not a distinct NotFound status. + let db = make_db("GJ1c", "GJ-1", Some("first_wave")); + let reader = CityContextReader::open(&db).expect("open"); + + let resp = handle_feature_names_request( + &FeatureNamesRequest { + feature_names: true, + body_id: "ghost".into(), + }, + Some(&reader), + ); + assert_eq!(resp.status, FeatureNamesStatus::Ready); + assert!(resp.features.is_empty()); + } } diff --git a/server/src/atlas/believability.rs b/server/src/atlas/believability.rs index 5b75f10d6..fc9b17db2 100644 --- a/server/src/atlas/believability.rs +++ b/server/src/atlas/believability.rs @@ -582,6 +582,7 @@ pub fn cascade_snapshot_for_body( .read_body_params(body_id) .map_err(|e| format!("read body params: {e:?}"))?; let cities = read_cities(&db, body_id)?; + let (river_names, mountain_names) = read_feature_names(&db, body_id)?; let hm = load_heightmap_png(&hm_path, body_id, DEFAULT_SEA_LEVEL) .map_err(|e| format!("load heightmap: {e:?}"))?; @@ -597,6 +598,8 @@ pub fn cascade_snapshot_for_body( &cities, None, Some(¶ms), + &river_names, + &mountain_names, CascadeLayer::Region, ); Ok((snapshot, params)) @@ -658,6 +661,44 @@ fn read_cities(db: &PathBuf, body_id: &str) -> Result, String> { Ok(rows) } +/// Read a body's reserved river/mountain name pools from `atlas_feature_names` +/// (T-1169, D-223), mirroring `read_cities`' own lightweight raw-SQL +/// discipline (this module reads `systems.db` directly rather than going +/// through `CityContextReader` — keep the two independent, matching the +/// existing `read_cities` precedent). Returns `(river_names, mountain_names)`. +fn read_feature_names(db: &PathBuf, body_id: &str) -> Result<(Vec, Vec), String> { + let conn = rusqlite::Connection::open(db).map_err(|e| format!("open db: {e}"))?; + let mut stmt = conn + .prepare( + "SELECT name, feature_type FROM atlas_feature_names + WHERE body_id = ?1 ORDER BY id", + ) + .map_err(|e| format!("prepare feature name query: {e}"))?; + let rows = stmt + .query_map([body_id], |r| { + Ok((r.get::<_, String>(0)?, r.get::<_, String>(1)?)) + }) + .map_err(|e| format!("feature name query: {e}"))?; + + let mut river_names = Vec::new(); + let mut mountain_names = Vec::new(); + for row in rows { + let (name, feature_type) = match row { + Ok(r) => r, + Err(e) => { + eprintln!("[believability] skipped a malformed feature name row for {body_id}: {e}"); + continue; + } + }; + match feature_type.as_str() { + "river" => river_names.push(name), + "mountain" => mountain_names.push(name), + _ => {} + } + } + Ok((river_names, mountain_names)) +} + /// Glob `*/bodies//heightmap.png` under the committed wiki tree (either CWD). fn find_heightmap(body_id: &str) -> Option { for base in ["wiki/star-systems", "../wiki/star-systems"] { diff --git a/server/src/atlas/body_world_state.rs b/server/src/atlas/body_world_state.rs index 147e65aa8..348136319 100644 --- a/server/src/atlas/body_world_state.rs +++ b/server/src/atlas/body_world_state.rs @@ -15,6 +15,7 @@ use serde::{Deserialize, Serialize}; use crate::atlas::attractor_matching::CityPlacement; use crate::atlas::district_profile::DistrictProfile; +use crate::atlas::layer1::FeatureNameAssignment; use crate::atlas::region_profile::RegionProfile; use crate::atlas::road_graph::RoadGraph; use crate::atlas::scale::{RegionPos, SurveyCellPos}; @@ -182,6 +183,12 @@ pub struct BodyWorldState { pub drainage_basins: Vec, /// Geographic attractors (D-195, D-209). Empty until attractor task completes. pub attractors: Vec, + /// Named-feature position assignments (T-1169, D-223) — river-mouth and + /// alpine-peak attractors paired with reserved pool names, via + /// `layer1::attach_feature_names`. Empty until the Topography task + /// completes (mirrors `attractors`' own "empty until" convention), or if + /// the body has no reserved names in `atlas_feature_names`. + pub feature_names: Vec, /// Settlement placements (D-211, #955). Attractor-matched city positions. /// Empty until the Layer-3 placement task completes. pub placements: Vec, @@ -332,6 +339,7 @@ mod tests { river_network: RiverNetwork::default(), drainage_basins: vec![], attractors: vec![], + feature_names: vec![], placements: vec![], road_graph: RoadGraph::default(), quarters: BTreeMap::new(), diff --git a/server/src/atlas/cascade.rs b/server/src/atlas/cascade.rs index d19953f13..de452d18a 100644 --- a/server/src/atlas/cascade.rs +++ b/server/src/atlas/cascade.rs @@ -147,9 +147,14 @@ impl CascadeSnapshot { /// `terrain_analysis` (transient, ~2 MB) is **dropped here** — it is not /// persisted on `BodyWorldState` per the D-203/T-1048 size budget. pub fn into_body_world_state(self) -> BodyWorldState { - let (river_network, drainage_basins, attractors) = match self.layer1 { - Some(l1) => (l1.river_network, l1.drainage_basins, l1.attractors), - None => (RiverNetwork::default(), Vec::new(), Vec::new()), + let (river_network, drainage_basins, attractors, feature_names) = match self.layer1 { + Some(l1) => ( + l1.river_network, + l1.drainage_basins, + l1.attractors, + l1.feature_names, + ), + None => (RiverNetwork::default(), Vec::new(), Vec::new(), Vec::new()), }; let placements = self.layer3.map(|l3| l3.placements).unwrap_or_default(); let districts = self @@ -170,6 +175,7 @@ impl CascadeSnapshot { river_network, drainage_basins, attractors, + feature_names, placements, road_graph, quarters: std::collections::BTreeMap::new(), @@ -224,12 +230,20 @@ fn run_layer3( /// character (#956). `None` → `FrontierUnclaimed`. /// `body_params` supplies the physical parameters needed for the DistrictProfile /// layer (T-1023); `None` → district layer skips (empty `districts` map). +/// `river_names`/`mountain_names` are the body's reserved-name pools (T-1169, +/// D-223, from `atlas_feature_names`), supplied by the caller — mirrors +/// `cities`' own pre-resolved, DB-free-cascade pattern. Empty slices are the +/// correct input for a body with no reserved names, or a caller (tests, +/// `aliveness_probe`) that hasn't pre-resolved them; `attach_feature_names` +/// degrades gracefully (every attractor position simply gets no name). pub fn run_cascade_from_heightmap( body_seed: SeedChain, heightmap: BodyHeightmap, cities: &[CityRecord], dominant_faction: Option<&str>, body_params: Option<&BodyParams>, + river_names: &[String], + mountain_names: &[String], up_to: CascadeLayer, ) -> CascadeSnapshot { let mut snapshot = CascadeSnapshot { @@ -272,6 +286,31 @@ pub fn run_cascade_from_heightmap( for basin in &mut l1.drainage_basins { basin.territorial_status = territorial_status.clone(); } + // T-1169: attach reserved names (D-223) to the strongest river-mouth + // and alpine-peak attractors. Cheap (two sorts + zips over the + // already-computed attractor list, no new terrain work) and + // deterministic given the caller-supplied pools — mirrors the + // TerritorialStatus stamp above in running once, right after Layer 1 + // produces the attractors this reads. + let (river_assignments, mountain_assignments) = + layer1::attach_feature_names(&l1, river_names, mountain_names); + l1.feature_names = river_assignments + .into_iter() + .map(|(position, name)| layer1::FeatureNameAssignment { + position, + name, + feature_type: layer1::FeatureNameType::River, + }) + .chain( + mountain_assignments + .into_iter() + .map(|(position, name)| layer1::FeatureNameAssignment { + position, + name, + feature_type: layer1::FeatureNameType::Mountain, + }), + ) + .collect(); snapshot.layer1 = Some(l1); snapshot.terrain_analysis = Some(ta); } @@ -456,6 +495,8 @@ pub fn run_cascade( cities: &[CityRecord], dominant_faction: Option<&str>, body_params: Option<&BodyParams>, + river_names: &[String], + mountain_names: &[String], up_to: CascadeLayer, ) -> Result { // Layer 0 — the cascade's input; always loaded. @@ -466,6 +507,8 @@ pub fn run_cascade( cities, dominant_faction, body_params, + river_names, + mountain_names, up_to, )) } @@ -508,6 +551,8 @@ mod tests { &[], None, None, // body_params + &[], + &[], CascadeLayer::Heightmap, ); assert_eq!(snap.body_id, "test_body"); @@ -525,12 +570,91 @@ mod tests { &[], None, None, // body_params + &[], + &[], CascadeLayer::Topography, ); let l1 = snap.layer1.expect("Layer 1 should have run"); assert_eq!(l1.body_id, "test_body"); } + /// T-1169: `run_cascade_from_heightmap` attaches reserved names to the + /// strongest river-mouth/alpine attractors when the caller supplies name + /// pools — proves the cascade wiring (`attach_feature_names` call site + /// inside the Topography block), not just the function in isolation + /// (`layer1::tests` already covers `attach_feature_names` itself). + #[test] + fn topography_layer_attaches_feature_names_when_pools_supplied() { + let river_names = vec!["Kaltfluss".to_string(), "Silberbach".to_string()]; + let mountain_names = vec!["Wiesenbach".to_string()]; + let snap = run_cascade_from_heightmap( + body_seed(), + test_heightmap(), + &[], + None, + None, // body_params + &river_names, + &mountain_names, + CascadeLayer::Topography, + ); + let l1 = snap.layer1.expect("Layer 1 should have run"); + + // The test heightmap's `0.6r + 0.4c` ramp crosses sea_level=0.3, + // producing real river-mouth/coastal attractors — assert against + // WHATEVER attach_feature_names actually paired, not a hardcoded + // count (the exact attractor set is an implementation detail of + // feature extraction, not this test's concern). + let river_attractor_count = l1 + .attractors + .iter() + .filter(|a| { + a.attractor_type == crate::simulation::generator::AttractorType::RiverMouth + }) + .count(); + let expected_river_assignments = river_attractor_count.min(river_names.len()); + let actual_river_assignments = l1 + .feature_names + .iter() + .filter(|f| f.feature_type == crate::atlas::layer1::FeatureNameType::River) + .count(); + assert_eq!( + actual_river_assignments, expected_river_assignments, + "every river mouth (up to pool size) must get a name" + ); + if expected_river_assignments > 0 { + let names: std::collections::BTreeSet<&str> = l1 + .feature_names + .iter() + .filter(|f| f.feature_type == crate::atlas::layer1::FeatureNameType::River) + .map(|f| f.name.as_str()) + .collect(); + assert!( + names.iter().all(|n| river_names.contains(&n.to_string())), + "assigned names must come from the supplied pool" + ); + } + + // No pools supplied -> no assignments (the pre-wiring default). + let snap_no_pools = run_cascade_from_heightmap( + body_seed(), + test_heightmap(), + &[], + None, + None, + &[], + &[], + CascadeLayer::Topography, + ); + assert!( + snap_no_pools + .layer1 + .expect("Layer 1 should have run") + .feature_names + .is_empty(), + "empty pools must yield zero assignments" + ); + } + #[test] fn cascade_is_deterministic() { let extract = |s: CascadeSnapshot| { @@ -546,6 +670,8 @@ mod tests { &[], None, None, // body_params + &[], + &[], CascadeLayer::Topography, )); let b = extract(run_cascade_from_heightmap( @@ -554,6 +680,8 @@ mod tests { &[], None, None, // body_params + &[], + &[], CascadeLayer::Topography, )); assert_eq!( @@ -586,6 +714,8 @@ mod tests { &[], None, None, // body_params + &[], + &[], CascadeLayer::Heightmap, ); assert!(res.is_err(), "missing heightmap must Err, not panic"); @@ -609,6 +739,8 @@ mod tests { &[], None, Some(¶ms), + &[], + &[], CascadeLayer::DistrictProfile, ) }; @@ -668,6 +800,8 @@ mod tests { &cities, Some("concord_assembly"), None, // body_params + &[], + &[], CascadeLayer::Settlement, ) }; @@ -756,6 +890,8 @@ mod tests { &cities, Some("independent"), None, // body_params — road graph needs none + &[], + &[], CascadeLayer::RoadGraph, ) }; @@ -860,6 +996,8 @@ mod tests { &cities, Some("independent"), None, + &[], + &[], CascadeLayer::RoadGraph, ); let graph = snap.road_graph.as_ref().expect("RoadGraph layer ran"); @@ -927,6 +1065,8 @@ mod tests { &[], None, Some(¶ms), + &[], + &[], CascadeLayer::Region, ) }; @@ -980,6 +1120,8 @@ mod tests { &[], None, None, + &[], + &[], CascadeLayer::Region, ); assert!(no_params.layer_region.is_none()); diff --git a/server/src/atlas/city_context_reader.rs b/server/src/atlas/city_context_reader.rs index ad11903c4..ccdd9cc37 100644 --- a/server/src/atlas/city_context_reader.rs +++ b/server/src/atlas/city_context_reader.rs @@ -467,6 +467,58 @@ impl CityContextReader { } Ok(out) } + + /// Read every reserved geographic feature **name** on `body_id` from + /// `atlas_feature_names` (T-1169 — mirrors [`read_body_city_names`] + /// exactly, D-236 pattern, over `atlas_feature_names` instead of + /// `atlas_city_names`). Returns the id/name/feature_type triple — this is + /// the raw reserved-name POOL, not a position assignment (positions come + /// from `layer1::attach_feature_names` at cascade generation time, not + /// from this reader). Ordered by `id`. An unknown body yields an empty + /// list (matches `read_body_city_names`'s convention); only a DB/mutex + /// error fails. **No Sol check here** — unlike city names, this is a raw + /// pool read with no per-body caller-facing status enum; the proxy + /// handler (`atlas_data_proxy::handle_feature_names_request`) applies the + /// same D-236 Sol exclusion `handle_city_names_request` does, BEFORE + /// calling this method, so Sol bodies never reach this query in practice. + pub fn read_body_feature_names( + &self, + body_id: &str, + ) -> Result, CityContextReadError> { + let conn = self + .conn + .lock() + .map_err(|e| CityContextReadError::Db(format!("mutex poisoned: {e}")))?; + let mut stmt = conn + .prepare( + "SELECT id, name, feature_type + FROM atlas_feature_names + WHERE body_id = ?1 + ORDER BY id", + ) + .map_err(|e| CityContextReadError::Db(e.to_string()))?; + let rows = stmt + .query_map([body_id], |row| { + Ok(( + row.get::<_, i64>(0)?, + row.get::<_, String>(1)?, + row.get::<_, String>(2)?, + )) + }) + .map_err(|e| CityContextReadError::Db(e.to_string()))?; + + let mut out = Vec::new(); + for r in rows { + let (id, name, feature_type) = + r.map_err(|e| CityContextReadError::Db(e.to_string()))?; + out.push(FeatureNameRow { + feature_id: id as u64, + name, + feature_type, + }); + } + Ok(out) + } } /// One row of the T-949 names-only read (see @@ -478,6 +530,20 @@ pub struct CityNameRow { pub is_capital: bool, } +/// One row of the T-1169 names-only read (see +/// [`CityContextReader::read_body_feature_names`]). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FeatureNameRow { + pub feature_id: u64, + pub name: String, + /// `atlas_feature_names.feature_type` — `"river"` | `"mountain"` (the two + /// pools `import_economics`/`atlas.py::populate_atlas_feature_names` + /// currently populates, T-1169 scope). Carried as a raw string, not an + /// enum — mirrors `CityNameRow.is_capital`'s discipline of staying a thin + /// passthrough of the DB row, no server-side vocabulary gate here. + pub feature_type: String, +} + // --------------------------------------------------------------------------- // prosperity_baseline_bps derivation (D-197, partial — integer basis points) // --------------------------------------------------------------------------- @@ -1355,6 +1421,61 @@ mod tests { ); } + // ─── read_body_feature_names (T-1169) ──────────────────────────────────── + + /// Minimal db for `read_body_feature_names`: just `atlas_feature_names`. + fn make_features_db(rows: &[(&str, &str)]) -> PathBuf { + let n = SEQ.fetch_add(1, Ordering::Relaxed); + let path = std::env::temp_dir().join(format!("sr_ctxfeat_{}_{n}.db", std::process::id())); + let _ = std::fs::remove_file(&path); + let conn = Connection::open(&path).expect("create db"); + conn.execute_batch( + "CREATE TABLE atlas_feature_names ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + body_id TEXT NOT NULL, + name TEXT NOT NULL, + feature_type TEXT NOT NULL + );", + ) + .expect("create table"); + for (name, feature_type) in rows { + conn.execute( + "INSERT INTO atlas_feature_names (body_id, name, feature_type) + VALUES ('PlanetX', ?1, ?2)", + rusqlite::params![name, feature_type], + ) + .expect("insert feature"); + } + drop(conn); + path + } + + #[test] + fn read_body_feature_names_returns_id_name_type() { + let db = make_features_db(&[("Wiesenbach", "mountain"), ("Kaltfluss", "river")]); + let reader = CityContextReader::open(&db).expect("open"); + let names = reader.read_body_feature_names("PlanetX").expect("read"); + assert_eq!(names.len(), 2); + // Ordered by id == insertion order. + assert_eq!(names[0].name, "Wiesenbach"); + assert_eq!(names[0].feature_type, "mountain"); + assert_eq!(names[1].name, "Kaltfluss"); + assert_eq!(names[1].feature_type, "river"); + } + + #[test] + fn read_body_feature_names_unknown_body_is_empty() { + let db = make_features_db(&[("Solo Peak", "mountain")]); + let reader = CityContextReader::open(&db).expect("open"); + assert!( + reader + .read_body_feature_names("Ghost") + .expect("read") + .is_empty(), + "unknown body yields no names, matching read_body_city_names' convention" + ); + } + // ─── is_sol_body (T-949, D-236) ────────────────────────────────────────── /// Minimal db for `is_sol_body`: one `bodies` row + an optional diff --git a/server/src/atlas/gen_queue.rs b/server/src/atlas/gen_queue.rs index 65e7198e4..386776355 100644 --- a/server/src/atlas/gen_queue.rs +++ b/server/src/atlas/gen_queue.rs @@ -91,6 +91,15 @@ pub enum GenWorkItem { /// Boxed: `BodyParams` is large relative to other variants (clippy /// large_enum_variant) — boxing keeps `GenWorkItem` compact. body_params: Option>, + /// The body's reserved river-name pool (`atlas_feature_names`, + /// `feature_type = 'river'`), pre-resolved at dispatch time (T-1169, + /// D-223) — mirrors `cities`' own DB-free-cascade pattern. Empty if + /// the body has no reserved river names. + river_names: Vec, + /// The body's reserved mountain-name pool (`atlas_feature_names`, + /// `feature_type = 'mountain'`), pre-resolved at dispatch time + /// (T-1169, D-223). Empty if the body has no reserved mountain names. + mountain_names: Vec, }, /// Generate a Phase 1 QuarterSkeleton for this city. /// @@ -974,6 +983,8 @@ fn run_work_item( cities, dominant_faction, body_params, + river_names, + mountain_names, } => match load_heightmap_png(heightmap_path, body_id, *sea_level) { Ok(hm) => { // Layer 1 runs at the GRID_W×GRID_H working resolution (D-202): @@ -997,6 +1008,8 @@ fn run_work_item( cities, dominant_faction.as_deref(), body_params.as_deref(), + river_names, + mountain_names, up_to, ); GenCompletion::BodyAnalyzed { @@ -1296,6 +1309,8 @@ mod tests { cities: vec![], dominant_faction: None, body_params: None, // T-1023: no body params in queue-mechanic unit tests + river_names: vec![], + mountain_names: vec![], } } diff --git a/server/src/atlas/layer1.rs b/server/src/atlas/layer1.rs index 739765eff..7561ea369 100644 --- a/server/src/atlas/layer1.rs +++ b/server/src/atlas/layer1.rs @@ -71,6 +71,35 @@ pub struct Layer1Output { /// after the cascade consumes it. #[serde(skip)] pub survey_basin_dirs: BTreeMap, + /// Named-feature position assignments (T-1169, D-223): river-mouth and + /// alpine-peak attractors paired with a reserved name from the + /// `atlas_feature_names` pool, via [`attach_feature_names`]. Empty when + /// the caller supplied no name pools (e.g. `run_layer1`'s two-arg + /// convenience form, or a body with no reserved names) — never populated + /// automatically inside `run_layer1`/`run_layer1_with_moisture` + /// themselves, since those have no DB access (D-225 DB-free-worker + /// discipline); the cascade caller pre-resolves the pools and calls + /// [`attach_feature_names`] itself (`cascade::run_cascade_from_heightmap`). + #[serde(default)] + pub feature_names: Vec, +} + +/// One reserved-name-to-position pairing (T-1169) — see [`attach_feature_names`]. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct FeatureNameAssignment { + pub position: (u16, u16), + pub name: String, + pub feature_type: FeatureNameType, +} + +/// The two pools [`attach_feature_names`] currently draws from (T-1169 +/// scope — mirrors `atlas_feature_names.feature_type`'s `"river"` | +/// `"mountain"` values, but typed rather than a raw string on this +/// server-internal cascade carrier). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum FeatureNameType { + River, + Mountain, } /// Body-wide moisture ceiling fallback for [`run_layer1`]'s hydrology solve @@ -216,6 +245,11 @@ pub fn run_layer1_with_moisture( grid_w: hm.width, grid_h: hm.height, survey_basin_dirs, + // T-1169: run_layer1/run_layer1_with_moisture have no DB access + // (D-225 DB-free-worker discipline) — feature-name attachment + // happens one level up, in the cascade caller that pre-resolved the + // name pools (see attach_feature_names's own doc). + feature_names: Vec::new(), }; (l1, ta) } diff --git a/server/src/atlas/layer_proxy.rs b/server/src/atlas/layer_proxy.rs index 6fbb50f9c..d609c05b8 100644 --- a/server/src/atlas/layer_proxy.rs +++ b/server/src/atlas/layer_proxy.rs @@ -83,46 +83,22 @@ pub const DISTRICT_WINDOW_MAX_N_REGION: u32 = const WIRE_CAP_CELLS_SQRT: u32 = 64; const _: () = assert!(WIRE_CAP_CELLS_SQRT * WIRE_CAP_CELLS_SQRT == WIRE_CAP_CELLS); -/// [`AtlasLayerRequest::window_granularity`] encoding (T-1150, zoom ladder -/// design doc §3/§5): the number of derived cells per district side. `0` on -/// the wire (the `#[serde(default)]` absent case) and `1` both mean district -/// spacing (2,048 m/cell, [`DISTRICT_WINDOW_MAX_N`]'s existing behavior, -/// byte-compatible with every pre-T-1150 caller). `4` means quarter spacing -/// (512 m/cell, D-243) — Option B from the design doc: full reclassification -/// at the finer spacing via `derive_at_metres`, not a coarser-cell -/// interpolation. No other values are legal; `resolve_window_granularity` -/// clamps unrecognized values down to district (never trust the wire, same -/// discipline as `window_n`). +/// Finer-than-district spacing multipliers (T-1150, zoom ladder design doc +/// §3/§5): the number of derived cells per district side. `1` = district +/// spacing (2,048 m/cell, [`DISTRICT_WINDOW_MAX_N`]'s existing behavior). +/// `4` = quarter spacing (512 m/cell, D-243) — Option B from the design doc: +/// full reclassification at the finer spacing via `derive_at_metres`, not a +/// coarser-cell interpolation. +/// +/// **T-1159:** these used to also be legal VALUES of the wire-facing +/// `AtlasLayerRequest::window_granularity: u32` field (resolved via the +/// now-removed `resolve_window_granularity`) — that field is retired, fully +/// shadowed by [`WindowGranularity`] since T-1152. These constants remain as +/// internal spacing-multiplier values (see [`WindowGranularity::spacing_multiplier`], +/// [`clamp_window_n`]). pub const WINDOW_GRANULARITY_DISTRICT: u32 = 1; pub const WINDOW_GRANULARITY_QUARTER: u32 = 4; -/// The `granularity: u32` value [`DistrictWindowLayer`]'s echo (and the -/// internal cache/coalescing keys) use for [`WindowGranularity::Region`] — -/// **a key-space value, never a legal WIRE INPUT.** [`resolve_window_granularity`] -/// (the legacy field's resolver) never produces this value, and a client -/// sending it on the wire in the legacy `window_granularity` field is -/// indistinguishable from any other unrecognized value — it still resolves -/// to `District` (§ `resolve_window_granularity`'s exhaustive fallback), NOT -/// `Region`. The only way to actually request `Region` is -/// `window_granularity_v2 = Some(WindowGranularity::Region)`. -/// -/// **Why this exists at all, given `Region` has no legacy representation:** -/// the aliasing discipline (T-1150 design doc §3, the mandatory -/// granularity-4-vs-1 test) requires that every distinct [`WindowGranularity`] -/// occupy a distinct slot in [`DistrictWindowKey`]/the coalescing key, both of -/// which carry a `u32` granularity component for wire back-compat. Reusing -/// `0` (today's "absent" sentinel, mapped to `District`) or any value -/// `resolve_window_granularity` could legally receive would silently alias a -/// `Region` window onto a `District` or `Quarter` cache slot depending on -/// what a future caller happened to pass — exactly the bug class §3 exists -/// to close. `u32::MAX` can never collide with a real multiplier (multipliers -/// are small integers by construction — 1, 4, and any future finer rung), -/// so it's the natural "this is a key-space tag, not a spacing multiplier" -/// value. A T-1152-aware client reads `granularity_v2` and never looks at -/// this number for `Region` responses; it exists purely so the legacy `u32` -/// slot in the key tuple stays total and never lies about aliasing. -pub const WINDOW_GRANULARITY_REGION_KEY: u32 = u32::MAX; - /// Server-side wire-size ceiling (T-1150, design doc §3 "Cell-count cap"): /// `window_n² × granularity² ≤ WIRE_CAP_CELLS`. At `WIRE_CAP_CELLS = 4,096`, /// district `n=64` (the existing [`DISTRICT_WINDOW_MAX_N`] cap) sits exactly @@ -161,20 +137,19 @@ pub const WIRE_CAP_CELLS: u32 = 4_096; /// reasoning `MorphologyZone`'s exhaustive-match discipline already /// established for this codebase (D-239 §6). /// -/// **Precedence over the legacy `u32` field (documented here, the single -/// place both fields are reconciled):** [`AtlasLayerRequest::window_granularity_v2`] -/// wins whenever present and non-`None`; the legacy `u32` -/// [`AtlasLayerRequest::window_granularity`] is consulted ONLY when -/// `window_granularity_v2` is absent (`#[serde(default)]`, every pre-T-1152 -/// client). This is a strict either/or, not a merge — a client sending BOTH -/// fields (a mixed old/new build, or a future client hedging compatibility) -/// gets the `v2` field's answer, silently ignoring the legacy `u32`. See -/// [`resolve_window_granularity_v2`], the single widening point for this -/// enum (mirroring `resolve_window_granularity`'s role for the legacy field). +/// **Resolution (T-1152, simplified T-1159):** [`AtlasLayerRequest::window_granularity_v2`] +/// resolves directly via [`resolve_window_granularity_v2`], the single +/// widening point for this enum. Absent (`#[serde(default)]`, `None`) +/// resolves to `District`. +/// +/// **T-1159:** this used to also reconcile against a legacy +/// `AtlasLayerRequest::window_granularity: u32` field (whenever THIS field +/// was absent) — that field is retired, fully shadowed since T-1152 and +/// never sent as anything but its byte-compatible default by any caller in +/// this codebase (no external client exists, single-repo client/server pair). /// /// **Unknown → District** at every resolution boundary (never trust the -/// wire) — same posture as the legacy `u32` path and every other wire-decoded -/// enum in this module. +/// wire) — same posture as every other wire-decoded enum in this module. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] pub enum WindowGranularity { /// 512 m/cell (D-243 `QUARTER_M`) — finer than district, T-1150 Option B. @@ -207,14 +182,18 @@ impl WindowGranularity { } } - /// The legacy `window_granularity: u32` value this variant maps to/from - /// for finer-than-district rungs — `None` for `Region`, which the legacy - /// field cannot express by construction (Tyre's note; this is precisely - /// the gap the enum exists to close). Used only by - /// [`resolve_window_granularity_v2`]'s legacy-fallback branch and by - /// [`DistrictWindowLayer`]'s echo, which still carries the legacy `u32` - /// unchanged for wire back-compat (see that struct's doc). - fn legacy_u32(self) -> Option { + /// The finer-than-district spacing multiplier ([`WINDOW_GRANULARITY_DISTRICT`] + /// / [`WINDOW_GRANULARITY_QUARTER`]) this variant corresponds to — `None` + /// for `Region`, which has no such multiplier (its spacing is coarser, + /// not a finer subdivision of a district). Used only by + /// [`clamp_window_n_v2`]'s District/Quarter branch to reuse + /// [`clamp_window_n`]'s per-axis-cap math rather than re-deriving it. + /// + /// **T-1159:** this used to double as the wire-back-compat value for the + /// now-retired `window_granularity: u32` echo (`DistrictWindowLayer.granularity`) + /// — that role, and the `key_u32()` method that served it, are gone; this + /// is purely an internal spacing-multiplier lookup now. + fn spacing_multiplier(self) -> Option { match self { WindowGranularity::District => Some(WINDOW_GRANULARITY_DISTRICT), WindowGranularity::Quarter => Some(WINDOW_GRANULARITY_QUARTER), @@ -222,19 +201,6 @@ impl WindowGranularity { } } - /// The `u32` this variant occupies in [`DistrictWindowKey`]/the - /// coalescing key/`DistrictWindowLayer.granularity`'s echo — total over - /// every variant (unlike [`Self::legacy_u32`], which is partial). Finer- - /// than-district variants echo their real legacy multiplier (so a - /// T-1150-only client — one that reads `granularity` but has never heard - /// of `granularity_v2` — still sees the correct, meaningful number for - /// District/Quarter); `Region` echoes the reserved - /// [`WINDOW_GRANULARITY_REGION_KEY`] key-space tag (see that const's doc - /// for why `0` or any real multiplier would be unsafe here). - fn key_u32(self) -> u32 { - self.legacy_u32().unwrap_or(WINDOW_GRANULARITY_REGION_KEY) - } - /// The derived cell-grid side length (in CELLS, at this granularity) for /// a window whose extent is `n` DISTRICTS (T-1152 step 4: "work out what /// n means at region granularity against D-243's region=100-district @@ -276,55 +242,22 @@ impl WindowGranularity { } } -/// Resolve a wire-supplied `window_granularity` value to one of the two legal -/// granularities, clamping anything else down to district spacing — **never -/// trust the wire** (same posture as `window_n`/`normalize_window_center`). -/// -/// **This is THE single widening point (Tyre C2, PR #191 review) for the -/// LEGACY `u32` field only.** The field only ever expresses finer-than-district -/// integer multiples (see [`AtlasLayerRequest::window_granularity`]'s doc for -/// the full type-seam contract); adding a future finer rung means adding its -/// legal value here and nowhere else. Do NOT add a value < 1 or attempt to -/// encode coarser-than-district rungs (region/orbital) through this function -/// — that direction is [`WindowGranularity::Region`]'s job via -/// [`resolve_window_granularity_v2`], never a new magic `u32` value (the R5 -/// redesign this enum performs is EXACTLY the alternative to doing that). -fn resolve_window_granularity(raw: u32) -> u32 { - if raw == WINDOW_GRANULARITY_QUARTER { - WINDOW_GRANULARITY_QUARTER - } else { - WINDOW_GRANULARITY_DISTRICT - } -} - /// Resolve a request's granularity to a [`WindowGranularity`] — **the single /// widening point for the full (finer- and coarser-than-district) vocabulary** -/// (T-1152, mirroring [`resolve_window_granularity`]'s role for the legacy -/// `u32` alone). Precedence (documented once, here — see -/// [`WindowGranularity`]'s struct doc for the rationale): +/// (T-1152). Absent (`None`, `#[serde(default)]`) resolves to +/// [`WindowGranularity::District`] — unknown/malformed variants can't reach +/// this function at all (`rmp_serde` rejects an unrecognized enum variant +/// name at decode time, so "unknown" for this field means "absent", never +/// "present but garbage"). /// -/// 1. `window_granularity_v2` present → resolved directly (unknown/malformed -/// variants can't reach this function at all — `rmp_serde` rejects an -/// unrecognized enum variant name at decode time, so "unknown" for THIS -/// field means "absent", not "present but garbage"; the legacy `u32` -/// path is what actually needs the value-level fallback because a `u32` -/// has no closed vocabulary). -/// 2. `window_granularity_v2` absent → fall back to the legacy `u32` path via -/// [`resolve_window_granularity`], mapped onto the two variants it can -/// express. -/// -/// A future coarser-than-region rung is added by widening this function's -/// match AND [`WindowGranularity`]'s variant list together — never by -/// smuggling a new value through the legacy `u32` (that field's ceiling is -/// permanent per Tyre's note, not a restriction this function works around). +/// **T-1159:** this function used to fall back to the legacy `window_granularity: u32` +/// field (via the now-removed `resolve_window_granularity`) when +/// `window_granularity_v2` was absent — that fallback is retired along with +/// the field itself (fully shadowed since T-1152, no pre-T-1152 client +/// exists). A future coarser-than-region rung is added by widening this +/// function's match AND [`WindowGranularity`]'s variant list together. fn resolve_window_granularity_v2(req: &AtlasLayerRequest) -> WindowGranularity { - match req.window_granularity_v2 { - Some(g) => g, - None => match resolve_window_granularity(req.window_granularity) { - WINDOW_GRANULARITY_QUARTER => WindowGranularity::Quarter, - _ => WindowGranularity::District, - }, - } + req.window_granularity_v2.unwrap_or(WindowGranularity::District) } /// Clamp `window_n` against BOTH the existing per-axis cap @@ -392,7 +325,7 @@ fn clamp_window_n_v2(raw_n: u32, granularity: WindowGranularity) -> u32 { WindowGranularity::District | WindowGranularity::Quarter => clamp_window_n( raw_n, granularity - .legacy_u32() + .spacing_multiplier() .unwrap_or(WINDOW_GRANULARITY_DISTRICT), ), WindowGranularity::Region => { @@ -520,36 +453,19 @@ pub struct AtlasLayerRequest { /// from the wire** (D-226 T-1124 amendment §4). #[serde(default)] pub window_n: u32, - /// Window derivation granularity (T-1150, zoom ladder design doc §3/§5). - /// `0` (absent, `#[serde(default)]`) or `1` = district spacing (2,048 m, - /// today's behavior, byte-compatible with every pre-T-1150 caller); `4` = - /// quarter spacing (512 m, D-243). See [`WINDOW_GRANULARITY_DISTRICT`] / - /// [`WINDOW_GRANULARITY_QUARTER`]. Resolved via - /// [`resolve_window_granularity`] — **never trusted from the wire**, - /// unrecognized values fall back to district. + /// Window derivation granularity (T-1152, R5 redesign — see + /// [`WindowGranularity`]'s doc for the full rationale). `#[serde(default)]` + /// (`None`) resolves to [`WindowGranularity::District`] (today's default + /// behavior, byte-compatible with every pre-T-1150 caller) via + /// [`resolve_window_granularity_v2`] — **never trusted from the wire**, + /// unrecognized/absent values fall back to district. /// - /// **Type seam (Tyre C2, PR #191 review):** this field expresses ONLY - /// finer-than-district integer multiples of the district spacing — `1` - /// and `4` are legal today, and each new finer rung (e.g. a future - /// block/tile value) is a deliberate widening of - /// [`resolve_window_granularity`]'s whitelist, the single point where - /// that widening happens. It CANNOT express coarser-than-district rungs - /// (region/orbital, granularity < 1) — reusing this field for those is - /// explicitly out of scope; design doc §5/§9 R5 requires a - /// signed/log-scale value or an explicit rung enum instead. Do not smuggle - /// a "granularity 0 means region" convention into this `u32` — that is - /// the redesign R5 already flags, not a value to add here. - #[serde(default)] - pub window_granularity: u32, - /// The R5-redesigned granularity vocabulary (T-1152), able to express - /// coarser-than-district rungs the legacy `window_granularity: u32` - /// cannot (Tyre's wire-contract note — see [`WindowGranularity`]'s doc - /// for the full rationale). `#[serde(default)]` (`None`) is the absent - /// case: every pre-T-1152 client (and every T-1150 client that only ever - /// sends the legacy `u32`) omits this field entirely and is byte-compatible - /// — [`resolve_window_granularity_v2`] falls back to the legacy field - /// when this is `None`. When BOTH fields are present, THIS field wins - /// (documented once, on [`WindowGranularity`], not duplicated here). + /// **T-1159:** the legacy `window_granularity: u32` field this superseded + /// (T-1150's finer-than-district-only encoding) is retired — this enum + /// fully shadowed it since T-1152 landed, and no pre-T-1152 client exists + /// (single-repo client/server pair). See D-255(c): the `district_window` + /// carrier itself stays alive byte-unchanged for its existing consumer; + /// only the redundant `u32` alongside this enum is gone. #[serde(default)] pub window_granularity_v2: Option, /// Octave cutoff for the invented-terrain scatter (T-1149's @@ -805,25 +721,14 @@ pub struct DistrictWindowLayer { /// (T-1150 design doc §2: "the window's `n` stays the DISTRICT extent"). /// The derived cell grid's actual side length is `n * granularity`. pub n: u32, - /// Derivation granularity (T-1150): [`WINDOW_GRANULARITY_DISTRICT`] (1) - /// or [`WINDOW_GRANULARITY_QUARTER`] (4) for finer-than-district rungs; - /// [`WINDOW_GRANULARITY_REGION_KEY`] (a reserved key-space tag, NOT a - /// spacing multiplier) when [`Self::granularity_v2`] is `Region` — see - /// that constant's doc. Kept unchanged (never repurposed or removed) for - /// wire back-compat with every pre-T-1152 client, which reads only this - /// field and has no concept of `granularity_v2`. Echoed so the client's - /// cache key and staleness guard can distinguish windows at different - /// finer-than-district rungs requested at the identical `(center, n)`. - pub granularity: u32, /// The R5-redesigned granularity (T-1152) — see [`WindowGranularity`]'s /// doc. Always populated (never `None`): the server always resolves a - /// concrete rung internally via [`resolve_window_granularity_v2`] - /// regardless of which wire field the request used, so the response - /// always carries the enum echo alongside the legacy `u32` one. A - /// T-1152-aware client reads THIS field for staleness/cache-key - /// comparison; `granularity` (above) exists only for pre-T-1152 clients, - /// who never see `Region` responses in the first place (they have no way - /// to request one). + /// concrete rung internally via [`resolve_window_granularity_v2`]. + /// + /// **T-1159:** the legacy `granularity: u32` echo this field used to sit + /// alongside (a wire-back-compat value for pre-T-1152 clients, which do + /// not exist — single-repo client/server pair) is retired. This enum + /// echo has been the sole granularity signal since T-1152. pub granularity_v2: WindowGranularity, /// The `min_wavelength_m` octave cutoff (T-1149) this window was derived /// with, in whole metres (`0` = no cutoff). Echoed for the same reason as @@ -1616,7 +1521,6 @@ pub fn build_district_window_layer( DistrictWindowLayer { center, n, - granularity: granularity.key_u32(), granularity_v2: granularity, min_wl_m, morphology, @@ -1713,7 +1617,6 @@ fn build_district_window_layer_serial( DistrictWindowLayer { center, n, - granularity: granularity.key_u32(), granularity_v2: granularity, min_wl_m, morphology, @@ -2295,6 +2198,11 @@ pub fn handle_atlas_request( // in DistrictProfile.basin_direction (BodyWorldState.districts) and is // not needed again here. Supply an empty map. survey_basin_dirs: std::collections::BTreeMap::new(), + // T-1169: mirrors state.attractors.clone() above — feature_names + // is stored on BodyWorldState (see cascade::into_body_world_state) + // and cloned back out here, same reconstruction discipline as + // every other Layer1Output field on this cache-hit path. + feature_names: state.feature_names.clone(), }; let district_grid = build_district_grid(state); let road_graph = build_road_graph_layer(state); @@ -2346,6 +2254,36 @@ pub fn handle_atlas_request( } None => (Vec::new(), None), }; + // Pre-resolve this body's reserved river/mountain name pools so + // the Rayon work item stays DB-free (T-1169, D-223, same D-225 + // pattern as `cities` above). Read failure is non-fatal: log and + // fall back to no names (attach_feature_names degrades gracefully + // — every attractor simply gets no name). + let (river_names, mountain_names) = match city_reader { + Some(reader) => match reader.read_body_feature_names(&req.body_id) { + Ok(rows) => { + let mut rivers = Vec::new(); + let mut mountains = Vec::new(); + for row in rows { + match row.feature_type.as_str() { + "river" => rivers.push(row.name), + "mountain" => mountains.push(row.name), + _ => {} + } + } + (rivers, mountains) + } + Err(e) => { + tracing::warn!( + body_id = %req.body_id, + error = %e, + "feature name read failed; attaching no names" + ); + (Vec::new(), Vec::new()) + } + }, + None => (Vec::new(), Vec::new()), + }; // Pre-resolve body physical params so the Rayon work item stays // DB-free (D-225 pattern). Read failures are non-fatal: log and // fall back to None (cascade stops at Settlement, pre-T-1032 @@ -2373,6 +2311,8 @@ pub fn handle_atlas_request( cities, dominant_faction, body_params, + river_names, + mountain_names, }, GenPriority::Immediate, ); @@ -2457,6 +2397,7 @@ mod tests { river_network: RiverNetwork::default(), drainage_basins: vec![], attractors: vec![], + feature_names: vec![], placements: vec![], road_graph: crate::atlas::road_graph::RoadGraph::default(), quarters: std::collections::BTreeMap::new(), @@ -2636,7 +2577,6 @@ mod tests { assert_eq!(layer.center, (10, -5)); assert_eq!(layer.n, n); - assert_eq!(layer.granularity, WINDOW_GRANULARITY_DISTRICT); assert_eq!(layer.granularity_v2, WindowGranularity::District); assert_eq!(layer.min_wl_m, 0); let cells = (n * n) as usize; @@ -3894,7 +3834,6 @@ mod tests { let mk = |center, n| DistrictWindowLayer { center, n, - granularity: WINDOW_GRANULARITY_DISTRICT, granularity_v2: WindowGranularity::District, min_wl_m: 0, morphology: vec![0; (n * n) as usize], @@ -3943,7 +3882,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: Some((0, 0)), window_n: DISTRICT_WINDOW_MAX_N * 10, // wildly over the wire — must clamp, not trust - window_granularity: 0, window_granularity_v2: None, window_min_wl_m: 0, }; @@ -4004,8 +3942,7 @@ mod tests { up_to: CascadeLayer::Topography, window_center: Some((0, 0)), window_n: 32, - window_granularity: WINDOW_GRANULARITY_QUARTER, - window_granularity_v2: None, + window_granularity_v2: Some(WindowGranularity::Quarter), window_min_wl_m: 0, }; @@ -4035,7 +3972,8 @@ mod tests { }); let layer = window_completion.expect("DeriveWindow must complete for GJ1c"); assert_eq!( - layer.granularity, WINDOW_GRANULARITY_QUARTER, + layer.granularity_v2, + WindowGranularity::Quarter, "granularity must echo back as requested (4 is within budget on its own)" ); assert_eq!( @@ -4045,36 +3983,9 @@ mod tests { } // ------------------------------------------------------------------- - // resolve_window_granularity / clamp_window_n (T-1150) + // clamp_window_n (T-1150) // ------------------------------------------------------------------- - #[test] - fn resolve_window_granularity_maps_known_values() { - assert_eq!(resolve_window_granularity(0), WINDOW_GRANULARITY_DISTRICT); - assert_eq!( - resolve_window_granularity(WINDOW_GRANULARITY_DISTRICT), - WINDOW_GRANULARITY_DISTRICT - ); - assert_eq!( - resolve_window_granularity(WINDOW_GRANULARITY_QUARTER), - WINDOW_GRANULARITY_QUARTER - ); - } - - /// Never trust the wire: an unrecognized granularity value (garbage, or a - /// future rung not yet implemented) falls back to district, never panics - /// or propagates un-vetted. - #[test] - fn resolve_window_granularity_unknown_value_falls_back_to_district() { - for garbage in [2, 3, 5, 100, u32::MAX] { - assert_eq!( - resolve_window_granularity(garbage), - WINDOW_GRANULARITY_DISTRICT, - "unrecognized granularity {garbage} must fall back to district" - ); - } - } - /// District granularity: the per-axis DISTRICT_WINDOW_MAX_N cap alone /// governs (64² × 1² = 4,096 = WIRE_CAP_CELLS exactly, so the cap is /// never tighter than DISTRICT_WINDOW_MAX_N at granularity 1). @@ -4283,7 +4194,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: Some((10, -5)), window_n: 4, - window_granularity: WINDOW_GRANULARITY_DISTRICT, window_granularity_v2: None, window_min_wl_m: 4_000, }; @@ -4292,7 +4202,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: Some((10, -5)), window_n: 4, - window_granularity: WINDOW_GRANULARITY_DISTRICT, window_granularity_v2: None, window_min_wl_m: 4_300, }; @@ -4510,7 +4419,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: Some((12276, 3021)), window_n: 4, - window_granularity: 0, window_granularity_v2: None, window_min_wl_m: 0, }; @@ -4567,7 +4475,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: Some((4, 383)), // the hand-computed canonical twin window_n: 4, - window_granularity: 0, window_granularity_v2: None, window_min_wl_m: 0, }; @@ -4650,7 +4557,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: center, window_n: n, - window_granularity: WINDOW_GRANULARITY_DISTRICT, window_granularity_v2: None, window_min_wl_m: 0, }; @@ -4659,8 +4565,7 @@ mod tests { up_to: CascadeLayer::Topography, window_center: center, window_n: n, - window_granularity: WINDOW_GRANULARITY_QUARTER, - window_granularity_v2: None, + window_granularity_v2: Some(WindowGranularity::Quarter), window_min_wl_m: 0, }; @@ -4750,8 +4655,6 @@ mod tests { .district_window .expect("quarter request must hit its own cached entry"); - assert_eq!(district_layer.granularity, WINDOW_GRANULARITY_DISTRICT); - assert_eq!(quarter_layer.granularity, WINDOW_GRANULARITY_QUARTER); assert_eq!(district_layer.granularity_v2, WindowGranularity::District); assert_eq!(quarter_layer.granularity_v2, WindowGranularity::Quarter); // n echoes the DISTRICT extent unchanged at both granularities @@ -4810,7 +4713,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: center, window_n: n, - window_granularity: WINDOW_GRANULARITY_DISTRICT, window_granularity_v2: None, window_min_wl_m: 0, }; @@ -4819,9 +4721,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: center, window_n: n, - // Legacy field is irrelevant here — window_granularity_v2 takes - // precedence per resolve_window_granularity_v2's documented rule. - window_granularity: 0, window_granularity_v2: Some(WindowGranularity::Region), window_min_wl_m: 0, }; @@ -4911,12 +4810,7 @@ mod tests { assert_eq!(district_layer.granularity_v2, WindowGranularity::District); assert_eq!(region_layer.granularity_v2, WindowGranularity::Region); - // The legacy u32 echo must NEVER collide with a real multiplier — - // WINDOW_GRANULARITY_REGION_KEY is the reserved key-space tag (see - // that constant's doc), distinct from both WINDOW_GRANULARITY_DISTRICT - // (1) and WINDOW_GRANULARITY_QUARTER (4). - assert_eq!(region_layer.granularity, WINDOW_GRANULARITY_REGION_KEY); - assert_ne!(region_layer.granularity, district_layer.granularity); + assert_ne!(region_layer.granularity_v2, district_layer.granularity_v2); // n echoes the DISTRICT extent unchanged (design doc §2), same as // every other rung — the derived CELL GRID is what differs. @@ -4959,7 +4853,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: Some((0, 0)), window_n: DISTRICT_WINDOW_MAX_N_REGION * 10, - window_granularity: 0, window_granularity_v2: Some(WindowGranularity::Region), window_min_wl_m: 0, }; @@ -5019,7 +4912,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: Some((12276, 3021)), // raw, out-of-range window_n: 4, - window_granularity: 0, window_granularity_v2: None, window_min_wl_m: 0, }; @@ -5100,7 +4992,6 @@ mod tests { let window = DistrictWindowLayer { center: (10, -5), n: 2, - granularity: WINDOW_GRANULARITY_DISTRICT, granularity_v2: WindowGranularity::District, min_wl_m: 0, morphology: vec![0, 8, 14, 16], @@ -5161,8 +5052,8 @@ mod tests { assert_eq!(decoded.window_center, None); assert_eq!(decoded.window_n, 0); assert_eq!( - decoded.window_granularity, 0, - "T-1150: absent window_granularity decodes to 0 (district), byte-compatible" + decoded.window_granularity_v2, None, + "T-1152/T-1159: absent window_granularity_v2 decodes to None (district), byte-compatible" ); assert_eq!( decoded.window_min_wl_m, 0, @@ -5514,6 +5405,7 @@ mod tests { river_network: RiverNetwork::default(), drainage_basins: vec![], attractors: vec![], + feature_names: vec![], placements: vec![], road_graph: crate::atlas::road_graph::RoadGraph::default(), quarters: std::collections::BTreeMap::new(), @@ -5784,7 +5676,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: None, window_n: 0, - window_granularity: 0, window_granularity_v2: None, window_min_wl_m: 0, } @@ -5861,6 +5752,7 @@ mod tests { river_network: RiverNetwork::default(), drainage_basins: vec![], attractors: vec![], + feature_names: vec![], placements: vec![], road_graph: crate::atlas::road_graph::RoadGraph::default(), quarters: std::collections::BTreeMap::new(), diff --git a/server/src/atlas/plugin.rs b/server/src/atlas/plugin.rs index 574581e36..8565d5e53 100644 --- a/server/src/atlas/plugin.rs +++ b/server/src/atlas/plugin.rs @@ -15,7 +15,8 @@ use std::collections::BTreeMap; use std::sync::Arc; use crate::atlas::atlas_data_proxy::{ - handle_city_names_request, handle_star_map_request, StarMapDataPath, + handle_city_names_request, handle_feature_names_request, handle_star_map_request, + StarMapDataPath, }; use crate::atlas::attractor_matching::CityPlacement; use crate::atlas::body_params_reader::BodyParamsReaderResource; @@ -52,7 +53,8 @@ use crate::atlas::trait_swerve::{ }; use crate::bridge::{ AtlasRequestBuffer, AtlasResponseBuffer, BrowseRequestBuffer, BrowseResponseBuffer, - CityNamesRequestBuffer, CityNamesResponseBuffer, StarMapRequestBuffer, StarMapResponseBuffer, + CityNamesRequestBuffer, CityNamesResponseBuffer, FeatureNamesRequestBuffer, + FeatureNamesResponseBuffer, StarMapRequestBuffer, StarMapResponseBuffer, StepCanvasRequestBuffer, StepCanvasResponseBuffer, }; use crate::seed::{SeedChain, SeedDomain}; @@ -83,6 +85,10 @@ impl Plugin for GenerationPlugin { Update, serve_city_names_requests.in_set(TickPhase::PreInput), ) + .add_systems( + Update, + serve_feature_names_requests.in_set(TickPhase::PreInput), + ) .add_systems(Update, serve_browse_requests.in_set(TickPhase::PreInput)) .add_systems( Update, @@ -201,6 +207,26 @@ fn serve_city_names_requests( } } +/// Drain inbound feature-names requests and serve each through the proxy +/// (T-1169): D-236 Sol check, then the names-only `atlas_feature_names` read. +/// Mirrors [`serve_city_names_requests`] exactly. +fn serve_feature_names_requests( + mut requests: ResMut, + mut responses: ResMut, + city_reader: Option>, +) { + if requests.0.is_empty() { + return; + } + let reader = city_reader.as_ref().map(|r| &r.0); + let pending: Vec<_> = requests.0.drain(..).collect(); + for (conn_id, req) in pending { + responses + .0 + .push((conn_id, handle_feature_names_request(&req, reader))); + } +} + /// Drain inbound data-browser requests and serve each through the proxy /// (D-254 §4, T-1131): one of the six v1 registry-tier entity kinds, dispatched /// to `BrowseReader` by `(kind, query)`. @@ -1043,6 +1069,8 @@ mod tests { cities: vec![], dominant_faction: None, body_params: None, // T-1023: no DB params in this unit test + river_names: vec![], + mountain_names: vec![], }, GenPriority::Immediate, ); @@ -1079,7 +1107,6 @@ mod tests { up_to: CascadeLayer::Topography, window_center: None, window_n: 0, - window_granularity: 0, window_granularity_v2: None, window_min_wl_m: 0, }, @@ -1193,6 +1220,36 @@ mod tests { assert!(world.resource::().0.is_empty()); } + #[test] + fn serve_feature_names_without_reader_is_error() { + use crate::atlas::atlas_data_proxy::{FeatureNamesRequest, FeatureNamesStatus}; + + let mut world = World::new(); + world.insert_resource(FeatureNamesRequestBuffer(vec![( + ConnectionId(0), + FeatureNamesRequest { + feature_names: true, + body_id: "GJ1c".to_string(), + }, + )])); + world.insert_resource(FeatureNamesResponseBuffer::default()); + // No CityContextReaderResource. + + let mut sched = Schedule::default(); + sched.add_systems(serve_feature_names_requests); + sched.run(&mut world); + + let responses = world.resource::(); + assert_eq!(responses.0.len(), 1); + assert_eq!(responses.0[0].0, ConnectionId(0), "connection id preserved"); + assert_eq!(responses.0[0].1.body_id, "GJ1c"); + assert!(matches!( + responses.0[0].1.status, + FeatureNamesStatus::Error(_) + )); + assert!(world.resource::().0.is_empty()); + } + fn sample_read_set() -> CityEconomicReadSet { use crate::simulation::generator::SettlementClass; CityEconomicReadSet { diff --git a/server/src/bridge/local.rs b/server/src/bridge/local.rs index 242ec5410..92a1d11fa 100644 --- a/server/src/bridge/local.rs +++ b/server/src/bridge/local.rs @@ -3,7 +3,7 @@ // Deterministic client-server communication via Unix domain sockets use super::{decode_inbound, BridgeError, Inbound, ObserverSnapshot, SimBridge}; -use crate::atlas::atlas_data_proxy::{CityNamesResponse, StarMapResponse}; +use crate::atlas::atlas_data_proxy::{CityNamesResponse, FeatureNamesResponse, StarMapResponse}; use crate::atlas::browse_proxy::BrowseResponse; use crate::atlas::layer_proxy::AtlasLayerResponse; use crate::atlas::step_canvas::StepCanvasResponse; @@ -169,6 +169,16 @@ impl SimBridge for LocalBridge { Ok(()) } + fn send_feature_names_response(&self, resp: &FeatureNamesResponse) -> Result<(), BridgeError> { + let payload = rmp_serde::to_vec_named(resp)?; + let mut writer = self + .writer + .lock() + .map_err(|e| BridgeError::MutexPoisoned(format!("writer: {}", e)))?; + write_framed(writer.get_mut(), &payload)?; + Ok(()) + } + fn send_browse_response(&self, resp: &BrowseResponse) -> Result<(), BridgeError> { let payload = rmp_serde::to_vec_named(resp)?; let mut writer = self diff --git a/server/src/bridge/mod.rs b/server/src/bridge/mod.rs index 085c63faa..79b1bc43f 100644 --- a/server/src/bridge/mod.rs +++ b/server/src/bridge/mod.rs @@ -7,7 +7,8 @@ use bevy_ecs::prelude::*; use bevy_ecs::schedule::IntoScheduleConfigs; use crate::atlas::atlas_data_proxy::{ - CityNamesRequest, CityNamesResponse, StarMapRequest, StarMapResponse, + CityNamesRequest, CityNamesResponse, FeatureNamesRequest, FeatureNamesResponse, + StarMapRequest, StarMapResponse, }; use crate::atlas::browse_proxy::{BrowseRequest, BrowseResponse}; use crate::atlas::layer_proxy::{AtlasLayerRequest, AtlasLayerResponse}; @@ -80,7 +81,11 @@ pub enum BridgeError { /// exactly this kind of required marker field — there is no natural ceiling /// on the DEMUX mechanism itself, only a discipline reminder that a new /// shape should justify why it can't ride an existing one (as -/// `StepCanvasRequest` does, D-255(c)). +/// `StepCanvasRequest` does, D-255(c)). [`FeatureNamesRequest`] (T-1169) is +/// the SEVENTH shape, riding the identical discriminator convention +/// (`feature_names: bool` + `body_id`, D-236 pattern mirroring +/// `CityNamesRequest` exactly) — confirming the "no natural ceiling, just +/// justify the new shape" discipline this note predicted. #[derive(Debug)] pub enum Inbound { /// A batch of player inputs (the gameplay path). @@ -97,6 +102,9 @@ pub enum Inbound { /// A D-255(a) step-canvas data-canvas request (T-1181, the D-225 /// tagged-envelope migration, executed). StepCanvasRequest(StepCanvasRequest), + /// A per-body reserved-feature-names request (T-1169, D-236 pattern — + /// mirrors [`Self::CityNamesRequest`]). + FeatureNamesRequest(FeatureNamesRequest), } /// Key-presence probe for the defensive multi-shape check in @@ -111,14 +119,17 @@ struct ShapeProbe { city_names: Option, browse: Option, step_canvas: Option, + feature_names: Option, } /// Demux a received frame payload into an [`Inbound`] (D-225, T-949, T-1131, -/// T-1181). Tries, in order: `Vec` (array) → `AtlasLayerRequest` -/// (map, `body_id`+`up_to`) → `StarMapRequest` (map, `star_map` -/// discriminator) → `CityNamesRequest` (map, `city_names` discriminator + -/// `body_id`) → `BrowseRequest` (map, `browse` discriminator) → -/// `StepCanvasRequest` (map, `step_canvas` discriminator). +/// T-1181, T-1169). Tries, in order: `Vec` (array) → +/// `AtlasLayerRequest` (map, `body_id`+`up_to`) → `StarMapRequest` (map, +/// `star_map` discriminator) → `CityNamesRequest` (map, `city_names` +/// discriminator + `body_id`) → `BrowseRequest` (map, `browse` +/// discriminator) → `FeatureNamesRequest` (map, `feature_names` +/// discriminator + `body_id`) → `StepCanvasRequest` (map, `step_canvas` +/// discriminator). /// /// Mutual exclusivity is enforced, not assumed: no minimal well-formed /// instance of one shape satisfies another (see the [`Inbound`] doc), and a @@ -126,7 +137,7 @@ struct ShapeProbe { /// more than one shape — e.g. a buggy encoder emitting /// `{"star_map": true, "city_names": true, ...}` — instead of silently /// routing it to whichever shape is tried first (PR #176 review H1). A frame -/// satisfying none of the six shapes is a genuinely malformed input frame. +/// satisfying none of the seven shapes is a genuinely malformed input frame. pub fn decode_inbound(payload: &[u8]) -> Result { if let Ok(inputs) = rmp_serde::from_slice::>(payload) { return Ok(Inbound::Inputs(inputs)); @@ -141,21 +152,24 @@ pub fn decode_inbound(payload: &[u8]) -> Result { let city_names = probe.city_names.is_some(); let browse = probe.browse.is_some(); let step_canvas = probe.step_canvas.is_some(); + let feature_names = probe.feature_names.is_some(); let shapes = usize::from(atlas) + usize::from(star_map) + usize::from(city_names) + usize::from(browse) - + usize::from(step_canvas); + + usize::from(step_canvas) + + usize::from(feature_names); if shapes > 1 { let dump_len = payload.len().min(256); tracing::error!( - "inbound frame matches {} request shapes at once (atlas={}, star_map={}, city_names={}, browse={}, step_canvas={}) — rejecting ambiguous frame. Raw ({} of {} bytes): {:02x?}", + "inbound frame matches {} request shapes at once (atlas={}, star_map={}, city_names={}, browse={}, step_canvas={}, feature_names={}) — rejecting ambiguous frame. Raw ({} of {} bytes): {:02x?}", shapes, atlas, star_map, city_names, browse, step_canvas, + feature_names, dump_len, payload.len(), &payload[..dump_len] @@ -178,6 +192,9 @@ pub fn decode_inbound(payload: &[u8]) -> Result { if let Ok(req) = rmp_serde::from_slice::(payload) { return Ok(Inbound::BrowseRequest(req)); } + if let Ok(req) = rmp_serde::from_slice::(payload) { + return Ok(Inbound::FeatureNamesRequest(req)); + } match rmp_serde::from_slice::(payload) { Ok(req) => Ok(Inbound::StepCanvasRequest(req)), Err(e) => { @@ -229,6 +246,9 @@ pub trait SimBridge: Send + Sync { /// Send a city-names response to the client (T-949b). fn send_city_names_response(&self, resp: &CityNamesResponse) -> Result<(), BridgeError>; + /// Send a feature-names response to the client (T-1169). + fn send_feature_names_response(&self, resp: &FeatureNamesResponse) -> Result<(), BridgeError>; + /// Send a browse response to the client (T-1131). fn send_browse_response(&self, resp: &BrowseResponse) -> Result<(), BridgeError>; @@ -479,6 +499,27 @@ impl BridgeResource { } } + /// Send a feature-names response to exactly the connection that + /// requested it (T-1169 — mirrors [`Self::send_city_names_response_to`] + /// exactly). + pub fn send_feature_names_response_to( + &self, + id: ConnectionId, + resp: &FeatureNamesResponse, + ) -> Result<(), BridgeError> { + match self.connection(id) { + Some(c) => c.bridge.send_feature_names_response(resp), + None => { + tracing::debug!( + "feature names response for {:?} dropped — connection {:?} no longer present", + resp.body_id, + id + ); + Ok(()) + } + } + } + /// Send a browse response to exactly the connection that requested it /// (T-1131 — same per-connection routing D-254 §2 established for /// atlas/star-map/city-names). @@ -573,6 +614,7 @@ pub fn receive_bridge_inputs( mut city_names_requests: ResMut, mut browse_requests: ResMut, mut step_canvas_requests: ResMut, + mut feature_names_requests: ResMut, time: Option>, ) { let Some(mut bridge) = bridge else { return }; @@ -621,6 +663,9 @@ pub fn receive_bridge_inputs( Ok(Some(Inbound::StepCanvasRequest(req))) => { step_canvas_requests.0.push((player_id, req)); } + Ok(Some(Inbound::FeatureNamesRequest(req))) => { + feature_names_requests.0.push((player_id, req)); + } // No complete frame ready — the backlog is drained. Ok(None) => break, Err(BridgeError::Disconnected) => { @@ -738,6 +783,9 @@ pub fn receive_bridge_inputs( Ok(Some(Inbound::StepCanvasRequest(req))) => { step_canvas_requests.0.push((reader_id, req)); } + Ok(Some(Inbound::FeatureNamesRequest(req))) => { + feature_names_requests.0.push((reader_id, req)); + } Ok(None) => break, Err(BridgeError::Disconnected) => { tracing::info!("Reader connection {:?} disconnected", reader_id); @@ -952,6 +1000,37 @@ pub fn send_city_names_responses( } } +/// Inbound feature-names requests routed off the bridge (T-1169), drained by +/// the proxy serve system in `PreInput`. Connection-tagged (D-254 §2). +#[derive(Resource, Default)] +pub struct FeatureNamesRequestBuffer(pub Vec<(ConnectionId, FeatureNamesRequest)>); + +/// Outbound feature-names responses, filled by the proxy serve system and +/// flushed to the client in `PostSnapshot` (T-1169). Connection-tagged +/// (D-254 §2). +#[derive(Resource, Default)] +pub struct FeatureNamesResponseBuffer(pub Vec<(ConnectionId, FeatureNamesResponse)>); + +/// Flush buffered feature-names responses to their requesting connections +/// (T-1169; D-254 §2 per-connection routing). A failed send is logged but not +/// fatal. Mirrors [`send_city_names_responses`] exactly. +pub fn send_feature_names_responses( + bridge: Option>, + mut buffer: ResMut, +) { + let Some(bridge) = bridge else { return }; + for (id, resp) in buffer.0.drain(..) { + if let Err(e) = bridge.send_feature_names_response_to(id, &resp) { + tracing::warn!( + "failed to send feature names response for {} to {:?}: {}", + resp.body_id, + id, + e + ); + } + } +} + /// Inbound data-browser requests routed off the bridge (T-1131), drained by /// the proxy serve system in `PreInput`. Connection-tagged (D-254 §2). #[derive(Resource, Default)] @@ -1170,6 +1249,8 @@ impl Plugin for BridgePlugin { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -1198,6 +1279,10 @@ impl Plugin for BridgePlugin { Update, send_city_names_responses.in_set(TickPhase::PostSnapshot), ) + .add_systems( + Update, + send_feature_names_responses.in_set(TickPhase::PostSnapshot), + ) .add_systems( Update, send_browse_responses.in_set(TickPhase::PostSnapshot), @@ -1288,7 +1373,6 @@ mod inbound_tests { up_to: CascadeLayer::Topography, window_center: None, window_n: 0, - window_granularity: 0, window_granularity_v2: None, window_min_wl_m: 0, }; @@ -1324,6 +1408,56 @@ mod inbound_tests { )); } + /// T-1169: `FeatureNamesRequest` mirrors `CityNamesRequest`'s demux test + /// exactly — the seventh shape rides the same discriminator convention. + #[test] + fn demux_routes_feature_names_requests() { + let req = FeatureNamesRequest { + feature_names: true, + body_id: "GJ1c".into(), + }; + let frame = rmp_serde::to_vec_named(&req).unwrap(); + assert!(matches!( + decode_inbound(&frame), + Ok(Inbound::FeatureNamesRequest(r)) if r.body_id == "GJ1c" + )); + + // Cross-check: a CityNamesRequest frame must NOT decode as + // FeatureNamesRequest even though both key on `body_id` — the + // missing `feature_names` discriminator makes that a hard failure. + let city_names_frame = rmp_serde::to_vec_named(&CityNamesRequest { + city_names: true, + body_id: "GJ1c".into(), + }) + .unwrap(); + assert!(rmp_serde::from_slice::(&city_names_frame).is_err()); + assert!(rmp_serde::from_slice::(&frame).is_err()); + } + + /// PR #176 review H1's union-frame rejection extended to the seventh + /// shape (T-1169): a frame carrying BOTH `city_names` and + /// `feature_names` discriminators must be rejected, not silently routed + /// to whichever shape `decode_inbound` tries first. + #[test] + fn ambiguous_city_and_feature_names_union_frame_is_rejected() { + #[derive(serde::Serialize)] + struct CityAndFeature { + city_names: bool, + feature_names: bool, + body_id: String, + } + let frame = rmp_serde::to_vec_named(&CityAndFeature { + city_names: true, + feature_names: true, + body_id: "GJ1c".into(), + }) + .unwrap(); + assert!( + decode_inbound(&frame).is_err(), + "city_names+feature_names union frame must be rejected" + ); + } + /// T-949: the array-vs-map trick (D-225) still separates `Inputs` from /// everything else, and the three map shapes' discriminator fields keep /// them mutually exclusive — each of the four frame shapes decodes to @@ -1336,7 +1470,6 @@ mod inbound_tests { up_to: CascadeLayer::Topography, window_center: None, window_n: 0, - window_granularity: 0, window_granularity_v2: None, window_min_wl_m: 0, }) diff --git a/server/src/bridge/tcp.rs b/server/src/bridge/tcp.rs index 1566e6d2e..5a7006ab5 100644 --- a/server/src/bridge/tcp.rs +++ b/server/src/bridge/tcp.rs @@ -4,7 +4,7 @@ // Used for Godot client which lacks Unix socket support use super::{decode_inbound, BridgeError, Inbound, ObserverSnapshot, SimBridge}; -use crate::atlas::atlas_data_proxy::{CityNamesResponse, StarMapResponse}; +use crate::atlas::atlas_data_proxy::{CityNamesResponse, FeatureNamesResponse, StarMapResponse}; use crate::atlas::browse_proxy::BrowseResponse; use crate::atlas::layer_proxy::AtlasLayerResponse; use crate::atlas::step_canvas::StepCanvasResponse; @@ -316,6 +316,20 @@ impl SimBridge for TcpBridge { Ok(()) } + fn send_feature_names_response(&self, resp: &FeatureNamesResponse) -> Result<(), BridgeError> { + let payload = rmp_serde::to_vec_named(resp)?; + let mut writer = self + .writer + .lock() + .map_err(|e| BridgeError::MutexPoisoned(format!("writer: {}", e)))?; + let stream = writer.get_mut(); + stream.set_nonblocking(false).map_err(BridgeError::Io)?; + let result = write_framed(stream, &payload); + stream.set_nonblocking(true).map_err(BridgeError::Io)?; + result?; + Ok(()) + } + fn send_browse_response(&self, resp: &BrowseResponse) -> Result<(), BridgeError> { let payload = rmp_serde::to_vec_named(resp)?; let mut writer = self diff --git a/server/tests/bridge_tcp.rs b/server/tests/bridge_tcp.rs index 52b5cdf97..b5c9797bb 100644 --- a/server/tests/bridge_tcp.rs +++ b/server/tests/bridge_tcp.rs @@ -342,21 +342,23 @@ fn single_tick_drains_all_ready_inbound_frames() { use settled_reach_server::atlas::layer_proxy::AtlasLayerRequest; use settled_reach_server::bridge::{ receive_bridge_inputs, AtlasRequestBuffer, BridgeResource, BrowseRequestBuffer, - CityNamesRequestBuffer, HandshakeState, ServerRunning, StarMapRequestBuffer, - StepCanvasRequestBuffer, + CityNamesRequestBuffer, FeatureNamesRequestBuffer, HandshakeState, ServerRunning, + StarMapRequestBuffer, StepCanvasRequestBuffer, }; use settled_reach_server::simulation::input::InputQueue; let listener = TcpListener::bind("127.0.0.1:0").expect("failed to bind"); let server_addr = listener.local_addr().expect("failed to get local address"); - // Client: five frames back-to-back in one tick window — two input - // batches plus one of EACH request shape (atlas, star-map, city-names: - // the full D-225/T-949 demux surface over the real framing/poll path — - // PR #176 review H6). Returns the stream so it stays open until - // assertions complete (no EOF race). + // Client: six frames back-to-back in one tick window — two input + // batches plus one of EACH request shape (atlas, star-map, city-names, + // feature-names: the full D-225/T-949/T-1169 demux surface over the real + // framing/poll path — PR #176 review H6). Returns the stream so it stays + // open until assertions complete (no EOF race). let client_handle = thread::spawn(move || { - use settled_reach_server::atlas::atlas_data_proxy::{CityNamesRequest, StarMapRequest}; + use settled_reach_server::atlas::atlas_data_proxy::{ + CityNamesRequest, FeatureNamesRequest, StarMapRequest, + }; let mut stream = TcpStream::connect(server_addr).expect("failed to connect"); for tick in [20u64, 21] { @@ -372,7 +374,6 @@ fn single_tick_drains_all_ready_inbound_frames() { up_to: CascadeLayer::Topography, window_center: None, window_n: 0, - window_granularity: 0, window_granularity_v2: None, window_min_wl_m: 0, }; @@ -387,6 +388,13 @@ fn single_tick_drains_all_ready_inbound_frames() { }; let payload = rmp_serde::to_vec_named(&cn).expect("failed to serialize city names"); write_framed(&mut stream, &payload).expect("write city names frame"); + let fn_req = FeatureNamesRequest { + feature_names: true, + body_id: "GJ1c".into(), + }; + let payload = + rmp_serde::to_vec_named(&fn_req).expect("failed to serialize feature names"); + write_framed(&mut stream, &payload).expect("write feature names frame"); stream }); @@ -406,6 +414,7 @@ fn single_tick_drains_all_ready_inbound_frames() { world.init_resource::(); world.init_resource::(); world.init_resource::(); + world.init_resource::(); world .run_system_once(receive_bridge_inputs) @@ -433,6 +442,13 @@ fn single_tick_drains_all_ready_inbound_frames() { "the city-names request must drain in the same tick (H6: real wire path)" ); assert_eq!(city_names[0].1.body_id, "GJ1c"); + let feature_names = &world.resource::().0; + assert_eq!( + feature_names.len(), + 1, + "the feature-names request must drain in the same tick (T-1169: real wire path)" + ); + assert_eq!(feature_names[0].1.body_id, "GJ1c"); assert!( world.resource::().0, "draining must not shut the server down" @@ -502,8 +518,9 @@ fn new_multi_connection_world() -> (bevy_ecs::world::World, std::net::SocketAddr use settled_reach_server::bridge::{ AtlasRequestBuffer, AtlasResponseBuffer, BridgeResource, BrowseRequestBuffer, BrowseResponseBuffer, CityNamesRequestBuffer, CityNamesResponseBuffer, ConnectionListener, - HandshakeState, PendingConnections, ServerRunning, SnapshotBuffer, StarMapRequestBuffer, - StarMapResponseBuffer, StepCanvasRequestBuffer, StepCanvasResponseBuffer, + FeatureNamesRequestBuffer, FeatureNamesResponseBuffer, HandshakeState, PendingConnections, + ServerRunning, SnapshotBuffer, StarMapRequestBuffer, StarMapResponseBuffer, + StepCanvasRequestBuffer, StepCanvasResponseBuffer, }; use settled_reach_server::simulation::input::InputQueue; @@ -531,6 +548,8 @@ fn new_multi_connection_world() -> (bevy_ecs::world::World, std::net::SocketAddr world.init_resource::(); world.init_resource::(); world.init_resource::(); + world.init_resource::(); + world.init_resource::(); world.init_resource::(); (world, addr) } @@ -1515,7 +1534,6 @@ mod connection_zero_window_delivery { up_to: CascadeLayer::Topography, window_center: Some(center), window_n: 4, - window_granularity: 0, window_granularity_v2: Some(WindowGranularity::Region), window_min_wl_m: 0, } diff --git a/server/tests/cascade_golden.rs b/server/tests/cascade_golden.rs index dda47ece2..f2f5b1baa 100644 --- a/server/tests/cascade_golden.rs +++ b/server/tests/cascade_golden.rs @@ -112,6 +112,14 @@ //! additive at the position level, exactly as designed. No duplicate //! `(row, col)` positions exist in the final array (verified — the //! `edge_ids_are_unique` invariant `build_edges` depends on holds). +//! +//! **Fourth deliberate re-pin (T-1169, D-223):** `Layer1Output` gained an +//! additive `feature_names: Vec` field (river-mouth/ +//! alpine-peak name assignments from the new `attach_feature_names` cascade +//! wiring). This fixture calls `run_cascade_from_heightmap` with empty +//! `river_names`/`mountain_names` pools (no DB access from this test), so +//! the field serializes as `[]` — a pure additive-shape change, zero effect +//! on every pre-existing field. use std::path::PathBuf; @@ -149,8 +157,16 @@ fn cascade_layer0_to_1_matches_golden() { // ── Layer 1 — downsample, then run the cascade to topography. ─────────── let small = heightmap.downsample(DOWNSAMPLE.0, DOWNSAMPLE.1); let body_seed = SeedChain::for_body(WORLD_SEED, "GJ1c"); - let snapshot = - run_cascade_from_heightmap(body_seed, small, &[], None, None, CascadeLayer::Topography); + let snapshot = run_cascade_from_heightmap( + body_seed, + small, + &[], + None, + None, + &[], + &[], + CascadeLayer::Topography, + ); let layer1 = snapshot.layer1.expect("Layer 1 ran"); let actual = json!({ diff --git a/server/tests/gen_fixtures.rs b/server/tests/gen_fixtures.rs index 9e5862d78..57f423c7f 100644 --- a/server/tests/gen_fixtures.rs +++ b/server/tests/gen_fixtures.rs @@ -7,7 +7,6 @@ use settled_reach_server::atlas::layer_proxy::{ AtlasLayerResponse, AtlasLayerStatus, DistrictWindowLayer, QuarterFootprintEntry, QuarterFootprintLayer, RegionGridLayer, RoadGraphEdge, RoadGraphLayer, RoadGraphNode, SettlementEntry, SettlementLayer, SettlementSizeClass, WindowGranularity, REGION_TEMP_NONE_DC, - WINDOW_GRANULARITY_DISTRICT, }; use settled_reach_server::atlas::region_profile::{SeasonPhase, WeatherState}; use settled_reach_server::atlas::road_graph::RoadNodeKind; @@ -579,6 +578,7 @@ fn generate_atlas_layer_response_fixtures() { grid_w: 512, grid_h: 256, survey_basin_dirs: std::collections::BTreeMap::new(), + feature_names: vec![], }; // T-960 §1/§2: a small populated RoadGraphLayer + SettlementLayer, one // settlement (a capital) connected to one waypoint-free short edge. @@ -729,7 +729,6 @@ fn generate_atlas_layer_response_fixtures() { let window = DistrictWindowLayer { center: (10, -5), n: 2, - granularity: WINDOW_GRANULARITY_DISTRICT, granularity_v2: WindowGranularity::District, min_wl_m: 0, morphology: vec![0, 8, 14, 16], // OpenOcean, AlluvialPlain, Alpine, Wetland diff --git a/server/tests/golden/cascade_layer1.json b/server/tests/golden/cascade_layer1.json index 5e25ace28..d857aaacf 100644 --- a/server/tests/golden/cascade_layer1.json +++ b/server/tests/golden/cascade_layer1.json @@ -7761,6 +7761,7 @@ "territorial_status": "FrontierUnclaimed" } ], + "feature_names": [], "grid_h": 128, "grid_w": 256, "river_network": { diff --git a/tooling/economy-db/economy_import/atlas.py b/tooling/economy-db/economy_import/atlas.py index 7781f4cfd..d6c101972 100644 --- a/tooling/economy-db/economy_import/atlas.py +++ b/tooling/economy-db/economy_import/atlas.py @@ -164,6 +164,93 @@ def populate_atlas_city_names(conn: sqlite3.Connection, dry_run: bool) -> int: return len(rows) +# markers.json `names` key -> atlas_feature_names.feature_type value. Only +# rivers/mountain_ranges are populated here (T-1169 scope — the two pools +# server/src/atlas/layer1.rs's `attach_feature_names` actually consumes, +# river_names/mountain_names). `oceans`/`passes`/`regions` pools are NOT +# wired to any production consumer yet (no `attach_feature_names`-equivalent +# reads them) — populating rows for them here would be dead data with +# nothing to keep it honest, the same D-223 discipline that keeps +# atlas_city_names to names an actual placement pipeline consumes. +_FEATURE_NAME_POOL_KEYS: dict[str, str] = { + "rivers": "river", + "mountain_ranges": "mountain", +} + + +def populate_atlas_feature_names(conn: sqlite3.Connection, dry_run: bool) -> int: + """Populate atlas_feature_names from the names-only markers.json pools + (D-223, T-1169) — mirrors `populate_atlas_city_names` exactly, over the + `rivers`/`mountain_ranges` marker keys instead of `cities`: + - body_id : directory name (e.g. GJ0e) + - name : pooled feature name + - feature_type : 'river' | 'mountain' (see `_FEATURE_NAME_POOL_KEYS`) + - priority : 0 — no authored ranking signal in markers.json; the + server-side name attachment (`layer1::attach_feature_names`) + ranks by computed mouth/peak strength at assignment time, + not by a priority stored on the pool row. + + markers.json is a names-only flavoured pool (D-223): it carries no + geometry or position. `attach_feature_names` (Phase 4 cascade) attaches + these names to computed river mouths / alpine peaks at generation time; + this importer just loads the pool, exactly as `populate_atlas_city_names` + loads the city pool for the settlement placement pipeline. + + Deterministic rebuild: clears atlas_feature_names first, so re-runs are + idempotent — there is no UNIQUE(body_id, name, feature_type), so without + the clear a re-run would accumulate duplicates. Skips body directories not + found in the bodies table (missing FK), and Sol bodies (D-223 permanent + exemption — same as `populate_atlas_city_names`, Sol keeps authored + geometry-bearing markers.json via sol_import.py, not the names pool). + """ + valid_body_ids: set[str] = { + r[0] for r in conn.execute("SELECT body_id FROM bodies").fetchall() + } + sol_body_ids: set[str] = { + r[0] for r in conn.execute( + "SELECT body_id FROM bodies WHERE system_id = 'GJ 0'" + ).fetchall() + } + + rows: list[tuple] = [] + skipped_bodies: list[str] = [] + + pattern = str(WIKI_STAR_SYSTEMS / "*" / "bodies" / "*" / "markers.json") + for markers_path in sorted(glob.glob(pattern)): + body_id = markers_path.split("/bodies/")[1].split("/")[0] + if body_id not in valid_body_ids or body_id in sol_body_ids: + if body_id not in valid_body_ids: + skipped_bodies.append(body_id) + continue + + with open(markers_path) as fh: + data = json.load(fh) + + names = data.get("names") or {} + for pool_key, feature_type in _FEATURE_NAME_POOL_KEYS.items(): + for raw_name in names.get(pool_key) or []: + name = (raw_name or "").strip() + if not name: + continue + rows.append((body_id, name, feature_type, 0)) + + if skipped_bodies: + unique = sorted(set(skipped_bodies)) + print(f" warning: {len(unique)} body dirs not in DB — skipped: {unique[:5]}") + + if not dry_run: + conn.execute("DELETE FROM atlas_feature_names") + if rows: + conn.executemany( + """INSERT INTO atlas_feature_names + (body_id, name, feature_type, priority) + VALUES (?, ?, ?, ?)""", + rows, + ) + + return len(rows) + + # body_type rank for the most_populated_body_in_system LAST-RESORT tiebreak — # lower sorts first. This is NOT "belts can't host settlements" (they can and # DO in this world: GJ845-belt hosts Orkney Ceramics, GJ268-belt hosts Jeju diff --git a/tooling/economy-db/import_economics.py b/tooling/economy-db/import_economics.py index cc05b274d..a6523ba6a 100755 --- a/tooling/economy-db/import_economics.py +++ b/tooling/economy-db/import_economics.py @@ -228,6 +228,15 @@ def main() -> None: n_cities = atlas.populate_atlas_city_names(conn, args.dry_run) print(f" {n_cities} city name rows") + # 12b. atlas_feature_names (rivers/mountains) from wiki markers.json + # (D-223, T-1169) — mirrors step 12's pool-load shape exactly, over a + # different names-only markers.json key set. Independent of the + # settlement pool, so order relative to step 13 doesn't matter; placed + # here to stay adjacent to its sibling pool-load step. + print(" [12b/13] Populating atlas_feature_names from wiki content...") + n_features = atlas.populate_atlas_feature_names(conn, args.dry_run) + print(f" {n_features} feature name rows") + # 13. Standalone-HQ settlements + CityTenant city links (D-242, T-1074) — Phase B. # SUPERSEDES the retired corp-HQ cross-reference (D-207, #909) — that # step inserted one atlas_city_names row per corp HQ with no