Skip to content

Basic inherited components/entity prefabs #18767

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft
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
3 changes: 1 addition & 2 deletions benches/benches/bevy_ecs/change_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,7 @@ fn few_changed_detection_generic<T: Component<Mutability = Mutable> + Default +
let mut world = setup::<T>(entity_count);
world.clear_trackers();
let mut query = world.query::<&mut T>();
let mut to_modify: Vec<bevy_ecs::prelude::Mut<T>> =
query.iter_mut(&mut world).collect();
let mut to_modify: Vec<_> = query.iter_mut(&mut world).collect();
to_modify.shuffle(&mut deterministic_rand());
for component in to_modify[0..amount_to_modify].iter_mut() {
black_box(component.bench_modify());
Expand Down
10 changes: 8 additions & 2 deletions crates/bevy_asset/src/asset_changed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

Copy link
Member

@cart cart Apr 8, 2025

Choose a reason for hiding this comment

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

I haven't done a review yet, but this PR seems similar in shape to the Flecs approach, which is cool and worthy of consideration, but I've always been hesitant about it. My biggest concerns about that approach:

  1. Lookup overhead: data accesses need to do more work to check if a component is inherited or not, and to reach into the correct table. I'd want comprehensive benchmarks of every code path this touches.
  2. Data access issues: inherited components are "shared data". Our whole access model assumes that if we lock down to the scope of an entity, its accesses will not intersect with other entities. Providing mutable access would violate this naively.
  3. User interface concerns: especially in combination with (2), this would almost certainly introduce user-facing interface weirdness. How would we write a system that modifies the position of all Transforms, when some of those transforms are inherited? Foisting this complexity on every system author isn't viable, so we would need to internalize it. Would we make Mut "smart" and have it check on every access whether it can write directly to the pointer or whether it needs to queue up a command to "clone on write"? That would introduce massive overhead as Mut is very "hot". Do we make only immutable components inheritable? If so then this system becomes effectively useless for prefabs, as we need a prefab system to support the whole range of bevy components.

There are certainly benefits to this model (memory usage and "inherited spawn speed" being big ones). But before taking a single step on this road (in terms of merged features), I'd want satisfying / holistic answers to all of those questions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've though about these concerns when working on this implementation, I think it should be possible?

  1. All entities that inherit components are stored in a separate archetype. This means that looking up which component is actually used needs to be done only once per archetype when iterating. It theoretically should be fast enough if there are many instances of a single entity prefab (and maybe even faster since there is less data that needs to be cached?).
  2. For this, I think that as long as inherited entities' components are not accessible mutably in the same system as entities which inherit these components, this should still probably work.
  3. Copy-on-write behavior should be a fairly rare occasion most of the time (it would happen only once) and I hope that we can optimize for the branch predictor to consider this branch cold.

Copy link
Member

@cart cart Apr 8, 2025

Choose a reason for hiding this comment

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

All entities that inherit components are stored in a separate archetype. This means that looking up which component is actually used needs to be done only once per archetype when iterating

This will also add a fixed cost to Query::get and EntityWorldMut::get ops right?

For this, I think that as long as inherited entities' components are not accessible mutably in the same system as entities which inherit these components, this should still probably work

This was referring to two entities that both inherit from a third entity both trying to mutably access the same component.

Copy-on-write behavior should be a fairly rare occasion most of the time (it would happen only once) and I hope that we can optimize for the branch predictor to consider this branch cold.

I'm curious to see what Mut COW looks like in practice. There are also corner cases to consider like two queries in a query set that both access the same (inherited) component mutably. How would they coordinate to ensure that both changes are applied / they don't step on each other? Note that structural changes cannot be applied within a normal system.

Copy link
Member

Choose a reason for hiding this comment

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

I'm dubious of the overhead and complexity this will introduce, but I'm also very excited and curious to see where this lands!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will also add a fixed cost to Query::get and EntityWorldMut::get ops right?

Yes, there's a flag for the archetype and the lookup is done only if the archetype contains any inherited components, which is what flecs does.

There are also corner cases to consider like two queries in a query set that both access the same (inherited) component mutably. How would they coordinate to ensure that both changes are applied / they don't step on each other? Note that structural changes cannot be applied within a normal system.

We can make a staging hashmap for inherited components that have been mutated during system runtime and return them if they're requested multiple times in the same system, if I understood your question.

use crate::{AsAssetId, Asset, AssetId};
use bevy_ecs::component::Components;
use bevy_ecs::storage::TableId;
use bevy_ecs::{
archetype::Archetype,
component::{ComponentId, Tick},
Expand Down Expand Up @@ -216,11 +217,16 @@ unsafe impl<A: AsAssetId> WorldQuery for AssetChanged<A> {
}
}

unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table) {
unsafe fn set_table<'w>(
fetch: &mut Self::Fetch<'w>,
state: &Self::State,
table: &'w Table,
table_id: TableId,
) {
if let Some(inner) = &mut fetch.inner {
// SAFETY: We delegate to the inner `set_table` for `A`
unsafe {
<&A>::set_table(inner, &state.asset_id, table);
<&A>::set_table(inner, &state.asset_id, table, table_id);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ indexmap = { version = "2.5.0", default-features = false }
variadics_please = { version = "1.1", default-features = false }
tracing = { version = "0.1", default-features = false, optional = true }
log = { version = "0.4", default-features = false }
bumpalo = "3"
bumpalo = { version = "3", features = ["boxed"] }

concurrent-queue = { version = "2.5.0", default-features = false }
[target.'cfg(not(all(target_has_atomic = "8", target_has_atomic = "16", target_has_atomic = "32", target_has_atomic = "64", target_has_atomic = "ptr")))'.dependencies]
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,9 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
}
}

unsafe fn new_archetype(state: &mut Self::State, archetype: &#path::archetype::Archetype, system_meta: &mut #path::system::SystemMeta) {
unsafe fn new_archetype(state: &mut Self::State, archetype: &#path::archetype::Archetype, inherited_components: &#path::inheritance::InheritedComponents, system_meta: &mut #path::system::SystemMeta) {
// SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`.
unsafe { <#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::new_archetype(&mut state.state, archetype, system_meta) }
unsafe { <#fields_alias::<'_, '_, #punctuated_generic_idents> as #path::system::SystemParam>::new_archetype(&mut state.state, archetype, inherited_components, system_meta) }
}

fn apply(state: &mut Self::State, system_meta: &#path::system::SystemMeta, world: &mut #path::world::World) {
Expand Down
5 changes: 3 additions & 2 deletions crates/bevy_ecs/macros/src/world_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,10 @@ pub(crate) fn world_query_impl(
unsafe fn set_table<'__w>(
_fetch: &mut <Self as #path::query::WorldQuery>::Fetch<'__w>,
_state: &Self::State,
_table: &'__w #path::storage::Table
_table: &'__w #path::storage::Table,
_table_id: #path::storage::TableId
) {
#(<#field_types>::set_table(&mut _fetch.#named_field_idents, &_state.#named_field_idents, _table);)*
#(<#field_types>::set_table(&mut _fetch.#named_field_idents, &_state.#named_field_idents, _table, _table_id);)*
}

fn update_component_access(state: &Self::State, _access: &mut #path::query::FilteredAccess<#path::component::ComponentId>) {
Expand Down
67 changes: 59 additions & 8 deletions crates/bevy_ecs/src/archetype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ use crate::{
bundle::BundleId,
component::{ComponentId, Components, RequiredComponentConstructor, StorageType},
entity::{Entity, EntityLocation},
inheritance::InheritedArchetypeComponent,
observer::Observers,
storage::{ImmutableSparseSet, SparseArray, SparseSet, SparseSetIndex, TableId, TableRow},
world::{INHERITED, INHERIT_FROM},
};
use alloc::{boxed::Box, vec::Vec};
use bevy_platform::collections::HashMap;
Expand Down Expand Up @@ -360,6 +362,8 @@ bitflags::bitflags! {
const ON_REPLACE_OBSERVER = (1 << 7);
const ON_REMOVE_OBSERVER = (1 << 8);
const ON_DESPAWN_OBSERVER = (1 << 9);
const HAS_INHERITED_COMPONENTS = (1 << 10);
const IS_INHERITED= (1 << 11);
}
}

Expand All @@ -376,6 +380,7 @@ pub struct Archetype {
entities: Vec<ArchetypeEntity>,
components: ImmutableSparseSet<ComponentId, ArchetypeComponentInfo>,
pub(crate) flags: ArchetypeFlags,
pub(crate) inherited_components: HashMap<ComponentId, InheritedArchetypeComponent>,
}

impl Archetype {
Expand Down Expand Up @@ -408,10 +413,16 @@ impl Archetype {
// 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
component_index
.entry(component_id)
.or_default()
.insert(id, ArchetypeRecord { column: Some(idx) });
component_index.entry(component_id).or_default().insert(
id,
ArchetypeRecord {
column: Some(idx),
is_inherited: false,
},
);
if component_id == INHERIT_FROM {
flags.set(ArchetypeFlags::HAS_INHERITED_COMPONENTS, true);
}
}

for (component_id, archetype_component_id) in sparse_set_components {
Expand All @@ -426,10 +437,16 @@ impl Archetype {
archetype_component_id,
},
);
component_index
.entry(component_id)
.or_default()
.insert(id, ArchetypeRecord { column: None });
component_index.entry(component_id).or_default().insert(
id,
ArchetypeRecord {
column: None,
is_inherited: false,
},
);
if component_id == INHERITED {
flags.set(ArchetypeFlags::IS_INHERITED, true);
}
}
Self {
id,
Expand All @@ -438,6 +455,7 @@ impl Archetype {
components: archetype_components.into_immutable(),
edges: Default::default(),
flags,
inherited_components: Default::default(),
}
}

Expand Down Expand Up @@ -517,6 +535,16 @@ impl Archetype {
.map(|(component_id, info)| (*component_id, info.archetype_component_id))
}

pub(crate) fn components_with_inherited_and_archetype_component_id(
&self,
) -> impl Iterator<Item = (ComponentId, ArchetypeComponentId)> + '_ {
self.components_with_archetype_component_id().chain(
self.inherited_components
.iter()
.map(|(component_id, info)| (*component_id, info.archetype_component_id())),
)
}

/// Fetches an immutable reference to the archetype's [`Edges`], a cache of
/// archetypal relationships.
#[inline]
Expand Down Expand Up @@ -622,6 +650,11 @@ impl Archetype {
self.components.contains(component_id)
}

#[inline]
pub fn contains_with_inherited(&self, component_id: ComponentId) -> bool {
self.contains(component_id) || self.inherited_components.contains_key(&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.
Expand Down Expand Up @@ -719,6 +752,23 @@ impl Archetype {
pub fn has_despawn_observer(&self) -> bool {
self.flags().contains(ArchetypeFlags::ON_DESPAWN_OBSERVER)
}

#[inline]
/// Returns `true` if this archetype has components inherited from another archetype.
///
/// Use [`crate::inheritance::InheritedComponents`] to get get the list of inherited components.
pub fn has_inherited_components(&self) -> bool {
self.flags()
.contains(ArchetypeFlags::HAS_INHERITED_COMPONENTS)
}

#[inline]
/// Returns `true` if this archetype is inherited by any other archetypes.
///
/// Use [`crate::inheritance::InheritedComponents`] to get get the list of inherited components.
pub fn is_inherited(&self) -> bool {
self.flags().contains(ArchetypeFlags::IS_INHERITED)
}
}

/// The next [`ArchetypeId`] in an [`Archetypes`] collection.
Expand Down Expand Up @@ -810,6 +860,7 @@ pub struct ArchetypeRecord {
reason = "Currently unused, but planned to be used to implement a component index to improve performance of fragmenting relations."
)]
pub(crate) column: Option<usize>,
pub(crate) is_inherited: bool,
}

impl Archetypes {
Expand Down
54 changes: 53 additions & 1 deletion crates/bevy_ecs/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::{
RequiredComponents, StorageType, Tick,
},
entity::{Entities, Entity, EntityLocation},
inheritance::InheritedComponents,
observer::Observers,
prelude::World,
query::DebugCheckedUnwrap,
Expand Down Expand Up @@ -733,6 +734,8 @@ impl BundleInfo {
storages: &mut Storages,
components: &Components,
observers: &Observers,
entities: &Entities,
inherited_components: &mut InheritedComponents,
archetype_id: ArchetypeId,
) -> ArchetypeId {
if let Some(archetype_after_insert_id) = archetypes[archetype_id]
Expand Down Expand Up @@ -836,6 +839,12 @@ impl BundleInfo {
table_components,
sparse_set_components,
);
inherited_components.init_inherited_components(
entities,
archetypes,
&mut storages.tables,
new_archetype_id,
);
// Add an edge from the old archetype to the new archetype.
archetypes[archetype_id]
.edges_mut()
Expand Down Expand Up @@ -872,6 +881,8 @@ impl BundleInfo {
storages: &mut Storages,
components: &Components,
observers: &Observers,
entities: &Entities,
inherited_components: &mut InheritedComponents,
archetype_id: ArchetypeId,
intersection: bool,
) -> Option<ArchetypeId> {
Expand Down Expand Up @@ -947,6 +958,12 @@ impl BundleInfo {
next_table_components,
next_sparse_set_components,
);
inherited_components.init_inherited_components(
entities,
archetypes,
&mut storages.tables,
new_archetype_id,
);
Some(new_archetype_id)
};
let current_archetype = &mut archetypes[archetype_id];
Expand Down Expand Up @@ -1028,6 +1045,8 @@ impl<'w> BundleInserter<'w> {
&mut world.storages,
&world.components,
&world.observers,
&world.entities,
&mut world.inherited_components,
archetype_id,
);
if new_archetype_id == archetype_id {
Expand Down Expand Up @@ -1289,7 +1308,38 @@ impl<'w> BundleInserter<'w> {
}
};

let new_archetype = &*new_archetype;
let (new_archetype_id, is_inherited) = (new_archetype.id(), new_archetype.is_inherited());

if is_inherited {
let old_archetype_id = location.archetype_id;
let world = self.world.world_mut();
if new_location.table_id != location.table_id {
world
.inherited_components
.update_inherited_archetypes::<true>(
&mut world.archetypes,
&mut world.storages.tables,
new_archetype_id,
old_archetype_id,
entity,
new_location,
);
} else {
world
.inherited_components
.update_inherited_archetypes::<false>(
&mut world.archetypes,
&mut world.storages.tables,
new_archetype_id,
old_archetype_id,
entity,
new_location,
);
}
}

let new_archetype = &self.world.archetypes()[new_archetype_id];

// SAFETY: We have no outstanding mutable references to world as they were dropped
let mut deferred_world = unsafe { self.world.into_deferred() };

Expand Down Expand Up @@ -1399,6 +1449,8 @@ impl<'w> BundleSpawner<'w> {
&mut world.storages,
&world.components,
&world.observers,
&world.entities,
&mut world.inherited_components,
ArchetypeId::EMPTY,
);
let archetype = &mut world.archetypes[new_archetype_id];
Expand Down
5 changes: 4 additions & 1 deletion crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
bundle::BundleInfo,
change_detection::{MaybeLocation, MAX_CHANGE_AGE},
entity::{ComponentCloneCtx, Entity, EntityMapper, SourceComponent},
inheritance::InheritedBehavior,
query::DebugCheckedUnwrap,
relationship::RelationshipHookMode,
resource::Resource,
Expand Down Expand Up @@ -486,6 +487,8 @@ pub trait Component: Send + Sync + 'static {
/// A constant indicating the storage type used for this component.
const STORAGE_TYPE: StorageType;

const INHERITED_BEHAVIOR: InheritedBehavior = InheritedBehavior::Shared;

/// A marker type to assist Bevy with determining if this component is
/// mutable, or immutable. Mutable components will have [`Component<Mutability = Mutable>`],
/// while immutable components will instead have [`Component<Mutability = Immutable>`].
Expand Down Expand Up @@ -2641,7 +2644,7 @@ impl<'a> TickCells<'a> {
}

/// Records when a component or resource was added and when it was last mutably dereferenced (or added).
#[derive(Copy, Clone, Debug)]
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Debug, Clone))]
pub struct ComponentTicks {
/// Tick recording the time this component or resource was added.
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_ecs/src/entity/entity_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ impl<I: Iterator<Item: EntityEquivalent> + Debug> Debug for UniqueEntityIter<I>
mod tests {
use alloc::{vec, vec::Vec};

use crate::inheritance::MutComponent;
use crate::prelude::{Schedule, World};

use crate::component::Component;
Expand Down Expand Up @@ -522,7 +523,7 @@ mod tests {
.cloned();

// With `iter_many_mut` collecting is not possible, because you need to drop each `Mut`/`&mut` before the next is retrieved.
let _results: Vec<Mut<Thing>> =
let _results: Vec<MutComponent<Thing>> =
query.iter_many_unique_mut(&mut world, entity_set).collect();
}

Expand Down
Loading
Loading