Skip to content

Commit

Permalink
Fix spawning empty bundles (#6425)
Browse files Browse the repository at this point in the history
# Objective

Alternative to #6424 
Fixes #6226

Fixes spawning empty bundles

## Solution

Add `BundleComponentStatus` trait and implement it for `AddBundle` and a new `SpawnBundleStatus` type (which always returns an Added status). `write_components` is now generic on `BundleComponentStatus` instead of taking `AddBundle` directly. This means BundleSpawner can now avoid needing AddBundle from the Empty archetype, which means BundleSpawner no longer needs a reference to the original archetype.

In theory this cuts down on the work done in `write_components` when spawning, but I'm seeing no change in the spawn benchmarks.
  • Loading branch information
cart committed Nov 3, 2022
1 parent e6a0164 commit 2e653e5
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 10 deletions.
31 changes: 31 additions & 0 deletions crates/bevy_ecs/src/archetype.rs
Expand Up @@ -35,6 +35,7 @@ impl ArchetypeId {
}
}

#[derive(Copy, Clone)]
pub(crate) enum ComponentStatus {
Added,
Mutated,
Expand All @@ -45,6 +46,36 @@ pub struct AddBundle {
pub(crate) bundle_status: Vec<ComponentStatus>,
}

/// This trait is used to report the status of [`Bundle`](crate::bundle::Bundle) components
/// being added to a given entity, relative to that entity's original archetype.
/// See [`crate::bundle::BundleInfo::write_components`] for more info.
pub(crate) trait BundleComponentStatus {
/// Returns the Bundle's component status for the given "bundle index"
///
/// # Safety
/// Callers must ensure that index is always a valid bundle index for the
/// Bundle associated with this [`BundleComponentStatus`]
unsafe fn get_status(&self, index: usize) -> ComponentStatus;
}

impl BundleComponentStatus for AddBundle {
#[inline]
unsafe fn get_status(&self, index: usize) -> ComponentStatus {
// SAFETY: caller has ensured index is a valid bundle index for this bundle
*self.bundle_status.get_unchecked(index)
}
}

pub(crate) struct SpawnBundleStatus;

impl BundleComponentStatus for SpawnBundleStatus {
#[inline]
unsafe fn get_status(&self, _index: usize) -> ComponentStatus {
// Components added during a spawn_bundle call are always treated as added
ComponentStatus::Added
}
}

/// Archetypes and bundles form a graph. Adding or removing a bundle moves
/// an [`Entity`] to a new [`Archetype`].
///
Expand Down
33 changes: 23 additions & 10 deletions crates/bevy_ecs/src/bundle.rs
Expand Up @@ -5,7 +5,10 @@
pub use bevy_ecs_macros::Bundle;

use crate::{
archetype::{AddBundle, Archetype, ArchetypeId, Archetypes, ComponentStatus},
archetype::{
Archetype, ArchetypeId, Archetypes, BundleComponentStatus, ComponentStatus,
SpawnBundleStatus,
},
component::{Component, ComponentId, ComponentTicks, Components, StorageType},
entity::{Entities, Entity, EntityLocation},
storage::{SparseSetIndex, SparseSets, Storages, Table},
Expand Down Expand Up @@ -340,13 +343,10 @@ impl BundleInfo {
) -> BundleSpawner<'a, 'b> {
let new_archetype_id =
self.add_bundle_to_archetype(archetypes, storages, components, ArchetypeId::EMPTY);
let (empty_archetype, archetype) =
archetypes.get_2_mut(ArchetypeId::EMPTY, new_archetype_id);
let archetype = &mut archetypes[new_archetype_id];
let table = &mut storages.tables[archetype.table_id()];
let add_bundle = empty_archetype.edges().get_add_bundle(self.id()).unwrap();
BundleSpawner {
archetype,
add_bundle,
bundle_info: self,
table,
entities,
Expand All @@ -355,16 +355,29 @@ impl BundleInfo {
}
}

/// This writes components from a given [`Bundle`] to the given entity.
///
/// # Safety
///
/// `bundle_component_status` must return the "correct" [`ComponentStatus`] for each component
/// in the [`Bundle`], with respect to the entity's original archetype (prior to the bundle being added)
/// For example, if the original archetype already has `ComponentA` and `T` also has `ComponentA`, the status
/// should be `Mutated`. If the original archetype does not have `ComponentA`, the status should be `Added`.
/// When "inserting" a bundle into an existing entity, [`AddBundle`](crate::archetype::AddBundle)
/// should be used, which will report `Added` vs `Mutated` status based on the current archetype's structure.
/// When spawning a bundle, [`SpawnBundleStatus`] can be used instead, which removes the need
/// to look up the [`AddBundle`](crate::archetype::AddBundle) in the archetype graph, which requires
/// ownership of the entity's current archetype.
///
/// `table` must be the "new" table for `entity`. `table_row` must have space allocated for the
/// `entity`, `bundle` must match this [`BundleInfo`]'s type
#[inline]
#[allow(clippy::too_many_arguments)]
unsafe fn write_components<T: Bundle>(
unsafe fn write_components<T: Bundle, S: BundleComponentStatus>(
&self,
table: &mut Table,
sparse_sets: &mut SparseSets,
add_bundle: &AddBundle,
bundle_component_status: &S,
entity: Entity,
table_row: usize,
change_tick: u32,
Expand All @@ -378,7 +391,8 @@ impl BundleInfo {
match self.storage_types[bundle_component] {
StorageType::Table => {
let column = table.get_column_mut(component_id).unwrap();
match add_bundle.bundle_status.get_unchecked(bundle_component) {
// SAFETY: bundle_component is a valid index for this bundle
match bundle_component_status.get_status(bundle_component) {
ComponentStatus::Added => {
column.initialize(
table_row,
Expand Down Expand Up @@ -624,7 +638,6 @@ impl<'a, 'b> BundleInserter<'a, 'b> {
pub(crate) struct BundleSpawner<'a, 'b> {
pub(crate) archetype: &'a mut Archetype,
pub(crate) entities: &'a mut Entities,
add_bundle: &'a AddBundle,
bundle_info: &'b BundleInfo,
table: &'a mut Table,
sparse_sets: &'a mut SparseSets,
Expand All @@ -649,7 +662,7 @@ impl<'a, 'b> BundleSpawner<'a, 'b> {
self.bundle_info.write_components(
self.table,
self.sparse_sets,
self.add_bundle,
&SpawnBundleStatus,
entity,
table_row,
self.change_tick,
Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_ecs/src/world/mod.rs
Expand Up @@ -1940,4 +1940,10 @@ mod tests {

assert_eq!(entity_counters.len(), 0);
}

#[test]
fn spawn_empty_bundle() {
let mut world = World::new();
world.spawn(());
}
}

0 comments on commit 2e653e5

Please sign in to comment.