diff --git a/_release-content/migration-guides/components-as-entities.md b/_release-content/migration-guides/components-as-entities.md new file mode 100644 index 0000000000000..6408add58a080 --- /dev/null +++ b/_release-content/migration-guides/components-as-entities.md @@ -0,0 +1,36 @@ +--- +title: "Components as Entities" +pull_requests: [24728] +--- + +- `ComponentId::new` now takes `Entity` as an argument instead of `usize`. For debugging, you can use `ComponentId::from_u32`. +- `ComponentId::index` was removed in favor of implementing `ContainsEntity`, call `ComponentId::entity` to get the underlying entity. +- `ComponentIdSet` is now an `EntityEquivalentHashSet` instead of a `FixedBitSet`. + - `ComponentIdSet::is_clear` has changed to `ComponentIdSet::is_empty`. + - `ComponentIdSet::difference` has changed to `-`, i.e.: `difference = set - other`. + - `ComponentIdSet::intersection` has changed to `&`, i.e.: `intersection = set & other`. + - `ComponentIdSet::union` has changed to `|`, i.e.: `union = set | other`. + - Other methods have remained the same. +- `ComponentIds` has been removed. Instead of `ComponentIds`, `ComponentsRegistrator::new` now takes `EntityAllocator`, while `ComponentsQueuedRegistrator::new` now takes `RemoteAllocator`. +- `Access` and `EcsAccessType` no longer derive `Hash`. +- `ResourceEntities` was removed. The following methods have been removed with it: `World::resource_entities`, `EntityWorldMut::resource_entities`, `UnsafeWorldCell::resource_entities`. It can also no longer be used as a system param. If you need the entity linked with a `ComponentId`, simply call `component_id.entity()`. +- The `component_id` field has been removed from the `IsResource` component, along with `IsResource::new` and `IsResource::resource_component_id`. If you need the `ComponentId` for a resource dynamically, you can simply query `Entity` alongside the resource you want and wrap it: `ComponentId::new(entity)`. + +In 0.19, you could attach components to a resource by simply calling `world.spawn((Res1, Comp1, Comp2))`. In 0.20, this no longer works as `Res1` needs to be on the resource entity allocated by `world.register_component()`. In 0.20, adding components looks as follows: + +```rust +let entity = world.register_component::().entity(); +world.spawn_at(entity, (Res1, Comp1, Comp2)); +``` + +Additionally, manually implementing `Resource` through + +```rust +#[derive(Component, Default)] +struct R; + +impl Resource for R {} +``` + +has become less viable, as now `Res` and `ResMut` panic when `IsResource` has not been made a required component for a resource. +Use `#[derive(Resource)]` instead. diff --git a/benches/benches/bevy_ecs/empty_archetypes.rs b/benches/benches/bevy_ecs/empty_archetypes.rs index 4938120ae9a12..1bd4a6cccf3f6 100644 --- a/benches/benches/bevy_ecs/empty_archetypes.rs +++ b/benches/benches/bevy_ecs/empty_archetypes.rs @@ -156,6 +156,7 @@ fn add_archetypes(world: &mut World, count: u16) { if i & (1 << 15) != 0 { e.insert(A::<28>(1.0)); } + e.despawn(); } } @@ -166,7 +167,6 @@ fn empty_archetypes(criterion: &mut Criterion) { schedule.add_systems(iter); }); add_archetypes(&mut world, archetype_count); - world.clear_entities(); let mut e = world.spawn_empty(); e.insert(A::<0>(1.0)); e.insert(A::<1>(1.0)); @@ -197,7 +197,6 @@ fn empty_archetypes(criterion: &mut Criterion) { schedule.add_systems(for_each); }); add_archetypes(&mut world, archetype_count); - world.clear_entities(); let mut e = world.spawn_empty(); e.insert(A::<0>(1.0)); e.insert(A::<1>(1.0)); @@ -228,7 +227,6 @@ fn empty_archetypes(criterion: &mut Criterion) { schedule.add_systems(par_for_each); }); add_archetypes(&mut world, archetype_count); - world.clear_entities(); let mut e = world.spawn_empty(); e.insert(A::<0>(1.0)); e.insert(A::<1>(1.0)); diff --git a/crates/bevy_dev_tools/src/inspection/component_inspection.rs b/crates/bevy_dev_tools/src/inspection/component_inspection.rs index b59d6847405aa..e047644c397c8 100644 --- a/crates/bevy_dev_tools/src/inspection/component_inspection.rs +++ b/crates/bevy_dev_tools/src/inspection/component_inspection.rs @@ -175,10 +175,9 @@ impl ComponentMetadataMap { pub fn generate(world: &World) -> Self { let mut map = HashMap::new(); - for index in 0..world.components().num_registered() { - let component_id = ComponentId::new(index); - if let Ok(metadata) = ComponentTypeMetadata::new(world, component_id) { - map.insert(component_id, metadata); + for (id, _) in world.components().iter_registered() { + if let Ok(metadata) = ComponentTypeMetadata::new(world, id) { + map.insert(id, metadata); } } @@ -209,12 +208,11 @@ impl ComponentMetadataMap { /// Adds entries for component types that are not yet in the map, leaving existing entries alone. pub fn update(&mut self, world: &World) { - for index in 0..world.components().num_registered() { - let component_id = ComponentId::new(index); - if !self.map.contains_key(&component_id) - && let Ok(metadata) = ComponentTypeMetadata::new(world, component_id) + for (id, _) in world.components().iter_registered() { + if !self.map.contains_key(&id) + && let Ok(metadata) = ComponentTypeMetadata::new(world, id) { - self.map.insert(component_id, metadata); + self.map.insert(id, metadata); } } } @@ -449,7 +447,7 @@ mod tests { #[test] fn unregistered_component_id_returns_error() { let world = test_world(); - let component_id = ComponentId::new(usize::MAX); + let component_id = ComponentId::from_u32(464149); let result = ComponentTypeMetadata::new(&world, component_id); diff --git a/crates/bevy_dev_tools/src/inspection/world_summary.rs b/crates/bevy_dev_tools/src/inspection/world_summary.rs index 8a015d1c18e62..fed269058cf7d 100644 --- a/crates/bevy_dev_tools/src/inspection/world_summary.rs +++ b/crates/bevy_dev_tools/src/inspection/world_summary.rs @@ -2,7 +2,10 @@ //! //! See [`WorldSummary`] for the output, and [`WorldSummaryExt`] to generate. -use bevy_ecs::{archetype::ArchetypeId, component::ComponentId, system::Commands, world::World}; +use bevy_ecs::{ + archetype::ArchetypeId, component::ComponentId, storage::SparseSetIndex, system::Commands, + world::World, +}; use bevy_log::info; use bevy_utils::{memory_size::MemorySize, prelude::DebugName}; use core::{cmp::Reverse, fmt}; @@ -143,7 +146,7 @@ pub trait WorldSummaryExt { impl WorldSummaryExt for World { fn summarize(&self, settings: SummarySettings) -> WorldSummary { let total_entities = self.entities().count_spawned(); - let total_send_resources = self.resource_entities().iter().count(); + let total_send_resources = self.iter_resources().count(); let total_non_send_resources = self.storages().non_sends.len(); let total_archetypes = self.archetypes().len(); let mut archetype_summaries: Vec = self @@ -161,7 +164,7 @@ impl WorldSummaryExt for World { self.components() .get_name(*component_id) .unwrap_or_else(|| { - let component_index = component_id.index(); + let component_index = component_id.sparse_set_index(); DebugName::owned(format!("Component #{component_index}")) }) }) diff --git a/crates/bevy_dev_tools/src/schedule_data/serde.rs b/crates/bevy_dev_tools/src/schedule_data/serde.rs index ddbc762f3536c..ace6c8f3e5e38 100644 --- a/crates/bevy_dev_tools/src/schedule_data/serde.rs +++ b/crates/bevy_dev_tools/src/schedule_data/serde.rs @@ -129,11 +129,11 @@ impl AccessData { let writes = value.writes().as_finite_set(); let (reads_inverted, reads) = match reads { - Some(reads) => (false, trace.get_indexes(reads.iter())), + Some(reads) => (false, trace.get_indexes(reads.iter().copied())), None => (true, vec![]), }; let (writes_inverted, writes) = match writes { - Some(writes) => (false, trace.get_indexes(writes.iter())), + Some(writes) => (false, trace.get_indexes(writes.iter().copied())), None => (true, vec![]), }; @@ -142,7 +142,7 @@ impl AccessData { writes, reads_inverted, writes_inverted, - archetypal: trace.get_indexes(value.archetypal().iter()), + archetypal: trace.get_indexes(value.archetypal().iter().copied()), } } } @@ -164,8 +164,8 @@ pub struct AccessFiltersData { impl AccessFiltersData { fn new(value: &bevy_ecs::query::AccessFilters, trace: &mut ComponentTrace) -> Self { Self { - with: trace.get_indexes(value.with().iter()), - without: trace.get_indexes(value.without().iter()), + with: trace.get_indexes(value.with().iter().copied()), + without: trace.get_indexes(value.without().iter().copied()), } } } diff --git a/crates/bevy_ecs/macro_logic/src/component.rs b/crates/bevy_ecs/macro_logic/src/component.rs index cdf131161f1ac..8da1eae5f4acc 100644 --- a/crates/bevy_ecs/macro_logic/src/component.rs +++ b/crates/bevy_ecs/macro_logic/src/component.rs @@ -53,6 +53,8 @@ pub struct DeriveComponent { pub map_entities: Option, /// Additional required component registrations that are added in `Component::register_required_components` pub additional_requires: Vec, + /// Additional `on_insert` hook + pub additional_insert_hook: Option, } impl DeriveComponent { @@ -73,6 +75,7 @@ impl DeriveComponent { clone_behavior: None, map_entities: None, additional_requires: Vec::new(), + additional_insert_hook: None, }; let mut require_paths = HashSet::new(); @@ -250,6 +253,10 @@ impl DeriveComponent { let mut on_despawn_path = Vec::from_iter(self.on_despawn.map(|path| path.to_token_stream(bevy_ecs))); + if let Some(extra_insert_hook) = self.additional_insert_hook { + on_insert_path.push(extra_insert_hook); + } + if relationship.is_some() { on_insert_path.push(quote!(::on_insert)); on_discard_path diff --git a/crates/bevy_ecs/macros/src/resource.rs b/crates/bevy_ecs/macros/src/resource.rs index 2401d63a35d7e..f0e9dee24b876 100644 --- a/crates/bevy_ecs/macros/src/resource.rs +++ b/crates/bevy_ecs/macros/src/resource.rs @@ -1,5 +1,4 @@ use bevy_ecs_macro_logic::component::{DeriveComponent, StorageAttribute, StorageTy}; -use bevy_macro_utils::fq_std::FQOption; use proc_macro2::TokenStream; use quote::quote; use syn::{DeriveInput, Path}; @@ -11,17 +10,12 @@ pub fn derive_resource(ast: &mut DeriveInput) -> TokenStream { Err(e) => return e.into_compile_error(), }; - let struct_name = &ast.ident; - let (_, type_generics, _) = &ast.generics.split_for_impl(); + // We add an extra insert hook to the resource + derive_component.additional_insert_hook = Some(quote!(#bevy_ecs::resource::on_resource_insert)); // We add the component_id existence check here to avoid recursive init during required components initialization. derive_component.additional_requires.push(quote! { - let resource_component_id = if let #FQOption::Some(id) = required_components.components_registrator().component_id::<#struct_name #type_generics>() { - id - } else { - required_components.components_registrator().register_component::<#struct_name #type_generics>() - }; - required_components.register_required::<#bevy_ecs::resource::IsResource>(move || #bevy_ecs::resource::IsResource::new(resource_component_id)); + required_components.register_required::<#bevy_ecs::resource::IsResource>(|| #bevy_ecs::resource::IsResource); }); let component_impl = match derive_component.impl_component(ast, &bevy_ecs, StorageTy::SparseSet) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index f42f7edb80426..c91946d2cacd6 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -9,7 +9,7 @@ //! Archetypes are not to be confused with [`Table`]s. Each archetype stores its table //! components in one table, and each archetype uniquely points to one table, but multiple //! archetypes may store their table components in the same table. These archetypes -//! differ only by the [`SparseSet`] components. +//! differ only by the [`crate::storage::SparseSet`] components. //! //! Like tables, archetypes can be created but are never cleaned up. Empty archetypes are //! not removed, and persist until the world is dropped. @@ -21,12 +21,12 @@ use crate::{ bundle::BundleId, - component::{ComponentId, Components, RequiredComponentConstructor, StorageType}, - entity::{Entity, EntityLocation}, + component::{ComponentId, Components, RequiredComponentConstructor}, + entity::{Entity, EntityEquivalentHashMap, EntityLocation}, event::{Event, EventKey}, observer::Observers, query::DebugCheckedUnwrap, - storage::{ImmutableSparseSet, SparseArray, SparseSet, TableId, TableRow}, + storage::{SparseArray, TableId, TableRow}, }; use alloc::{boxed::Box, vec::Vec}; use bevy_platform::collections::{hash_map::Entry, HashMap}; @@ -353,13 +353,6 @@ pub(crate) struct ArchetypeSwapRemoveResult { pub(crate) table_row: TableRow, } -/// Internal metadata for a [`Component`] within a given [`Archetype`]. -/// -/// [`Component`]: crate::component::Component -struct ArchetypeComponentInfo { - storage_type: StorageType, -} - bitflags::bitflags! { /// Flags used to keep track of metadata about the component in this [`Archetype`] /// @@ -390,7 +383,7 @@ pub struct Archetype { table_id: TableId, edges: Edges, entities: Vec, - components: ImmutableSparseSet, + component_ids: Box<[ComponentId]>, pub(crate) flags: ArchetypeFlags, } @@ -408,18 +401,14 @@ impl Archetype { let (min_table, _) = table_components.size_hint(); let (min_sparse, _) = sparse_set_components.size_hint(); let mut flags = ArchetypeFlags::empty(); - let mut archetype_components = SparseSet::with_capacity(min_table + min_sparse); + let mut component_ids = Vec::with_capacity(min_table + min_sparse); 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) }; info.update_archetype_flags(&mut flags); observers.update_archetype_flags(component_id, &mut flags); - archetype_components.insert( - component_id, - ArchetypeComponentInfo { - storage_type: StorageType::Table, - }, - ); + component_ids.push(component_id); + // NOTE: the `table_components` are sorted AND they were inserted in the `Table` in the same // sorted order, so the index of the `Column` in the `Table` is the same as the index of the // component in the `table_components` vector @@ -434,22 +423,19 @@ impl Archetype { let info = unsafe { components.get_info_unchecked(component_id) }; info.update_archetype_flags(&mut flags); observers.update_archetype_flags(component_id, &mut flags); - archetype_components.insert( - component_id, - ArchetypeComponentInfo { - storage_type: StorageType::SparseSet, - }, - ); + component_ids.push(component_id); + component_index .entry(component_id) .or_default() .insert(id, ArchetypeRecord { column: None }); } + Self { id, table_id, entities: Vec::new(), - components: archetype_components.into_immutable(), + component_ids: component_ids.into_boxed_slice(), edges: Default::default(), flags, } @@ -502,38 +488,12 @@ impl Archetype { ) } - /// Gets an iterator of all of the components stored in [`Table`]s. - /// - /// All of the IDs are unique. - /// - /// [`Table`]: crate::storage::Table - #[inline] - pub fn 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. - /// - /// [`ComponentSparseSet`]: crate::storage::ComponentSparseSet - #[inline] - pub fn sparse_set_components(&self) -> impl Iterator + '_ { - self.components - .iter() - .filter(|(_, component)| component.storage_type == StorageType::SparseSet) - .map(|(id, _)| *id) - } - /// Returns a slice of all of the components in the archetype. /// /// All of the IDs are unique. #[inline] pub fn components(&self) -> &[ComponentId] { - self.components.indices() + &self.component_ids } /// Gets an iterator of all of the components in the archetype. @@ -541,13 +501,13 @@ impl Archetype { /// All of the IDs are unique. #[inline] pub fn iter_components(&self) -> impl Iterator + Clone { - self.components.indices().iter().copied() + self.component_ids.iter().copied() } /// Returns the total number of components in the archetype #[inline] pub fn component_count(&self) -> usize { - self.components.len() + self.component_ids.len() } /// Fetches an immutable reference to the archetype's [`Edges`], a cache of @@ -652,20 +612,10 @@ impl Archetype { self.entities.is_empty() } - /// Checks if the archetype contains a specific component. This runs in `O(1)` time. + /// Checks if the archetype contains a specific component. This runs in `O(N)` time. #[inline] pub fn contains(&self, component_id: ComponentId) -> bool { - self.components.contains(component_id) - } - - /// Gets the type of storage where a component in the archetype can be found. - /// Returns `None` if the component is not part of the archetype. - /// This runs in `O(1)` time. - #[inline] - pub fn get_storage_type(&self, component_id: ComponentId) -> Option { - self.components - .get(component_id) - .map(|info| info.storage_type) + self.component_ids.contains(&component_id) } /// Clears all entities from the archetype. @@ -767,7 +717,8 @@ struct ArchetypeComponents { /// Maps a [`ComponentId`] to the list of [`Archetypes`]([`Archetype`]) that contain the [`Component`](crate::component::Component), /// along with an [`ArchetypeRecord`] which contains some metadata about how the component is stored in the archetype. -pub type ComponentIndex = HashMap>; +pub type ComponentIndex = + EntityEquivalentHashMap>; /// The backing store of all [`Archetype`]s within a [`World`]. /// diff --git a/crates/bevy_ecs/src/bundle/insert.rs b/crates/bevy_ecs/src/bundle/insert.rs index b0dce8a76c040..cc7622e0deea4 100644 --- a/crates/bevy_ecs/src/bundle/insert.rs +++ b/crates/bevy_ecs/src/bundle/insert.rs @@ -617,12 +617,21 @@ impl BundleInfo { // The archetype changes when we insert this bundle. Prepare the new archetype and storages. { let current_archetype = &archetypes[archetype_id]; + let (current_table_components, current_sparse_set_components): (Vec<_>, Vec<_>) = + current_archetype + .iter_components() + .partition(|&component_id| { + // SAFETY: Every component in an archetype is registered in this world. + unsafe { components.get_info_unchecked(component_id) }.storage_type() + == StorageType::Table + }); + table_components = if new_table_components.is_empty() { // If there are no new table components, we can keep using this table. table_id = current_archetype.table_id(); - current_archetype.table_components().collect() + current_table_components } else { - new_table_components.extend(current_archetype.table_components()); + new_table_components.extend(current_table_components); // Sort to ignore order while hashing. new_table_components.sort_unstable(); // SAFETY: all component ids in `new_table_components` exist @@ -636,9 +645,9 @@ impl BundleInfo { }; sparse_set_components = if new_sparse_set_components.is_empty() { - current_archetype.sparse_set_components().collect() + current_sparse_set_components } else { - new_sparse_set_components.extend(current_archetype.sparse_set_components()); + new_sparse_set_components.extend(current_sparse_set_components); // Sort to ignore order while hashing. new_sparse_set_components.sort_unstable(); new_sparse_set_components diff --git a/crates/bevy_ecs/src/bundle/remove.rs b/crates/bevy_ecs/src/bundle/remove.rs index 4e3edf9cf4516..d61f6e52817c1 100644 --- a/crates/bevy_ecs/src/bundle/remove.rs +++ b/crates/bevy_ecs/src/bundle/remove.rs @@ -238,9 +238,12 @@ impl<'w> BundleRemover<'w> { if old_archetype.contains(component_id) { world.removed_components.write(component_id, entity); + // SAFETY: `component_id` is valid because it comes from the bundle's components. + let storage_type = + unsafe { world.components.get_info_unchecked(component_id) }.storage_type(); // 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) = old_archetype.get_storage_type(component_id) { + if storage_type == StorageType::SparseSet { world .storages .sparse_sets @@ -401,10 +404,7 @@ impl BundleInfo { // This bundle removal result is cached. Just return that! (result, false) } else { - let mut next_table_components; - let mut next_sparse_set_components; - let next_table_id; - { + let (next_table_components, next_sparse_set_components, 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(); @@ -432,15 +432,23 @@ impl BundleInfo { // Archetype components are already sorted. removed_table_components.sort_unstable(); removed_sparse_set_components.sort_unstable(); - next_table_components = current_archetype.table_components().collect(); - next_sparse_set_components = current_archetype.sparse_set_components().collect(); + + let (mut next_table_components, mut next_sparse_set_components): (Vec<_>, Vec<_>) = + current_archetype + .iter_components() + .partition(|&component_id| { + // SAFETY: Every archetype component is registered in this world. + unsafe { components.get_info_unchecked(component_id) }.storage_type() + == StorageType::Table + }); + sorted_remove(&mut next_table_components, &removed_table_components); sorted_remove( &mut next_sparse_set_components, &removed_sparse_set_components, ); - next_table_id = if removed_table_components.is_empty() { + let next_table_id = if removed_table_components.is_empty() { current_archetype.table_id() } else { // SAFETY: all components in next_table_components exist @@ -450,7 +458,13 @@ impl BundleInfo { .get_id_or_insert(&next_table_components, components) } }; - } + + ( + next_table_components, + next_sparse_set_components, + next_table_id, + ) + }; // SAFETY: // - table id was created if it doesn't exist diff --git a/crates/bevy_ecs/src/change_detection/params.rs b/crates/bevy_ecs/src/change_detection/params.rs index c1dcfd3ea4172..73cb24be2116c 100644 --- a/crates/bevy_ecs/src/change_detection/params.rs +++ b/crates/bevy_ecs/src/change_detection/params.rs @@ -95,7 +95,7 @@ impl<'w> ContiguousComponentTicksRef<'w> { } /// Creates a new `ContiguousComponentTicksRef` using provided values or returns [`None`] if lengths of - /// `added`, `changed` and `changed_by` do not match + /// `added`, `changed` and `changed_by` do not match /// /// This is an advanced feature, `ContiguousComponentTicksRef`s are designed to be _created_ by /// engine-internal code and _consumed_ by end-user code. @@ -338,7 +338,7 @@ impl<'w> ContiguousComponentTicksMut<'w> { } /// Creates a new `ContiguousComponentTicksMut` using provided values or returns [`None`] if lengths of - /// `added`, `changed` and `changed_by` do not match + /// `added`, `changed` and `changed_by` do not match /// /// This is an advanced feature, `ContiguousComponentTicksMut`s are designed to be _created_ by /// engine-internal code and _consumed_ by end-user code. @@ -869,7 +869,7 @@ impl<'w, T> ContiguousRef<'w, T> { } /// Creates a new `ContiguousRef` using provided values or returns [`None`] if lengths of - /// `value`, `added`, `changed` and `changed_by` do not match + /// `value`, `added`, `changed` and `changed_by` do not match /// /// This is an advanced feature, `ContiguousRef`s are designed to be _created_ by /// engine-internal code and _consumed_ by end-user code. @@ -1153,7 +1153,7 @@ impl<'w, T> ContiguousMut<'w, T> { } /// Creates a new `ContiguousMut` using provided values or returns [`None`] if lengths of - /// `value`, `added`, `changed` and `changed_by` do not match + /// `value`, `added`, `changed` and `changed_by` do not match /// /// This is an advanced feature, `ContiguousMut`s are designed to be _created_ by /// engine-internal code and _consumed_ by end-user code. diff --git a/crates/bevy_ecs/src/component/constants.rs b/crates/bevy_ecs/src/component/constants.rs index 000bbd801883b..49203cb70794b 100644 --- a/crates/bevy_ecs/src/component/constants.rs +++ b/crates/bevy_ecs/src/component/constants.rs @@ -3,16 +3,16 @@ use crate::component::ComponentId; /// [`ComponentId`] of the [`Add`](crate::lifecycle::Add) component used in lifecycle observers. -pub const ADD: ComponentId = ComponentId::new(0); +pub const ADD: ComponentId = ComponentId::from_u32(0); /// [`ComponentId`] of the [`Insert`](crate::lifecycle::Insert) component used in lifecycle observers. -pub const INSERT: ComponentId = ComponentId::new(1); +pub const INSERT: ComponentId = ComponentId::from_u32(1); /// [`ComponentId`] of the [`Discard`](crate::lifecycle::Discard) component used in lifecycle observers. -pub const DISCARD: ComponentId = ComponentId::new(2); +pub const DISCARD: ComponentId = ComponentId::from_u32(2); /// [`ComponentId`] of the [`Remove`](crate::lifecycle::Remove) component used in lifecycle observers. -pub const REMOVE: ComponentId = ComponentId::new(3); +pub const REMOVE: ComponentId = ComponentId::from_u32(3); /// [`ComponentId`] of the [`Despawn`](crate::lifecycle::Despawn) component used in lifecycle observers. -pub const DESPAWN: ComponentId = ComponentId::new(4); +pub const DESPAWN: ComponentId = ComponentId::from_u32(4); /// [`ComponentId`] of the [`IsResource`](crate::resource::IsResource) component used to mark entities with resources. -pub const IS_RESOURCE: ComponentId = ComponentId::new(5); +pub const IS_RESOURCE: ComponentId = ComponentId::from_u32(5); /// [`ComponentId`] of the [`ArchetypeCreated`](crate::archetype::ArchetypeCreated) component used as an observer event key. -pub(crate) const ARCHETYPE_CREATED: ComponentId = ComponentId::new(6); +pub(crate) const ARCHETYPE_CREATED: ComponentId = ComponentId::from_u32(6); diff --git a/crates/bevy_ecs/src/component/info.rs b/crates/bevy_ecs/src/component/info.rs index 29bcd10eda3ee..2a49393fbe6e3 100644 --- a/crates/bevy_ecs/src/component/info.rs +++ b/crates/bevy_ecs/src/component/info.rs @@ -1,4 +1,4 @@ -use alloc::{borrow::Cow, vec::Vec}; +use alloc::borrow::Cow; use bevy_platform::{hash::FixedHasher, sync::PoisonError}; use bevy_ptr::OwningPtr; #[cfg(feature = "bevy_reflect")] @@ -18,6 +18,9 @@ use crate::{ Component, ComponentCloneBehavior, ComponentMutability, QueuedComponents, RequiredComponents, StorageType, }, + entity::{ + ContainsEntity, Entity, EntityEquivalent, EntityEquivalentHashMap, EntityEquivalentHashSet, + }, lifecycle::ComponentHooks, query::DebugCheckedUnwrap as _, relationship::{ @@ -177,37 +180,53 @@ impl ComponentInfo { derive(Reflect), reflect(Debug, Hash, PartialEq, Clone) )] -pub struct ComponentId(usize); +pub struct ComponentId(pub(super) Entity); + +impl ContainsEntity for ComponentId { + fn entity(&self) -> Entity { + self.0 + } +} + +// SAFETY: EntityWrapper is a newtype around Entity that derives its comparison traits. +unsafe impl EntityEquivalent for ComponentId {} impl ComponentId { - /// Creates a new [`ComponentId`]. - /// - /// The `index` is a unique value associated with each type of component in a given world. - /// Usually, this value is taken from a counter incremented for each type of component registered with the world. + /// Creates a new [`ComponentId`] from an entity. + /// Usually this entity is created by an [`EntityAllocator`](crate::entity::EntityAllocator). + /// `Entity::PLACEHOLDER` is not valid for `ComponentId`. #[inline] - pub const fn new(index: usize) -> ComponentId { - ComponentId(index) + pub const fn new(entity: Entity) -> ComponentId { + ComponentId(entity) } - /// Returns the index of the current component. + /// Creates a [`ComponentId`] from a u32. + /// This is for debugging purposes, because in a real application you have to ensure the id doesn't conflict with any other entity. + /// Moreover, this function panics when `index` is `u32::MAX`. #[inline] - pub fn index(self) -> usize { - self.0 + pub const fn from_u32(index: u32) -> ComponentId { + ComponentId(Entity::from_raw_u32(index).unwrap()) } } impl SparseSetIndex for ComponentId { #[inline] fn sparse_set_index(&self) -> usize { - self.index() + self.entity().sparse_set_index() } #[inline] fn get_sparse_set_index(value: usize) -> Self { - Self(value) + Self(Entity::get_sparse_set_index(value)) } } +/// A map with [`ComponentId`]s as keys. +pub type ComponentIdMap = EntityEquivalentHashMap; + +/// A set of [`ComponentId`]s. +pub type ComponentIdSet = EntityEquivalentHashSet; + /// A value describing a component or resource, which may or may not correspond to a Rust type. #[derive(Clone)] pub struct ComponentDescriptor { @@ -384,7 +403,7 @@ impl ComponentDescriptor { /// Stores metadata associated with each kind of [`Component`] in a given [`World`](crate::world::World). #[derive(Debug, Default)] pub struct Components { - pub(super) components: Vec>, + pub(super) components: ComponentIdMap, pub(super) indices: TypeIdHashMap, // This is kept internal and local to verify that no deadlocks can occur. pub(super) queued: bevy_platform::sync::RwLock, @@ -393,26 +412,18 @@ pub struct Components { impl Components { /// This registers any descriptor, component or resource. /// - /// # Safety - /// - /// The id must have never been registered before. This must be a fresh registration. + /// This function panics if the id has never been registered before. #[inline] - pub(super) unsafe fn register_component_inner( + pub(super) fn register_component_inner( &mut self, id: ComponentId, mut descriptor: ComponentDescriptor, ) { descriptor.initialize(id, self); let info = ComponentInfo::new(descriptor); - let least_len = id.0 + 1; - if self.components.len() < least_len { - self.components.resize_with(least_len, || None); - } - // SAFETY: We just extended the vec to make this index valid. - let slot = unsafe { self.components.get_mut(id.0).debug_checked_unwrap() }; - // Caller ensures id is unique - debug_assert!(slot.is_none()); - *slot = Some(info); + self.components + .try_insert(id, info) + .expect("this component has already been registered"); } /// Returns the number of components registered or queued with this instance. @@ -474,7 +485,7 @@ impl Components { /// This will return an incorrect result if `id` did not come from the same world as `self`. It may return `None` or a garbage value. #[inline] pub fn get_info(&self, id: ComponentId) -> Option<&ComponentInfo> { - self.components.get(id.0).and_then(|info| info.as_ref()) + self.components.get(&id) } /// Gets the [`ComponentDescriptor`] of the component with this [`ComponentId`] if it is present. @@ -486,8 +497,8 @@ impl Components { #[inline] pub fn get_descriptor<'a>(&'a self, id: ComponentId) -> Option> { self.components - .get(id.0) - .and_then(|info| info.as_ref().map(|info| Cow::Borrowed(&info.descriptor))) + .get(&id) + .map(|info| Cow::Borrowed(&info.descriptor)) .or_else(|| { let queued = self.queued.read().unwrap_or_else(PoisonError::into_inner); // first check components, then resources, then dynamic @@ -507,8 +518,8 @@ impl Components { #[inline] pub fn get_name<'a>(&'a self, id: ComponentId) -> Option { self.components - .get(id.0) - .and_then(|info| info.as_ref().map(|info| info.descriptor.name())) + .get(&id) + .map(|info| info.descriptor.name()) .or_else(|| { let queued = self.queued.read().unwrap_or_else(PoisonError::into_inner); // first check components, then resources, then dynamic @@ -528,27 +539,19 @@ impl Components { #[inline] pub unsafe fn get_info_unchecked(&self, id: ComponentId) -> &ComponentInfo { // SAFETY: The caller ensures `id` is valid. - unsafe { - self.components - .get(id.0) - .debug_checked_unwrap() - .as_ref() - .debug_checked_unwrap() - } + unsafe { self.components.get(&id).debug_checked_unwrap() } } #[inline] pub(crate) fn get_hooks_mut(&mut self, id: ComponentId) -> Option<&mut ComponentHooks> { - self.components - .get_mut(id.0) - .and_then(|info| info.as_mut().map(|info| &mut info.hooks)) + self.components.get_mut(&id).map(|info| &mut info.hooks) } #[inline] pub(crate) fn get_required_components(&self, id: ComponentId) -> Option<&RequiredComponents> { self.components - .get(id.0) - .and_then(|info| info.as_ref().map(|info| &info.required_components)) + .get(&id) + .map(|info| &info.required_components) } #[inline] @@ -557,8 +560,8 @@ impl Components { id: ComponentId, ) -> Option<&mut RequiredComponents> { self.components - .get_mut(id.0) - .and_then(|info| info.as_mut().map(|info| &mut info.required_components)) + .get_mut(&id) + .map(|info| &mut info.required_components) } #[inline] @@ -566,9 +569,7 @@ impl Components { &self, id: ComponentId, ) -> Option<&IndexSet> { - self.components - .get(id.0) - .and_then(|info| info.as_ref().map(|info| &info.required_by)) + self.components.get(&id).map(|info| &info.required_by) } #[inline] @@ -577,8 +578,8 @@ impl Components { id: ComponentId, ) -> Option<&mut IndexSet> { self.components - .get_mut(id.0) - .and_then(|info| info.as_mut().map(|info| &mut info.required_by)) + .get_mut(&id) + .map(|info| &mut info.required_by) } /// Returns true if the [`ComponentId`] is fully registered and valid. @@ -586,7 +587,7 @@ impl Components { /// Those ids are still correct, but they are not usable in every context yet. #[inline] pub fn is_id_valid(&self, id: ComponentId) -> bool { - self.components.get(id.0).is_some_and(Option::is_some) + self.components.get(&id).is_some() } /// Type-erased equivalent of [`Components::valid_component_id()`]. @@ -669,7 +670,6 @@ impl Components { /// # Safety /// /// The [`ComponentDescriptor`] must match the [`TypeId`]. - /// The [`ComponentId`] must be unique. /// The [`TypeId`] and [`ComponentId`] must not be registered or queued. #[inline] pub(super) unsafe fn register_non_send_unchecked( @@ -678,20 +678,14 @@ impl Components { component_id: ComponentId, descriptor: ComponentDescriptor, ) { - // SAFETY: ensured by caller - unsafe { - self.register_component_inner(component_id, descriptor); - } + self.register_component_inner(component_id, descriptor); let prev = self.indices.insert(type_id, component_id); debug_assert!(prev.is_none()); } /// Gets an iterator over all components fully registered with this instance. pub fn iter_registered(&self) -> impl Iterator + '_ { - self.components - .iter() - .enumerate() - .filter_map(|(index, info)| info.as_ref().map(|info| (ComponentId::new(index), info))) + self.components.iter().map(|(index, info)| (*index, info)) } pub(crate) fn get_relationship_accessor_mut( @@ -699,10 +693,7 @@ impl Components { component_id: ComponentId, ) -> Option<&mut MaybeRelationshipAccessor> { self.components - .get_mut(component_id.index()) - .and_then(|info| { - info.as_mut() - .map(|info| &mut info.descriptor.relationship_accessor) - }) + .get_mut(&component_id) + .map(|info| &mut info.descriptor.relationship_accessor) } } diff --git a/crates/bevy_ecs/src/component/register.rs b/crates/bevy_ecs/src/component/register.rs index 4f8c70ffe02ee..0001083253944 100644 --- a/crates/bevy_ecs/src/component/register.rs +++ b/crates/bevy_ecs/src/component/register.rs @@ -4,66 +4,20 @@ use bevy_utils::TypeIdHashMap; use core::any::Any; use core::{any::TypeId, fmt::Debug, ops::Deref}; -use crate::component::{enforce_no_required_components_recursion, RequiredComponentsRegistrator}; -use crate::lifecycle::ComponentHooks; use crate::{ component::{ - Component, ComponentDescriptor, ComponentId, Components, RequiredComponents, StorageType, + enforce_no_required_components_recursion, Component, ComponentDescriptor, ComponentId, + Components, RequiredComponents, RequiredComponentsRegistrator, StorageType, }, + entity::EntityAllocator, + lifecycle::ComponentHooks, query::DebugCheckedUnwrap as _, }; -/// Generates [`ComponentId`]s. -#[derive(Debug, Default)] -pub struct ComponentIds { - next: bevy_platform::sync::atomic::AtomicUsize, -} - -impl ComponentIds { - /// Peeks the next [`ComponentId`] to be generated without generating it. - pub fn peek(&self) -> ComponentId { - ComponentId::new( - self.next - .load(bevy_platform::sync::atomic::Ordering::Relaxed), - ) - } - - /// Generates and returns the next [`ComponentId`]. - pub fn next(&self) -> ComponentId { - ComponentId::new( - self.next - .fetch_add(1, bevy_platform::sync::atomic::Ordering::Relaxed), - ) - } - - /// Peeks the next [`ComponentId`] to be generated without generating it. - pub fn peek_mut(&mut self) -> ComponentId { - ComponentId::new(*self.next.get_mut()) - } - - /// Generates and returns the next [`ComponentId`]. - pub fn next_mut(&mut self) -> ComponentId { - let id = self.next.get_mut(); - let result = ComponentId::new(*id); - *id += 1; - result - } - - /// Returns the number of [`ComponentId`]s generated. - pub fn len(&self) -> usize { - self.peek().index() - } - - /// Returns true if and only if no ids have been generated. - pub fn is_empty(&self) -> bool { - self.len() == 0 - } -} - /// A [`Components`] wrapper that enables additional features, like registration. pub struct ComponentsRegistrator<'w> { pub(super) components: &'w mut Components, - pub(super) ids: &'w mut ComponentIds, + pub(super) allocator: &'w EntityAllocator, pub(super) recursion_check_stack: Vec, } @@ -80,12 +34,11 @@ impl<'w> ComponentsRegistrator<'w> { /// /// # Safety /// - /// The [`Components`] and [`ComponentIds`] must match. - /// For example, they must be from the same world. - pub unsafe fn new(components: &'w mut Components, ids: &'w mut ComponentIds) -> Self { + /// The [`Components`] and [`EntityAllocator`] must come from the same world. + pub unsafe fn new(components: &'w mut Components, allocator: &'w EntityAllocator) -> Self { Self { components, - ids, + allocator, recursion_check_stack: Vec::new(), } } @@ -95,7 +48,7 @@ impl<'w> ComponentsRegistrator<'w> { /// It is generally not a good idea to queue a registration when you can instead register directly on this type. pub fn as_queued(&self) -> ComponentsQueuedRegistrator<'_> { // SAFETY: ensured by the caller that created self. - unsafe { ComponentsQueuedRegistrator::new(self.components, self.ids) } + unsafe { ComponentsQueuedRegistrator::new(self.components, self.allocator) } } /// Applies every queued registration. @@ -195,7 +148,7 @@ impl<'w> ComponentsRegistrator<'w> { return registrator.register(self); } - let id = self.ids.next_mut(); + let id = ComponentId::new(self.allocator.alloc()); // SAFETY: The component is not currently registered, and the id is fresh. unsafe { self.register_component_unchecked( @@ -221,10 +174,7 @@ impl<'w> ComponentsRegistrator<'w> { register_required_components: fn(ComponentId, &mut RequiredComponentsRegistrator), update_from_component: fn(&mut ComponentHooks) -> &mut ComponentHooks, ) { - // SAFETY: ensured by caller. - unsafe { - self.components.register_component_inner(id, descriptor); - } + self.components.register_component_inner(id, descriptor); let prev = self.components.indices.insert(type_id, id); debug_assert!(prev.is_none()); @@ -249,9 +199,7 @@ impl<'w> ComponentsRegistrator<'w> { &mut self .components .components - .get_mut(id.index()) - .debug_checked_unwrap() - .as_mut() + .get_mut(&id) .debug_checked_unwrap() }; @@ -282,11 +230,8 @@ impl<'w> ComponentsRegistrator<'w> { &mut self, descriptor: ComponentDescriptor, ) -> ComponentId { - let id = self.ids.next_mut(); - // SAFETY: The id is fresh. - unsafe { - self.components.register_component_inner(id, descriptor); - } + let id = ComponentId::new(self.allocator.alloc()); + self.components.register_component_inner(id, descriptor); id } @@ -331,7 +276,7 @@ impl<'w> ComponentsRegistrator<'w> { return registrator.register(self); } - let id = self.ids.next_mut(); + let id = ComponentId::new(self.allocator.alloc()); // SAFETY: The resource is not currently registered, the id is fresh, and the [`ComponentDescriptor`] matches the [`TypeId`] unsafe { self.components @@ -425,10 +370,9 @@ impl Debug for QueuedComponents { /// /// As a rule of thumb, if you have mutable access to [`ComponentsRegistrator`], prefer to use that instead. /// Use this only if you need to know the id of a component but do not need to modify the contents of the world based on that id. -#[derive(Clone, Copy)] pub struct ComponentsQueuedRegistrator<'w> { components: &'w Components, - ids: &'w ComponentIds, + allocator: &'w EntityAllocator, } impl Deref for ComponentsQueuedRegistrator<'_> { @@ -444,10 +388,12 @@ impl<'w> ComponentsQueuedRegistrator<'w> { /// /// # Safety /// - /// The [`Components`] and [`ComponentIds`] must match. - /// For example, they must be from the same world. - pub unsafe fn new(components: &'w Components, ids: &'w ComponentIds) -> Self { - Self { components, ids } + /// The [`Components`] and [`EntityAllocator`] must come from the same world. + pub unsafe fn new(components: &'w Components, allocator: &'w EntityAllocator) -> Self { + Self { + components, + allocator, + } } /// Queues this function to run as a component registrator if the given @@ -469,8 +415,9 @@ impl<'w> ComponentsQueuedRegistrator<'w> { .components .entry(type_id) .or_insert_with(|| { + let id = ComponentId::new(self.allocator.alloc()); // SAFETY: The id was just generated. - unsafe { QueuedRegistration::new(self.ids.next(), descriptor, func) } + unsafe { QueuedRegistration::new(id, descriptor, func) } }) .id } @@ -481,7 +428,7 @@ impl<'w> ComponentsQueuedRegistrator<'w> { descriptor: ComponentDescriptor, func: fn(&mut ComponentsRegistrator, ComponentId, ComponentDescriptor), ) -> ComponentId { - let id = self.ids.next(); + let id = ComponentId::new(self.allocator.alloc()); self.components .queued .write() @@ -551,12 +498,9 @@ impl<'w> ComponentsQueuedRegistrator<'w> { descriptor: ComponentDescriptor, ) -> ComponentId { self.register_arbitrary_dynamic(descriptor, |registrator, id, descriptor| { - // SAFETY: Id uniqueness handled by caller. - unsafe { - registrator - .components - .register_component_inner(id, descriptor); - } + registrator + .components + .register_component_inner(id, descriptor); }) } diff --git a/crates/bevy_ecs/src/entity/hash_set.rs b/crates/bevy_ecs/src/entity/hash_set.rs index f3b1ab3d740c5..1568b98c726fc 100644 --- a/crates/bevy_ecs/src/entity/hash_set.rs +++ b/crates/bevy_ecs/src/entity/hash_set.rs @@ -88,6 +88,23 @@ impl EntityEquivalentHashSet { } } +impl EntityEquivalentHashSet { + /// In-place union of two `EntityEquivalentHashSet`s.. + pub fn union_with(&mut self, rhs: &Self) { + self.0.bitor_assign(&rhs.0); + } + + /// In-place intersection of two `EntityEquivalentHashSet`s. + pub fn intersect_with(&mut self, rhs: &Self) { + self.0.bitand_assign(&rhs.0); + } + + /// In-place difference of two `EntityEquivalentHashSet`s. + pub fn difference_with(&mut self, rhs: &Self) { + self.0.sub_assign(&rhs.0); + } +} + impl Deref for EntityEquivalentHashSet { type Target = HashSet; diff --git a/crates/bevy_ecs/src/entity_disabling.rs b/crates/bevy_ecs/src/entity_disabling.rs index 16ef646ac6dbb..b02f183589c22 100644 --- a/crates/bevy_ecs/src/entity_disabling.rs +++ b/crates/bevy_ecs/src/entity_disabling.rs @@ -245,7 +245,8 @@ mod tests { use super::*; use crate::{ - component::ComponentIds, + component::ComponentId, + entity::EntityAllocator, prelude::{EntityMut, EntityRef, World}, query::{Has, With}, }; @@ -253,10 +254,10 @@ mod tests { #[test] fn filters_modify_access() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_4 = ids.next_mut(); + let ids = EntityAllocator::default(); + let id_1 = ComponentId::new(ids.alloc()); + let id_2 = ComponentId::new(ids.alloc()); + let id_4 = ComponentId::new(ids.alloc()); let mut filters = DefaultQueryFilters::empty(); filters.register_disabling_component(id_1); @@ -294,7 +295,7 @@ mod tests { let mut applied_access = component_access.clone(); filters.modify_access(&mut applied_access); assert_eq!( - vec![id_1, id_4], + vec![id_4, id_1], applied_access.with_filters().collect::>() ); assert_eq!(0, applied_access.without_filters().count()); diff --git a/crates/bevy_ecs/src/name.rs b/crates/bevy_ecs/src/name.rs index bb2b98d85581a..926c5c7a22211 100644 --- a/crates/bevy_ecs/src/name.rs +++ b/crates/bevy_ecs/src/name.rs @@ -299,7 +299,7 @@ mod tests { let mut query = world.query::(); let d1 = query.get(&world, e1).unwrap(); // NameOrEntity Display for entities without a Name should be {index}v{generation} - assert_eq!(d1.to_string(), "1v0"); + assert_eq!(d1.to_string(), "9v0"); let d2 = query.get(&world, e2).unwrap(); // NameOrEntity Display for entities with a Name should be the Name assert_eq!(d2.to_string(), "MyName"); diff --git a/crates/bevy_ecs/src/observer/centralized_storage.rs b/crates/bevy_ecs/src/observer/centralized_storage.rs index 8b09e95a0ec90..23c4e79eb7afe 100644 --- a/crates/bevy_ecs/src/observer/centralized_storage.rs +++ b/crates/bevy_ecs/src/observer/centralized_storage.rs @@ -39,13 +39,18 @@ impl Observers { pub(crate) fn get_observers_mut(&mut self, event_key: EventKey) -> &mut CachedObservers { use crate::lifecycle::*; - match event_key { - ADD => &mut self.add, - INSERT => &mut self.insert, - DISCARD => &mut self.discard, - REMOVE => &mut self.remove, - DESPAWN => &mut self.despawn, - _ => self.cache.entry(event_key).or_default(), + if event_key == ADD { + &mut self.add + } else if event_key == INSERT { + &mut self.insert + } else if event_key == DISCARD { + &mut self.discard + } else if event_key == REMOVE { + &mut self.remove + } else if event_key == DESPAWN { + &mut self.despawn + } else { + self.cache.entry(event_key).or_default() } } @@ -62,26 +67,36 @@ impl Observers { pub fn try_get_observers(&self, event_key: EventKey) -> Option<&CachedObservers> { use crate::lifecycle::*; - match event_key { - ADD => Some(&self.add), - INSERT => Some(&self.insert), - DISCARD => Some(&self.discard), - REMOVE => Some(&self.remove), - DESPAWN => Some(&self.despawn), - _ => self.cache.get(&event_key), + if event_key == ADD { + Some(&self.add) + } else if event_key == INSERT { + Some(&self.insert) + } else if event_key == DISCARD { + Some(&self.discard) + } else if event_key == REMOVE { + Some(&self.remove) + } else if event_key == DESPAWN { + Some(&self.despawn) + } else { + self.cache.get(&event_key) } } pub(crate) fn is_archetype_cached(event_key: EventKey) -> Option { use crate::lifecycle::*; - match event_key { - ADD => Some(ArchetypeFlags::ON_ADD_OBSERVER), - INSERT => Some(ArchetypeFlags::ON_INSERT_OBSERVER), - DISCARD => Some(ArchetypeFlags::ON_DISCARD_OBSERVER), - REMOVE => Some(ArchetypeFlags::ON_REMOVE_OBSERVER), - DESPAWN => Some(ArchetypeFlags::ON_DESPAWN_OBSERVER), - _ => None, + if event_key == ADD { + Some(ArchetypeFlags::ON_ADD_OBSERVER) + } else if event_key == INSERT { + Some(ArchetypeFlags::ON_INSERT_OBSERVER) + } else if event_key == DISCARD { + Some(ArchetypeFlags::ON_DISCARD_OBSERVER) + } else if event_key == REMOVE { + Some(ArchetypeFlags::ON_REMOVE_OBSERVER) + } else if event_key == DESPAWN { + Some(ArchetypeFlags::ON_DESPAWN_OBSERVER) + } else { + None } } diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index 0b4b1f8f4cc1d..7eb0ac37b898e 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -674,7 +674,7 @@ mod tests { world.add_observer(|_: On>, mut res: ResMut| res.observed("add_2")); world.spawn(A).flush(); - assert_eq!(vec!["add_2", "add_1"], world.resource::().0); + assert_eq!(vec!["add_1", "add_2"], world.resource::().0); // we have one A entity and two observers assert_eq!(world.query::<&A>().query(&world).count(), 1); assert_eq!(world.query::<&Observer>().query(&world).count(), 2); diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index e55e39ba545b9..01e357a134aae 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -1,11 +1,12 @@ use crate::world::unsafe_world_cell::UnsafeWorldCell; -use crate::{component::ComponentId, resource::IS_RESOURCE}; +use crate::{ + component::{ComponentId, ComponentIdSet}, + resource::IS_RESOURCE, +}; use alloc::{format, string::String, vec, vec::Vec}; -use core::iter::FusedIterator; +use core::fmt::Debug; use core::mem; -use core::{fmt, fmt::Debug}; use derive_more::From; -use fixedbitset::{Difference, FixedBitSet, Intersection, IntoOnes, Ones, Union}; use thiserror::Error; /// A set of bits that is either a finite set, or the set complement of a finite set. @@ -40,8 +41,12 @@ impl InvertibleComponentIdSet { #[inline] pub fn insert(&mut self, index: ComponentId) { match self { - Self::Included(included) => included.insert(index), - Self::Excluded(excluded) => excluded.remove(index), + Self::Included(included) => { + included.insert(index); + } + Self::Excluded(excluded) => { + excluded.remove(&index); + } } } @@ -49,8 +54,12 @@ impl InvertibleComponentIdSet { #[inline] pub fn remove(&mut self, index: ComponentId) { match self { - Self::Included(included) => included.remove(index), - Self::Excluded(excluded) => excluded.insert(index), + Self::Included(included) => { + included.remove(&index); + } + Self::Excluded(excluded) => { + excluded.insert(index); + } } } @@ -70,8 +79,8 @@ impl InvertibleComponentIdSet { #[inline] pub fn contains(&self, index: ComponentId) -> bool { match self { - Self::Included(included) => included.contains(index), - Self::Excluded(excluded) => !excluded.contains(index), + Self::Included(included) => included.contains(&index), + Self::Excluded(excluded) => !excluded.contains(&index), } } @@ -80,7 +89,7 @@ impl InvertibleComponentIdSet { #[inline] pub fn is_clear(&self) -> bool { match self { - Self::Included(included) => included.is_clear(), + Self::Included(included) => included.is_empty(), Self::Excluded(_) => false, } } @@ -91,7 +100,7 @@ impl InvertibleComponentIdSet { pub fn is_all(&self) -> bool { match self { Self::Included(_) => false, - Self::Excluded(excluded) => excluded.is_clear(), + Self::Excluded(excluded) => excluded.is_empty(), } } @@ -140,7 +149,7 @@ impl InvertibleComponentIdSet { match (&mut *self, other) { (Self::Included(this), Self::Included(other)) => this.union_with(other), (Self::Included(this), Self::Excluded(other)) => { - this.difference_from(other); + *this = other - this; *self = Self::Excluded(mem::take(this)); } (Self::Excluded(this), Self::Included(other)) => this.difference_with(other), @@ -162,7 +171,7 @@ impl InvertibleComponentIdSet { (Self::Included(this), Self::Excluded(other)) => this.intersect_with(other), (Self::Excluded(this), Self::Included(other)) => this.union_with(other), (Self::Excluded(this), Self::Excluded(other)) => { - this.difference_from(other); + *this = other - this; *self = Self::Included(mem::take(this)); } } @@ -181,7 +190,7 @@ impl InvertibleComponentIdSet { (Self::Included(this), Self::Included(other)) => this.intersect_with(other), (Self::Included(this), Self::Excluded(other)) => this.difference_with(other), (Self::Excluded(this), Self::Included(other)) => { - this.difference_from(other); + *this = other - this; *self = Self::Included(mem::take(this)); } (Self::Excluded(this), Self::Excluded(other)) => this.union_with(other), @@ -369,7 +378,7 @@ impl Access { /// /// [`Has`]: crate::query::Has pub fn has_archetypal(&self, index: ComponentId) -> bool { - self.archetypal.contains(index) + self.archetypal.contains(&index) } /// Sets this as having access to all components (i.e. `EntityRef` and `&World`). @@ -512,9 +521,9 @@ impl Access { /// # use bevy_ecs::component::ComponentId; /// let mut access = Access::default(); /// - /// access.add_read(ComponentId::new(1)); - /// access.add_write(ComponentId::new(2)); - /// access.add_archetypal(ComponentId::new(3)); + /// access.add_read(ComponentId::from_u32(1)); + /// access.add_write(ComponentId::from_u32(2)); + /// access.add_archetypal(ComponentId::from_u32(3)); /// /// let result = access /// .try_iter_access() @@ -523,9 +532,9 @@ impl Access { /// assert_eq!( /// result, /// Ok(vec![ - /// ComponentAccessKind::Shared(ComponentId::new(1)), - /// ComponentAccessKind::Exclusive(ComponentId::new(2)), - /// ComponentAccessKind::Archetypal(ComponentId::new(3)), + /// ComponentAccessKind::Exclusive(ComponentId::from_u32(2)), + /// ComponentAccessKind::Shared(ComponentId::from_u32(1)), + /// ComponentAccessKind::Archetypal(ComponentId::from_u32(3)), /// ]), /// ); /// ``` @@ -537,16 +546,17 @@ impl Access { reads_inverted: self.reads.is_unbounded(), })?; let accesses = reads.iter().map(|index| { - if self.writes.contains(index) { - ComponentAccessKind::Exclusive(index) + if self.writes.contains(*index) { + ComponentAccessKind::Exclusive(*index) } else { - ComponentAccessKind::Shared(index) + ComponentAccessKind::Shared(*index) } }); let archetypal = self .archetypal .difference(reads) + .copied() .map(ComponentAccessKind::Archetypal); Ok(accesses.chain(archetypal)) @@ -671,7 +681,7 @@ impl AccessConflicts { pub fn is_empty(&self) -> bool { match self { Self::All => false, - Self::Individual(set) => set.is_clear(), + Self::Individual(set) => set.is_empty(), } } @@ -683,7 +693,7 @@ impl AccessConflicts { .map(|index| { format!( "{}", - world.components().get_name(index).unwrap().shortname() + world.components().get_name(*index).unwrap().shortname() ) }) .collect::>() @@ -889,12 +899,14 @@ impl FilteredAccess { /// Returns the indices of the elements that this access filters for. pub fn with_filters(&self) -> impl Iterator + '_ { - self.filter_sets.iter().flat_map(|f| f.with.iter()) + self.filter_sets.iter().flat_map(|f| f.with.iter().copied()) } /// Returns the indices of the elements that this access filters out. pub fn without_filters(&self) -> impl Iterator + '_ { - self.filter_sets.iter().flat_map(|f| f.without.iter()) + self.filter_sets + .iter() + .flat_map(|f| f.without.iter().copied()) } /// Returns true if the index is used by this `FilteredAccess` in filters or archetypal access. @@ -905,7 +917,7 @@ impl FilteredAccess { || self .filter_sets .iter() - .any(|f| f.with.contains(index) || f.without.contains(index)) + .any(|f| f.with.contains(&index) || f.without.contains(&index)) } } @@ -1161,246 +1173,28 @@ impl FilteredAccessSet { } } -/// A set of [`ComponentId`]s. -#[derive(Default, Eq)] -#[repr(transparent)] -pub struct ComponentIdSet(FixedBitSet); - -impl PartialEq for ComponentIdSet { - fn eq(&self, other: &Self) -> bool { - // `FixedBitSet` requires equal lengths for equality, - // but we consider two sets equal if they have the same bits set - self.0.symmetric_difference(&other.0).next().is_none() - } -} - -impl ComponentIdSet { - /// Create a new empty `ComponentIdSet`. - #[inline] - pub const fn new() -> Self { - Self(FixedBitSet::new()) - } - - #[cfg(test)] - pub(crate) fn from_bits(bits: FixedBitSet) -> Self { - Self(bits) - } - - /// Adds a [`ComponentId`] to the set. - #[inline] - pub fn insert(&mut self, index: ComponentId) { - self.0.grow_and_insert(index.index()); - } - - /// Removes a [`ComponentId`] from the set. - #[inline] - pub fn remove(&mut self, index: ComponentId) { - if index.index() < self.0.len() { - self.0.remove(index.index()); - } - } - - /// Removes all [`ComponentId`]s from the set. - #[inline] - pub fn clear(&mut self) { - self.0.clear(); - } - - /// Returns `true` if the [`ComponentId`] is in the set. - #[inline] - pub fn contains(&self, index: ComponentId) -> bool { - self.0.contains(index.index()) - } - - /// Returns `true` if `self` has no elements in common with `other`. This - /// is equivalent to checking for an empty intersection. - #[inline] - pub fn is_disjoint(&self, other: &ComponentIdSet) -> bool { - self.0.is_disjoint(&other.0) - } - - /// Returns `true` if the set is a subset of another, i.e. `other` contains - /// at least all the values in `self`. - #[inline] - pub fn is_subset(&self, other: &ComponentIdSet) -> bool { - self.0.is_subset(&other.0) - } - - /// Returns `true` if the set is empty. - #[inline] - pub fn is_clear(&self) -> bool { - self.0.is_clear() - } - - /// Iterates the [`ComponentId`]s in the set. - #[inline] - pub fn iter(&self) -> ComponentIdIter> { - ComponentIdIter(self.0.ones()) - } - - /// Returns a lazy iterator over the union of two [`ComponentIdSet`]s. - #[inline] - pub fn union<'a>(&'a self, other: &'a ComponentIdSet) -> ComponentIdIter> { - ComponentIdIter(self.0.union(&other.0)) - } - - /// Returns a lazy iterator over the intersection of two [`ComponentIdSet`]s. - #[inline] - pub fn intersection<'a>( - &'a self, - other: &'a ComponentIdSet, - ) -> ComponentIdIter> { - ComponentIdIter(self.0.intersection(&other.0)) - } - - /// Returns a lazy iterator over the difference of two [`ComponentIdSet`]s. - #[inline] - pub fn difference<'a>(&'a self, other: &'a ComponentIdSet) -> ComponentIdIter> { - ComponentIdIter(self.0.difference(&other.0)) - } - - /// In-place union of two [`ComponentIdSet`]s. - #[inline] - pub fn union_with(&mut self, other: &ComponentIdSet) { - self.0.union_with(&other.0); - } - - /// In-place intersection of two [`ComponentIdSet`]s. - #[inline] - pub fn intersect_with(&mut self, other: &ComponentIdSet) { - self.0.intersect_with(&other.0); - } - - /// In-place difference of two [`ComponentIdSet`]s. - #[inline] - pub fn difference_with(&mut self, other: &ComponentIdSet) { - self.0.difference_with(&other.0); - } - - /// In-place reversed difference of two [`ComponentIdSet`]s. - /// This sets `self` to be `other.difference(self)`. - #[inline] - pub fn difference_from(&mut self, other: &ComponentIdSet) { - // Calculate `other - self` as `!self & other` - // We have to grow here because the new bits are going to get flipped to 1. - self.0.grow(other.0.len()); - self.0.toggle_range(..); - self.0.intersect_with(&other.0); - } -} - -impl Debug for ComponentIdSet { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - // `FixedBitSet` normally has a `Debug` output like: - // FixedBitSet { data: [ 160 ], length: 8 } - // Instead, print the list of set values, like: - // [ 5, 7 ] - // Don't wrap in `ComponentId`, since that would just output: - // [ ComponentId(5), ComponentId(7) ] - f.debug_list().entries(self.0.ones()).finish() - } -} - -impl Clone for ComponentIdSet { - #[inline] - fn clone(&self) -> Self { - Self(self.0.clone()) - } - - #[inline] - fn clone_from(&mut self, source: &Self) { - self.0.clone_from(&source.0); - } -} - -impl IntoIterator for ComponentIdSet { - type Item = ComponentId; - - type IntoIter = ComponentIdIter; - - #[inline] - fn into_iter(self) -> Self::IntoIter { - ComponentIdIter(self.0.into_ones()) - } -} - -impl<'a> IntoIterator for &'a ComponentIdSet { - type Item = ComponentId; - - type IntoIter = ComponentIdIter>; - - #[inline] - fn into_iter(self) -> Self::IntoIter { - self.iter() - } -} - -impl FromIterator for ComponentIdSet { - #[inline] - fn from_iter>(iter: T) -> Self { - Self(FixedBitSet::from_iter( - iter.into_iter().map(ComponentId::index), - )) - } -} - -impl Extend for ComponentIdSet { - #[inline] - fn extend>(&mut self, iter: T) { - self.0.extend(iter.into_iter().map(ComponentId::index)); - } -} - -/// An iterator of [`ComponentId`]s. -/// -/// This is equivalent to `map(ComponentId::new)`, -/// but is a named type to allow it to be used in associated types. -#[repr(transparent)] -pub struct ComponentIdIter(I); - -impl> Iterator for ComponentIdIter { - type Item = ComponentId; - - #[inline] - fn next(&mut self) -> Option { - self.0.next().map(ComponentId::new) - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - self.0.size_hint() - } -} - -impl> DoubleEndedIterator for ComponentIdIter { - #[inline] - fn next_back(&mut self) -> Option { - self.0.next_back().map(ComponentId::new) - } -} - -impl> FusedIterator for ComponentIdIter {} - #[cfg(test)] mod tests { use crate::{ - component::ComponentIds, + component::{ComponentId, ComponentIdSet}, + entity::EntityAllocator, query::{ access::{AccessFilters, InvertibleComponentIdSet}, - Access, AccessConflicts, ComponentAccessKind, ComponentIdSet, FilteredAccess, - FilteredAccessSet, UnboundedAccessError, + Access, AccessConflicts, ComponentAccessKind, FilteredAccess, FilteredAccessSet, + UnboundedAccessError, }, }; use alloc::{vec, vec::Vec}; - use fixedbitset::FixedBitSet; + + fn generate_ids(count: u32) -> Vec { + let ids = EntityAllocator::default(); + ids.alloc_many(count).map(ComponentId::new).collect() + } #[test] fn test_access_clone() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - let id_5 = ids.next_mut(); + let ids = generate_ids(4); + let (id_1, id_2, id_3, id_5) = (ids[0], ids[1], ids[2], ids[3]); let mut original = Access::default(); original.add_read(id_1); @@ -1416,14 +1210,9 @@ mod tests { #[test] fn test_access_clone_from() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - let id_4 = ids.next_mut(); - let id_5 = ids.next_mut(); - let id_7 = ids.next_mut(); - let id_8 = ids.next_mut(); + let ids = generate_ids(7); + let (id_1, id_2, id_3, id_4, id_5, id_7, id_8) = + (ids[0], ids[1], ids[2], ids[3], ids[4], ids[5], ids[6]); let mut original = Access::default(); original.add_read(id_1); @@ -1446,11 +1235,8 @@ mod tests { #[test] fn test_filtered_access_clone() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - let id_4 = ids.next_mut(); + let ids = generate_ids(4); + let (id_1, id_2, id_3, id_4) = (ids[0], ids[1], ids[2], ids[3]); let mut original = FilteredAccess::default(); original.add_write(id_1); @@ -1465,12 +1251,8 @@ mod tests { #[test] fn test_filtered_access_clone_from() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - let id_4 = ids.next_mut(); - let id_7 = ids.next_mut(); + let ids = generate_ids(5); + let (id_1, id_2, id_3, id_4, id_7) = (ids[0], ids[1], ids[2], ids[3], ids[4]); let mut original = FilteredAccess::default(); original.add_write(id_1); @@ -1491,9 +1273,8 @@ mod tests { #[test] fn test_access_filters_clone() { - let mut ids = ComponentIds::default(); - let id_3 = ids.next_mut(); - let id_5 = ids.next_mut(); + let ids = generate_ids(5); + let (id_3, id_5) = (ids[0], ids[1]); let mut original = AccessFilters::default(); original.with.insert(id_3); @@ -1506,11 +1287,8 @@ mod tests { #[test] fn test_access_filters_clone_from() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - let id_5 = ids.next_mut(); + let ids = generate_ids(4); + let (id_1, id_2, id_3, id_5) = (ids[0], ids[1], ids[2], ids[3]); let mut original = AccessFilters::default(); original.with.insert(id_3); @@ -1528,9 +1306,8 @@ mod tests { #[test] fn test_filtered_access_set_clone() { - let mut ids = ComponentIds::default(); - let id_2 = ids.next_mut(); - let id_4 = ids.next_mut(); + let ids = generate_ids(5); + let (id_2, id_4) = (ids[0], ids[1]); let mut original = FilteredAccessSet::default(); original.add_unfiltered_component_read(id_2); @@ -1544,11 +1321,8 @@ mod tests { #[test] fn test_filtered_access_set_from() { - let mut ids = ComponentIds::default(); - let id_2 = ids.next_mut(); - let id_4 = ids.next_mut(); - let id_7 = ids.next_mut(); - let id_9 = ids.next_mut(); + let ids = generate_ids(4); + let (id_2, id_4, id_7, id_9) = (ids[0], ids[1], ids[2], ids[3]); let mut original = FilteredAccessSet::default(); original.add_unfiltered_component_read(id_2); @@ -1568,8 +1342,8 @@ mod tests { #[test] fn read_all_access_conflicts() { - let mut ids = ComponentIds::default(); - let id_0 = ids.next_mut(); + let ids = generate_ids(1); + let id_0 = ids[0]; // read_all / single write let mut access_a = Access::default(); @@ -1592,9 +1366,8 @@ mod tests { #[test] fn access_get_conflicts() { - let mut ids = ComponentIds::default(); - let id_0 = ids.next_mut(); - let id_1 = ids.next_mut(); + let ids = generate_ids(2); + let (id_0, id_1) = (ids[0], ids[1]); let mut access_a = Access::default(); access_a.add_read(id_0); @@ -1623,8 +1396,8 @@ mod tests { #[test] fn filtered_combined_access() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); + let ids = generate_ids(1); + let id_1 = ids[0]; let mut access_a = FilteredAccessSet::default(); access_a.add_unfiltered_component_read(id_1); @@ -1642,12 +1415,8 @@ mod tests { #[test] fn filtered_access_extend() { - let mut ids = ComponentIds::default(); - let id_0 = ids.next_mut(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - let id_4 = ids.next_mut(); + let ids = generate_ids(5); + let (id_0, id_1, id_2, id_3, id_4) = (ids[0], ids[1], ids[2], ids[3], ids[4]); let mut access_a = FilteredAccess::default(); access_a.add_read(id_0); @@ -1671,14 +1440,14 @@ mod tests { assert!(access_a.eq(&expected)); } + fn set_from_u32s(iter: Vec) -> ComponentIdSet { + ComponentIdSet::from_iter(iter.into_iter().map(ComponentId::from_u32)) + } + #[test] fn filtered_access_extend_or() { - let mut ids = ComponentIds::default(); - let id_0 = ids.next_mut(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - let id_4 = ids.next_mut(); + let ids = generate_ids(5); + let (id_0, id_1, id_2, id_3, id_4) = (ids[0], ids[1], ids[2], ids[3], ids[4]); let mut access_a = FilteredAccess::default(); // Exclusive access to `(&mut A, &mut B)`. @@ -1709,15 +1478,12 @@ mod tests { // The resulted access is expected to represent `Or<((With, With, With), (With, With, With, Without))>`. expected.filter_sets = vec![ AccessFilters { - with: ComponentIdSet::from_bits(FixedBitSet::with_capacity_and_blocks(3, [0b111])), + with: set_from_u32s(vec![0, 1, 2]), without: ComponentIdSet::default(), }, AccessFilters { - with: ComponentIdSet::from_bits(FixedBitSet::with_capacity_and_blocks(4, [0b1011])), - without: ComponentIdSet::from_bits(FixedBitSet::with_capacity_and_blocks( - 5, - [0b10000], - )), + with: set_from_u32s(vec![0, 1, 3]), + without: set_from_u32s(vec![4]), }, ]; @@ -1726,11 +1492,10 @@ mod tests { #[test] fn try_iter_component_access_simple() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - let id_5 = ids.next_mut(); + use bevy_platform::collections::HashSet; + + let ids = generate_ids(4); + let (id_1, id_2, id_3, id_5) = (ids[0], ids[1], ids[2], ids[3]); let mut access = Access::default(); @@ -1739,24 +1504,25 @@ mod tests { access.add_write(id_3); access.add_archetypal(id_5); - let result = access.try_iter_access().map(Iterator::collect::>); + let result = access + .try_iter_access() + .map(Iterator::collect::>); assert_eq!( result, - Ok(vec![ + Ok(HashSet::from_iter([ ComponentAccessKind::Shared(id_1), ComponentAccessKind::Shared(id_2), ComponentAccessKind::Exclusive(id_3), ComponentAccessKind::Archetypal(id_5), - ]), + ])), ); } #[test] fn try_iter_component_access_unbounded_write_all() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); + let ids = generate_ids(2); + let (id_1, id_2) = (ids[0], ids[1]); let mut access = Access::default(); @@ -1777,9 +1543,8 @@ mod tests { #[test] fn try_iter_component_access_unbounded_read_all() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); + let ids = generate_ids(2); + let (id_1, id_2) = (ids[0], ids[1]); let mut access = Access::default(); @@ -1798,20 +1563,10 @@ mod tests { ); } - /// Create a `ComponentIdSet` with a given number of total bits and a given list of bits to set. - /// Setting the number of bits is important in tests since the `PartialEq` impl checks that the length matches. - fn bit_set(bits: usize, iter: impl IntoIterator) -> ComponentIdSet { - let mut result = FixedBitSet::with_capacity(bits); - result.extend(iter); - ComponentIdSet::from_bits(result) - } - #[test] fn invertible_union_tests() { - let mut ids = ComponentIds::default(); - let id_0 = ids.next_mut(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); + let ids = generate_ids(3); + let (id_0, id_1, id_2) = (ids[0], ids[1], ids[2]); let set0 = ComponentIdSet::from_iter([id_0]); let set1 = ComponentIdSet::from_iter([id_1]); @@ -1849,29 +1604,10 @@ mod tests { ); } - #[test] - fn invertible_union_with_different_lengths() { - // When adding a large inverted set to a small normal set, - // make sure we invert the bits beyond the original length. - // Failing to call `grow` before `toggle_range` would cause bit 1 to be zero, - // which would incorrectly treat it as included in the output set. - let mut self_set = InvertibleComponentIdSet::Included(bit_set(1, [0])); - let other_set = InvertibleComponentIdSet::Excluded(bit_set(3, [0, 1])); - self_set.union_with(&other_set); - - // [0] | [2, ...] = [0, 2, ...] - assert_eq!( - self_set, - InvertibleComponentIdSet::Excluded(bit_set(3, [1])) - ); - } - #[test] fn invertible_difference_tests() { - let mut ids = ComponentIds::default(); - let id_0 = ids.next_mut(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); + let ids = generate_ids(3); + let (id_0, id_1, id_2) = (ids[0], ids[1], ids[2]); let set0 = ComponentIdSet::from_iter([id_0]); let set1 = ComponentIdSet::from_iter([id_1]); @@ -1911,10 +1647,8 @@ mod tests { #[test] fn invertible_intersection_tests() { - let mut ids = ComponentIds::default(); - let id_0 = ids.next_mut(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); + let ids = generate_ids(3); + let (id_0, id_1, id_2) = (ids[0], ids[1], ids[2]); let set0 = ComponentIdSet::from_iter([id_0]); let set1 = ComponentIdSet::from_iter([id_1]); @@ -1951,135 +1685,4 @@ mod tests { InvertibleComponentIdSet::Excluded(set012.clone()) ); } - - #[test] - fn component_id_set_insert_remove_clear() { - let mut ids = ComponentIds::default(); - let id_0 = ids.next_mut(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - - let mut set = ComponentIdSet::new(); - assert!(!set.contains(id_0)); - assert!(!set.contains(id_1)); - assert!(!set.contains(id_2)); - assert!(set.is_clear()); - set.insert(id_2); - set.insert(id_1); - assert!(!set.contains(id_0)); - assert!(set.contains(id_1)); - assert!(set.contains(id_2)); - assert!(!set.is_clear()); - set.remove(id_1); - assert!(!set.contains(id_0)); - assert!(!set.contains(id_1)); - assert!(set.contains(id_2)); - assert!(!set.is_clear()); - set.insert(id_2); - set.insert(id_1); - assert!(!set.contains(id_0)); - assert!(set.contains(id_1)); - assert!(set.contains(id_2)); - assert!(!set.is_clear()); - set.clear(); - assert!(!set.contains(id_0)); - assert!(!set.contains(id_1)); - assert!(!set.contains(id_2)); - assert!(set.is_clear()); - } - - #[test] - fn component_id_set_remove_out_of_range() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_3 = ids.next_mut(); - let id_4 = ids.next_mut(); - - let mut set = ComponentIdSet::new(); - set.remove(id_3); - set.insert(id_1); - set.remove(id_4); - assert!(set.iter().eq([id_1])); - } - - #[test] - fn component_id_set_is_subset_is_disjoint() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - let id_4 = ids.next_mut(); - let id_5 = ids.next_mut(); - - let set_1234 = ComponentIdSet::from_iter([id_1, id_2, id_3, id_4]); - let set_23 = ComponentIdSet::from_iter([id_2, id_3]); - let set_45 = ComponentIdSet::from_iter([id_4, id_5]); - assert!(set_23.is_subset(&set_1234)); - assert!(!set_1234.is_subset(&set_23)); - assert!(set_23.is_disjoint(&set_45)); - assert!(set_45.is_disjoint(&set_23)); - assert!(!set_1234.is_disjoint(&set_23)); - assert!(!set_23.is_disjoint(&set_1234)); - } - - #[test] - fn component_id_set_union_intersection_difference() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - - let set_13 = ComponentIdSet::from_iter([id_1, id_3]); - let set_23 = ComponentIdSet::from_iter([id_2, id_3]); - - assert!(set_13.union(&set_23).eq([id_1, id_3, id_2])); - assert!(set_23.union(&set_13).eq([id_2, id_3, id_1])); - assert!(set_13.intersection(&set_23).eq([id_3])); - assert!(set_23.intersection(&set_13).eq([id_3])); - assert!(set_13.difference(&set_23).eq([id_1])); - assert!(set_23.difference(&set_13).eq([id_2])); - } - - #[test] - fn component_id_set_union_intersection_difference_with() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); - let id_3 = ids.next_mut(); - - let set_13 = ComponentIdSet::from_iter([id_1, id_3]); - let set_23 = ComponentIdSet::from_iter([id_2, id_3]); - - let mut s = set_13.clone(); - s.union_with(&set_23); - assert!(s.iter().eq([id_1, id_2, id_3])); - - let mut s = set_23.clone(); - s.union_with(&set_13); - assert!(s.iter().eq([id_1, id_2, id_3])); - - let mut s = set_13.clone(); - s.intersect_with(&set_23); - assert!(s.iter().eq([id_3])); - - let mut s = set_23.clone(); - s.intersect_with(&set_13); - assert!(s.iter().eq([id_3])); - - let mut s = set_13.clone(); - s.difference_with(&set_23); - assert!(s.iter().eq([id_1])); - - let mut s = set_23.clone(); - s.difference_with(&set_13); - assert!(s.iter().eq([id_2])); - - let mut s = set_13.clone(); - s.difference_from(&set_23); - assert!(s.iter().eq([id_2])); - - let mut s = set_23.clone(); - s.difference_from(&set_13); - assert!(s.iter().eq([id_1])); - } } diff --git a/crates/bevy_ecs/src/query/access_iter.rs b/crates/bevy_ecs/src/query/access_iter.rs index d518328a3e9c0..4a3ab8056ad34 100644 --- a/crates/bevy_ecs/src/query/access_iter.rs +++ b/crates/bevy_ecs/src/query/access_iter.rs @@ -70,7 +70,7 @@ fn has_conflicts_large<'a, Q: QueryData>( let needs_check = match access { EcsAccessType::Component(EcsAccessLevel::Read(component_id)) | EcsAccessType::Component(EcsAccessLevel::Write(component_id)) => { - filter.check_insert(&component_id.index()) + filter.check_insert(&component_id) } EcsAccessType::Component(EcsAccessLevel::ReadAll) | EcsAccessType::Component(EcsAccessLevel::WriteAll) => true, @@ -81,7 +81,7 @@ fn has_conflicts_large<'a, Q: QueryData>( let index = match kind { crate::query::ComponentAccessKind::Shared(id) | crate::query::ComponentAccessKind::Exclusive(id) - | crate::query::ComponentAccessKind::Archetypal(id) => id.index(), + | crate::query::ComponentAccessKind::Archetypal(id) => id, }; if filter.check_insert(&index) { needs_check = true; diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 63d9efa8a1e5a..9bc2b24a5ba8a 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -195,6 +195,10 @@ impl QueryState { /// Adds all access from this query and any nested queries to the `component_access_set`. /// Panics if the access from this query and any nested queries conflict with each other /// or with any previous access. + #[expect( + clippy::result_large_err, + reason = "Boxing `FilteredAccessSet` adds unnecessary noise." + )] pub fn init_access( &self, component_access_set: &mut FilteredAccessSet, @@ -573,7 +577,7 @@ impl QueryState { self.validate_world(world.id()); D::update_archetypes(&mut self.fetch_state, world); F::update_archetypes(&mut self.filter_state, world); - if self.component_access.required.is_clear() { + if self.component_access.required.is_empty() { let archetypes = world.archetypes(); let old_generation = core::mem::replace(&mut self.archetype_generation, archetypes.generation()); @@ -600,7 +604,7 @@ impl QueryState { world .archetypes() .component_index() - .get(&component_id) + .get(component_id) .map(|index| index.keys()) }) // select the component with the fewest archetypes @@ -679,8 +683,8 @@ impl QueryState { /// Returns `true` if this query matches a set of components. Otherwise, returns `false`. pub fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool { self.component_access.filter_sets.iter().any(|set| { - set.with.iter().all(set_contains_id) - && set.without.iter().all(|index| !set_contains_id(index)) + set.with.iter().copied().all(set_contains_id) + && set.without.iter().all(|index| !set_contains_id(*index)) }) } diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index dda8a21f4920e..ddba0a4fa6f4e 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -128,6 +128,10 @@ pub unsafe trait WorldQuery { /// /// This is used for queries to request access to entities other than the current one, /// such as to read resources or to follow relations. + #[expect( + clippy::result_large_err, + reason = "Boxing `FilteredAccessSet` adds unnecessary noise for implementors of this trait" + )] fn init_nested_access( state: &Self::State, component_access_set: &mut FilteredAccessSet, diff --git a/crates/bevy_ecs/src/resource.rs b/crates/bevy_ecs/src/resource.rs index cf5eec615bdab..1c39d892c82cd 100644 --- a/crates/bevy_ecs/src/resource.rs +++ b/crates/bevy_ecs/src/resource.rs @@ -4,16 +4,54 @@ use log::warn; use crate::{ component::{Component, ComponentId}, - entity::Entity, + entity::ContainsEntity, 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; + +/// The main hook that checks that a resource is inserted on the right entity, thereby guaranteeing uniqueness. +/// It's still possible to define your own insert hooks on resources, which will exist alongside this one. +pub fn on_resource_insert(mut world: DeferredWorld, context: HookContext) { + let resource_id = context.component_id; + let resource_entity = resource_id.entity(); + + if !world.entities().contains(resource_entity) { + let name = world + .components() + .get_name(resource_id) + .expect("resource is registered"); + warn!( + "Resource entity {} of {} has been despawned, when it's not supposed to be.", + resource_entity, name + ); + } + + if resource_entity != context.entity { + // the resource already exists and the new one should be removed + world + .commands() + .entity(context.entity) + .remove_by_id(resource_id); + world + .commands() + .entity(context.entity) + .remove_by_id(IS_RESOURCE); + let name = world + .components() + .get_name(resource_id) + .expect("resource is registered"); + warn!( + "Tried inserting the resource {} on the wrong entity. \ + Resources are unique components stored on the entity matching their `component_id`. \ + Inserting on a different entity causes the new value to be removed.", + name + ); + } +} /// A type that can be inserted into a [`World`] as a singleton. /// @@ -74,6 +112,21 @@ use bevy_platform::cell::SyncUnsafeCell; /// } /// ``` /// +/// # Resources entities +/// +/// Under the hood, resource data is stored as a component on a resource entity. This resource +/// is first [allocated](crate::entity::EntityAllocator) upon registration, through +/// `world.register_component::()` or its alias `world.register_resource::()`. +/// When initializing or inserting a resource, we spawn the actual entity on the world. +/// Additionally, we add both the resource as a component *and* the [`IsResource`] marker to +/// the entity. Because [`IsResource`] is a required component for every resource this is done +/// automatically. +/// +/// From this point forward, hooks on [`IsResource`] warn against despawning the resource entity, +/// while a hook on the resource type ensures that it is unique. While one shouldn't despawn the +/// resource entity, you can still remove resources. This simply removes the resource component +/// from the resource entity. +/// /// [`Exclusive`]: https://doc.rust-lang.org/nightly/std/sync/struct.Exclusive.html /// [`World`]: crate::world::World /// [`Res`]: crate::system::Res @@ -86,128 +139,32 @@ 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)] -pub struct IsResource(ComponentId); +#[component(on_discard, on_despawn)] +pub struct IsResource; impl IsResource { - /// Creates a new instance with the given `component_id` - pub fn new(component_id: ComponentId) -> Self { - Self(component_id) - } - - /// The [`ComponentId`] of the resource component (the _actual_ resource value component, not the [`IsResource`] component). - pub fn resource_component_id(&self) -> ComponentId { - self.0 - } + pub(crate) fn on_discard(world: DeferredWorld, context: HookContext) { + let maybe_resource_id = ComponentId::new(context.entity); - 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); + // If IsResource exists on a resource entity then maybe_resource_id is + // a valid component id and we should not remove it. + if world.components().is_id_valid(maybe_resource_id) { + warn!("IsResource components should not be removed from a resource entity"); } } - pub(crate) fn on_discard(mut world: DeferredWorld, context: HookContext) { - let resource_component_id = world - .entity(context.entity) - .get::() - .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); + pub(crate) fn on_despawn(world: DeferredWorld, context: HookContext) { + let maybe_resource_id = ComponentId::new(context.entity); - world - .commands() - .entity(context.entity) - .remove_by_id(resource_component_id); + // If IsResource exists on a resource entity then maybe_resource_id is + // a valid component id and we should not despawn the entity. + if world.components().is_id_valid(maybe_resource_id) { + warn!("Resource entities are not supposed to be despawned."); } } - - pub(crate) fn on_despawn(_world: DeferredWorld, _context: HookContext) { - warn!("Resource entities are not supposed to be despawned."); - } } pub use crate::component::IS_RESOURCE; @@ -219,11 +176,11 @@ mod tests { use crate::{ change_detection::MaybeLocation, component::Components, - entity::Entity, + entity::{ContainsEntity, Entity}, lifecycle::HookContext, prelude::{EntityRef, Query, SystemParamBuilder}, ptr::OwningPtr, - resource::{IsResource, Resource, ResourceEntities}, + resource::{IsResource, Resource}, system::{ParamBuilder, QueryParamBuilder, RunSystemOnce}, world::{DeferredWorld, FilteredEntityRef, World}, }; @@ -261,7 +218,7 @@ mod tests { } }); assert_eq!(world.entities().count_spawned(), start + 3); - let e3 = world.resource_entities().get(id3).unwrap(); + let e3 = id3.entity(); 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); @@ -271,13 +228,13 @@ mod tests { world.insert_resource_by_id(id3, ptr, MaybeLocation::caller()); } }); - assert_eq!(e3, world.resource_entities().get(id3).unwrap()); + assert_eq!(e3, id3.entity()); // again, the entity is stable: see previous explanation - let e1 = world.resource_entities().get(id1).unwrap(); + let e1 = id1.entity(); 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, id1.entity()); // 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,33 +254,25 @@ mod tests { let first_entity = { let resources = query.iter(&world).collect::>(); assert_eq!(resources.len(), 1); - let (entity, _test_resource, is_resource) = resources[0]; - assert_eq!(is_resource.resource_component_id(), id); + let (entity, _test_resource, _is_resource) = resources[0]; + assert_eq!(entity, id.entity()); entity }; - // 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()); - - assert!( - !world.entity(first_entity).contains::(), - "Removing IsResource should also remove the Resource component it corresponds to" - ); + world.remove_resource::(); world.init_resource::(); let second_entity = { let resources = query.iter(&world).collect::>(); assert_eq!(resources.len(), 1); - let (entity, _test_resource, is_resource) = resources[0]; - assert_eq!(is_resource.resource_component_id(), id); + let (entity, _test_resource, _is_resource) = resources[0]; + assert_eq!(entity, id.entity()); entity }; - assert_ne!( + assert_eq!( first_entity, second_entity, - "The first resource entity was invalidated, so the second initialization should be new" + "The resource should always be inserted on the component entity" ); let id = world.spawn(TestResource).id(); @@ -399,21 +348,16 @@ mod tests { }); }), ParamBuilder, - ParamBuilder, ) .build_state(&mut world) .build_system(resource_system); - fn resource_system( - query: Query, - resource_entities: &ResourceEntities, - components: &Components, - ) { + fn resource_system(query: Query, components: &Components) { let component_id_a = components.get_id(TypeId::of::()).unwrap(); let component_id_b = components.get_id(TypeId::of::()).unwrap(); - let entity_a = resource_entities.get(component_id_a).unwrap(); - let entity_b = resource_entities.get(component_id_b).unwrap(); + let entity_a = component_id_a.entity(); + let entity_b = component_id_b.entity(); let entity_ref_a: FilteredEntityRef = query.get(entity_a).unwrap(); assert_eq!(entity_ref_a.get::().unwrap().0, 12); diff --git a/crates/bevy_ecs/src/schedule/node.rs b/crates/bevy_ecs/src/schedule/node.rs index a884103463f9d..6db5697d5723c 100644 --- a/crates/bevy_ecs/src/schedule/node.rs +++ b/crates/bevy_ecs/src/schedule/node.rs @@ -620,6 +620,7 @@ impl Systems { AccessConflicts::Individual(conflicts) => { let conflicts: Box<[_]> = conflicts .iter() + .copied() .filter(|id| !ignored_ambiguities.contains(id)) .collect(); if !conflicts.is_empty() { diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 830e383e2ca4e..5dc1c4c0c7f3f 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -40,7 +40,10 @@ pub(crate) struct SparseArray { /// /// However, it may use a lot of excess memory if the /// values are large or the set is sparsely populated. - +#[expect( + dead_code, + reason = "ImmutableSparseArray may be used again in the future." +)] #[derive(Debug)] pub(crate) struct ImmutableSparseArray { values: Box<[Option]>, @@ -86,7 +89,8 @@ macro_rules! impl_sparse_array { } impl_sparse_array!(SparseArray); -impl_sparse_array!(ImmutableSparseArray); +// ImmutableSparseArray may be used again in the future. +// impl_sparse_array!(ImmutableSparseArray); impl SparseArray { /// Inserts `value` at `index` in the array. @@ -135,19 +139,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. @@ -538,6 +529,10 @@ pub struct SparseSet { /// the dense storage of values takes less memory when `V` is large, /// although the overhead of tracking which entries have values /// may make it larger when `V` is small or the set is densely populated. +#[expect( + dead_code, + reason = "ImmutableSparseSet may be used again in the future." +)] #[derive(Debug)] pub(crate) struct ImmutableSparseSet { /// The mapping from dense index to value. @@ -619,7 +614,8 @@ macro_rules! impl_sparse_set { } impl_sparse_set!(SparseSet); -impl_sparse_set!(ImmutableSparseSet); +// ImmutableSparseSet may be used again in the future. +// impl_sparse_set!(ImmutableSparseSet); impl Default for SparseSet { fn default() -> Self { @@ -735,6 +731,10 @@ impl SparseSet { } /// Converts the sparse set into its immutable variant. + #[expect( + dead_code, + reason = "ImmutableSparseArray may be used again in the future." + )] pub(crate) fn into_immutable(self) -> ImmutableSparseSet { ImmutableSparseSet { dense: self.dense.into_boxed_slice(), @@ -852,8 +852,8 @@ impl SparseSets { mod tests { use super::SparseSets; use crate::{ - component::{Component, ComponentDescriptor, ComponentId, ComponentIds, ComponentInfo}, - entity::{Entity, EntityIndex}, + component::{Component, ComponentDescriptor, ComponentId, ComponentInfo}, + entity::{Entity, EntityAllocator, EntityIndex}, storage::SparseSet, }; use alloc::{vec, vec::Vec}; @@ -912,7 +912,7 @@ mod tests { #[test] fn sparse_sets() { - let mut ids = ComponentIds::default(); + let ids = EntityAllocator::default(); let mut sets = SparseSets::default(); #[derive(Component, Default, Debug)] @@ -921,8 +921,8 @@ mod tests { #[derive(Component, Default, Debug)] struct TestComponent2; - let id_1 = ids.next_mut(); - let id_2 = ids.next_mut(); + let id_1 = ComponentId::new(ids.alloc()); + let id_2 = ComponentId::new(ids.alloc()); assert_eq!(sets.len(), 0); assert!(sets.is_empty()); @@ -939,7 +939,7 @@ mod tests { .map(|(id, set)| (id, set.len())) .collect::>(); collected_sets.sort(); - assert_eq!(collected_sets, vec![(id_1, 0), (id_2, 0),]); + assert_eq!(collected_sets, vec![(id_2, 0), (id_1, 0),]); fn register_component(sets: &mut SparseSets, id: ComponentId) { let descriptor = ComponentDescriptor::new::(); diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index 666343532c35b..af367a98724c2 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -4,10 +4,10 @@ pub use column::Column; use crate::{ change_detection::{AtomicTick, CheckChangeTicks, ComponentTicks, MaybeLocation, Tick}, - component::{ComponentId, ComponentInfo, Components}, + component::{ComponentId, ComponentIdMap, ComponentInfo, Components}, entity::Entity, query::DebugCheckedUnwrap, - storage::{AbortOnPanic, ImmutableSparseSet, SparseSet}, + storage::AbortOnPanic, }; use alloc::{boxed::Box, vec, vec::Vec}; use bevy_platform::collections::HashMap; @@ -139,7 +139,7 @@ impl TableRow { // it must be the correct capacity to allocate, reallocate, and deallocate all columns. This // means the safety invariant must be enforced even in `TableBuilder`. pub(crate) struct TableBuilder { - columns: SparseSet, + columns: ComponentIdMap, entities: Vec, } @@ -148,7 +148,7 @@ impl TableBuilder { /// `column_capacity` (How many columns?) and `capacity` (How many entities per column?). pub fn with_capacity(capacity: usize, column_capacity: usize) -> Self { Self { - columns: SparseSet::with_capacity(column_capacity), + columns: ComponentIdMap::with_capacity(column_capacity), entities: Vec::with_capacity(capacity), } } @@ -176,9 +176,8 @@ impl TableBuilder { /// - If the table's columns were not added in order, sorted by [`ComponentId`]. #[must_use] pub fn build(self) -> Table { - assert!(self.columns.indices().is_sorted()); Table { - columns: self.columns.into_immutable(), + columns: self.columns, entities: self.entities, } } @@ -202,7 +201,7 @@ impl TableBuilder { // it must be the correct capacity to allocate, reallocate, and deallocate all columns. This // means the safety invariant must be enforced even in `TableBuilder`. pub struct Table { - columns: ImmutableSparseSet, + columns: ComponentIdMap, entities: Vec, } @@ -373,7 +372,7 @@ impl Table { /// [`Component`]: crate::component::Component #[inline] pub fn get_column(&self, component_id: ComponentId) -> Option<&Column> { - self.columns.get(component_id) + self.columns.get(&component_id) } /// Fetches a mutable reference to the [`Column`] for a given [`Component`] within the @@ -384,7 +383,7 @@ impl Table { /// [`Component`]: crate::component::Component #[inline] pub(crate) fn get_column_mut(&mut self, component_id: ComponentId) -> Option<&mut Column> { - self.columns.get_mut(component_id) + self.columns.get_mut(&component_id) } /// Checks if the table contains a [`Column`] for a given [`Component`]. @@ -394,7 +393,7 @@ impl Table { /// [`Component`]: crate::component::Component #[inline] pub fn has_column(&self, component_id: ComponentId) -> bool { - self.columns.contains(component_id) + self.columns.contains_key(&component_id) } /// Reserves `additional` elements worth of capacity within the table. @@ -764,21 +763,10 @@ impl Tables { // - The caller ensures that all new columns will be written to immediately. let dst_row = unsafe { dst_table.allocate(src_table.entities.swap_remove(row.index())) }; - let mut dst_iter = dst_table.columns.iter_mut().peekable(); - for (src_component_id, src_column) in src_table.columns.iter_mut() { - // Skip past any destination columns that don't exist in the source table. - // The caller is responsible for initializing those columns. - while dst_iter - .next_if(|(dst_component_id, _)| *dst_component_id < src_component_id) - .is_some() - {} - - // Then move the value in the source column if it exists in the destination table, + // Move the value in the source column if it exists in the destination table, // or remove it if it does not. - if let Some((_, dst_column)) = - dst_iter.next_if(|(dst_component_id, _)| *dst_component_id == src_component_id) - { + if let Some(dst_column) = dst_table.columns.get_mut(src_component_id) { // SAFETY: // - `src_column` and `dst_column` correspond to the same `ComponentId`. // - The caller ensures `row` is in-bounds for `src_column`. @@ -807,9 +795,6 @@ impl Tables { } } - // Need to end the mutable borrow so we can return `dst_table`. - drop(dst_iter); - TableMoveResult { new_table: dst_table, new_row: dst_row, @@ -857,8 +842,8 @@ impl Drop for Table { mod tests { use crate::{ change_detection::{MaybeLocation, Tick}, - component::{Component, ComponentIds, Components, ComponentsRegistrator}, - entity::{Entity, EntityIndex}, + component::{Component, Components, ComponentsRegistrator}, + entity::{Entity, EntityAllocator, EntityIndex}, ptr::OwningPtr, storage::{TableBuilder, TableId, TableRow, Tables}, }; @@ -882,10 +867,9 @@ mod tests { #[test] fn table() { let mut components = Components::default(); - let mut componentids = ComponentIds::default(); + let allocator = EntityAllocator::default(); // SAFETY: They are both new. - let mut registrator = - unsafe { ComponentsRegistrator::new(&mut components, &mut componentids) }; + let mut registrator = unsafe { ComponentsRegistrator::new(&mut components, &allocator) }; let component_id = registrator.register_component::>(); let columns = &[component_id]; let mut table = TableBuilder::with_capacity(0, columns.len()) diff --git a/crates/bevy_ecs/src/system/access.rs b/crates/bevy_ecs/src/system/access.rs index a80f14fd489a1..d32dd31fc34cd 100644 --- a/crates/bevy_ecs/src/system/access.rs +++ b/crates/bevy_ecs/src/system/access.rs @@ -23,6 +23,10 @@ pub enum SystemAccess { Exclusive, } +#[expect( + clippy::result_large_err, + reason = "Boxing `SystemAccess` adds unnecessary noise to the various methods / APIs" +)] impl SystemAccess { /// Returns true if the system does not access the world at all, so it can run /// in parallel with any other system. @@ -217,7 +221,8 @@ impl SystemAccess { #[cfg(test)] mod tests { use crate::{ - component::ComponentIds, + component::ComponentId, + entity::EntityAllocator, query::{FilteredAccess, FilteredAccessSet}, system::SystemAccess, }; @@ -281,11 +286,11 @@ mod tests { #[test] fn check_compatibility() { - let mut ids = ComponentIds::default(); + let ids = EntityAllocator::default(); let access_none = SystemAccess::None; let access_shared = SystemAccess::Shared({ let mut set = FilteredAccessSet::default(); - set.add_unfiltered_component_read(ids.next_mut()); + set.add_unfiltered_component_read(ComponentId::new(ids.alloc())); set }); let access_exclusive = SystemAccess::Exclusive; @@ -305,11 +310,11 @@ mod tests { #[test] fn conflict_reporting() { - let mut ids = ComponentIds::default(); + let ids = EntityAllocator::default(); let access_none = SystemAccess::None; let access_shared = SystemAccess::Shared({ let mut set = FilteredAccessSet::default(); - set.add_unfiltered_component_read(ids.next_mut()); + set.add_unfiltered_component_read(ComponentId::new(ids.alloc())); set }); let access_exclusive = SystemAccess::Exclusive; @@ -373,8 +378,8 @@ mod tests { #[test] fn conversion_to_access_sets() { - let mut ids = ComponentIds::default(); - let id_1 = ids.next_mut(); + let ids = EntityAllocator::default(); + let id_1 = ComponentId::new(ids.alloc()); let access_none = SystemAccess::None; let access_shared = SystemAccess::Shared({ @@ -404,13 +409,13 @@ mod tests { #[test] fn extending_access() { - let mut ids = ComponentIds::default(); + let ids = EntityAllocator::default(); let mut access = SystemAccess::default(); let access_none = SystemAccess::None; let access_shared = SystemAccess::Shared({ let mut set = FilteredAccessSet::default(); - set.add_unfiltered_component_read(ids.next_mut()); + set.add_unfiltered_component_read(ComponentId::new(ids.alloc())); set }); let access_exclusive = SystemAccess::Exclusive; diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 0299950bfecb2..cee2d0b1731b9 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -9,12 +9,12 @@ use crate::{ bundle::Bundles, change_detection::{ComponentTicksMut, ComponentTicksRef, Tick}, component::{ComponentId, Components, Mutable}, - entity::{Entities, EntityAllocator}, + entity::{ContainsEntity, Entities, EntityAllocator}, query::{ Access, FilteredAccess, IterQueryData, QueryData, QueryFilter, QuerySingleError, QueryState, ReadOnlyQueryData, }, - resource::{Resource, ResourceEntities, IS_RESOURCE}, + resource::{Resource, IS_RESOURCE}, system::{Query, Single, SystemAccess, SystemMeta, SystemState}, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FromWorld, World}, }; @@ -33,6 +33,7 @@ use core::{ marker::PhantomData, ops::{Deref, DerefMut}, }; +use log::warn; use smallvec::SmallVec; use thiserror::Error; @@ -397,6 +398,10 @@ unsafe impl SystemParam for Qu unsafe { QueryState::new_unchecked(world) } } + #[expect( + clippy::result_large_err, + reason = "Boxing `FilteredAccessSet` adds unnecessary noise." + )] fn init_access( state: &Self::State, _system_meta: &mut SystemMeta, @@ -788,7 +793,21 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { type Item<'w, 's> = Res<'w, T>; fn init_state(world: &mut World) -> Self::State { - world.components_registrator().register_component::() + let component_id = world.components_registrator().register_component::(); + if world + .get_required_components_by_id(component_id) + .is_some_and(|required| required.direct.contains_key(&IS_RESOURCE)) + { + let name = world + .components() + .get_name(component_id) + .expect("resource is registered"); + warn!( + "Resource {} does not have IsResource as a required component, hence it cannot be queried through Res.", + name + ); + } + component_id } fn init_access( @@ -814,9 +833,20 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Result, SystemParamValidationError> { - let (ptr, ticks) = world.get_resource_with_ticks(component_id).ok_or_else(|| { - SystemParamValidationError::invalid::("Resource does not exist") - })?; + let entity = component_id.entity(); + let entity_cell = world + .get_entity(entity) + .map_err(|_| SystemParamValidationError::invalid::("Resource does not exist"))?; + if !entity_cell.contains_id(IS_RESOURCE) { + return Err(SystemParamValidationError::invalid::( + "Resource does not have IsResource", + )); + } + // SAFETY: Through the scheduler we have unique access to this resource + let (ptr, ticks) = + unsafe { entity_cell.get_by_id_with_ticks(component_id) }.ok_or_else(|| { + SystemParamValidationError::invalid::("Resource does not exist") + })?; Ok(Res { value: ptr.deref(), ticks: ComponentTicksRef { @@ -837,7 +867,21 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> type Item<'w, 's> = ResMut<'w, T>; fn init_state(world: &mut World) -> Self::State { - world.components_registrator().register_component::() + let component_id = world.components_registrator().register_component::(); + if world + .get_required_components_by_id(component_id) + .is_some_and(|required| required.direct.contains_key(&IS_RESOURCE)) + { + let name = world + .components() + .get_name(component_id) + .expect("resource is registered"); + warn!( + "Resource {} does not have IsResource as a required component, hence it cannot be queried through ResMut.", + name + ); + } + component_id } fn init_access( @@ -863,9 +907,20 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Result, SystemParamValidationError> { - let value = world.get_resource_mut_by_id(component_id).ok_or_else(|| { + let entity = component_id.entity(); + let entity_cell = world + .get_entity(entity) + .map_err(|_| SystemParamValidationError::invalid::("Resource does not exist"))?; + if !entity_cell.contains_id(IS_RESOURCE) { + return Err(SystemParamValidationError::invalid::( + "Resource does not have IsResource", + )); + } + // SAFETY: Through the scheduler we have unique access to this resource + let value = unsafe { entity_cell.get_mut_by_id(component_id).ok() }.ok_or_else(|| { SystemParamValidationError::invalid::("Resource does not exist") })?; + Ok(ResMut { value: value.value.deref_mut::(), ticks: ComponentTicksMut { @@ -1538,38 +1593,6 @@ unsafe impl<'a> SystemParam for &'a Archetypes { } } -// SAFETY: Only reads World resource entities -unsafe impl<'a> ReadOnlySystemParam for &'a ResourceEntities {} - -// SAFETY: no component value access -unsafe impl<'a> SystemParam for &'a ResourceEntities { - type State = (); - type Item<'w, 's> = &'w ResourceEntities; - - fn init_state(_world: &mut World) -> Self::State {} - - fn init_access( - _state: &Self::State, - _system_meta: &mut SystemMeta, - system_access: &mut SystemAccess, - ) -> Result<(), SystemParamAccessConflict> { - system_access.try_extend_metadata().map_err(|access| { - SystemParamAccessConflict::new::(access) - .with_suggestion_if_exclusive(system_access, "Calling `World::resource_entities()`") - }) - } - - #[inline] - unsafe fn get_param<'w, 's>( - _state: &'s mut Self::State, - _system_meta: &SystemMeta, - world: UnsafeWorldCell<'w>, - _change_tick: Tick, - ) -> Result, SystemParamValidationError> { - Ok(world.resource_entities()) - } -} - // SAFETY: Only reads World components unsafe impl<'a> ReadOnlySystemParam for &'a Components {} @@ -2952,7 +2975,7 @@ mod tests { use crate::query::Without; use crate::resource::IsResource; use crate::schedule::Schedule; - use crate::system::{assert_is_system, Commands, IntoSystem, System}; + use crate::system::{assert_is_system, Commands, IntoSystem, RegisteredSystemError, System}; use crate::world::EntityMut; use core::cell::RefCell; @@ -3268,6 +3291,23 @@ mod tests { fn message_system(_: MessageReader) {} } + #[test] + fn missing_resource_marker() { + #[derive(Component, Default)] + struct R; + // In order for Res and ResMut queries to work, there should always be `IsResource` + // on the resource entity for every type `R` that implements Resource. + impl Resource for R {} + + let mut world = World::new(); + world.init_resource::(); + + assert!(matches!( + world.run_system_cached(|_: Res, _: Option>>| {}), + Err(RegisteredSystemError::Failed(_)) + ),); + } + #[test] fn test_exclusive_system_params() { #[derive(Resource, Default)] 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 3782cab77927e..1c2f9cd7e7bd3 100644 --- a/crates/bevy_ecs/src/world/entity_access/world_mut.rs +++ b/crates/bevy_ecs/src/world/entity_access/world_mut.rs @@ -14,7 +14,7 @@ use crate::{ ReleaseStateQueryData, SingleEntityQueryData, }, relationship::RelationshipHookMode, - resource::{Resource, ResourceEntities}, + resource::Resource, storage::{SparseSets, Table}, system::EntityCommands, template::{SceneEntityReferences, Template, TemplateContext}, @@ -735,13 +735,6 @@ impl<'w> EntityWorldMut<'w> { }) } - /// Retrieves this world's [`ResourceEntities`]. - #[inline] - #[track_caller] - pub fn resource_entities(&self) -> &ResourceEntities { - self.world.resource_entities() - } - /// Retrieves the [`Entity`] associated with the resource of type `R`, if it exists. #[inline] #[track_caller] @@ -1822,7 +1815,13 @@ impl<'w> EntityWorldMut<'w> { } table_row = remove_result.table_row; - for component_id in archetype.sparse_set_components() { + let sparse_set_components = archetype.iter_components().filter(|&id| { + // SAFETY: Every archetype component is registered in this world. + unsafe { self.world.components.get_info_unchecked(id) }.storage_type() + == StorageType::SparseSet + }); + + for component_id in sparse_set_components { // set must have existed for the component to be added. let sparse_set = self .world diff --git a/crates/bevy_ecs/src/world/filtered_resource.rs b/crates/bevy_ecs/src/world/filtered_resource.rs index 2475f0175032a..9d2a46b99d598 100644 --- a/crates/bevy_ecs/src/world/filtered_resource.rs +++ b/crates/bevy_ecs/src/world/filtered_resource.rs @@ -3,6 +3,7 @@ use crate::{ component::ComponentId, query::Access, resource::Resource, + resource::IS_RESOURCE, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; use bevy_ptr::Ptr; @@ -184,6 +185,17 @@ impl<'w, 's> FilteredResources<'w, 's> { /// Gets a pointer to the resource with the given [`ComponentId`] if it exists and the `FilteredResources` has access to it. pub fn get_by_id(&self, component_id: ComponentId) -> Result, ResourceFetchError> { + assert!( + // SAFETY: We only access required components + unsafe { + self.world + .world_metadata() + .get_required_components_by_id(component_id) + .is_some_and(|required| required.direct.contains_key(&IS_RESOURCE)) + }, + "resource does not have IsResource as a required component" + ); + if !self.access.has_read(component_id) { return Err(ResourceFetchError::NoResourceAccess(component_id)); } @@ -483,6 +495,18 @@ impl<'w, 's> FilteredResourcesMut<'w, 's> { .components() .valid_component_id::() .ok_or(ResourceFetchError::NotRegistered)?; + + assert!( + // SAFETY: We only access required components + unsafe { + self.world + .world_metadata() + .get_required_components_by_id(component_id) + .is_some_and(|required| required.direct.contains_key(&IS_RESOURCE)) + }, + "resource does not have IsResource as a required component" + ); + // SAFETY: THe caller ensures that there are no conflicting borrows. unsafe { self.get_mut_by_id_unchecked(component_id) } // SAFETY: The underlying type of the resource is `R`. diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index cef2af70c7507..cc2466b108ef9 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -39,11 +39,13 @@ use crate::{ CheckChangeTicks, ComponentTicks, ComponentTicksMut, MaybeLocation, MutUntyped, Tick, }, component::{ - Component, ComponentDescriptor, ComponentId, ComponentIds, ComponentInfo, Components, + Component, ComponentDescriptor, ComponentId, ComponentInfo, Components, ComponentsQueuedRegistrator, ComponentsRegistrator, Mutable, RequiredComponents, RequiredComponentsError, }, - entity::{Entities, Entity, EntityAllocator, EntityNotSpawnedError, SpawnError}, + entity::{ + ContainsEntity, Entities, Entity, EntityAllocator, EntityNotSpawnedError, SpawnError, + }, entity_disabling::DefaultQueryFilters, error::{ErrorHandler, FallbackErrorHandler}, lifecycle::{ @@ -54,7 +56,7 @@ use crate::{ observer::Observers, 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}, system::Commands, @@ -100,8 +102,6 @@ pub struct World { pub(crate) entities: Entities, 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, @@ -133,7 +133,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(), @@ -147,7 +146,6 @@ impl Default for World { last_trigger_id: 0, command_queue_start: 0, command_queue: SyncUnsafeCell::new(CommandQueue::silent()), - component_ids: ComponentIds::default(), }; world.bootstrap(); world @@ -262,26 +260,20 @@ 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. #[inline] pub fn components_queue(&self) -> ComponentsQueuedRegistrator<'_> { // SAFETY: These are from the same world. - unsafe { ComponentsQueuedRegistrator::new(&self.components, &self.component_ids) } + unsafe { ComponentsQueuedRegistrator::new(&self.components, &self.entity_allocator) } } /// Prepares a [`ComponentsRegistrator`] for the world. #[inline] pub fn components_registrator(&mut self) -> ComponentsRegistrator<'_> { // SAFETY: These are from the same world. - unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) } + unsafe { ComponentsRegistrator::new(&mut self.components, &self.entity_allocator) } } /// Retrieves this world's [`Storages`] collection. @@ -1490,7 +1482,8 @@ impl World { f: impl FnOnce(&mut R) -> S, ) -> Result, EntityMutableFetchError> { let component_id = self.register_component::(); - if let Some(entity) = self.resource_entities.get(component_id) { + let entity = component_id.entity(); + if self.entities().contains_spawned(entity) { let mut world = DeferredWorld::from(&mut *self); let result = world.modify_component_with_relationship_hook_mode( entity, @@ -1526,7 +1519,8 @@ impl World { component_id: ComponentId, f: impl for<'a> FnOnce(MutUntyped<'a>) -> S, ) -> Result, EntityMutableFetchError> { - if let Some(entity) = self.resource_entities.get(component_id) { + let entity = component_id.entity(); + if self.entities().contains_spawned(entity) { let mut world = DeferredWorld::from(&mut *self); let result = world.modify_component_by_id_with_relationship_hook_mode( @@ -1995,9 +1989,9 @@ impl World { caller: MaybeLocation, ) -> (ComponentId, EntityWorldMut<'_>) { let resource_id = self.register_component::(); + let entity = resource_id.entity(); - if let Some(entity) = self.resource_entities.get(resource_id) { - let entity_ref = self.get_entity(entity).expect("ResourceCache is in sync"); + if let Ok(entity_ref) = self.get_entity(entity) { if !entity_ref.contains_id(resource_id) { let resource = func(self); move_as_ptr!(resource); @@ -2013,7 +2007,9 @@ impl World { let resource = func(self); move_as_ptr!(resource); - let entity_mut = self.spawn_with_caller(resource, caller); // ResourceCache is updated automatically + // cannot panic because self.get_entity(entity) was None (entity hasn't been spawned) + // and self.register_component ensures that the entity is not invalid. + let entity_mut = self.spawn_at_unchecked(entity, resource, caller); (resource_id, entity_mut) } @@ -2119,7 +2115,7 @@ 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 = resource_id.entity(); let value = self .get_entity_mut(entity) .expect("ResourceCache is in sync") @@ -2157,9 +2153,7 @@ 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) - { + if let Ok(entity_ref) = self.get_entity(component_id.entity()) { return entity_ref.contains_id(component_id); } false @@ -2247,7 +2241,7 @@ impl World { &self, component_id: ComponentId, ) -> Option { - let entity = self.resource_entities.get(component_id)?; + let entity = component_id.entity(); let entity_ref = self.get_entity(entity).ok()?; entity_ref.get_change_ticks_by_id(component_id) } @@ -2433,7 +2427,7 @@ impl World { #[track_caller] pub fn resource_entity(&self) -> Option { let component_id = self.component_id::()?; - self.resource_entities().get(component_id) + Some(component_id.entity()) } /// Gets an immutable reference to the non-send data of the given type, if it exists. @@ -2883,7 +2877,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 = component_id.entity(); let mut entity_mut = self.get_entity_mut(entity).ok()?; let mut ticks = entity_mut.get_change_ticks::()?; @@ -3067,12 +3061,12 @@ impl World { value: OwningPtr<'_>, caller: MaybeLocation, ) { + let entity = component_id.entity(); // 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 self.entities().contains_spawned(entity) { + self.entity_mut(entity) } else { - self.spawn_empty() + self.spawn_empty_at_unchecked(entity, caller) }; // SAFETY: pointer valid for this component id per precondition unsafe { @@ -3439,9 +3433,17 @@ 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(); - for (component_id, entity) in pairs { - self.entity_mut(entity).remove_by_id(component_id); + let ids: Vec = self + .components() + .iter_registered() + .map(|(id, _)| id) + .collect(); + for component_id in ids { + let entity = component_id.entity(); + if let Ok(mut entity) = self.get_entity_mut(entity) { + // only resource entities with a matching component_id should have a component. + entity.remove_by_id(component_id); + } } } @@ -3471,7 +3473,7 @@ impl World { // SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too. let mut registrator = - unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; + unsafe { ComponentsRegistrator::new(&mut self.components, &self.entity_allocator) }; // SAFETY: `registrator`, `self.storages` and `self.bundles` all come from this world. unsafe { @@ -3488,7 +3490,7 @@ impl World { // SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too. let mut registrator = - unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) }; + unsafe { ComponentsRegistrator::new(&mut self.components, &self.entity_allocator) }; // SAFETY: `registrator`, `self.bundles` and `self.storages` are all from this world. unsafe { @@ -3650,13 +3652,13 @@ impl World { /// ``` #[inline] pub fn iter_resources(&self) -> impl Iterator)> { - self.resource_entities - .iter() - .filter_map(|(component_id, entity)| { - let component_info = self.components().get_info(component_id)?; + self.components + .iter_registered() + .filter_map(|(id, component_info)| { + let entity = id.entity(); let entity_cell = self.get_entity(entity).ok()?; - let resource = entity_cell.get_by_id(component_id).ok()?; - Some((component_id, component_info, resource)) + let resource = entity_cell.get_by_id(id).ok()?; + Some((id, component_info, resource)) }) } @@ -3729,18 +3731,12 @@ impl World { &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 - .iter() - .filter_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()?; + unsafe_world + .components() + .iter_registered() + .filter_map(move |(id, component_info)| { + let entity_cell = unsafe_world.get_entity(id.entity()).ok()?; // SAFETY: // - We have exclusive world access @@ -3748,9 +3744,9 @@ impl World { // 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()? }; + let mut_untyped = unsafe { entity_cell.get_mut_by_id(id).ok()? }; - Some((component_id, component_info, mut_untyped)) + Some((id, component_info, mut_untyped)) }) } @@ -3801,8 +3797,8 @@ 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) - && let Ok(mut entity_mut) = self.get_entity_mut(entity) + let entity = component_id.entity(); + if let Ok(mut entity_mut) = self.get_entity_mut(entity) && entity_mut.contains_id(component_id) { entity_mut.remove_by_id(component_id); @@ -4082,7 +4078,7 @@ mod tests { prelude::{DetectChanges, Event, Mut, On, Res}, ptr::OwningPtr, resource::Resource, - world::{error::EntityMutableFetchError, DeferredWorld}, + world::{error::EntityMutableFetchError, DeferredWorld, MutUntyped}, }; use alloc::{ borrow::ToOwned, @@ -4093,6 +4089,7 @@ mod tests { }; use bevy_ecs_macros::Component; use bevy_platform::collections::{HashMap, HashSet}; + use bevy_ptr::Ptr; use bevy_utils::prelude::DebugName; use core::{ any::TypeId, @@ -4274,19 +4271,14 @@ mod tests { world.insert_resource(TestResource3); world.remove_resource::(); - let id1 = world.component_id::().unwrap(); - let id2 = world.component_id::().unwrap(); + let mut resources = world + .iter_resources() + .collect::)>>(); + resources.sort_by_key(|a| a.0); - let mut iter = world.iter_resources(); - - let (id, info, ptr) = iter.next().unwrap(); - assert_eq!(id, id1); - assert_eq!(info.name(), DebugName::type_name::()); - // SAFETY: We know that the resource is of type `TestResource` - assert_eq!(unsafe { ptr.deref::().0 }, 42); + assert_eq!(resources.len(), 2); - let (id, info, ptr) = iter.next().unwrap(); - assert_eq!(id, id2); + let (_, info, ptr) = resources[0]; assert_eq!(info.name(), DebugName::type_name::()); assert_eq!( // SAFETY: We know that the resource is of type `TestResource2` @@ -4294,7 +4286,10 @@ mod tests { &"Hello, world!".to_string() ); - assert!(iter.next().is_none()); + let (_, info, ptr) = resources[1]; + assert_eq!(info.name(), DebugName::type_name::()); + // SAFETY: We know that the resource is of type `TestResource` + assert_eq!(unsafe { ptr.deref::().0 }, 42); } #[test] @@ -4307,27 +4302,28 @@ mod tests { world.insert_resource(TestResource3); world.remove_resource::(); - let id1 = world.component_id::().unwrap(); - let id2 = world.component_id::().unwrap(); + let mut resources = + world + .iter_resources_mut() + .collect::)>>(); + resources.sort_by_key(|a| a.0); - let mut iter = world.iter_resources_mut(); + let mut iter = resources.into_iter(); - let (id, info, mut mut_untyped) = iter.next().unwrap(); - assert_eq!(id, id1); - assert_eq!(info.name(), DebugName::type_name::()); - // SAFETY: We know that the resource is of type `TestResource` - unsafe { - mut_untyped.as_mut().deref_mut::().0 = 43; - }; - - let (id, info, mut mut_untyped) = iter.next().unwrap(); - assert_eq!(id, id2); + let (_, info, mut mut_untyped) = iter.next().unwrap(); assert_eq!(info.name(), DebugName::type_name::()); // SAFETY: We know that the resource is of type `TestResource2` unsafe { mut_untyped.as_mut().deref_mut::().0 = "Hello, world?".to_string(); }; + let (_, info, mut mut_untyped) = iter.next().unwrap(); + assert_eq!(info.name(), DebugName::type_name::()); + // SAFETY: We know that the resource is of type `TestResource` + unsafe { + mut_untyped.as_mut().deref_mut::().0 = 43; + }; + assert!(iter.next().is_none()); drop(iter); diff --git a/crates/bevy_ecs/src/world/reflect.rs b/crates/bevy_ecs/src/world/reflect.rs index 59e07e1e4c1d4..6fc49b37b4f87 100644 --- a/crates/bevy_ecs/src/world/reflect.rs +++ b/crates/bevy_ecs/src/world/reflect.rs @@ -199,10 +199,13 @@ impl World { resource_id: ComponentId, reflected_resource: Box, ) { - if let Some(entity) = self.resource_entities().get(resource_id) { + let entity = resource_id.entity(); + if self.entities().contains_spawned(entity) { self.entity_mut(entity).insert_reflect(reflected_resource); } else { - self.spawn_empty().insert_reflect(reflected_resource); + self.spawn_empty_at(entity) + .expect("entity isn't already spawned") + .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 2616efd34f7c5..42e21019c99c9 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -17,7 +17,7 @@ use crate::{ observer::Observers, prelude::Component, query::{DebugCheckedUnwrap, QueryAccessError, ReleaseStateQueryData, SingleEntityQueryData}, - resource::{Resource, ResourceEntities}, + resource::Resource, storage::{ComponentSparseSet, Storages, Table}, system::Commands, }; @@ -342,17 +342,6 @@ impl<'w> UnsafeWorldCell<'w> { &unsafe { self.world_metadata() }.components } - /// 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. - #[inline] - pub unsafe fn resource_entities(self) -> &'w ResourceEntities { - // SAFETY: - // - we only access world metadata - &unsafe { self.world_metadata() }.resource_entities - } - /// Retrieves this world's collection of [removed components](RemovedComponentMessages). pub fn removed_components(self) -> &'w RemovedComponentMessages { // SAFETY: @@ -518,8 +507,7 @@ impl<'w> UnsafeWorldCell<'w> { /// - no mutable reference to the resource exists at the same time #[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 = component_id.entity(); let entity_cell = self.get_entity(entity).ok()?; // SAFETY: Exclusive access per preconditions unsafe { entity_cell.get_by_id(component_id) } @@ -604,8 +592,7 @@ 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 = component_id.entity(); let entity_cell = self.get_entity(entity).ok()?; // SAFETY: Access permissions and uniqueness per preconditions unsafe { entity_cell.get_mut_by_id(component_id).ok() } @@ -703,16 +690,13 @@ impl<'w> UnsafeWorldCell<'w> { self, 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(); + let entity = self.get_entity(component_id.entity()).ok()?; // 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 - unsafe { get_component_and_ticks(self, component_id, storage_type, entity, location) } + unsafe { entity.get_by_id_with_ticks(component_id) } } // Shorthand helper function for getting the data and change ticks for a resource. @@ -1152,6 +1136,38 @@ impl<'w> UnsafeEntityCell<'w> { } } + /// Gets the component and ticks of the given [`ComponentId`] from the entity. + /// + /// **You should prefer to use the typed API where possible and only + /// use this in cases where the actual component types are not known at + /// compile time.** + /// + /// Unlike [`UnsafeEntityCell::get`], this returns a raw pointer to the component, + /// which is only valid while the `'w` borrow of the lifetime is active. + /// + /// # Safety + /// It is the caller's responsibility to ensure that + /// - the [`UnsafeEntityCell`] has permission to access the component + /// - no other mutable references to the component exist at the same time + #[inline] + pub unsafe fn get_by_id_with_ticks( + self, + component_id: ComponentId, + ) -> Option<(Ptr<'w>, ComponentTickCells<'w>)> { + let info = self.world.components().get_info(component_id)?; + + // SAFETY: + unsafe { + get_component_and_ticks( + self.world, + component_id, + info.storage_type(), + self.entity, + self.location, + ) + } + } + /// Retrieves a mutable untyped reference to the given `entity`'s [`Component`] of the given [`ComponentId`]. /// Returns `None` if the `entity` does not have a [`Component`] of the given type, /// or if the component is immutable. diff --git a/crates/bevy_remote/src/builtin_methods.rs b/crates/bevy_remote/src/builtin_methods.rs index 47af768a3a45b..242f0432e5495 100644 --- a/crates/bevy_remote/src/builtin_methods.rs +++ b/crates/bevy_remote/src/builtin_methods.rs @@ -9,7 +9,7 @@ use bevy_dev_tools::schedule_data::serde::ScheduleData; use bevy_diagnostic::{DiagnosticPath, DiagnosticsStore}; use bevy_ecs::{ component::ComponentId, - entity::Entity, + entity::{ContainsEntity, Entity}, hierarchy::ChildOf, lifecycle::RemovedComponentEntity, message::MessageCursor, @@ -2174,10 +2174,7 @@ fn get_resource_entity_pair( .components() .get_id(type_id) .ok_or(anyhow!("Resource not registered: `{}`", resource_path))?; - let entity = world - .resource_entities() - .get(component_id) - .ok_or(anyhow!("Resource entity does not exist."))?; + let entity = component_id.entity(); Ok((entity, component_id)) } diff --git a/crates/bevy_settings/src/lib.rs b/crates/bevy_settings/src/lib.rs index 68e9512d4bb71..5bd6e59399322 100644 --- a/crates/bevy_settings/src/lib.rs +++ b/crates/bevy_settings/src/lib.rs @@ -22,6 +22,7 @@ use std::collections::HashMap; use bevy_app::{App, Plugin, PostUpdate}; use bevy_ecs::{ change_detection::Tick, + entity::ContainsEntity, reflect::{AppTypeRegistry, ReflectComponent, ReflectResource}, resource::Resource, system::{Command, Commands, Res, ResMut}, @@ -402,14 +403,15 @@ fn resources_to_toml( continue; }; - let Some(res_entity) = world.resource_entities().get(component_id) else { + let res_entity = component_id.entity(); + if !world.entities().contains_spawned(res_entity) { continue; - }; + } + let res_entity_ref = world.entity(res_entity); let Some(reflect) = cmp.reflect(res_entity_ref) else { continue; }; - let serializer = TypedReflectSerializer::new(reflect.as_partial_reflect(), types); let toml_value = if let Some(settings_key) = settings_key { @@ -541,10 +543,10 @@ fn apply_settings_to_world( let settings_key = reflect_settings_group.settings_key_name; 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 component_id = reflect_component.register_component(world); + let res_entity = component_id.entity(); - if let Some(res_entity) = res_entity { + if world.entities().contains_spawned(res_entity) { // Resource already exists, so apply toml properties to it. let res_entity_mut = world.entity_mut(res_entity); let Some(mut reflect) = reflect_component.reflect_mut(res_entity_mut) else { @@ -567,9 +569,12 @@ fn apply_settings_to_world( } } else { // The resource does not exist, so create a default. + let mut res_entity = world + .spawn_empty_at(component_id.entity()) + .expect("entity was just allocated"); + let reflect_default = ty.data::().unwrap(); let mut default_value = reflect_default.default(); - let mut res_entity = world.spawn_empty(); if let Some(toml) = toml && let Some(value) = toml.get(settings_group) diff --git a/crates/bevy_world_serialization/src/dynamic_world.rs b/crates/bevy_world_serialization/src/dynamic_world.rs index 526357c64e1df..44de62d00d19c 100644 --- a/crates/bevy_world_serialization/src/dynamic_world.rs +++ b/crates/bevy_world_serialization/src/dynamic_world.rs @@ -2,7 +2,7 @@ use crate::{DynamicWorldBuilder, WorldAsset, WorldInstanceSpawnError}; use bevy_asset::Asset; use bevy_ecs::reflect::ReflectResource; use bevy_ecs::{ - entity::{Entity, EntityHashMap, SceneEntityMapper}, + entity::{ContainsEntity, Entity, EntityHashMap, SceneEntityMapper}, reflect::{AppTypeRegistry, ReflectComponent}, world::World, }; @@ -181,13 +181,12 @@ impl DynamicWorld { .expect("ReflectComponent is depended on ReflectResource"); let resource_id = reflect_component.register_component(world); + let entity = resource_id.entity(); - // 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) { - entity - } else { - world.spawn_empty().id() - }; + // check if the resource already exists, if not spawn it + if !world.entities().contains_spawned(entity) { + let _ = world.spawn_empty_at(entity); + } SceneEntityMapper::world_scope(entity_map, world, |world, mapper| { reflect_component.apply_or_insert_mapped( diff --git a/crates/bevy_world_serialization/src/dynamic_world_builder.rs b/crates/bevy_world_serialization/src/dynamic_world_builder.rs index 727d7adc9fecc..eb30fa10e0035 100644 --- a/crates/bevy_world_serialization/src/dynamic_world_builder.rs +++ b/crates/bevy_world_serialization/src/dynamic_world_builder.rs @@ -6,6 +6,7 @@ use alloc::collections::BTreeMap; use bevy_ecs::resource::IS_RESOURCE; use bevy_ecs::{ component::{Component, ComponentId}, + entity::ContainsEntity, entity_disabling::DefaultQueryFilters, prelude::Entity, reflect::{ReflectComponent, ReflectResource}, @@ -377,10 +378,17 @@ impl<'w> DynamicWorldBuilder<'w> { .components() .get_valid_id(TypeId::of::()); - for (component_id, entity) in self.original_world.resource_entities().iter() { + for (component_id, _) in self.original_world.components().iter_registered() { + let entity = component_id.entity(); + + if !self.original_world.entities().contains_spawned(entity) { + continue; + } + if Some(component_id) == original_world_dqf_id { continue; } + let mut extract_and_push = || { let type_id = self .original_world @@ -501,8 +509,8 @@ mod tests { assert_eq!(dynamic_world.entities.len(), 1); assert_eq!(dynamic_world.entities[0].entity, entity); assert_eq!(dynamic_world.entities[0].components.len(), 2); - assert!(dynamic_world.entities[0].components[0].represents::()); - assert!(dynamic_world.entities[0].components[1].represents::()); + assert!(dynamic_world.entities[0].components[0].represents::()); + assert!(dynamic_world.entities[0].components[1].represents::()); } #[test] diff --git a/crates/bevy_world_serialization/src/serde.rs b/crates/bevy_world_serialization/src/serde.rs index 64505eda4b8ee..ab8cdde410dfb 100644 --- a/crates/bevy_world_serialization/src/serde.rs +++ b/crates/bevy_world_serialization/src/serde.rs @@ -694,25 +694,25 @@ mod tests { ), }, entities: { - 4294967290: ( + 4294967279: ( components: { "bevy_world_serialization::serde::tests::FakeMesh3d": (Uuid("00000000-0000-0000-0000-000000000001")), }, ), - 4294967291: ( + 4294967281: ( components: { "bevy_world_serialization::serde::tests::Bar": (345), "bevy_world_serialization::serde::tests::Baz": (789), "bevy_world_serialization::serde::tests::Foo": (123), }, ), - 4294967292: ( + 4294967283: ( components: { "bevy_world_serialization::serde::tests::Bar": (345), "bevy_world_serialization::serde::tests::Foo": (123), }, ), - 4294967293: ( + 4294967285: ( components: { "bevy_world_serialization::serde::tests::Foo": (123), }, @@ -920,7 +920,7 @@ mod tests { assert_eq!( vec![ - 0, 1, 253, 255, 255, 255, 15, 1, 51, 98, 101, 118, 121, 95, 119, 111, 114, 108, + 0, 1, 245, 255, 255, 255, 15, 1, 51, 98, 101, 118, 121, 95, 119, 111, 114, 108, 100, 95, 115, 101, 114, 105, 97, 108, 105, 122, 97, 116, 105, 111, 110, 58, 58, 115, 101, 114, 100, 101, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77, 121, 67, 111, 109, 112, 111, 110, 101, 110, 116, 1, 2, 3, 102, 102, 166, 63, 205, 204, 108, 64, @@ -963,7 +963,7 @@ mod tests { assert_eq!( vec![ - 146, 128, 129, 206, 255, 255, 255, 253, 145, 129, 217, 51, 98, 101, 118, 121, 95, + 146, 128, 129, 206, 255, 255, 255, 245, 145, 129, 217, 51, 98, 101, 118, 121, 95, 119, 111, 114, 108, 100, 95, 115, 101, 114, 105, 97, 108, 105, 122, 97, 116, 105, 111, 110, 58, 58, 115, 101, 114, 100, 101, 58, 58, 116, 101, 115, 116, 115, 58, 58, 77, 121, 67, 111, 109, 112, 111, 110, 101, 110, 116, 147, 147, 1, 2, 3, 146, 202, diff --git a/crates/bevy_world_serialization/src/world_asset.rs b/crates/bevy_world_serialization/src/world_asset.rs index 458974e898533..40162a4e95d1c 100644 --- a/crates/bevy_world_serialization/src/world_asset.rs +++ b/crates/bevy_world_serialization/src/world_asset.rs @@ -5,8 +5,8 @@ use crate::{DynamicWorld, WorldInstanceSpawnError}; use bevy_asset::Asset; use bevy_ecs::resource::IS_RESOURCE; use bevy_ecs::{ - component::ComponentCloneBehavior, - entity::{Entity, EntityHashMap, SceneEntityMapper}, + component::{ComponentCloneBehavior, ComponentId}, + entity::{ContainsEntity, Entity, EntityHashMap, SceneEntityMapper}, entity_disabling::DefaultQueryFilters, reflect::{AppTypeRegistry, ReflectComponent, ReflectResource}, relationship::RelationshipHookMode, @@ -74,8 +74,19 @@ impl WorldAsset { .components() .get_id(TypeId::of::()); + let ids: Vec = self + .world + .components() + .iter_registered() + .map(|(id, _)| id) + .collect(); // Resources archetype - for (component_id, source_entity) in self.world.resource_entities().iter() { + for component_id in ids { + let source_entity = component_id.entity(); + + if !self.world.entities().contains_spawned(source_entity) { + continue; + } if Some(component_id) == self_dqf_id { continue; } @@ -112,21 +123,29 @@ impl WorldAsset { .data::() .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() - }; - - reflect_component.copy( - &self.world, - world, - source_entity, - destination_entity, - &type_registry, - ); + let Some(resource) = reflect_component + .reflect(self.world.entity(source_entity)) + .map(|component| clone_reflect_value(component.as_partial_reflect(), registration)) + else { + continue; + }; + + let destination_component_id = reflect_component.register_component(world); + let destination_entity = destination_component_id.entity(); + + if !world.entities().contains_spawned(destination_entity) { + let _ = world.spawn_empty_at(destination_entity); + } + + SceneEntityMapper::world_scope(entity_map, world, |world, mapper| { + reflect_component.apply_or_insert_mapped( + &mut world.entity_mut(source_entity), + resource.as_partial_reflect(), + &type_registry, + mapper, + RelationshipHookMode::Skip, + ); + }); } // Ensure that all source world entities have been allocated in the destination diff --git a/examples/ecs/dynamic.rs b/examples/ecs/dynamic.rs index e17796cbf84a4..61fb8c6b0edb9 100644 --- a/examples/ecs/dynamic.rs +++ b/examples/ecs/dynamic.rs @@ -127,7 +127,7 @@ fn main() { }; component_names.insert(name.to_string(), id); component_info.insert(id, info.clone()); - println!("Component {} created with id: {}", name, id.index()); + println!("Component {} created with id: {}", name, id.entity()); }); } "s" => { @@ -253,7 +253,7 @@ fn main() { println!( "Event '{name}' registered (key: {}) with a dynamic observer", - event_component_id.index() + event_component_id.entity() ); });