diff --git a/_release-content/release-notes/resources_as_components.md b/_release-content/release-notes/resources_as_components.md index e4652ce4d97b5..bf33a15282f76 100644 --- a/_release-content/release-notes/resources_as_components.md +++ b/_release-content/release-notes/resources_as_components.md @@ -1,7 +1,7 @@ --- title: Resources as Components -authors: ["@Trashtalk", "@cart"] -pull_requests: [20934, 22910, 22911, 22919, 22930] +authors: ["@Trashtalk", "@cart", "@specificprotagonist"] +pull_requests: [20934, 22910, 22911, 22919, 22930, 24058] --- Resources are very similar to Components: they are both data that can be stored in the ECS and queried. diff --git a/benches/benches/bevy_ecs/main.rs b/benches/benches/bevy_ecs/main.rs index ec3447549f0cb..bce529de8f5ea 100644 --- a/benches/benches/bevy_ecs/main.rs +++ b/benches/benches/bevy_ecs/main.rs @@ -23,13 +23,13 @@ criterion_main!( bundles::benches, change_detection::benches, components::benches, + resources::benches, empty_archetypes::benches, entity_cloning::benches, events::benches, iteration::benches, fragmentation::benches, observers::benches, - resources::benches, scheduling::benches, world::benches, param::benches, diff --git a/crates/bevy_ecs/macro_logic/src/component.rs b/crates/bevy_ecs/macro_logic/src/component.rs index f0546b7d8c8da..eeb82523bba03 100644 --- a/crates/bevy_ecs/macro_logic/src/component.rs +++ b/crates/bevy_ecs/macro_logic/src/component.rs @@ -562,6 +562,8 @@ pub enum StorageTy { Table, /// Sparse set storage SparseSet, + /// Resource, not choosable from component derive macro + Resource, } /// Derived required component from the `#[require]` attribute. @@ -654,6 +656,7 @@ fn storage_path(bevy_ecs_path: &Path, ty: StorageTy) -> TokenStream { let storage_type = match ty { StorageTy::Table => Ident::new("Table", Span::call_site()), StorageTy::SparseSet => Ident::new("SparseSet", Span::call_site()), + StorageTy::Resource => Ident::new("Resource", Span::call_site()), }; quote! { #bevy_ecs_path::component::StorageType::#storage_type } diff --git a/crates/bevy_ecs/macros/src/resource.rs b/crates/bevy_ecs/macros/src/resource.rs index 406c24e3b323c..a89a23cb5ca77 100644 --- a/crates/bevy_ecs/macros/src/resource.rs +++ b/crates/bevy_ecs/macros/src/resource.rs @@ -10,7 +10,7 @@ pub fn derive_resource(ast: &mut DeriveInput) -> TokenStream { Ok(value) => value, Err(e) => return e.into_compile_error(), }; - derive_component.storage = StorageTy::SparseSet; + derive_component.storage = StorageTy::Resource; let struct_name = &ast.ident; let (_, type_generics, _) = &ast.generics.split_for_impl(); diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 64ca9740567e6..820e6badb7d04 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -390,7 +390,7 @@ pub struct Archetype { } impl Archetype { - /// `table_components` and `sparse_set_components` must be sorted + /// `table_components` must be sorted pub(crate) fn new( components: &Components, component_index: &mut ComponentIndex, @@ -398,12 +398,12 @@ impl Archetype { id: ArchetypeId, table_id: TableId, table_components: impl Iterator, - sparse_set_components: impl Iterator, + non_table_components: impl Iterator, ) -> Self { let (min_table, _) = table_components.size_hint(); - let (min_sparse, _) = sparse_set_components.size_hint(); + let (min_non_table, _) = non_table_components.size_hint(); let mut flags = ArchetypeFlags::empty(); - let mut archetype_components = SparseSet::with_capacity(min_table + min_sparse); + let mut archetype_components = SparseSet::with_capacity(min_table + min_non_table); for (idx, component_id) in table_components.enumerate() { // SAFETY: We are creating an archetype that includes this component so it must exist let info = unsafe { components.get_info_unchecked(component_id) }; @@ -424,7 +424,7 @@ impl Archetype { .insert(id, ArchetypeRecord { column: Some(idx) }); } - for component_id in sparse_set_components { + for component_id in non_table_components { // SAFETY: We are creating an archetype that includes this component so it must exist let info = unsafe { components.get_info_unchecked(component_id) }; info.update_archetype_flags(&mut flags); @@ -432,7 +432,7 @@ impl Archetype { archetype_components.insert( component_id, ArchetypeComponentInfo { - storage_type: StorageType::SparseSet, + storage_type: info.storage_type(), }, ); component_index @@ -440,6 +440,7 @@ impl Archetype { .or_default() .insert(id, ArchetypeRecord { column: None }); } + Self { id, table_id, @@ -510,6 +511,19 @@ impl Archetype { .map(|(id, _)| *id) } + /// Gets an iterator of all of the components not stored in [`Table`]s. + /// + /// All of the IDs are unique. + /// + /// [`Table`]: crate::storage::Table + #[inline] + pub fn non_table_components(&self) -> impl Iterator + '_ { + self.components + .iter() + .filter(|(_, component)| component.storage_type != StorageType::Table) + .map(|(id, _)| *id) + } + /// Gets an iterator of all of the components stored in [`ComponentSparseSet`]s. /// /// All of the IDs are unique. @@ -523,6 +537,19 @@ impl Archetype { .map(|(id, _)| *id) } + /// Gets an iterator of all of the components stored in [`ResourceStorages`]. + /// + /// All of the IDs are unique. + /// + /// [`ResourceStorages`]: crate::storage::ResourceStorages + #[inline] + pub fn resource_components(&self) -> impl Iterator + '_ { + self.components + .iter() + .filter(|(_, component)| component.storage_type == StorageType::Resource) + .map(|(id, _)| *id) + } + /// Returns a slice of all of the components in the archetype. /// /// All of the IDs are unique. @@ -754,10 +781,12 @@ impl ArchetypeGeneration { } } +/// Components must be sorted +/// (which allows `ArchetypeComponents` to be used as an archetype's identity). #[derive(Hash, PartialEq, Eq)] struct ArchetypeComponents { table_components: Box<[ComponentId]>, - sparse_set_components: Box<[ComponentId]>, + non_table_components: Box<[ComponentId]>, } /// Maps a [`ComponentId`] to the list of [`Archetypes`]([`Archetype`]) that contain the [`Component`](crate::component::Component), @@ -856,6 +885,37 @@ impl Archetypes { self.archetypes.get(id.index()) } + /// # Safety + /// - all ids must be valid and pairwise unequal + pub(crate) unsafe fn get_disjoint_unchecked_mut( + &mut self, + id_a: ArchetypeId, + id_b: ArchetypeId, + id_c: Option, + ) -> (&mut Archetype, &mut Archetype, Option<&mut Archetype>) { + match id_c { + Some(id_c) => { + // SAFETY: Same preconditions + let [a, b, c] = unsafe { + self.archetypes.get_disjoint_unchecked_mut([ + id_a.index(), + id_b.index(), + id_c.index(), + ]) + }; + (a, b, Some(c)) + } + None => { + // SAFETY: Same preconditions + let [a, b] = unsafe { + self.archetypes + .get_disjoint_unchecked_mut([id_a.index(), id_b.index()]) + }; + (a, b, None) + } + } + } + /// Tries to fetch mutable references to two disjoint archetypes. /// /// Returns `(&mut Archetype, None)` if the same [`ArchetypeId`] was provided twice. @@ -896,21 +956,21 @@ impl Archetypes { /// Specifically, it returns a tuple where the first element /// is the [`ArchetypeId`] that the given inputs belong to, and the second element is a boolean indicating whether a new archetype was created. /// - /// `table_components` and `sparse_set_components` must be sorted + /// `table_components` and `non_table_components` must be sorted /// /// # Safety /// [`TableId`] must exist in tables - /// `table_components` and `sparse_set_components` must exist in `components` + /// `table_components` and `non_table_components` must exist in `components` pub(crate) unsafe fn get_id_or_insert( &mut self, components: &Components, observers: &Observers, table_id: TableId, table_components: Vec, - sparse_set_components: Vec, + non_table_components: Vec, ) -> (ArchetypeId, bool) { let archetype_identity = ArchetypeComponents { - sparse_set_components: sparse_set_components.into_boxed_slice(), + non_table_components: non_table_components.into_boxed_slice(), table_components: table_components.into_boxed_slice(), }; @@ -921,7 +981,7 @@ impl Archetypes { Entry::Vacant(vacant) => { let ArchetypeComponents { table_components, - sparse_set_components, + non_table_components, } = vacant.key(); let id = ArchetypeId::new(archetypes.len()); archetypes.push(Archetype::new( @@ -931,7 +991,7 @@ impl Archetypes { id, table_id, table_components.iter().copied(), - sparse_set_components.iter().copied(), + non_table_components.iter().copied(), )); vacant.insert(id); (id, true) diff --git a/crates/bevy_ecs/src/bundle/info.rs b/crates/bevy_ecs/src/bundle/info.rs index 7a278e3c4d627..49ee84f491d38 100644 --- a/crates/bevy_ecs/src/bundle/info.rs +++ b/crates/bevy_ecs/src/bundle/info.rs @@ -17,7 +17,11 @@ use crate::{ }, entity::Entity, query::DebugCheckedUnwrap as _, - storage::{SparseSetIndex, SparseSets, Storages, Table, TableRow}, + resource::IsResource, + storage::{ + ResourceStorage, ResourceStorages, SparseSetIndex, SparseSets, Storages, Table, TableRow, + }, + world::{unsafe_world_cell::UnsafeWorldCell, World}, }; /// For a specific [`World`], this stores a unique value identifying a type of a registered [`Bundle`]. @@ -75,6 +79,9 @@ pub struct BundleInfo { /// The list of constructors for all required components indirectly contributed by this bundle. pub(super) required_component_constructors: Box<[RequiredComponentConstructor]>, + + /// Whether any of the components are resources. + pub(super) contains_resources: bool, } impl BundleInfo { @@ -143,6 +150,11 @@ impl BundleInfo { .map(|(_, required_component)| required_component.constructor) .collect::>(); + let contains_resources = component_ids.iter().any(|&id| + + // SAFETY: caller has verified that all ids are valid + unsafe{components.get_descriptor(id).unwrap_unchecked()}.storage_type() == StorageType::Resource); + // SAFETY: The caller ensures that component_ids: // - is valid for the associated world // - has had its storage initialized @@ -151,6 +163,7 @@ impl BundleInfo { id, contributed_component_ids: component_ids.into(), required_component_constructors: required_components, + contains_resources, } } @@ -240,6 +253,7 @@ impl BundleInfo { &self, table: &mut Table, sparse_sets: &mut SparseSets, + resource_storages: &mut ResourceStorages, bundle_component_status: &S, required_components: impl Iterator, entity: Entity, @@ -294,6 +308,25 @@ impl BundleInfo { } } } + StorageType::Resource => { + let resource_storage = + // SAFETY: If component_id is in self.component_ids, BundleInfo::new ensures that + // a resource storage exists for the component. + unsafe { resource_storages.get_mut(component_id).debug_checked_unwrap() }; + match (status, insert_mode) { + (ComponentStatus::Added, _) | (_, InsertMode::Replace) => { + // Try to insert the resource. + // If the resource already exists on another entity this will fail, + // but in that case we've already adjusted the target archetype. + resource_storage.insert(entity, component_ptr, change_tick, caller); + } + (ComponentStatus::Existing, InsertMode::Keep) => { + if let Some(drop_fn) = resource_storage.get_drop() { + drop_fn(component_ptr); + } + } + } + } } bundle_component += 1; }); @@ -302,6 +335,7 @@ impl BundleInfo { required_component.initialize( table, sparse_sets, + resource_storages, change_tick, table_row, entity, @@ -325,6 +359,7 @@ impl BundleInfo { pub(crate) unsafe fn initialize_required_component( table: &mut Table, sparse_sets: &mut SparseSets, + resource_storages: &mut ResourceStorages, change_tick: Tick, table_row: TableRow, entity: Entity, @@ -349,6 +384,13 @@ impl BundleInfo { unsafe { sparse_sets.get_mut(component_id).debug_checked_unwrap() }; sparse_set.insert(entity, component_ptr, change_tick, caller); } + StorageType::Resource => { + let resource_storage= + // SAFETY: If component_id is in required_components, BundleInfo::new requires that + // a resource storage exists for the component. + unsafe { resource_storages.get_mut(component_id).debug_checked_unwrap() }; + resource_storage.insert(entity, component_ptr, change_tick, caller); + } } } } @@ -600,3 +642,85 @@ fn initialize_dynamic_bundle( (id, storage_types) } + +/// Checks whether any of the component insertions will fail +/// due to being resources that are already present on a different +/// entity in the world and returns the actual new archetype. +/// +/// This may invalidate archetype pointers, in which case it will update +/// `archetype` and `new_archetype` to be valid again. +/// +/// # Safety +/// - `archetype` and `new_archetype` must be valid for reading +/// - world must have write access to archetypes after `archetype` and +/// `new_archetype` are not life anymore, and read access to resources +pub(super) unsafe fn find_archetype_after_fallible_resource_write_and_queue_cleanup( + world: &UnsafeWorldCell, + entity: Entity, + component_ids_to_check: &[ComponentId], + mut archetype: Option<&mut NonNull>, + new_archetype: &mut NonNull, +) -> NonNull { + let mut resulting_archetype = *new_archetype; + for &component_id in component_ids_to_check { + if world + .storages() + .resources + .get(component_id) + .and_then(ResourceStorage::entity) + .is_some_and(|existing| existing != entity) + { + // Writing this resource will fail + + let archetype_id = archetype.as_ref().map(|a| a.as_ref().id()); + let new_archetype_id = new_archetype.as_ref().id(); + + let resulting_archetype_ref = resulting_archetype.as_ref(); + let table_id = resulting_archetype_ref.table_id(); + let table_components = resulting_archetype_ref.table_components().collect(); + let non_table_components = resulting_archetype_ref + .non_table_components() + .filter(|&id| id == component_id) + .collect(); + let (new_archetype_without_resource, _) = world.archetypes_mut().get_id_or_insert( + world.components(), + world.observers(), + table_id, + table_components, + non_table_components, + ); + // The previous archetype pointers are now invalid + // SAFETY: + // - archetype id came from a valid archetype above + // - world reference not used anymore + unsafe { + let (new_archetype_ref, resulting_archetype_ref, relocated_archetype_ref) = + world.archetypes_mut().get_disjoint_unchecked_mut( + new_archetype_id, + new_archetype_without_resource, + archetype_id, + ); + if let Some(archetype) = &mut archetype { + **archetype = NonNull::from(relocated_archetype_ref.unwrap_unchecked()); + } + *new_archetype = NonNull::from(new_archetype_ref); + resulting_archetype = NonNull::from(resulting_archetype_ref); + } + + // The resource will not be inserted, but required components will still be, + // so `IsResource` needs to be removed. + world + .get_raw_command_queue() + .push(move |world: &mut World| { + if let Ok(mut entity) = world.get_entity_mut(entity) + && entity.get::().is_some_and(|is_resource| { + is_resource.resource_component_id() == component_id + }) + { + entity.remove::(); + } + }); + } + } + resulting_archetype +} diff --git a/crates/bevy_ecs/src/bundle/insert.rs b/crates/bevy_ecs/src/bundle/insert.rs index d1b8200089314..b1de5628dbdcb 100644 --- a/crates/bevy_ecs/src/bundle/insert.rs +++ b/crates/bevy_ecs/src/bundle/insert.rs @@ -7,7 +7,10 @@ use crate::{ Archetype, ArchetypeAfterBundleInsert, ArchetypeCreated, ArchetypeId, Archetypes, ComponentStatus, }, - bundle::{ArchetypeMoveType, Bundle, BundleId, BundleInfo, DynamicBundle, InsertMode}, + bundle::{ + find_archetype_after_fallible_resource_write_and_queue_cleanup, ArchetypeMoveType, Bundle, + BundleId, BundleInfo, DynamicBundle, InsertMode, + }, change_detection::{MaybeLocation, Tick}, component::{Components, StorageType}, entity::{Entities, Entity, EntityLocation}, @@ -16,7 +19,7 @@ use crate::{ observer::Observers, query::DebugCheckedUnwrap as _, relationship::RelationshipHookMode, - storage::{SparseSets, Storages, Table, TableRow}, + storage::{ResourceStorages, SparseSets, Storages, Table, TableRow}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; @@ -128,14 +131,16 @@ impl<'w> BundleInserter<'w> { insert_mode: InsertMode, caller: MaybeLocation, relationship_hook_mode: RelationshipHookMode, - mut archetype: NonNull, + archetype: &mut NonNull, archetype_after_insert: &ArchetypeAfterBundleInsert, world: &'a UnsafeWorldCell<'w>, archetype_move_type: &'a mut ArchetypeMoveType, + contains_resources: bool, ) -> ( &'a Archetype, EntityLocation, &'a mut SparseSets, + &'a mut ResourceStorages, &'a mut Table, TableRow, ) { @@ -177,43 +182,59 @@ impl<'w> BundleInserter<'w> { } } - // SAFETY: Archetype gets borrowed when running the on_discard observers above, - // so this reference can only be promoted from shared to &mut down here, after they have been ran - let archetype = archetype.as_mut(); - match archetype_move_type { ArchetypeMoveType::SameArchetype => { + let archetype_ref = archetype.as_ref(); + // SAFETY: Mutable references do not alias and will be dropped after this block - let (sparse_sets, table) = { + let (sparse_sets, resource_storages, table) = { let world = world.world_mut(); ( &mut world.storages.sparse_sets, - &mut world.storages.tables[archetype.table_id()], + &mut world.storages.resources, + &mut world.storages.tables[archetype_ref.table_id()], ) }; ( - &*archetype, + archetype_ref, location, sparse_sets, + resource_storages, table, location.table_row, ) } ArchetypeMoveType::NewArchetypeSameTable { new_archetype } => { + // Inserting a resource will fail if it already exists on another entity. + // Check whether this is the case and determine the correct resulting archetype if so. + let mut new_archetype = if contains_resources { + find_archetype_after_fallible_resource_write_and_queue_cleanup( + world, + entity, + archetype_after_insert.added(), + Some(archetype), + new_archetype, + ) + } else { + *new_archetype + }; + // SAFETY: No more references to archetypes are life let new_archetype = new_archetype.as_mut(); + let archetype_ref = archetype.as_mut(); // SAFETY: Mutable references do not alias and will be dropped after this block - let (sparse_sets, table, entities) = { + let (sparse_sets, resource_storages, table, entities) = { let world = world.world_mut(); ( &mut world.storages.sparse_sets, + &mut world.storages.resources, &mut world.storages.tables[new_archetype.table_id()], &mut world.entities, ) }; - let result = archetype.swap_remove(location.archetype_row); + let result = archetype_ref.swap_remove(location.archetype_row); if let Some(swapped_entity) = result.swapped_entity { let swapped_location = // SAFETY: If the swap was successful, swapped_entity must be valid. @@ -235,25 +256,43 @@ impl<'w> BundleInserter<'w> { &*new_archetype, new_location, sparse_sets, + resource_storages, table, result.table_row, ) } ArchetypeMoveType::NewArchetypeNewTable { new_archetype } => { + // Inserting a resource will fail if it already exists on another entity. + // Check whether this is the case and determine the correct resulting archetype if so. + let mut new_archetype = if contains_resources { + find_archetype_after_fallible_resource_write_and_queue_cleanup( + world, + entity, + archetype_after_insert.added(), + Some(archetype), + new_archetype, + ) + } else { + *new_archetype + }; + // SAFETY: Archetype gets borrowed when running the on_discard observers above, + // so this reference can only be promoted from shared to &mut down here, after they have been ran + let archetype_ref = archetype.as_mut(); let new_archetype = new_archetype.as_mut(); // SAFETY: Mutable references do not alias and will be dropped after this block - let (archetypes_ptr, tables, sparse_sets, entities) = { + let (archetypes_ptr, tables, sparse_sets, resource_storages, entities) = { let world = world.world_mut(); let archetype_ptr: *mut Archetype = world.archetypes.archetypes.as_mut_ptr(); ( archetype_ptr, &mut world.storages.tables, &mut world.storages.sparse_sets, + &mut world.storages.resources, &mut world.entities, ) }; - let result = archetype.swap_remove(location.archetype_row); + let result = archetype_ref.swap_remove(location.archetype_row); if let Some(swapped_entity) = result.swapped_entity { let swapped_location = // SAFETY: If the swap was successful, swapped_entity must be valid. @@ -305,8 +344,8 @@ impl<'w> BundleInserter<'w> { }), ); - if archetype.id() == swapped_location.archetype_id { - archetype + if archetype_ref.id() == swapped_location.archetype_id { + archetype_ref .set_entity_table_row(swapped_location.archetype_row, result.table_row); } else if new_archetype.id() == swapped_location.archetype_id { new_archetype @@ -322,6 +361,7 @@ impl<'w> BundleInserter<'w> { &*new_archetype, new_location, sparse_sets, + resource_storages, move_result.new_table, move_result.new_row, ) @@ -353,21 +393,24 @@ impl<'w> BundleInserter<'w> { let (new_archetype, new_location) = { // Non-generic prelude extracted to improve compile time by minimizing monomorphized code. - let (new_archetype, new_location, sparse_sets, table, table_row) = Self::before_insert( - entity, - location, - insert_mode, - caller, - relationship_hook_mode, - self.archetype, - archetype_after_insert, - &self.world, - &mut self.archetype_move_type, - ); + let (new_archetype, new_location, sparse_sets, resource_storages, table, table_row) = + Self::before_insert( + entity, + location, + insert_mode, + caller, + relationship_hook_mode, + &mut self.archetype, + archetype_after_insert, + &self.world, + &mut self.archetype_move_type, + self.bundle_info.as_ref().contains_resources, + ); self.bundle_info.as_ref().write_components( table, sparse_sets, + resource_storages, archetype_after_insert, archetype_after_insert.required_components.iter(), entity, @@ -518,7 +561,7 @@ impl BundleInfo { return (archetype_after_insert_id, false); } let mut new_table_components = Vec::new(); - let mut new_sparse_set_components = Vec::new(); + let mut new_non_table_components = Vec::new(); let mut bundle_status = Vec::with_capacity(self.explicit_components_len()); let mut added_required_components = Vec::new(); let mut added = Vec::new(); @@ -536,7 +579,9 @@ impl BundleInfo { 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 => new_sparse_set_components.push(component_id), + StorageType::SparseSet | StorageType::Resource => { + new_non_table_components.push(component_id); + } } } } @@ -551,14 +596,14 @@ impl BundleInfo { StorageType::Table => { new_table_components.push(component_id); } - StorageType::SparseSet => { - new_sparse_set_components.push(component_id); + StorageType::SparseSet | StorageType::Resource => { + new_non_table_components.push(component_id); } } } } - if new_table_components.is_empty() && new_sparse_set_components.is_empty() { + if new_table_components.is_empty() && new_non_table_components.is_empty() { let edges = current_archetype.edges_mut(); // The archetype does not change when we insert this bundle. edges.cache_archetype_after_bundle_insert( @@ -573,7 +618,7 @@ impl BundleInfo { } else { let table_id; let table_components; - let sparse_set_components; + let non_table_components; // The archetype changes when we insert this bundle. Prepare the new archetype and storages. { let current_archetype = &archetypes[archetype_id]; @@ -595,13 +640,13 @@ impl BundleInfo { new_table_components }; - sparse_set_components = if new_sparse_set_components.is_empty() { - current_archetype.sparse_set_components().collect() + non_table_components = if new_non_table_components.is_empty() { + current_archetype.non_table_components().collect() } else { - new_sparse_set_components.extend(current_archetype.sparse_set_components()); + new_non_table_components.extend(current_archetype.non_table_components()); // Sort to ignore order while hashing. - new_sparse_set_components.sort_unstable(); - new_sparse_set_components + new_non_table_components.sort_unstable(); + new_non_table_components }; }; // SAFETY: ids in self must be valid @@ -611,7 +656,7 @@ impl BundleInfo { observers, table_id, table_components, - sparse_set_components, + non_table_components, ) }; diff --git a/crates/bevy_ecs/src/bundle/remove.rs b/crates/bevy_ecs/src/bundle/remove.rs index 6b9f59186a835..a13a27142a3ef 100644 --- a/crates/bevy_ecs/src/bundle/remove.rs +++ b/crates/bevy_ecs/src/bundle/remove.rs @@ -12,7 +12,7 @@ use crate::{ lifecycle::{Discard, Remove, DISCARD, REMOVE}, observer::Observers, relationship::RelationshipHookMode, - storage::{SparseSets, Storages, Table, TableId}, + storage::{ResourceStorages, SparseSets, Storages, Table, TableId}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; @@ -107,6 +107,7 @@ impl<'w> BundleRemover<'w> { /// This can be passed to [`remove`](Self::remove) as the `pre_remove` function if you don't want to do anything before removing. pub fn empty_pre_remove( _: &mut SparseSets, + _: &mut ResourceStorages, _: Option<&mut Table>, _: &Components, _: &[ComponentId], @@ -129,6 +130,7 @@ impl<'w> BundleRemover<'w> { caller: MaybeLocation, pre_remove: impl FnOnce( &mut SparseSets, + &mut ResourceStorages, Option<&mut Table>, &Components, &[ComponentId], @@ -193,6 +195,7 @@ impl<'w> BundleRemover<'w> { let (needs_drop, pre_remove_result) = pre_remove( &mut world.storages.sparse_sets, + &mut world.storages.resources, // SAFETY: // - The `TableId`s in `old_and_new_table` were retrieved from valid `Archetype`s. self.old_and_new_table.map(|old_and_new_table| unsafe { @@ -202,24 +205,34 @@ impl<'w> BundleRemover<'w> { self.bundle_info.as_ref().explicit_components(), ); - // Handle sparse set removes + // Handle sparse set/resource removes for component_id in self.bundle_info.as_ref().iter_explicit_components() { if self.old_archetype.as_ref().contains(component_id) { world.removed_components.write(component_id, entity); - // Make sure to drop components stored in sparse sets. // Dense components are dropped later in `move_to_and_drop_missing_unchecked`. - if let Some(StorageType::SparseSet) = - self.old_archetype.as_ref().get_storage_type(component_id) - { - world - .storages - .sparse_sets - .get_mut(component_id) - // Set exists because the component existed on the entity - .unwrap() - // If it was already forgotten, it would not be in the set. - .remove(entity); + match self.old_archetype.as_ref().get_storage_type(component_id) { + Some(StorageType::SparseSet) => { + world + .storages + .sparse_sets + .get_mut(component_id) + // Set exists because the component existed on the entity + .unwrap() + // If it was already forgotten, it would not be in the set. + .remove(entity); + } + Some(StorageType::Resource) => { + world + .storages + .resources + .get_mut(component_id) + // Storage exists because the component existed on the entity + .unwrap() + // If it was already forgotten, it would not be in the set. + .remove(entity); + } + _ => {} } } } @@ -361,22 +374,23 @@ impl BundleInfo { (result, false) } else { let mut next_table_components; - let mut next_sparse_set_components; + let mut next_non_table_components; let next_table_id; { let current_archetype = &mut archetypes[archetype_id]; let mut removed_table_components = Vec::new(); - let mut removed_sparse_set_components = Vec::new(); + let mut removed_non_table_components = Vec::new(); for component_id in self.iter_explicit_components() { if current_archetype.contains(component_id) { // SAFETY: bundle components were already initialized by bundles.get_info let component_info = unsafe { components.get_info_unchecked(component_id) }; match component_info.storage_type() { - StorageType::Table => removed_table_components.push(component_id), - StorageType::SparseSet => { - removed_sparse_set_components.push(component_id); + StorageType::Table => &mut removed_table_components, + StorageType::SparseSet | StorageType::Resource => { + &mut removed_non_table_components } } + .push(component_id); } else if !intersection { // A component in the bundle was not present in the entity's archetype, so this // removal is invalid. Cache the result in the archetype graph. @@ -390,13 +404,13 @@ impl BundleInfo { // Sort removed components so we can do an efficient "sorted remove". // Archetype components are already sorted. removed_table_components.sort_unstable(); - removed_sparse_set_components.sort_unstable(); + removed_non_table_components.sort_unstable(); next_table_components = current_archetype.table_components().collect(); - next_sparse_set_components = current_archetype.sparse_set_components().collect(); + next_non_table_components = current_archetype.non_table_components().collect(); sorted_remove(&mut next_table_components, &removed_table_components); sorted_remove( - &mut next_sparse_set_components, - &removed_sparse_set_components, + &mut next_non_table_components, + &removed_non_table_components, ); next_table_id = if removed_table_components.is_empty() { @@ -416,7 +430,7 @@ impl BundleInfo { observers, next_table_id, next_table_components, - next_sparse_set_components, + next_non_table_components, ); (Some(new_archetype_id), is_new_created) }; diff --git a/crates/bevy_ecs/src/bundle/spawner.rs b/crates/bevy_ecs/src/bundle/spawner.rs index 8a43899bb28a2..94b8408c2145a 100644 --- a/crates/bevy_ecs/src/bundle/spawner.rs +++ b/crates/bevy_ecs/src/bundle/spawner.rs @@ -4,7 +4,10 @@ use bevy_ptr::{ConstNonNull, MovingPtr}; use crate::{ archetype::{Archetype, ArchetypeCreated, ArchetypeId, SpawnBundleStatus}, - bundle::{Bundle, BundleId, BundleInfo, DynamicBundle, InsertMode}, + bundle::{ + info::find_archetype_after_fallible_resource_write_and_queue_cleanup, Bundle, BundleId, + BundleInfo, DynamicBundle, InsertMode, + }, change_detection::{MaybeLocation, Tick}, entity::{Entity, EntityAllocator, EntityLocation}, event::EntityComponentsTrigger, @@ -96,20 +99,38 @@ impl<'w> BundleSpawner<'w> { ) -> EntityLocation { // SAFETY: We do not make any structural changes to the archetype graph through self.world so these pointers always remain valid let bundle_info = self.bundle_info.as_ref(); + + let mut archetype = if bundle_info.contains_resources { + find_archetype_after_fallible_resource_write_and_queue_cleanup( + &self.world, + entity, + bundle_info.contributed_components(), + None, + &mut self.archetype, + ) + } else { + self.archetype + }; + let location = { let table = self.table.as_mut(); - let archetype = self.archetype.as_mut(); + let archetype = archetype.as_mut(); // SAFETY: Mutable references do not alias and will be dropped after this block - let (sparse_sets, entities) = { + let (sparse_sets, resource_storages, entities) = { let world = self.world.world_mut(); - (&mut world.storages.sparse_sets, &mut world.entities) + ( + &mut world.storages.sparse_sets, + &mut world.storages.resources, + &mut world.entities, + ) }; let table_row = table.allocate(entity); let location = archetype.allocate(entity, table_row); bundle_info.write_components( table, sparse_sets, + resource_storages, &SpawnBundleStatus, bundle_info.required_component_constructors.iter(), entity, diff --git a/crates/bevy_ecs/src/component/mod.rs b/crates/bevy_ecs/src/component/mod.rs index a30f7379787ae..43f2d63cbfdc4 100644 --- a/crates/bevy_ecs/src/component/mod.rs +++ b/crates/bevy_ecs/src/component/mod.rs @@ -732,6 +732,17 @@ pub enum StorageType { Table, /// Provides fast addition and removal of components, but slower iteration. SparseSet, + /// Provides fast addition and removal, but only allows one entity. + Resource, +} + +impl StorageType { + pub(crate) const fn is_dense(self) -> bool { + match self { + StorageType::Table => true, + StorageType::SparseSet | StorageType::Resource => false, + } + } } /// A [`SystemParam`] that provides access to the [`ComponentId`] for a specific component type. diff --git a/crates/bevy_ecs/src/component/required.rs b/crates/bevy_ecs/src/component/required.rs index c1a9f06b8cf8f..dde4a7f503483 100644 --- a/crates/bevy_ecs/src/component/required.rs +++ b/crates/bevy_ecs/src/component/required.rs @@ -11,7 +11,7 @@ use crate::{ component::{Component, ComponentId, Components, ComponentsRegistrator}, entity::Entity, query::DebugCheckedUnwrap as _, - storage::{SparseSets, Table, TableRow}, + storage::{ResourceStorages, SparseSets, Table, TableRow}, }; /// Metadata associated with a required component. See [`Component`] for details. @@ -25,7 +25,17 @@ pub struct RequiredComponent { #[derive(Clone)] pub struct RequiredComponentConstructor( // Note: this function makes `unsafe` assumptions, so it cannot be public. - Arc, + Arc< + dyn for<'a> Fn( + &'a mut Table, + &'a mut SparseSets, + &'a mut ResourceStorages, + Tick, + TableRow, + Entity, + MaybeLocation, + ), + >, ); impl RequiredComponentConstructor { @@ -48,9 +58,10 @@ impl RequiredComponentConstructor { #[cfg(not(target_has_atomic = "ptr"))] use alloc::boxed::Box; - type Constructor = dyn for<'a, 'b> Fn( + type Constructor = dyn for<'a> Fn( &'a mut Table, - &'b mut SparseSets, + &'a mut SparseSets, + &'a mut ResourceStorages, Tick, TableRow, Entity, @@ -64,7 +75,13 @@ impl RequiredComponentConstructor { type Intermediate = Arc; let boxed: Intermediate = Intermediate::new( - move |table, sparse_sets, change_tick, table_row, entity, caller| { + move |table, + sparse_sets, + resource_storages, + change_tick, + table_row, + entity, + caller| { OwningPtr::make(constructor(), |ptr| { // SAFETY: This will only be called in the context of `BundleInfo::write_components`, which will // pass in a valid table_row and entity requiring a C constructor @@ -74,6 +91,7 @@ impl RequiredComponentConstructor { BundleInfo::initialize_required_component( table, sparse_sets, + resource_storages, change_tick, table_row, entity, @@ -97,19 +115,28 @@ impl RequiredComponentConstructor { /// /// `table_row` and `entity` must correspond to a valid entity that currently needs a component initialized via the constructor stored /// on this [`RequiredComponentConstructor`]. The stored constructor must correspond to a component on `entity` that needs initialization. - /// `table` and `sparse_sets` must correspond to storages on a world where `entity` needs this required component initialized. + /// `table`, `sparse_sets` and `resource_storages` must correspond to storages on a world where `entity` needs this required component initialized. /// /// Again, don't call this anywhere but [`BundleInfo::write_components`]. pub(crate) unsafe fn initialize( &self, table: &mut Table, sparse_sets: &mut SparseSets, + resource_storages: &mut ResourceStorages, change_tick: Tick, table_row: TableRow, entity: Entity, caller: MaybeLocation, ) { - (self.0)(table, sparse_sets, change_tick, table_row, entity, caller); + (self.0)( + table, + sparse_sets, + resource_storages, + change_tick, + table_row, + entity, + caller, + ); } } diff --git a/crates/bevy_ecs/src/entity/clone_entities.rs b/crates/bevy_ecs/src/entity/clone_entities.rs index 2ff9d1ebd3e45..1ed3a395dca31 100644 --- a/crates/bevy_ecs/src/entity/clone_entities.rs +++ b/crates/bevy_ecs/src/entity/clone_entities.rs @@ -710,11 +710,16 @@ impl EntityCloner { &moved_components, MaybeLocation::caller(), RelationshipHookMode::RunIfNotLinked, - |sparse_sets, mut table, components, bundle| { + |sparse_sets, resources, mut table, components, bundle| { for &component_id in bundle { let Some(component_ptr) = sparse_sets .get(component_id) .and_then(|component| component.get(source)) + .or_else(|| { + resources + .get(component_id) + .and_then(|storage| storage.get_with_entity(source)) + }) .or_else(|| { // SAFETY: table_row is within this table because we just got it from entity's current location table.as_mut().and_then(|table| unsafe { diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index b8e45f714bd3c..15ce9f72c83d1 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -12,7 +12,7 @@ use crate::{ Access, DebugCheckedUnwrap, FilteredAccess, FilteredAccessSet, QueryFilter, QueryState, WorldQuery, }, - storage::{ComponentSparseSet, Table, TableRow}, + storage::{ComponentSparseSet, ResourceStorage, Table, TableRow}, system::Query, world::{ unsafe_world_cell::UnsafeWorldCell, EntityMut, EntityMutExcept, EntityRef, EntityRefExcept, @@ -1759,6 +1759,8 @@ pub struct ReadFetch<'w, T: Component> { Option>>, // T::STORAGE_TYPE = StorageType::SparseSet Option<&'w ComponentSparseSet>, + // T::STORAGE_TYPE = StorageType::Resource + Option<&'w ResourceStorage>, >, } @@ -1800,16 +1802,18 @@ unsafe impl WorldQuery for &T { // reference to the sparse set, which is used to access the components in `Self::fetch`. unsafe { world.storages().sparse_sets.get(component_id) } }, + || { + // SAFETY: The underlying type associated with `component_id` is `T`, + // which we are allowed to access since we registered it in `update_component_access`. + // Note that we do not actually access any components in this function, we just get a shared + // reference to the sparse set, which is used to access the components in `Self::fetch`. + unsafe { world.storages().resources.get(component_id) } + }, ), } } - const IS_DENSE: bool = { - match T::STORAGE_TYPE { - StorageType::Table => true, - StorageType::SparseSet => false, - } - }; + const IS_DENSE: bool = T::STORAGE_TYPE.is_dense(); #[inline] unsafe fn set_archetype<'w>( @@ -1905,6 +1909,16 @@ unsafe impl QueryData for &T { }; item.deref() }, + |resource| { + // SAFETY: The caller ensures that the component is present. + let item = unsafe { + resource + .debug_checked_unwrap() + .get_with_entity(entity) + .debug_checked_unwrap() + }; + item.deref() + }, )) } @@ -1937,6 +1951,13 @@ impl ContiguousQueryData for &T { #[cfg(not(debug_assertions))] core::hint::unreachable_unchecked(); }, + |_| { + #[cfg(debug_assertions)] + unreachable!(); + // SAFETY: The caller ensures query is dense + #[cfg(not(debug_assertions))] + core::hint::unreachable_unchecked(); + }, ) } } @@ -1972,6 +1993,9 @@ pub struct RefFetch<'w, T: Component> { // T::STORAGE_TYPE = StorageType::SparseSet // Can be `None` when the component has never been inserted Option<&'w ComponentSparseSet>, + // T::STORAGE_TYPE = StorageType::Resource + // Can be `None` when the component has never been inserted + Option<&'w ResourceStorage>, >, last_run: Tick, this_run: Tick, @@ -2015,18 +2039,20 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { // reference to the sparse set, which is used to access the components in `Self::fetch`. unsafe { world.storages().sparse_sets.get(component_id) } }, + || { + // SAFETY: The underlying type associated with `component_id` is `T`, + // which we are allowed to access since we registered it in `update_component_access`. + // Note that we do not actually access any components in this function, we just get a shared + // reference to the sparse set, which is used to access the components in `Self::fetch`. + unsafe { world.storages().resources.get(component_id) } + }, ), last_run, this_run, } } - const IS_DENSE: bool = { - match T::STORAGE_TYPE { - StorageType::Table => true, - StorageType::SparseSet => false, - } - }; + const IS_DENSE: bool = T::STORAGE_TYPE.is_dense(); #[inline] unsafe fn set_archetype<'w>( @@ -2147,6 +2173,24 @@ unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> { .debug_checked_unwrap() }; + Ref { + value: component.deref(), + ticks: ComponentTicksRef::from_tick_cells( + ticks, + fetch.last_run, + fetch.this_run, + ), + } + }, + |resource| { + // SAFETY: The caller ensures that the component is present. + let (component, ticks) = unsafe { + resource + .debug_checked_unwrap() + .get_with_ticks() + .debug_checked_unwrap() + }; + Ref { value: component.deref(), ticks: ComponentTicksRef::from_tick_cells( @@ -2221,6 +2265,13 @@ impl ContiguousQueryData for Ref<'_, T> { #[cfg(not(debug_assertions))] core::hint::unreachable_unchecked(); }, + |_| { + #[cfg(debug_assertions)] + unreachable!(); + // SAFETY: the caller ensures that [`Self::set_table`] was called beforehand. + #[cfg(not(debug_assertions))] + core::hint::unreachable_unchecked(); + }, ) } } @@ -2239,6 +2290,9 @@ pub struct WriteFetch<'w, T: Component> { // T::STORAGE_TYPE = StorageType::SparseSet // Can be `None` when the component has never been inserted Option<&'w ComponentSparseSet>, + // T::STORAGE_TYPE = StorageType::Resource + // Can be `None` when the component has never been inserted + Option<&'w ResourceStorage>, >, last_run: Tick, this_run: Tick, @@ -2282,18 +2336,20 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { // reference to the sparse set, which is used to access the components in `Self::fetch`. unsafe { world.storages().sparse_sets.get(component_id) } }, + || { + // SAFETY: The underlying type associated with `component_id` is `T`, + // which we are allowed to access since we registered it in `update_component_access`. + // Note that we do not actually access any components in this function, we just get a shared + // reference to the sparse set, which is used to access the components in `Self::fetch`. + unsafe { world.storages().resources.get(component_id) } + }, ), last_run, this_run, } } - const IS_DENSE: bool = { - match T::STORAGE_TYPE { - StorageType::Table => true, - StorageType::SparseSet => false, - } - }; + const IS_DENSE: bool = T::STORAGE_TYPE.is_dense(); #[inline] unsafe fn set_archetype<'w>( @@ -2414,6 +2470,24 @@ unsafe impl<'__w, T: Component> QueryData for &'__w mut T .debug_checked_unwrap() }; + Mut { + value: component.assert_unique().deref_mut(), + ticks: ComponentTicksMut::from_tick_cells( + ticks, + fetch.last_run, + fetch.this_run, + ), + } + }, + |resource| { + // SAFETY: The caller ensures that the component is present. + let (component, ticks) = unsafe { + resource + .debug_checked_unwrap() + .get_with_ticks() + .debug_checked_unwrap() + }; + Mut { value: component.assert_unique().deref_mut(), ticks: ComponentTicksMut::from_tick_cells( @@ -2485,6 +2559,13 @@ impl> ContiguousQueryData for &mut T { #[cfg(not(debug_assertions))] core::hint::unreachable_unchecked(); }, + |_| { + #[cfg(debug_assertions)] + unreachable!(); + // SAFETY: the caller ensures that [`Self::set_table`] was called beforehand. + #[cfg(not(debug_assertions))] + core::hint::unreachable_unchecked(); + }, ) } } @@ -3289,12 +3370,7 @@ unsafe impl WorldQuery for Has { false } - const IS_DENSE: bool = { - match T::STORAGE_TYPE { - StorageType::Table => true, - StorageType::SparseSet => false, - } - }; + const IS_DENSE: bool = T::STORAGE_TYPE.is_dense(); #[inline] unsafe fn set_archetype<'w, 's>( @@ -4003,23 +4079,32 @@ impl ArchetypeQueryData for PhantomData {} /// A compile-time checked union of two different types that differs based on the /// [`StorageType`] of a given component. -pub(super) union StorageSwitch { +pub(super) union StorageSwitch { /// The table variant. Requires the component to be a table component. table: T, /// The sparse set variant. Requires the component to be a sparse set component. sparse_set: S, + /// The resource variant. Requires the component to be a resource storage component. + resource: R, _marker: PhantomData, } -impl StorageSwitch { +impl StorageSwitch { /// Creates a new [`StorageSwitch`] using the given closures to initialize /// the variant corresponding to the component's [`StorageType`]. - pub fn new(table: impl FnOnce() -> T, sparse_set: impl FnOnce() -> S) -> Self { + pub fn new( + table: impl FnOnce() -> T, + sparse_set: impl FnOnce() -> S, + resource: impl FnOnce() -> R, + ) -> Self { match C::STORAGE_TYPE { StorageType::Table => Self { table: table() }, StorageType::SparseSet => Self { sparse_set: sparse_set(), }, + StorageType::Resource => Self { + resource: resource(), + }, } } @@ -4047,7 +4132,12 @@ impl StorageSwitch { /// Fetches the internal value from the variant that corresponds to the /// component's [`StorageType`]. - pub fn extract(&self, table: impl FnOnce(T) -> R, sparse_set: impl FnOnce(S) -> R) -> R { + pub fn extract( + &self, + table: impl FnOnce(T) -> V, + sparse_set: impl FnOnce(S) -> V, + resource: impl FnOnce(R) -> V, + ) -> V { match C::STORAGE_TYPE { StorageType::Table => table( // SAFETY: C::STORAGE_TYPE == StorageType::Table @@ -4057,17 +4147,21 @@ impl StorageSwitch { // SAFETY: C::STORAGE_TYPE == StorageType::SparseSet unsafe { self.sparse_set }, ), + StorageType::Resource => resource( + // SAFETY: C::STORAGE_TYPE == StorageType::Resource + unsafe { self.resource }, + ), } } } -impl Clone for StorageSwitch { +impl Clone for StorageSwitch { fn clone(&self) -> Self { *self } } -impl Copy for StorageSwitch {} +impl Copy for StorageSwitch {} #[cfg(test)] mod tests { diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 84cc7b2edbc8e..efe1ddf39bc80 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -1,10 +1,10 @@ use crate::{ archetype::Archetype, change_detection::Tick, - component::{Component, ComponentId, Components, StorageType}, + component::{Component, ComponentId, Components}, entity::{Entities, Entity}, query::{DebugCheckedUnwrap, FilteredAccess, FilteredAccessSet, StorageSwitch, WorldQuery}, - storage::{ComponentSparseSet, Table, TableRow}, + storage::{ComponentSparseSet, ResourceStorage, Table, TableRow}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref}; @@ -161,12 +161,7 @@ unsafe impl WorldQuery for With { ) { } - const IS_DENSE: bool = { - match T::STORAGE_TYPE { - StorageType::Table => true, - StorageType::SparseSet => false, - } - }; + const IS_DENSE: bool = T::STORAGE_TYPE.is_dense(); #[inline] unsafe fn set_archetype( @@ -262,12 +257,7 @@ unsafe impl WorldQuery for Without { ) { } - const IS_DENSE: bool = { - match T::STORAGE_TYPE { - StorageType::Table => true, - StorageType::SparseSet => false, - } - }; + const IS_DENSE: bool = T::STORAGE_TYPE.is_dense(); #[inline] unsafe fn set_archetype( @@ -735,6 +725,9 @@ pub struct AddedFetch<'w, T: Component> { // T::STORAGE_TYPE = StorageType::SparseSet // Can be `None` when the component has never been inserted Option<&'w ComponentSparseSet>, + // T::STORAGE_TYPE = StorageType::Resource + // Can be `None` when the component has never been inserted + Option<&'w ResourceStorage>, >, last_run: Tick, this_run: Tick, @@ -780,18 +773,20 @@ unsafe impl WorldQuery for Added { // reference to the sparse set, which is used to access the components' ticks in `Self::fetch`. unsafe { world.storages().sparse_sets.get(id) } }, + || { + // SAFETY: The underlying type associated with `component_id` is `T`, + // which we are allowed to access since we registered it in `update_component_access`. + // Note that we do not actually access any components' ticks in this function, we just get a shared + // reference to the sparse set, which is used to access the components' ticks in `Self::fetch`. + unsafe { world.storages().resources.get(id) } + }, ), last_run, this_run, } } - const IS_DENSE: bool = { - match T::STORAGE_TYPE { - StorageType::Table => true, - StorageType::SparseSet => false, - } - }; + const IS_DENSE: bool = T::STORAGE_TYPE.is_dense(); #[inline] unsafe fn set_archetype<'w, 's>( @@ -859,27 +854,36 @@ unsafe impl QueryFilter for Added { table_row: TableRow, ) -> bool { // SAFETY: The invariants are upheld by the caller. - fetch.ticks.extract( - |table| { - // SAFETY: set_table was previously called - let table = unsafe { table.debug_checked_unwrap() }; - // SAFETY: The caller ensures `table_row` is in range. - let tick = unsafe { table.get_unchecked(table_row.index()) }; - - tick.deref().is_newer_than(fetch.last_run, fetch.this_run) - }, - |sparse_set| { - // SAFETY: The caller ensures `entity` is in range. - let tick = unsafe { - sparse_set - .debug_checked_unwrap() - .get_added_tick(entity) - .debug_checked_unwrap() - }; - - tick.deref().is_newer_than(fetch.last_run, fetch.this_run) - }, - ) + fetch + .ticks + .extract( + |table| { + // SAFETY: set_table was previously called + let table = unsafe { table.debug_checked_unwrap() }; + // SAFETY: The caller ensures `table_row` is in range. + unsafe { table.get_unchecked(table_row.index()) } + }, + |sparse_set| { + // SAFETY: The caller ensures `entity` is in range. + unsafe { + sparse_set + .debug_checked_unwrap() + .get_added_tick(entity) + .debug_checked_unwrap() + } + }, + |resource| { + // SAFETY: The caller ensures an entity has this component. + unsafe { + resource + .debug_checked_unwrap() + .get_added_tick() + .debug_checked_unwrap() + } + }, + ) + .deref() + .is_newer_than(fetch.last_run, fetch.this_run) } } @@ -962,6 +966,8 @@ pub struct ChangedFetch<'w, T: Component> { Option>>, // Can be `None` when the component has never been inserted Option<&'w ComponentSparseSet>, + // Can be `None` when the component has never been inserted + Option<&'w ResourceStorage>, >, last_run: Tick, this_run: Tick, @@ -1007,18 +1013,20 @@ unsafe impl WorldQuery for Changed { // reference to the sparse set, which is used to access the components' ticks in `Self::fetch`. unsafe { world.storages().sparse_sets.get(id) } }, + || { + // SAFETY: The underlying type associated with `component_id` is `T`, + // which we are allowed to access since we registered it in `update_component_access`. + // Note that we do not actually access any components' ticks in this function, we just get a shared + // reference to the sparse set, which is used to access the components' ticks in `Self::fetch`. + unsafe { world.storages().resources.get(id) } + }, ), last_run, this_run, } } - const IS_DENSE: bool = { - match T::STORAGE_TYPE { - StorageType::Table => true, - StorageType::SparseSet => false, - } - }; + const IS_DENSE: bool = T::STORAGE_TYPE.is_dense(); #[inline] unsafe fn set_archetype<'w, 's>( @@ -1087,27 +1095,36 @@ unsafe impl QueryFilter for Changed { table_row: TableRow, ) -> bool { // SAFETY: The invariants are upheld by the caller. - fetch.ticks.extract( - |table| { - // SAFETY: set_table was previously called - let table = unsafe { table.debug_checked_unwrap() }; - // SAFETY: The caller ensures `table_row` is in range. - let tick = unsafe { table.get_unchecked(table_row.index()) }; - - tick.deref().is_newer_than(fetch.last_run, fetch.this_run) - }, - |sparse_set| { - // SAFETY: The caller ensures `entity` is in range. - let tick = unsafe { - sparse_set - .debug_checked_unwrap() - .get_changed_tick(entity) - .debug_checked_unwrap() - }; - - tick.deref().is_newer_than(fetch.last_run, fetch.this_run) - }, - ) + fetch + .ticks + .extract( + |table| { + // SAFETY: set_table was previously called + let table = unsafe { table.debug_checked_unwrap() }; + // SAFETY: The caller ensures `table_row` is in range. + unsafe { table.get_unchecked(table_row.index()) } + }, + |sparse_set| { + // SAFETY: The caller ensures `entity` is in range. + unsafe { + sparse_set + .debug_checked_unwrap() + .get_changed_tick(entity) + .debug_checked_unwrap() + } + }, + |resource| { + // SAFETY: The caller ensures an entity has this component. + unsafe { + resource + .debug_checked_unwrap() + .get_changed_tick() + .debug_checked_unwrap() + } + }, + ) + .deref() + .is_newer_than(fetch.last_run, fetch.this_run) } } diff --git a/crates/bevy_ecs/src/resource.rs b/crates/bevy_ecs/src/resource.rs index 3e8099f53b057..f76aa893cefa6 100644 --- a/crates/bevy_ecs/src/resource.rs +++ b/crates/bevy_ecs/src/resource.rs @@ -4,16 +4,13 @@ use log::warn; use crate::{ component::{Component, ComponentId, Mutable}, - entity::Entity, lifecycle::HookContext, - storage::SparseArray, world::DeferredWorld, }; #[cfg(feature = "bevy_reflect")] use {crate::reflect::ReflectComponent, bevy_reflect::Reflect}; // The derive macro for the `Resource` trait pub use bevy_ecs_macros::Resource; -use bevy_platform::cell::SyncUnsafeCell; /// A type that can be inserted into a [`World`] as a singleton. /// @@ -86,41 +83,10 @@ use bevy_platform::cell::SyncUnsafeCell; )] pub trait Resource: Component {} -/// A cache that links each `ComponentId` from a resource to the corresponding entity. -#[derive(Default)] -pub struct ResourceEntities(SyncUnsafeCell>); - -impl ResourceEntities { - /// Returns an iterator over all registered resource components and their corresponding entity. - /// - /// This must scan the entire array of components to find non-empty values, - /// which may be slow even if there are few resources. - #[inline] - pub fn iter(&self) -> impl Iterator { - self.deref().iter().map(|(id, entity)| (id, *entity)) - } - - /// Returns the entity for the given resource component, or `None` if there is no entity. - #[inline] - pub fn get(&self, id: ComponentId) -> Option { - self.deref().get(id).copied() - } - - #[inline] - fn deref(&self) -> &SparseArray { - // SAFETY: There are no other mutable references to the map. - // The underlying `SyncUnsafeCell` is never exposed outside this module, - // so mutable references are only created by the resource hooks. - // We only expose `&ResourceCache` to code with access to a resource (such as `&World`), - // and that would conflict with the `DeferredWorld` passed to the resource hook. - unsafe { &*self.0.get() } - } -} - /// A marker component for entities that have a Resource component. #[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Component, Debug))] #[derive(Component, Debug)] -#[component(on_insert, on_discard, on_despawn)] +#[component(on_discard, on_despawn)] pub struct IsResource(ComponentId); impl IsResource { @@ -134,53 +100,6 @@ impl IsResource { self.0 } - pub(crate) fn on_insert(mut world: DeferredWorld, context: HookContext) { - let resource_component_id = world - .entity(context.entity) - .get::() - .unwrap() - .resource_component_id(); - - if let Some(original_entity) = world.resource_entities.get(resource_component_id) { - if !world.entities().contains(original_entity) { - let name = world - .components() - .get_name(resource_component_id) - .expect("resource is registered"); - panic!( - "Resource entity {} of {} has been despawned, when it's not supposed to be.", - original_entity, name - ); - } - - if original_entity != context.entity { - // the resource already exists and the new one should be removed - world - .commands() - .entity(context.entity) - .remove_by_id(resource_component_id); - world - .commands() - .entity(context.entity) - .remove_by_id(context.component_id); - let name = world - .components() - .get_name(resource_component_id) - .expect("resource is registered"); - warn!("Tried inserting the resource {} while one already exists. - Resources are unique components stored on a single entity. - Inserting on a different entity, when one already exists, causes the new value to be removed.", name); - } - } else { - // SAFETY: We have exclusive world access (as long as we don't make structural changes). - let cache = unsafe { world.as_unsafe_world_cell().resource_entities() }; - // SAFETY: There are no shared references to the map. - // We only expose `&ResourceCache` to code with access to a resource (such as `&World`), - // and that would conflict with the `DeferredWorld` passed to the resource hook. - unsafe { &mut *cache.0.get() }.insert(resource_component_id, context.entity); - } - } - pub(crate) fn on_discard(mut world: DeferredWorld, context: HookContext) { let resource_component_id = world .entity(context.entity) @@ -188,21 +107,17 @@ impl IsResource { .unwrap() .resource_component_id(); - if let Some(resource_entity) = world.resource_entities.get(resource_component_id) - && resource_entity == context.entity - { - // SAFETY: We have exclusive world access (as long as we don't make structural changes). - let cache = unsafe { world.as_unsafe_world_cell().resource_entities() }; - // SAFETY: There are no shared references to the map. - // We only expose `&ResourceCache` to code with access to a resource (such as `&World`), - // and that would conflict with the `DeferredWorld` passed to the resource hook. - unsafe { &mut *cache.0.get() }.remove(resource_component_id); - - world - .commands() - .entity(context.entity) - .remove_by_id(resource_component_id); - } + world + .commands() + .entity(context.entity) + .remove_by_id(resource_component_id); + world + .commands() + .queue(move |world: &mut crate::world::World| { + if let Some(storage) = world.storages.resources.get_mut(resource_component_id) { + storage.clear_entity_association(context.entity); + } + }); } pub(crate) fn on_despawn(_world: DeferredWorld, _context: HookContext) { @@ -255,7 +170,7 @@ mod tests { } }); assert_eq!(world.entities().count_spawned(), start + 3); - let e3 = world.resource_entities().get(id3).unwrap(); + let e3 = world.storages.resources.get(id3).unwrap().entity().unwrap(); assert!(world.remove_resource_by_id(id3)); // the entity is stable: removing the resource should only remove the component from the entity, not despawn the entity assert_eq!(world.entities().count_spawned(), start + 3); @@ -265,13 +180,19 @@ mod tests { world.insert_resource_by_id(id3, ptr, MaybeLocation::caller()); } }); - assert_eq!(e3, world.resource_entities().get(id3).unwrap()); + assert_eq!( + e3, + world.storages.resources.get(id3).unwrap().entity().unwrap() + ); // again, the entity is stable: see previous explanation - let e1 = world.resource_entities().get(id1).unwrap(); + let e1 = world.storages.resources.get(id1).unwrap().entity().unwrap(); world.remove_resource::(); assert_eq!(world.entities().count_spawned(), start + 3); world.init_resource::(); - assert_eq!(e1, world.resource_entities().get(id1).unwrap()); + assert_eq!( + e1, + world.storages.resources.get(id1).unwrap().entity().unwrap() + ); // make sure that trying to add a resource twice results, doesn't change the entity count world.insert_resource(TestResource2(String::from("Bar"))); assert_eq!(world.entities().count_spawned(), start + 3); @@ -297,7 +218,6 @@ mod tests { }; // Removing IsResource should invalidate the current TestResource entity - // This uses commands because IsResource's despawn-on-removal invalidates the EntityWorldMut and panics world.entity_mut(first_entity).remove::(); assert!(world.get_resource::().is_none()); diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index fb518b034b0d5..b826efd52ae8a 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -27,11 +27,13 @@ mod blob_array; mod non_send; +mod resource_storage; mod sparse_set; mod table; mod thin_array_ptr; pub use non_send::*; +pub use resource_storage::*; pub use sparse_set::*; pub use table::*; @@ -44,6 +46,8 @@ pub struct Storages { /// Backing storage for [`SparseSet`] components. /// Note that sparse sets are only present for components that have been spawned or have had a relevant bundle registered. pub sparse_sets: SparseSets, + /// Backing storage for [`Resource`](crate::resource::Resource) components. + pub resources: ResourceStorages, /// Backing storage for [`Table`] components. pub tables: Tables, /// Backing storage for `!Send` data. @@ -51,15 +55,18 @@ pub struct Storages { } impl Storages { - /// ensures that the component has its necessary storage initialize. + /// Ensures that the component has its necessary storage initialize. pub fn prepare_component(&mut self, component: &ComponentInfo) { match component.storage_type() { StorageType::Table => { - // table needs no preparation + // no preparation needed } StorageType::SparseSet => { self.sparse_sets.get_or_insert(component); } + StorageType::Resource => { + self.resources.init(component); + } } } } diff --git a/crates/bevy_ecs/src/storage/resource_storage.rs b/crates/bevy_ecs/src/storage/resource_storage.rs new file mode 100644 index 0000000000000..a10200527e9b1 --- /dev/null +++ b/crates/bevy_ecs/src/storage/resource_storage.rs @@ -0,0 +1,332 @@ +use core::{cell::UnsafeCell, panic::Location}; + +use bevy_ptr::{OwningPtr, Ptr}; +use nonmax::NonMaxU32; + +use crate::{ + change_detection::{CheckChangeTicks, ComponentTickCells, ComponentTicks, MaybeLocation, Tick}, + component::{ComponentId, ComponentInfo}, + entity::Entity, + storage::{Column, SparseSet, TableRow}, +}; + +/// A collection of resource storages, indexed by [`ComponentId`] +/// +/// Can be accessed via [`Storages`](crate::storage::Storages) +#[derive(Default)] +pub struct ResourceStorages { + /// Column is always one element long. + resources: SparseSet, +} + +impl ResourceStorages { + pub(crate) fn init(&mut self, component_info: &ComponentInfo) { + self.resources + .get_or_insert_with(component_info.id(), || ResourceStorage::new(component_info)); + } + + /// Gets a reference to the [`ResourceStorage`] of a [`ComponentId`]. + /// This may be `None` if the component has never been spawned. + pub fn get(&self, component_id: ComponentId) -> Option<&ResourceStorage> { + self.resources.get(component_id) + } + + /// Gets a reference to the [`ResourceStorage`] of a [`ComponentId`]. + /// This may be `None` if the component has never been spawned. + pub fn get_mut(&mut self, component_id: ComponentId) -> Option<&mut ResourceStorage> { + self.resources.get_mut(component_id) + } + + /// Iterate all resources. + pub fn iter(&self) -> impl Iterator { + self.resources + .iter() + .filter_map(|(&id, storage)| match storage.state { + Populated(entity) => Some((id, entity)), + _ => None, + }) + } + + pub(crate) fn check_change_ticks(&mut self, check: CheckChangeTicks) { + for storage in self.resources.values_mut() { + storage.check_change_ticks(check); + } + } + + /// Clears all resource entities + /// + /// # Panics + /// - Panics if any of the components stored within implement [`Drop`] and any of them panic. + pub(crate) fn clear_entities(&mut self) { + for storage in self.resources.values_mut() { + if let Some(entity) = storage.entity() { + storage.remove(entity); + } + } + } +} + +const ROW: TableRow = TableRow::new(NonMaxU32::ZERO); + +enum ResourceState { + /// There is no entity assigned to hold this resource. + /// This can be because the resource hasn't been inserted so far, + /// or because `IsResource` has been removed + /// (possibly because the entity has been despawned). + NoEntity, + /// There is an entity assigned to hold this resource, but the + /// resource currently isn't present in the world. + Unpopulated(Entity), + /// The resource currently is present in the world. + Populated(Entity), +} + +use ResourceState::*; + +/// Storage for an individual resource. +pub struct ResourceStorage { + state: ResourceState, + /// capacity: 1 + /// length: 1 if populated, 0 otherwise + data: Column, +} + +impl ResourceStorage { + fn new(component_info: &ComponentInfo) -> Self { + Self { + state: NoEntity, + data: Column::with_capacity(component_info, 1), + } + } + + /// Returns the entity responsible for holding this resource, + /// even if the resource doesn't currently exist in the world. + /// + /// Can return `None` if the resource has never been inserted before, + /// or has been despawned. + pub fn entity(&self) -> Option { + match self.state { + NoEntity => None, + Unpopulated(entity) | Populated(entity) => Some(entity), + } + } + + /// Returns whether the given resource exists. + pub fn populated(&self) -> bool { + match self.state { + NoEntity | Unpopulated(_) => false, + Populated(_) => true, + } + } + + /// Inserts the component `value` into this resource storage. + /// + /// Will fail if another entity already has this resource. + /// + /// # Safety + /// The `value` pointer must point to a valid address that matches the [`Layout`](std::alloc::Layout) + /// inside the [`ComponentInfo`] given when constructing this sparse set. + pub(crate) unsafe fn insert( + &mut self, + entity: Entity, + value: OwningPtr<'_>, + change_tick: Tick, + caller: MaybeLocation, + ) { + match self.state { + NoEntity | Unpopulated(_) => { + self.state = Populated(entity); + self.data.initialize(ROW, value, change_tick, caller); + } + Populated(existing_entity) if existing_entity == entity => { + self.data.replace(ROW, value, change_tick, caller); + } + Populated(_) => { + if let Some(drop) = self.get_drop() { + // SAFETY: Drop function came from value's component descriptor + unsafe { + drop(value); + } + } + } + } + } + + /// Returns a reference to the entity's component value. + /// + /// Returns `None` if this entity doesn't have this component. + #[inline] + pub fn get_with_entity(&self, entity: Entity) -> Option> { + match self.state { + Populated(existing) if existing == entity => Some( + // SAFETY: length is 1 + unsafe { self.data.get_data_unchecked(ROW) }, + ), + _ => None, + } + } + + /// Returns a reference to the resource's component value. + /// + /// Returns `None` if no entity has this component. + #[inline] + pub fn get(&self) -> Option> { + match self.state { + Populated(_) => Some( + // SAFETY: length is 1 + unsafe { self.data.get_data_unchecked(ROW) }, + ), + _ => None, + } + } + + /// Returns references to the entity's component value and its added and changed ticks. + /// + /// Returns `None` if no entity has this component. + #[inline] + pub fn get_with_ticks(&self) -> Option<(Ptr<'_>, ComponentTickCells<'_>)> { + match self.state { + NoEntity | Unpopulated(_) => None, + Populated(_) => Some( + // SAFETY: length is 1 + unsafe { + ( + self.data.get_data_unchecked(ROW), + ComponentTickCells { + added: self.data.get_added_tick_unchecked(ROW), + changed: self.data.get_changed_tick_unchecked(ROW), + changed_by: self.data.get_changed_by_unchecked(ROW), + }, + ) + }, + ), + } + } + + /// Returns a reference to the "added" tick of the entity's component value. + /// + /// Returns `None` if no entity has this component. + #[inline] + pub fn get_added_tick(&self) -> Option<&UnsafeCell> { + match self.state { + NoEntity | Unpopulated(_) => None, + Populated(_) => Some( + // SAFETY: length is 1 + unsafe { self.data.get_added_tick_unchecked(ROW) }, + ), + } + } + + /// Returns a reference to the "changed" tick of the entity's component value. + /// + /// Returns `None` if no entity has this component. + #[inline] + pub fn get_changed_tick(&self) -> Option<&UnsafeCell> { + match self.state { + NoEntity | Unpopulated(_) => None, + Populated(_) => Some( + // SAFETY: length is 1 + unsafe { self.data.get_changed_tick_unchecked(ROW) }, + ), + } + } + + /// Returns a reference to the "added" and "changed" ticks of the entity's component value. + /// + /// Returns `None` if no entity has this component. + #[inline] + pub fn get_ticks(&self) -> Option { + match self.state { + NoEntity | Unpopulated(_) => None, + Populated(_) => Some( + // SAFETY: length is 1 + unsafe { self.data.get_ticks_unchecked(ROW) }, + ), + } + } + + /// Returns a reference to the calling location that last changed the entity's component value. + /// + /// Returns `None` if no entity has this component. + #[inline] + pub fn get_changed_by(&self) -> MaybeLocation>>> { + MaybeLocation::new_with_flattened(|| { + match self.state { + NoEntity | Unpopulated(_) => None, + Populated(_) => Some( + // SAFETY: length is 1 + unsafe { self.data.get_changed_by_unchecked(ROW) }, + ), + } + }) + } + + /// Returns the drop function for the component type stored in the sparse set, + /// or `None` if it doesn't need to be dropped. + #[inline] + pub fn get_drop(&self) -> Option)> { + self.data.get_drop() + } + + /// Removes (and drops) the entity's component value from the sparse set. + /// + /// Returns `true` if `entity` had a component value in the sparse set. + pub(crate) fn remove(&mut self, entity: Entity) -> bool { + match self.state { + Populated(existing_entity) if existing_entity == entity => { + self.state = Unpopulated(existing_entity); + // SAFETY: Value is being removed + unsafe { + self.data.drop_last_component(0); + } + true + } + _ => false, + } + } + + /// Removes the resource and returns a pointer to the associated value (if it exists). + #[must_use = "The returned pointer must be used to drop the removed component."] + pub(crate) fn remove_and_forget(&mut self, entity: Entity) -> Option> { + match self.state { + Populated(existing_entity) if existing_entity == entity => { + self.state = Unpopulated(existing_entity); + // SAFETY: Value is being removed + Some(unsafe { OwningPtr::new(self.data.get_data_unchecked(ROW).into()) }) + } + _ => None, + } + } + + pub(crate) fn check_change_ticks(&mut self, check: CheckChangeTicks) { + if matches!(self.state, Populated(_)) { + // SAFETY: Data has one element + unsafe { self.data.check_change_ticks(1, check) }; + } + } + + pub(crate) fn clear_entity_association(&mut self, entity: Entity) { + if let Unpopulated(existing_entity) = self.state + && (existing_entity == entity) + { + self.state = NoEntity; + } + } +} + +impl Drop for ResourceStorage { + fn drop(&mut self) { + // SAFETY: + // `cap` and `len` always as specified on `data` doc comment. + // `data` is never accessed again after this call. + unsafe { + self.data.drop( + 1, + match self.state { + Populated(_) => 1, + _ => 0, + }, + ); + } + } +} diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 13460e2defd1e..02731c0e8bf1c 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -135,19 +135,6 @@ impl SparseArray { marker: PhantomData, } } - - /// Returns an iterator over the non-empty values in the array. - /// - /// This must scan the entire array to find non-empty values, - /// which may be slow even if the array is sparsely populated. - #[inline] - pub(crate) fn iter(&self) -> impl Iterator { - self.values.iter().enumerate().filter_map(|(index, value)| { - value - .as_ref() - .map(|value| (SparseSetIndex::get_sparse_set_index(index), value)) - }) - } } /// A sparse data structure of [`Component`](crate::component::Component)s. diff --git a/crates/bevy_ecs/src/world/entity_access/world_mut.rs b/crates/bevy_ecs/src/world/entity_access/world_mut.rs index 9b3471f4ff541..1ad6b079c7fa0 100644 --- a/crates/bevy_ecs/src/world/entity_access/world_mut.rs +++ b/crates/bevy_ecs/src/world/entity_access/world_mut.rs @@ -15,7 +15,7 @@ use crate::{ }, relationship::RelationshipHookMode, resource::Resource, - storage::{SparseSets, Table}, + storage::{ResourceStorages, SparseSets, Table}, template::{EntityScopes, ScopedEntities, Template, TemplateContext}, world::{ error::EntityComponentError, unsafe_world_cell::UnsafeEntityCell, ComponentEntry, @@ -1243,30 +1243,38 @@ impl<'w> EntityWorldMut<'w> { entity, location, MaybeLocation::caller(), - |sets, table, components, bundle_components| { + |sets, resources, table, components, bundle_components| { let mut bundle_components = bundle_components.iter().copied(); ( false, - T::from_components(&mut (sets, table), &mut |(sets, table)| { - let component_id = bundle_components.next().unwrap(); - // SAFETY: the component existed to be removed, so its id must be valid. - let component_info = components.get_info_unchecked(component_id); - match component_info.storage_type() { - StorageType::Table => { - table - .as_mut() - // SAFETY: The table must be valid if the component is in it. - .debug_checked_unwrap() - // SAFETY: The remover is cleaning this up. - .take_component(component_id, location.table_row) + T::from_components( + &mut (sets, resources, table), + &mut |(sets, resources, table)| { + let component_id = bundle_components.next().unwrap(); + // SAFETY: the component existed to be removed, so its id must be valid. + let component_info = components.get_info_unchecked(component_id); + match component_info.storage_type() { + StorageType::Table => { + table + .as_mut() + // SAFETY: The table must be valid if the component is in it. + .debug_checked_unwrap() + // SAFETY: The remover is cleaning this up. + .take_component(component_id, location.table_row) + } + StorageType::SparseSet => sets + .get_mut(component_id) + .unwrap() + .remove_and_forget(entity) + .unwrap(), + StorageType::Resource => resources + .get_mut(component_id) + .unwrap() + .remove_and_forget(entity) + .unwrap(), } - StorageType::SparseSet => sets - .get_mut(component_id) - .unwrap() - .remove_and_forget(entity) - .unwrap(), - } - }), + }, + ), ) }, ) @@ -1498,6 +1506,7 @@ impl<'w> EntityWorldMut<'w> { relationship_hook_mode: RelationshipHookMode, pre_remove: impl FnOnce( &mut SparseSets, + &mut ResourceStorages, Option<&mut Table>, &Components, &[ComponentId], @@ -1747,6 +1756,11 @@ impl<'w> EntityWorldMut<'w> { .unwrap(); sparse_set.remove(self.entity); } + for component_id in archetype.resource_components() { + // resource storage must have existed for the component to be added. + let resource_storage = self.world.storages.resources.get_mut(component_id).unwrap(); + resource_storage.remove(self.entity); + } // SAFETY: table rows stored in archetypes always exist moved_entity = unsafe { self.world.storages.tables[archetype.table_id()].swap_remove_unchecked(table_row) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 8ee75de95f147..65661f9e675e3 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -57,9 +57,9 @@ use crate::{ prelude::{Add, Despawn, DetectChangesMut, Discard, Insert, Remove}, query::{DebugCheckedUnwrap, QueryData, QueryFilter, QueryState}, relationship::RelationshipHookMode, - resource::{IsResource, Resource, ResourceEntities, IS_RESOURCE}, + resource::{IsResource, Resource, IS_RESOURCE}, schedule::{Schedule, ScheduleLabel, Schedules}, - storage::{NonSendData, Storages}, + storage::{NonSendData, ResourceStorage, Storages}, system::Commands, world::{ command_queue::RawCommandQueue, @@ -101,7 +101,6 @@ pub struct World { pub(crate) entity_allocator: EntityAllocator, pub(crate) components: Components, pub(crate) component_ids: ComponentIds, - pub(crate) resource_entities: ResourceEntities, pub(crate) archetypes: Archetypes, pub(crate) storages: Storages, pub(crate) bundles: Bundles, @@ -121,7 +120,6 @@ impl Default for World { entities: Entities::new(), entity_allocator: EntityAllocator::default(), components: Default::default(), - resource_entities: Default::default(), archetypes: Archetypes::new(), storages: Default::default(), bundles: Default::default(), @@ -259,12 +257,6 @@ impl World { &self.components } - /// Retrieves this world's [`ResourceEntities`]. - #[inline] - pub fn resource_entities(&self) -> &ResourceEntities { - &self.resource_entities - } - /// Prepares a [`ComponentsQueuedRegistrator`] for the world. /// **NOTE:** [`ComponentsQueuedRegistrator`] is easily misused. /// See its docs for important notes on when and how it should be used. @@ -1860,7 +1852,12 @@ impl World { ) -> (ComponentId, EntityWorldMut<'_>) { let resource_id = self.register_resource::(); - if let Some(entity) = self.resource_entities.get(resource_id) { + if let Some(entity) = self + .storages + .resources + .get(resource_id) + .and_then(ResourceStorage::entity) + { let entity_ref = self.get_entity(entity).expect("ResourceCache is in sync"); if !entity_ref.contains_id(resource_id) { let resource = func(self); @@ -1872,13 +1869,13 @@ impl World { RelationshipHookMode::Run, ); } - return (resource_id, self.entity_mut(entity)); + (resource_id, self.entity_mut(entity)) + } else { + let resource = func(self); + move_as_ptr!(resource); + let entity_mut = self.spawn_with_caller(resource, caller); + (resource_id, entity_mut) } - - let resource = func(self); - move_as_ptr!(resource); - let entity_mut = self.spawn_with_caller(resource, caller); // ResourceCache is updated automatically - (resource_id, entity_mut) } /// Initializes a new resource and returns the [`ComponentId`] created for it. @@ -1995,10 +1992,10 @@ impl World { #[inline] pub fn remove_resource(&mut self) -> Option { let resource_id = self.component_id::()?; - let entity = self.resource_entities.get(resource_id)?; + let entity = self.storages.resources.get(resource_id)?.entity()?; let value = self .get_entity_mut(entity) - .expect("ResourceCache is in sync") + .expect("Despawning a resource should remove its resource storage entry") .take::()?; Some(value) } @@ -2039,12 +2036,10 @@ impl World { /// Returns `true` if a resource with provided `component_id` exists. Otherwise returns `false`. #[inline] pub fn contains_resource_by_id(&self, component_id: ComponentId) -> bool { - if let Some(entity) = self.resource_entities.get(component_id) - && let Ok(entity_ref) = self.get_entity(entity) - { - return entity_ref.contains_id(component_id); - } - false + self.storages + .resources + .get(component_id) + .is_some_and(ResourceStorage::populated) } /// Returns `true` if `!Send` data of type `R` exists. Otherwise returns `false`. @@ -2129,9 +2124,7 @@ impl World { &self, component_id: ComponentId, ) -> Option { - let entity = self.resource_entities.get(component_id)?; - let entity_ref = self.get_entity(entity).ok()?; - entity_ref.get_change_ticks_by_id(component_id) + self.storages.resources.get(component_id)?.get_ticks() } /// Gets a reference to the resource of the given type @@ -2781,7 +2774,7 @@ impl World { let change_tick = self.change_tick(); let component_id = self.components.valid_component_id::()?; - let entity = self.resource_entities.get(component_id)?; + let entity = self.storages.resources.get(component_id)?.entity()?; let mut entity_mut = self.get_entity_mut(entity).ok()?; let mut ticks = entity_mut.get_change_ticks::()?; @@ -2973,9 +2966,13 @@ impl World { caller: MaybeLocation, ) { // if the resource already exists, we replace it on the same entity - let mut entity_mut = if let Some(entity) = self.resource_entities.get(component_id) { - self.get_entity_mut(entity) - .expect("ResourceCache is in sync") + let mut entity_mut = if let Some(entity) = self + .storages + .resources + .get(component_id) + .and_then(ResourceStorage::entity) + { + self.get_entity_mut(entity).unwrap() } else { self.spawn_empty() }; @@ -3249,6 +3246,7 @@ impl World { let Storages { ref mut tables, ref mut sparse_sets, + ref mut resources, ref mut non_sends, } = self.storages; @@ -3256,6 +3254,7 @@ impl World { let _span = tracing::info_span!("check component ticks").entered(); tables.check_change_ticks(check); sparse_sets.check_change_ticks(check); + resources.check_change_ticks(check); non_sends.check_change_ticks(check); self.entities.check_change_ticks(check); @@ -3290,6 +3289,7 @@ impl World { pub fn clear_entities(&mut self) { self.storages.tables.clear(); self.storages.sparse_sets.clear_entities(); + self.storages.resources.clear_entities(); self.archetypes.clear_entities(); self.entities.clear(); self.entity_allocator.restart(); @@ -3303,7 +3303,7 @@ impl World { /// This can easily cause systems expecting certain resources to immediately start panicking. /// Use with caution. pub fn clear_resources(&mut self) { - let pairs: Vec<(ComponentId, Entity)> = self.resource_entities().iter().collect(); + let pairs: Vec<(ComponentId, Entity)> = self.storages.resources.iter().collect(); for (component_id, entity) in pairs { self.entity_mut(entity).remove_by_id(component_id); } @@ -3504,7 +3504,8 @@ impl World { /// ``` #[inline] pub fn iter_resources(&self) -> impl Iterator)> { - self.resource_entities + self.storages + .resources .iter() .filter_map(|(component_id, entity)| { let component_info = self.components().get_info(component_id)?; @@ -3581,28 +3582,31 @@ impl World { /// ``` pub fn iter_resources_mut(&mut self) -> impl Iterator)> { let unsafe_world = self.as_unsafe_world_cell(); - // SAFETY: exclusive world access to all resources - let resource_entities = unsafe { unsafe_world.resource_entities() }; let components = unsafe_world.components(); - - resource_entities + // SAFETY: We have exclusive world access + unsafe { unsafe_world.storages() } + .resources .iter() - .filter_map(move |(component_id, entity)| { + .map(move |(component_id, entity)| { // SAFETY: If a resource has been initialized, a corresponding ComponentInfo must exist with its ID. let component_info = unsafe { components.get_info(component_id).debug_checked_unwrap() }; - let entity_cell = unsafe_world.get_entity(entity).ok()?; + // SAFETY: Resource is present + let entity_cell = unsafe { unsafe_world.get_entity(entity).debug_checked_unwrap() }; // SAFETY: // - We have exclusive world access - // - `UnsafeEntityCell::get_mut_by_id` doesn't access components - // or resource_entities mutably - // - `resource_entities` doesn't contain duplicate entities, so - // no duplicate references are created - let mut_untyped = unsafe { entity_cell.get_mut_by_id(component_id).ok()? }; + // - `UnsafeEntityCell::get_mut_by_id` doesn't access components/storage mutably + // - we iterate over storages, so no duplicate references are created and storage guarantees + // that component is present + let mut_untyped = unsafe { + entity_cell + .get_mut_by_id(component_id) + .debug_checked_unwrap() + }; - Some((component_info, mut_untyped)) + (component_info, mut_untyped) }) } @@ -3653,9 +3657,12 @@ impl World { /// **You should prefer to use the typed API [`World::remove_resource`] where possible and only /// use this in cases where the actual types are not known at compile time.** pub fn remove_resource_by_id(&mut self, component_id: ComponentId) -> bool { - if let Some(entity) = self.resource_entities.get(component_id) + if let Some(entity) = self + .storages + .resources + .get(component_id) + .and_then(ResourceStorage::entity) && let Ok(mut entity_mut) = self.get_entity_mut(entity) - && entity_mut.contains_id(component_id) { entity_mut.remove_by_id(component_id); true diff --git a/crates/bevy_ecs/src/world/reflect.rs b/crates/bevy_ecs/src/world/reflect.rs index 0c7909d5bc4aa..a1dd4f2a45581 100644 --- a/crates/bevy_ecs/src/world/reflect.rs +++ b/crates/bevy_ecs/src/world/reflect.rs @@ -8,7 +8,7 @@ use thiserror::Error; use bevy_reflect::{PartialReflect, Reflect, ReflectFromPtr}; use bevy_utils::prelude::DebugName; -use crate::{prelude::*, world::ComponentId}; +use crate::{prelude::*, storage::ResourceStorage, world::ComponentId}; impl World { /// Retrieves a reference to the given `entity`'s [`Component`] of the given `type_id` using @@ -199,7 +199,12 @@ impl World { resource_id: ComponentId, reflected_resource: Box, ) { - if let Some(entity) = self.resource_entities().get(resource_id) { + if let Some(entity) = self + .storages + .resources + .get(resource_id) + .and_then(ResourceStorage::entity) + { self.entity_mut(entity).insert_reflect(reflected_resource); } else { self.spawn_empty().insert_reflect(reflected_resource); diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 7a9c7a8fa3757..46d7e5e7920f9 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -17,8 +17,8 @@ use crate::{ observer::Observers, prelude::Component, query::{DebugCheckedUnwrap, QueryAccessError, ReleaseStateQueryData, SingleEntityQueryData}, - resource::{Resource, ResourceEntities}, - storage::{ComponentSparseSet, Storages, Table}, + resource::Resource, + storage::{ComponentSparseSet, ResourceStorage, Storages, Table}, world::RawCommandQueue, }; use bevy_platform::sync::atomic::Ordering; @@ -280,23 +280,23 @@ impl<'w> UnsafeWorldCell<'w> { &unsafe { self.world_metadata() }.archetypes } - /// Retrieves this world's [`Components`] collection. + /// Mutably retrieves this world's [`Archetypes`] collection. + /// + /// # Safety + /// No other reference to the archetypes may exist #[inline] - pub fn components(self) -> &'w Components { + pub(crate) fn archetypes_mut(self) -> &'w mut Archetypes { // SAFETY: // - we only access world metadata - &unsafe { self.world_metadata() }.components + unsafe { &mut (*self.ptr).archetypes } } - /// Retrieves this world's resource-entity map. - /// - /// # Safety - /// The caller must have exclusive read or write access to the resources that are updated in the cache. + /// Retrieves this world's [`Components`] collection. #[inline] - pub unsafe fn resource_entities(self) -> &'w ResourceEntities { + pub fn components(self) -> &'w Components { // SAFETY: // - we only access world metadata - &unsafe { self.world_metadata() }.resource_entities + &unsafe { self.world_metadata() }.components } /// Retrieves this world's collection of [removed components](RemovedComponentMessages). @@ -465,9 +465,10 @@ impl<'w> UnsafeWorldCell<'w> { #[inline] pub unsafe fn get_resource_by_id(self, component_id: ComponentId) -> Option> { // SAFETY: We have permission to access the resource of `component_id`. - let entity = unsafe { self.resource_entities() }.get(component_id)?; - let entity_cell = self.get_entity(entity).ok()?; - entity_cell.get_by_id(component_id) + unsafe { self.storages() } + .resources + .get(component_id) + .and_then(ResourceStorage::get) } /// Gets a reference to a non-send resource of the given type if it exists. @@ -573,10 +574,25 @@ impl<'w> UnsafeWorldCell<'w> { component_id: ComponentId, ) -> Option> { self.assert_allows_mutable_access(); - // SAFETY: We have permission to access the resource of `component_id`. - let entity = unsafe { self.resource_entities() }.get(component_id)?; - let entity_cell = self.get_entity(entity).ok()?; - entity_cell.get_mut_by_id(component_id).ok() + + let info = self.components().get_info(component_id)?; + + // If a component is immutable then a mutable reference to it doesn't exist + if !info.mutable() { + return None; + } + + self.fetch_resource(component_id)? + .get_with_ticks() + .map(|(value, cells)| MutUntyped { + // SAFETY: world access validated by caller and ties world lifetime to `MutUntyped` lifetime + value: unsafe { value.assert_unique() }, + ticks: ComponentTicksMut::from_tick_cells( + cells, + self.last_change_tick(), + self.change_tick(), + ), + }) } /// Gets a mutable reference to the non-send resource of the given type if it exists @@ -678,15 +694,7 @@ impl<'w> UnsafeWorldCell<'w> { component_id: ComponentId, ) -> Option<(Ptr<'w>, ComponentTickCells<'w>)> { // SAFETY: We have permission to access the resource of `component_id`. - let entity = unsafe { self.resource_entities() }.get(component_id)?; - let storage_type = self.components().get_info(component_id)?.storage_type(); - let location = self.get_entity(entity).ok()?.location(); - // SAFETY: - // - caller ensures there is no `&mut World` - // - caller ensures there are no mutable borrows of this resource - // - caller ensures that we have permission to access this resource - // - storage_type and location are valid - get_component_and_ticks(self, component_id, storage_type, entity, location) + unsafe { self.fetch_resource(component_id) }?.get_with_ticks() } // Shorthand helper function for getting the data and change ticks for a resource. @@ -1267,6 +1275,17 @@ impl<'w> UnsafeWorldCell<'w> { // of component/resource data unsafe { self.storages() }.sparse_sets.get(component_id) } + + #[inline] + /// # Safety + /// - the returned `ComponentSparseSet` is only used in ways that this [`UnsafeWorldCell`] has permission for. + /// - the returned `ComponentSparseSet` is only used in ways that would not conflict with any existing + /// borrows of world data. + unsafe fn fetch_resource(self, component_id: ComponentId) -> Option<&'w ResourceStorage> { + // SAFETY: caller ensures returned data is not misused and we have not created any borrows + // of component/resource data + unsafe { self.storages() }.resources.get(component_id) + } } /// Get an untyped pointer to a particular [`Component`] on a particular [`Entity`] in the provided [`World`]. @@ -1293,6 +1312,7 @@ unsafe fn get_component( table.get_component(component_id, location.table_row) } StorageType::SparseSet => world.fetch_sparse_set(component_id)?.get(entity), + StorageType::Resource => world.fetch_resource(component_id)?.get_with_entity(entity), } } @@ -1332,6 +1352,7 @@ unsafe fn get_component_and_ticks( )) } StorageType::SparseSet => world.fetch_sparse_set(component_id)?.get_with_ticks(entity), + StorageType::Resource => world.fetch_resource(component_id)?.get_with_ticks(), } } @@ -1358,6 +1379,7 @@ unsafe fn get_ticks( table.get_ticks_unchecked(component_id, location.table_row) } StorageType::SparseSet => world.fetch_sparse_set(component_id)?.get_ticks(entity), + StorageType::Resource => world.fetch_resource(component_id)?.get_ticks(), } } @@ -1383,6 +1405,7 @@ unsafe fn get_changed_by( .fetch_table(location)? .get_changed_by(component_id, location.table_row), StorageType::SparseSet => world.fetch_sparse_set(component_id)?.get_changed_by(entity), + StorageType::Resource => world.fetch_resource(component_id)?.get_changed_by(), }; Some( caller diff --git a/crates/bevy_remote/src/builtin_methods.rs b/crates/bevy_remote/src/builtin_methods.rs index 2f10c3397d6a0..ebff389edd668 100644 --- a/crates/bevy_remote/src/builtin_methods.rs +++ b/crates/bevy_remote/src/builtin_methods.rs @@ -2013,11 +2013,19 @@ fn get_resource_entity_pair( let component_id = world .components() .get_id(type_id) - .ok_or(anyhow!("Resource not registered: `{}`", resource_path))?; + .ok_or_else(|| anyhow!("Resource not registered: `{}`", resource_path))?; let entity = world - .resource_entities() + .storages() + .resources .get(component_id) - .ok_or(anyhow!("Resource entity does not exist."))?; + .and_then(|storage| { + if storage.populated() { + storage.entity() + } else { + None + } + }) + .ok_or_else(|| anyhow!("Resource entity does not exist."))?; Ok((entity, component_id)) } diff --git a/crates/bevy_remote/src/schemas/json_schema.rs b/crates/bevy_remote/src/schemas/json_schema.rs index 9324f26c09537..03a02144f78cb 100644 --- a/crates/bevy_remote/src/schemas/json_schema.rs +++ b/crates/bevy_remote/src/schemas/json_schema.rs @@ -374,6 +374,8 @@ pub enum StorageKind { Table, /// Provides fast addition and removal of components, but slower iteration. SparseSet, + /// Used by resources to guarantee uniqueness. + Resource, } impl From for StorageKind { @@ -381,6 +383,7 @@ impl From for StorageKind { match value { StorageType::Table => StorageKind::Table, StorageType::SparseSet => StorageKind::SparseSet, + StorageType::Resource => StorageKind::Resource, } } } diff --git a/crates/bevy_settings/src/lib.rs b/crates/bevy_settings/src/lib.rs index 4b9c4eff84fae..b54e03ee11be0 100644 --- a/crates/bevy_settings/src/lib.rs +++ b/crates/bevy_settings/src/lib.rs @@ -22,6 +22,7 @@ use bevy_ecs::{ change_detection::Tick, reflect::{AppTypeRegistry, ReflectComponent, ReflectResource}, resource::Resource, + storage::ResourceStorage, system::{Command, Commands, Res, ResMut}, world::World, }; @@ -341,7 +342,12 @@ fn resources_to_toml( continue; }; - let Some(res_entity) = world.resource_entities().get(component_id) else { + let Some(res_entity) = world + .storages() + .resources + .get(component_id) + .and_then(ResourceStorage::entity) + else { continue; }; let res_entity_ref = world.entity(res_entity); @@ -470,7 +476,13 @@ fn apply_settings_to_world( let reflect_component = ty.data::().unwrap(); let component_id = world.components().get_id(*tid); - let res_entity = component_id.and_then(|cid| world.resource_entities().get(cid)); + let res_entity = component_id.and_then(|cid| { + world + .storages() + .resources + .get(cid) + .and_then(ResourceStorage::entity) + }); if let Some(res_entity) = res_entity { // Resource already exists, so apply toml properties to it. diff --git a/crates/bevy_world_serialization/src/dynamic_world.rs b/crates/bevy_world_serialization/src/dynamic_world.rs index 87bbdcbb8c154..01541ae4d3680 100644 --- a/crates/bevy_world_serialization/src/dynamic_world.rs +++ b/crates/bevy_world_serialization/src/dynamic_world.rs @@ -1,6 +1,7 @@ use crate::{DynamicWorldBuilder, WorldAsset, WorldInstanceSpawnError}; use bevy_asset::Asset; use bevy_ecs::reflect::ReflectResource; +use bevy_ecs::storage::ResourceStorage; use bevy_ecs::{ entity::{Entity, EntityHashMap, SceneEntityMapper}, reflect::{AppTypeRegistry, ReflectComponent}, @@ -183,7 +184,12 @@ impl DynamicWorld { let resource_id = reflect_component.register_component(world); // check if the resource already exists, if not spawn it, otherwise override the value - let entity = if let Some(entity) = world.resource_entities().get(resource_id) { + let entity = if let Some(entity) = world + .storages() + .resources + .get(resource_id) + .and_then(ResourceStorage::entity) + { entity } else { world.spawn_empty().id() diff --git a/crates/bevy_world_serialization/src/dynamic_world_builder.rs b/crates/bevy_world_serialization/src/dynamic_world_builder.rs index 727d7adc9fecc..72404f8c40a46 100644 --- a/crates/bevy_world_serialization/src/dynamic_world_builder.rs +++ b/crates/bevy_world_serialization/src/dynamic_world_builder.rs @@ -377,7 +377,7 @@ impl<'w> DynamicWorldBuilder<'w> { .components() .get_valid_id(TypeId::of::()); - for (component_id, entity) in self.original_world.resource_entities().iter() { + for (component_id, entity) in self.original_world.storages().resources.iter() { if Some(component_id) == original_world_dqf_id { continue; } diff --git a/crates/bevy_world_serialization/src/world_asset.rs b/crates/bevy_world_serialization/src/world_asset.rs index 458974e898533..319f8a395795e 100644 --- a/crates/bevy_world_serialization/src/world_asset.rs +++ b/crates/bevy_world_serialization/src/world_asset.rs @@ -4,6 +4,7 @@ use crate::reflect_utils::clone_reflect_value; use crate::{DynamicWorld, WorldInstanceSpawnError}; use bevy_asset::Asset; use bevy_ecs::resource::IS_RESOURCE; +use bevy_ecs::storage::ResourceStorage; use bevy_ecs::{ component::ComponentCloneBehavior, entity::{Entity, EntityHashMap, SceneEntityMapper}, @@ -75,7 +76,7 @@ impl WorldAsset { .get_id(TypeId::of::()); // Resources archetype - for (component_id, source_entity) in self.world.resource_entities().iter() { + for (component_id, source_entity) in self.world.storages().resources.iter() { if Some(component_id) == self_dqf_id { continue; } @@ -113,12 +114,16 @@ impl WorldAsset { .expect("ReflectComponent is depended on ReflectResource"); // check if the resource already exists in the other world, if not spawn it - let destination_entity = - if let Some(entity) = world.resource_entities().get(component_id) { - entity - } else { - world.spawn_empty().id() - }; + let destination_entity = if let Some(entity) = world + .storages() + .resources + .get(component_id) + .and_then(ResourceStorage::entity) + { + entity + } else { + world.spawn_empty().id() + }; reflect_component.copy( &self.world,