Skip to content

Commit c988264

Browse files
committed
Mark mutable APIs under ECS storage as pub(crate) (#5065)
# Objective Closes #1557. Partially addresses #3362. Cleanup the public facing API for storage types. Most of these APIs are difficult to use safely when directly interfacing with these types, and is also currently impossible to interact with in normal ECS use as there is no `World::storages_mut`. The majority of these types should be easy enough to read, and perhaps mutate the contents, but never structurally altered without the same checks in the rest of bevy_ecs code. This both cleans up the public facing types and helps use unused code detection to remove a few of the APIs we're not using internally. ## Solution - Mark all APIs that take `&mut T` under `bevy_ecs::storage` as `pub(crate)` or `pub(super)` - Cleanup after it all. Entire type visibility changes: - `BlobVec` is `pub(super)`, only storage code should be directly interacting with it. - `SparseArray` is now `pub(crate)` for the entire type. It's an implementation detail for `Table` and `(Component)SparseSet`. - `TableMoveResult` is now `pub(crate) --- ## Changelog TODO ## Migration Guide Dear God, I hope not.
1 parent 389df18 commit c988264

File tree

6 files changed

+82
-119
lines changed

6 files changed

+82
-119
lines changed

crates/bevy_ecs/src/archetype.rs

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,35 @@ impl ArchetypeId {
3232
}
3333
}
3434

