From cd82509515e83b7ca9c525b3c7fa247744d1c71f Mon Sep 17 00:00:00 2001 From: Patrick Owen <patowen95@gmail.com> Date: Sun, 11 Aug 2024 16:55:27 -0400 Subject: [PATCH] Ensure the server state is consistent whenever snapshot is called Before this commit, if snapshot was called before the server ran its first step, there would be some unpopulated nodes in the graph, resulting in such nodes being transmitted to the client twice, once during the call to snapshot, and once as an accumulated change when calling step --- server/src/sim.rs | 48 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/server/src/sim.rs b/server/src/sim.rs index b2ef6e12..08ece64e 100644 --- a/server/src/sim.rs +++ b/server/src/sim.rs @@ -73,6 +73,11 @@ impl Sim { result .load_all_entities(save) .expect("save file must be of a valid format"); + // Loading entities can cause graph nodes to also be created, so we should populate them before returning. + result.populate_fresh_graph_nodes(); + // As no players have logged in yet, and `snapshot` may be called before the first call of `step`, + // make sure that `accumulated_changes` is empty to avoid accidental double-spawns of anything. + result.accumulated_changes = AccumulatedChanges::default(); result } @@ -114,6 +119,10 @@ impl Sim { Ok(()) } + /// Loads all entities from the given save file. Note that this must be called before any players + /// log in, as `accumulated_changes` will not properly reflect the entities that were loaded in. + /// It is also important to call `populate_fresh_graph_nodes` after calling this function to keep + /// `Sim` in a consistent state, as this function can expand the graph without populating the graph nodes. fn load_all_entities(&mut self, save: &save::Save) -> anyhow::Result<()> { let mut read = save.read()?; for node_hash in read.get_all_entity_node_ids()? { @@ -445,21 +454,7 @@ impl Sim { ensure_nearby(&mut self.graph, position, self.cfg.view_distance); } - self.accumulated_changes.fresh_nodes = self.graph.fresh().to_vec(); - populate_fresh_nodes(&mut self.graph); - - for fresh_node in self.accumulated_changes.fresh_nodes.iter().copied() { - for vertex in Vertex::iter() { - let chunk = ChunkId::new(fresh_node, vertex); - if let Some(voxel_data) = self.preloaded_voxel_data.remove(&chunk) { - self.accumulated_changes - .fresh_voxel_data - .push((chunk, voxel_data.serialize(self.cfg.chunk_size))); - self.modified_chunks.insert(chunk); - self.graph.populate_chunk(chunk, voxel_data) - } - } - } + self.populate_fresh_graph_nodes(); // We want to load all chunks that a player can interact with in a single step, so chunk_generation_distance // is set up to cover that distance. @@ -554,6 +549,29 @@ impl Sim { (spawns, delta) } + /// Should be called after any set of changes is made to the graph to ensure that the server + /// does not have any partially-initialized graph nodes. + fn populate_fresh_graph_nodes(&mut self) { + let fresh_nodes = self.graph.fresh().to_vec(); + populate_fresh_nodes(&mut self.graph); + + self.accumulated_changes + .fresh_nodes + .extend_from_slice(&fresh_nodes); + for fresh_node in fresh_nodes.iter().copied() { + for vertex in Vertex::iter() { + let chunk = ChunkId::new(fresh_node, vertex); + if let Some(voxel_data) = self.preloaded_voxel_data.remove(&chunk) { + self.accumulated_changes + .fresh_voxel_data + .push((chunk, voxel_data.serialize(self.cfg.chunk_size))); + self.modified_chunks.insert(chunk); + self.graph.populate_chunk(chunk, voxel_data) + } + } + } + } + fn new_id(&mut self) -> EntityId { loop { let id = self.rng.gen();