Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions _release-content/migration-guides/components-as-entities.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
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`. This means that methods like `union_with` no longer work, use `bitor_assign` instead.
- `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`. If you need the entity linked with a `ComponentId`, simply call `component_id.entity()`.
3 changes: 0 additions & 3 deletions benches/benches/bevy_ecs/empty_archetypes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ fn empty_archetypes(criterion: &mut Criterion) {
schedule.add_systems(iter);
});
add_archetypes(&mut world, archetype_count);
world.clear_entities();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is clear_entities() completely unusable now? Any resource that had been spawned can never be created again, right? Should we just remove it completely?

And, should this bench still despawn the entities that are spawned by add_archetypes? We could presumably add a marker component to identify them.

let mut e = world.spawn_empty();
e.insert(A::<0>(1.0));
e.insert(A::<1>(1.0));
Expand Down Expand Up @@ -197,7 +196,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));
Expand Down Expand Up @@ -228,7 +226,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));
Expand Down
5 changes: 3 additions & 2 deletions crates/bevy_ecs/src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use crate::{
bundle::BundleId,
component::{ComponentId, Components, RequiredComponentConstructor, StorageType},
entity::{Entity, EntityLocation},
entity::{Entity, EntityEquivalentHashMap, EntityLocation},
event::Event,
observer::Observers,
query::DebugCheckedUnwrap,
Expand Down Expand Up @@ -762,7 +762,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<ComponentId, HashMap<ArchetypeId, ArchetypeRecord>>;
pub type ComponentIndex =
EntityEquivalentHashMap<ComponentId, HashMap<ArchetypeId, ArchetypeRecord>>;

/// The backing store of all [`Archetype`]s within a [`World`].
///
Expand Down
24 changes: 12 additions & 12 deletions crates/bevy_ecs/src/component/constants.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
//! Constant components included in every world.

/// `usize` for the [`Add`](crate::lifecycle::Add) component used in lifecycle observers.
pub const ADD: usize = 0;
/// `usize` for the [`Insert`](crate::lifecycle::Insert) component used in lifecycle observers.
pub const INSERT: usize = 1;
/// `usize` for the [`Discard`](crate::lifecycle::Discard) component used in lifecycle observers.
pub const DISCARD: usize = 2;
/// `usize` for the [`Remove`](crate::lifecycle::Remove) component used in lifecycle observers.
pub const REMOVE: usize = 3;
/// `usize` for [`Despawn`](crate::lifecycle::Despawn) component used in lifecycle observers.
pub const DESPAWN: usize = 4;
/// `usize` of the [`IsResource`](crate::resource::IsResource) component used to mark entities with resources.
pub const IS_RESOURCE: usize = 5;
/// `u32` for the [`Add`](crate::lifecycle::Add) component used in lifecycle observers.
pub const ADD: u32 = 0;
/// `u32` for the [`Insert`](crate::lifecycle::Insert) component used in lifecycle observers.
pub const INSERT: u32 = 1;
/// `u32` for the [`Discard`](crate::lifecycle::Discard) component used in lifecycle observers.
pub const DISCARD: u32 = 2;
/// `u32` for the [`Remove`](crate::lifecycle::Remove) component used in lifecycle observers.
pub const REMOVE: u32 = 3;
/// `u32` for [`Despawn`](crate::lifecycle::Despawn) component used in lifecycle observers.
pub const DESPAWN: u32 = 4;
/// `u32` of the [`IsResource`](crate::resource::IsResource) component used to mark entities with resources.
pub const IS_RESOURCE: u32 = 5;
112 changes: 59 additions & 53 deletions crates/bevy_ecs/src/component/info.rs
Original file line number Diff line number Diff line change
@@ -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")]
Expand All @@ -18,6 +18,9 @@ use crate::{
Component, ComponentCloneBehavior, ComponentMutability, QueuedComponents,
RequiredComponents, StorageType,
},
entity::{
ContainsEntity, Entity, EntityEquivalent, EntityEquivalentHashMap, EntityEquivalentHashSet,
},
lifecycle::ComponentHooks,
query::DebugCheckedUnwrap as _,
relationship::{
Expand Down Expand Up @@ -177,37 +180,53 @@ impl ComponentInfo {
derive(Reflect),
reflect(Debug, Hash, PartialEq, Clone)
)]
pub struct ComponentId(pub(super) 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<V> = EntityEquivalentHashMap<ComponentId, V>;

/// A set of [`ComponentId`]s.
pub type ComponentIdSet = EntityEquivalentHashSet<ComponentId>;

/// A value describing a component or resource, which may or may not correspond to a Rust type.
#[derive(Clone)]
pub struct ComponentDescriptor {
Expand Down Expand Up @@ -359,7 +378,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<Option<ComponentInfo>>,
pub(super) components: ComponentIdMap<ComponentInfo>,
pub(super) indices: TypeIdMap<ComponentId>,
// This is kept internal and local to verify that no deadlocks can occur.
pub(super) queued: bevy_platform::sync::RwLock<QueuedComponents>,
Expand All @@ -379,15 +398,10 @@ impl Components {
) {
descriptor.initialize(id, self);
let info = ComponentInfo::new(id, descriptor);
let least_len = id.0 + 1;
if self.components.len() < least_len {
self.components.resize_with(least_len, || None);
// SAFETY: The id has never been registered before.
unsafe {
self.components.insert_unique_unchecked(id, info);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How important is the perf from using insert_unique_unchecked here instead of an ordinary insert? I'd be inclined to use the safer version if we can.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a genuine performance difference, and I think it'd be best to keep using it.
However, "never having been registered before" meaning "does not exist in this collection" should be more extensively documented somewhere.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a genuine performance difference, and I think it'd be best to keep using it.

Does that difference matter for our case? The linked issue says it makes the insert 30% faster, but that's only a part of component registration, and I'm not sure we're even all that perf-sensitive for something that only runs once per component.


And thinking about it more, I'm not sure we can guarantee that the id is unique. A user can call world.entity_allocator_mut().free(entity) from safe code with the same Entity in a loop, and then register_component could allocate that same Entity for multiple components! It's fine to panic for something that bizarre, but I don't think it should allow UB.

Which also means we need to panic rather than silently overwrite if the key exists, so something like try_insert(id, info).expect(...).

}
// 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);
}

/// Returns the number of components registered or queued with this instance.
Expand Down Expand Up @@ -449,7 +463,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.
Expand All @@ -461,8 +475,8 @@ impl Components {
#[inline]
pub fn get_descriptor<'a>(&'a self, id: ComponentId) -> Option<Cow<'a, ComponentDescriptor>> {
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
Expand All @@ -482,8 +496,8 @@ impl Components {
#[inline]
pub fn get_name<'a>(&'a self, id: ComponentId) -> Option<DebugName> {
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
Expand All @@ -503,27 +517,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]
Expand All @@ -532,18 +538,16 @@ 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]
pub(crate) fn get_required_by(
&self,
id: ComponentId,
) -> Option<&IndexSet<ComponentId, FixedHasher>> {
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]
Expand All @@ -552,16 +556,16 @@ impl Components {
id: ComponentId,
) -> Option<&mut IndexSet<ComponentId, FixedHasher>> {
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.
/// Ids may be invalid if they are still queued to be registered.
/// 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()`].
Expand Down Expand Up @@ -663,18 +667,20 @@ impl Components {

/// Gets an iterator over all components fully registered with this instance.
pub fn iter_registered(&self) -> impl Iterator<Item = &ComponentInfo> + '_ {
self.components.iter().filter_map(Option::as_ref)
self.components.values()
}

/// Gets an iterator over all `ComponentId`s fully registered with this instance.
pub fn iter_registered_ids(&self) -> impl Iterator<Item = ComponentId> + '_ {
self.components.keys().copied()
}

pub(crate) fn get_relationship_accessor_mut(
&mut self,
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)
}
}
Loading
Loading