fix(simulation): address PR #125 review — tick truncation, ordering, perf, protocol test
- Widen EventPort tick methods from u32 to u64 (prevents overflow) - Add is_identity() guard on hot-path String allocation in modifiers - Replace Vec::remove(0) with VecDeque::pop_front() in price history - Add .after(tick_economy_simulation) ordering for debug commands - Fix stale PROTOCOL_VERSION assertion (20 → 21) in serialization test - Add D-181 Phase 2 visibility scope comment on serve_econ_state_query - Eliminate double lookup in rebuild_signals via single-pass extraction - Track economy seed TODO with backlog ticket reference Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -227,6 +227,7 @@ impl Plugin for BridgePlugin {
|
||||
receive_bridge_inputs.before(crate::simulation::input::process_player_input),
|
||||
debug::handle_debug_commands
|
||||
.after(crate::simulation::input::process_player_input)
|
||||
.after(crate::simulation::economy::tick_economy_simulation)
|
||||
.before(crate::perception::observer::compute_observer_snapshot),
|
||||
crate::perception::observer::compute_visibility_geometry
|
||||
.after(crate::simulation::movement::validate_movement),
|
||||
|
||||
@@ -31,7 +31,7 @@
|
||||
//! 6. `production_vs_baseline` — supply ÷ initial baseline supply (Private)
|
||||
//! 7. `official_coverage_ratio` — 1 − shadow_intensity (Meta-signal)
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::{BTreeMap, VecDeque};
|
||||
|
||||
use bevy_ecs::prelude::*;
|
||||
use econ_sim::Simulation;
|
||||
@@ -61,7 +61,7 @@ pub struct EconSimResource {
|
||||
pub sim: Simulation,
|
||||
/// Price history for signal 2 (price_trend) computation.
|
||||
/// Ring-buffer keyed by (system_id, commodity_id) → last TREND_WINDOW prices.
|
||||
price_history: BTreeMap<(String, String), Vec<f64>>,
|
||||
price_history: BTreeMap<(String, String), VecDeque<f64>>,
|
||||
/// Baseline supply from first economy tick for signal 6 (production_vs_baseline).
|
||||
baseline_supply: BTreeMap<(String, String), f64>,
|
||||
}
|
||||
@@ -188,15 +188,17 @@ fn rebuild_signals(
|
||||
|
||||
// Snapshot current node states into the price_history and baseline_supply maps,
|
||||
// then build signals. We need to separate the borrow from the iteration.
|
||||
let node_snapshots: Vec<(String, Vec<(String, f64, f64)>)> = econ_sim
|
||||
let node_snapshots: Vec<(String, Vec<(String, f64, f64, f64, f64)>)> = econ_sim
|
||||
.sim
|
||||
.nodes
|
||||
.iter()
|
||||
.map(|(system_id, node)| {
|
||||
let commodities: Vec<(String, f64, f64)> = node
|
||||
let commodities: Vec<(String, f64, f64, f64, f64)> = node
|
||||
.commodities
|
||||
.iter()
|
||||
.map(|(commodity_id, state)| (commodity_id.clone(), state.price, state.supply))
|
||||
.map(|(commodity_id, state)| {
|
||||
(commodity_id.clone(), state.price, state.supply, state.stockpile, state.demand)
|
||||
})
|
||||
.collect();
|
||||
(system_id.clone(), commodities)
|
||||
})
|
||||
@@ -206,7 +208,7 @@ fn rebuild_signals(
|
||||
let shadow_intensity = shadow_intensities.get(system_id).copied().unwrap_or(0.0);
|
||||
let corp_count = corp_counts.get(system_id).copied().unwrap_or(0);
|
||||
|
||||
for (commodity_id, price, supply) in commodities {
|
||||
for (commodity_id, price, supply, stockpile, demand) in commodities {
|
||||
let key = (system_id.clone(), commodity_id.clone());
|
||||
|
||||
// Signal 6 baseline: record first-tick supply
|
||||
@@ -218,9 +220,9 @@ fn rebuild_signals(
|
||||
|
||||
// Signal 2: price trend via ring buffer
|
||||
let history = econ_sim.price_history.entry(key.clone()).or_default();
|
||||
history.push(*price);
|
||||
history.push_back(*price);
|
||||
if history.len() > TREND_WINDOW {
|
||||
history.remove(0);
|
||||
history.pop_front();
|
||||
}
|
||||
let price_trend = if history.len() >= 2 {
|
||||
price - history[0]
|
||||
@@ -229,23 +231,9 @@ fn rebuild_signals(
|
||||
};
|
||||
|
||||
// Signal 5: stockpile in weeks (7 economy ticks per week approximation)
|
||||
let stockpile = econ_sim
|
||||
.sim
|
||||
.nodes
|
||||
.get(system_id)
|
||||
.and_then(|n| n.commodities.get(commodity_id))
|
||||
.map(|s| s.stockpile)
|
||||
.unwrap_or(0.0);
|
||||
let demand = econ_sim
|
||||
.sim
|
||||
.nodes
|
||||
.get(system_id)
|
||||
.and_then(|n| n.commodities.get(commodity_id))
|
||||
.map(|s| s.demand)
|
||||
.unwrap_or(0.0);
|
||||
let weekly_demand = demand * 7.0; // 7 econ ticks ≈ 1 week
|
||||
let stockpile_weeks = if weekly_demand > 1e-9 {
|
||||
stockpile / weekly_demand
|
||||
*stockpile / weekly_demand
|
||||
} else {
|
||||
0.0
|
||||
};
|
||||
@@ -297,6 +285,11 @@ pub struct EconQueryBuffer {
|
||||
/// Runs after `tick_economy_simulation` (signals must be fresh) and before
|
||||
/// `compute_observer_snapshot` (which consumes the response).
|
||||
/// No-op when `EconStateResource` is absent or no query is pending.
|
||||
///
|
||||
/// **D-181 visibility (Phase 2):** All 7 signals are sent unfiltered.
|
||||
/// Phase 3 will gate signals 3–7 behind the D-181 visibility ladder
|
||||
/// (Observable → Semi-private → Private → Meta) based on the player's
|
||||
/// information access at the queried node.
|
||||
pub fn serve_econ_state_query(
|
||||
mut query_buf: ResMut<EconQueryBuffer>,
|
||||
econ_state: Option<Res<EconStateResource>>,
|
||||
|
||||
@@ -168,7 +168,7 @@ impl Plugin for SimulationPlugin {
|
||||
app.init_resource::<ticker::TickerPool>();
|
||||
|
||||
// Economy simulation (#821, D-031) — loaded once at startup, no-op when DB absent.
|
||||
// Uses seed 0 for now; will be threaded through StartupMessage world seed (#TODO).
|
||||
// Uses seed 0 for now; will be threaded through StartupMessage world seed (#826).
|
||||
if let Some((econ_sim, econ_state)) = economy::try_load_economy(0) {
|
||||
app.insert_resource(econ_sim).insert_resource(econ_state);
|
||||
}
|
||||
|
||||
@@ -360,7 +360,7 @@ fn protocol_version_constant_matches_snapshot() {
|
||||
let snapshot = test_snapshot(0, vec![]);
|
||||
assert_eq!(snapshot.version, PROTOCOL_VERSION);
|
||||
assert_eq!(
|
||||
PROTOCOL_VERSION, 20,
|
||||
PROTOCOL_VERSION, 21,
|
||||
"bump this assertion when protocol version changes"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -130,7 +130,7 @@ struct ActiveEvent {
|
||||
|
||||
struct ScheduledEvent {
|
||||
/// The simulation tick at which to activate this event.
|
||||
inject_at_tick: u32,
|
||||
inject_at_tick: u64,
|
||||
event: EconEvent,
|
||||
}
|
||||
|
||||
@@ -165,7 +165,7 @@ impl EventPort {
|
||||
///
|
||||
/// The event becomes active at the START of `inject_at_tick`, before
|
||||
/// `compute_modifiers` is called for that tick.
|
||||
pub fn push_at(&mut self, inject_at_tick: u32, event: EconEvent) {
|
||||
pub fn push_at(&mut self, inject_at_tick: u64, event: EconEvent) {
|
||||
self.scheduled.push(ScheduledEvent {
|
||||
inject_at_tick,
|
||||
event,
|
||||
@@ -175,7 +175,7 @@ impl EventPort {
|
||||
/// Activate any events scheduled for `current_tick`.
|
||||
///
|
||||
/// Call at the START of each tick, before `compute_modifiers`.
|
||||
pub fn activate_scheduled(&mut self, current_tick: u32) {
|
||||
pub fn activate_scheduled(&mut self, current_tick: u64) {
|
||||
// Stable Rust: partition scheduled list manually (no drain_filter).
|
||||
let mut still_pending = Vec::new();
|
||||
let mut to_activate = Vec::new();
|
||||
@@ -289,7 +289,11 @@ pub struct EventModifiers {
|
||||
impl EventModifiers {
|
||||
/// Combined demand multiplier for `(system_id, commodity_id)`.
|
||||
/// Returns `1.0` if no active demand shock targets this pair.
|
||||
/// Guards String allocation: fast-path returns 1.0 when no demand events are active.
|
||||
pub fn demand_for(&self, system_id: &str, commodity_id: &str) -> f64 {
|
||||
if self.demand.is_empty() {
|
||||
return 1.0;
|
||||
}
|
||||
*self
|
||||
.demand
|
||||
.get(&(system_id.to_string(), commodity_id.to_string()))
|
||||
@@ -297,7 +301,11 @@ impl EventModifiers {
|
||||
}
|
||||
|
||||
/// Combined productivity multiplier for `(system_id, commodity_id)`.
|
||||
/// Guards String allocation: fast-path returns 1.0 when no productivity events are active.
|
||||
pub fn productivity_for(&self, system_id: &str, commodity_id: &str) -> f64 {
|
||||
if self.productivity.is_empty() {
|
||||
return 1.0;
|
||||
}
|
||||
*self
|
||||
.productivity
|
||||
.get(&(system_id.to_string(), commodity_id.to_string()))
|
||||
@@ -305,7 +313,11 @@ impl EventModifiers {
|
||||
}
|
||||
|
||||
/// Combined capacity multiplier for `(system_id, commodity_id)`.
|
||||
/// Guards String allocation: fast-path returns 1.0 when no capacity events are active.
|
||||
pub fn capacity_for(&self, system_id: &str, commodity_id: &str) -> f64 {
|
||||
if self.capacity.is_empty() {
|
||||
return 1.0;
|
||||
}
|
||||
*self
|
||||
.capacity
|
||||
.get(&(system_id.to_string(), commodity_id.to_string()))
|
||||
|
||||
@@ -376,7 +376,7 @@ fn run_shock_response_test(
|
||||
// Schedule: inject 90% capacity disruption at tick WARMUP_TICKS
|
||||
let mut port = events::EventPort::new();
|
||||
port.push_at(
|
||||
WARMUP_TICKS,
|
||||
WARMUP_TICKS as u64,
|
||||
events::EconEvent {
|
||||
target: events::EconEventTarget::Node(shock_node.clone()),
|
||||
effect: events::EconEventEffect::CapacityMultiplier(0.1),
|
||||
|
||||
@@ -135,7 +135,7 @@ pub fn run_with_events(
|
||||
|
||||
for tick in 0..ticks {
|
||||
// Activate any events scheduled for this tick (D-180)
|
||||
events.activate_scheduled(tick);
|
||||
events.activate_scheduled(tick as u64);
|
||||
|
||||
let mods = events.compute_modifiers(economy);
|
||||
step_inner(economy, productivity, shadow, &archetypes, &mut nodes, &mods, ALPHA);
|
||||
|
||||
@@ -103,8 +103,7 @@ impl Simulation {
|
||||
/// event port. Call once per economy tick (every ECON_TICK_RATE game ticks).
|
||||
pub fn step(&mut self) {
|
||||
// Activate any events scheduled for this tick (D-180)
|
||||
let tick32 = self.tick as u32;
|
||||
self.events.activate_scheduled(tick32);
|
||||
self.events.activate_scheduled(self.tick);
|
||||
|
||||
let mods = self.events.compute_modifiers(&self.economy);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user