From e3db7233e92b89e0db959d59c2cc42dd603532d9 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 17 Aug 2021 13:59:22 -0700 Subject: [PATCH 1/8] Refactor spawning and inserting, insert_or_spawn_batch, enable spawning specific entities, world clearing --- benches/benches/bevy_ecs/commands.rs | 97 ++++- crates/bevy_ecs/src/archetype.rs | 16 + crates/bevy_ecs/src/bundle.rs | 390 ++++++++++++++++++++- crates/bevy_ecs/src/entity/mod.rs | 102 ++++-- crates/bevy_ecs/src/storage/sparse_set.rs | 17 + crates/bevy_ecs/src/storage/table.rs | 22 ++ crates/bevy_ecs/src/system/commands/mod.rs | 53 +++ crates/bevy_ecs/src/world/entity_ref.rs | 204 +---------- crates/bevy_ecs/src/world/mod.rs | 155 +++++++- crates/bevy_ecs/src/world/spawn_batch.rs | 76 +--- 10 files changed, 832 insertions(+), 300 deletions(-) diff --git a/benches/benches/bevy_ecs/commands.rs b/benches/benches/bevy_ecs/commands.rs index d120589702903..2d2c27f648e2d 100644 --- a/benches/benches/bevy_ecs/commands.rs +++ b/benches/benches/bevy_ecs/commands.rs @@ -1,4 +1,5 @@ use bevy::ecs::{ + entity::Entity, system::{Command, CommandQueue, Commands}, world::World, }; @@ -8,10 +9,12 @@ criterion_group!( benches, empty_commands, spawn_commands, + insert_commands, fake_commands, zero_sized_commands, medium_sized_commands, large_sized_commands + get_or_spawn, ); criterion_main!(benches); @@ -76,6 +79,58 @@ fn spawn_commands(criterion: &mut Criterion) { group.finish(); } +#[derive(Default)] +struct Matrix([[f32; 4]; 4]); + +#[derive(Default)] +struct Vec3([f32; 3]); + +fn insert_commands(criterion: &mut Criterion) { + let mut group = criterion.benchmark_group("insert_commands"); + group.warm_up_time(std::time::Duration::from_millis(500)); + group.measurement_time(std::time::Duration::from_secs(4)); + + let entity_count = 10_000; + group.bench_function(format!("insert"), |bencher| { + let mut world = World::default(); + let mut command_queue = CommandQueue::default(); + let mut entities = Vec::new(); + for i in 0..entity_count { + entities.push(world.spawn().id()); + } + + bencher.iter(|| { + let mut commands = Commands::new(&mut command_queue, &world); + for entity in entities.iter() { + commands.entity(*entity).insert_bundle((Matrix::default(), Vec3::default())); + } + drop(commands); + command_queue.apply(&mut world); + }); + }); + group.bench_function(format!("insert_batch"), |bencher| { + let mut world = World::default(); + let mut command_queue = CommandQueue::default(); + let mut entities = Vec::new(); + for i in 0..entity_count { + entities.push(world.spawn().id()); + } + + bencher.iter(|| { + let mut commands = Commands::new(&mut command_queue, &world); + let mut values = Vec::with_capacity(entity_count); + for entity in entities.iter() { + values.push((*entity, (Matrix::default(), Vec3::default()))); + } + commands.insert_or_spawn_batch(values); + drop(commands); + command_queue.apply(&mut world); + }); + }); + + group.finish(); +} + struct FakeCommandA; struct FakeCommandB(u64); @@ -106,7 +161,7 @@ fn fake_commands(criterion: &mut Criterion) { bencher.iter(|| { let mut commands = Commands::new(&mut command_queue, &world); for i in 0..command_count { - if black_box(i % 2 == 0) + if black_box(i % 2 == 0) { commands.add(FakeCommandA); } else { commands.add(FakeCommandB(0)); @@ -125,7 +180,7 @@ fn fake_commands(criterion: &mut Criterion) { struct SizedCommand(T); impl Command for SizedCommand { - fn write(self: Box, world: &mut World) { + fn write(self, world: &mut World) { black_box(self); black_box(world); } @@ -175,3 +230,41 @@ fn medium_sized_commands(criterion: &mut Criterion) { fn large_sized_commands(criterion: &mut Criterion) { sized_commands_impl::>(criterion); } + +fn get_or_spawn(criterion: &mut Criterion) { + let mut group = criterion.benchmark_group("get_or_spawn"); + group.warm_up_time(std::time::Duration::from_millis(500)); + group.measurement_time(std::time::Duration::from_secs(4)); + + group.bench_function("individual", |bencher| { + let mut world = World::default(); + let mut command_queue = CommandQueue::default(); + + bencher.iter(|| { + let mut commands = Commands::new(&mut command_queue, &world); + for i in 0..10_000 { + commands + .get_or_spawn(Entity::new(i)) + .insert_bundle((Matrix::default(), Vec3::default())); + } + command_queue.apply(&mut world); + }); + }); + + group.bench_function("batched", |bencher| { + let mut world = World::default(); + let mut command_queue = CommandQueue::default(); + + bencher.iter(|| { + let mut commands = Commands::new(&mut command_queue, &world); + let mut values = Vec::with_capacity(10_000); + for i in 0..10_000 { + values.push((Entity::new(i), (Matrix::default(), Vec3::default()))); + } + commands.insert_or_spawn_batch(values); + command_queue.apply(&mut world); + }); + }); + + group.finish(); +} diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 5a718bd6baf8d..b213660cd703d 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -25,6 +25,11 @@ impl ArchetypeId { ArchetypeId(0) } + #[inline] + pub const fn invalid() -> ArchetypeId { + ArchetypeId(usize::MAX) + } + #[inline] pub const fn resource() -> ArchetypeId { ArchetypeId(1) @@ -309,6 +314,11 @@ impl Archetype { .get(component_id) .map(|info| info.archetype_component_id) } + + pub(crate) fn clear_entities(&mut self) { + self.entities.clear(); + self.table_info.entity_rows.clear(); + } } /// A generational id that changes every time the set of archetypes changes @@ -519,6 +529,12 @@ impl Archetypes { pub fn archetype_components_len(&self) -> usize { self.archetype_component_count } + + pub fn clear_entities(&mut self) { + for archetype in self.archetypes.iter_mut() { + archetype.clear_entities(); + } + } } impl Index for Archetypes { diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index b911b89ebafa4..9136d220bc7ba 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1,10 +1,10 @@ pub use bevy_ecs_macros::Bundle; use crate::{ - archetype::ComponentStatus, + archetype::{AddBundle, Archetype, ArchetypeId, Archetypes, ComponentStatus}, component::{Component, ComponentId, ComponentTicks, Components, StorageType}, - entity::Entity, - storage::{SparseSetIndex, SparseSets, Table}, + entity::{Entities, Entity, EntityLocation}, + storage::{SparseSetIndex, SparseSets, Storages, Table}, }; use bevy_ecs_macros::all_tuples; use std::{any::TypeId, collections::HashMap}; @@ -122,19 +122,119 @@ pub struct BundleInfo { } impl BundleInfo { + #[inline] + pub fn id(&self) -> BundleId { + self.id + } + + #[inline] + pub fn components(&self) -> &[ComponentId] { + &self.component_ids + } + + #[inline] + pub fn storage_types(&self) -> &[StorageType] { + &self.storage_types + } + + pub(crate) fn get_bundle_inserter<'a, 'b>( + &'b self, + entities: &'a mut Entities, + archetypes: &'a mut Archetypes, + components: &mut Components, + storages: &'a mut Storages, + archetype_id: ArchetypeId, + change_tick: u32, + ) -> BundleInserter<'a, 'b> { + let new_archetype_id = + self.add_bundle_to_archetype(archetypes, storages, components, archetype_id); + let archetypes_ptr = archetypes.archetypes.as_mut_ptr(); + if new_archetype_id == archetype_id { + let archetype = &mut archetypes[archetype_id]; + let table_id = archetype.table_id(); + BundleInserter { + bundle_info: self, + archetype, + entities, + sparse_sets: &mut storages.sparse_sets, + table: &mut storages.tables[table_id], + archetypes_ptr, + change_tick, + result: InsertBundleResult::SameArchetype, + } + } else { + let (archetype, new_archetype) = archetypes.get_2_mut(archetype_id, new_archetype_id); + let table_id = archetype.table_id(); + if table_id == new_archetype.table_id() { + BundleInserter { + bundle_info: self, + archetype, + archetypes_ptr, + entities, + sparse_sets: &mut storages.sparse_sets, + table: &mut storages.tables[table_id], + change_tick, + result: InsertBundleResult::NewArchetypeSameTable { new_archetype }, + } + } else { + let (table, new_table) = storages + .tables + .get_2_mut(table_id, new_archetype.table_id()); + BundleInserter { + bundle_info: self, + archetype, + sparse_sets: &mut storages.sparse_sets, + entities, + archetypes_ptr, + table, + change_tick, + result: InsertBundleResult::NewArchetypeNewTable { + new_archetype, + new_table, + }, + } + } + } + } + + pub(crate) fn get_bundle_spawner<'a, 'b>( + &'b self, + entities: &'a mut Entities, + archetypes: &'a mut Archetypes, + components: &mut Components, + storages: &'a mut Storages, + change_tick: u32, + ) -> BundleSpawner<'a, 'b> { + let new_archetype_id = + self.add_bundle_to_archetype(archetypes, storages, components, ArchetypeId::empty()); + let (empty_archetype, archetype) = + archetypes.get_2_mut(ArchetypeId::empty(), new_archetype_id); + let table = &mut storages.tables[archetype.table_id()]; + let add_bundle = empty_archetype.edges().get_add_bundle(self.id()).unwrap(); + BundleSpawner { + archetype, + add_bundle, + bundle_info: self, + table, + entities, + sparse_sets: &mut storages.sparse_sets, + change_tick, + } + } + /// # Safety - /// table row must exist, entity must be valid - #[allow(clippy::too_many_arguments)] + /// `table` must be the "new" table for `entity`. `table_row` must have space allocated for the `entity`, `bundle` must match this BundleInfo's type #[inline] - pub(crate) unsafe fn write_components( + #[allow(clippy::too_many_arguments)] + unsafe fn write_components( &self, + table: &mut Table, sparse_sets: &mut SparseSets, + add_bundle: &AddBundle, entity: Entity, - table: &mut Table, table_row: usize, - bundle_status: &[ComponentStatus], - bundle: T, change_tick: u32, + bundle: T, ) { // NOTE: get_components calls this closure on each component in "bundle order". // bundle_info.component_ids are also in "bundle order" @@ -144,7 +244,7 @@ impl BundleInfo { match self.storage_types[bundle_component] { StorageType::Table => { let column = table.get_column_mut(component_id).unwrap(); - match bundle_status.get_unchecked(bundle_component) { + match add_bundle.bundle_status.get_unchecked(bundle_component) { ComponentStatus::Added => { column.initialize( table_row, @@ -166,19 +266,277 @@ impl BundleInfo { }); } + /// Adds a bundle to the given archetype and returns the resulting archetype. This could be the same + /// [ArchetypeId], in the event that adding the given bundle does not result in an Archetype change. + /// Results are cached in the Archetype Graph to avoid redundant work. + pub(crate) fn add_bundle_to_archetype( + &self, + archetypes: &mut Archetypes, + storages: &mut Storages, + components: &mut Components, + archetype_id: ArchetypeId, + ) -> ArchetypeId { + if let Some(add_bundle) = archetypes[archetype_id].edges().get_add_bundle(self.id) { + return add_bundle.archetype_id; + } + let mut new_table_components = Vec::new(); + let mut new_sparse_set_components = Vec::new(); + let mut bundle_status = Vec::with_capacity(self.component_ids.len()); + + let current_archetype = &mut archetypes[archetype_id]; + for component_id in self.component_ids.iter().cloned() { + if current_archetype.contains(component_id) { + bundle_status.push(ComponentStatus::Mutated); + } else { + bundle_status.push(ComponentStatus::Added); + // SAFE: component_id exists + let component_info = unsafe { components.get_info_unchecked(component_id) }; + match component_info.storage_type() { + StorageType::Table => new_table_components.push(component_id), + StorageType::SparseSet => { + storages.sparse_sets.get_or_insert(component_info); + new_sparse_set_components.push(component_id) + } + } + } + } + + if new_table_components.is_empty() && new_sparse_set_components.is_empty() { + let edges = current_archetype.edges_mut(); + // the archetype does not change when we add this bundle + edges.set_add_bundle(self.id, archetype_id, bundle_status); + archetype_id + } else { + let table_id; + let table_components; + let sparse_set_components; + // the archetype changes when we add this bundle. prepare the new archetype and storages + { + let current_archetype = &archetypes[archetype_id]; + table_components = if new_table_components.is_empty() { + // if there are no new table components, we can keep using this table + table_id = current_archetype.table_id(); + current_archetype.table_components().to_vec() + } else { + new_table_components.extend(current_archetype.table_components()); + // sort to ignore order while hashing + new_table_components.sort(); + // SAFE: all component ids in `new_table_components` exist + table_id = unsafe { + storages + .tables + .get_id_or_insert(&new_table_components, components) + }; + + new_table_components + }; + + sparse_set_components = if new_sparse_set_components.is_empty() { + current_archetype.sparse_set_components().to_vec() + } else { + new_sparse_set_components.extend(current_archetype.sparse_set_components()); + // sort to ignore order while hashing + new_sparse_set_components.sort(); + new_sparse_set_components + }; + }; + let new_archetype_id = + archetypes.get_id_or_insert(table_id, table_components, sparse_set_components); + // add an edge from the old archetype to the new archetype + archetypes[archetype_id].edges_mut().set_add_bundle( + self.id, + new_archetype_id, + bundle_status, + ); + new_archetype_id + } + } +} + +pub(crate) struct BundleInserter<'a, 'b> { + pub(crate) archetype: &'a mut Archetype, + pub(crate) entities: &'a mut Entities, + bundle_info: &'b BundleInfo, + table: &'a mut Table, + sparse_sets: &'a mut SparseSets, + result: InsertBundleResult<'a>, + archetypes_ptr: *mut Archetype, + change_tick: u32, +} + +pub(crate) enum InsertBundleResult<'a> { + SameArchetype, + NewArchetypeSameTable { + new_archetype: &'a mut Archetype, + }, + NewArchetypeNewTable { + new_archetype: &'a mut Archetype, + new_table: &'a mut Table, + }, +} + +impl<'a, 'b> BundleInserter<'a, 'b> { + /// # Safety + /// `entity` must currently exist in the source archetype for this inserter. `archetype_index` must be `entity`'s location in the archetype. + /// `T` must match this BundleInfo's type #[inline] - pub fn id(&self) -> BundleId { - self.id + pub unsafe fn insert( + &mut self, + entity: Entity, + archetype_index: usize, + 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) + let add_bundle = self + .archetype + .edges() + .get_add_bundle(self.bundle_info.id) + .unwrap(); + self.bundle_info.write_components( + self.table, + self.sparse_sets, + add_bundle, + entity, + self.archetype.entity_table_row(archetype_index), + self.change_tick, + bundle, + ); + location + } + InsertBundleResult::NewArchetypeSameTable { new_archetype } => { + let result = self.archetype.swap_remove(location.index); + if let Some(swapped_entity) = result.swapped_entity { + self.entities.meta[swapped_entity.id as usize].location = location; + } + let new_location = new_archetype.allocate(entity, result.table_row); + self.entities.meta[entity.id as usize].location = new_location; + + // PERF: this could be looked up during Inserter construction and stored (but borrowing makes this nasty) + let add_bundle = self + .archetype + .edges() + .get_add_bundle(self.bundle_info.id) + .unwrap(); + self.bundle_info.write_components( + self.table, + self.sparse_sets, + add_bundle, + entity, + result.table_row, + self.change_tick, + bundle, + ); + new_location + } + InsertBundleResult::NewArchetypeNewTable { + new_archetype, + new_table, + } => { + let result = self.archetype.swap_remove(location.index); + if let Some(swapped_entity) = result.swapped_entity { + self.entities.meta[swapped_entity.id as usize].location = location; + } + // PERF: store "non bundle" components in edge, then just move those to avoid + // redundant copies + let move_result = self + .table + .move_to_superset_unchecked(result.table_row, &mut *new_table); + let new_location = new_archetype.allocate(entity, move_result.new_row); + self.entities.meta[entity.id as usize].location = new_location; + + // if an entity was moved into this entity's table spot, update its table row + if let Some(swapped_entity) = move_result.swapped_entity { + let swapped_location = self.entities.get(swapped_entity).unwrap(); + let swapped_archetype = if self.archetype.id() == swapped_location.archetype_id + { + &mut *self.archetype + } else if new_archetype.id() == swapped_location.archetype_id { + &mut *new_archetype + } else { + // SAFE: the only two borrowed archetypes are above and we just did collision checks + &mut *self + .archetypes_ptr + .add(swapped_location.archetype_id.index()) + }; + + swapped_archetype + .set_entity_table_row(swapped_location.index, result.table_row); + } + + // PERF: this could be looked up during Inserter construction and stored (but borrowing makes this nasty) + let add_bundle = self + .archetype + .edges() + .get_add_bundle(self.bundle_info.id) + .unwrap(); + self.bundle_info.write_components( + new_table, + self.sparse_sets, + add_bundle, + entity, + move_result.new_row, + self.change_tick, + bundle, + ); + new_location + } + } } +} + +pub(crate) struct BundleSpawner<'a, 'b> { + pub(crate) archetype: &'a mut Archetype, + pub(crate) entities: &'a mut Entities, + add_bundle: &'a AddBundle, + bundle_info: &'b BundleInfo, + table: &'a mut Table, + sparse_sets: &'a mut SparseSets, + change_tick: u32, +} +impl<'a, 'b> BundleSpawner<'a, 'b> { + pub fn reserve_storage(&mut self, additional: usize) { + self.archetype.reserve(additional); + self.table.reserve(additional); + } + /// # Safety + /// `entity` must be allocated (but non existent), `T` must match this BundleInfo's type #[inline] - pub fn components(&self) -> &[ComponentId] { - &self.component_ids + pub unsafe fn spawn_non_existent( + &mut self, + entity: Entity, + bundle: T, + ) -> EntityLocation { + let table_row = self.table.allocate(entity); + let location = self.archetype.allocate(entity, table_row); + self.bundle_info.write_components( + self.table, + self.sparse_sets, + self.add_bundle, + entity, + table_row, + self.change_tick, + bundle, + ); + self.entities.meta[entity.id as usize].location = location; + + location } + /// # Safety + /// `T` must match this BundleInfo's type #[inline] - pub fn storage_types(&self) -> &[StorageType] { - &self.storage_types + pub unsafe fn spawn(&mut self, bundle: T) -> Entity { + let entity = self.entities.alloc(); + // SAFE: entity is allocated (but non-existent), `T` matches this BundleInfo's type + self.spawn_non_existent(entity, bundle); + entity } } diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index df8b3abb489d6..7be98eac0e621 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -26,6 +26,12 @@ pub struct Entity { pub(crate) id: u32, } +pub enum AllocAtWithoutReplacement { + Exists(EntityLocation), + DidNotExist, + ExistsWithWrongGeneration, +} + impl Entity { /// Creates a new entity reference with a generation of 0. pub fn new(id: u32) -> Entity { @@ -242,11 +248,8 @@ impl Entities { } /// Allocate an entity ID directly. - /// - /// Location should be written immediately. pub fn alloc(&mut self) -> Entity { self.verify_flushed(); - self.len += 1; if let Some(id) = self.pending.pop() { let new_free_cursor = self.pending.len() as i64; @@ -294,6 +297,40 @@ impl Entities { loc } + /// Allocate a specific entity ID, overwriting its generation. + /// + /// Returns the location of the entity currently using the given ID, if any. + pub fn alloc_at_without_replacement(&mut self, entity: Entity) -> AllocAtWithoutReplacement { + self.verify_flushed(); + + let result = if entity.id as usize >= self.meta.len() { + self.pending.extend((self.meta.len() as u32)..entity.id); + let new_free_cursor = self.pending.len() as i64; + *self.free_cursor.get_mut() = new_free_cursor; + self.meta.resize(entity.id as usize + 1, EntityMeta::EMPTY); + self.len += 1; + AllocAtWithoutReplacement::DidNotExist + } else if let Some(index) = self.pending.iter().position(|item| *item == entity.id) { + self.pending.swap_remove(index); + let new_free_cursor = self.pending.len() as i64; + *self.free_cursor.get_mut() = new_free_cursor; + self.len += 1; + AllocAtWithoutReplacement::DidNotExist + } else { + let current_meta = &mut self.meta[entity.id as usize]; + if current_meta.location.archetype_id == ArchetypeId::invalid() { + AllocAtWithoutReplacement::DidNotExist + } else if current_meta.generation == entity.generation { + AllocAtWithoutReplacement::Exists(current_meta.location) + } else { + return AllocAtWithoutReplacement::ExistsWithWrongGeneration; + } + }; + + self.meta[entity.id as usize].generation = entity.generation; + result + } + /// Destroy an entity, allowing it to be reused. /// /// Must not be called while reserved entities are awaiting `flush()`. @@ -339,27 +376,16 @@ impl Entities { self.meta.clear(); self.pending.clear(); *self.free_cursor.get_mut() = 0; + self.len = 0; } - /// Access the location storage of an entity. - /// - /// Must not be called on pending entities. - #[inline] - pub fn get_mut(&mut self, entity: Entity) -> Option<&mut EntityLocation> { - let meta = &mut self.meta[entity.id as usize]; - if meta.generation == entity.generation { - Some(&mut meta.location) - } else { - None - } - } - - /// Returns `Ok(Location { archetype: 0, index: undefined })` for pending entities. - #[inline] + /// Returns `Ok(Location { archetype: Archetype::invalid(), index: undefined })` for pending entities. pub fn get(&self, entity: Entity) -> Option { if (entity.id as usize) < self.meta.len() { let meta = &self.meta[entity.id as usize]; - if meta.generation != entity.generation { + if meta.generation != entity.generation + || meta.location.archetype_id == ArchetypeId::invalid() + { return None; } Some(meta.location) @@ -402,7 +428,12 @@ impl Entities { /// Allocates space for entities previously reserved with `reserve_entity` or /// `reserve_entities`, then initializes each one using the supplied function. - pub fn flush(&mut self, mut init: impl FnMut(Entity, &mut EntityLocation)) { + /// + /// # Safety + /// Flush _must_ set the entity location to the correct ArchetypeId for the given Entity + /// each time init is called. This _can_ be ArchetypeId::invalid(), provided the Entity has + /// not been assigned to an Archetype. + pub unsafe fn flush(&mut self, mut init: impl FnMut(Entity, &mut EntityLocation)) { let free_cursor = self.free_cursor.get_mut(); let current_free_cursor = *free_cursor; @@ -440,6 +471,16 @@ impl Entities { } } + // Flushes all reserved entities to an "invalid" state. Attempting to retrieve them will return None + // unless they are later populated with a valid archetype. + pub fn flush_as_invalid(&mut self) { + unsafe { + self.flush(|_entity, location| { + location.archetype_id = ArchetypeId::invalid(); + }) + } + } + #[inline] pub fn len(&self) -> u32 { self.len @@ -461,7 +502,7 @@ impl EntityMeta { const EMPTY: EntityMeta = EntityMeta { generation: 0, location: EntityLocation { - archetype_id: ArchetypeId::empty(), + archetype_id: ArchetypeId::invalid(), index: usize::max_value(), // dummy value, to be filled in }, }; @@ -494,7 +535,24 @@ mod tests { fn reserve_entity_len() { let mut e = Entities::default(); e.reserve_entity(); - e.flush(|_, _| {}); + unsafe { e.flush(|_, _| {}) }; assert_eq!(e.len(), 1); } + + #[test] + fn get_reserved_and_invalid() { + let mut entities = Entities::default(); + let e = entities.reserve_entity(); + assert!(entities.contains(e)); + assert!(entities.get(e).is_none()); + + unsafe { + entities.flush(|_entity, _location| { + // do nothing ... leaving entity location invalid + }) + }; + + assert!(entities.contains(e)); + assert!(entities.get(e).is_none()); + } } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 6d0b85ffbbe90..42357ec87c6a3 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -82,6 +82,10 @@ impl SparseArray { *value = Some(func()); value.as_mut().unwrap() } + + pub fn clear(&mut self) { + self.values.clear(); + } } #[derive(Debug)] @@ -102,6 +106,13 @@ impl ComponentSparseSet { } } + pub fn clear(&mut self) { + self.dense.clear(); + self.ticks.clear(); + self.entities.clear(); + self.sparse.clear(); + } + #[inline] pub fn len(&self) -> usize { self.dense.len() @@ -400,6 +411,12 @@ impl SparseSets { self.sets.get_mut(component_id) } + pub fn clear(&mut self) { + for set in self.sets.values_mut() { + set.clear(); + } + } + pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { for set in self.sets.values_mut() { set.check_change_ticks(change_tick); diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 3b015076132ec..60d41f25ad127 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -179,6 +179,11 @@ impl Column { self.ticks.get_unchecked(row).get() } + pub fn clear(&mut self) { + self.data.clear(); + self.ticks.clear(); + } + #[inline] pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { for component_ticks in &mut self.ticks { @@ -396,6 +401,13 @@ impl Table { pub fn iter(&self) -> impl Iterator { self.columns.values() } + + pub fn clear(&mut self) { + self.entities.clear(); + for column in self.columns.values_mut() { + column.clear(); + } + } } pub struct Tables { @@ -475,6 +487,16 @@ impl Tables { self.tables.iter() } + pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, Table> { + self.tables.iter_mut() + } + + pub fn clear(&mut self) { + for table in self.tables.iter_mut() { + table.clear(); + } + } + pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { for table in self.tables.iter_mut() { table.check_change_ticks(change_tick); diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 97304959fcdcb..00d582ce27437 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -58,6 +58,18 @@ impl<'w, 's> Commands<'w, 's> { } } + pub fn get_or_spawn<'a>(&'a mut self, entity: Entity) -> EntityCommands<'w, 's, 'a> { + self.add(GetOrSpawn { entity }); + EntityCommands { + entity, + commands: self, + } + } + + pub fn spawn_and_forget(&mut self, bundle: impl Bundle) { + self.queue.push(Spawn { bundle }) + } + /// Creates a new entity with the components contained in `bundle`. /// /// This returns an [`EntityCommands`] builder, which enables inserting more components and @@ -140,6 +152,17 @@ impl<'w, 's> Commands<'w, 's> { self.queue.push(SpawnBatch { bundles_iter }); } + /// For an iterator of (Entity, Bundle) pairs, inserts the Bundle into the Entity, + /// if the entity exists, and spawns the entity with the Bundle if it does not exist. + pub fn insert_or_spawn_batch(&mut self, bundles_iter: I) + where + I: IntoIterator + Send + Sync + 'static, + I::IntoIter: Iterator, + B: Bundle, + { + self.queue.push(InsertOrSpawnBatch { bundles_iter }); + } + /// See [`World::insert_resource`]. pub fn insert_resource(&mut self, resource: T) { self.queue.push(InsertResource { resource }) @@ -271,6 +294,16 @@ where } } +pub struct GetOrSpawn { + entity: Entity, +} + +impl Command for GetOrSpawn { + fn write(self, world: &mut World) { + world.get_or_spawn(self.entity); + } +} + pub struct SpawnBatch where I: IntoIterator, @@ -289,6 +322,26 @@ where } } +pub struct InsertOrSpawnBatch +where + I: IntoIterator + Send + Sync + 'static, + B: Bundle, + I::IntoIter: Iterator, +{ + pub bundles_iter: I, +} + +impl Command for InsertOrSpawnBatch +where + I: IntoIterator + Send + Sync + 'static, + B: Bundle, + I::IntoIter: Iterator, +{ + fn write(self, world: &mut World) { + world.insert_or_spawn_batch(self.bundles_iter); + } +} + #[derive(Debug)] pub struct Despawn { pub entity: Entity, diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 6e7185dc5b625..3034e07a8202a 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1,5 +1,5 @@ use crate::{ - archetype::{Archetype, ArchetypeId, Archetypes, ComponentStatus}, + archetype::{Archetype, ArchetypeId, Archetypes}, bundle::{Bundle, BundleInfo}, change_detection::Ticks, component::{Component, ComponentId, ComponentTicks, Components, StorageType}, @@ -189,112 +189,29 @@ impl<'w> EntityMut<'w> { }) } - /// # Safety: - /// Partially moves the entity to a new archetype based on the provided bundle info - /// You must handle the other part of moving the entity yourself - unsafe fn get_insert_bundle_info<'a>( - entities: &mut Entities, - archetypes: &'a mut Archetypes, - components: &mut Components, - storages: &mut Storages, - bundle_info: &BundleInfo, - current_location: EntityLocation, - entity: Entity, - ) -> (&'a Archetype, &'a Vec, EntityLocation) { - // SAFE: component ids in `bundle_info` and self.location are valid - let new_archetype_id = add_bundle_to_archetype( - archetypes, - storages, - components, - current_location.archetype_id, - bundle_info, - ); - if new_archetype_id == current_location.archetype_id { - let archetype = &archetypes[current_location.archetype_id]; - let edge = archetype.edges().get_add_bundle(bundle_info.id).unwrap(); - (archetype, &edge.bundle_status, current_location) - } else { - let (old_table_row, old_table_id) = { - let old_archetype = &mut archetypes[current_location.archetype_id]; - let result = old_archetype.swap_remove(current_location.index); - if let Some(swapped_entity) = result.swapped_entity { - entities.meta[swapped_entity.id as usize].location = current_location; - } - (result.table_row, old_archetype.table_id()) - }; - - let new_table_id = archetypes[new_archetype_id].table_id(); - - let new_location = if old_table_id == new_table_id { - archetypes[new_archetype_id].allocate(entity, old_table_row) - } else { - let (old_table, new_table) = storages.tables.get_2_mut(old_table_id, new_table_id); - // PERF: store "non bundle" components in edge, then just move those to avoid - // redundant copies - let move_result = old_table.move_to_superset_unchecked(old_table_row, new_table); - - let new_location = - archetypes[new_archetype_id].allocate(entity, move_result.new_row); - // if an entity was moved into this entity's table spot, update its table row - 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); - } - new_location - }; - - entities.meta[entity.id as usize].location = new_location; - let (old_archetype, new_archetype) = - archetypes.get_2_mut(current_location.archetype_id, new_archetype_id); - let edge = old_archetype - .edges() - .get_add_bundle(bundle_info.id) - .unwrap(); - (&*new_archetype, &edge.bundle_status, new_location) - - // Sparse set components are intentionally ignored here. They don't need to move - } - } - - // TODO: move relevant methods to World (add/remove bundle) pub fn insert_bundle(&mut self, bundle: T) -> &mut Self { let change_tick = self.world.change_tick(); let bundle_info = self .world .bundles .init_info::(&mut self.world.components); - - let (archetype, bundle_status, new_location) = unsafe { - Self::get_insert_bundle_info( - &mut self.world.entities, - &mut self.world.archetypes, - &mut self.world.components, - &mut self.world.storages, - bundle_info, - self.location, - self.entity, - ) - }; - self.location = new_location; - - let table = &mut self.world.storages.tables[archetype.table_id()]; - let table_row = archetype.entity_table_row(new_location.index); - // SAFE: table row is valid + let mut bundle_inserter = bundle_info.get_bundle_inserter( + &mut self.world.entities, + &mut self.world.archetypes, + &mut self.world.components, + &mut self.world.storages, + self.location.archetype_id, + change_tick, + ); + // SAFE: location matches current entity. `T` matches `bundle_info` unsafe { - bundle_info.write_components( - &mut self.world.storages.sparse_sets, - self.entity, - table, - table_row, - bundle_status, - bundle, - change_tick, - ) - }; + self.location = bundle_inserter.insert(self.entity, self.location.index, bundle); + } + self } + // TODO: move to BundleInfo pub fn remove_bundle(&mut self) -> Option { let archetypes = &mut self.world.archetypes; let storages = &mut self.world.storages; @@ -415,6 +332,7 @@ impl<'w> EntityMut<'w> { entities.meta[entity.id as usize].location = new_location; } + // TODO: move to BundleInfo /// Remove any components in the bundle that the entity has. pub fn remove_bundle_intersection(&mut self) { let archetypes = &mut self.world.archetypes; @@ -545,6 +463,7 @@ impl<'w> EntityMut<'w> { } } +// TODO: move to Storages? /// # Safety /// `entity_location` must be within bounds of the given archetype and `entity` must exist inside /// the archetype @@ -574,6 +493,7 @@ unsafe fn get_component( } } +// TODO: move to Storages? /// # Safety /// Caller must ensure that `component_id` is valid #[inline] @@ -604,6 +524,7 @@ unsafe fn get_component_and_ticks( } } +// TODO: move to Storages? /// Moves component data out of storage. /// /// This function leaves the underlying memory unchanged, but the component behind @@ -685,95 +606,6 @@ fn contains_component_with_id( world.archetypes[location.archetype_id].contains(component_id) } -/// Adds a bundle to the given archetype and returns the resulting archetype. This could be the same -/// [ArchetypeId], in the event that adding the given bundle does not result in an Archetype change. -/// Results are cached in the Archetype Graph to avoid redundant work. -/// -/// # Safety -/// components in `bundle_info` must exist -pub(crate) unsafe fn add_bundle_to_archetype( - archetypes: &mut Archetypes, - storages: &mut Storages, - components: &mut Components, - archetype_id: ArchetypeId, - bundle_info: &BundleInfo, -) -> ArchetypeId { - if let Some(add_bundle) = archetypes[archetype_id] - .edges() - .get_add_bundle(bundle_info.id) - { - return add_bundle.archetype_id; - } - let mut new_table_components = Vec::new(); - let mut new_sparse_set_components = Vec::new(); - let mut bundle_status = Vec::with_capacity(bundle_info.component_ids.len()); - - let current_archetype = &mut archetypes[archetype_id]; - for component_id in bundle_info.component_ids.iter().cloned() { - if current_archetype.contains(component_id) { - bundle_status.push(ComponentStatus::Mutated); - } else { - bundle_status.push(ComponentStatus::Added); - let component_info = components.get_info_unchecked(component_id); - match component_info.storage_type() { - StorageType::Table => new_table_components.push(component_id), - StorageType::SparseSet => { - storages.sparse_sets.get_or_insert(component_info); - new_sparse_set_components.push(component_id) - } - } - } - } - - if new_table_components.is_empty() && new_sparse_set_components.is_empty() { - let edges = current_archetype.edges_mut(); - // the archetype does not change when we add this bundle - edges.set_add_bundle(bundle_info.id, archetype_id, bundle_status); - archetype_id - } else { - let table_id; - let table_components; - let sparse_set_components; - // the archetype changes when we add this bundle. prepare the new archetype and storages - { - let current_archetype = &archetypes[archetype_id]; - table_components = if new_table_components.is_empty() { - // if there are no new table components, we can keep using this table - table_id = current_archetype.table_id(); - current_archetype.table_components().to_vec() - } else { - new_table_components.extend(current_archetype.table_components()); - // sort to ignore order while hashing - new_table_components.sort(); - // SAFE: all component ids in `new_table_components` exist - table_id = storages - .tables - .get_id_or_insert(&new_table_components, components); - - new_table_components - }; - - sparse_set_components = if new_sparse_set_components.is_empty() { - current_archetype.sparse_set_components().to_vec() - } else { - new_sparse_set_components.extend(current_archetype.sparse_set_components()); - // sort to ignore order while hashing - new_sparse_set_components.sort(); - new_sparse_set_components - }; - }; - let new_archetype_id = - archetypes.get_id_or_insert(table_id, table_components, sparse_set_components); - // add an edge from the old archetype to the new archetype - archetypes[archetype_id].edges_mut().set_add_bundle( - bundle_info.id, - new_archetype_id, - bundle_status, - ); - new_archetype_id - } -} - /// Removes a bundle from the given archetype and returns the resulting archetype (or None if the /// removal was invalid). in the event that adding the given bundle does not result in an Archetype /// change. Results are cached in the Archetype Graph to avoid redundant work. diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 862181538328b..24f67102c43a6 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -9,13 +9,13 @@ pub use world_cell::*; use crate::{ archetype::{ArchetypeComponentId, ArchetypeComponentInfo, ArchetypeId, Archetypes}, - bundle::{Bundle, Bundles}, + bundle::{Bundle, BundleInserter, BundleSpawner, Bundles}, change_detection::Ticks, component::{ Component, ComponentDescriptor, ComponentId, ComponentTicks, Components, ComponentsError, StorageType, }, - entity::{Entities, Entity}, + entity::{AllocAtWithoutReplacement, Entities, Entity}, query::{FilterFetch, QueryState, WorldQuery}, storage::{Column, SparseSet, Storages}, }; @@ -93,6 +93,12 @@ impl World { &self.entities } + /// Retrieves this world's [Entities] collection mutably + #[inline] + pub fn entities_mut(&mut self) -> &mut Entities { + &mut self.entities + } + /// Retrieves this world's [Archetypes] collection #[inline] pub fn archetypes(&self) -> &Archetypes { @@ -214,6 +220,24 @@ impl World { self.get_entity_mut(entity).expect("Entity does not exist") } + /// Returns an EntityMut for an existing entity or creates one if it doesn't exist. + /// This will return `None` if the entity exists with a different generation. + #[inline] + pub fn get_or_spawn(&mut self, entity: Entity) -> Option { + self.flush(); + match self.entities.alloc_at_without_replacement(entity) { + AllocAtWithoutReplacement::Exists(location) => { + // SAFE: `entity` exists and `location` is that entity's location + Some(unsafe { EntityMut::new(self, entity, location) }) + } + AllocAtWithoutReplacement::DidNotExist => { + // SAFE: entity was just allocated + Some(unsafe { self.spawn_at_internal(entity) }) + } + AllocAtWithoutReplacement::ExistsWithWrongGeneration => None, + } + } + /// Retrieves an [EntityRef] that exposes read-only operations for the given `entity`. /// Returns [None] if the `entity` does not exist. Use [World::entity] if you don't want /// to unwrap the [EntityRef] yourself. @@ -292,20 +316,25 @@ impl World { pub fn spawn(&mut self) -> EntityMut { self.flush(); let entity = self.entities.alloc(); + // SAFE: entity was just allocated + unsafe { self.spawn_at_internal(entity) } + } + + /// # Safety + /// must be called on an entity that was just allocated + unsafe fn spawn_at_internal(&mut self, entity: Entity) -> EntityMut { let archetype = self.archetypes.empty_mut(); - unsafe { - // PERF: consider avoiding allocating entities in the empty archetype unless needed - let table_row = self.storages.tables[archetype.table_id()].allocate(entity); - // SAFE: no components are allocated by archetype.allocate() because the archetype is - // empty - let location = archetype.allocate(entity, table_row); - // SAFE: entity index was just allocated - self.entities - .meta - .get_unchecked_mut(entity.id() as usize) - .location = location; - EntityMut::new(self, entity, location) - } + // PERF: consider avoiding allocating entities in the empty archetype unless needed + let table_row = self.storages.tables[archetype.table_id()].allocate(entity); + // SAFE: no components are allocated by archetype.allocate() because the archetype is + // empty + let location = archetype.allocate(entity, table_row); + // SAFE: entity index was just allocated + self.entities + .meta + .get_unchecked_mut(entity.id() as usize) + .location = location; + EntityMut::new(self, entity, location) } /// Spawns a batch of entities with the same component [Bundle] type. Takes a given [Bundle] @@ -669,6 +698,94 @@ impl World { self.get_non_send_unchecked_mut_with_id(component_id) } + pub fn insert_or_spawn_batch(&mut self, iter: I) + where + I: IntoIterator, + I::IntoIter: Iterator, + B: Bundle, + { + self.flush(); + + let iter = iter.into_iter(); + let change_tick = *self.change_tick.get_mut(); + + let bundle_info = self.bundles.init_info::(&mut self.components); + enum SpawnOrInsert<'a, 'b> { + Spawn(BundleSpawner<'a, 'b>), + Insert(BundleInserter<'a, 'b>, ArchetypeId), + } + + impl<'a, 'b> SpawnOrInsert<'a, 'b> { + fn entities(&mut self) -> &mut Entities { + match self { + SpawnOrInsert::Spawn(spawner) => spawner.entities, + SpawnOrInsert::Insert(inserter, _) => inserter.entities, + } + } + } + let mut spawn_or_insert = SpawnOrInsert::Spawn(bundle_info.get_bundle_spawner( + &mut self.entities, + &mut self.archetypes, + &mut self.components, + &mut self.storages, + change_tick, + )); + for (entity, bundle) in iter { + match spawn_or_insert + .entities() + .alloc_at_without_replacement(entity) + { + AllocAtWithoutReplacement::Exists(location) => { + match spawn_or_insert { + SpawnOrInsert::Insert(ref mut inserter, archetype) + if location.archetype_id == archetype => + { + // SAFE: `entity` is valid, `location` matches entity, bundle matches inserter + unsafe { inserter.insert(entity, location.index, bundle) }; + } + _ => { + let mut inserter = bundle_info.get_bundle_inserter( + &mut self.entities, + &mut self.archetypes, + &mut self.components, + &mut self.storages, + location.archetype_id, + change_tick, + ); + // SAFE: `entity` is valid, `location` matches entity, bundle matches inserter + unsafe { inserter.insert(entity, location.index, bundle) }; + spawn_or_insert = + SpawnOrInsert::Insert(inserter, location.archetype_id); + } + }; + } + AllocAtWithoutReplacement::DidNotExist => { + match spawn_or_insert { + SpawnOrInsert::Spawn(ref mut spawner) => { + // SAFE: `entity` is allocated (but non existent), bundle matches inserter + unsafe { spawner.spawn_non_existent(entity, bundle) }; + } + _ => { + let mut spawner = bundle_info.get_bundle_spawner( + &mut self.entities, + &mut self.archetypes, + &mut self.components, + &mut self.storages, + change_tick, + ); + // SAFE: `entity` is valid, `location` matches entity, bundle matches inserter + unsafe { spawner.spawn_non_existent(entity, bundle) }; + spawn_or_insert = SpawnOrInsert::Spawn(spawner); + } + }; + } + AllocAtWithoutReplacement::ExistsWithWrongGeneration => { + todo!("This should probably return an error!") + } + } + } + } + /// Temporarily removes the requested resource from this [World], then re-adds it before /// returning. This enables safe mutable access to a resource while still providing mutable /// world access @@ -877,6 +994,7 @@ impl World { unsafe { let table = &mut self.storages.tables[empty_archetype.table_id()]; // PERF: consider pre-allocating space for flushed entities + // SAFE: entity is set to a valid location self.entities.flush(|entity, location| { // SAFE: no components are allocated by archetype.allocate() because the archetype // is empty @@ -916,6 +1034,13 @@ impl World { column.check_change_ticks(change_tick); } } + + pub fn clear_entities(&mut self) { + self.storages.tables.clear(); + self.storages.sparse_sets.clear(); + self.archetypes.clear_entities(); + self.entities.clear(); + } } impl fmt::Debug for World { diff --git a/crates/bevy_ecs/src/world/spawn_batch.rs b/crates/bevy_ecs/src/world/spawn_batch.rs index 95830dc72b696..6f84557684633 100644 --- a/crates/bevy_ecs/src/world/spawn_batch.rs +++ b/crates/bevy_ecs/src/world/spawn_batch.rs @@ -1,9 +1,7 @@ use crate::{ - archetype::{Archetype, ArchetypeId, ComponentStatus}, - bundle::{Bundle, BundleInfo}, - entity::{Entities, Entity}, - storage::{SparseSets, Table}, - world::{add_bundle_to_archetype, World}, + bundle::{Bundle, BundleSpawner}, + entity::Entity, + world::World, }; pub struct SpawnBatchIter<'w, I> @@ -12,13 +10,7 @@ where I::Item: Bundle, { inner: I, - entities: &'w mut Entities, - archetype: &'w mut Archetype, - table: &'w mut Table, - sparse_sets: &'w mut SparseSets, - bundle_info: &'w BundleInfo, - bundle_status: &'w [ComponentStatus], - change_tick: u32, + spawner: BundleSpawner<'w, 'w>, } impl<'w, I> SpawnBatchIter<'w, I> @@ -33,40 +25,22 @@ where world.flush(); let (lower, upper) = iter.size_hint(); + let length = upper.unwrap_or(lower); let bundle_info = world.bundles.init_info::(&mut world.components); - - let length = upper.unwrap_or(lower); - // SAFE: empty archetype exists and bundle components were initialized above - let archetype_id = unsafe { - add_bundle_to_archetype( - &mut world.archetypes, - &mut world.storages, - &mut world.components, - ArchetypeId::empty(), - bundle_info, - ) - }; - let (empty_archetype, archetype) = world - .archetypes - .get_2_mut(ArchetypeId::empty(), archetype_id); - let table = &mut world.storages.tables[archetype.table_id()]; - archetype.reserve(length); - table.reserve(length); world.entities.reserve(length as u32); - let edge = empty_archetype - .edges() - .get_add_bundle(bundle_info.id()) - .unwrap(); + let mut spawner = bundle_info.get_bundle_spawner( + &mut world.entities, + &mut world.archetypes, + &mut world.components, + &mut world.storages, + *world.change_tick.get_mut(), + ); + spawner.reserve_storage(length); + Self { inner: iter, - entities: &mut world.entities, - archetype, - table, - sparse_sets: &mut world.storages.sparse_sets, - bundle_info, - change_tick: *world.change_tick.get_mut(), - bundle_status: &edge.bundle_status, + spawner, } } } @@ -90,24 +64,8 @@ where fn next(&mut self) -> Option { let bundle = self.inner.next()?; - let entity = self.entities.alloc(); - // SAFE: component values are immediately written to relevant storages (which have been - // allocated) - unsafe { - let table_row = self.table.allocate(entity); - let location = self.archetype.allocate(entity, table_row); - self.bundle_info.write_components( - self.sparse_sets, - entity, - self.table, - table_row, - self.bundle_status, - bundle, - self.change_tick, - ); - self.entities.meta[entity.id as usize].location = location; - } - Some(entity) + // SAFE: bundle matches spawner type + unsafe { Some(self.spawner.spawn(bundle)) } } fn size_hint(&self) -> (usize, Option) { From f6bf9eefe7d684f9c0a91ea93f5f06001a5dc865 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 17 Aug 2021 18:14:46 -0700 Subject: [PATCH 2/8] add tests --- crates/bevy_ecs/src/lib.rs | 206 +++++++++++++++++++++++++++++++++++++ 1 file changed, 206 insertions(+) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index cd474009a0d16..7263adff9b237 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -60,7 +60,9 @@ mod tests { #[derive(Debug, PartialEq, Eq)] struct A(usize); + #[derive(Debug, PartialEq, Eq)] struct B(usize); + #[derive(Debug, PartialEq, Eq)] struct C; #[derive(Clone, Debug)] @@ -1231,4 +1233,208 @@ mod tests { assert_eq!(dropped1.load(Ordering::Relaxed), 1); assert_eq!(dropped2.load(Ordering::Relaxed), 1); } + + #[test] + fn clear_entities() { + let mut world = World::default(); + world + .register_component(ComponentDescriptor::new::(StorageType::SparseSet)) + .unwrap(); + world.insert_resource::(0); + world.spawn().insert(1u32); + world.spawn().insert(1.0f32); + + let mut q1 = world.query::<&u32>(); + let mut q2 = world.query::<&f32>(); + + assert_eq!(q1.iter(&world).len(), 1); + assert_eq!(q2.iter(&world).len(), 1); + assert_eq!(world.entities().len(), 2); + + world.clear_entities(); + + assert_eq!( + q1.iter(&world).len(), + 0, + "world should not contain table components" + ); + assert_eq!( + q2.iter(&world).len(), + 0, + "world should not contain sparse set components" + ); + assert_eq!( + world.entities().len(), + 0, + "world should not have any entities" + ); + assert_eq!( + *world.get_resource::().unwrap(), + 0, + "world should still contain resources" + ); + } + + #[test] + fn reserve_entities_across_worlds() { + let mut world_a = World::default(); + let mut world_b = World::default(); + + let e1 = world_a.spawn().insert(1u32).id(); + let e2 = world_a.spawn().insert(2u32).id(); + let e3 = world_a.entities().reserve_entity(); + world_a.flush(); + + let world_a_max_entities = world_a.entities().meta.len(); + world_b + .entities + .reserve_entities(world_a_max_entities as u32); + world_b.entities.flush_as_invalid(); + + let e4 = world_b.spawn().insert(4u32).id(); + assert_eq!( + e4, + Entity { + generation: 0, + id: 3, + }, + "new entity is created immediately after world_a's max entity" + ); + assert!(world_b.get::(e1).is_none()); + assert!(world_b.get_entity(e1).is_none()); + + assert!(world_b.get::(e2).is_none()); + assert!(world_b.get_entity(e2).is_none()); + + assert!(world_b.get::(e3).is_none()); + assert!(world_b.get_entity(e3).is_none()); + + world_b.get_or_spawn(e1).unwrap().insert(1.0f32); + assert_eq!( + world_b.get::(e1), + Some(&1.0f32), + "spawning into 'world_a' entities works" + ); + + world_b.get_or_spawn(e4).unwrap().insert(4.0f32); + assert_eq!( + world_b.get::(e4), + Some(&4.0f32), + "spawning into existing `world_b` entities works" + ); + assert_eq!( + world_b.get::(e4), + Some(&4u32), + "spawning into existing `world_b` entities works" + ); + + let e4_mismatched_generation = Entity { + generation: 1, + id: 3, + }; + assert!( + world_b.get_or_spawn(e4_mismatched_generation).is_none(), + "attempting to spawn on top of an entity with a mismatched entity generation fails" + ); + assert_eq!( + world_b.get::(e4), + Some(&4.0f32), + "failed mismatched spawn doesn't change existing entity" + ); + assert_eq!( + world_b.get::(e4), + Some(&4u32), + "failed mismatched spawn doesn't change existing entity" + ); + + let high_non_existent_entity = Entity { + generation: 0, + id: 6, + }; + world_b + .get_or_spawn(high_non_existent_entity) + .unwrap() + .insert(10.0f32); + assert_eq!( + world_b.get::(high_non_existent_entity), + Some(&10.0f32), + "inserting into newly allocated high / non-continous entity id works" + ); + + let high_non_existent_but_reserved_entity = Entity { + generation: 0, + id: 5, + }; + assert!( + world_b.get_entity(high_non_existent_but_reserved_entity).is_none(), + "entities between high-newly allocated entity and continuous block of existing entities don't exist" + ); + + let reserved_entities = vec![ + world_b.entities().reserve_entity(), + world_b.entities().reserve_entity(), + world_b.entities().reserve_entity(), + world_b.entities().reserve_entity(), + ]; + + assert_eq!( + reserved_entities, + vec![ + Entity { + generation: 0, + id: 5 + }, + Entity { + generation: 0, + id: 4 + }, + Entity { + generation: 0, + id: 7, + }, + Entity { + generation: 0, + id: 8, + }, + ], + "space between original entities and high entities is used for new entity ids" + ); + } + + #[test] + fn insert_or_spawn_batch() { + let mut world = World::default(); + let e0 = world.spawn().insert(A(0)).id(); + let e1 = Entity::new(1); + + let values = vec![(e0, (B(0), C)), (e1, (B(1), C))]; + + world.insert_or_spawn_batch(values); + + assert_eq!( + world.get::(e0), + Some(&A(0)), + "existing component was preserved" + ); + assert_eq!( + world.get::(e0), + Some(&B(0)), + "pre-existing entity received correct B component" + ); + assert_eq!( + world.get::(e1), + Some(&B(1)), + "new entity was spawned and received correct B component" + ); + assert_eq!( + world.get::(e0), + Some(&C), + "pre-existing entity received C component" + ); + assert_eq!( + world.get::(e1), + Some(&C), + "new entity was spawned and received C component" + ); + } } From d53671f729715d9db20b483916c1739a15e375c4 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 17 Aug 2021 18:33:17 -0700 Subject: [PATCH 3/8] Document new methods --- crates/bevy_ecs/src/system/commands/mod.rs | 19 +++++++++++-- crates/bevy_ecs/src/world/mod.rs | 31 ++++++++++++++++++++-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 00d582ce27437..d527690a73578 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -58,6 +58,13 @@ impl<'w, 's> Commands<'w, 's> { } } + /// Returns an [EntityCommands] for the given `entity` (if it exists) or spawns one if it doesn't exist. + /// This will return [None] if the `entity` exists with a different generation. + /// + /// # Note + /// Spawning a specific `entity` value is rarely the right choice. Most apps should favor [`Commands::spawn`]. + /// This method should generally only be used for sharing entities across apps, and only when they have a + /// scheme worked out to share an ID space (which doesn't happen by default). pub fn get_or_spawn<'a>(&'a mut self, entity: Entity) -> EntityCommands<'w, 's, 'a> { self.add(GetOrSpawn { entity }); EntityCommands { @@ -66,6 +73,8 @@ impl<'w, 's> Commands<'w, 's> { } } + /// Spawns a [Bundle] without pre-allocating an [Entity]. The [Entity] will be allocated when + /// this [Command] is applied. pub fn spawn_and_forget(&mut self, bundle: impl Bundle) { self.queue.push(Spawn { bundle }) } @@ -152,8 +161,14 @@ impl<'w, 's> Commands<'w, 's> { self.queue.push(SpawnBatch { bundles_iter }); } - /// For an iterator of (Entity, Bundle) pairs, inserts the Bundle into the Entity, - /// if the entity exists, and spawns the entity with the Bundle if it does not exist. + /// For a given batch of ([Entity], [Bundle]) pairs, either spawns each [Entity] with the given + /// bundle (if the entity does not exist), or inserts the [Bundle] (if the entity already exists). + /// This is faster than doing equivalent operations one-by-one. + /// + /// # Note + /// Spawning a specific `entity` value is rarely the right choice. Most apps should use [`Commands::spawn_batch`]. + /// This method should generally only be used for sharing entities across apps, and only when they have a scheme + /// worked out to share an ID space (which doesn't happen by default). pub fn insert_or_spawn_batch(&mut self, bundles_iter: I) where I: IntoIterator + Send + Sync + 'static, diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 24f67102c43a6..c6f0de28cd62c 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -220,8 +220,13 @@ impl World { self.get_entity_mut(entity).expect("Entity does not exist") } - /// Returns an EntityMut for an existing entity or creates one if it doesn't exist. - /// This will return `None` if the entity exists with a different generation. + /// Returns an [EntityMut] for the given `entity` (if it exists) or spawns one if it doesn't exist. + /// This will return [None] if the `entity` exists with a different generation. + /// + /// # Note + /// Spawning a specific `entity` value is rarely the right choice. Most apps should favor [`World::spawn`]. + /// This method should generally only be used for sharing entities across apps, and only when they have a + /// scheme worked out to share an ID space (which doesn't happen by default). #[inline] pub fn get_or_spawn(&mut self, entity: Entity) -> Option { self.flush(); @@ -698,6 +703,28 @@ impl World { self.get_non_send_unchecked_mut_with_id(component_id) } + /// For a given batch of ([Entity], [Bundle]) pairs, either spawns each [Entity] with the given + /// bundle (if the entity does not exist), or inserts the [Bundle] (if the entity already exists). + /// This is faster than doing equivalent operations one-by-one. + /// + /// # Note + /// Spawning a specific `entity` value is rarely the right choice. Most apps should use [`World::spawn_batch`]. + /// This method should generally only be used for sharing entities across apps, and only when they have a scheme + /// worked out to share an ID space (which doesn't happen by default). + /// + /// ``` + /// use bevy_ecs::{entity::Entity, world::World}; + /// + /// let mut world = World::new(); + /// let e0 = world.spawn().id(); + /// let e1 = world.spawn().id(); + /// world.insert_or_spawn_batch(vec![ + /// (e0, ("a", 0.0)), // the first entity + /// (e1, ("b", 1.0)), // the second entity + /// ]); + /// + /// assert_eq!(world.get::(e0), Some(&0.0)); + /// ``` pub fn insert_or_spawn_batch(&mut self, iter: I) where I: IntoIterator, From 68fb4dfd71c1b2f582e9f7e8a52cc94db9629daa Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 17 Aug 2021 18:52:48 -0700 Subject: [PATCH 4/8] Replace const ArchetypeId methods with const values --- crates/bevy_ecs/src/archetype.rs | 29 ++++++++----------------- crates/bevy_ecs/src/bundle.rs | 4 ++-- crates/bevy_ecs/src/entity/mod.rs | 10 ++++----- crates/bevy_ecs/src/world/mod.rs | 2 +- crates/bevy_ecs/src/world/world_cell.rs | 2 +- 5 files changed, 18 insertions(+), 29 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index b213660cd703d..f4d236e81fd4e 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -15,26 +15,15 @@ use std::{ pub struct ArchetypeId(usize); impl ArchetypeId { + pub const EMPTY: ArchetypeId = ArchetypeId(0); + pub const RESOURCE: ArchetypeId = ArchetypeId(1); + pub const INVALID: ArchetypeId = ArchetypeId(usize::MAX); + #[inline] pub const fn new(index: usize) -> Self { ArchetypeId(index) } - #[inline] - pub const fn empty() -> ArchetypeId { - ArchetypeId(0) - } - - #[inline] - pub const fn invalid() -> ArchetypeId { - ArchetypeId(usize::MAX) - } - - #[inline] - pub const fn resource() -> ArchetypeId { - ArchetypeId(1) - } - #[inline] pub fn index(self) -> usize { self.0 @@ -387,7 +376,7 @@ impl Default for Archetypes { // adds the resource archetype. it is "special" in that it is inaccessible via a "hash", // which prevents entities from being added to it archetypes.archetypes.push(Archetype::new( - ArchetypeId::resource(), + ArchetypeId::RESOURCE, TableId::empty(), Cow::Owned(Vec::new()), Cow::Owned(Vec::new()), @@ -412,7 +401,7 @@ impl Archetypes { #[inline] pub fn empty(&self) -> &Archetype { // SAFE: empty archetype always exists - unsafe { self.archetypes.get_unchecked(ArchetypeId::empty().index()) } + unsafe { self.archetypes.get_unchecked(ArchetypeId::EMPTY.index()) } } #[inline] @@ -420,7 +409,7 @@ impl Archetypes { // SAFE: empty archetype always exists unsafe { self.archetypes - .get_unchecked_mut(ArchetypeId::empty().index()) + .get_unchecked_mut(ArchetypeId::EMPTY.index()) } } @@ -429,7 +418,7 @@ impl Archetypes { // SAFE: resource archetype always exists unsafe { self.archetypes - .get_unchecked(ArchetypeId::resource().index()) + .get_unchecked(ArchetypeId::RESOURCE.index()) } } @@ -438,7 +427,7 @@ impl Archetypes { // SAFE: resource archetype always exists unsafe { self.archetypes - .get_unchecked_mut(ArchetypeId::resource().index()) + .get_unchecked_mut(ArchetypeId::RESOURCE.index()) } } diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 9136d220bc7ba..4d712352b55c6 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -206,9 +206,9 @@ impl BundleInfo { change_tick: u32, ) -> BundleSpawner<'a, 'b> { let new_archetype_id = - self.add_bundle_to_archetype(archetypes, storages, components, ArchetypeId::empty()); + self.add_bundle_to_archetype(archetypes, storages, components, ArchetypeId::EMPTY); let (empty_archetype, archetype) = - archetypes.get_2_mut(ArchetypeId::empty(), new_archetype_id); + archetypes.get_2_mut(ArchetypeId::EMPTY, new_archetype_id); let table = &mut storages.tables[archetype.table_id()]; let add_bundle = empty_archetype.edges().get_add_bundle(self.id()).unwrap(); BundleSpawner { diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 7be98eac0e621..c4f339b6d1e73 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -318,7 +318,7 @@ impl Entities { AllocAtWithoutReplacement::DidNotExist } else { let current_meta = &mut self.meta[entity.id as usize]; - if current_meta.location.archetype_id == ArchetypeId::invalid() { + if current_meta.location.archetype_id == ArchetypeId::INVALID { AllocAtWithoutReplacement::DidNotExist } else if current_meta.generation == entity.generation { AllocAtWithoutReplacement::Exists(current_meta.location) @@ -384,7 +384,7 @@ impl Entities { if (entity.id as usize) < self.meta.len() { let meta = &self.meta[entity.id as usize]; if meta.generation != entity.generation - || meta.location.archetype_id == ArchetypeId::invalid() + || meta.location.archetype_id == ArchetypeId::INVALID { return None; } @@ -431,7 +431,7 @@ impl Entities { /// /// # Safety /// Flush _must_ set the entity location to the correct ArchetypeId for the given Entity - /// each time init is called. This _can_ be ArchetypeId::invalid(), provided the Entity has + /// each time init is called. This _can_ be ArchetypeId::INVALID, provided the Entity has /// not been assigned to an Archetype. pub unsafe fn flush(&mut self, mut init: impl FnMut(Entity, &mut EntityLocation)) { let free_cursor = self.free_cursor.get_mut(); @@ -476,7 +476,7 @@ impl Entities { pub fn flush_as_invalid(&mut self) { unsafe { self.flush(|_entity, location| { - location.archetype_id = ArchetypeId::invalid(); + location.archetype_id = ArchetypeId::INVALID; }) } } @@ -502,7 +502,7 @@ impl EntityMeta { const EMPTY: EntityMeta = EntityMeta { generation: 0, location: EntityLocation { - archetype_id: ArchetypeId::invalid(), + archetype_id: ArchetypeId::INVALID, index: usize::max_value(), // dummy value, to be filled in }, }; diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index c6f0de28cd62c..8c9b65f0d3593 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -952,7 +952,7 @@ impl World { let resource_archetype = self .archetypes .archetypes - .get_unchecked_mut(ArchetypeId::resource().index()); + .get_unchecked_mut(ArchetypeId::RESOURCE.index()); let resource_archetype_components = &mut resource_archetype.components; let archetype_component_count = &mut self.archetypes.archetype_component_count; let components = &self.components; diff --git a/crates/bevy_ecs/src/world/world_cell.rs b/crates/bevy_ecs/src/world/world_cell.rs index e128167d5610d..8ac21ac32654b 100644 --- a/crates/bevy_ecs/src/world/world_cell.rs +++ b/crates/bevy_ecs/src/world/world_cell.rs @@ -296,7 +296,7 @@ mod tests { .components .get_resource_id(TypeId::of::()) .unwrap(); - let resource_archetype = world.archetypes.get(ArchetypeId::resource()).unwrap(); + let resource_archetype = world.archetypes.get(ArchetypeId::RESOURCE).unwrap(); let u32_archetype_component_id = resource_archetype .get_archetype_component_id(u32_component_id) .unwrap(); From 2cc1655307e1022a22bf091dcd68e84cf5b1c76e Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 17 Aug 2021 19:13:04 -0700 Subject: [PATCH 5/8] fmt --- crates/bevy_ecs/src/archetype.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index f4d236e81fd4e..66dcbb948fba2 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -416,10 +416,7 @@ impl Archetypes { #[inline] pub fn resource(&self) -> &Archetype { // SAFE: resource archetype always exists - unsafe { - self.archetypes - .get_unchecked(ArchetypeId::RESOURCE.index()) - } + unsafe { self.archetypes.get_unchecked(ArchetypeId::RESOURCE.index()) } } #[inline] From 422885bb573cc8cb75ebecb13cfff7fc7f617587 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Wed, 18 Aug 2021 17:56:08 -0700 Subject: [PATCH 6/8] Make insert_or_spawn_batch fallible --- crates/bevy_ecs/src/lib.rs | 50 +++++++++++++++++++++- crates/bevy_ecs/src/system/commands/mod.rs | 10 ++++- crates/bevy_ecs/src/world/mod.rs | 15 ++++++- 3 files changed, 70 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 7263adff9b237..c51925b710569 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1409,7 +1409,55 @@ mod tests { let values = vec![(e0, (B(0), C)), (e1, (B(1), C))]; - world.insert_or_spawn_batch(values); + world.insert_or_spawn_batch(values).unwrap(); + + assert_eq!( + world.get::(e0), + Some(&A(0)), + "existing component was preserved" + ); + assert_eq!( + world.get::(e0), + Some(&B(0)), + "pre-existing entity received correct B component" + ); + assert_eq!( + world.get::(e1), + Some(&B(1)), + "new entity was spawned and received correct B component" + ); + assert_eq!( + world.get::(e0), + Some(&C), + "pre-existing entity received C component" + ); + assert_eq!( + world.get::(e1), + Some(&C), + "new entity was spawned and received C component" + ); + } + + #[test] + fn insert_or_spawn_batch_invalid() { + let mut world = World::default(); + let e0 = world.spawn().insert(A(0)).id(); + let e1 = Entity::new(1); + let e2 = world.spawn().id(); + let invalid_e2 = Entity { + generation: 1, + id: e2.id, + }; + + let values = vec![(e0, (B(0), C)), (e1, (B(1), C)), (invalid_e2, (B(2), C))]; + + let result = world.insert_or_spawn_batch(values); + + assert_eq!( + result, + Err(vec![invalid_e2]), + "e2 failed to be spawned or inserted into" + ); assert_eq!( world.get::(e0), diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index d527690a73578..5e2873a29a39a 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -6,7 +6,7 @@ use crate::{ entity::{Entities, Entity}, world::World, }; -use bevy_utils::tracing::debug; +use bevy_utils::tracing::{debug, error}; pub use command_queue::CommandQueue; use std::marker::PhantomData; @@ -353,7 +353,13 @@ where I::IntoIter: Iterator, { fn write(self, world: &mut World) { - world.insert_or_spawn_batch(self.bundles_iter); + if let Err(invalid_entities) = world.insert_or_spawn_batch(self.bundles_iter) { + error!( + "Failed to 'insert or spawn' bundle of type {} into the following invalid entities: {:?}", + std::any::type_name::(), + invalid_entities + ); + } } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 8c9b65f0d3593..121951dddaa02 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -706,6 +706,9 @@ impl World { /// For a given batch of ([Entity], [Bundle]) pairs, either spawns each [Entity] with the given /// bundle (if the entity does not exist), or inserts the [Bundle] (if the entity already exists). /// This is faster than doing equivalent operations one-by-one. + /// Returns [Ok] if all entities were successfully inserted into or spawned. Otherwise it returns an [Err] + /// with a list of entities that could not be spawned or inserted into. A "spawn or insert" operation can + /// only fail if an [Entity] is passed in with an "invalid generation" that conflicts with an existing [Entity]. /// /// # Note /// Spawning a specific `entity` value is rarely the right choice. Most apps should use [`World::spawn_batch`]. @@ -725,7 +728,7 @@ impl World { /// /// assert_eq!(world.get::(e0), Some(&0.0)); /// ``` - pub fn insert_or_spawn_batch(&mut self, iter: I) + pub fn insert_or_spawn_batch(&mut self, iter: I) -> Result<(), Vec> where I: IntoIterator, I::IntoIter: Iterator, @@ -757,6 +760,8 @@ impl World { &mut self.storages, change_tick, )); + + let mut invalid_entities = Vec::new(); for (entity, bundle) in iter { match spawn_or_insert .entities() @@ -807,10 +812,16 @@ impl World { }; } AllocAtWithoutReplacement::ExistsWithWrongGeneration => { - todo!("This should probably return an error!") + invalid_entities.push(entity); } } } + + if invalid_entities.is_empty() { + Ok(()) + } else { + Err(invalid_entities) + } } /// Temporarily removes the requested resource from this [World], then re-adds it before From 05d001e618d39a68a49b3b2e2fb6ee116210e08f Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Wed, 18 Aug 2021 17:57:51 -0700 Subject: [PATCH 7/8] rename set_X to insert_X --- crates/bevy_ecs/src/archetype.rs | 6 +++--- crates/bevy_ecs/src/bundle.rs | 4 ++-- crates/bevy_ecs/src/world/entity_ref.rs | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 66dcbb948fba2..3d8aadbd132d6 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -54,7 +54,7 @@ impl Edges { } #[inline] - pub fn set_add_bundle( + pub fn insert_add_bundle( &mut self, bundle_id: BundleId, archetype_id: ArchetypeId, @@ -75,7 +75,7 @@ impl Edges { } #[inline] - pub fn set_remove_bundle(&mut self, bundle_id: BundleId, archetype_id: Option) { + pub fn insert_remove_bundle(&mut self, bundle_id: BundleId, archetype_id: Option) { self.remove_bundle.insert(bundle_id, archetype_id); } @@ -88,7 +88,7 @@ impl Edges { } #[inline] - pub fn set_remove_bundle_intersection( + pub fn insert_remove_bundle_intersection( &mut self, bundle_id: BundleId, archetype_id: Option, diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 4d712352b55c6..432a3d11e66cb 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -304,7 +304,7 @@ impl BundleInfo { if new_table_components.is_empty() && new_sparse_set_components.is_empty() { let edges = current_archetype.edges_mut(); // the archetype does not change when we add this bundle - edges.set_add_bundle(self.id, archetype_id, bundle_status); + edges.insert_add_bundle(self.id, archetype_id, bundle_status); archetype_id } else { let table_id; @@ -343,7 +343,7 @@ impl BundleInfo { let new_archetype_id = archetypes.get_id_or_insert(table_id, table_components, sparse_set_components); // add an edge from the old archetype to the new archetype - archetypes[archetype_id].edges_mut().set_add_bundle( + archetypes[archetype_id].edges_mut().insert_add_bundle( self.id, new_archetype_id, bundle_status, diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 3034e07a8202a..c2bae02ca01da 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -660,7 +660,7 @@ unsafe fn remove_bundle_from_archetype( // graph current_archetype .edges_mut() - .set_remove_bundle(bundle_info.id, None); + .insert_remove_bundle(bundle_info.id, None); return None; } } @@ -699,11 +699,11 @@ unsafe fn remove_bundle_from_archetype( if intersection { current_archetype .edges_mut() - .set_remove_bundle_intersection(bundle_info.id, result); + .insert_remove_bundle_intersection(bundle_info.id, result); } else { current_archetype .edges_mut() - .set_remove_bundle(bundle_info.id, result); + .insert_remove_bundle(bundle_info.id, result); } result } From 1f1cfcb5363048aec72e08a1959267e24db0308b Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Wed, 18 Aug 2021 17:59:52 -0700 Subject: [PATCH 8/8] add note to Entity::new --- crates/bevy_ecs/src/entity/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index c4f339b6d1e73..350d83d86c699 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -34,6 +34,11 @@ pub enum AllocAtWithoutReplacement { impl Entity { /// Creates a new entity reference with a generation of 0. + /// + /// # Note + /// Spawning a specific `entity` value is rarely the right choice. Most apps should favor [`Commands::spawn`]. + /// This method should generally only be used for sharing entities across apps, and only when they have a + /// scheme worked out to share an ID space (which doesn't happen by default). pub fn new(id: u32) -> Entity { Entity { id, generation: 0 } }