Sandbox: Refactor to use ObjectStore for components/wires

And don't directly map raw IDs to sandbox, reallocate and remap.

src/lib.rs

@@ -2,6 +2,7 @@ mod convert;
 pub mod custom_data;
 pub mod error;
 pub(crate) mod io;
+pub(crate) mod misc;
 pub mod sandbox;
 pub mod v5;
 pub mod v6;

src/misc/mod.rs

@@ -0,0 +1,3 @@
+//! Miscellaneous internal utilities.
+
+pub mod object_store;

src/misc/object_store.rs

@@ -0,0 +1,161 @@
+use std::{
+    fmt::Debug,
+    hash::{Hash, Hasher},
+    marker::PhantomData,
+    mem::replace,
+};
+
+/// An object store that stores a free-list inline with its elements.
+///
+/// The original idea for this allocator is to have a stack of indexes that have
+/// been vacated when items are removed from those locations. When adding an
+/// item, the stack is checked first for a free index; if the stack is empty,
+/// then the list has no vacant entries and the item is simply appended to the
+/// end.
+///
+/// Instead of storing this stack separately; the cost of the free-list stack is
+/// amortized by storing it inline with the items themselves. This may require
+/// up to 8 bytes in addition to the size of each item.
+pub struct ObjectStore<T> {
+    first_vacant: usize,
+    entries: Vec<Entry<T>>,
+}
+
+impl<T> ObjectStore<T> {
+    pub fn new() -> Self {
+        Self {
+            first_vacant: usize::MAX,
+            entries: Vec::new(),
+        }
+    }
+
+    pub fn contains(&self, address: Address<T>) -> bool {
+        self.get(address).is_some()
+    }
+
+    pub fn insert(&mut self, item: T) -> Address<T> {
+        if let Some(entry) = self.entries.get_mut(self.first_vacant) {
+            let address = Address::from_raw(self.first_vacant);
+            let replaced = replace(entry, Entry::Occupied(item));
+            match replaced {
+                Entry::Vacant { next_vacant } => {
+                    self.first_vacant = next_vacant;
+                }
+                _ => {
+                    unreachable!("occupied entry in free list");
+                }
+            }
+            address
+        } else {
+            let address = Address::from_raw(self.entries.len());
+            self.entries.push(Entry::Occupied(item));
+            address
+        }
+    }
+
+    pub fn get(&self, address: Address<T>) -> Option<&T> {
+        self.entries.get(address.into_raw()).and_then(Entry::get)
+    }
+
+    pub fn get_mut(&mut self, address: Address<T>) -> Option<&mut T> {
+        self.entries
+            .get_mut(address.into_raw())
+            .and_then(Entry::get_mut)
+    }
+
+    pub fn remove(&mut self, address: Address<T>) -> Option<T> {
+        let index = address.into_raw();
+
+        match self.entries.get(index)? {
+            Entry::Occupied(_) => {
+                let replaced = replace(
+                    &mut self.entries[index],
+                    Entry::Vacant {
+                        next_vacant: self.first_vacant,
+                    },
+                );
+                self.first_vacant = index;
+                match replaced {
+                    Entry::Occupied(x) => Some(x),
+                    _ => unreachable!("occupied is not occupied?"),
+                }
+            }
+            Entry::Vacant { .. } => None,
+        }
+    }
+
+    pub fn iter(&self) -> impl Iterator<Item = (Address<T>, &T)> {
+        self.entries
+            .iter()
+            .enumerate()
+            .flat_map(|(index, entry)| entry.get().map(|item| (Address::from_raw(index), item)))
+    }
+
+    pub fn iter_mut(&mut self) -> impl Iterator<Item = (Address<T>, &mut T)> {
+        self.entries
+            .iter_mut()
+            .enumerate()
+            .flat_map(|(index, entry)| entry.get_mut().map(|item| (Address::from_raw(index), item)))
+    }
+}
+
+enum Entry<T> {
+    Vacant { next_vacant: usize },
+    Occupied(T),
+}
+
+impl<T> Entry<T> {
+    fn get(&self) -> Option<&T> {
+        match self {
+            Self::Occupied(x) => Some(x),
+            _ => None,
+        }
+    }
+
+    fn get_mut(&mut self) -> Option<&mut T> {
+        match self {
+            Self::Occupied(x) => Some(x),
+            _ => None,
+        }
+    }
+}
+
+pub struct Address<T>(usize, PhantomData<T>);
+
+impl<T> Address<T> {
+    pub fn from_raw(raw: usize) -> Self {
+        Self(raw, PhantomData)
+    }
+
+    pub fn into_raw(self) -> usize {
+        self.0
+    }
+}
+
+impl<T> Debug for Address<T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_tuple("Address").field(&self.0).finish()
+    }
+}
+
+impl<T> Clone for Address<T> {
+    fn clone(&self) -> Self {
+        Self(self.0, PhantomData)
+    }
+}
+
+impl<T> Copy for Address<T> {}
+
+impl<T> PartialEq for Address<T> {
+    fn eq(&self, other: &Self) -> bool {
+        self.0 == other.0
+    }
+}
+
+impl<T> Eq for Address<T> {}
+
+impl<T> Hash for Address<T> {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.0.hash(state);
+    }
+}

