feat(simulation): feature-name pipeline wired + legacy window_granularity u32 retired (T-1169, T-1159)
One commit for two tickets whose changes share the bridge/plugin plumbing files. T-1169 connects the three dormant feature-name pieces: atlas_feature_names populated at regen (17,891 rows — 15,190 mountain, 2,701 river — via populate_atlas_feature_names mirroring the city-names importer; systems.db regenerated, stamp fresh), attach_feature_names wired into the cascade's Topography block with name pools threaded DB-free through AnalyzeBody (D-225 pattern) and assignments stored on Layer1Output/BodyWorldState for future consumers, and a FeatureNamesRequest/Response read proxy as the bridge's 7th tagged envelope (D-236 pattern, both SimBridge impls). Client label DRAW is deliberately NOT here — implementation proved both river and mountain labels need a wire-carried position (the pool is position-free; course polylines aren't correlated with the named attractors by construction) — deferred to T-1195's single design pass. cascade_layer1 golden re-pinned (additive feature_names field). T-1159 retires the legacy u32 granularity field fully shadowed by window_granularity_v2: AtlasLayerRequest.window_granularity, DistrictWindowLayer.granularity echo, the u32::MAX sentinel, and resolve_window_granularity are gone server-side; client encode paths and the caller-less atlas_window_cache legacy key component dropped; msgpack fixtures regenerated; the T-1150 aliasing regression test now drives through the surviving enum field. The district_window carrier itself survives byte-compatible per D-255(c). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
+145
-12
@@ -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<serde::de::IgnoredAny>,
|
||||
browse: Option<serde::de::IgnoredAny>,
|
||||
step_canvas: Option<serde::de::IgnoredAny>,
|
||||
feature_names: Option<serde::de::IgnoredAny>,
|
||||
}
|
||||
|
||||
/// Demux a received frame payload into an [`Inbound`] (D-225, T-949, T-1131,
|
||||
/// T-1181). Tries, in order: `Vec<PlayerInput>` (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<PlayerInput>` (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<Inbound, BridgeError> {
|
||||
if let Ok(inputs) = rmp_serde::from_slice::<Vec<PlayerInput>>(payload) {
|
||||
return Ok(Inbound::Inputs(inputs));
|
||||
@@ -141,21 +152,24 @@ pub fn decode_inbound(payload: &[u8]) -> Result<Inbound, BridgeError> {
|
||||
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<Inbound, BridgeError> {
|
||||
if let Ok(req) = rmp_serde::from_slice::<BrowseRequest>(payload) {
|
||||
return Ok(Inbound::BrowseRequest(req));
|
||||
}
|
||||
if let Ok(req) = rmp_serde::from_slice::<FeatureNamesRequest>(payload) {
|
||||
return Ok(Inbound::FeatureNamesRequest(req));
|
||||
}
|
||||
match rmp_serde::from_slice::<StepCanvasRequest>(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<CityNamesRequestBuffer>,
|
||||
mut browse_requests: ResMut<BrowseRequestBuffer>,
|
||||
mut step_canvas_requests: ResMut<StepCanvasRequestBuffer>,
|
||||
mut feature_names_requests: ResMut<FeatureNamesRequestBuffer>,
|
||||
time: Option<Res<crate::simulation::time::SimulationTime>>,
|
||||
) {
|
||||
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<Res<BridgeResource>>,
|
||||
mut buffer: ResMut<FeatureNamesResponseBuffer>,
|
||||
) {
|
||||
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::<StarMapResponseBuffer>()
|
||||
.init_resource::<CityNamesRequestBuffer>()
|
||||
.init_resource::<CityNamesResponseBuffer>()
|
||||
.init_resource::<FeatureNamesRequestBuffer>()
|
||||
.init_resource::<FeatureNamesResponseBuffer>()
|
||||
.init_resource::<BrowseRequestBuffer>()
|
||||
.init_resource::<BrowseResponseBuffer>()
|
||||
.init_resource::<StepCanvasRequestBuffer>()
|
||||
@@ -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::<FeatureNamesRequest>(&city_names_frame).is_err());
|
||||
assert!(rmp_serde::from_slice::<CityNamesRequest>(&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,
|
||||
})
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user