Addressed review comments

Cargo.lock

@@ -6710,7 +6710,6 @@ dependencies = [
  "crossbeam-utils 0.8.11",
  "csv",
  "dot_vox",
- "enum-iterator 1.1.3",
  "enum-map",
  "fxhash",
  "hashbrown 0.12.3",

client/src/lib.rs

@@ -1855,7 +1855,7 @@ impl Client {
             true,
             None,
             &self.connected_server_constants,
-            |_, _, _, _| {},
+            |_, _| {},
         );
         // TODO: avoid emitting these in the first place
         let _ = self

common/Cargo.toml

@@ -26,7 +26,6 @@ common-base = { package = "veloren-common-base", path = "base" }
 serde = { version = "1.0.110", features = ["derive", "rc"] }
 
 # Util
-enum-iterator = "1.1.3"
 enum-map = "2.4"
 vek = { version = "0.15.8", features = ["serde"] }
 cfg-if = "1.0.0"

common/src/character.rs

@@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
 
 /// The limit on how many characters that a player can have
 pub const MAX_CHARACTERS_PER_PLAYER: usize = 8;
-#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Hash)]
+#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)]
 #[serde(transparent)]
 pub struct CharacterId(pub i64);
 

common/src/cmd.rs

@@ -659,7 +659,7 @@ impl ServerChatCommand {
                     Enum("entity", ENTITIES.clone(), Required),
                     Integer("amount", 1, Optional),
                     Boolean("ai", "true".to_string(), Optional),
-                    Float("ai", 1.0, Optional),
+                    Float("scale", 1.0, Optional),
                 ],
                 "Spawn a test entity",
                 Some(Admin),

common/state/src/lib.rs

@@ -6,4 +6,4 @@ mod build_areas;
 mod state;
 // TODO: breakup state module and remove glob
 pub use build_areas::{BuildAreaError, BuildAreas};
-pub use state::{BlockChange, State, TerrainChanges};
+pub use state::{BlockChange, BlockDiff, State, TerrainChanges};

common/state/src/state.rs

@@ -113,6 +113,13 @@ impl TerrainChanges {
     }
 }
 
+#[derive(Clone)]
+pub struct BlockDiff {
+    pub wpos: Vec3<i32>,
+    pub old: Block,
+    pub new: Block,
+}
+
 /// A type used to represent game state stored on both the client and the
 /// server. This includes things like entity components, terrain data, and
 /// global states like weather, time of day, etc.
@@ -525,10 +532,7 @@ impl State {
     }
 
     // Apply terrain changes
