Replace u64 with Uid in some places and add/modify some comments

common/net/src/sync/packet.rs

@@ -101,26 +101,26 @@ pub enum CompUpdateKind<P: CompPacket> {
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct EntityPackage<P: CompPacket> {
-    pub uid: u64,
+    pub uid: Uid,
     pub comps: Vec<P>,
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct EntitySyncPackage {
-    pub created_entities: Vec<u64>,
+    pub created_entities: Vec<Uid>,
-    pub deleted_entities: Vec<u64>,
+    pub deleted_entities: Vec<Uid>,
 }
 impl EntitySyncPackage {
     pub fn new(
         uids: &ReadStorage<'_, Uid>,
         uid_tracker: &UpdateTracker<Uid>,
         filter: impl Join + Copy,
-        deleted_entities: Vec<u64>,
+        deleted_entities: Vec<Uid>,
     ) -> Self {
         // Add created and deleted entities
         let created_entities = (uids, filter, uid_tracker.inserted())
             .join()
-            .map(|(uid, _, _)| (*uid).into())
+            .map(|(uid, _, _)| *uid)
             .collect();
 
         Self {

common/net/src/sync/sync_ext.rs

@@ -146,8 +146,7 @@ impl WorldSyncExt for specs::World {
 // Private utilities
 //
 // Only used on the client.
-fn create_entity_with_uid(specs_world: &mut specs::World, entity_uid: u64) -> specs::Entity {
+fn create_entity_with_uid(specs_world: &mut specs::World, entity_uid: Uid) -> specs::Entity {
-    let entity_uid = Uid::from(entity_uid);
     let existing_entity = specs_world.read_resource::<IdMaps>().uid_entity(entity_uid);
 
     // TODO: Are there any expected cases where there is an existing entity with

common/src/region.rs

@@ -7,6 +7,9 @@ use vek::*;
 
 pub enum Event {
     // Contains the key of the region the entity moved to
+    // TODO: We only actually use the info from this when the entity moves to another region and
+    // isn't deleted, but we still generate and process these events in the case where the entity
+    // was deleted.
     Left(u32, Option<Vec2<i32>>),
     // Contains the key of the region the entity came from
     Entered(u32, Option<Vec2<i32>>),
@@ -147,7 +150,8 @@ impl RegionMap {
                         // component) TODO: distribute this between ticks
                         None => {
                             // TODO: shouldn't there be a way to extract the bitset of entities with
-                            // positions directly from specs?
+                            // positions directly from specs? Yes, with `.mask()` on the component
+                            // storage.
                             entities_to_remove.push((i, id));
                         },
                     }

server/src/sys/entity_sync.rs

@@ -8,7 +8,6 @@ use common::{
     region::{Event as RegionEvent, RegionMap},
     resources::{PlayerPhysicsSettings, Time, TimeOfDay, TimeScale},
     terrain::TerrainChunkSize,
-    uid::Uid,
     vol::RectVolSize,
 };
 use common_ecs::{Job, Origin, Phase, System};
@@ -192,7 +191,8 @@ impl<'a> System<'a> for Sys {
                                         .unwrap_or(true)
                                     {
                                         // TODO: I suspect it would be more efficient (in terms of
-                                        // bandwidth) to batch messages like this.
+                                        // bandwidth) to batch messages like this (same in
+                                        // subscription.rs).
                                         client.send_fallible(ServerGeneral::DeleteEntity(uid));
                                     }
                                 }
@@ -352,7 +352,7 @@ impl<'a> System<'a> for Sys {
                 })
             {
                 for uid in &deleted {
-                    client.send_fallible(ServerGeneral::DeleteEntity(Uid(*uid)));
+                    client.send_fallible(ServerGeneral::DeleteEntity(*uid));
                 }
             }
         }

server/src/sys/sentinel.rs

@@ -80,7 +80,6 @@ macro_rules! trackers {
                 vel: Option<Vel>,
                 ori: Option<Ori>,
             ) -> EntityPackage<EcsCompPacket> {
-                let uid = uid.0;
                 let mut comps = Vec::new();
                 // NOTE: we could potentially include a bitmap indicating which components are present instead of tagging
                 // components with the type in order to save bandwidth
@@ -208,7 +207,7 @@ macro_rules! trackers {
                 &self,
                 comps: &TrackedStorages,
                 filter: impl Join + Copy,
-                deleted_entities: Vec<u64>,
+                deleted_entities: Vec<Uid>,
             ) -> (EntitySyncPackage, CompSyncPackage<EcsCompPacket>) {
                 let entity_sync_package =
                     EntitySyncPackage::new(&comps.uid, &self.uid, filter, deleted_entities);
@@ -280,7 +279,7 @@ common_net::synced_components!(trackers);
 
 /// Deleted entities grouped by region
 pub struct DeletedEntities {
-    map: HashMap<Vec2<i32>, Vec<u64>>,
+    map: HashMap<Vec2<i32>, Vec<Uid>>,
 }
 
 impl Default for DeletedEntities {
@@ -293,18 +292,18 @@ impl Default for DeletedEntities {
 
 impl DeletedEntities {
     pub fn record_deleted_entity(&mut self, uid: Uid, region_key: Vec2<i32>) {
-        self.map.entry(region_key).or_default().push(uid.into());
+        self.map.entry(region_key).or_default().push(uid);
     }
 
-    pub fn take_deleted_in_region(&mut self, key: Vec2<i32>) -> Vec<u64> {
+    pub fn take_deleted_in_region(&mut self, key: Vec2<i32>) -> Vec<Uid> {
         self.map.remove(&key).unwrap_or_default()
     }
 
-    pub fn get_deleted_in_region(&self, key: Vec2<i32>) -> &[u64] {
+    pub fn get_deleted_in_region(&self, key: Vec2<i32>) -> &[Uid] {
         self.map.get(&key).map_or(&[], |v| v.as_slice())
     }
 
-    pub fn take_remaining_deleted(&mut self) -> impl Iterator<Item = (Vec2<i32>, Vec<u64>)> + '_ {
+    pub fn take_remaining_deleted(&mut self) -> impl Iterator<Item = (Vec2<i32>, Vec<Uid>)> + '_ {
         self.map.drain()
     }
 }

server/src/sys/subscription.rs

@@ -135,10 +135,10 @@ impl<'a> System<'a> for Sys {
                         // TODO: consider changing system ordering??
                         for event in region.events() {
                             match event {
-                                RegionEvent::Entered(_, _) => {}, /* These don't need to be */
+                                RegionEvent::Entered(_, _) => {
-                                // processed because this
+                                    // These don't need to be processed because
-                                // region is being thrown out
+                                    // this region is being thrown out anyway
-                                // anyway
+                                },
                                 RegionEvent::Left(id, maybe_key) => {
                                     // Lookup UID for entity
                                     // Doesn't overlap with entity deletion in sync packages
@@ -147,7 +147,9 @@ impl<'a> System<'a> for Sys {
                                     if let Some(&uid) = uids.get(entities.entity(*id)) {
                                         if !maybe_key
                                             .as_ref()
-                                            // Don't need to check that this isn't also in the regions to remove since the entity will be removed when we get to that one
+                                            // Don't need to check that this isn't also in the
+                                            // regions to remove since the entity will be removed 
+                                            // when we get to that one.
                                             .map(|key| subscription.regions.contains(key))
                                             .unwrap_or(false)
                                         {
@@ -162,10 +164,10 @@ impl<'a> System<'a> for Sys {
                             client.send_fallible(ServerGeneral::DeleteEntity(uid));
                         }
                     }
-                    // Send deleted entities since they won't be processed for this client in entity
+                    // Send deleted entities since they won't be processed for this client
-                    // sync
+                    // in entity sync
                     for uid in deleted_entities.get_deleted_in_region(key).iter() {
-                        client.send_fallible(ServerGeneral::DeleteEntity(Uid(*uid)));
+                        client.send_fallible(ServerGeneral::DeleteEntity(*uid));
                     }
                 }
 
@@ -195,6 +197,7 @@ impl<'a> System<'a> for Sys {
                                         ori.copied(),
                                     )
                                 })
+                                // TODO: batch this into a single message
                                 .for_each(|msg| {
                                     // Send message to create entity and tracked components and
                                     // physics components