diff --git a/crates/bevy_ecs/macros/src/component.rs b/crates/bevy_ecs/macros/src/component.rs index 1322022581f2f..58237e1da32bc 100644 --- a/crates/bevy_ecs/macros/src/component.rs +++ b/crates/bevy_ecs/macros/src/component.rs @@ -236,42 +236,20 @@ pub fn derive_component(input: TokenStream) -> TokenStream { .push(parse_quote! { Self: Send + Sync + 'static }); let requires = &attrs.requires; - let mut register_required = Vec::with_capacity(attrs.requires.iter().len()); - let mut register_recursive_requires = Vec::with_capacity(attrs.requires.iter().len()); + let mut register_required = Vec::with_capacity(requires.as_ref().map_or(0, Punctuated::len)); if let Some(requires) = requires { for require in requires { let ident = &require.path; - register_recursive_requires.push(quote! { - <#ident as #bevy_ecs_path::component::Component>::register_required_components( - requiree, - components, + let constructor = match &require.func { + Some(func) => quote!(|| { let x: #ident = (#func)().into(); x }), + None => quote!(<#ident as Default>::default), + }; + register_required.push(quote! { + components.register_required_components_manual::( required_components, - inheritance_depth + 1, - recursion_check_stack + #constructor, ); }); - match &require.func { - Some(func) => { - register_required.push(quote! { - components.register_required_components_manual::( - required_components, - || { let x: #ident = (#func)().into(); x }, - inheritance_depth, - recursion_check_stack - ); - }); - } - None => { - register_required.push(quote! { - components.register_required_components_manual::( - required_components, - <#ident as Default>::default, - inheritance_depth, - recursion_check_stack - ); - }); - } - } } } let struct_name = &ast.ident; @@ -304,26 +282,16 @@ pub fn derive_component(input: TokenStream) -> TokenStream { ) }; - // This puts `register_required` before `register_recursive_requires` to ensure that the constructors of _all_ top - // level components are initialized first, giving them precedence over recursively defined constructors for the same component type TokenStream::from(quote! { #required_component_docs impl #impl_generics #bevy_ecs_path::component::Component for #struct_name #type_generics #where_clause { const STORAGE_TYPE: #bevy_ecs_path::component::StorageType = #storage; type Mutability = #mutable_type; fn register_required_components( - requiree: #bevy_ecs_path::component::ComponentId, components: &mut #bevy_ecs_path::component::ComponentsRegistrator, required_components: &mut #bevy_ecs_path::component::RequiredComponents, - inheritance_depth: u16, - recursion_check_stack: &mut #bevy_ecs_path::__macro_exports::Vec<#bevy_ecs_path::component::ComponentId> ) { - #bevy_ecs_path::component::enforce_no_required_components_recursion(components, recursion_check_stack); - let self_id = components.register_component::(); - recursion_check_stack.push(self_id); #(#register_required)* - #(#register_recursive_requires)* - recursion_check_stack.pop(); } #on_add diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 8efdc60ad9345..67474b2a0532e 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -283,14 +283,7 @@ unsafe impl Bundle for C { components: &mut ComponentsRegistrator, required_components: &mut RequiredComponents, ) { - let component_id = components.register_component::(); - ::register_required_components( - component_id, - components, - required_components, - 0, - &mut Vec::new(), - ); + ::register_required_components(components, required_components); } fn get_component_ids(components: &Components, ids: &mut impl FnMut(Option)) { diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 615c5903f8fa8..17711b4d7e844 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -538,11 +538,8 @@ pub trait Component: Send + Sync + 'static { /// Registers required components. fn register_required_components( - _component_id: ComponentId, _components: &mut ComponentsRegistrator, _required_components: &mut RequiredComponents, - _inheritance_depth: u16, - _recursion_check_stack: &mut Vec, ) { } @@ -1323,7 +1320,7 @@ impl<'w> ComponentsQueuedRegistrator<'w> { // SAFETY: We just checked that this is not currently registered or queued, and if it was registered since, this would have been dropped from the queue. #[expect(unused_unsafe, reason = "More precise to specify.")] unsafe { - registrator.register_component_unchecked::(&mut Vec::new(), id); + registrator.register_component_unchecked::(id); } }, ) @@ -1442,6 +1439,7 @@ impl<'w> ComponentsQueuedRegistrator<'w> { pub struct ComponentsRegistrator<'w> { components: &'w mut Components, ids: &'w mut ComponentIds, + recursion_check_stack: Vec, } impl Deref for ComponentsRegistrator<'_> { @@ -1466,7 +1464,11 @@ impl<'w> ComponentsRegistrator<'w> { /// 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 { - Self { components, ids } + Self { + components, + ids, + recursion_check_stack: Vec::new(), + } } /// Converts this [`ComponentsRegistrator`] into a [`ComponentsQueuedRegistrator`]. @@ -1555,15 +1557,6 @@ impl<'w> ComponentsRegistrator<'w> { /// * [`ComponentsRegistrator::register_component_with_descriptor()`] #[inline] pub fn register_component(&mut self) -> ComponentId { - self.register_component_checked::(&mut Vec::new()) - } - - /// Same as [`Self::register_component_unchecked`] but keeps a checks for safety. - #[inline] - fn register_component_checked( - &mut self, - recursion_check_stack: &mut Vec, - ) -> ComponentId { let type_id = TypeId::of::(); if let Some(id) = self.indices.get(&type_id) { return *id; @@ -1585,7 +1578,7 @@ impl<'w> ComponentsRegistrator<'w> { let id = self.ids.next_mut(); // SAFETY: The component is not currently registered, and the id is fresh. unsafe { - self.register_component_unchecked::(recursion_check_stack, id); + self.register_component_unchecked::(id); } id } @@ -1594,11 +1587,7 @@ impl<'w> ComponentsRegistrator<'w> { /// /// Neither this component, nor its id may be registered or queued. This must be a new registration. #[inline] - unsafe fn register_component_unchecked( - &mut self, - recursion_check_stack: &mut Vec, - id: ComponentId, - ) { + unsafe fn register_component_unchecked(&mut self, id: ComponentId) { // SAFETY: ensured by caller. unsafe { self.register_component_inner(id, ComponentDescriptor::new::()); @@ -1607,14 +1596,11 @@ impl<'w> ComponentsRegistrator<'w> { let prev = self.indices.insert(type_id, id); debug_assert!(prev.is_none()); + self.recursion_check_stack.push(id); let mut required_components = RequiredComponents::default(); - T::register_required_components( - id, - self, - &mut required_components, - 0, - recursion_check_stack, - ); + T::register_required_components(self, &mut required_components); + self.recursion_check_stack.pop(); + // SAFETY: we just inserted it in `register_component_inner` let info = unsafe { &mut self @@ -1661,13 +1647,6 @@ impl<'w> ComponentsRegistrator<'w> { /// Registers the given component `R` and [required components] inherited from it as required by `T`, /// and adds `T` to their lists of requirees. /// - /// The given `inheritance_depth` determines how many levels of inheritance deep the requirement is. - /// A direct requirement has a depth of `0`, and each level of inheritance increases the depth by `1`. - /// Lower depths are more specific requirements, and can override existing less specific registrations. - /// - /// The `recursion_check_stack` allows checking whether this component tried to register itself as its - /// own (indirect) required component. - /// /// This method does *not* register any components as required by components that require `T`. /// /// Only use this method if you know what you are doing. In most cases, you should instead use [`World::register_required_components`], @@ -1679,11 +1658,15 @@ impl<'w> ComponentsRegistrator<'w> { &mut self, required_components: &mut RequiredComponents, constructor: fn() -> R, - inheritance_depth: u16, - recursion_check_stack: &mut Vec, ) { - let requiree = self.register_component_checked::(recursion_check_stack); - let required = self.register_component_checked::(recursion_check_stack); + let requiree = self.register_component::(); + let required = self.register_component::(); + + enforce_no_required_components_recursion( + required, + self.components, + &self.recursion_check_stack, + ); // SAFETY: We just created the components. unsafe { @@ -1692,7 +1675,6 @@ impl<'w> ComponentsRegistrator<'w> { required, required_components, constructor, - inheritance_depth, ); } } @@ -2132,10 +2114,6 @@ impl Components { /// Registers the given component `R` and [required components] inherited from it as required by `T`, /// and adds `T` to their lists of requirees. /// - /// The given `inheritance_depth` determines how many levels of inheritance deep the requirement is. - /// A direct requirement has a depth of `0`, and each level of inheritance increases the depth by `1`. - /// Lower depths are more specific requirements, and can override existing less specific registrations. - /// /// This method does *not* register any components as required by components that require `T`. /// /// [required component]: Component#required-components @@ -2149,7 +2127,6 @@ impl Components { required: ComponentId, required_components: &mut RequiredComponents, constructor: fn() -> R, - inheritance_depth: u16, ) { // Components cannot require themselves. if required == requiree { @@ -2157,7 +2134,7 @@ impl Components { } // Register the required component `R` for the requiree. - required_components.register_by_id(required, constructor, inheritance_depth); + required_components.register_by_id(required, constructor, 0); // Add the requiree to the list of components that require `R`. // SAFETY: The caller ensures that the component ID is valid. @@ -2852,37 +2829,32 @@ impl RequiredComponents { } } -// NOTE: This should maybe be private, but it is currently public so that `bevy_ecs_macros` can use it. -// This exists as a standalone function instead of being inlined into the component derive macro so as -// to reduce the amount of generated code. -#[doc(hidden)] -pub fn enforce_no_required_components_recursion( +fn enforce_no_required_components_recursion( + requiree: ComponentId, components: &Components, recursion_check_stack: &[ComponentId], ) { - if let Some((&requiree, check)) = recursion_check_stack.split_last() { - if let Some(direct_recursion) = check - .iter() - .position(|&id| id == requiree) - .map(|index| index == check.len() - 1) - { - panic!( - "Recursive required components detected: {}\nhelp: {}", - recursion_check_stack - .iter() - .map(|id| format!("{}", components.get_name(*id).unwrap().shortname())) - .collect::>() - .join(" → "), - if direct_recursion { - format!( - "Remove require({}).", - components.get_name(requiree).unwrap().shortname() - ) - } else { - "If this is intentional, consider merging the components.".into() - } - ); - } + if let Some(direct_recursion) = recursion_check_stack + .iter() + .position(|&id| id == requiree) + .map(|index| index == recursion_check_stack.len() - 1) + { + panic!( + "Recursive required components detected: {}\nhelp: {}", + recursion_check_stack + .iter() + .map(|id| format!("{}", components.get_name(*id).unwrap().shortname())) + .collect::>() + .join(" → "), + if direct_recursion { + format!( + "Remove require({}).", + components.get_name(requiree).unwrap().shortname() + ) + } else { + "If this is intentional, consider merging the components.".into() + } + ); } } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 8a07cdc8e1b92..7ee5721f3d0f0 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2479,16 +2479,16 @@ mod tests { // // Requirements with `require` attribute: // - // A -> B -> C - // 0 1 + // A -> B -> C -> D + // 0 1 2 // // Runtime requirements: // - // X -> A -> B -> C - // 0 1 2 - // - // X -> Y -> Z -> B -> C + // X -> A -> B -> C -> D // 0 1 2 3 + // + // X -> Y -> Z -> B -> C -> D + // 0 1 2 3 4 #[derive(Component, Default)] #[require(B)] @@ -2499,8 +2499,12 @@ mod tests { struct B; #[derive(Component, Default)] + #[require(D)] struct C; + #[derive(Component, Default)] + struct D; + #[derive(Component, Default)] struct X; @@ -2515,6 +2519,7 @@ mod tests { let a = world.register_component::(); let b = world.register_component::(); let c = world.register_component::(); + let d = world.register_component::(); let y = world.register_component::(); let z = world.register_component::(); @@ -2528,6 +2533,7 @@ mod tests { let required_a = world.get_required_components::().unwrap(); let required_b = world.get_required_components::().unwrap(); let required_c = world.get_required_components::().unwrap(); + let required_d = world.get_required_components::().unwrap(); let required_x = world.get_required_components::().unwrap(); let required_y = world.get_required_components::().unwrap(); let required_z = world.get_required_components::().unwrap(); @@ -2545,15 +2551,16 @@ mod tests { } // Check that the inheritance depths are correct for each component. - assert_eq!(to_vec(required_a), vec![(b, 0), (c, 1)]); - assert_eq!(to_vec(required_b), vec![(c, 0)]); - assert_eq!(to_vec(required_c), vec![]); + assert_eq!(to_vec(required_a), vec![(b, 0), (c, 1), (d, 2)]); + assert_eq!(to_vec(required_b), vec![(c, 0), (d, 1)]); + assert_eq!(to_vec(required_c), vec![(d, 0)]); + assert_eq!(to_vec(required_d), vec![]); assert_eq!( to_vec(required_x), - vec![(a, 0), (b, 1), (c, 2), (y, 0), (z, 1)] + vec![(a, 0), (b, 1), (c, 2), (d, 3), (y, 0), (z, 1)] ); - assert_eq!(to_vec(required_y), vec![(b, 1), (c, 2), (z, 0)]); - assert_eq!(to_vec(required_z), vec![(b, 0), (c, 1)]); + assert_eq!(to_vec(required_y), vec![(b, 1), (c, 2), (d, 3), (z, 0)]); + assert_eq!(to_vec(required_z), vec![(b, 0), (c, 1), (d, 2)]); } #[test] diff --git a/release-content/migration-guides/component_register_required_components.md b/release-content/migration-guides/component_register_required_components.md new file mode 100644 index 0000000000000..925673bbec657 --- /dev/null +++ b/release-content/migration-guides/component_register_required_components.md @@ -0,0 +1,6 @@ +--- +title: Refactor of Component::register_required_component +pull_requests: [19972] +--- + +The `register_required_component` method of the `Component` trait has been refactored to no longer be aware of the recursive registration, and in particular its `requiree`, `inheritance_depth` and `recursion_check_stack` parameters have been removed.