-    pub fn apply_terrain_changes(
+    pub fn apply_terrain_changes(&self, block_update: impl FnMut(&specs::World, Vec<BlockDiff>)) {
-        &self,
-        block_update: impl FnMut(&specs::World, Vec3<i32>, Block, Block),
-    ) {
         self.apply_terrain_changes_internal(false, block_update);
     }
 
@@ -543,7 +547,7 @@ impl State {
     fn apply_terrain_changes_internal(
         &self,
         during_tick: bool,
-        mut block_update: impl FnMut(&specs::World, Vec3<i32>, Block, Block),
+        mut block_update: impl FnMut(&specs::World, Vec<BlockDiff>),
     ) {
         span!(
             _guard,
@@ -585,20 +589,30 @@ impl State {
         }
         // Apply block modifications
         // Only include in `TerrainChanges` if successful
-        modified_blocks.retain(|pos, new_block| {
+        let mut updated_blocks = Vec::with_capacity(modified_blocks.len());
-            let res = terrain.map(*pos, |old_block| {
+        modified_blocks.retain(|wpos, new| {
-                block_update(&self.ecs, *pos, old_block, *new_block);
+            let res = terrain.map(*wpos, |old| {
-                *new_block
+                updated_blocks.push(BlockDiff {
+                    wpos: *wpos,
+                    old,
+                    new: *new,
+                });
+                *new
             });
-            if let (&Ok(old_block), true) = (&res, during_tick) {
+            if let (&Ok(old), true) = (&res, during_tick) {
                 // NOTE: If the changes are applied during the tick, we push the *old* value as
                 // the modified block (since it otherwise can't be recovered after the tick).
                 // Otherwise, the changes will be applied after the tick, so we push the *new*
                 // value.
-                *new_block = old_block;
+                *new = old;
             }
             res.is_ok()
         });
+
+        if !updated_blocks.is_empty() {
+            block_update(&self.ecs, updated_blocks);
+        }
+
         self.ecs.write_resource::<TerrainChanges>().modified_blocks = modified_blocks;
     }
 
@@ -610,7 +624,7 @@ impl State {
         update_terrain_and_regions: bool,
         mut metrics: Option<&mut StateTickMetrics>,
         server_constants: &ServerConstants,
-        block_update: impl FnMut(&specs::World, Vec3<i32>, Block, Block),
+        block_update: impl FnMut(&specs::World, Vec<BlockDiff>),
     ) {
         span!(_guard, "tick", "State::tick");
 

rtsim/src/ai/mod.rs

@@ -184,6 +184,9 @@ pub trait Action<R = ()>: Any + Send + Sync {
     /// want to return one of many actions (each with different types) from
     /// the same function.
     ///
+    /// Note that [`Either`] can often be used to unify mismatched types without
+    /// the need for boxing.
+    ///
     /// # Example
     ///
     /// ```ignore
@@ -569,7 +572,7 @@ where
 /// The inner function will be run every tick to decide on an action. When an
 /// action is chosen, it will be performed until completed unless a different
 /// action of the same or higher priority is chosen in a subsequent tick.
-/// [`watch`] is very unfocussed and will happily switch between actions
+/// [`watch`] is very unfocused and will happily switch between actions
 /// rapidly between ticks if conditions change. If you want something that
 /// tends to commit to actions until they are completed, see [`choose`].
 ///

rtsim/src/data/mod.rs

@@ -24,8 +24,19 @@ use std::{
     marker::PhantomData,
 };
 
+/// The current version of rtsim data.
+///
+/// Note that this number does *not* need incrementing on every change: most
+/// field removals/additions are fine. This number should only be incremented
+/// when we wish to perform a *hard purge* of rtsim data.
+pub const CURRENT_VERSION: u32 = 0;
+
 #[derive(Clone, Serialize, Deserialize)]
 pub struct Data {
+    // Absence of field just implied version = 0
+    #[serde(default)]
+    pub version: u32,
+
     pub nature: Nature,
     #[serde(default)]
     pub npcs: Npcs,
@@ -46,7 +57,21 @@ pub struct Data {
     pub should_purge: bool,
 }
 
-pub type ReadError = rmp_serde::decode::Error;
+pub enum ReadError {
+    Load(rmp_serde::decode::Error),
+    // Preserve old data
+    VersionMismatch(Box<Data>),
+}
+
+impl fmt::Debug for ReadError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::Load(err) => err.fmt(f),
+            Self::VersionMismatch(_) => write!(f, "VersionMismatch"),
+        }
+    }
+}
+
 pub type WriteError = rmp_serde::encode::Error;
 
 impl Data {
@@ -59,8 +84,16 @@ impl Data {
         id
     }
 
-    pub fn from_reader<R: Read>(reader: R) -> Result<Self, ReadError> {
+    pub fn from_reader<R: Read>(reader: R) -> Result<Box<Self>, ReadError> {
         rmp_serde::decode::from_read(reader)
+            .map_err(ReadError::Load)
+            .and_then(|data: Data| {
+                if data.version == CURRENT_VERSION {
+                    Ok(Box::new(data))
+                } else {
+                    Err(ReadError::VersionMismatch(Box::new(data)))
+                }
+            })
     }
 
     pub fn write_to<W: Write>(&self, mut writer: W) -> Result<(), WriteError> {

rtsim/src/data/nature.rs

@@ -55,6 +55,8 @@ pub struct Chunk {
     /// generation. This value represents only the variable 'depletion' factor
     /// of that resource, which shall change over time as the world evolves
     /// and players interact with it.
+    // TODO: Consider whether we can use `i16` or similar here instead: `f32` has more resolution
+    // than we might need.
     #[serde(rename = "r")]
     #[serde(serialize_with = "crate::data::rugged_ser_enum_map::<_, _, _, 1>")]
     #[serde(deserialize_with = "crate::data::rugged_de_enum_map::<_, _, _, 1>")]

rtsim/src/data/npc.rs

@@ -179,21 +179,25 @@ impl Npc {
         }
     }
 
+    // TODO: have a dedicated `NpcBuilder` type for this.
     pub fn with_personality(mut self, personality: Personality) -> Self {
         self.personality = personality;
         self
     }
 
+    // TODO: have a dedicated `NpcBuilder` type for this.
     pub fn with_profession(mut self, profession: impl Into<Option<Profession>>) -> Self {
         self.profession = profession.into();
         self
     }
 
+    // TODO: have a dedicated `NpcBuilder` type for this.
     pub fn with_home(mut self, home: impl Into<Option<SiteId>>) -> Self {
         self.home = home.into();
         self
     }
 
+    // TODO: have a dedicated `NpcBuilder` type for this.
     pub fn steering(mut self, vehicle: impl Into<Option<VehicleId>>) -> Self {
         self.riding = vehicle.into().map(|vehicle| Riding {
             vehicle,
@@ -202,6 +206,7 @@ impl Npc {
         self
     }
 
+    // TODO: have a dedicated `NpcBuilder` type for this.
     pub fn riding(mut self, vehicle: impl Into<Option<VehicleId>>) -> Self {
         self.riding = vehicle.into().map(|vehicle| Riding {
             vehicle,
@@ -210,6 +215,7 @@ impl Npc {
         self
     }
 
+    // TODO: have a dedicated `NpcBuilder` type for this.
     pub fn with_faction(mut self, faction: impl Into<Option<FactionId>>) -> Self {
         self.faction = faction.into();
         self
@@ -217,10 +223,14 @@ impl Npc {
 
     pub fn rng(&self, perm: u32) -> impl Rng { RandomPerm::new(self.seed.wrapping_add(perm)) }
 
+    // TODO: Don't make this depend on deterministic RNG, actually persist names
+    // once we've decided that we want to
     pub fn get_name(&self) -> String { name::generate(&mut self.rng(Self::PERM_NAME)) }
 
     pub fn cleanup(&mut self, reports: &Reports) {
         // Clear old or superfluous sentiments
+        // TODO: It might be worth giving more important NPCs a higher sentiment
+        // 'budget' than less important ones.
         self.sentiments
             .cleanup(crate::data::sentiment::NPC_MAX_SENTIMENTS);
         // Clear reports that have been forgotten
@@ -305,6 +315,7 @@ pub struct Npcs {
     pub npcs: HopSlotMap<NpcId, Npc>,
     pub vehicles: HopSlotMap<VehicleId, Vehicle>,
     // TODO: This feels like it should be its own rtsim resource
+    // TODO: Consider switching to `common::util::SpatialGrid` instead
     #[serde(skip, default = "construct_npc_grid")]
     pub npc_grid: Grid<GridCell>,
     #[serde(skip)]
@@ -332,6 +343,8 @@ impl Npcs {
     }
 
     /// Queries nearby npcs, not garantueed to work if radius > 32.0
+    // TODO: Find a more efficient way to implement this, it's currently
+    // (theoretically) O(n^2).
     pub fn nearby(
         &self,
         this_npc: Option<NpcId>,

rtsim/src/data/sentiment.rs

@@ -5,6 +5,7 @@ use common::{
 use hashbrown::HashMap;
 use rand::prelude::*;
 use serde::{Deserialize, Serialize};
+use std::collections::BinaryHeap;
 
 // Factions have a larger 'social memory' than individual NPCs and so we allow
 // them to have more sentiments
@@ -22,7 +23,7 @@ const DECAY_TIME_FACTOR: f32 = 1.0; //6.0; TODO: Use this value when we're happy
 // - Occupations (hatred of hunters or chefs?)
 // - Ideologies (dislikes democracy, likes monarchy?)
 // - etc.
-#[derive(Copy, Clone, Hash, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Copy, Clone, Hash, PartialEq, Eq, Serialize, Deserialize, PartialOrd, Ord)]
 pub enum Target {
     Character(CharacterId),
     Npc(NpcId),
@@ -105,13 +106,12 @@ impl Sentiments {
                 // For each sentiment, calculate how valuable it is for us to remember.
                 // For now, we just use the absolute value of the sentiment but later on we might want to favour
                 // sentiments toward factions and other 'larger' groups over, say, sentiments toward players/other NPCs
-                .map(|(tgt, sentiment)| (*tgt, sentiment.positivity.unsigned_abs()))
+                .map(|(tgt, sentiment)| (sentiment.positivity.unsigned_abs(), *tgt))
-                .collect::<Vec<_>>();
+                .collect::<BinaryHeap<_>>();
-            sentiments.sort_unstable_by_key(|(_, value)| *value);
 
             // Remove the superfluous sentiments
-            for (tgt, _) in &sentiments[0..self.map.len() - max_sentiments] {
+            for (_, tgt) in sentiments.drain().take(self.map.len() - max_sentiments) {
-                self.map.remove(tgt);
+                self.map.remove(&tgt);
             }
         }
     }
@@ -156,7 +156,7 @@ impl Sentiment {
     /// generally try to harm the actor in any way they can.
     pub const VILLAIN: f32 = -0.8;
 
-    fn value(&self) -> f32 { self.positivity as f32 / 126.0 }
+    fn value(&self) -> f32 { self.positivity as f32 * (1.0 / 126.0) }
 
     fn change_by(&mut self, change: f32, cap: f32) {
         // There's a bit of ceremony here for two reasons:
@@ -175,8 +175,8 @@ impl Sentiment {
 
     fn decay(&mut self, rng: &mut impl Rng, dt: f32) {
         if self.positivity != 0 {
-            // TODO: Make dt-independent so we can slow tick rates
+            // TODO: Find a slightly nicer way to have sentiment decay, perhaps even by
-            // 36 = 6 * 6
+            // remembering the last interaction instead of constant updates.
             if rng.gen_bool(
                 (1.0 / (self.positivity.unsigned_abs() as f32 * DECAY_TIME_FACTOR.powi(2) * dt))
                     as f64,

rtsim/src/data/site.rs

@@ -37,6 +37,7 @@ pub struct Site {
     #[serde(skip_serializing, skip_deserializing)]
     pub world_site: Option<Id<WorldSite>>,
 
+    // Note: there's currently no guarantee that site populations are non-intersecting
     #[serde(skip_serializing, skip_deserializing)]
     pub population: HashSet<NpcId>,
 }

rtsim/src/gen/mod.rs

@@ -6,7 +6,7 @@ use crate::data::{
     faction::Faction,
     npc::{Npc, Npcs, Profession, Vehicle},
     site::Site,
-    Data, Nature,
+    Data, Nature, CURRENT_VERSION,
 };
 use common::{
     comp::{self, Body},
@@ -30,6 +30,7 @@ impl Data {
         let mut rng = SmallRng::from_seed(seed);
 
         let mut this = Self {
+            version: CURRENT_VERSION,
             nature: Nature::generate(world),
             npcs: Npcs {
                 npcs: Default::default(),

rtsim/src/gen/name.rs

@@ -9,7 +9,7 @@ pub fn generate(rng: &mut impl Rng) -> String {
 
     name += starts.choose(rng).unwrap();
 
-    for _ in 0..thread_rng().gen_range(1..=3) {
+    for _ in 0..rng.gen_range(1..=3) {
         name += vowels.choose(rng).unwrap();
         name += cons.choose(rng).unwrap();
     }

rtsim/src/lib.rs

@@ -91,6 +91,9 @@ impl RtState {
             .borrow_mut()
     }
 
+    // TODO: Consider whether it's worth explicitly calling rule event handlers
+    // instead of allowing them to bind event handlers. Less modular, but
+    // potentially easier to deal with data dependencies?
     pub fn bind<R: Rule, E: Event>(
         &mut self,
         f: impl FnMut(EventCtx<R, E>) + Send + Sync + 'static,
@@ -114,6 +117,8 @@ impl RtState {
 
     pub fn data_mut(&self) -> impl DerefMut<Target = Data> + '_ { self.resource_mut() }
 
+    pub fn get_data_mut(&mut self) -> &mut Data { self.get_resource_mut() }
+
     pub fn resource<R: Send + Sync + 'static>(&self) -> impl Deref<Target = R> + '_ {
         self.resources
             .get::<AtomicRefCell<R>>()
@@ -126,6 +131,18 @@ impl RtState {
             .borrow()
     }
 
+    pub fn get_resource_mut<R: Send + Sync + 'static>(&mut self) -> &mut R {
+        self.resources
+            .get_mut::<AtomicRefCell<R>>()
+            .unwrap_or_else(|| {
+                panic!(
+                    "Tried to access resource '{}' but it does not exist",
+                    type_name::<R>()
+                )
+            })
+            .get_mut()
+    }
+
     pub fn resource_mut<R: Send + Sync + 'static>(&self) -> impl DerefMut<Target = R> + '_ {
         self.resources
             .get::<AtomicRefCell<R>>()
@@ -139,6 +156,8 @@ impl RtState {
     }
 
     pub fn emit<E: Event>(&mut self, e: E, world: &World, index: IndexRef) {
+        // TODO: Queue these events up and handle them on a regular rtsim tick instead
+        // of executing their handlers immediately.
         if let Some(handlers) = self.event_handlers.get::<EventHandlersOf<E>>() {
             handlers.iter().for_each(|f| f(self, world, index, &e));
         }

rtsim/src/rule/cleanup.rs

@@ -20,13 +20,13 @@ impl Rule for CleanUp {
             let data = &mut *ctx.state.data_mut();
             let mut rng = ChaChaRng::from_seed(thread_rng().gen::<[u8; 32]>());
 
-            for (_, npc) in data.npcs
+            // TODO: Use `.into_par_iter()` for these by implementing rayon traits in upstream slotmap.
+
+            data.npcs
                 .iter_mut()
                 // Only cleanup NPCs every few ticks
                 .filter(|(_, npc)| (npc.seed as u64 + ctx.event.tick) % NPC_SENTIMENT_TICK_SKIP == 0)
-            {
+                .for_each(|(_, npc)| npc.sentiments.decay(&mut rng, ctx.event.dt * NPC_SENTIMENT_TICK_SKIP as f32));
-                npc.sentiments.decay(&mut rng, ctx.event.dt * NPC_SENTIMENT_TICK_SKIP as f32);
-            }
 
             // Clean up entities
             data.npcs

rtsim/src/rule/migrate.rs

@@ -2,6 +2,7 @@ use crate::{data::Site, event::OnSetup, RtState, Rule, RuleError};
 use rand::prelude::*;
 use rand_chacha::ChaChaRng;
 use tracing::warn;
+use world::site::SiteKind;
 
 /// This rule runs at rtsim startup and broadly acts to perform some primitive
 /// migration/sanitisation in order to ensure that the state of rtsim is mostly
@@ -36,11 +37,6 @@ impl Rule for Migrate {
                 }
             });
 
-            for npc in data.npcs.values_mut() {
-                // TODO: Consider what to do with homeless npcs.
-                npc.home = npc.home.filter(|home| data.sites.contains_key(*home));
-            }
-
             // Generate rtsim sites for world sites that don't have a corresponding rtsim
             // site yet
             for (world_site_id, _) in ctx.index.sites.iter() {
@@ -65,7 +61,25 @@ impl Rule for Migrate {
                 }
             }
 
-            // TODO: Reassign sites for NPCs if they don't have one
+            // Reassign NPCs to sites if their old one was deleted. If they were already homeless, no need to do anything.
+            for npc in data.npcs.values_mut() {
+                if let Some(home) = npc.home
+                    && !data.sites.contains_key(home)
+                {
+                    // Choose the closest habitable site as the new home for the NPC
+                    npc.home = data.sites.sites
+                        .iter()
+                        .filter(|(_, site)| {
+                            // TODO: This is a bit silly, but needs to wait on the removal of site1
+                            site.world_site.map_or(false, |ws| matches!(&ctx.index.sites.get(ws).kind, SiteKind::Refactor(_)
+                                | SiteKind::CliffTown(_)
+                                | SiteKind::SavannahPit(_)
+                                | SiteKind::DesertCity(_)))
+                        })
+                        .min_by_key(|(_, site)| site.wpos.as_().distance_squared(npc.wpos.xy()) as i32)
+                        .map(|(site_id, _)| site_id);
+                }
+            }
         });
 
         Ok(Self)

rtsim/src/rule/npc_ai.rs

@@ -469,24 +469,27 @@ fn timeout(time: f64) -> impl FnMut(&mut NpcCtx) -> bool + Clone + Send + Sync {
 fn socialize() -> impl Action {
     now(|ctx| {
         // TODO: Bit odd, should wait for a while after greeting
-        if ctx.rng.gen_bool(0.002) && let Some(other) = ctx
+        if ctx.rng.gen_bool(0.002) {
-            .state
+            if ctx.rng.gen_bool(0.15) {
-            .data()
+                return just(|ctx| ctx.controller.do_dance())
-            .npcs
+                    .repeat()
-            .nearby(Some(ctx.npc_id), ctx.npc.wpos, 8.0)
+                    .stop_if(timeout(6.0))
-            .choose(&mut ctx.rng)
+                    .debug(|| "dancing")
-        {
+                    .map(|_| ())
-            just(move |ctx| ctx.controller.say(other, "npc-speech-villager_open")).boxed()
+                    .boxed();
-        } else if ctx.rng.gen_bool(0.0003) {
+            } else if let Some(other) = ctx
-            just(|ctx| ctx.controller.do_dance())
+                .state
-                .repeat()
+                .data()
-                .stop_if(timeout(6.0))
+                .npcs
-                .debug(|| "dancing")
+                .nearby(Some(ctx.npc_id), ctx.npc.wpos, 8.0)
-                .map(|_| ())
+                .choose(&mut ctx.rng)
-                .boxed()
+            {
-        } else {
+                return just(move |ctx| ctx.controller.say(other, "npc-speech-villager_open"))
-            idle().boxed()
+                    .boxed();
+            }
         }
+
+        idle().boxed()
     })
 }
 
@@ -914,6 +917,12 @@ fn check_inbox(ctx: &mut NpcCtx) -> Option<impl Action> {
 }
 
 fn check_for_enemies(ctx: &mut NpcCtx) -> Option<impl Action> {
+    // TODO: Instead of checking all nearby actors every tick, it would be more
+    // effective to have the actor grid generate a per-tick diff so that we only
+    // need to check new actors in the local area. Be careful though:
+    // implementing this means accounting for changes in sentiment (that could
+    // suddenly make a nearby actor an enemy) as well as variable NPC tick
+    // rates!
     ctx.state
         .data()
         .npcs

rtsim/src/rule/replenish_resources.rs

@@ -9,7 +9,11 @@ pub struct ReplenishResources;
 // TODO: Non-renewable resources?
 pub const REPLENISH_TIME: f32 = 60.0 * 60.0;
 /// How many chunks should be replenished per tick?
-pub const REPLENISH_PER_TICK: usize = 100000;
+// TODO: It should be possible to optimise this be remembering the last
+// modification time for each chunk, then lazily projecting forward using a
+// closed-form solution to the replenishment to calculate resources in a lazy
+// manner.
+pub const REPLENISH_PER_TICK: usize = 8192;
 
 impl Rule for ReplenishResources {
     fn start(rtstate: &mut RtState) -> Result<Self, RuleError> {
@@ -19,9 +23,9 @@ impl Rule for ReplenishResources {
 
             // How much should be replenished for each chosen chunk to hit our target
             // replenishment rate?
-            let replenish_amount = world_size.product() as f32 * ctx.event.dt
+            let replenish_amount = world_size.product() as f32
-                / REPLENISH_TIME
+                * ctx.event.dt
-                / REPLENISH_PER_TICK as f32;
+                * (1.0 / REPLENISH_TIME / REPLENISH_PER_TICK as f32);
             for _ in 0..REPLENISH_PER_TICK {
                 let key = world_size.map(|e| thread_rng().gen_range(0..e as i32));
 

rtsim/src/rule/report.rs

@@ -33,6 +33,9 @@ fn on_death(ctx: EventCtx<ReportEvents, OnDeath>) {
                 at: data.time_of_day,
             });
 
+            // TODO: Don't push report to NPC inboxes, have a dedicated data structure that
+            // tracks reports by chunks and then have NPCs decide to query this
+            // data structure in their own time.
             for npc_id in nearby {
                 if let Some(npc) = data.npcs.get_mut(npc_id) {
                     npc.inbox.push_back(report);

rtsim/src/rule/simulate_npcs.rs

@@ -52,7 +52,7 @@ fn on_death(ctx: EventCtx<SimulateNpcs, OnDeath>) {
         let mut rng = ChaChaRng::from_seed(thread_rng().gen::<[u8; 32]>());
 
         // Respawn dead NPCs
-        match npc.body {
+        let details = match npc.body {
             Body::Humanoid(_) => {
                 if let Some((site_id, site)) = data
                     .sites
@@ -76,15 +76,17 @@ fn on_death(ctx: EventCtx<SimulateNpcs, OnDeath>) {
                         let species = comp::humanoid::ALL_SPECIES.choose(&mut *rng).unwrap();
                         Body::Humanoid(comp::humanoid::Body::random_with(rng, species))
                     };
-                    data.spawn_npc(
+                    let npc_id = data.spawn_npc(
                         Npc::new(rng.gen(), rand_wpos(&mut rng), random_humanoid(&mut rng))
                             .with_personality(Personality::random(&mut rng))
                             .with_home(site_id)
                             .with_faction(npc.faction)
                             .with_profession(npc.profession.clone()),
                     );
+                    Some((npc_id, site_id))
                 } else {
                     warn!("No site found for respawning humaniod");
+                    None
                 }
             },
             Body::BirdLarge(_) => {
@@ -112,7 +114,7 @@ fn on_death(ctx: EventCtx<SimulateNpcs, OnDeath>) {
                     ]
                     .choose(&mut rng)
                     .unwrap();
-                    data.npcs.create_npc(
+                    let npc_id = data.npcs.create_npc(
                         Npc::new(
                             rng.gen(),
                             rand_wpos(&mut rng),
@@ -122,11 +124,23 @@ fn on_death(ctx: EventCtx<SimulateNpcs, OnDeath>) {
                         )
                         .with_home(site_id),
                     );
+                    Some((npc_id, site_id))
                 } else {
                     warn!("No site found for respawning bird");
+                    None
                 }
             },
-            _ => unimplemented!(),
+            body => {
+                error!("Tried to respawn rtsim NPC with invalid body: {:?}", body);
+                None
+            },
+        };
+
+        // Add the NPC to their home site
+        if let Some((npc_id, home_site)) = details {
+            if let Some(home) = data.sites.get_mut(home_site) {
+                home.population.insert(npc_id);
+            }
         }
     }
 }

rtsim/src/rule/sync_npcs.rs

@@ -22,7 +22,7 @@ fn on_setup(ctx: EventCtx<SyncNpcs, OnSetup>) {
     // Create NPC grid
     data.npcs.npc_grid = Grid::new(ctx.world.sim().get_size().as_(), Default::default());
 
-    // Add NPCs to home population (TODO: Do this on entity creation?)
+    // Add NPCs to home population
     for (npc_id, npc) in data.npcs.npcs.iter() {
         if let Some(home) = npc.home.and_then(|home| data.sites.get_mut(home)) {
             home.population.insert(npc_id);

server/src/cmd.rs

@@ -1329,6 +1329,8 @@ fn handle_rtsim_npc(
     }
 }
 
+// TODO: Remove this command when rtsim becomes more mature and we're sure we
+// don't need purges to fix broken state.
 fn handle_rtsim_purge(
     server: &mut Server,
     client: EcsEntity,
@@ -1337,6 +1339,14 @@ fn handle_rtsim_purge(
     action: &ServerChatCommand,
 ) -> CmdResult<()> {
     use crate::rtsim::RtSim;
+    let client_uuid = uuid(server, client, "client")?;
+    if !matches!(real_role(server, client_uuid, "client")?, AdminRole::Admin) {
+        return Err(
+            "You must be a real admin (not just a temporary admin) to purge rtsim data."
+                .to_string(),
+        );
+    }
+
     if let Some(should_purge) = parse_cmd_args!(args, bool) {
         server
             .state
@@ -2082,6 +2092,7 @@ fn handle_kill_npcs(
         let healths = ecs.write_storage::<comp::Health>();
         let players = ecs.read_storage::<comp::Player>();
         let alignments = ecs.read_storage::<Alignment>();
+        let rtsim_entities = ecs.read_storage::<common::rtsim::RtSimEntity>();
 
         (
             &entities,
@@ -2101,11 +2112,7 @@ fn handle_kill_npcs(
                     };
 
                 if should_kill {
-                    if let Some(rtsim_entity) = ecs
+                    if let Some(rtsim_entity) = rtsim_entities.get(entity).copied() {
-                        .read_storage::<common::rtsim::RtSimEntity>()
-                        .get(entity)
-                        .copied()
-                    {
                         ecs.write_resource::<crate::rtsim::RtSim>()
                             .hook_rtsim_actor_death(
                                 &ecs.read_resource::<Arc<world::World>>(),

server/src/events/entity_creation.rs

@@ -195,7 +195,7 @@ pub fn handle_create_ship(
     ship: comp::ship::Body,
     rtsim_vehicle: Option<RtSimVehicle>,
     driver: Option<NpcBuilder>,
-    passangers: Vec<NpcBuilder>,
+    passengers: Vec<NpcBuilder>,
 ) {
     let mut entity = server
         .state
@@ -234,8 +234,8 @@ pub fn handle_create_ship(
         }
     }
 
-    for passanger in passangers {
+    for passenger in passengers {
-        handle_create_npc(server, Pos(pos.0 + Vec3::unit_z() * 5.0), passanger);
+        handle_create_npc(server, Pos(pos.0 + Vec3::unit_z() * 5.0), passenger);
     }
 }
 

server/src/lib.rs

@@ -80,7 +80,7 @@ use common::{
     rtsim::{RtSimEntity, RtSimVehicle},
     shared_server_config::ServerConstants,
     slowjob::SlowJobPool,
-    terrain::{Block, TerrainChunk, TerrainChunkSize},
+    terrain::{TerrainChunk, TerrainChunkSize},
     vol::RectRasterableVol,
 };
 use common_ecs::run_now;
@@ -88,7 +88,7 @@ use common_net::{
     msg::{ClientType, DisconnectReason, ServerGeneral, ServerInfo, ServerMsg},
     sync::WorldSyncExt,
 };
-use common_state::{BuildAreas, State};
+use common_state::{BlockDiff, BuildAreas, State};
 use common_systems::add_local_systems;
 use metrics::{EcsSystemMetrics, PhysicsMetrics, TickMetrics};
 use network::{ListenAddr, Network, Pid};
@@ -698,21 +698,15 @@ impl Server {
 
         let before_state_tick = Instant::now();
 
-        fn on_block_update(
+        fn on_block_update(ecs: &specs::World, changes: Vec<BlockDiff>) {
-            ecs: &specs::World,
-            wpos: Vec3<i32>,
-            old_block: Block,
-            new_block: Block,
-        ) {
             // When a resource block updates, inform rtsim
-            if old_block.get_rtsim_resource().is_some() || new_block.get_rtsim_resource().is_some()
+            if changes.iter().any(|c| {
-            {
+                c.old.get_rtsim_resource().is_some() || c.new.get_rtsim_resource().is_some()
+            }) {
                 ecs.write_resource::<rtsim::RtSim>().hook_block_update(
                     &ecs.read_resource::<Arc<world::World>>(),
                     ecs.read_resource::<world::IndexOwned>().as_index_ref(),
-                    wpos,
+                    changes,
-                    old_block,
-                    new_block,
                 );
             }
         }
@@ -838,35 +832,26 @@ impl Server {
                 .collect::<Vec<_>>()
         };
 
-        for entity in to_delete {
+        {
-            // Assimilate entities that are part of the real-time world simulation
+            let mut rtsim = self.state.ecs().write_resource::<rtsim::RtSim>();
-            #[cfg(feature = "worldgen")]
+            let rtsim_entities = self.state.ecs().read_storage::<RtSimEntity>();
-            if let Some(rtsim_entity) = self
+            let rtsim_vehicles = self.state.ecs().read_storage::<RtSimVehicle>();
-                .state
-                .ecs()
-                .read_storage::<RtSimEntity>()
-                .get(entity)
-                .copied()
-            {
-                self.state
-                    .ecs()
-                    .write_resource::<rtsim::RtSim>()
-                    .hook_rtsim_entity_unload(rtsim_entity);
-            }
-            #[cfg(feature = "worldgen")]
-            if let Some(rtsim_vehicle) = self
-                .state
-                .ecs()
-                .read_storage::<RtSimVehicle>()
-                .get(entity)
-                .copied()
-            {
-                self.state
-                    .ecs()
-                    .write_resource::<rtsim::RtSim>()
-                    .hook_rtsim_vehicle_unload(rtsim_vehicle);
-            }
 
+            // Assimilate entities that are part of the real-time world simulation
+            for entity in &to_delete {
+                #[cfg(feature = "worldgen")]
+                if let Some(rtsim_entity) = rtsim_entities.get(*entity) {
+                    rtsim.hook_rtsim_entity_unload(*rtsim_entity);
+                }
+                #[cfg(feature = "worldgen")]
+                if let Some(rtsim_vehicle) = rtsim_vehicles.get(*entity) {
+                    rtsim.hook_rtsim_vehicle_unload(*rtsim_vehicle);
+                }
+            }
+        }
+
+        // Actually perform entity deletion
+        for entity in to_delete {
             if let Err(e) = self.state.delete_entity_recorded(entity) {
                 error!(?e, "Failed to delete agent outside the terrain");
             }

server/src/rtsim/event.rs

@@ -1,12 +1,9 @@
-use common::terrain::Block;
+use common_state::BlockDiff;
 use rtsim::Event;
-use vek::*;
 
 #[derive(Clone)]
 pub struct OnBlockChange {
-    pub wpos: Vec3<i32>,
+    pub changes: Vec<BlockDiff>,
-    pub old: Block,
-    pub new: Block,
 }
 
 impl Event for OnBlockChange {}

server/src/rtsim/mod.rs

@@ -6,13 +6,13 @@ use atomicwrites::{AtomicFile, OverwriteBehavior};
 use common::{
     grid::Grid,
     rtsim::{Actor, ChunkResource, RtSimEntity, RtSimVehicle, WorldSettings},
-    terrain::Block,
 };
 use common_ecs::dispatch;
+use common_state::BlockDiff;
 use crossbeam_channel::{unbounded, Receiver, Sender};
 use enum_map::EnumMap;
 use rtsim::{
-    data::{npc::SimulationMode, Data},
+    data::{npc::SimulationMode, Data, ReadError},
     event::{OnDeath, OnSetup},
     RtState,
 };
@@ -51,8 +51,17 @@ impl RtSim {
                 match File::open(&file_path) {
                     Ok(file) => {
                         info!("Rtsim data found. Attempting to load...");
+
+                        let ignore_version = std::env::var("RTSIM_IGNORE_VERSION").is_ok();
+
                         match Data::from_reader(io::BufReader::new(file)) {
-                            Ok(data) => {
+                            Err(ReadError::VersionMismatch(_)) if !ignore_version => {
+                                warn!(
+                                    "Rtsim data version mismatch (implying a breaking change), \
+                                     rtsim data will be purged"
+                                );
+                            },
+                            Ok(data) | Err(ReadError::VersionMismatch(data)) => {
                                 info!("Rtsim data loaded.");
                                 if data.should_purge {
                                     warn!(
@@ -60,11 +69,11 @@ impl RtSim {
                                          generating afresh"
                                     );
                                 } else {
-                                    break 'load data;
+                                    break 'load *data;
                                 }
                             },
-                            Err(e) => {
+                            Err(ReadError::Load(err)) => {
-                                error!("Rtsim data failed to load: {}", e);
+                                error!("Rtsim data failed to load: {}", err);
                                 info!("Old rtsim data will now be moved to a backup file");
                                 let mut i = 0;
                                 loop {
@@ -139,37 +148,30 @@ impl RtSim {
     }
 
     pub fn hook_load_chunk(&mut self, key: Vec2<i32>, max_res: EnumMap<ChunkResource, usize>) {
-        if let Some(chunk_state) = self.state.resource_mut::<ChunkStates>().0.get_mut(key) {
+        if let Some(chunk_state) = self.state.get_resource_mut::<ChunkStates>().0.get_mut(key) {
             *chunk_state = Some(LoadedChunkState { max_res });
         }
     }
 
     pub fn hook_unload_chunk(&mut self, key: Vec2<i32>) {
-        if let Some(chunk_state) = self.state.resource_mut::<ChunkStates>().0.get_mut(key) {
+        if let Some(chunk_state) = self.state.get_resource_mut::<ChunkStates>().0.get_mut(key) {
             *chunk_state = None;
         }
     }
 
-    pub fn hook_block_update(
+    pub fn hook_block_update(&mut self, world: &World, index: IndexRef, changes: Vec<BlockDiff>) {
-        &mut self,
-        world: &World,
-        index: IndexRef,
-        wpos: Vec3<i32>,
-        old: Block,
-        new: Block,
-    ) {
         self.state
-            .emit(event::OnBlockChange { wpos, old, new }, world, index);
+            .emit(event::OnBlockChange { changes }, world, index);
     }
 
     pub fn hook_rtsim_entity_unload(&mut self, entity: RtSimEntity) {
-        if let Some(npc) = self.state.data_mut().npcs.get_mut(entity.0) {
+        if let Some(npc) = self.state.get_data_mut().npcs.get_mut(entity.0) {
             npc.mode = SimulationMode::Simulated;
         }
     }
 
     pub fn hook_rtsim_vehicle_unload(&mut self, entity: RtSimVehicle) {
-        if let Some(vehicle) = self.state.data_mut().npcs.vehicles.get_mut(entity.0) {
+        if let Some(vehicle) = self.state.get_data_mut().npcs.vehicles.get_mut(entity.0) {
             vehicle.mode = SimulationMode::Simulated;
         }
     }

server/src/rtsim/rule/deplete_resources.rs

@@ -7,32 +7,35 @@ pub struct DepleteResources;
 impl Rule for DepleteResources {
     fn start(rtstate: &mut RtState) -> Result<Self, RuleError> {
         rtstate.bind::<Self, OnBlockChange>(|ctx| {
-            let key = ctx.event.wpos.xy().wpos_to_cpos();
+            let chunk_states = ctx.state.resource::<ChunkStates>();
-            if let Some(Some(chunk_state)) = ctx.state.resource_mut::<ChunkStates>().0.get(key) {
+            let mut data = ctx.state.data_mut();
-                let mut chunk_res = ctx.state.data().nature.get_chunk_resources(key);
+            for change in &ctx.event.changes {
-                // Remove resources
+                let key = change.wpos.xy().wpos_to_cpos();
-                if let Some(res) = ctx.event.old.get_rtsim_resource() {
+                if let Some(Some(chunk_state)) = chunk_states.0.get(key) {
-                    if chunk_state.max_res[res] > 0 {
+                    let mut chunk_res = data.nature.get_chunk_resources(key);
-                        chunk_res[res] = (chunk_res[res] * chunk_state.max_res[res] as f32 - 1.0)
+                    // Remove resources
-                            .round()
+                    if let Some(res) = change.old.get_rtsim_resource() {
-                            .max(0.0)
+                        if chunk_state.max_res[res] > 0 {
-                            / chunk_state.max_res[res] as f32;
+                            chunk_res[res] = (chunk_res[res] * chunk_state.max_res[res] as f32
+                                - 1.0)
+                                .round()
+                                .max(0.0)
+                                / chunk_state.max_res[res] as f32;
+                        }
                     }
-                }
+                    // Replenish resources
-                // Replenish resources
+                    if let Some(res) = change.new.get_rtsim_resource() {
-                if let Some(res) = ctx.event.new.get_rtsim_resource() {
+                        if chunk_state.max_res[res] > 0 {
-                    if chunk_state.max_res[res] > 0 {
+                            chunk_res[res] = (chunk_res[res] * chunk_state.max_res[res] as f32
-                        chunk_res[res] = (chunk_res[res] * chunk_state.max_res[res] as f32 + 1.0)
+                                + 1.0)
-                            .round()
+                                .round()
-                            .max(0.0)
+                                .max(0.0)
-                            / chunk_state.max_res[res] as f32;
+                                / chunk_state.max_res[res] as f32;
+                        }
                     }
-                }
 
-                ctx.state
+                    data.nature.set_chunk_resources(key, chunk_res);
-                    .data_mut()
+                }
-                    .nature
-                    .set_chunk_resources(key, chunk_res);
             }
         });