src/sandbox/mod.rs

@@ -2,24 +2,27 @@
 
 mod serialize;
 
-use crate::latest::ModInfo;
+use crate::{
+    latest::ModInfo,
+    misc::object_store::{Address, ObjectStore},
+};
 use std::{
     collections::{HashMap, HashSet},
     iter::repeat_with,
-    num::NonZeroU32,
 };
 
 /// An in-memory representation of a Sandbox that is easy to modify.
 pub struct Sandbox {
-    next_component: u32,
-    next_net: u32,
-    next_wire: u32,
-    next_type: u16,
     root_components: HashSet<ComponentId>,
-    components: HashMap<ComponentId, ComponentInfo>,
+    components: ObjectStore<ComponentInfo>,
-    wires: HashMap<WireId, WireInfo>,
+    wires: ObjectStore<WireInfo>,
+
+    next_net: u32,
     nets: HashMap<NetId, NetInfo>,
+
+    next_type: u16,
     component_types: HashMap<String, u16>,
+
     mods: Vec<ModInfo>,
 }
 
@@ -32,15 +35,16 @@ impl Sandbox {
     /// Create an empty sandbox level with custom component-type / mod info.
     fn with_meta_info(component_types: HashMap<String, u16>, mods: Vec<ModInfo>) -> Self {
         Self {
-            next_component: 1,
-            next_net: 0,
-            next_wire: 0,
-            next_type: component_types.values().copied().max().unwrap_or(0),
             root_components: HashSet::new(),
-            components: HashMap::new(),
+            components: ObjectStore::new(),
-            wires: HashMap::new(),
+            wires: ObjectStore::new(),
+
+            next_net: 0,
             nets: HashMap::new(),
+
+            next_type: component_types.values().copied().max().unwrap_or(0),
             component_types,
+
             mods,
         }
     }
@@ -66,30 +70,23 @@ impl Sandbox {
             .collect(),
             custom_data: component.custom_data.clone(),
         };
-        let id = ComponentId(NonZeroU32::new(self.next_component).unwrap());
+        self.insert_component(info)
-        self.insert_component(id, info);
-        id
     }
 
