From 4da38b0993db734ece6c639ef0969e541c8e0868 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Fri, 18 Nov 2022 03:18:15 -0800
Subject: [PATCH 01/15] Add TableId and ArchetypeId

---
 crates/bevy_ecs/src/archetype.rs        | 16 +++++++-----
 crates/bevy_ecs/src/bundle.rs           | 14 ++++------
 crates/bevy_ecs/src/entity/mod.rs       | 13 +++++++---
 crates/bevy_ecs/src/query/iter.rs       |  9 ++++---
 crates/bevy_ecs/src/query/state.rs      |  7 +++--
 crates/bevy_ecs/src/storage/table.rs    | 10 +++++---
 crates/bevy_ecs/src/world/entity_ref.rs | 34 ++++++++++++-------------
 crates/bevy_ecs/src/world/mod.rs        |  4 +--
 8 files changed, 57 insertions(+), 50 deletions(-)

diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs
index b6bcc154cd561..a6b6fa4c9a9fa 100644
--- a/crates/bevy_ecs/src/archetype.rs
+++ b/crates/bevy_ecs/src/archetype.rs
@@ -275,8 +275,8 @@ impl Archetype {
     }
 
     #[inline]
-    pub(crate) fn set_entity_table_row(&mut self, index: usize, table_row: usize) {
-        self.entities[index].table_row = table_row;
+    pub(crate) fn set_entity_table_row(&mut self, index: u32, table_row: usize) {
+        self.entities[index as usize].table_row = table_row;
     }
 
     /// # Safety
@@ -287,7 +287,9 @@ impl Archetype {
 
         EntityLocation {
             archetype_id: self.id,
-            index: self.entities.len() - 1,
+            archetype_index: (self.entities.len() - 1) as u32,
+            table_id: self.table_id,
+            table_row: table_row as u32,
         }
     }
 
@@ -297,14 +299,14 @@ impl Archetype {
 
     /// Removes the entity at `index` by swapping it out. Returns the table row the entity is stored
     /// in.
-    pub(crate) fn swap_remove(&mut self, index: usize) -> ArchetypeSwapRemoveResult {
-        let is_last = index == self.entities.len() - 1;
-        let entity = self.entities.swap_remove(index);
+    pub(crate) fn swap_remove(&mut self, index: u32) -> ArchetypeSwapRemoveResult {
+        let is_last = index as usize == self.entities.len() - 1;
+        let entity = self.entities.swap_remove(index as usize);
         ArchetypeSwapRemoveResult {
             swapped_entity: if is_last {
                 None
             } else {
-                Some(self.entities[index].entity)
+                Some(self.entities[index as usize].entity)
             },
             table_row: entity.table_row,
         }
diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs
index 91f29e4ac57e2..0b1b81ccda550 100644
--- a/crates/bevy_ecs/src/bundle.rs
+++ b/crates/bevy_ecs/src/bundle.rs
@@ -528,13 +528,9 @@ impl<'a, 'b> BundleInserter<'a, 'b> {
     pub unsafe fn insert<T: Bundle>(
         &mut self,
         entity: Entity,
-        archetype_index: usize,
+        location: EntityLocation,
         bundle: T,
     ) -> EntityLocation {
-        let location = EntityLocation {
-            index: archetype_index,
-            archetype_id: self.archetype.id(),
-        };
         match &mut self.result {
             InsertBundleResult::SameArchetype => {
                 // PERF: this could be looked up during Inserter construction and stored (but borrowing makes this nasty)
@@ -548,14 +544,14 @@ impl<'a, 'b> BundleInserter<'a, 'b> {
                     self.sparse_sets,
                     add_bundle,
                     entity,
-                    self.archetype.entity_table_row(archetype_index),
+                    location.table_row as usize,
                     self.change_tick,
                     bundle,
                 );
                 location
             }
             InsertBundleResult::NewArchetypeSameTable { new_archetype } => {
-                let result = self.archetype.swap_remove(location.index);
+                let result = self.archetype.swap_remove(location.archetype_index);
                 if let Some(swapped_entity) = result.swapped_entity {
                     self.entities.meta[swapped_entity.index as usize].location = location;
                 }
@@ -583,7 +579,7 @@ impl<'a, 'b> BundleInserter<'a, 'b> {
                 new_archetype,
                 new_table,
             } => {
-                let result = self.archetype.swap_remove(location.index);
+                let result = self.archetype.swap_remove(location.archetype_index);
                 if let Some(swapped_entity) = result.swapped_entity {
                     self.entities.meta[swapped_entity.index as usize].location = location;
                 }
@@ -611,7 +607,7 @@ impl<'a, 'b> BundleInserter<'a, 'b> {
                     };
 
                     swapped_archetype
-                        .set_entity_table_row(swapped_location.index, result.table_row);
+                        .set_entity_table_row(swapped_location.archetype_index, result.table_row);
                 }
 
                 // PERF: this could be looked up during Inserter construction and stored (but borrowing makes this nasty)
diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs
index 844885413716e..07a767d65cd79 100644
--- a/crates/bevy_ecs/src/entity/mod.rs
+++ b/crates/bevy_ecs/src/entity/mod.rs
@@ -34,7 +34,10 @@ mod map_entities;
 
 pub use map_entities::*;
 
-use crate::{archetype::ArchetypeId, storage::SparseSetIndex};
+use crate::{
+    archetype::ArchetypeId,
+    storage::{SparseSetIndex, TableId},
+};
 use serde::{Deserialize, Serialize};
 use std::{convert::TryFrom, fmt, mem, sync::atomic::Ordering};
 
@@ -676,7 +679,9 @@ impl EntityMeta {
         generation: 0,
         location: EntityLocation {
             archetype_id: ArchetypeId::INVALID,
-            index: usize::MAX, // dummy value, to be filled in
+            archetype_index: u32::MAX, // dummy value, to be filled in
+            table_id: TableId::INVALID,
+            table_row: u32::MAX, // dummy value, to be filled in
         },
     };
 }
@@ -689,7 +694,9 @@ pub struct EntityLocation {
     pub archetype_id: ArchetypeId,
 
     /// The index of the entity in the archetype
-    pub index: usize,
+    pub archetype_index: u32,
+    pub table_id: TableId,
+    pub table_row: u32,
 }
 
 #[cfg(test)]
diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs
index 8924c54829a2d..9c2a1a80496c3 100644
--- a/crates/bevy_ecs/src/query/iter.rs
+++ b/crates/bevy_ecs/src/query/iter.rs
@@ -176,12 +176,15 @@ where
                 table,
             );
 
-            let table_row = archetype.entity_table_row(location.index);
             // SAFETY: set_archetype was called prior.
             // `location.index` is an archetype index row in range of the current archetype, because if it was not, the match above would have `continue`d
-            if F::filter_fetch(&mut self.filter, entity, table_row) {
+            if F::filter_fetch(&mut self.filter, entity, location.table_row as usize) {
                 // SAFETY: set_archetype was called prior, `location.index` is an archetype index in range of the current archetype
-                return Some(Q::fetch(&mut self.fetch, entity, table_row));
+                return Some(Q::fetch(
+                    &mut self.fetch,
+                    entity,
+                    location.table_row as usize,
+                ));
             }
         }
         None
diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs
index 245ff7dacb7b4..b48cb791b91df 100644
--- a/crates/bevy_ecs/src/query/state.rs
+++ b/crates/bevy_ecs/src/query/state.rs
@@ -418,17 +418,16 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
         let mut fetch = Q::init_fetch(world, &self.fetch_state, last_change_tick, change_tick);
         let mut filter = F::init_fetch(world, &self.filter_state, last_change_tick, change_tick);
 
-        let table_row = archetype.entity_table_row(location.index);
         let table = world
             .storages()
             .tables
-            .get(archetype.table_id())
+            .get(location.table_id)
             .debug_checked_unwrap();
         Q::set_archetype(&mut fetch, &self.fetch_state, archetype, table);
         F::set_archetype(&mut filter, &self.filter_state, archetype, table);
 
-        if F::filter_fetch(&mut filter, entity, table_row) {
-            Ok(Q::fetch(&mut fetch, entity, table_row))
+        if F::filter_fetch(&mut filter, entity, location.table_row as usize) {
+            Ok(Q::fetch(&mut fetch, entity, location.table_row as usize))
         } else {
             Err(QueryEntityError::QueryDoesNotMatch(entity))
         }
diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs
index aa7bb32e535ec..3b6b498de4b4f 100644
--- a/crates/bevy_ecs/src/storage/table.rs
+++ b/crates/bevy_ecs/src/storage/table.rs
@@ -13,17 +13,19 @@ use std::{
 };
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub struct TableId(usize);
+pub struct TableId(u32);
 
 impl TableId {
+    pub(crate) const INVALID: TableId = TableId(u32::MAX);
+
     #[inline]
     pub fn new(index: usize) -> Self {
-        TableId(index)
+        TableId(index as u32)
     }
 
     #[inline]
     pub fn index(self) -> usize {
-        self.0
+        self.0 as usize
     }
 
     #[inline]
@@ -585,7 +587,7 @@ impl Tables {
                     table.add_column(components.get_info_unchecked(*component_id));
                 }
                 tables.push(table.build());
-                (component_ids.to_vec(), TableId(tables.len() - 1))
+                (component_ids.to_vec(), TableId::new(tables.len() - 1))
             });
 
         *value
diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs
index 66be80be0840c..148e9c3f86fcc 100644
--- a/crates/bevy_ecs/src/world/entity_ref.rs
+++ b/crates/bevy_ecs/src/world/entity_ref.rs
@@ -266,7 +266,7 @@ impl<'w> EntityMut<'w> {
         );
         // SAFETY: location matches current entity. `T` matches `bundle_info`
         unsafe {
-            self.location = bundle_inserter.insert(self.entity, self.location.index, bundle);
+            self.location = bundle_inserter.insert(self.entity, self.location, bundle);
         }
 
         self
@@ -368,7 +368,7 @@ impl<'w> EntityMut<'w> {
         new_archetype_id: ArchetypeId,
     ) {
         let old_archetype = &mut archetypes[old_archetype_id];
-        let remove_result = old_archetype.swap_remove(old_location.index);
+        let remove_result = old_archetype.swap_remove(old_location.archetype_index);
         if let Some(swapped_entity) = remove_result.swapped_entity {
             entities.meta[swapped_entity.index as usize].location = old_location;
         }
@@ -397,7 +397,7 @@ impl<'w> EntityMut<'w> {
             if let Some(swapped_entity) = move_result.swapped_entity {
                 let swapped_location = entities.get(swapped_entity).unwrap();
                 archetypes[swapped_location.archetype_id]
-                    .set_entity_table_row(swapped_location.index, old_table_row);
+                    .set_entity_table_row(swapped_location.archetype_index, old_table_row);
             }
 
             new_location
@@ -498,7 +498,7 @@ impl<'w> EntityMut<'w> {
                     .get_or_insert_with(component_id, Vec::new);
                 removed_components.push(self.entity);
             }
-            let remove_result = archetype.swap_remove(location.index);
+            let remove_result = archetype.swap_remove(location.archetype_index);
             if let Some(swapped_entity) = remove_result.swapped_entity {
                 world.entities.meta[swapped_entity.index as usize].location = location;
             }
@@ -517,7 +517,7 @@ impl<'w> EntityMut<'w> {
         if let Some(moved_entity) = moved_entity {
             let moved_location = world.entities.get(moved_entity).unwrap();
             world.archetypes[moved_location.archetype_id]
-                .set_entity_table_row(moved_location.index, table_row);
+                .set_entity_table_row(moved_location.archetype_index, table_row);
         }
     }
 
@@ -610,11 +610,10 @@ pub(crate) unsafe fn get_component(
     let component_info = world.components.get_info_unchecked(component_id);
     match component_info.storage_type() {
         StorageType::Table => {
-            let table = &world.storages.tables[archetype.table_id()];
+            let table = &world.storages.tables[location.table_id];
             let components = table.get_column(component_id)?;
-            let table_row = archetype.entity_table_row(location.index);
             // SAFETY: archetypes only store valid table_rows and the stored component type is T
-            Some(components.get_data_unchecked(table_row))
+            Some(components.get_data_unchecked(location.table_row as usize))
         }
         StorageType::SparseSet => world
             .storages
@@ -640,13 +639,12 @@ unsafe fn get_component_and_ticks(
     let component_info = world.components.get_info_unchecked(component_id);
     match component_info.storage_type() {
         StorageType::Table => {
-            let table = &world.storages.tables[archetype.table_id()];
+            let table = &world.storages.tables[location.table_id];
             let components = table.get_column(component_id)?;
-            let table_row = archetype.entity_table_row(location.index);
             // SAFETY: archetypes only store valid table_rows and the stored component type is T
             Some((
-                components.get_data_unchecked(table_row),
-                components.get_ticks_unchecked(table_row),
+                components.get_data_unchecked(location.table_row as usize),
+                components.get_ticks_unchecked(location.table_row as usize),
             ))
         }
         StorageType::SparseSet => world
@@ -668,11 +666,10 @@ unsafe fn get_ticks(
     let component_info = world.components.get_info_unchecked(component_id);
     match component_info.storage_type() {
         StorageType::Table => {
-            let table = &world.storages.tables[archetype.table_id()];
+            let table = &world.storages.tables[location.table_id];
             let components = table.get_column(component_id)?;
-            let table_row = archetype.entity_table_row(location.index);
             // SAFETY: archetypes only store valid table_rows and the stored component type is T
-            Some(components.get_ticks_unchecked(table_row))
+            Some(components.get_ticks_unchecked(location.table_row as usize))
         }
         StorageType::SparseSet => world
             .storages
@@ -708,12 +705,13 @@ unsafe fn take_component<'a>(
     removed_components.push(entity);
     match component_info.storage_type() {
         StorageType::Table => {
-            let table = &mut storages.tables[archetype.table_id()];
+            let table = &mut storages.tables[location.table_id];
             // SAFETY: archetypes will always point to valid columns
             let components = table.get_column_mut(component_id).unwrap();
-            let table_row = archetype.entity_table_row(location.index);
             // SAFETY: archetypes only store valid table_rows and the stored component type is T
-            components.get_data_unchecked_mut(table_row).promote()
+            components
+                .get_data_unchecked_mut(location.table_row as usize)
+                .promote()
         }
         StorageType::SparseSet => storages
             .sparse_sets
diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs
index 1aab56a9f4ae7..c149bb97360bc 100644
--- a/crates/bevy_ecs/src/world/mod.rs
+++ b/crates/bevy_ecs/src/world/mod.rs
@@ -1091,7 +1091,7 @@ impl World {
                             if location.archetype_id == archetype =>
                         {
                             // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter
-                            unsafe { inserter.insert(entity, location.index, bundle) };
+                            unsafe { inserter.insert(entity, location, bundle) };
                         }
                         _ => {
                             let mut inserter = bundle_info.get_bundle_inserter(
@@ -1103,7 +1103,7 @@ impl World {
                                 change_tick,
                             );
                             // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter
-                            unsafe { inserter.insert(entity, location.index, bundle) };
+                            unsafe { inserter.insert(entity, location, bundle) };
                             spawn_or_insert =
                                 SpawnOrInsert::Insert(inserter, location.archetype_id);
                         }

From 2da3328491ac1d623965ea552908b1ff2e06af5c Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Fri, 18 Nov 2022 03:20:47 -0800
Subject: [PATCH 02/15] Cleanup

---
 crates/bevy_ecs/src/world/entity_ref.rs | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs
index 148e9c3f86fcc..070c7c4a1ec86 100644
--- a/crates/bevy_ecs/src/world/entity_ref.rs
+++ b/crates/bevy_ecs/src/world/entity_ref.rs
@@ -310,7 +310,6 @@ impl<'w> EntityMut<'w> {
             return None;
         }
 
-        let old_archetype = &mut archetypes[old_location.archetype_id];
         let mut bundle_components = bundle_info.component_ids.iter().cloned();
         let entity = self.entity;
         // SAFETY: bundle components are iterated in order, which guarantees that the component type
@@ -322,7 +321,6 @@ impl<'w> EntityMut<'w> {
                 take_component(
                     components,
                     storages,
-                    old_archetype,
                     removed_components,
                     component_id,
                     entity,
@@ -605,7 +603,6 @@ pub(crate) unsafe fn get_component(
     entity: Entity,
     location: EntityLocation,
 ) -> Option<Ptr<'_>> {
-    let archetype = &world.archetypes[location.archetype_id];
     // SAFETY: component_id exists and is therefore valid
     let component_info = world.components.get_info_unchecked(component_id);
     match component_info.storage_type() {
@@ -635,7 +632,6 @@ unsafe fn get_component_and_ticks(
     entity: Entity,
     location: EntityLocation,
 ) -> Option<(Ptr<'_>, &UnsafeCell<ComponentTicks>)> {
-    let archetype = &world.archetypes[location.archetype_id];
     let component_info = world.components.get_info_unchecked(component_id);
     match component_info.storage_type() {
         StorageType::Table => {
@@ -662,7 +658,6 @@ unsafe fn get_ticks(
     entity: Entity,
     location: EntityLocation,
 ) -> Option<&UnsafeCell<ComponentTicks>> {
-    let archetype = &world.archetypes[location.archetype_id];
     let component_info = world.components.get_info_unchecked(component_id);
     match component_info.storage_type() {
         StorageType::Table => {
@@ -694,7 +689,6 @@ unsafe fn get_ticks(
 unsafe fn take_component<'a>(
     components: &Components,
     storages: &'a mut Storages,
-    archetype: &Archetype,
     removed_components: &mut SparseSet<ComponentId, Vec<Entity>>,
     component_id: ComponentId,
     entity: Entity,

From be7bf27067b45bc105cd832df0b1d662a3c6bc04 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Fri, 18 Nov 2022 03:40:27 -0800
Subject: [PATCH 03/15] Shrink ArchetypeId

---
 crates/bevy_ecs/src/archetype.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs
index a6b6fa4c9a9fa..df378ebc62e6d 100644
--- a/crates/bevy_ecs/src/archetype.rs
+++ b/crates/bevy_ecs/src/archetype.rs
@@ -15,23 +15,23 @@ use std::{
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
 #[repr(transparent)]
-pub struct ArchetypeId(usize);
+pub struct ArchetypeId(u32);
 
 impl ArchetypeId {
     pub const EMPTY: ArchetypeId = ArchetypeId(0);
     /// # Safety:
     ///
     /// This must always have an all-1s bit pattern to ensure soundness in fast entity id space allocation.
-    pub const INVALID: ArchetypeId = ArchetypeId(usize::MAX);
+    pub const INVALID: ArchetypeId = ArchetypeId(u32::MAX);
 
     #[inline]
     pub const fn new(index: usize) -> Self {
-        ArchetypeId(index)
+        ArchetypeId(index as u32)
     }
 
     #[inline]
     pub fn index(self) -> usize {
-        self.0
+        self.0 as usize
     }
 }
 
@@ -493,7 +493,7 @@ impl Archetypes {
             .archetype_ids
             .entry(archetype_identity)
             .or_insert_with(move || {
-                let id = ArchetypeId(archetypes.len());
+                let id = ArchetypeId::new(archetypes.len());
                 let table_start = *archetype_component_count;
                 *archetype_component_count += table_components.len();
                 let table_archetype_components =

From 0b9c1d3b365ee2d30e1fd2bdb5dc80a423ce7e38 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Fri, 18 Nov 2022 04:02:10 -0800
Subject: [PATCH 04/15] Use TableId from EntityLocation

---
 crates/bevy_ecs/src/query/iter.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs
index 9c2a1a80496c3..9d8cc5843a9b6 100644
--- a/crates/bevy_ecs/src/query/iter.rs
+++ b/crates/bevy_ecs/src/query/iter.rs
@@ -157,7 +157,7 @@ where
                 .archetypes
                 .get(location.archetype_id)
                 .debug_checked_unwrap();
-            let table = self.tables.get(archetype.table_id()).debug_checked_unwrap();
+            let table = self.tables.get(location.table_id).debug_checked_unwrap();
 
             // SAFETY: `archetype` is from the world that `fetch/filter` were created for,
             // `fetch_state`/`filter_state` are the states that `fetch/filter` were initialized with

From a9d4be9d7857180a8ff8cb7a87af27bb14ee8896 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Fri, 18 Nov 2022 14:37:36 -0800
Subject: [PATCH 05/15] Add docs for TableId.

---
 crates/bevy_ecs/src/storage/table.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs
index 3b6b498de4b4f..cdf97732cd38a 100644
--- a/crates/bevy_ecs/src/storage/table.rs
+++ b/crates/bevy_ecs/src/storage/table.rs
@@ -12,6 +12,9 @@ use std::{
     ops::{Index, IndexMut},
 };
 
+/// An opaque unique ID for a [`Table`] within a [`World`].
+/// 
+/// [`World`]: crate::world::World
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct TableId(u32);
 

From bf10fe85e74d431ebd6e5157ac258dcc22369708 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Fri, 18 Nov 2022 14:51:28 -0800
Subject: [PATCH 06/15] Type level docs for TableId

---
 crates/bevy_ecs/src/storage/table.rs | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs
index cdf97732cd38a..599a4c509ae0a 100644
--- a/crates/bevy_ecs/src/storage/table.rs
+++ b/crates/bevy_ecs/src/storage/table.rs
@@ -13,8 +13,18 @@ use std::{
 };
 
 /// An opaque unique ID for a [`Table`] within a [`World`].
-/// 
+///
+/// Can be used with [`Tables::get`] to fetch the corresponding
+/// table.
+///
+/// Each [`Archetype`] always points to a table via [`Archetype::table_id`]. 
+/// Multiple archetypes can point to the same table so long as the components 
+/// stored in the table are identical, but do not share the same sparse set 
+/// components.
+///
 /// [`World`]: crate::world::World
+/// [`Archetype`]: crate::archetype::Archetype
+/// [`Archetype::table_id`]: crate::archetype::Archetype::table_id
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct TableId(u32);
 

From 58753f7a722feb363b64a95361afad9be3b6df34 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Fri, 18 Nov 2022 15:09:33 -0800
Subject: [PATCH 07/15] Add safety docs for EntityMeta and EntityLocation.

---
 crates/bevy_ecs/src/entity/mod.rs    | 25 +++++++++++++++++++++++--
 crates/bevy_ecs/src/storage/table.rs |  6 +++---
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs
index 07a767d65cd79..ff75a603f90ae 100644
--- a/crates/bevy_ecs/src/entity/mod.rs
+++ b/crates/bevy_ecs/src/entity/mod.rs
@@ -665,12 +665,17 @@ impl Entities {
     }
 }
 
+// This type is repr(C) to ensure that the layout and values within it can be safe to fully fill
+// with u8::MAX, as required by [`Entities::flush_and_reserve_invalid_assuming_no_entities`].
 // Safety:
 // This type must not contain any pointers at any level, and be safe to fully fill with u8::MAX.
+/// Metadata for a given [`Entity`].
 #[derive(Copy, Clone, Debug)]
 #[repr(C)]
 pub struct EntityMeta {
+    /// The current generation of the [`Entity`].
     pub generation: u32,
+    /// The current location of the [`Entity`]
     pub location: EntityLocation,
 }
 
@@ -686,16 +691,32 @@ impl EntityMeta {
     };
 }
 
+// This type is repr(C) to ensure that the layout and values within it can be safe to fully fill
+// with u8::MAX, as required by [`Entities::flush_and_reserve_invalid_assuming_no_entities`].
+// SAFETY:
+// This type must not contain any pointers at any level, and be safe to fully fill with u8::MAX.
 /// A location of an entity in an archetype.
 #[derive(Copy, Clone, Debug)]
 #[repr(C)]
 pub struct EntityLocation {
-    /// The archetype index
+    /// The ID of the [`Archetype`] the [`Entity`] belongs to.
+    ///
+    /// [`Archetype`]: crate::archetype::Archetype
     pub archetype_id: ArchetypeId,
 
-    /// The index of the entity in the archetype
+    /// The index of the [`Entity`] within its [`Archetype`].
+    ///
+    /// [`Archetype`]: crate::archetype::Archetype
     pub archetype_index: u32,
+
+    /// The ID of the [`Table`] the [`Entity`] belongs to.
+    ///
+    /// [`Table`]: crate::storage::Table
     pub table_id: TableId,
+
+    /// The index of the [`Entity`] within its [`Table`].
+    ///
+    /// [`Table`]: crate::storage::Table
     pub table_row: u32,
 }
 
diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs
index 599a4c509ae0a..8c8c3c23d40ae 100644
--- a/crates/bevy_ecs/src/storage/table.rs
+++ b/crates/bevy_ecs/src/storage/table.rs
@@ -17,9 +17,9 @@ use std::{
 /// Can be used with [`Tables::get`] to fetch the corresponding
 /// table.
 ///
-/// Each [`Archetype`] always points to a table via [`Archetype::table_id`]. 
-/// Multiple archetypes can point to the same table so long as the components 
-/// stored in the table are identical, but do not share the same sparse set 
+/// Each [`Archetype`] always points to a table via [`Archetype::table_id`].
+/// Multiple archetypes can point to the same table so long as the components
+/// stored in the table are identical, but do not share the same sparse set
 /// components.
 ///
 /// [`World`]: crate::world::World

From 32873bbf1f57e82fca21a456833a0f0c25991794 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Fri, 18 Nov 2022 15:25:21 -0800
Subject: [PATCH 08/15] Update safety docs for EntityRef functions

---
 crates/bevy_ecs/src/world/entity_ref.rs | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs
index 070c7c4a1ec86..fd86a2d9e3892 100644
--- a/crates/bevy_ecs/src/world/entity_ref.rs
+++ b/crates/bevy_ecs/src/world/entity_ref.rs
@@ -593,8 +593,8 @@ impl<'w> EntityMut<'w> {
 /// Get a raw pointer to a particular [`Component`] on a particular [`Entity`] in the provided [`World`].
 ///
 /// # Safety
-/// - `entity_location` must be within bounds of the given archetype and `entity` must exist inside
-/// the archetype
+/// - location` must be within bounds of the given archetype and table and `entity` must exist inside
+/// the archetype and table
 /// - `component_id` must be valid
 #[inline]
 pub(crate) unsafe fn get_component(
@@ -624,7 +624,8 @@ pub(crate) unsafe fn get_component(
 /// Get a raw pointer to the [`ComponentTicks`] of a particular [`Component`] on a particular [`Entity`] in the provided [World].
 ///
 /// # Safety
-/// Caller must ensure that `component_id` is valid
+///  - Caller must ensure that `component_id` is valid
+///  - Caller must ensure that `location` is valid for `entity`.
 #[inline]
 unsafe fn get_component_and_ticks(
     world: &World,
@@ -682,7 +683,8 @@ unsafe fn get_ticks(
 /// Caller is responsible to drop component data behind returned pointer.
 ///
 /// # Safety
-/// - `entity_location` must be within bounds of the given archetype and `entity` must exist inside the archetype
+/// - `location` must be within bounds of the given archetype and table and `entity` must exist inside the archetype
+///   and table.
 /// - `component_id` must be valid
 /// - The relevant table row **must be removed** by the caller once all components are taken
 #[inline]

From 18b08b8393f2edb2d9b1f61aa9926a8d350a12d1 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Fri, 18 Nov 2022 15:30:54 -0800
Subject: [PATCH 09/15] Formatting

---
 crates/bevy_ecs/src/world/entity_ref.rs | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs
index fd86a2d9e3892..8b2fe3f71c3fa 100644
--- a/crates/bevy_ecs/src/world/entity_ref.rs
+++ b/crates/bevy_ecs/src/world/entity_ref.rs
@@ -101,6 +101,8 @@ impl<'w> EntityRef<'w> {
         last_change_tick: u32,
         change_tick: u32,
     ) -> Option<Mut<'w, T>> {
+        // SAFETY: entity location is valid and returned component is of type T. Caller guarentees that
+        // this reference will not alias.
         get_component_and_ticks_with_type(self.world, TypeId::of::<T>(), self.entity, self.location)
             .map(|(value, ticks)| Mut {
                 value: value.assert_unique().deref_mut::<T>(),
@@ -125,7 +127,7 @@ impl<'w> EntityRef<'w> {
     #[inline]
     pub fn get_by_id(&self, component_id: ComponentId) -> Option<Ptr<'w>> {
         self.world.components().get_info(component_id)?;
-        // SAFETY: entity_location is valid, component_id is valid as checked by the line above
+        // SAFETY: location is valid, component_id is valid as checked by the line above
         unsafe { get_component(self.world, component_id, self.entity, self.location) }
     }
 }
@@ -192,7 +194,8 @@ impl<'w> EntityMut<'w> {
 
     #[inline]
     pub fn get<T: Component>(&self) -> Option<&'_ T> {
-        // SAFETY: lifetimes enforce correct usage of returned borrow
+        // SAFETY: lifetimes enforce correct usage of returned borrow. entity location is valid and
+        // returned component is of type T.
         unsafe {
             get_component_with_type(self.world, TypeId::of::<T>(), self.entity, self.location)
                 .map(|value| value.deref::<T>())

From b25800e5652549f7b01a24a66a64620ec67e4b56 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Fri, 18 Nov 2022 16:08:02 -0800
Subject: [PATCH 10/15] Fix backtick

---
 crates/bevy_ecs/src/world/entity_ref.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs
index 8b2fe3f71c3fa..7dd740b58b7cb 100644
--- a/crates/bevy_ecs/src/world/entity_ref.rs
+++ b/crates/bevy_ecs/src/world/entity_ref.rs
@@ -596,8 +596,8 @@ impl<'w> EntityMut<'w> {
 /// Get a raw pointer to a particular [`Component`] on a particular [`Entity`] in the provided [`World`].
 ///
 /// # Safety
-/// - location` must be within bounds of the given archetype and table and `entity` must exist inside
-/// the archetype and table
+/// - `location` must be within bounds of the given archetype and table and `entity` must exist inside
+///    the archetype and table
 /// - `component_id` must be valid
 #[inline]
 pub(crate) unsafe fn get_component(

From ac7f1eb9400242a72d57b3fca6ca6f531a642cbb Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Sat, 3 Dec 2022 16:12:55 -0800
Subject: [PATCH 11/15] Fix docs

---
 crates/bevy_ecs/src/archetype.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs
index c7b4f33712b4c..d424b9d49ab6b 100644
--- a/crates/bevy_ecs/src/archetype.rs
+++ b/crates/bevy_ecs/src/archetype.rs
@@ -380,14 +380,14 @@ impl Archetype {
     /// Fetches the row in the [`Table`] where the components for the entity at `index`
     /// is stored.
     ///
-    /// An entity's archetype index can be fetched from [`EntityLocation::index`], which
+    /// An entity's archetype index can be fetched from [`EntityLocation::archetype_index`], which
     /// can be retrieved from [`Entities::get`].
     ///
     /// # Panics
     /// This function will panic if `index >= self.len()`.
     ///
     /// [`Table`]: crate::storage::Table
-    /// [`EntityLocation`]: crate::entity::EntityLocation::index
+    /// [`EntityLocation::archetype_index`]: crate::entity::EntityLocation::archetype_index
     /// [`Entities::get`]: crate::entity::Entities::get
     #[inline]
     pub fn entity_table_row(&self, index: usize) -> usize {

From 9b133d9616a53cecfc2f40b3be6e8267d0b5a10a Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Mon, 5 Dec 2022 16:20:23 -0800
Subject: [PATCH 12/15] Fix build

---
 crates/bevy_ecs/src/world/mod.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs
index a83eaeaae6507..b1b5d0d0a5961 100644
--- a/crates/bevy_ecs/src/world/mod.rs
+++ b/crates/bevy_ecs/src/world/mod.rs
@@ -329,10 +329,12 @@ impl World {
                 .entities()
                 .iter()
                 .enumerate()
-                .map(|(index, archetype_entity)| {
+                .map(|(archetype_index, archetype_entity)| {
                     let location = EntityLocation {
                         archetype_id: archetype.id(),
-                        index,
+                        archetype_index: archetype_index as u32,
+                        table_id: archetype.table_id(),
+                        table_row: archetype_entity.table_row() as u32,
                     };
                     EntityRef::new(self, archetype_entity.entity(), location)
                 })

From 416cdbbcfdfa81ea90b939501df10b2bb2d07be8 Mon Sep 17 00:00:00 2001
From: James Liu <contact@jamessliu.com>
Date: Thu, 15 Dec 2022 16:14:06 -0500
Subject: [PATCH 13/15] less verbose docs

Co-authored-by: JoJoJet <21144246+JoJoJet@users.noreply.github.com>
---
 crates/bevy_ecs/src/entity/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs
index 1ec5434b8d193..c7f74c5f8d052 100644
--- a/crates/bevy_ecs/src/entity/mod.rs
+++ b/crates/bevy_ecs/src/entity/mod.rs
@@ -720,7 +720,7 @@ impl Entities {
 // with u8::MAX, as required by [`Entities::flush_and_reserve_invalid_assuming_no_entities`].
 // Safety:
 // This type must not contain any pointers at any level, and be safe to fully fill with u8::MAX.
-/// Metadata for a given [`Entity`].
+/// Metadata for an [`Entity`].
 #[derive(Copy, Clone, Debug)]
 #[repr(C)]
 struct EntityMeta {

From d9c338972abfe5a0a51f25c4e9554705b4023e02 Mon Sep 17 00:00:00 2001
From: James Liu <contact@jamessliu.com>
Date: Fri, 16 Dec 2022 06:58:40 -0500
Subject: [PATCH 14/15] Move safety comment

---
 crates/bevy_ecs/src/world/entity_ref.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs
index 351a46db5bc9c..b6434d47f37db 100644
--- a/crates/bevy_ecs/src/world/entity_ref.rs
+++ b/crates/bevy_ecs/src/world/entity_ref.rs
@@ -149,7 +149,6 @@ impl<'w> EntityRef<'w> {
         // - entity location and entity is valid
         // - returned component is of type T
         // - the storage type provided is correct for T
-        // - Caller guarentees that this reference will not alias.
         get_component_and_ticks_with_type(
             self.world,
             TypeId::of::<T>(),
@@ -158,6 +157,9 @@ impl<'w> EntityRef<'w> {
             self.location,
         )
         .map(|(value, ticks)| Mut {
+            // SAFETY:
+            // - returned component is of type T
+            // - Caller guarentees that this reference will not alias.
             value: value.assert_unique().deref_mut::<T>(),
             ticks: Ticks::from_tick_cells(ticks, last_change_tick, change_tick),
         })

From c603483f9d859212cc807d8d28fa228fe56085a3 Mon Sep 17 00:00:00 2001
From: James Liu <contact@jamessliu.com>
Date: Fri, 16 Dec 2022 07:04:27 -0500
Subject: [PATCH 15/15] Simplify fetch_table

---
 crates/bevy_ecs/src/world/entity_ref.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs
index b6434d47f37db..251945d711b09 100644
--- a/crates/bevy_ecs/src/world/entity_ref.rs
+++ b/crates/bevy_ecs/src/world/entity_ref.rs
@@ -703,9 +703,7 @@ fn fetch_table(
     location: EntityLocation,
     component_id: ComponentId,
 ) -> Option<&Column> {
-    let table = &world.storages.tables[location.table_id];
-    let components = table.get_column(component_id)?;
-    Some(components)
+    world.storages.tables[location.table_id].get_column(component_id)
 }
 
 #[inline]