fix bug when removing many machines (splitting networks)

docs/roadmap.md

@@ -48,6 +48,6 @@ Or `git tag -n  | grep -E "^[^.]+\.[^.]+\.0"` for the big features only.
       when it's not. I might be very wrong on this.
 - [ ] after clicking some transformation, the map gets a cell selected, while
       in the macroquad UI the cell that got the transformation keeps the selection.
-- [ ] removing several cells that may split the network creates a broken state
+- [x] removing several cells that may split the network creates a broken state
 - [ ] flooded machines still work
 - [ ] storage machines work without power

logic/src/screen/gui/panels/task_queue.rs

@@ -113,6 +113,7 @@ fn format_reasons(reasons: &Option<HashSet<TransformationResult>>) -> Vec<String
                 TransformationResult::CanNotDeconstructShip => {
                     "  You're not allowed to remove the spaceship"
                 }
+                TransformationResult::SplitNetwork => "  The machine network should not be split",
             }
             .to_string();
             reasons_lines.push(reason_line);

logic/src/world/map/transform_cells.rs

@@ -20,6 +20,7 @@ pub enum TransformationResult {
     NoSturdyBase,
     OutOfShipReach,
     CanNotDeconstructShip,
+    SplitNetwork,
 }
 
 pub fn allowed_transformations(cells: &HashSet<CellIndex>, map: &Map) -> Vec<Transformation> {
@@ -224,12 +225,11 @@ impl From<Addition> for TransformationResult {
 impl From<Replacement> for TransformationResult {
     fn from(replacement: Replacement) -> Self {
         match replacement {
-            Replacement::Ok | Replacement::SplitNetwork | Replacement::None => {
-                TransformationResult::Ok
-            }
+            Replacement::Ok | Replacement::None => TransformationResult::Ok,
             Replacement::NotEnoughMaterial => TransformationResult::NotEnoughMaterial,
             Replacement::NotEnoughStorage => TransformationResult::NotEnoughStorage,
             Replacement::Forbidden => TransformationResult::CanNotDeconstructShip,
+            Replacement::SplitNetwork => TransformationResult::SplitNetwork,
         }
     }
 }

logic/src/world/networks.rs

@@ -71,7 +71,7 @@ impl Networks {
         old_tile: TileType,
         storage: &mut Grams,
     ) -> TransformationResult {
-        let replacement = self.replace_if_present_with_storage(cell_index, new_machine, storage);
+        let replacement = self.replace_if_present_with_storage(cell_index, new_machine);
         match replacement {
             Replacement::SplitNetwork
             | Replacement::Ok
@@ -120,25 +120,19 @@ impl Networks {
 
     #[cfg(test)]
     fn replace_if_present(&mut self, cell_index: CellIndex, new_machine: TileType) -> Replacement {
-        self.replace_if_present_with_storage(cell_index, new_machine, &mut 0.0)
+        self.replace_if_present_with_storage(cell_index, new_machine)
     }
     fn replace_if_present_with_storage(
         &mut self,
         cell_index: CellIndex,
         new_machine: TileType,
-        storage: &mut Grams,
     ) -> Replacement {
         let replacement = self
             .ship_network
             .replace_if_present(cell_index, new_machine);
         match replacement {
-            Replacement::SplitNetwork => {
-                let network_to_split = std::mem::take(&mut self.ship_network);
-                self.re_add_network(network_to_split);
-                *storage = self.ship_network.try_add_resources(*storage);
-                return replacement;
-            }
-            Replacement::Ok
+            Replacement::SplitNetwork
+            | Replacement::Ok
             | Replacement::Forbidden
             | Replacement::NotEnoughStorage
             | Replacement::NotEnoughMaterial => {
@@ -149,18 +143,15 @@ impl Networks {
         for (i, network) in &mut self.unconnected_networks.iter_mut().enumerate() {
             let replacement = network.replace_if_present(cell_index, new_machine);
             match replacement {
-                Replacement::SplitNetwork => {
-                    self.split_network(i);
-                    return replacement;
-                }
                 Replacement::Ok => {
                     if self.unconnected_networks.get(i).unwrap().len() == 0 {
                         self.unconnected_networks.remove(i);
                         // TODO: can this happen?
                     }
                     return replacement;
                 }
-                Replacement::Forbidden
+                Replacement::SplitNetwork
+                | Replacement::Forbidden
                 | Replacement::NotEnoughStorage
                 | Replacement::NotEnoughMaterial => {
                     return replacement;
@@ -305,11 +296,6 @@ impl Networks {
         self.air_cleaned = air_cleaned;
     }
 
-    fn split_network(&mut self, network_index: usize) {
-        let network_to_split = self.unconnected_networks.swap_remove(network_index);
-        self.re_add_network(network_to_split)
-    }
-
     pub fn clear(&mut self) {
         *self = Self::new(self.ship_position)
     }
@@ -338,6 +324,7 @@ mod tests {
     use super::*;
     use crate::world::map::TileType::{MachineStorage, WallRock, Wire};
     use crate::world::map::{CellIndex, TileType};
+    use crate::world::networks::network::WALL_WEIGHT;
     use TileType::{Air, MachineAirCleaner, MachineAssembler};
 
     #[test]
@@ -366,20 +353,34 @@ mod tests {
     #[test]
     fn test_split_and_join_networks_keeps_storage() {
         let mut networks = Networks::new_default();
+
+        let resources_before_constructing = networks.get_stored_resources();
         assert_eq!(
             networks.add(CellIndex::new(0, 0, 1), MachineStorage, WallRock),
             true
         );
         assert_eq!(networks.add(CellIndex::new(0, 0, 2), Wire, Air), true);
         assert_eq!(networks.add(CellIndex::new(0, 0, 3), Wire, Air), true);
-        let resources_before = networks.get_stored_resources();
+
+        let resources_after_constructing = networks.get_stored_resources();
+        assert_eq!(networks.add(CellIndex::new(0, 0, 2), Air, Air), false);
+        assert_eq!(
+            networks.get_stored_resources(),
+            resources_after_constructing
+        );
+
+        assert_eq!(networks.add(CellIndex::new(0, 0, 3), Air, Air), true);
         assert_eq!(networks.add(CellIndex::new(0, 0, 2), Air, Air), true);
+        assert_eq!(networks.add(CellIndex::new(0, 0, 1), WallRock, Air), true);
         assert_eq!(
             networks.get_stored_resources(),
-            resources_before + MATERIAL_NEEDED_FOR_A_MACHINE
+            resources_before_constructing
         );
-        assert_eq!(networks.add(CellIndex::new(0, 0, 2), Wire, Air), true);
-        assert_eq!(networks.get_stored_resources(), resources_before);
+
+        assert_eq!(
+            resources_after_constructing - resources_before_constructing,
+            WALL_WEIGHT - 3.0 * MATERIAL_NEEDED_FOR_A_MACHINE
+        )
     }
 
     #[test]
@@ -424,9 +425,7 @@ mod tests {
             networks.replace_if_present(CellIndex::new(0, 0, 1), Air),
             Replacement::SplitNetwork
         );
-        assert_eq!(networks.len(), 2);
-        assert_eq!(networks.unconnected_networks.get(0).unwrap().nodes.len(), 1);
-        networks.add(CellIndex::new(0, 0, 1), MachineAssembler, Air);
+        assert_eq!(networks.len(), 1);
         networks.replace_if_present(CellIndex::new(0, 0, 2), Air);
         networks.replace_if_present(CellIndex::new(0, 0, 1), Air);
         assert_eq!(networks.len(), 1);
@@ -452,9 +451,9 @@ mod tests {
         assert_eq!(networks.get_non_ship_machine_count(), 3);
         assert_eq!(
             networks.add(CellIndex::new(0, 0, 11), TileType::Air, Air),
-            true
+            false
         );
-        assert_eq!(networks.len(), 3);
+        assert_eq!(networks.len(), 2);
     }
 
     #[test]
@@ -508,7 +507,9 @@ mod tests {
 mod storage_tests {
     use super::*;
     use crate::world::map::TileType::WallRock;
-    use crate::world::networks::network::{SPACESHIP_INITIAL_STORAGE, WALL_WEIGHT};
+    use crate::world::networks::network::{
+        MAX_STORAGE_PER_SPACESHIP, SPACESHIP_INITIAL_STORAGE, WALL_WEIGHT,
+    };
     use TileType::{Air, MachineStorage, Wire};
 
     #[test]
@@ -518,7 +519,7 @@ mod storage_tests {
             networks.get_stored_resources(),
             SPACESHIP_INITIAL_STORAGE - MATERIAL_NEEDED_FOR_A_MACHINE
         );
-        assert_eq!(networks.get_storage_capacity(), SPACESHIP_INITIAL_STORAGE);
+        assert_eq!(networks.get_storage_capacity(), MAX_STORAGE_PER_SPACESHIP);
     }
     #[test]
     fn test_can_not_add_machine_without_resources() {

logic/src/world/networks/network.rs

@@ -219,20 +219,19 @@ impl Network {
             } else if future_storage < 0.0 {
                 return Replacement::NotEnoughMaterial;
             }
-            self.stored_resources += old_material_regained;
-            self.stored_resources -= new_material_spent;
             let replacement_is_really_a_removal = !is_networkable(new_machine);
             if replacement_is_really_a_removal {
-                self.nodes.remove(i);
-                if self.is_connected() {
-                    Replacement::Ok
-                } else {
-                    Replacement::SplitNetwork
+                let removed = self.nodes.swap_remove(i);
+                if !self.is_connected() {
+                    self.nodes.push(removed);
+                    return Replacement::SplitNetwork;
                 }
             } else {
                 self.nodes.get_mut(i).unwrap().tile = new_machine;
-                Replacement::Ok
             }
+            self.stored_resources += old_material_regained;
+            self.stored_resources -= new_material_spent;
+            Replacement::Ok
         } else {
             Replacement::None
         };