From b578134798dad904574409819f5a88bd86c1f2b2 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Fri, 1 Nov 2024 17:06:56 -0700 Subject: [PATCH] Allow components to be "explicitly required". Now, users can specify that components must be inserted (or at least present) when inserting or spawning another component. --- crates/bevy_app/src/app.rs | 25 ++++--- crates/bevy_ecs/macros/src/component.rs | 11 +++- crates/bevy_ecs/src/bundle.rs | 45 ++++++++++++- crates/bevy_ecs/src/component.rs | 87 +++++++++++++------------ crates/bevy_ecs/src/lib.rs | 56 ++++++++++++++-- crates/bevy_ecs/src/world/mod.rs | 41 +++++++----- 6 files changed, 188 insertions(+), 77 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 4cdc3a2473d55..255e75f41d5b3 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -849,18 +849,23 @@ impl App { /// #[derive(Component, Default, PartialEq, Eq, Debug)] /// struct C(u32); /// + /// #[derive(Component, Default, PartialEq, Eq, Debug)] + /// struct D(u32); + /// /// # let mut app = App::new(); /// # app.add_plugins(MinimalPlugins).add_systems(Startup, setup); /// // Register B and C as required by A and C as required by B. /// // A requiring C directly will overwrite the indirect requirement through B. /// app.register_required_components::(); - /// app.register_required_components_with::(|| C(1)); - /// app.register_required_components_with::(|| C(2)); + /// app.register_required_components_with::(Some(|| C(1))); + /// app.register_required_components_with::(Some(|| C(2))); + /// // Register D as explicitly required by B. + /// app.register_required_components_with::(None); /// /// fn setup(mut commands: Commands) { - /// // This will implicitly also insert B with its Default constructor and C - /// // with the custom constructor defined by A. - /// commands.spawn(A); + /// // This will implicitly also insert B with its Default constructor and C with the custom + /// // constructor defined by A. D must also be inserted since B explicitly requires it. + /// commands.spawn((A, D(5))); /// } /// /// fn validate(query: Option>) { @@ -872,7 +877,7 @@ impl App { /// ``` pub fn register_required_components_with( &mut self, - constructor: fn() -> R, + constructor: Option R>, ) -> &mut Self { self.world_mut() .register_required_components_with::(constructor); @@ -981,11 +986,11 @@ impl App { /// // Register B and C as required by A and C as required by B. /// // A requiring C directly will overwrite the indirect requirement through B. /// app.register_required_components::(); - /// app.register_required_components_with::(|| C(1)); - /// app.register_required_components_with::(|| C(2)); + /// app.register_required_components_with::(Some(|| C(1))); + /// app.register_required_components_with::(Some(|| C(2))); /// /// // Duplicate registration! Even if the constructors were different, this would fail. - /// assert!(app.try_register_required_components_with::(|| C(1)).is_err()); + /// assert!(app.try_register_required_components_with::(Some(|| C(1))).is_err()); /// /// fn setup(mut commands: Commands) { /// // This will implicitly also insert B with its Default constructor and C @@ -1002,7 +1007,7 @@ impl App { /// ``` pub fn try_register_required_components_with( &mut self, - constructor: fn() -> R, + constructor: Option R>, ) -> Result<(), RequiredComponentsError> { self.world_mut() .try_register_required_components_with::(constructor) diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index 08b4e73056635..13950d35405a8 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -92,11 +92,16 @@ pub fn derive_component(input: TokenStream) -> TokenStream { }); match &require.func { Some(RequireFunc::Path(func)) => { + let constructor = if func.is_ident("explicit") { + quote! { None } + } else { + quote! {Some(|| { let x: #ident = #func().into(); x })} + }; register_required.push(quote! { components.register_required_components_manual::( storages, required_components, - || { let x: #ident = #func().into(); x }, + #constructor, inheritance_depth ); }); @@ -106,7 +111,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream { components.register_required_components_manual::( storages, required_components, - || { let x: #ident = (#func)().into(); x }, + Some(|| { let x: #ident = (#func)().into(); x }), inheritance_depth ); }); @@ -116,7 +121,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream { components.register_required_components_manual::( storages, required_components, - <#ident as Default>::default, + Some(<#ident as Default>::default), inheritance_depth ); }); diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 384b7517c77b2..7a82fcfe44888 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -356,7 +356,7 @@ pub struct BundleInfo { /// and the range (0..`explicit_components_len`) must be in the same order as the source bundle /// type writes its components in. component_ids: Vec, - required_components: Vec, + required_components: Vec>, explicit_components_len: usize, } @@ -648,6 +648,35 @@ impl BundleInfo { if let Some(add_bundle_id) = archetypes[archetype_id].edges().get_add_bundle(self.id) { return add_bundle_id; } + + fn find_missing_explicit_required_components( + bundle_info: &BundleInfo, + archetype: &Archetype, + ) -> Vec { + bundle_info.component_ids[bundle_info.explicit_components_len..] + .iter() + .copied() + .enumerate() + .filter(|&(i, _)| bundle_info.required_components[i].is_none()) + .map(|(_, component)| component) + .filter(|&component| !archetype.contains(component)) + .collect() + } + + let missing_required_components = + find_missing_explicit_required_components(self, &archetypes[archetype_id]); + if !missing_required_components.is_empty() { + let names = missing_required_components + .into_iter() + .map(|id| { + // SAFETY: the caller ensures component_id is valid. + unsafe { components.get_info_unchecked(id).name() } + }) + .collect::>() + .join(", "); + panic!("Bundle is missing explicitly required components: {names}"); + } + let mut new_table_components = Vec::new(); let mut new_sparse_set_components = Vec::new(); let mut bundle_status = Vec::with_capacity(self.explicit_components_len); @@ -674,7 +703,12 @@ impl BundleInfo { for (index, component_id) in self.iter_required_components().enumerate() { if !current_archetype.contains(component_id) { - added_required_components.push(self.required_components[index].clone()); + added_required_components.push( + self.required_components[index] + .as_ref() + .expect("there are no explicitly required components missing from the archetype, but we are still trying to initialize this explicitly required component") + .clone(), + ); added.push(component_id); // SAFETY: component_id exists let component_info = unsafe { components.get_info_unchecked(component_id) }; @@ -1205,7 +1239,12 @@ impl<'w> BundleSpawner<'w> { table, sparse_sets, &SpawnBundleStatus, - bundle_info.required_components.iter(), + bundle_info + .required_components + .iter() + // We already checked whether the archetype contains the explicitly required + // components, so filter them out. + .filter_map(|constructor| constructor.as_ref()), entity, table_row, self.change_tick, diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index c6a1abe75bca1..226892719ca30 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -253,14 +253,19 @@ use derive_more::derive::{Display, Error}; /// #[derive(Component, PartialEq, Eq, Debug)] /// struct C(u32); /// +/// #[derive(Component, PartialEq, Eq, Debug)] +/// struct D(u32); +/// /// # let mut world = World::default(); /// // Register B as required by A and C as required by B. /// world.register_required_components::(); -/// world.register_required_components_with::(|| C(2)); +/// world.register_required_components_with::(Some(|| C(2))); +/// // Register D as explicitly required by B. +/// world.register_required_components_with::(None); /// -/// // This will implicitly also insert B with its Default constructor -/// // and C with the custom constructor defined by B. -/// let id = world.spawn(A).id(); +/// // This will implicitly also insert B with its Default constructor and C with the custom +/// // constructor defined by B. D must also be inserted since B explicitly requires it. +/// let id = world.spawn((A, D(5))).id(); /// assert_eq!(&B(0), world.entity(id).get::().unwrap()); /// assert_eq!(&C(2), world.entity(id).get::().unwrap()); /// ``` @@ -1031,7 +1036,7 @@ impl Components { &mut self, required: ComponentId, requiree: ComponentId, - constructor: fn() -> R, + constructor: Option R>, ) -> Result<(), RequiredComponentsError> { // SAFETY: The caller ensures that the `requiree` is valid. let required_components = unsafe { @@ -1178,7 +1183,7 @@ impl Components { &mut self, storages: &mut Storages, required_components: &mut RequiredComponents, - constructor: fn() -> R, + constructor: Option R>, inheritance_depth: u16, ) { let requiree = self.register_component::(storages); @@ -1216,7 +1221,7 @@ impl Components { requiree: ComponentId, required: ComponentId, required_components: &mut RequiredComponents, - constructor: fn() -> R, + constructor: Option R>, inheritance_depth: u16, ) { // Components cannot require themselves. @@ -1687,7 +1692,7 @@ impl RequiredComponentConstructor { #[derive(Clone)] pub struct RequiredComponent { /// The constructor used for the required component. - pub constructor: RequiredComponentConstructor, + pub constructor: Option, /// The depth of the component requirement in the requirement hierarchy for this component. /// This is used for determining which constructor is used in cases where there are duplicate requires. @@ -1731,7 +1736,7 @@ impl RequiredComponents { pub unsafe fn register_dynamic( &mut self, component_id: ComponentId, - constructor: RequiredComponentConstructor, + constructor: Option, inheritance_depth: u16, ) { self.0 @@ -1757,7 +1762,7 @@ impl RequiredComponents { &mut self, components: &mut Components, storages: &mut Storages, - constructor: fn() -> C, + constructor: Option C>, inheritance_depth: u16, ) { let component_id = components.register_component::(storages); @@ -1771,38 +1776,40 @@ impl RequiredComponents { pub fn register_by_id( &mut self, component_id: ComponentId, - constructor: fn() -> C, + constructor: Option C>, inheritance_depth: u16, ) { - let erased: RequiredComponentConstructor = RequiredComponentConstructor(Arc::new( - move |table, - sparse_sets, - change_tick, - table_row, - entity, - #[cfg(feature = "track_change_detection")] caller| { - OwningPtr::make(constructor(), |ptr| { - // SAFETY: This will only be called in the context of `BundleInfo::write_components`, which will - // pass in a valid table_row and entity requiring a C constructor - // C::STORAGE_TYPE is the storage type associated with `component_id` / `C` - // `ptr` points to valid `C` data, which matches the type associated with `component_id` - unsafe { - BundleInfo::initialize_required_component( - table, - sparse_sets, - change_tick, - table_row, - entity, - component_id, - C::STORAGE_TYPE, - ptr, - #[cfg(feature = "track_change_detection")] - caller, - ); - } - }); - }, - )); + let erased = constructor.map(|constructor| { + RequiredComponentConstructor(Arc::new( + move |table, + sparse_sets, + change_tick, + table_row, + entity, + #[cfg(feature = "track_change_detection")] caller| { + OwningPtr::make(constructor(), |ptr| { + // SAFETY: This will only be called in the context of `BundleInfo::write_components`, which will + // pass in a valid table_row and entity requiring a C constructor + // C::STORAGE_TYPE is the storage type associated with `component_id` / `C` + // `ptr` points to valid `C` data, which matches the type associated with `component_id` + unsafe { + BundleInfo::initialize_required_component( + table, + sparse_sets, + change_tick, + table_row, + entity, + component_id, + C::STORAGE_TYPE, + ptr, + #[cfg(feature = "track_change_detection")] + caller, + ); + } + }); + }, + )) + }); // SAFETY: // `component_id` matches the type initialized by the `erased` constructor above. // `erased` initializes a component for `component_id` in such a way that diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 67254d6298f11..39317cfd84a60 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2214,7 +2214,7 @@ mod tests { let mut world = World::new(); world.register_required_components::(); - world.register_required_components_with::(|| Z(7)); + world.register_required_components_with::(Some(|| Z(7))); let id = world.spawn(X).id(); @@ -2278,8 +2278,8 @@ mod tests { // - Y requires Z with custom constructor // - X requires Z with custom constructor (more specific than X -> Y -> Z) world.register_required_components::(); - world.register_required_components_with::(|| Z(5)); - world.register_required_components_with::(|| Z(7)); + world.register_required_components_with::(Some(|| Z(5))); + world.register_required_components_with::(Some(|| Z(7))); let id = world.spawn(X).id(); @@ -2309,8 +2309,8 @@ mod tests { // - X requires Z with custom constructor (more specific than X -> Y -> Z) // - Y requires Z with custom constructor world.register_required_components::(); - world.register_required_components_with::(|| Z(7)); - world.register_required_components_with::(|| Z(5)); + world.register_required_components_with::(Some(|| Z(7))); + world.register_required_components_with::(Some(|| Z(5))); let id = world.spawn(X).id(); @@ -2358,6 +2358,52 @@ mod tests { )); } + #[test] + fn runtime_explicitly_required_components_succeeds_on_spawn() { + #[derive(Component)] + #[require(Y(explicit))] + struct X; + + #[derive(Component, Default)] + struct Y; + + let mut world = World::new(); + // No panic since we spawned both components. + world.spawn((X, Y)); + } + + #[test] + fn runtime_explicitly_required_components_succeeds_on_insert() { + #[derive(Component)] + #[require(Y(explicit))] + struct X; + + #[derive(Component, Default)] + struct Y; + + let mut world = World::new(); + // No panic since we inserted both components. + world.spawn(()).insert((X, Y)); + // No panic since we inserted Y, then X. + world.spawn(Y).insert(X); + } + + #[test] + #[should_panic( + expected = "Bundle is missing explicitly required components: bevy_ecs::tests::runtime_explicitly_required_components_panics_when_missing::Y" + )] + fn runtime_explicitly_required_components_panics_when_missing() { + #[derive(Component)] + #[require(Y(explicit))] + struct X; + + #[derive(Component, Default)] + struct Y; + + let mut world = World::new(); + world.spawn(X); + } + #[test] fn required_components_inheritance_depth() { // Test that inheritance depths are computed correctly for requirements. diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d50f8f421cfc2..1abf3b96a1182 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -344,8 +344,10 @@ impl World { /// Registers the given component `R` as a [required component] for `T`. /// /// When `T` is added to an entity, `R` and its own required components will also be added - /// if `R` was not already provided. The given `constructor` will be used for the creation of `R`. - /// If a [`Default`] constructor is desired, use [`World::register_required_components`] instead. + /// if `R` was not already provided. If `constructor` is [`Some`], it will be used for the + /// creation of `R`. Otherwise, the component will be considered "explicitly required" - the + /// component must be present on an entity or inserted in the same bundle. If a [`Default`] + /// constructor is desired, use [`World::register_required_components`] instead. /// /// For the non-panicking version, see [`World::try_register_required_components_with`]. /// @@ -375,22 +377,27 @@ impl World { /// #[derive(Component, PartialEq, Eq, Debug)] /// struct C(u32); /// + /// #[derive(Component, PartialEq, Eq, Debug)] + /// struct D(u32); + /// /// # let mut world = World::default(); /// // Register B and C as required by A and C as required by B. /// // A requiring C directly will overwrite the indirect requirement through B. /// world.register_required_components::(); - /// world.register_required_components_with::(|| C(1)); - /// world.register_required_components_with::(|| C(2)); - /// - /// // This will implicitly also insert B with its Default constructor and C - /// // with the custom constructor defined by A. - /// let id = world.spawn(A).id(); + /// world.register_required_components_with::(Some(|| C(1))); + /// world.register_required_components_with::(Some(|| C(2))); + /// // Register D as explicitly required by B. + /// world.register_required_components_with::(None); + /// + /// // This will implicitly also insert B with its Default constructor and C with the custom + /// // constructor defined by A. D must also be inserted since B explicitly requires it. + /// let id = world.spawn((A, D(5))).id(); /// assert_eq!(&B(0), world.entity(id).get::().unwrap()); /// assert_eq!(&C(2), world.entity(id).get::().unwrap()); /// ``` pub fn register_required_components_with( &mut self, - constructor: fn() -> R, + constructor: Option R>, ) { self.try_register_required_components_with::(constructor) .unwrap(); @@ -446,14 +453,16 @@ impl World { pub fn try_register_required_components( &mut self, ) -> Result<(), RequiredComponentsError> { - self.try_register_required_components_with::(R::default) + self.try_register_required_components_with::(Some(R::default)) } /// Tries to register the given component `R` as a [required component] for `T`. /// /// When `T` is added to an entity, `R` and its own required components will also be added - /// if `R` was not already provided. The given `constructor` will be used for the creation of `R`. - /// If a [`Default`] constructor is desired, use [`World::register_required_components`] instead. + /// if `R` was not already provided. If `constructor` is [`Some`], it will be used for the + /// creation of `R`. Otherwise, the component will be considered "explicitly required" - the + /// component must be present on an entity or inserted in the same bundle. If a [`Default`] + /// constructor is desired, use [`World::register_required_components`] instead. /// /// For the panicking version, see [`World::register_required_components_with`]. /// @@ -487,11 +496,11 @@ impl World { /// // Register B and C as required by A and C as required by B. /// // A requiring C directly will overwrite the indirect requirement through B. /// world.register_required_components::(); - /// world.register_required_components_with::(|| C(1)); - /// world.register_required_components_with::(|| C(2)); + /// world.register_required_components_with::(Some(|| C(1))); + /// world.register_required_components_with::(Some(|| C(2))); /// /// // Duplicate registration! Even if the constructors were different, this would fail. - /// assert!(world.try_register_required_components_with::(|| C(1)).is_err()); + /// assert!(world.try_register_required_components_with::(Some(|| C(1))).is_err()); /// /// // This will implicitly also insert B with its Default constructor and C /// // with the custom constructor defined by A. @@ -501,7 +510,7 @@ impl World { /// ``` pub fn try_register_required_components_with( &mut self, - constructor: fn() -> R, + constructor: Option R>, ) -> Result<(), RequiredComponentsError> { let requiree = self.register_component::();