From 3eb051b19be6b03f505cdd12f91bef3d52f049eb Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 20 Feb 2023 16:50:41 -0800 Subject: [PATCH 01/12] Document bevy_ecs::storage --- crates/bevy_ecs/src/storage/blob_vec.rs | 33 ++++- crates/bevy_ecs/src/storage/mod.rs | 24 +++ crates/bevy_ecs/src/storage/resource.rs | 6 + crates/bevy_ecs/src/storage/sparse_set.rs | 74 +++++++++ crates/bevy_ecs/src/storage/table.rs | 173 ++++++++++++++++++++-- 5 files changed, 298 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 49b370b7b8e3a..734482dc96b61 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -35,6 +35,8 @@ impl std::fmt::Debug for BlobVec { } impl BlobVec { + /// Creates a new [`BlobVec`] with the specified `capacity`. + /// /// # Safety /// /// `drop` should be safe to call with an [`OwningPtr`] pointing to any item that's been pushed into this [`BlobVec`]. @@ -70,26 +72,37 @@ impl BlobVec { } } + /// Gets the number of elements currently stored in the [`BlobVec`]. #[inline] pub fn len(&self) -> usize { self.len } + /// Checks if the [`BlobVec`] is currently empty. #[inline] pub fn is_empty(&self) -> bool { self.len == 0 } + /// Gets the maximum possible number of elements the [`BlobVec`] can store without + /// reallocating it's underlying buffer. #[inline] pub fn capacity(&self) -> usize { self.capacity } + /// Fetches the [`Layout`] of the underlying type stored within the [`BlobVec`]. #[inline] pub fn layout(&self) -> Layout { self.item_layout } + /// Reserves the minimum capacity for at least additional more elements to be inserted in the given `BlobVec`. + /// After calling `reserve_exact`, capacity will be greater than or equal to self.len() + additional. Does nothing if + /// the capacity is already sufficient. + /// + /// Note that the allocator may give the collection more space than it requests. Therefore, capacity can not be relied upon + /// to be precisely minimal. pub fn reserve_exact(&mut self, additional: usize) { let available_space = self.capacity - self.len; if available_space < additional && self.item_layout.size() > 0 { @@ -134,6 +147,8 @@ impl BlobVec { self.capacity = new_capacity; } + /// Initializes the value at `index` to `value`. This function does not do any bounds checking. + /// /// # Safety /// - index must be in bounds /// - the memory in the [`BlobVec`] starting at index `index`, of a size matching this [`BlobVec`]'s @@ -145,6 +160,8 @@ impl BlobVec { std::ptr::copy_nonoverlapping::(value.as_ptr(), ptr.as_ptr(), self.item_layout.size()); } + /// Replaces the value at `index` with `value`. This function does not do any bounds checking. + /// /// # Safety /// - index must be in-bounds /// - the memory in the [`BlobVec`] starting at index `index`, of a size matching this @@ -201,7 +218,7 @@ impl BlobVec { std::ptr::copy_nonoverlapping::(source, destination.as_ptr(), self.item_layout.size()); } - /// Pushes a value to the [`BlobVec`]. + /// Appends a value to the back of the [`BlobVec`]. /// /// # Safety /// `value` must be valid to add to this [`BlobVec`] @@ -213,6 +230,8 @@ impl BlobVec { self.initialize_unchecked(index, value); } + /// Forces the length of the vector to `len`. + /// /// # Safety /// `len` must be <= `capacity`. if length is decreased, "out of bounds" items must be dropped. /// Newly added items must be immediately populated with valid values and length must be @@ -255,6 +274,7 @@ impl BlobVec { /// Removes the value at `index` and copies the value stored into `ptr`. /// Does not do any bounds checking on `index`. + /// The removed element is replaced by the last element of the `BlobVec`. /// /// # Safety /// It is the caller's responsibility to ensure that `index` is < `self.len()` @@ -274,6 +294,10 @@ impl BlobVec { self.len -= 1; } + /// Removes the value at `index` and drops it. + /// Does not do any bounds checking on `index`. + /// The removed element is replaced by the last element of the `BlobVec`. + /// /// # Safety /// It is the caller's responsibility to ensure that `index` is < self.len() #[inline] @@ -286,6 +310,9 @@ impl BlobVec { } } + /// Fetches a read-only reference to the value at `index`. + /// Does not do any bounds checking on `index`. + /// /// # Safety /// It is the caller's responsibility to ensure that `index` is < self.len() #[inline] @@ -300,6 +327,9 @@ impl BlobVec { self.get_ptr().byte_add(index * size) } + /// Fetches a mutable reference to the value at `index`. + /// Does not do any bounds checking on `index`. + /// /// # Safety /// It is the caller's responsibility to ensure that `index` is < self.len() #[inline] @@ -337,6 +367,7 @@ impl BlobVec { std::slice::from_raw_parts(self.data.as_ptr() as *const UnsafeCell, self.len) } + /// Clears the [`BlobVec`] by removing and dropping all of it's elements. pub fn clear(&mut self) { let len = self.len; // We set len to 0 _before_ dropping elements for unwind safety. This ensures we don't diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index b2fab3fdcb590..bbc4f2662ae7d 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -1,4 +1,24 @@ //! Storage layouts for ECS data. +//! +//! This module contains the low level backing data stores for ECS. These all offer minimal and often +//! unsafe APIs, exposed primarily for debugging and monitoring purposes. +//! +//! # Fetching Storages +//! Each of the primary backing data stores can be fetched via [`Storages`], which can be fetched from a +//! [`World`] via [`World::storages`]. It exposes a top level container for each class of storage within +//! ECS: +//! +//! - [`Tables`] - columnar contiguous blocks of memory, optimized for fast iteration. +//! - [`SparseSets`] - sparse `HashMap`-like mappings from entities to components, optimized for random +//! lookup and regular insertion/removal of components. +//! - [`Resources`] - singleton storage for the resources in the world +//! +//! # Safety +//! To avoid trivially unsound use of the APIs in this module, it is explicitly impoosible to get a mutable +//! reference to [`Storages`] from [`World`], and none of the types publicly expose a mutable interface. +//! +//! [`World`]: crate::world::World +//! [`World::storages`]: crate::world::World::storages mod blob_vec; mod resource; @@ -12,8 +32,12 @@ pub use table::*; /// The raw data stores of a [World](crate::world::World) #[derive(Default)] pub struct Storages { + /// Backing storage for [`SparseSet`] components. pub sparse_sets: SparseSets, + /// Backing storage for [`Table`] components. pub tables: Tables, + /// Backing storage for resources. pub resources: Resources, + /// Backing storage for `!Send` resources. pub non_send_resources: Resources, } diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index f1a0e9fb53552..10e44533fc128 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -42,6 +42,10 @@ impl ResourceData { /// The only row in the underlying column. const ROW: TableRow = TableRow::new(0); + /// Validates the access to `!Send` resources is only done on the thread they were created from. + /// + /// # Panics + /// If `SEND` is false, this will panic if called from a different thread than the one it was inserted from. #[inline] fn validate_access(&self) { if SEND { @@ -89,6 +93,8 @@ impl ResourceData { self.column.get_ticks(Self::ROW) } + /// Gets a read-only reference and the change detection ticks for the resource. + /// /// # Panics /// If `SEND` is false, this will panic if a value is present and is not accessed from the /// original thread it was inserted in. diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 54412b54c25dd..5f0b724475ab2 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -41,12 +41,18 @@ impl SparseArray { macro_rules! impl_sparse_array { ($ty:ident) => { impl $ty { + /// Checks if a value is present at `index`. + /// + /// Returns `true` if there's a value present. #[inline] pub fn contains(&self, index: I) -> bool { let index = index.sparse_set_index(); self.values.get(index).map(|v| v.is_some()).unwrap_or(false) } + /// Fetches a read-only reference to the value at `index`. + /// + /// Returns `None` if it's not populated or if `index` is out of bounds. #[inline] pub fn get(&self, index: I) -> Option<&V> { let index = index.sparse_set_index(); @@ -60,6 +66,9 @@ impl_sparse_array!(SparseArray); impl_sparse_array!(ImmutableSparseArray); impl SparseArray { + /// Inserts `value` at `index` in the array. + /// + /// If `index` is out-of-bounds, this will grow the buffer until it can accomodate it. #[inline] pub fn insert(&mut self, index: I, value: V) { let index = index.sparse_set_index(); @@ -69,6 +78,9 @@ impl SparseArray { self.values[index] = Some(value); } + /// Fetches a mutable reference to the value at `index`. + /// + /// Returns `None` if it's not populated or if `index` is out of bounds. #[inline] pub fn get_mut(&mut self, index: I) -> Option<&mut V> { let index = index.sparse_set_index(); @@ -78,16 +90,21 @@ impl SparseArray { .unwrap_or(None) } + /// Removes the value stored at `index`, if present. + /// + /// Returns `None` if it's not populated or if `index` is out of bounds. #[inline] pub fn remove(&mut self, index: I) -> Option { let index = index.sparse_set_index(); self.values.get_mut(index).and_then(|value| value.take()) } + /// Removes all of the values stored within. pub fn clear(&mut self) { self.values.clear(); } + /// Converts the [`SparseArray`] into an immutable variant. pub(crate) fn into_immutable(self) -> ImmutableSparseArray { ImmutableSparseArray { values: self.values.into_boxed_slice(), @@ -113,6 +130,8 @@ pub struct ComponentSparseSet { } impl ComponentSparseSet { + /// Creates a new [`ComponentSparseSet`] with a given component type layout and + /// initial `capacity`. pub(crate) fn new(component_info: &ComponentInfo, capacity: usize) -> Self { Self { dense: Column::with_capacity(component_info, capacity), @@ -121,17 +140,20 @@ impl ComponentSparseSet { } } + /// Removes all of the values stored within. pub(crate) fn clear(&mut self) { self.dense.clear(); self.entities.clear(); self.sparse.clear(); } + /// Gets the number of components currently stored in the sparse set. #[inline] pub fn len(&self) -> usize { self.dense.len() } + /// Checks if the sparse set is empty. Returns `true` if it contains no elements, false otherwise. #[inline] pub fn is_empty(&self) -> bool { self.dense.len() == 0 @@ -162,6 +184,7 @@ impl ComponentSparseSet { } } + /// Checks if the [`ComponentSparseSet`] contains a component for `entity`. #[inline] pub fn contains(&self, entity: Entity) -> bool { #[cfg(debug_assertions)] @@ -178,6 +201,9 @@ impl ComponentSparseSet { self.sparse.contains(entity.index()) } + /// Fetches a read-only reference to the component for `entity`, if present. + /// + /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] pub fn get(&self, entity: Entity) -> Option> { self.sparse.get(entity.index()).map(|dense_index| { @@ -189,6 +215,10 @@ impl ComponentSparseSet { }) } + /// Fetches a read-only reference to the component for `entity` and it's change detection ticks, + /// if present. + /// + /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] pub fn get_with_ticks(&self, entity: Entity) -> Option<(Ptr<'_>, TickCells<'_>)> { let dense_index = TableRow::new(*self.sparse.get(entity.index())? as usize); @@ -206,6 +236,9 @@ impl ComponentSparseSet { } } + /// Fetches a reference to `entity`'s "added" change detection ticks, if present. + /// + /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] pub fn get_added_ticks(&self, entity: Entity) -> Option<&UnsafeCell> { let dense_index = *self.sparse.get(entity.index())? as usize; @@ -220,6 +253,9 @@ impl ComponentSparseSet { } } + /// Fetches a reference to `entity`'s "changed" change detection ticks, if present. + /// + /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] pub fn get_changed_ticks(&self, entity: Entity) -> Option<&UnsafeCell> { let dense_index = *self.sparse.get(entity.index())? as usize; @@ -234,6 +270,9 @@ impl ComponentSparseSet { } } + /// Fetches `entity`'s change detection ticks, if present. + /// + /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] pub fn get_ticks(&self, entity: Entity) -> Option { let dense_index = *self.sparse.get(entity.index())? as usize; @@ -270,6 +309,11 @@ impl ComponentSparseSet { }) } + /// Removes `entity`'s component from the sparse set, if present. + /// + /// If `entity` has a component stored in the sparse set, it will be dropped. + /// + /// Returns `true` if `entity` had a component in the sparse set. pub(crate) fn remove(&mut self, entity: Entity) -> bool { if let Some(dense_index) = self.sparse.remove(entity.index()) { let dense_index = dense_index as usize; @@ -320,16 +364,21 @@ pub(crate) struct ImmutableSparseSet { macro_rules! impl_sparse_set { ($ty:ident) => { impl $ty { + /// Gets the number of elements currently stored in the sparse set. #[inline] pub fn len(&self) -> usize { self.dense.len() } + /// Checks if there's a value stored at `index`. #[inline] pub fn contains(&self, index: I) -> bool { self.sparse.contains(index) } + /// Fetches a read-only reference to the value at `index`, if present. + /// + /// Returns `None` if `index` is not present in the sparse set. pub fn get(&self, index: I) -> Option<&V> { self.sparse.get(index).map(|dense_index| { // SAFETY: if the sparse index points to something in the dense vec, it exists @@ -337,6 +386,9 @@ macro_rules! impl_sparse_set { }) } + /// Fetches a mutable reference to the value at `index`, if present. + /// + /// Returns `None` if `index` is not present in the sparse set. pub fn get_mut(&mut self, index: I) -> Option<&mut V> { let dense = &mut self.dense; self.sparse.get(index).map(move |dense_index| { @@ -345,22 +397,27 @@ macro_rules! impl_sparse_set { }) } + /// Iterates over the indicies (keys) of the sparse set. pub fn indices(&self) -> impl Iterator + '_ { self.indices.iter().cloned() } + /// Iterates over read-only references to the values of the sparse set. pub fn values(&self) -> impl Iterator { self.dense.iter() } + /// Iterates over mutable references to the values of the sparse set. pub fn values_mut(&mut self) -> impl Iterator { self.dense.iter_mut() } + /// Iterates over key-value pairs the sparse set. pub fn iter(&self) -> impl Iterator { self.indices.iter().zip(self.dense.iter()) } + /// Iterates over key-value pairs of the sparse set. pub fn iter_mut(&mut self) -> impl Iterator { self.indices.iter().zip(self.dense.iter_mut()) } @@ -378,6 +435,7 @@ impl Default for SparseSet { } impl SparseSet { + /// Creates a new [`SparseSet`]. pub const fn new() -> Self { Self { dense: Vec::new(), @@ -388,6 +446,7 @@ impl SparseSet { } impl SparseSet { + /// Creates a new [`SparseSet`] with a specified initial capacity. pub fn with_capacity(capacity: usize) -> Self { Self { dense: Vec::with_capacity(capacity), @@ -396,11 +455,16 @@ impl SparseSet { } } + /// The total number of elements the [`SparseSet`] can hold without needing to + /// allocate a larger buffer. #[inline] pub fn capacity(&self) -> usize { self.dense.capacity() } + /// Inserts `value` at `index`. + /// + /// If a value was already present at `index`, it will overwrite it. pub fn insert(&mut self, index: I, value: V) { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { // SAFETY: dense indices stored in self.sparse always exist @@ -414,6 +478,8 @@ impl SparseSet { } } + /// Attempts to fetch the value at `index`. If not present, `func` will be called to + /// initialize a value. pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut V { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { // SAFETY: dense indices stored in self.sparse always exist @@ -429,11 +495,17 @@ impl SparseSet { } } + /// Checks if the [`SparseSet`] is currently empty or not. + /// + /// Returns `true` if it currently contains zero elements, `false` otherwise. #[inline] pub fn is_empty(&self) -> bool { self.dense.len() == 0 } + /// Removes and returns the value stored at `index`, if present. + /// + /// Returns `None` the value at `index` is not populated. pub fn remove(&mut self, index: I) -> Option { self.sparse.remove(index).map(|dense_index| { let is_last = dense_index == self.dense.len() - 1; @@ -447,12 +519,14 @@ impl SparseSet { }) } + /// Clears all of the elements from the sparse set. pub fn clear(&mut self) { self.dense.clear(); self.indices.clear(); self.sparse.clear(); } + /// Converts the sparse set into it's immutable variant. pub(crate) fn into_immutable(self) -> ImmutableSparseSet { ImmutableSparseSet { dense: self.dense.into_boxed_slice(), diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index a8077be288631..2a20155dff1f3 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -33,16 +33,20 @@ pub struct TableId(u32); impl TableId { pub(crate) const INVALID: TableId = TableId(u32::MAX); + /// Creates a new [`TableId`]. `index` *must* be retrieved from [`TableId::index`] + /// from a table of a given [`World`] or the created ID may be invalid. #[inline] pub fn new(index: usize) -> Self { TableId(index as u32) } + /// Gets the underlying table index from the ID. #[inline] pub fn index(self) -> usize { self.0 as usize } + /// The [`TableId`] of the [`Table`] without any components. #[inline] pub const fn empty() -> TableId { TableId(0) @@ -71,7 +75,7 @@ impl TableId { pub struct TableRow(u32); impl TableRow { - pub const INVALID: TableRow = TableRow(u32::MAX); + pub(crate) const INVALID: TableRow = TableRow(u32::MAX); /// Creates a `TableRow`. #[inline] @@ -86,6 +90,19 @@ impl TableRow { } } +/// A type-erased contiguous container for data of a homogenous type. +/// +/// Conceptually, a [`Column`] is very similar to a `Vec` with it's generic type erased. +/// It also store the change detection ticks for the stored components, kept in two separate +/// contiugous buffers internally. An element shares it's data across these buffers by using the +/// saame index (i.e. the entity at row 3 has it's data at index 3 and it's change detection ticks at +/// index 3). A slice to these contiguous blocks of memory can be fetched +/// via [`Column::get_data_slice`], [`Column::get_added_ticks_slice`], and +/// [`Column::get_changed_ticks_slice`]. +/// +/// Like many other low-level storage types, [`Column`] has a limited and highly unsafe +/// interface. It's highly advised to use higher level types and their safe abstractions +/// instead of working directly with [`Column`]. #[derive(Debug)] pub struct Column { data: BlobVec, @@ -94,6 +111,7 @@ pub struct Column { } impl Column { + /// Constructs a new [`Column`], configured with a component's layout and a initial `capacity`. #[inline] pub(crate) fn with_capacity(component_info: &ComponentInfo, capacity: usize) -> Self { Column { @@ -104,6 +122,7 @@ impl Column { } } + /// Fetches the [`Layout`] for the underlying type. #[inline] pub fn item_layout(&self) -> Layout { self.data.layout() @@ -150,18 +169,29 @@ impl Column { self.data.replace_unchecked(row.index(), data); } + /// Gets the current number of elements stored in the column. #[inline] pub fn len(&self) -> usize { self.data.len() } + /// Checks if the column is empty. Returns `true` if there are no elements, false otherwise. #[inline] pub fn is_empty(&self) -> bool { self.data.is_empty() } + /// Removes an element from the [`Column`]. + /// + /// - The value will be dropped if it implements [`Drop`]. + /// - This does not preserve ordering, but is O(1). + /// - This does do any bounds checking. + /// - The element is replaced last element in the [`Column`]. + /// /// # Safety - /// index must be in-bounds + /// `row` must be within the range `[0, self.len())`. + /// + /// [`Drop`]: std::ops::Drop #[inline] pub(crate) unsafe fn swap_remove_unchecked(&mut self, row: TableRow) { self.data.swap_remove_and_drop_unchecked(row.index()); @@ -169,6 +199,14 @@ impl Column { self.changed_ticks.swap_remove(row.index()); } + /// Removes an element from the [`Column`] and returns it and it's change detection ticks. + /// This does not preserve ordering, but is O(1). + /// + /// The element is replaced last element in the [`Column`]. + /// + /// It's the caller's responsibility to ensure that the removed value is dropped or used. + /// + /// Returns `None` if `row` is out of bounds. #[inline] #[must_use = "The returned pointer should be used to drop the removed component"] pub(crate) fn swap_remove_and_forget( @@ -184,8 +222,16 @@ impl Column { }) } + /// Removes an element from the [`Column`] and returns it and it's change detection ticks. + /// This does not preserve ordering, but is O(1). Unlike [`Column::swap_remove_and_forget`] + /// this does do any bounds checking. + /// + /// The element is replaced last element in the [`Column`]. + /// + /// It's the caller's responsibility to ensure that the removed value is dropped or used. + /// /// # Safety - /// index must be in-bounds + /// `row` must be within the range `[0, self.len())`. #[inline] #[must_use = "The returned pointer should be used to dropped the removed component"] pub(crate) unsafe fn swap_remove_and_forget_unchecked( @@ -225,8 +271,10 @@ impl Column { other.changed_ticks.swap_remove(src_row.index()); } - // # Safety - // - ptr must point to valid data of this column's component type + /// Pushes a new value onto the end of the [`Column`]. + /// + /// # Safety + /// `ptr` must point to valid data of this column's component type pub(crate) unsafe fn push(&mut self, ptr: OwningPtr<'_>, ticks: ComponentTicks) { self.data.push(ptr); self.added_ticks.push(UnsafeCell::new(ticks.added)); @@ -240,27 +288,55 @@ impl Column { self.changed_ticks.reserve_exact(additional); } + /// Fetches the data pointer to the first element of the [`Column`]. + /// + /// The pointer is type erased, so using this function to fetch #[inline] pub fn get_data_ptr(&self) -> Ptr<'_> { self.data.get_ptr() } + /// Fetches the slice to the [`Column`]'s data cast to a given type. + /// + /// Note: The values stored within are [`UnsafeCell`]. + /// Users of this API must ensure that accesses to each individual element + /// adhere to the safety invariants of [`UnsafeCell`]. + /// /// # Safety /// The type `T` must be the type of the items in this column. + /// + /// [`UnsafeCell`]: std::cell::UnsafeCell pub unsafe fn get_data_slice(&self) -> &[UnsafeCell] { self.data.get_slice() } + /// Fetches the slice to the [`Column`]'s "added" change detection ticks. + /// + /// Note: The values stored within are [`UnsafeCell`]. + /// Users of this API must ensure that accesses to each individual element + /// adhere to the safety invariants of [`UnsafeCell`]. + /// + /// [`UnsafeCell`]: std::cell::UnsafeCell #[inline] pub fn get_added_ticks_slice(&self) -> &[UnsafeCell] { &self.added_ticks } + /// Fetches the slice to the [`Column`]'s "changed" change detection ticks. + /// + /// Note: The values stored within are [`UnsafeCell`]. + /// Users of this API must ensure that accesses to each individual element + /// adhere to the safety invariants of [`UnsafeCell`]. + /// + /// [`UnsafeCell`]: std::cell::UnsafeCell #[inline] pub fn get_changed_ticks_slice(&self) -> &[UnsafeCell] { &self.changed_ticks } + /// Fetches a reference to the data and change detection ticks at `row`. + /// + /// Returns `None` if `row` is out of bounds. #[inline] pub fn get(&self, row: TableRow) -> Option<(Ptr<'_>, TickCells<'_>)> { (row.index() < self.data.len()) @@ -277,6 +353,9 @@ impl Column { }) } + /// Fetches a read-only reference to the data at `row`. + /// + /// Returns `None` if `row` is out of bounds. #[inline] pub fn get_data(&self, row: TableRow) -> Option> { // SAFETY: The row is length checked before fetching the pointer. This is being @@ -284,15 +363,21 @@ impl Column { (row.index() < self.data.len()).then(|| unsafe { self.data.get_unchecked(row.index()) }) } + /// Fetches a read-only reference to the data at `row`. Unlike [`Column::get`] this does not + /// do any bounds checking. + /// /// # Safety - /// - index must be in-bounds - /// - no other reference to the data of the same row can exist at the same time + /// - `row` must be within the range `[0, self.len())`. + /// - no other mutable reference to the data of the same row can exist at the same time #[inline] pub unsafe fn get_data_unchecked(&self, row: TableRow) -> Ptr<'_> { debug_assert!(row.index() < self.data.len()); self.data.get_unchecked(row.index()) } + /// Fetches a mutable reference to the data at `row`. + /// + /// Returns `None` if `row` is out of bounds. #[inline] pub fn get_data_mut(&mut self, row: TableRow) -> Option> { // SAFETY: The row is length checked before fetching the pointer. This is being @@ -300,6 +385,9 @@ impl Column { (row.index() < self.data.len()).then(|| unsafe { self.data.get_unchecked_mut(row.index()) }) } + /// Fetches a mutable reference to the data at `row`. Unlike [`Column::get_mut`] this does not + /// do any bounds checking. + /// /// # Safety /// - index must be in-bounds /// - no other reference to the data of the same row can exist at the same time @@ -309,16 +397,37 @@ impl Column { self.data.get_unchecked_mut(row.index()) } + /// Fetches the "added" change detection ticks for the value at `row`. + /// + /// Returns `None` if `row` is out of bounds. + /// + /// Note: The values stored within are [`UnsafeCell`]. + /// Users of this API must ensure that accesses to each individual element + /// adhere to the safety invariants of [`UnsafeCell`]. + /// + /// [`UnsafeCell`]: std::cell::UnsafeCell #[inline] pub fn get_added_ticks(&self, row: TableRow) -> Option<&UnsafeCell> { self.added_ticks.get(row.index()) } + /// Fetches the "changed" change detection ticks for the value at `row`. + /// + /// Returns `None` if `row` is out of bounds. + /// + /// Note: The values stored within are [`UnsafeCell`]. + /// Users of this API must ensure that accesses to each individual element + /// adhere to the safety invariants of [`UnsafeCell`]. + /// + /// [`UnsafeCell`]: std::cell::UnsafeCell #[inline] pub fn get_changed_ticks(&self, row: TableRow) -> Option<&UnsafeCell> { self.changed_ticks.get(row.index()) } + /// Fetches the change detection ticks for the value at `row`. + /// + /// Returns `None` if `row` is out of bounds. #[inline] pub fn get_ticks(&self, row: TableRow) -> Option { if row.index() < self.data.len() { @@ -329,24 +438,33 @@ impl Column { } } + /// Fetches the "added" change detection ticks for the value at `row`. Unlike [`Column::get_added_ticks`] + /// this function does not do any bounds checking. + /// /// # Safety - /// index must be in-bounds + /// `row` must be within the range `[0, self.len())`. #[inline] pub unsafe fn get_added_ticks_unchecked(&self, row: TableRow) -> &UnsafeCell { debug_assert!(row.index() < self.added_ticks.len()); self.added_ticks.get_unchecked(row.index()) } + /// Fetches the "changed" change detection ticks for the value at `row`. Unlike [`Column::get_changed_ticks`] + /// this function does not do any bounds checking. + /// /// # Safety - /// index must be in-bounds + /// `row` must be within the range `[0, self.len())`. #[inline] pub unsafe fn get_changed_ticks_unchecked(&self, row: TableRow) -> &UnsafeCell { debug_assert!(row.index() < self.changed_ticks.len()); self.changed_ticks.get_unchecked(row.index()) } + /// Fetches the change detection ticks for the value at `row`. Unlike [`Column::get_ticks`] + /// this function does not do any bounds checking. + /// /// # Safety - /// index must be in-bounds + /// `row` must be within the range `[0, self.len())`. #[inline] pub unsafe fn get_ticks_unchecked(&self, row: TableRow) -> ComponentTicks { debug_assert!(row.index() < self.added_ticks.len()); @@ -357,6 +475,9 @@ impl Column { } } + /// Clears the column, removing all values. + /// + /// Note that this function has no effect on the allocated capacity of the [`Column`]> pub fn clear(&mut self) { self.data.clear(); self.added_ticks.clear(); @@ -417,7 +538,7 @@ impl TableBuilder { /// in a [`World`]. /// /// Conceptually, a `Table` can be thought of as an `HashMap`, where -/// each `Column` is a type-erased `Vec`. Each row corresponds to a single entity +/// each [`Column`] is a type-erased `Vec`. Each row corresponds to a single entity /// (i.e. index 3 in Column A and index 3 in Column B point to different components on the same /// entity). Fetching components from a table involves fetching the associated column for a /// component type (via it's [`ComponentId`]), then fetching the entity's row within that column. @@ -431,6 +552,7 @@ pub struct Table { } impl Table { + /// Fetches a read-only slice of the entiities stored within the [`Table`]. #[inline] pub fn entities(&self) -> &[Entity] { &self.entities @@ -548,21 +670,33 @@ impl Table { } } + /// Fetches a read-only reference to the [`Column`] for a given [`Component`] within the + /// table. + /// + /// Returns `None` if the corresponding component does not belong to the table. #[inline] pub fn get_column(&self, component_id: ComponentId) -> Option<&Column> { self.columns.get(component_id) } + /// Fetches a mutable reference to the [`Column`] for a given [`Component`] within the + /// table. + /// + /// Returns `None` if the corresponding component does not belong to the table. #[inline] pub(crate) fn get_column_mut(&mut self, component_id: ComponentId) -> Option<&mut Column> { self.columns.get_mut(component_id) } + /// Checks if the table contains a[`Column`] for a given [`Component`]. + /// + /// Returns `true` if the column is present, `false` otherwise. #[inline] pub fn has_column(&self, component_id: ComponentId) -> bool { self.columns.contains(component_id) } + /// Reserves `additional` elements worth of capacity within the table. pub(crate) fn reserve(&mut self, additional: usize) { if self.entities.capacity() - self.entities.len() < additional { self.entities.reserve(additional); @@ -592,21 +726,27 @@ impl Table { TableRow::new(index) } + /// Gets the number of entities currently being stored in the table. #[inline] pub fn entity_count(&self) -> usize { self.entities.len() } + /// Gets the number of components being stored in the table. #[inline] pub fn component_count(&self) -> usize { self.columns.len() } + /// Gets the number of components the table can currently store. #[inline] pub fn entity_capacity(&self) -> usize { self.entities.capacity() } + /// Checks if the [`Table`] is empty or not. + /// + /// Returns `true` if the table contains no entities, `false` otherwise. #[inline] pub fn is_empty(&self) -> bool { self.entities.is_empty() @@ -618,10 +758,12 @@ impl Table { } } + /// Iterates over the [`Column`]s of the [`Table`]. pub fn iter(&self) -> impl Iterator { self.columns.values() } + /// Clears all of the stored components in the [`Table`]. pub(crate) fn clear(&mut self) { self.entities.clear(); for column in self.columns.values_mut() { @@ -666,11 +808,15 @@ impl Tables { self.tables.is_empty() } + /// Fetches a [`Table`] by it's [`TableId`]. + /// + /// Returns `None` if `id` is invalid. #[inline] pub fn get(&self, id: TableId) -> Option<&Table> { self.tables.get(id.index()) } + /// Fetches mutable references to two different [`Table`]s. #[inline] pub(crate) fn get_2_mut(&mut self, a: TableId, b: TableId) -> (&mut Table, &mut Table) { if a.index() > b.index() { @@ -682,6 +828,9 @@ impl Tables { } } + /// Attempts to fetch a table based on the provided components. If none is found, + /// a new [`Table`] is created and returned. + /// /// # Safety /// `component_ids` must contain components that exist in `components` pub(crate) unsafe fn get_id_or_insert( @@ -706,10 +855,12 @@ impl Tables { *value } + /// Iterates through all of the tables stored within in [`TableId`] order. pub fn iter(&self) -> std::slice::Iter<'_, Table> { self.tables.iter() } + /// Clears all data from all [`Table`]s stored within. pub(crate) fn clear(&mut self) { for table in &mut self.tables { table.clear(); From 803f05269af9f8177786f352957a7a8cee27a301 Mon Sep 17 00:00:00 2001 From: James Liu Date: Mon, 20 Feb 2023 20:54:36 -0800 Subject: [PATCH 02/12] Apply suggestions from code review Co-authored-by: Alice Cecile Co-authored-by: Rob Parrett Co-authored-by: Carter Weinberg --- crates/bevy_ecs/src/storage/blob_vec.rs | 8 ++--- crates/bevy_ecs/src/storage/mod.rs | 4 +-- crates/bevy_ecs/src/storage/sparse_set.rs | 14 ++++----- crates/bevy_ecs/src/storage/table.rs | 38 +++++++++++++---------- 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 734482dc96b61..e0f4d5e293668 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -85,7 +85,7 @@ impl BlobVec { } /// Gets the maximum possible number of elements the [`BlobVec`] can store without - /// reallocating it's underlying buffer. + /// reallocating its underlying buffer. #[inline] pub fn capacity(&self) -> usize { self.capacity @@ -97,8 +97,8 @@ impl BlobVec { self.item_layout } - /// Reserves the minimum capacity for at least additional more elements to be inserted in the given `BlobVec`. - /// After calling `reserve_exact`, capacity will be greater than or equal to self.len() + additional. Does nothing if + /// Reserves the minimum capacity for at least `additional` more elements to be inserted in the given `BlobVec`. + /// After calling `reserve_exact`, capacity will be greater than or equal to `self.len() + additional`. Does nothing if /// the capacity is already sufficient. /// /// Note that the allocator may give the collection more space than it requests. Therefore, capacity can not be relied upon @@ -367,7 +367,7 @@ impl BlobVec { std::slice::from_raw_parts(self.data.as_ptr() as *const UnsafeCell, self.len) } - /// Clears the [`BlobVec`] by removing and dropping all of it's elements. + /// Clears the [`BlobVec`] by removing and dropping all of its elements. pub fn clear(&mut self) { let len = self.len; // We set len to 0 _before_ dropping elements for unwind safety. This ensures we don't diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index bbc4f2662ae7d..ee61d13b8ed93 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -1,6 +1,6 @@ //! Storage layouts for ECS data. //! -//! This module contains the low level backing data stores for ECS. These all offer minimal and often +//! This module contains the low-level backing data stores for ECS. These all offer minimal and often //! unsafe APIs, exposed primarily for debugging and monitoring purposes. //! //! # Fetching Storages @@ -14,7 +14,7 @@ //! - [`Resources`] - singleton storage for the resources in the world //! //! # Safety -//! To avoid trivially unsound use of the APIs in this module, it is explicitly impoosible to get a mutable +//! To avoid trivially unsound use of the APIs in this module, it is explicitly impossible to get a mutable //! reference to [`Storages`] from [`World`], and none of the types publicly expose a mutable interface. //! //! [`World`]: crate::world::World diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 5f0b724475ab2..c5e504d00fe06 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -68,7 +68,7 @@ impl_sparse_array!(ImmutableSparseArray); impl SparseArray { /// Inserts `value` at `index` in the array. /// - /// If `index` is out-of-bounds, this will grow the buffer until it can accomodate it. + /// If `index` is out-of-bounds, this will enlarge the buffer to accommodate it. #[inline] pub fn insert(&mut self, index: I, value: V) { let index = index.sparse_set_index(); @@ -215,7 +215,7 @@ impl ComponentSparseSet { }) } - /// Fetches a read-only reference to the component for `entity` and it's change detection ticks, + /// Fetches a read-only reference to the component for `entity` and its change detection ticks, /// if present. /// /// Returns `None` if `entity` does not have a component in the sparse set. @@ -397,7 +397,7 @@ macro_rules! impl_sparse_set { }) } - /// Iterates over the indicies (keys) of the sparse set. + /// Iterates over the indices (keys) of the sparse set. pub fn indices(&self) -> impl Iterator + '_ { self.indices.iter().cloned() } @@ -412,12 +412,12 @@ macro_rules! impl_sparse_set { self.dense.iter_mut() } - /// Iterates over key-value pairs the sparse set. + /// Iterates over key-value pairs the of the sparse set. pub fn iter(&self) -> impl Iterator { self.indices.iter().zip(self.dense.iter()) } - /// Iterates over key-value pairs of the sparse set. + /// Iterates over pairs of keys and mutable values of the sparse set. pub fn iter_mut(&mut self) -> impl Iterator { self.indices.iter().zip(self.dense.iter_mut()) } @@ -464,7 +464,7 @@ impl SparseSet { /// Inserts `value` at `index`. /// - /// If a value was already present at `index`, it will overwrite it. + /// If a value was already present at `index`, it will be overwritten. pub fn insert(&mut self, index: I, value: V) { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { // SAFETY: dense indices stored in self.sparse always exist @@ -526,7 +526,7 @@ impl SparseSet { self.sparse.clear(); } - /// Converts the sparse set into it's immutable variant. + /// Converts the sparse set into its immutable variant. pub(crate) fn into_immutable(self) -> ImmutableSparseSet { ImmutableSparseSet { dense: self.dense.into_boxed_slice(), diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 2a20155dff1f3..63a9a83a46882 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -35,6 +35,8 @@ impl TableId { /// Creates a new [`TableId`]. `index` *must* be retrieved from [`TableId::index`] /// from a table of a given [`World`] or the created ID may be invalid. + /// + /// [`World`]: crate::world::World #[inline] pub fn new(index: usize) -> Self { TableId(index as u32) @@ -92,10 +94,10 @@ impl TableRow { /// A type-erased contiguous container for data of a homogenous type. /// -/// Conceptually, a [`Column`] is very similar to a `Vec` with it's generic type erased. -/// It also store the change detection ticks for the stored components, kept in two separate -/// contiugous buffers internally. An element shares it's data across these buffers by using the -/// saame index (i.e. the entity at row 3 has it's data at index 3 and it's change detection ticks at +/// Conceptually, a [`Column`] is very similar to a type-erased `Vec`. +/// It also stores the change detection ticks for its components, kept in two separate +/// contiguous buffers internally. An element shares it's data across these buffers by using the +/// same index (i.e. the entity at row 3 has it's data at index 3 and it's change detection ticks at /// index 3). A slice to these contiguous blocks of memory can be fetched /// via [`Column::get_data_slice`], [`Column::get_added_ticks_slice`], and /// [`Column::get_changed_ticks_slice`]. @@ -111,7 +113,7 @@ pub struct Column { } impl Column { - /// Constructs a new [`Column`], configured with a component's layout and a initial `capacity`. + /// Constructs a new [`Column`], configured with a component's layout and an initial `capacity`. #[inline] pub(crate) fn with_capacity(component_info: &ComponentInfo, capacity: usize) -> Self { Column { @@ -175,7 +177,7 @@ impl Column { self.data.len() } - /// Checks if the column is empty. Returns `true` if there are no elements, false otherwise. + /// Checks if the column is empty. Returns `true` if there are no elements, `false` otherwise. #[inline] pub fn is_empty(&self) -> bool { self.data.is_empty() @@ -185,8 +187,8 @@ impl Column { /// /// - The value will be dropped if it implements [`Drop`]. /// - This does not preserve ordering, but is O(1). - /// - This does do any bounds checking. - /// - The element is replaced last element in the [`Column`]. + /// - This does not do any bounds checking. + /// - The element is replaced with the last element in the [`Column`]. /// /// # Safety /// `row` must be within the range `[0, self.len())`. @@ -199,12 +201,12 @@ impl Column { self.changed_ticks.swap_remove(row.index()); } - /// Removes an element from the [`Column`] and returns it and it's change detection ticks. + /// Removes an element from the [`Column`] and returns it and its change detection ticks. /// This does not preserve ordering, but is O(1). /// - /// The element is replaced last element in the [`Column`]. + /// The element is replaced with the last element in the [`Column`]. /// - /// It's the caller's responsibility to ensure that the removed value is dropped or used. + /// It is the caller's responsibility to ensure that the removed value is dropped or used. /// /// Returns `None` if `row` is out of bounds. #[inline] @@ -552,7 +554,7 @@ pub struct Table { } impl Table { - /// Fetches a read-only slice of the entiities stored within the [`Table`]. + /// Fetches a read-only slice of the entities stored within the [`Table`]. #[inline] pub fn entities(&self) -> &[Entity] { &self.entities @@ -674,6 +676,8 @@ impl Table { /// table. /// /// Returns `None` if the corresponding component does not belong to the table. + /// + /// [`Component`]: crate::component::Component #[inline] pub fn get_column(&self, component_id: ComponentId) -> Option<&Column> { self.columns.get(component_id) @@ -683,12 +687,14 @@ impl Table { /// table. /// /// Returns `None` if the corresponding component does not belong to the table. + /// + /// [`Component`]: crate::component::Component #[inline] pub(crate) fn get_column_mut(&mut self, component_id: ComponentId) -> Option<&mut Column> { self.columns.get_mut(component_id) } - /// Checks if the table contains a[`Column`] for a given [`Component`]. + /// Checks if the table contains a [`Column`] for a given [`Component`]. /// /// Returns `true` if the column is present, `false` otherwise. #[inline] @@ -808,7 +814,7 @@ impl Tables { self.tables.is_empty() } - /// Fetches a [`Table`] by it's [`TableId`]. + /// Fetches a [`Table`] by its [`TableId`]. /// /// Returns `None` if `id` is invalid. #[inline] @@ -828,8 +834,8 @@ impl Tables { } } - /// Attempts to fetch a table based on the provided components. If none is found, - /// a new [`Table`] is created and returned. + /// Attempts to fetch a table based on the provided components, + /// creating and returning a new [`Table`] if one did not already exist. /// /// # Safety /// `component_ids` must contain components that exist in `components` From ce551df81220a5a5db02db4d7a4899fe33609e65 Mon Sep 17 00:00:00 2001 From: James Liu Date: Mon, 20 Feb 2023 21:51:04 -0800 Subject: [PATCH 03/12] Apply suggestions from code review Co-authored-by: Rob Parrett --- crates/bevy_ecs/src/storage/table.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 63a9a83a46882..5e33d073a59a4 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -96,8 +96,8 @@ impl TableRow { /// /// Conceptually, a [`Column`] is very similar to a type-erased `Vec`. /// It also stores the change detection ticks for its components, kept in two separate -/// contiguous buffers internally. An element shares it's data across these buffers by using the -/// same index (i.e. the entity at row 3 has it's data at index 3 and it's change detection ticks at +/// contiguous buffers internally. An element shares its data across these buffers by using the +/// same index (i.e. the entity at row 3 has it's data at index 3 and its change detection ticks at /// index 3). A slice to these contiguous blocks of memory can be fetched /// via [`Column::get_data_slice`], [`Column::get_added_ticks_slice`], and /// [`Column::get_changed_ticks_slice`]. @@ -224,11 +224,11 @@ impl Column { }) } - /// Removes an element from the [`Column`] and returns it and it's change detection ticks. + /// Removes an element from the [`Column`] and returns it and its change detection ticks. /// This does not preserve ordering, but is O(1). Unlike [`Column::swap_remove_and_forget`] - /// this does do any bounds checking. + /// this does not do any bounds checking. /// - /// The element is replaced last element in the [`Column`]. + /// The element is replaced with the last element in the [`Column`]. /// /// It's the caller's responsibility to ensure that the removed value is dropped or used. /// @@ -387,7 +387,7 @@ impl Column { (row.index() < self.data.len()).then(|| unsafe { self.data.get_unchecked_mut(row.index()) }) } - /// Fetches a mutable reference to the data at `row`. Unlike [`Column::get_mut`] this does not + /// Fetches a mutable reference to the data at `row`. Unlike [`Column::get_data_mut`] this does not /// do any bounds checking. /// /// # Safety @@ -697,6 +697,8 @@ impl Table { /// Checks if the table contains a [`Column`] for a given [`Component`]. /// /// Returns `true` if the column is present, `false` otherwise. + /// + /// [`Component`]: crate::component::Component #[inline] pub fn has_column(&self, component_id: ComponentId) -> bool { self.columns.contains(component_id) From 85238e873f76b7d6ed484d0138eb019909585c2f Mon Sep 17 00:00:00 2001 From: James Liu Date: Mon, 20 Feb 2023 21:51:25 -0800 Subject: [PATCH 04/12] Update crates/bevy_ecs/src/storage/table.rs Co-authored-by: Rob Parrett --- crates/bevy_ecs/src/storage/table.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 5e33d073a59a4..8523c47c8f0e8 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -543,7 +543,7 @@ impl TableBuilder { /// each [`Column`] is a type-erased `Vec`. Each row corresponds to a single entity /// (i.e. index 3 in Column A and index 3 in Column B point to different components on the same /// entity). Fetching components from a table involves fetching the associated column for a -/// component type (via it's [`ComponentId`]), then fetching the entity's row within that column. +/// component type (via its [`ComponentId`]), then fetching the entity's row within that column. /// /// [structure-of-arrays]: https://en.wikipedia.org/wiki/AoS_and_SoA#Structure_of_arrays /// [`Component`]: crate::component::Component From 5a718a908060f2b69726a98e4e20cb81874116cd Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 20 Feb 2023 23:34:40 -0800 Subject: [PATCH 05/12] Mention potential memory leaks if failing to drop --- crates/bevy_ecs/src/storage/table.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 8523c47c8f0e8..d8fa96cd05554 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -207,6 +207,8 @@ impl Column { /// The element is replaced with the last element in the [`Column`]. /// /// It is the caller's responsibility to ensure that the removed value is dropped or used. + /// Failure to do so may result in resources not being released (i.e. files handles not being + /// released, memory leaks, etc.) /// /// Returns `None` if `row` is out of bounds. #[inline] @@ -231,6 +233,8 @@ impl Column { /// The element is replaced with the last element in the [`Column`]. /// /// It's the caller's responsibility to ensure that the removed value is dropped or used. + /// Failure to do so may result in resources not being released (i.e. files handles not being + /// released, memory leaks, etc.) /// /// # Safety /// `row` must be within the range `[0, self.len())`. @@ -581,7 +585,8 @@ impl Table { /// Moves the `row` column values to `new_table`, for the columns shared between both tables. /// Returns the index of the new row in `new_table` and the entity in this table swapped in /// to replace it (if an entity was swapped in). missing columns will be "forgotten". It is - /// the caller's responsibility to drop them + /// the caller's responsibility to drop them. Failure to do so may result in resources not + /// being released (i.e. files handles not being released, memory leaks, etc.) /// /// # Safety /// Row must be in-bounds From 50fa3210de0b51669391fec00fbaf597c9190516 Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 20 Feb 2023 23:45:24 -0800 Subject: [PATCH 06/12] Address Alice's comments --- crates/bevy_ecs/src/storage/blob_vec.rs | 6 ++++++ crates/bevy_ecs/src/storage/table.rs | 9 ++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index e0f4d5e293668..6d528c225e57d 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -37,6 +37,11 @@ impl std::fmt::Debug for BlobVec { impl BlobVec { /// Creates a new [`BlobVec`] with the specified `capacity`. /// + /// `drop` is an optional function pointer that is meant to be invoked when any element in the [`BlobVec`] + /// should be dropped. For all Rust-based types, this should match 1:1 with the implementation of [`Drop`] + /// if present, and should be `None` if `T: !Drop`. For non-Rust based types, this should match any cleanup + /// processes typically associated with the stored + /// /// # Safety /// /// `drop` should be safe to call with an [`OwningPtr`] pointing to any item that's been pushed into this [`BlobVec`]. @@ -44,6 +49,7 @@ impl BlobVec { /// If `drop` is `None`, the items will be leaked. This should generally be set as None based on [`needs_drop`]. /// /// [`needs_drop`]: core::mem::needs_drop + /// [`Drop`]: core::mem::Drop pub unsafe fn new( item_layout: Layout, drop: Option)>, diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index d8fa96cd05554..947e49737a86a 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -296,7 +296,9 @@ impl Column { /// Fetches the data pointer to the first element of the [`Column`]. /// - /// The pointer is type erased, so using this function to fetch + /// The pointer is type erased, so using this function to fetch anything + /// other than the first element will require computing the offset using + /// via [`Column::layout`]. #[inline] pub fn get_data_ptr(&self) -> Ptr<'_> { self.data.get_ptr() @@ -585,7 +587,7 @@ impl Table { /// Moves the `row` column values to `new_table`, for the columns shared between both tables. /// Returns the index of the new row in `new_table` and the entity in this table swapped in /// to replace it (if an entity was swapped in). missing columns will be "forgotten". It is - /// the caller's responsibility to drop them. Failure to do so may result in resources not + /// the caller's responsibility to drop them. Failure to do so may result in resources not /// being released (i.e. files handles not being released, memory leaks, etc.) /// /// # Safety @@ -751,7 +753,8 @@ impl Table { self.columns.len() } - /// Gets the number of components the table can currently store. + /// Gets the maximum number of entities the table can currently store + /// without reallocating the underlying memory. #[inline] pub fn entity_capacity(&self) -> usize { self.entities.capacity() From 8affcdb60a963f993f51b0670350be25c1ad8560 Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 20 Feb 2023 23:45:57 -0800 Subject: [PATCH 07/12] Add more docs for resources --- crates/bevy_ecs/src/storage/resource.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 10e44533fc128..5f4f83fbc836b 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -74,7 +74,7 @@ impl ResourceData { self.id } - /// Gets a read-only pointer to the underlying resource, if available. + /// Gets a read-only pointer to the underlying resource, if present. /// /// # Panics /// If `SEND` is false, this will panic if a value is present and is not accessed from the @@ -87,13 +87,13 @@ impl ResourceData { }) } - /// Gets a read-only reference to the change ticks of the underlying resource, if available. + /// Gets a read-only reference to the change ticks of the underlying resource, if present. #[inline] pub fn get_ticks(&self) -> Option { self.column.get_ticks(Self::ROW) } - /// Gets a read-only reference and the change detection ticks for the resource. + /// Gets a read-only reference and the change detection ticks for the resource, if present. /// /// # Panics /// If `SEND` is false, this will panic if a value is present and is not accessed from the @@ -106,6 +106,11 @@ impl ResourceData { }) } + /// Gets a mutable reference for the resource, if present. + /// + /// # Panics + /// If `SEND` is false, this will panic if a value is present and is not accessed from the + /// original thread it was inserted in. pub(crate) fn get_mut( &mut self, last_change_tick: u32, From 50537c6d3c0f2c78c04eeda9c694f3daa8fceb90 Mon Sep 17 00:00:00 2001 From: James Liu Date: Tue, 21 Feb 2023 12:18:44 -0800 Subject: [PATCH 08/12] Apply suggestions from code review Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> Co-authored-by: Cameron <51241057+maniwani@users.noreply.github.com> --- crates/bevy_ecs/src/storage/blob_vec.rs | 29 +++++---- crates/bevy_ecs/src/storage/mod.rs | 4 +- crates/bevy_ecs/src/storage/resource.rs | 8 +-- crates/bevy_ecs/src/storage/sparse_set.rs | 76 ++++++++++------------- crates/bevy_ecs/src/storage/table.rs | 6 +- 5 files changed, 58 insertions(+), 65 deletions(-) diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 6d528c225e57d..04d04e1d33bb1 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -40,7 +40,7 @@ impl BlobVec { /// `drop` is an optional function pointer that is meant to be invoked when any element in the [`BlobVec`] /// should be dropped. For all Rust-based types, this should match 1:1 with the implementation of [`Drop`] /// if present, and should be `None` if `T: !Drop`. For non-Rust based types, this should match any cleanup - /// processes typically associated with the stored + /// processes typically associated with the stored element. /// /// # Safety /// @@ -78,26 +78,25 @@ impl BlobVec { } } - /// Gets the number of elements currently stored in the [`BlobVec`]. + /// Returns the number of elements in the vector. #[inline] pub fn len(&self) -> usize { self.len } - /// Checks if the [`BlobVec`] is currently empty. + /// Returns `true` if the vector contains no elements. #[inline] pub fn is_empty(&self) -> bool { self.len == 0 } - /// Gets the maximum possible number of elements the [`BlobVec`] can store without - /// reallocating its underlying buffer. + /// Returns the total number of elements the vector can hold without reallocating. #[inline] pub fn capacity(&self) -> usize { self.capacity } - /// Fetches the [`Layout`] of the underlying type stored within the [`BlobVec`]. + /// Returns the [`Layout`] of the element type stored in the vector. #[inline] pub fn layout(&self) -> Layout { self.item_layout @@ -224,10 +223,10 @@ impl BlobVec { std::ptr::copy_nonoverlapping::(source, destination.as_ptr(), self.item_layout.size()); } - /// Appends a value to the back of the [`BlobVec`]. + /// Appends an element to the back of the vector. /// /// # Safety - /// `value` must be valid to add to this [`BlobVec`] + /// The `value` must match the [`layout`](`BlobVec::layout`) of the elements in the [`BlobVec`]. #[inline] pub unsafe fn push(&mut self, value: OwningPtr<'_>) { self.reserve_exact(1); @@ -316,11 +315,10 @@ impl BlobVec { } } - /// Fetches a read-only reference to the value at `index`. - /// Does not do any bounds checking on `index`. + /// Returns a reference to the element at `index`, without doing bounds checking. /// /// # Safety - /// It is the caller's responsibility to ensure that `index` is < self.len() + /// It is the caller's responsibility to ensure that `index < self.len()`. #[inline] pub unsafe fn get_unchecked(&self, index: usize) -> Ptr<'_> { debug_assert!(index < self.len()); @@ -333,11 +331,10 @@ impl BlobVec { self.get_ptr().byte_add(index * size) } - /// Fetches a mutable reference to the value at `index`. - /// Does not do any bounds checking on `index`. + /// Returns a mutable reference to the element at `index`, without doing bounds checking. /// /// # Safety - /// It is the caller's responsibility to ensure that `index` is < self.len() + /// It is the caller's responsibility to ensure that `index < self.len()`. #[inline] pub unsafe fn get_unchecked_mut(&mut self, index: usize) -> PtrMut<'_> { debug_assert!(index < self.len()); @@ -373,7 +370,9 @@ impl BlobVec { std::slice::from_raw_parts(self.data.as_ptr() as *const UnsafeCell, self.len) } - /// Clears the [`BlobVec`] by removing and dropping all of its elements. + /// Clears the vector, removing (and dropping) all values. + /// + /// Note that this method has no effect on the allocated capacity of the vector. pub fn clear(&mut self) { let len = self.len; // We set len to 0 _before_ dropping elements for unwind safety. This ensures we don't diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index ee61d13b8ed93..998e525c21598 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -1,7 +1,7 @@ //! Storage layouts for ECS data. //! -//! This module contains the low-level backing data stores for ECS. These all offer minimal and often -//! unsafe APIs, exposed primarily for debugging and monitoring purposes. +//! This module implements the low-level collections that store data in a [`World`]. These all offer minimal and often +//! unsafe APIs, and have been made `pub` primarily for debugging and monitoring purposes. //! //! # Fetching Storages //! Each of the primary backing data stores can be fetched via [`Storages`], which can be fetched from a diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 5f4f83fbc836b..bdebad152e716 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -74,7 +74,7 @@ impl ResourceData { self.id } - /// Gets a read-only pointer to the underlying resource, if present. + /// Returns a reference to the resource, if it exists. /// /// # Panics /// If `SEND` is false, this will panic if a value is present and is not accessed from the @@ -87,13 +87,13 @@ impl ResourceData { }) } - /// Gets a read-only reference to the change ticks of the underlying resource, if present. + /// Returns a reference to the resource's change ticks, if it exists. #[inline] pub fn get_ticks(&self) -> Option { self.column.get_ticks(Self::ROW) } - /// Gets a read-only reference and the change detection ticks for the resource, if present. + /// Returns references to the resource and its change ticks, if it exists. /// /// # Panics /// If `SEND` is false, this will panic if a value is present and is not accessed from the @@ -106,7 +106,7 @@ impl ResourceData { }) } - /// Gets a mutable reference for the resource, if present. + /// Returns a mutable reference to the resource, if it exists. /// /// # Panics /// If `SEND` is false, this will panic if a value is present and is not accessed from the diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index c5e504d00fe06..ff79b537e7c90 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -41,18 +41,16 @@ impl SparseArray { macro_rules! impl_sparse_array { ($ty:ident) => { impl $ty { - /// Checks if a value is present at `index`. - /// - /// Returns `true` if there's a value present. + /// Returns `true` if the collection contains a value for the specified `index`. #[inline] pub fn contains(&self, index: I) -> bool { let index = index.sparse_set_index(); self.values.get(index).map(|v| v.is_some()).unwrap_or(false) } - /// Fetches a read-only reference to the value at `index`. + /// Returns a reference to the value at `index`. /// - /// Returns `None` if it's not populated or if `index` is out of bounds. + /// Returns `None` if `index` does not have a value or if `index` is out of bounds. #[inline] pub fn get(&self, index: I) -> Option<&V> { let index = index.sparse_set_index(); @@ -78,9 +76,9 @@ impl SparseArray { self.values[index] = Some(value); } - /// Fetches a mutable reference to the value at `index`. + /// Returns a mutable reference to the value at `index`. /// - /// Returns `None` if it's not populated or if `index` is out of bounds. + /// Returns `None` if `index` does not have a value or if `index` is out of bounds. #[inline] pub fn get_mut(&mut self, index: I) -> Option<&mut V> { let index = index.sparse_set_index(); @@ -90,9 +88,9 @@ impl SparseArray { .unwrap_or(None) } - /// Removes the value stored at `index`, if present. + /// Removes and returns the value stored at `index`. /// - /// Returns `None` if it's not populated or if `index` is out of bounds. + /// Returns `None` if `index` did not have a value or if `index` is out of bounds. #[inline] pub fn remove(&mut self, index: I) -> Option { let index = index.sparse_set_index(); @@ -147,13 +145,13 @@ impl ComponentSparseSet { self.sparse.clear(); } - /// Gets the number of components currently stored in the sparse set. + /// Returns the number of component values in the sparse set. #[inline] pub fn len(&self) -> usize { self.dense.len() } - /// Checks if the sparse set is empty. Returns `true` if it contains no elements, false otherwise. + /// Returns `true` if the sparse set contains no component values. #[inline] pub fn is_empty(&self) -> bool { self.dense.len() == 0 @@ -184,7 +182,7 @@ impl ComponentSparseSet { } } - /// Checks if the [`ComponentSparseSet`] contains a component for `entity`. + /// Returns `true` if the sparse set has a component value for the provided `entity`. #[inline] pub fn contains(&self, entity: Entity) -> bool { #[cfg(debug_assertions)] @@ -201,7 +199,7 @@ impl ComponentSparseSet { self.sparse.contains(entity.index()) } - /// Fetches a read-only reference to the component for `entity`, if present. + /// Returns a reference to the entity's component value. /// /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] @@ -215,8 +213,7 @@ impl ComponentSparseSet { }) } - /// Fetches a read-only reference to the component for `entity` and its change detection ticks, - /// if present. + /// Returns references to the entity's component value and its added and changed ticks. /// /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] @@ -236,7 +233,7 @@ impl ComponentSparseSet { } } - /// Fetches a reference to `entity`'s "added" change detection ticks, if present. + /// Returns a reference to the "added" tick of the entity's component value. /// /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] @@ -253,7 +250,7 @@ impl ComponentSparseSet { } } - /// Fetches a reference to `entity`'s "changed" change detection ticks, if present. + /// Returns a reference to the "changed" tick of the entity's component value. /// /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] @@ -270,7 +267,7 @@ impl ComponentSparseSet { } } - /// Fetches `entity`'s change detection ticks, if present. + /// Returns a reference to the "added" and "changed" ticks of the entity's component value. /// /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] @@ -309,11 +306,9 @@ impl ComponentSparseSet { }) } - /// Removes `entity`'s component from the sparse set, if present. - /// - /// If `entity` has a component stored in the sparse set, it will be dropped. + /// Removes (and drops) the entity's component value from the sparse set. /// - /// Returns `true` if `entity` had a component in the sparse set. + /// Returns `true` if `entity` had a component value in the sparse set. pub(crate) fn remove(&mut self, entity: Entity) -> bool { if let Some(dense_index) = self.sparse.remove(entity.index()) { let dense_index = dense_index as usize; @@ -364,21 +359,21 @@ pub(crate) struct ImmutableSparseSet { macro_rules! impl_sparse_set { ($ty:ident) => { impl $ty { - /// Gets the number of elements currently stored in the sparse set. + /// Returns the number of elements in the sparse set. #[inline] pub fn len(&self) -> usize { self.dense.len() } - /// Checks if there's a value stored at `index`. + /// Returns `true` if the sparse set contains a value for `index`. #[inline] pub fn contains(&self, index: I) -> bool { self.sparse.contains(index) } - /// Fetches a read-only reference to the value at `index`, if present. + /// Returns a reference to the value for `index`. /// - /// Returns `None` if `index` is not present in the sparse set. + /// Returns `None` if `index` does not have a value in the sparse set. pub fn get(&self, index: I) -> Option<&V> { self.sparse.get(index).map(|dense_index| { // SAFETY: if the sparse index points to something in the dense vec, it exists @@ -386,9 +381,9 @@ macro_rules! impl_sparse_set { }) } - /// Fetches a mutable reference to the value at `index`, if present. + /// Returns a mutable reference to the value for `index`. /// - /// Returns `None` if `index` is not present in the sparse set. + /// Returns `None` if `index` does not have a value in the sparse set. pub fn get_mut(&mut self, index: I) -> Option<&mut V> { let dense = &mut self.dense; self.sparse.get(index).map(move |dense_index| { @@ -397,27 +392,27 @@ macro_rules! impl_sparse_set { }) } - /// Iterates over the indices (keys) of the sparse set. + /// Returns an iterator visiting all keys (indices) in arbitrary order. pub fn indices(&self) -> impl Iterator + '_ { self.indices.iter().cloned() } - /// Iterates over read-only references to the values of the sparse set. + /// Returns an iterator visiting all values in arbitrary order. pub fn values(&self) -> impl Iterator { self.dense.iter() } - /// Iterates over mutable references to the values of the sparse set. + /// Returns an iterator visiting all values mutably in arbitrary order. pub fn values_mut(&mut self) -> impl Iterator { self.dense.iter_mut() } - /// Iterates over key-value pairs the of the sparse set. + /// Returns an iterator visiting all key-value pairs in arbitrary order, with references to the values. pub fn iter(&self) -> impl Iterator { self.indices.iter().zip(self.dense.iter()) } - /// Iterates over pairs of keys and mutable values of the sparse set. + /// Returns an iterator visiting all key-value pairs in arbitrary order, with mutable references to the values. pub fn iter_mut(&mut self) -> impl Iterator { self.indices.iter().zip(self.dense.iter_mut()) } @@ -455,8 +450,7 @@ impl SparseSet { } } - /// The total number of elements the [`SparseSet`] can hold without needing to - /// allocate a larger buffer. + /// Returns the total number of elements the [`SparseSet`] can hold without needing to reallocate. #[inline] pub fn capacity(&self) -> usize { self.dense.capacity() @@ -478,8 +472,8 @@ impl SparseSet { } } - /// Attempts to fetch the value at `index`. If not present, `func` will be called to - /// initialize a value. + /// Returns a reference to the value for `index`, inserting one computed from `func` + /// if not already present. pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut V { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { // SAFETY: dense indices stored in self.sparse always exist @@ -495,17 +489,15 @@ impl SparseSet { } } - /// Checks if the [`SparseSet`] is currently empty or not. - /// - /// Returns `true` if it currently contains zero elements, `false` otherwise. + /// Returns `true` if the sparse set contains no elements. #[inline] pub fn is_empty(&self) -> bool { self.dense.len() == 0 } - /// Removes and returns the value stored at `index`, if present. + /// Removes and returns the value for `index`. /// - /// Returns `None` the value at `index` is not populated. + /// Returns `None` if `index` does not have a value in the sparse set. pub fn remove(&mut self, index: I) -> Option { self.sparse.remove(index).map(|dense_index| { let is_last = dense_index == self.dense.len() - 1; diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 947e49737a86a..dde668ff45ef8 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -33,7 +33,9 @@ pub struct TableId(u32); impl TableId { pub(crate) const INVALID: TableId = TableId(u32::MAX); - /// Creates a new [`TableId`]. `index` *must* be retrieved from [`TableId::index`] + /// Creates a new [`TableId`]. + /// + /// `index` *must* be retrieved from calling [`TableId::index`] on a `TableId` you got /// from a table of a given [`World`] or the created ID may be invalid. /// /// [`World`]: crate::world::World @@ -298,7 +300,7 @@ impl Column { /// /// The pointer is type erased, so using this function to fetch anything /// other than the first element will require computing the offset using - /// via [`Column::layout`]. + /// [`Column::layout`]. #[inline] pub fn get_data_ptr(&self) -> Ptr<'_> { self.data.get_ptr() From 8b9553c6a7bd44235899d09eb4efd163829fdd33 Mon Sep 17 00:00:00 2001 From: James Liu Date: Sat, 25 Feb 2023 22:49:00 -0800 Subject: [PATCH 09/12] Improve module level docs Co-authored-by: Carter Weinberg --- crates/bevy_ecs/src/storage/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index 998e525c21598..2cabf44303040 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -4,7 +4,7 @@ //! unsafe APIs, and have been made `pub` primarily for debugging and monitoring purposes. //! //! # Fetching Storages -//! Each of the primary backing data stores can be fetched via [`Storages`], which can be fetched from a +//! Each of the below data stores can be fetched via [`Storages`], which can be fetched from a //! [`World`] via [`World::storages`]. It exposes a top level container for each class of storage within //! ECS: //! From 08e2c6e219ecd9394a25468b3dd017fd43d06f4c Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 25 Feb 2023 22:50:30 -0800 Subject: [PATCH 10/12] Document naming scheme of BlobVec --- crates/bevy_ecs/src/storage/blob_vec.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 04d04e1d33bb1..ab00418fa5b44 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -10,7 +10,9 @@ use bevy_utils::OnDrop; /// A flat, type-erased data storage type /// -/// Used to densely store homogeneous ECS data. +/// Used to densely store homogeneous ECS data. A blob is usually just an arbitrary block of contiguous memory without any identity, and +/// could be used to represent any arbitrary data (i.e. string, arrays, etc). This type is an extendable and reallcatable blob, which makes +/// it a blobby Vec, a BlobVec. pub(super) struct BlobVec { item_layout: Layout, capacity: usize, From 90b0516afe8f5122532f2b713ef2b5a6990b1561 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 25 Feb 2023 22:53:00 -0800 Subject: [PATCH 11/12] Add get_2_mut panic doc comment --- crates/bevy_ecs/src/archetype.rs | 3 +++ crates/bevy_ecs/src/storage/table.rs | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index bd2ceb5dcc582..fd26b0380a8b2 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -652,6 +652,9 @@ impl Archetypes { self.archetypes.get(id.index()) } + /// # Panics + /// + /// Panics if `a` and `b` are equal. #[inline] pub(crate) fn get_2_mut( &mut self, diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index dde668ff45ef8..65b57ac0ecb74 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -835,6 +835,10 @@ impl Tables { } /// Fetches mutable references to two different [`Table`]s. + /// + /// # Panics + /// + /// Panics if `a` and `b` are equal. #[inline] pub(crate) fn get_2_mut(&mut self, a: TableId, b: TableId) -> (&mut Table, &mut Table) { if a.index() > b.index() { From 4425ca709f4e23c3826fe46226fb792f8e50299d Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 26 Feb 2023 17:08:48 -0800 Subject: [PATCH 12/12] Fix CI --- crates/bevy_ecs/src/archetype.rs | 2 +- crates/bevy_ecs/src/storage/blob_vec.rs | 8 ++++---- crates/bevy_ecs/src/storage/sparse_set.rs | 4 ++-- crates/bevy_ecs/src/storage/table.rs | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index fd26b0380a8b2..9e7833b9552dc 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -653,7 +653,7 @@ impl Archetypes { } /// # Panics - /// + /// /// Panics if `a` and `b` are equal. #[inline] pub(crate) fn get_2_mut( diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index ab00418fa5b44..1f7f636fb6c7f 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -10,9 +10,9 @@ use bevy_utils::OnDrop; /// A flat, type-erased data storage type /// -/// Used to densely store homogeneous ECS data. A blob is usually just an arbitrary block of contiguous memory without any identity, and -/// could be used to represent any arbitrary data (i.e. string, arrays, etc). This type is an extendable and reallcatable blob, which makes -/// it a blobby Vec, a BlobVec. +/// Used to densely store homogeneous ECS data. A blob is usually just an arbitrary block of contiguous memory without any identity, and +/// could be used to represent any arbitrary data (i.e. string, arrays, etc). This type is an extendable and reallcatable blob, which makes +/// it a blobby Vec, a `BlobVec`. pub(super) struct BlobVec { item_layout: Layout, capacity: usize, @@ -51,7 +51,7 @@ impl BlobVec { /// If `drop` is `None`, the items will be leaked. This should generally be set as None based on [`needs_drop`]. /// /// [`needs_drop`]: core::mem::needs_drop - /// [`Drop`]: core::mem::Drop + /// [`Drop`]: core::ops::Drop pub unsafe fn new( item_layout: Layout, drop: Option)>, diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index ff79b537e7c90..217609dc8c5ac 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -213,7 +213,7 @@ impl ComponentSparseSet { }) } - /// Returns references to the entity's component value and its added and changed ticks. + /// Returns references to the entity's component value and its added and changed ticks. /// /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] @@ -472,7 +472,7 @@ impl SparseSet { } } - /// Returns a reference to the value for `index`, inserting one computed from `func` + /// Returns a reference to the value for `index`, inserting one computed from `func` /// if not already present. pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut V { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 65b57ac0ecb74..d456d1f900b56 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -33,7 +33,7 @@ pub struct TableId(u32); impl TableId { pub(crate) const INVALID: TableId = TableId(u32::MAX); - /// Creates a new [`TableId`]. + /// Creates a new [`TableId`]. /// /// `index` *must* be retrieved from calling [`TableId::index`] on a `TableId` you got /// from a table of a given [`World`] or the created ID may be invalid. @@ -300,7 +300,7 @@ impl Column { /// /// The pointer is type erased, so using this function to fetch anything /// other than the first element will require computing the offset using - /// [`Column::layout`]. + /// [`Column::item_layout`]. #[inline] pub fn get_data_ptr(&self) -> Ptr<'_> { self.data.get_ptr() @@ -837,7 +837,7 @@ impl Tables { /// Fetches mutable references to two different [`Table`]s. /// /// # Panics - /// + /// /// Panics if `a` and `b` are equal. #[inline] pub(crate) fn get_2_mut(&mut self, a: TableId, b: TableId) -> (&mut Table, &mut Table) {