-    /// Internal logic of `add_component` that is shared with savefile loading.
+    // Manually insert a generated component info.
-    /// Savefiles store the component and net IDs, so they can be directly
+    // Used to efficiently insert data from a save file.
-    /// passed instead of being allocated (as is done by `add_component`).
+    fn insert_component(&mut self, info: ComponentInfo) -> ComponentId {
-    fn insert_component(&mut self, id: ComponentId, info: ComponentInfo) {
+        // Add component info.
-        // TODO bubble this error (could be triggered by an invalid savefile).
+        let parent_id = info.parent;
-        assert!(!self.components.contains_key(&id));
+        let id = ComponentId(self.components.insert(info));
-
-        // Update component ID allocator.
-        // TODO make this less sparse, 2 billion is not very much to work with
-        // over the lifetime of a world.
-        self.next_component = self.next_component.max(id.into_raw() + 1);
 
         // Add parent-child cross-reference.
-        if let Some(parent) = info.parent {
+        if let Some(parent) = parent_id {
             // Valid savefiles will store and load the parent before the child,
             // so it is reasonable to assume the parent exists here.
             //TODO bubble this error; it should be recoverable in file loading.
             self.components
-                .get_mut(&parent)
+                .get_mut(parent.0)
                 .unwrap()
                 .children
                 .insert(id);
@@ -97,8 +94,7 @@ impl Sandbox {
             self.root_components.insert(id);
         }
 
-        // Add component info.
+        id
-        self.components.insert(id, info);
     }
 
     pub fn add_wire(
@@ -157,17 +153,13 @@ impl Sandbox {
         };
 
         // Create wire.
-        let wire_id = WireId(self.next_wire);
+        let info = WireInfo {
-        self.next_wire += 1;
-        self.wires.insert(
-            wire_id,
-            WireInfo {
             a: addr_a,
             b: addr_b,
             net_id,
             rotation,
-            },
+        };
-        );
+        let wire_id = WireId(self.wires.insert(info));
 
         // Register wire references in pegs and net.
         self.get_peg_mut(&addr_a).unwrap().wires.insert(wire_id);
@@ -179,7 +171,7 @@ impl Sandbox {
 
     pub fn remove_component(&mut self, id: ComponentId) {
         // Remove component.
-        let component = match self.components.remove(&id) {
+        let component = match self.components.remove(id.0) {
             Some(x) => x,
             None => {
                 // If component doesn't exist, nothing needs to be done.
@@ -229,7 +221,7 @@ impl Sandbox {
         // parent; ignore it.
         if let Some(parent) = component
             .parent
-            .and_then(|parent_id| self.components.get_mut(&parent_id))
+            .and_then(|parent_id| self.components.get_mut(parent_id.0))
         {
             parent.children.remove(&id);
         }
@@ -243,7 +235,7 @@ impl Sandbox {
 
     pub fn remove_wire(&mut self, id: WireId) {
         // Remove wire.
-        let wire = match self.wires.remove(&id) {
+        let wire = match self.wires.remove(id.0) {
             Some(x) => x,
             None => {
                 // If wire doesn't exist, nothing needs to be done.
@@ -302,7 +294,7 @@ impl Sandbox {
         let src = self.nets.remove(&id_src).unwrap();
         // Update net cross-references:
         for wire_id in &src.wires {
-            self.wires.get_mut(wire_id).unwrap().net_id = id_dest;
+            self.wires.get_mut(wire_id.0).unwrap().net_id = id_dest;
         }
         for peg_id in &src.pegs {
             self.get_peg_mut(peg_id).unwrap().net_id = id_dest;
@@ -338,7 +330,7 @@ impl Sandbox {
 
         while let Some(peg_addr) = frontier.pop() {
             for wire_id in &self.get_peg(&peg_addr).unwrap().wires {
-                let wire = &self.wires[wire_id];
+                let wire = self.wires.get(wire_id.0).unwrap();
                 let neighbor = if peg_addr == wire.a {
                     wire.b
                 } else if peg_addr == wire.b {
@@ -365,19 +357,19 @@ impl Sandbox {
             self.get_peg_mut(&peg_addr).unwrap().net_id = net_id;
         }
         for wire_id in visited_wires {
-            self.wires.get_mut(&wire_id).unwrap().net_id = net_id;
+            self.wires.get_mut(wire_id.0).unwrap().net_id = net_id;
         }
     }
 
     fn get_peg(&self, addr: &PegAddress) -> Option<&PegInfo> {
         self.components
-            .get(&addr.component)
+            .get(addr.component.0)
             .and_then(|component| component.get_peg(addr))
     }
 
     fn get_peg_mut(&mut self, addr: &PegAddress) -> Option<&mut PegInfo> {
         self.components
-            .get_mut(&addr.component)
+            .get_mut(addr.component.0)
             .and_then(|component| component.get_peg_mut(addr))
     }
 }
@@ -438,17 +430,7 @@ impl<'a> ComponentBuilder<'a> {
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct ComponentId(NonZeroU32);
+pub struct ComponentId(Address<ComponentInfo>);
-
-impl ComponentId {
-    fn into_raw(self) -> u32 {
-        self.0.into()
-    }
-
-    fn from_raw(raw: u32) -> Option<Self> {
-        NonZeroU32::new(raw).map(Self)
-    }
-}
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct NetId(u32);
@@ -464,7 +446,7 @@ impl NetId {
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct WireId(u32);
+pub struct WireId(Address<WireInfo>);
 
 #[derive(Debug)]
 pub enum AddWireError {

src/sandbox/serialize.rs

@@ -1,22 +1,104 @@
 //! Conversions between the Sandbox editor and Blotter serialization types.
 
 use crate::latest as blotter;
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 
 /// The current game version that this save is compatible with.
 const GAME_VERSION: [i32; 4] = [0, 91, 0, 510];
 
+/// Serialization context, mainly tracks ID mappings
+struct Serializer {
+    next_component_id: u32,
+    component_id_map: HashMap<super::ComponentId, u32>,
+}
+
+impl Serializer {
+    fn new() -> Self {
+        Self {
+            next_component_id: 1,
+            component_id_map: HashMap::new(),
+        }
+    }
+
+    fn register_component(&mut self, id: super::ComponentId) -> u32 {
+        let raw_id = self.next_component_id;
+        self.next_component_id += 1;
+        self.component_id_map.insert(id, raw_id);
+        raw_id
+    }
+
+    fn get_component(&self, id: super::ComponentId) -> u32 {
+        self.component_id_map[&id]
+    }
+
+    fn serialize_component(
+        &mut self,
+        id: super::ComponentId,
+        info: &super::ComponentInfo,
+    ) -> blotter::Component {
+        blotter::Component {
+            address: self.register_component(id),
+            parent: info.parent.map(|id| self.get_component(id)).unwrap_or(0),
+            type_id: info.type_id,
+            position: info.position,
+            rotation: info.rotation,
+            inputs: info
+                .inputs
+                .iter()
+                .map(|peg| self.serialize_input(peg))
+                .collect(),
+            outputs: info
+                .outputs
+                .iter()
+                .map(|peg| self.serialize_output(peg))
+                .collect(),
+            custom_data: info.custom_data.clone(),
+        }
+    }
+
+    fn serialize_input(&self, peg: &super::PegInfo) -> blotter::Input {
+        blotter::Input {
+            circuit_state_id: peg.net_id.into_raw(),
+        }
+    }
+
+    fn serialize_output(&self, peg: &super::PegInfo) -> blotter::Output {
+        blotter::Output {
+            circuit_state_id: peg.net_id.into_raw(),
+        }
+    }
+
+    fn serialize_wire(&self, wire: &super::WireInfo) -> blotter::Wire {
+        blotter::Wire {
+            start_peg: self.serialize_peg_address(&wire.a),
+            end_peg: self.serialize_peg_address(&wire.b),
+            circuit_state_id: wire.net_id.into_raw(),
+            rotation: wire.rotation,
+        }
+    }
+
+    fn serialize_peg_address(&self, addr: &super::PegAddress) -> blotter::PegAddress {
+        blotter::PegAddress {
+            peg_type: addr.peg_type.into(),
+            component_address: self.get_component(addr.component),
+            peg_index: addr.peg_index.try_into().unwrap(),
+        }
+    }
+}
+
 impl From<&super::Sandbox> for blotter::BlotterFile {
     fn from(sandbox: &super::Sandbox) -> Self {
+        let mut ser = Serializer::new();
+
         // Blotter format requires that parents must be serialized before children.
-        // TODO Serialize components with depth-first, pre-order traversal.
+        // Serialize components with depth-first, pre-order traversal.
         let mut components = Vec::new();
-        let mut stack = Vec::new();
+        let mut stack: Vec<super::ComponentId> = Vec::new();
         stack.extend(&sandbox.root_components);
         while let Some(component_id) = stack.pop() {
-            let component = &sandbox.components[&component_id];
+            let component = sandbox.components.get(component_id.0).unwrap();
             stack.extend(&component.children);
-            components.push((component_id, component).into())
+            components.push(ser.serialize_component(component_id, component))
         }
 
         Self {
@@ -35,7 +117,7 @@ impl From<&super::Sandbox> for blotter::BlotterFile {
             wires: sandbox
                 .wires
                 .iter()
-                .map(|(_id, wire)| wire.into())
+                .map(|(_id, wire)| ser.serialize_wire(wire))
                 .collect(),
             circuit_states: blotter::CircuitStates::WorldFormat {
                 circuit_states: vec![], //TODO
@@ -44,8 +126,87 @@ impl From<&super::Sandbox> for blotter::BlotterFile {
     }
 }
 
+/// Deserialization context, mainly tracks ID mappings
+struct Deserializer {
+    component_id_map: HashMap<u32, super::ComponentId>,
+}
+
+impl Deserializer {
+    fn new() -> Self {
+        Self {
+            component_id_map: HashMap::new(),
+        }
+    }
+
+    fn register_component(&mut self, raw_id: u32, id: super::ComponentId) {
+        self.component_id_map.insert(raw_id, id);
+    }
+
+    fn get_component(&self, id: u32) -> super::ComponentId {
+        self.component_id_map[&id]
+    }
+
+    fn deserialize_component(&self, component: &blotter::Component) -> super::ComponentInfo {
+        super::ComponentInfo {
+            type_id: component.type_id,
+            parent: if component.parent == 0 {
+                None
+            } else {
+                Some(self.get_component(component.parent))
+            },
+            position: component.position,
+            rotation: component.rotation,
+            children: HashSet::new(),
+            inputs: component
+                .inputs
+                .iter()
+                .map(|input| self.deserialize_input(input))
+                .collect(),
+            outputs: component
+                .outputs
+                .iter()
+                .map(|output| self.deserialize_output(output))
+                .collect(),
+            custom_data: component.custom_data.clone(),
+        }
+    }
+
+    fn deserialize_input(&self, input: &blotter::Input) -> super::PegInfo {
+        super::PegInfo {
+            net_id: super::NetId::from_raw(input.circuit_state_id),
+            wires: HashSet::new(),
+        }
+    }
+
+    fn deserialize_output(&self, output: &blotter::Output) -> super::PegInfo {
+        super::PegInfo {
+            net_id: super::NetId::from_raw(output.circuit_state_id),
+            wires: HashSet::new(),
+        }
+    }
+
+    fn deserialize_wire(&self, wire: &blotter::Wire) -> super::WireInfo {
+        super::WireInfo {
+            a: self.deserialize_peg_address(&wire.start_peg),
+            b: self.deserialize_peg_address(&wire.end_peg),
+            net_id: super::NetId::from_raw(wire.circuit_state_id),
+            rotation: wire.rotation,
+        }
+    }
+
+    fn deserialize_peg_address(&self, addr: &blotter::PegAddress) -> super::PegAddress {
+        super::PegAddress {
+            peg_type: addr.peg_type.into(),
+            component: self.get_component(addr.component_address),
+            peg_index: addr.peg_index.try_into().unwrap(),
+        }
+    }
+}
+
 impl From<&blotter::BlotterFile> for super::Sandbox {
     fn from(file: &blotter::BlotterFile) -> Self {
+        let mut de = Deserializer::new();
+
         // Instead of building the sandbox and all the internal cross-references
         // from scratch, re-use the sandbox API as much as possible when loading
         // so there is just one implementation of the cross-referencing.
@@ -56,19 +217,16 @@ impl From<&blotter::BlotterFile> for super::Sandbox {
                 .collect(),
             file.mods.clone(),
         );
+
         for component in &file.components {
-            let (id, info) = component.into();
+            let id = sandbox.insert_component(de.deserialize_component(component));
-            sandbox.insert_component(id, info);
+            de.register_component(component.address, id);
         }
         for wire in &file.wires {
             // TODO bubble error
+            let info = de.deserialize_wire(wire);
             sandbox
-                .insert_wire(
+                .insert_wire(info.a, info.b, info.rotation, Some(info.net_id))
-                    wire.start_peg.into(),
-                    wire.end_peg.into(),
-                    wire.rotation,
-                    Some(super::NetId::from_raw(wire.circuit_state_id)),
-                )
                 .unwrap();
         }
 
@@ -78,108 +236,6 @@ impl From<&blotter::BlotterFile> for super::Sandbox {
     }
 }
 
-impl From<(super::ComponentId, &super::ComponentInfo)> for blotter::Component {
-    fn from((id, component): (super::ComponentId, &super::ComponentInfo)) -> Self {
-        Self {
-            address: id.into_raw(),
-            parent: component.parent.map(|id| id.into_raw()).unwrap_or(0),
-            type_id: component.type_id,
-            position: component.position,
-            rotation: component.rotation,
-            inputs: component.inputs.iter().map(|peg| peg.into()).collect(),
-            outputs: component.outputs.iter().map(|peg| peg.into()).collect(),
-            custom_data: component.custom_data.clone(),
-        }
-    }
-}
-
-impl From<&blotter::Component> for (super::ComponentId, super::ComponentInfo) {
-    fn from(component: &blotter::Component) -> Self {
-        let id = super::ComponentId::from_raw(component.address).unwrap();
-        let info = super::ComponentInfo {
-            type_id: component.type_id,
-            parent: super::ComponentId::from_raw(component.parent),
-            position: component.position,
-            rotation: component.rotation,
-            children: HashSet::new(),
-            inputs: component.inputs.iter().map(|input| input.into()).collect(),
-            outputs: component
-                .outputs
-                .iter()
-                .map(|output| output.into())
-                .collect(),
-            custom_data: component.custom_data.clone(),
-        };
-        (id, info)
-    }
-}
-
-impl From<&super::PegInfo> for blotter::Input {
-    fn from(peg: &super::PegInfo) -> Self {
-        Self {
-            circuit_state_id: peg.net_id.into_raw(),
-        }
-    }
-}
-
-impl From<&blotter::Input> for super::PegInfo {
-    fn from(input: &blotter::Input) -> Self {
-        Self {
-            net_id: super::NetId::from_raw(input.circuit_state_id),
-            wires: HashSet::new(),
-        }
-    }
-}
-
-impl From<&super::PegInfo> for blotter::Output {
-    fn from(peg: &super::PegInfo) -> Self {
-        Self {
-            circuit_state_id: peg.net_id.into_raw(),
-        }
-    }
-}
-
-impl From<&blotter::Output> for super::PegInfo {
-    fn from(input: &blotter::Output) -> Self {
-        Self {
-            net_id: super::NetId::from_raw(input.circuit_state_id),
-            wires: HashSet::new(),
-        }
-    }
-}
-
-impl From<&super::WireInfo> for blotter::Wire {
-    fn from(wire: &super::WireInfo) -> Self {
-        Self {
-            start_peg: wire.a.into(),
-            end_peg: wire.b.into(),
-            circuit_state_id: wire.net_id.into_raw(),
-            rotation: wire.rotation,
-        }
-    }
-}
-
-impl From<super::PegAddress> for blotter::PegAddress {
-    fn from(addr: super::PegAddress) -> Self {
-        Self {
-            peg_type: addr.peg_type.into(),
-            component_address: addr.component.into_raw(),
-            peg_index: addr.peg_index.try_into().unwrap(),
-        }
-    }
-}
-
-impl From<blotter::PegAddress> for super::PegAddress {
-    fn from(addr: blotter::PegAddress) -> Self {
-        //TODO bubble errors
-        Self {
-            component: super::ComponentId::from_raw(addr.component_address).unwrap(),
-            peg_type: addr.peg_type.into(),
-            peg_index: addr.peg_index.try_into().unwrap(),
-        }
-    }
-}
-
 impl From<super::PegType> for blotter::PegType {
     fn from(peg_type: super::PegType) -> Self {
         match peg_type {