35-
pub enum ComponentStatus {
35+
pub(crate) enum ComponentStatus {
3636
Added,
3737
Mutated,
3838
}
3939

4040
pub struct AddBundle {
4141
pub archetype_id: ArchetypeId,
42-
pub bundle_status: Vec<ComponentStatus>,
42+
pub(crate) bundle_status: Vec<ComponentStatus>,
4343
}
4444

45+
/// Archetypes and bundles form a graph. Adding or removing a bundle moves
46+
/// an [`Entity`] to a new [`Archetype`].
47+
///
48+
/// [`Edges`] caches the results of these moves. Each archetype caches
49+
/// the result of a structural alteration. This can be used to monitor the
50+
/// state of the archetype graph.
51+
///
52+
/// Note: This type only contains edges the [`World`] has already traversed.
53+
/// If any of functions return `None`, it doesn't mean there is guarenteed
54+
/// not to be a result of adding or removing that bundle, but rather that
55+
/// operation that has moved an entity along that edge has not been performed
56+
/// yet.
57+
///
58+
/// [`World`]: crate::world::World
4559
#[derive(Default)]
4660
pub struct Edges {
47-
pub add_bundle: SparseArray<BundleId, AddBundle>,
48-
pub remove_bundle: SparseArray<BundleId, Option<ArchetypeId>>,
49-
pub remove_bundle_intersection: SparseArray<BundleId, Option<ArchetypeId>>,
61+
add_bundle: SparseArray<BundleId, AddBundle>,
62+
remove_bundle: SparseArray<BundleId, Option<ArchetypeId>>,
63+
remove_bundle_intersection: SparseArray<BundleId, Option<ArchetypeId>>,
5064
}
5165

5266
impl Edges {
@@ -56,7 +70,7 @@ impl Edges {
5670
}
5771

5872
#[inline]
59-
pub fn insert_add_bundle(
73+
pub(crate) fn insert_add_bundle(
6074
&mut self,
6175
bundle_id: BundleId,
6276
archetype_id: ArchetypeId,
@@ -77,7 +91,11 @@ impl Edges {
7791
}
7892

7993
#[inline]
80-
pub fn insert_remove_bundle(&mut self, bundle_id: BundleId, archetype_id: Option<ArchetypeId>) {
94+
pub(crate) fn insert_remove_bundle(
95+
&mut self,
96+
bundle_id: BundleId,
97+
archetype_id: Option<ArchetypeId>,
98+
) {
8199
self.remove_bundle.insert(bundle_id, archetype_id);
82100
}
83101

@@ -90,7 +108,7 @@ impl Edges {
90108
}
91109

92110
#[inline]
93-
pub fn insert_remove_bundle_intersection(
111+
pub(crate) fn insert_remove_bundle_intersection(
94112
&mut self,
95113
bundle_id: BundleId,
96114
archetype_id: Option<ArchetypeId>,
@@ -237,14 +255,14 @@ impl Archetype {
237255
}
238256

239257
#[inline]
240-
pub fn set_entity_table_row(&mut self, index: usize, table_row: usize) {
258+
pub(crate) fn set_entity_table_row(&mut self, index: usize, table_row: usize) {
241259
self.table_info.entity_rows[index] = table_row;
242260
}
243261

244262
/// # Safety
245263
/// valid component values must be immediately written to the relevant storages
246264
/// `table_row` must be valid
247-
pub unsafe fn allocate(&mut self, entity: Entity, table_row: usize) -> EntityLocation {
265+
pub(crate) unsafe fn allocate(&mut self, entity: Entity, table_row: usize) -> EntityLocation {
248266
self.entities.push(entity);
249267
self.table_info.entity_rows.push(table_row);
250268

@@ -254,7 +272,7 @@ impl Archetype {
254272
}
255273
}
256274

257-
pub fn reserve(&mut self, additional: usize) {
275+
pub(crate) fn reserve(&mut self, additional: usize) {
258276
self.entities.reserve(additional);
259277
self.table_info.entity_rows.reserve(additional);
260278
}
@@ -407,7 +425,7 @@ impl Archetypes {
407425
}
408426

409427
#[inline]
410-
pub fn empty_mut(&mut self) -> &mut Archetype {
428+
pub(crate) fn empty_mut(&mut self) -> &mut Archetype {
411429
// SAFE: empty archetype always exists
412430
unsafe {
413431
self.archetypes
@@ -422,7 +440,7 @@ impl Archetypes {
422440
}
423441

424442
#[inline]
425-
pub fn resource_mut(&mut self) -> &mut Archetype {
443+
pub(crate) fn resource_mut(&mut self) -> &mut Archetype {
426444
// SAFE: resource archetype always exists
427445
unsafe {
428446
self.archetypes
@@ -440,11 +458,6 @@ impl Archetypes {
440458
self.archetypes.get(id.index())
441459
}
442460

443-
#[inline]
444-
pub fn get_mut(&mut self, id: ArchetypeId) -> Option<&mut Archetype> {
445-
self.archetypes.get_mut(id.index())
446-
}
447-
448461
#[inline]
449462
pub(crate) fn get_2_mut(
450463
&mut self,
@@ -518,7 +531,7 @@ impl Archetypes {
518531
self.archetype_component_count
519532
}
520533

521-
pub fn clear_entities(&mut self) {
534+
pub(crate) fn clear_entities(&mut self) {
522535
for archetype in &mut self.archetypes {
523536
archetype.clear_entities();
524537
}

crates/bevy_ecs/src/storage/blob_vec.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use bevy_ptr::{OwningPtr, Ptr, PtrMut};
99
/// A flat, type-erased data storage type
1010
///
1111
/// Used to densely store homogeneous ECS data.
12-
pub struct BlobVec {
12+
pub(super) struct BlobVec {
1313
item_layout: Layout,
1414
capacity: usize,
1515
/// Number of elements, not bytes

crates/bevy_ecs/src/storage/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ mod blob_vec;
44
mod sparse_set;
55
mod table;
66

7-
pub use blob_vec::*;
87
pub use sparse_set::*;
98
pub use table::*;
109

crates/bevy_ecs/src/storage/sparse_set.rs

Lines changed: 6 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData};
99
type EntityId = u32;
1010

1111
#[derive(Debug)]
12-
pub struct SparseArray<I, V = I> {
12+
pub(crate) struct SparseArray<I, V = I> {
1313
values: Vec<Option<V>>,
1414
marker: PhantomData<I>,
1515
}
@@ -31,13 +31,6 @@ impl<I, V> SparseArray<I, V> {
3131
}
3232

3333
impl<I: SparseSetIndex, V> SparseArray<I, V> {
34-
pub fn with_capacity(capacity: usize) -> Self {
35-
Self {
36-
values: Vec::with_capacity(capacity),
37-
marker: PhantomData,
38-
}
39-
}
40-
4134
#[inline]
4235
pub fn insert(&mut self, index: I, value: V) {
4336
let index = index.sparse_set_index();
@@ -74,18 +67,6 @@ impl<I: SparseSetIndex, V> SparseArray<I, V> {
7467
self.values.get_mut(index).and_then(|value| value.take())
7568
}
7669

77-
#[inline]
78-
pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut V {
79-
let index = index.sparse_set_index();
80-
if index < self.values.len() {
81-
return self.values[index].get_or_insert_with(func);
82-
}
83-
self.values.resize_with(index + 1, || None);
84-
let value = &mut self.values[index];
85-
*value = Some(func());
86-
value.as_mut().unwrap()
87-
}
88-
8970
pub fn clear(&mut self) {
9071
self.values.clear();
9172
}
@@ -108,15 +89,15 @@ pub struct ComponentSparseSet {
10889
}
10990

11091
impl ComponentSparseSet {
111-
pub fn new(component_info: &ComponentInfo, capacity: usize) -> Self {
92+
pub(crate) fn new(component_info: &ComponentInfo, capacity: usize) -> Self {
11293
Self {
11394
dense: Column::with_capacity(component_info, capacity),
11495
entities: Vec::with_capacity(capacity),
11596
sparse: Default::default(),
11697
}
11798
}
11899

119-
pub fn clear(&mut self) {
100+
pub(crate) fn clear(&mut self) {
120101
self.dense.clear();
121102
self.entities.clear();
122103
self.sparse.clear();
@@ -138,7 +119,7 @@ impl ComponentSparseSet {
138119
/// # Safety
139120
/// The `value` pointer must point to a valid address that matches the [`Layout`](std::alloc::Layout)
140121
/// inside the [`ComponentInfo`] given when constructing this sparse set.
141-
pub unsafe fn insert(&mut self, entity: Entity, value: OwningPtr<'_>, change_tick: u32) {
122+
pub(crate) unsafe fn insert(&mut self, entity: Entity, value: OwningPtr<'_>, change_tick: u32) {
142123
if let Some(&dense_index) = self.sparse.get(entity.id()) {
143124
#[cfg(debug_assertions)]
144125
assert_eq!(entity, self.entities[dense_index as usize]);
@@ -209,7 +190,7 @@ impl ComponentSparseSet {
209190
/// Removes the `entity` from this sparse set and returns a pointer to the associated value (if
210191
/// it exists).
211192
#[must_use = "The returned pointer must be used to drop the removed component."]
212-
pub fn remove_and_forget(&mut self, entity: Entity) -> Option<OwningPtr<'_>> {
193+
pub(crate) fn remove_and_forget(&mut self, entity: Entity) -> Option<OwningPtr<'_>> {
213194
self.sparse.remove(entity.id()).map(|dense_index| {
214195
let dense_index = dense_index as usize;
215196
#[cfg(debug_assertions)]
@@ -230,7 +211,7 @@ impl ComponentSparseSet {
230211
})
231212
}
232213

233-
pub fn remove(&mut self, entity: Entity) -> bool {
214+
pub(crate) fn remove(&mut self, entity: Entity) -> bool {
234215
if let Some(dense_index) = self.sparse.remove(entity.id()) {
235216
let dense_index = dense_index as usize;
236217
#[cfg(debug_assertions)]
@@ -308,28 +289,6 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
308289
self.indices.push(index);
309290
self.dense.push(value);
310291
}
311-
312-
// PERF: switch to this. it's faster but it has an invalid memory access on
313-
// table_add_remove_many let dense = &mut self.dense;
314-
// let indices = &mut self.indices;
315-
// let dense_index = *self.sparse.get_or_insert_with(index.clone(), move || {
316-
// if dense.len() == dense.capacity() {
317-
// dense.reserve(64);
318-
// indices.reserve(64);
319-
// }
320-
// let len = dense.len();
321-
// // SAFE: we set the index immediately
322-
// unsafe {
323-
// dense.set_len(len + 1);
324-
// indices.set_len(len + 1);
325-
// }
326-
// len
327-
// });
328-
// // SAFE: index either already existed or was just allocated
329-
// unsafe {
330-
// *self.dense.get_unchecked_mut(dense_index) = value;
331-
// *self.indices.get_unchecked_mut(dense_index) = index;
332-
// }
333292
}
334293

335294
pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut V {

0 commit comments

Comments
 (0)