Merge a5c392180b into 877d278785

crates/bevy_ecs/macros/src/component.rs

@@ -238,41 +238,16 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
 
     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());
     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,
-                    required_components,
-                    inheritance_depth + 1,
-                    recursion_check_stack
-                );
+            let constructor = match &require.func {
+                Some(func) => quote! { || { let x: #ident = (#func)().into(); x } },
+                None => quote! { <#ident as Default>::default },
+            };
+            register_required.push(quote! {
+                required_components.register_required::<#ident>(#constructor);
             });
-            match &require.func {
-                Some(func) => {
-                    register_required.push(quote! {
-                        components.register_required_components_manual::<Self, #ident>(
-                            required_components,
-                            || { let x: #ident = (#func)().into(); x },
-                            inheritance_depth,
-                            recursion_check_stack
-                        );
-                    });
-                }
-                None => {
-                    register_required.push(quote! {
-                        components.register_required_components_manual::<Self, #ident>(
-                            required_components,
-                            <#ident as Default>::default,
-                            inheritance_depth,
-                            recursion_check_stack
-                        );
-                    });
-                }
-            }
         }
     }
     let struct_name = &ast.ident;
@@ -319,18 +294,10 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
             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>
+                _requiree: #bevy_ecs_path::component::ComponentId,
+                required_components: &mut #bevy_ecs_path::component::RequiredComponentsRegistrator,
             ) {
-                #bevy_ecs_path::component::enforce_no_required_components_recursion(components, recursion_check_stack);
-                let self_id = components.register_component::<Self>();
-                recursion_check_stack.push(self_id);
                 #(#register_required)*
-                #(#register_recursive_requires)*
-                recursion_check_stack.pop();
             }
 
             #on_add

crates/bevy_ecs/src/bundle/info.rs

@@ -1,16 +1,20 @@
 use alloc::{boxed::Box, vec, vec::Vec};
-use bevy_platform::collections::{HashMap, HashSet};
+use bevy_platform::{
+    collections::{HashMap, HashSet},
+    hash::FixedHasher,
+};
 use bevy_ptr::OwningPtr;
 use bevy_utils::TypeIdMap;
 use core::{any::TypeId, ptr::NonNull};
+use indexmap::{IndexMap, IndexSet};
 
 use crate::{
     archetype::{Archetype, BundleComponentStatus, ComponentStatus},
     bundle::{Bundle, DynamicBundle},
     change_detection::MaybeLocation,
     component::{
-        ComponentId, Components, ComponentsRegistrator, RequiredComponentConstructor,
-        RequiredComponents, StorageType, Tick,
+        ComponentId, Components, ComponentsRegistrator, RequiredComponentConstructor, StorageType,
+        Tick,
     },
     entity::Entity,
     query::DebugCheckedUnwrap as _,
@@ -59,6 +63,7 @@ pub enum InsertMode {
 /// [`World`]: crate::world::World
 pub struct BundleInfo {
     pub(super) id: BundleId,
+
     /// The list of all components contributed by the bundle (including Required Components). This is in
     /// the order `[EXPLICIT_COMPONENTS][REQUIRED_COMPONENTS]`
     ///
@@ -67,9 +72,10 @@ pub struct BundleInfo {
     /// must have its storage initialized (i.e. columns created in tables, sparse set created),
     /// and the range (0..`explicit_components_len`) must be in the same order as the source bundle
     /// type writes its components in.
-    pub(super) component_ids: Vec<ComponentId>,
-    pub(super) required_components: Vec<RequiredComponentConstructor>,
-    pub(super) explicit_components_len: usize,
+    pub(super) contributed_components: Vec<ComponentId>,
+
+    /// The list of constructors for all required components indirectly contributed by this bundle.
+    pub(super) required_component_constructors: Vec<RequiredComponentConstructor>,
 }
 
 impl BundleInfo {
@@ -86,11 +92,13 @@ impl BundleInfo {
         mut component_ids: Vec<ComponentId>,
         id: BundleId,
     ) -> BundleInfo {
+        let explicit_component_ids = component_ids
+            .iter()
+            .copied()
+            .collect::<IndexSet<_, FixedHasher>>();
+
         // check for duplicates
-        let mut deduped = component_ids.clone();
-        deduped.sort_unstable();
-        deduped.dedup();
-        if deduped.len() != component_ids.len() {
+        if explicit_component_ids.len() != component_ids.len() {
             // TODO: Replace with `Vec::partition_dedup` once https://github.com/rust-lang/rust/issues/54279 is stabilized
             let mut seen = <HashSet<_>>::default();
             let mut dups = Vec::new();
@@ -111,31 +119,30 @@ impl BundleInfo {
             panic!("Bundle {bundle_type_name} has duplicate components: {names:?}");
         }
 
-        // handle explicit components
-        let explicit_components_len = component_ids.len();
-        let mut required_components = RequiredComponents::default();
-        for component_id in component_ids.iter().copied() {
+        let mut depth_first_components = IndexMap::<_, _, FixedHasher>::default();
+        for &component_id in &component_ids {
             // SAFETY: caller has verified that all ids are valid
             let info = unsafe { components.get_info_unchecked(component_id) };
-            required_components.merge(info.required_components());
+
+            for (&required_id, required_component) in &info.required_components().all {
+                depth_first_components
+                    .entry(required_id)
+                    .or_insert_with(|| required_component.clone());
+            }
+
             storages.prepare_component(info);
         }
-        required_components.remove_explicit_components(&component_ids);
 
-        // handle required components
-        let required_components = required_components
-            .0
-            .into_iter()
-            .map(|(component_id, v)| {
-                // Safety: These ids came out of the passed `components`, so they must be valid.
-                let info = unsafe { components.get_info_unchecked(component_id) };
-                storages.prepare_component(info);
-                // This adds required components to the component_ids list _after_ using that list to remove explicitly provided
-                // components. This ordering is important!
-                component_ids.push(component_id);
-                v.constructor
+        let required_components = depth_first_components
+            .iter()
+            .filter(|&(required_id, _)| !explicit_component_ids.contains(required_id))
+            .inspect(|&(&required_id, _)| {
+                // SAFETY: These ids came out of the passed `components`, so they must be valid.
+                storages.prepare_component(unsafe { components.get_info_unchecked(required_id) });
+                component_ids.push(required_id);
             })
-            .collect();
+            .map(|(_, required_component)| required_component.constructor.clone())
+            .collect::<Vec<_>>();
 
         // SAFETY: The caller ensures that component_ids:
         // - is valid for the associated world
@@ -143,9 +150,8 @@ impl BundleInfo {
         // - is in the same order as the source bundle type
         BundleInfo {
             id,
-            component_ids,
-            required_components,
-            explicit_components_len,
+            contributed_components: component_ids,
+            required_component_constructors: required_components,
         }
     }
 
@@ -155,19 +161,24 @@ impl BundleInfo {
         self.id
     }
 
+    /// Returns the length of the explicit components part of the [`contributed_components`](Self::contributed_components) list.
+    pub(super) fn explicit_components_len(&self) -> usize {
+        self.contributed_components.len() - self.required_component_constructors.len()
+    }
+
     /// Returns the [ID](ComponentId) of each component explicitly defined in this bundle (ex: Required Components are excluded).
     ///
     /// For all components contributed by this bundle (including Required Components), see [`BundleInfo::contributed_components`]
     #[inline]
     pub fn explicit_components(&self) -> &[ComponentId] {
-        &self.component_ids[0..self.explicit_components_len]
+        &self.contributed_components[0..self.explicit_components_len()]
     }
 
     /// Returns the [ID](ComponentId) of each Required Component needed by this bundle. This _does not include_ Required Components that are
     /// explicitly provided by the bundle.
     #[inline]
     pub fn required_components(&self) -> &[ComponentId] {
-        &self.component_ids[self.explicit_components_len..]
+        &self.contributed_components[self.explicit_components_len()..]
     }
 
     /// Returns the [ID](ComponentId) of each component contributed by this bundle. This includes Required Components.
@@ -175,7 +186,7 @@ impl BundleInfo {
     /// For only components explicitly defined in this bundle, see [`BundleInfo::explicit_components`]
     #[inline]
     pub fn contributed_components(&self) -> &[ComponentId] {
-        &self.component_ids
+        &self.contributed_components
     }
 
     /// Returns an iterator over the [ID](ComponentId) of each component explicitly defined in this bundle (ex: this excludes Required Components).
@@ -190,7 +201,7 @@ impl BundleInfo {
     /// To iterate only components explicitly defined in this bundle, see [`BundleInfo::iter_explicit_components`]
     #[inline]
     pub fn iter_contributed_components(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
-        self.component_ids.iter().copied()
+        self.contributed_components().iter().copied()
     }
 
     /// Returns an iterator over the [ID](ComponentId) of each Required Component needed by this bundle. This _does not include_ Required Components that are
@@ -236,7 +247,7 @@ impl BundleInfo {
         // bundle_info.component_ids are also in "bundle order"
         let mut bundle_component = 0;
         let after_effect = bundle.get_components(&mut |storage_type, component_ptr| {
-            let component_id = *self.component_ids.get_unchecked(bundle_component);
+            let component_id = *self.contributed_components.get_unchecked(bundle_component);
             // SAFETY: bundle_component is a valid index for this bundle
             let status = unsafe { bundle_component_status.get_status(bundle_component) };
             match storage_type {

crates/bevy_ecs/src/bundle/insert.rs

@@ -433,7 +433,7 @@ impl BundleInfo {
         }
         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);
+        let mut bundle_status = Vec::with_capacity(self.explicit_components_len());
         let mut added_required_components = Vec::new();
         let mut added = Vec::new();
         let mut existing = Vec::new();
@@ -457,7 +457,7 @@ 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_component_constructors[index].clone());
                 added.push(component_id);
                 // SAFETY: component_id exists
                 let component_info = unsafe { components.get_info_unchecked(component_id) };

crates/bevy_ecs/src/bundle/spawner.rs

@@ -108,7 +108,7 @@ impl<'w> BundleSpawner<'w> {
                 table,
                 sparse_sets,
                 &SpawnBundleStatus,
-                bundle_info.required_components.iter(),
+                bundle_info.required_component_constructors.iter(),
                 entity,
                 table_row,
                 self.change_tick,

crates/bevy_ecs/src/component/info.rs

@@ -1,5 +1,5 @@
 use alloc::{borrow::Cow, vec::Vec};
-use bevy_platform::{collections::HashSet, sync::PoisonError};
+use bevy_platform::{hash::FixedHasher, sync::PoisonError};
 use bevy_ptr::OwningPtr;
 #[cfg(feature = "bevy_reflect")]
 use bevy_reflect::Reflect;
@@ -10,6 +10,7 @@ use core::{
     fmt::Debug,
     mem::needs_drop,
 };
+use indexmap::IndexSet;
 
 use crate::{
     archetype::ArchetypeFlags,
@@ -30,7 +31,10 @@ pub struct ComponentInfo {
     pub(super) descriptor: ComponentDescriptor,
     pub(super) hooks: ComponentHooks,
     pub(super) required_components: RequiredComponents,
-    pub(super) required_by: HashSet<ComponentId>,
+    /// The set of components that require this components.
+    /// Invariant: this is stored in a depth-first order, that is components are stored after the components
+    /// that they depend on.
+    pub(super) required_by: IndexSet<ComponentId, FixedHasher>,
 }
 
 impl ComponentInfo {
@@ -505,6 +509,13 @@ impl Components {
             .and_then(|info| info.as_mut().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))
+    }
+
     #[inline]
     pub(crate) fn get_required_components_mut(
         &mut self,
@@ -516,7 +527,10 @@ impl Components {
     }
 
     #[inline]
-    pub(crate) fn get_required_by(&self, id: ComponentId) -> Option<&HashSet<ComponentId>> {
+    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))
@@ -526,7 +540,7 @@ impl Components {
     pub(crate) fn get_required_by_mut(
         &mut self,
         id: ComponentId,
-    ) -> Option<&mut HashSet<ComponentId>> {
+    ) -> Option<&mut IndexSet<ComponentId, FixedHasher>> {
         self.components
             .get_mut(id.0)
             .and_then(|info| info.as_mut().map(|info| &mut info.required_by))

crates/bevy_ecs/src/component/mod.rs

@@ -18,7 +18,6 @@ use crate::{
     system::{Local, SystemParam},
     world::{FromWorld, World},
 };
-use alloc::vec::Vec;
 pub use bevy_ecs_macros::Component;
 use core::{fmt::Debug, marker::PhantomData, ops::Deref};
 
@@ -523,12 +522,13 @@ pub trait Component: Send + Sync + 'static {
     }
 
     /// Registers required components.
+    ///
+    /// # Safety
+    ///
+    /// - `_required_components` must only contain components valid in `_components`.
     fn register_required_components(
         _component_id: ComponentId,
-        _components: &mut ComponentsRegistrator,
-        _required_components: &mut RequiredComponents,
-        _inheritance_depth: u16,
-        _recursion_check_stack: &mut Vec<ComponentId>,
+        _required_components: &mut RequiredComponentsRegistrator,
     ) {
     }
 

crates/bevy_ecs/src/component/register.rs

@@ -5,6 +5,7 @@ use core::any::Any;
 use core::ops::DerefMut;
 use core::{any::TypeId, fmt::Debug, ops::Deref};
 
+use crate::component::{enforce_no_required_components_recursion, RequiredComponentsRegistrator};
 use crate::query::DebugCheckedUnwrap as _;
 use crate::{
     component::{
@@ -64,6 +65,7 @@ impl ComponentIds {
 pub struct ComponentsRegistrator<'w> {
     components: &'w mut Components,
     ids: &'w mut ComponentIds,
+    pub(super) recursion_check_stack: Vec<ComponentId>,
 }
 
 impl Deref for ComponentsRegistrator<'_> {
@@ -88,7 +90,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`].
@@ -177,18 +183,16 @@ impl<'w> ComponentsRegistrator<'w> {
     /// * [`ComponentsRegistrator::register_component_with_descriptor()`]
     #[inline]
     pub fn register_component<T: Component>(&mut self) -> ComponentId {
-        self.register_component_checked::<T>(&mut Vec::new())
+        self.register_component_checked::<T>()
     }
 
     /// Same as [`Self::register_component_unchecked`] but keeps a checks for safety.
     #[inline]
-    pub(super) fn register_component_checked<T: Component>(
-        &mut self,
-        recursion_check_stack: &mut Vec<ComponentId>,
-    ) -> ComponentId {
+    pub(super) fn register_component_checked<T: Component>(&mut self) -> ComponentId {
         let type_id = TypeId::of::<T>();
-        if let Some(id) = self.indices.get(&type_id) {
-            return *id;
+        if let Some(&id) = self.indices.get(&type_id) {
+            enforce_no_required_components_recursion(self, &self.recursion_check_stack, id);
+            return id;
         }
 
         if let Some(registrator) = self
@@ -207,7 +211,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::<T>(recursion_check_stack, id);
+            self.register_component_unchecked::<T>(id);
         }
         id
     }
@@ -216,11 +220,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<T: Component>(
-        &mut self,
-        recursion_check_stack: &mut Vec<ComponentId>,
-        id: ComponentId,
-    ) {
+    unsafe fn register_component_unchecked<T: Component>(&mut self, id: ComponentId) {
         // SAFETY: ensured by caller.
         unsafe {
             self.register_component_inner(id, ComponentDescriptor::new::<T>());
@@ -229,14 +229,18 @@ 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,
-        );
+        // SAFETY: `required_components` is empty
+        let mut required_components_registrator =
+            unsafe { RequiredComponentsRegistrator::new(self, &mut required_components) };
+        T::register_required_components(id, &mut required_components_registrator);
+        // SAFETY:
+        // - `id` was just registered in `self`
+        // - RequiredComponentsRegistrator guarantees that only components from `self` are included in `required_components`.
+        unsafe { self.register_required_by(id, &required_components) };
+        self.recursion_check_stack.pop();
+
         // SAFETY: we just inserted it in `register_component_inner`
         let info = unsafe {
             &mut self
@@ -563,7 +567,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::<T>(&mut Vec::new(), id);
+                            registrator.register_component_unchecked::<T>(id);
                         }
                     },
                 )

crates/bevy_ecs/src/lib.rs

@@ -148,7 +148,7 @@ mod tests {
     use crate::{
         bundle::Bundle,
         change_detection::Ref,
-        component::{Component, ComponentId, RequiredComponents, RequiredComponentsError},
+        component::{Component, ComponentId},
         entity::{Entity, EntityMapper},
         entity_disabling::DefaultQueryFilters,
         prelude::Or,
@@ -156,12 +156,7 @@ mod tests {
         resource::Resource,
         world::{EntityMut, EntityRef, Mut, World},
     };
-    use alloc::{
-        string::{String, ToString},
-        sync::Arc,
-        vec,
-        vec::Vec,
-    };
+    use alloc::{string::String, sync::Arc, vec, vec::Vec};
     use bevy_platform::collections::HashSet;
     use bevy_tasks::{ComputeTaskPool, TaskPool};
     use core::{
@@ -1830,791 +1825,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn required_components() {
-        #[derive(Component)]
-        #[require(Y)]
-        struct X;
-
-        #[derive(Component)]
-        #[require(Z = new_z())]
-        struct Y {
-            value: String,
-        }
-
-        #[derive(Component)]
-        struct Z(u32);
-
-        impl Default for Y {
-            fn default() -> Self {
-                Self {
-                    value: "hello".to_string(),
-                }
-            }
-        }
-
-        fn new_z() -> Z {
-            Z(7)
-        }
-
-        let mut world = World::new();
-        let id = world.spawn(X).id();
-        assert_eq!(
-            "hello",
-            world.entity(id).get::<Y>().unwrap().value,
-            "Y should have the default value"
-        );
-        assert_eq!(
-            7,
-            world.entity(id).get::<Z>().unwrap().0,
-            "Z should have the value provided by the constructor defined in Y"
-        );
-
-        let id = world
-            .spawn((
-                X,
-                Y {
-                    value: "foo".to_string(),
-                },
-            ))
-            .id();
-        assert_eq!(
-            "foo",
-            world.entity(id).get::<Y>().unwrap().value,
-            "Y should have the manually provided value"
-        );
-        assert_eq!(
-            7,
-            world.entity(id).get::<Z>().unwrap().0,
-            "Z should have the value provided by the constructor defined in Y"
-        );
-
-        let id = world.spawn((X, Z(8))).id();
-        assert_eq!(
-            "hello",
-            world.entity(id).get::<Y>().unwrap().value,
-            "Y should have the default value"
-        );
-        assert_eq!(
-            8,
-            world.entity(id).get::<Z>().unwrap().0,
-            "Z should have the manually provided value"
-        );
-    }
-
-    #[test]
-    fn generic_required_components() {
-        #[derive(Component)]
-        #[require(Y<usize>)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y<T> {
-            value: T,
-        }
-
-        let mut world = World::new();
-        let id = world.spawn(X).id();
-        assert_eq!(
-            0,
-            world.entity(id).get::<Y<usize>>().unwrap().value,
-            "Y should have the default value"
-        );
-    }
-
-    #[test]
-    fn required_components_spawn_nonexistent_hooks() {
-        #[derive(Component)]
-        #[require(Y)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        #[derive(Resource)]
-        struct A(usize);
-
-        #[derive(Resource)]
-        struct I(usize);
-
-        let mut world = World::new();
-        world.insert_resource(A(0));
-        world.insert_resource(I(0));
-        world
-            .register_component_hooks::<Y>()
-            .on_add(|mut world, _| world.resource_mut::<A>().0 += 1)
-            .on_insert(|mut world, _| world.resource_mut::<I>().0 += 1);
-
-        // Spawn entity and ensure Y was added
-        assert!(world.spawn(X).contains::<Y>());
-
-        assert_eq!(world.resource::<A>().0, 1);
-        assert_eq!(world.resource::<I>().0, 1);
-    }
-
-    #[test]
-    fn required_components_insert_existing_hooks() {
-        #[derive(Component)]
-        #[require(Y)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        #[derive(Resource)]
-        struct A(usize);
-
-        #[derive(Resource)]
-        struct I(usize);
-
-        let mut world = World::new();
-        world.insert_resource(A(0));
-        world.insert_resource(I(0));
-        world
-            .register_component_hooks::<Y>()
-            .on_add(|mut world, _| world.resource_mut::<A>().0 += 1)
-            .on_insert(|mut world, _| world.resource_mut::<I>().0 += 1);
-
-        // Spawn entity and ensure Y was added
-        assert!(world.spawn_empty().insert(X).contains::<Y>());
-
-        assert_eq!(world.resource::<A>().0, 1);
-        assert_eq!(world.resource::<I>().0, 1);
-    }
-
-    #[test]
-    fn required_components_take_leaves_required() {
-        #[derive(Component)]
-        #[require(Y)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        let mut world = World::new();
-        let e = world.spawn(X).id();
-        let _ = world.entity_mut(e).take::<X>().unwrap();
-        assert!(world.entity_mut(e).contains::<Y>());
-    }
-
-    #[test]
-    fn required_components_retain_keeps_required() {
-        #[derive(Component)]
-        #[require(Y)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        #[derive(Component, Default)]
-        struct Z;
-
-        let mut world = World::new();
-        let e = world.spawn((X, Z)).id();
-        world.entity_mut(e).retain::<X>();
-        assert!(world.entity_mut(e).contains::<X>());
-        assert!(world.entity_mut(e).contains::<Y>());
-        assert!(!world.entity_mut(e).contains::<Z>());
-    }
-
-    #[test]
-    fn required_components_spawn_then_insert_no_overwrite() {
-        #[derive(Component)]
-        #[require(Y)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y(usize);
-
-        let mut world = World::new();
-        let id = world.spawn((X, Y(10))).id();
-        world.entity_mut(id).insert(X);
-
-        assert_eq!(
-            10,
-            world.entity(id).get::<Y>().unwrap().0,
-            "Y should still have the manually provided value"
-        );
-    }
-
-    #[test]
-    fn dynamic_required_components() {
-        #[derive(Component)]
-        #[require(Y)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        let mut world = World::new();
-        let x_id = world.register_component::<X>();
-
-        let mut e = world.spawn_empty();
-
-        // SAFETY: x_id is a valid component id
-        bevy_ptr::OwningPtr::make(X, |ptr| unsafe {
-            e.insert_by_id(x_id, ptr);
-        });
-
-        assert!(e.contains::<Y>());
-    }
-
-    #[test]
-    fn remove_component_and_its_runtime_required_components() {
-        #[derive(Component)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        #[derive(Component, Default)]
-        struct Z;
-
-        #[derive(Component)]
-        struct V;
-
-        let mut world = World::new();
-        world.register_required_components::<X, Y>();
-        world.register_required_components::<Y, Z>();
-
-        let e = world.spawn((X, V)).id();
-        assert!(world.entity(e).contains::<X>());
-        assert!(world.entity(e).contains::<Y>());
-        assert!(world.entity(e).contains::<Z>());
-        assert!(world.entity(e).contains::<V>());
-
-        //check that `remove` works as expected
-        world.entity_mut(e).remove::<X>();
-        assert!(!world.entity(e).contains::<X>());
-        assert!(world.entity(e).contains::<Y>());
-        assert!(world.entity(e).contains::<Z>());
-        assert!(world.entity(e).contains::<V>());
-
-        world.entity_mut(e).insert(X);
-        assert!(world.entity(e).contains::<X>());
-        assert!(world.entity(e).contains::<Y>());
-        assert!(world.entity(e).contains::<Z>());
-        assert!(world.entity(e).contains::<V>());
-
-        //remove `X` again and ensure that `Y` and `Z` was removed too
-        world.entity_mut(e).remove_with_requires::<X>();
-        assert!(!world.entity(e).contains::<X>());
-        assert!(!world.entity(e).contains::<Y>());
-        assert!(!world.entity(e).contains::<Z>());
-        assert!(world.entity(e).contains::<V>());
-    }
-
-    #[test]
-    fn remove_component_and_its_required_components() {
-        #[derive(Component)]
-        #[require(Y)]
-        struct X;
-
-        #[derive(Component, Default)]
-        #[require(Z)]
-        struct Y;
-
-        #[derive(Component, Default)]
-        struct Z;
-
-        #[derive(Component)]
-        struct V;
-
-        let mut world = World::new();
-
-        let e = world.spawn((X, V)).id();
-        assert!(world.entity(e).contains::<X>());
-        assert!(world.entity(e).contains::<Y>());
-        assert!(world.entity(e).contains::<Z>());
-        assert!(world.entity(e).contains::<V>());
-
-        //check that `remove` works as expected
-        world.entity_mut(e).remove::<X>();
-        assert!(!world.entity(e).contains::<X>());
-        assert!(world.entity(e).contains::<Y>());
-        assert!(world.entity(e).contains::<Z>());
-        assert!(world.entity(e).contains::<V>());
-
-        world.entity_mut(e).insert(X);
-        assert!(world.entity(e).contains::<X>());
-        assert!(world.entity(e).contains::<Y>());
-        assert!(world.entity(e).contains::<Z>());
-        assert!(world.entity(e).contains::<V>());
-
-        //remove `X` again and ensure that `Y` and `Z` was removed too
-        world.entity_mut(e).remove_with_requires::<X>();
-        assert!(!world.entity(e).contains::<X>());
-        assert!(!world.entity(e).contains::<Y>());
-        assert!(!world.entity(e).contains::<Z>());
-        assert!(world.entity(e).contains::<V>());
-    }
-
-    #[test]
-    fn remove_bundle_and_his_required_components() {
-        #[derive(Component, Default)]
-        #[require(Y)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        #[derive(Component, Default)]
-        #[require(W)]
-        struct Z;
-
-        #[derive(Component, Default)]
-        struct W;
-
-        #[derive(Component)]
-        struct V;
-
-        #[derive(Bundle, Default)]
-        struct TestBundle {
-            x: X,
-            z: Z,
-        }
-
-        let mut world = World::new();
-        let e = world.spawn((TestBundle::default(), V)).id();
-
-        assert!(world.entity(e).contains::<X>());
-        assert!(world.entity(e).contains::<Y>());
-        assert!(world.entity(e).contains::<Z>());
-        assert!(world.entity(e).contains::<W>());
-        assert!(world.entity(e).contains::<V>());
-
-        world.entity_mut(e).remove_with_requires::<TestBundle>();
-        assert!(!world.entity(e).contains::<X>());
-        assert!(!world.entity(e).contains::<Y>());
-        assert!(!world.entity(e).contains::<Z>());
-        assert!(!world.entity(e).contains::<W>());
-        assert!(world.entity(e).contains::<V>());
-    }
-
-    #[test]
-    fn runtime_required_components() {
-        // Same as `required_components` test but with runtime registration
-
-        #[derive(Component)]
-        struct X;
-
-        #[derive(Component)]
-        struct Y {
-            value: String,
-        }
-
-        #[derive(Component)]
-        struct Z(u32);
-
-        impl Default for Y {
-            fn default() -> Self {
-                Self {
-                    value: "hello".to_string(),
-                }
-            }
-        }
-
-        let mut world = World::new();
-
-        world.register_required_components::<X, Y>();
-        world.register_required_components_with::<Y, Z>(|| Z(7));
-
-        let id = world.spawn(X).id();
-
-        assert_eq!(
-            "hello",
-            world.entity(id).get::<Y>().unwrap().value,
-            "Y should have the default value"
-        );
-        assert_eq!(
-            7,
-            world.entity(id).get::<Z>().unwrap().0,
-            "Z should have the value provided by the constructor defined in Y"
-        );
-
-        let id = world
-            .spawn((
-                X,
-                Y {
-                    value: "foo".to_string(),
-                },
-            ))
-            .id();
-        assert_eq!(
-            "foo",
-            world.entity(id).get::<Y>().unwrap().value,
-            "Y should have the manually provided value"
-        );
-        assert_eq!(
-            7,
-            world.entity(id).get::<Z>().unwrap().0,
-            "Z should have the value provided by the constructor defined in Y"
-        );
-
-        let id = world.spawn((X, Z(8))).id();
-        assert_eq!(
-            "hello",
-            world.entity(id).get::<Y>().unwrap().value,
-            "Y should have the default value"
-        );
-        assert_eq!(
-            8,
-            world.entity(id).get::<Z>().unwrap().0,
-            "Z should have the manually provided value"
-        );
-    }
-
-    #[test]
-    fn runtime_required_components_override_1() {
-        #[derive(Component)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        #[derive(Component)]
-        struct Z(u32);
-
-        let mut world = World::new();
-
-        // - X requires Y with default constructor
-        // - Y requires Z with custom constructor
-        // - X requires Z with custom constructor (more specific than X -> Y -> Z)
-        world.register_required_components::<X, Y>();
-        world.register_required_components_with::<Y, Z>(|| Z(5));
-        world.register_required_components_with::<X, Z>(|| Z(7));
-
-        let id = world.spawn(X).id();
-
-        assert_eq!(
-            7,
-            world.entity(id).get::<Z>().unwrap().0,
-            "Z should have the value provided by the constructor defined in X"
-        );
-    }
-
-    #[test]
-    fn runtime_required_components_override_2() {
-        // Same as `runtime_required_components_override_1` test but with different registration order
-
-        #[derive(Component)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        #[derive(Component)]
-        struct Z(u32);
-
-        let mut world = World::new();
-
-        // - X requires Y with default constructor
-        // - X requires Z with custom constructor (more specific than X -> Y -> Z)
-        // - Y requires Z with custom constructor
-        world.register_required_components::<X, Y>();
-        world.register_required_components_with::<X, Z>(|| Z(7));
-        world.register_required_components_with::<Y, Z>(|| Z(5));
-
-        let id = world.spawn(X).id();
-
-        assert_eq!(
-            7,
-            world.entity(id).get::<Z>().unwrap().0,
-            "Z should have the value provided by the constructor defined in X"
-        );
-    }
-
-    #[test]
-    fn runtime_required_components_propagate_up() {
-        // `A` requires `B` directly.
-        #[derive(Component)]
-        #[require(B)]
-        struct A;
-
-        #[derive(Component, Default)]
-        struct B;
-
-        #[derive(Component, Default)]
-        struct C;
-
-        let mut world = World::new();
-
-        // `B` requires `C` with a runtime registration.
-        // `A` should also require `C` because it requires `B`.
-        world.register_required_components::<B, C>();
-
-        let id = world.spawn(A).id();
-
-        assert!(world.entity(id).get::<C>().is_some());
-    }
-
-    #[test]
-    fn runtime_required_components_propagate_up_even_more() {
-        #[derive(Component)]
-        struct A;
-
-        #[derive(Component, Default)]
-        struct B;
-
-        #[derive(Component, Default)]
-        struct C;
-
-        #[derive(Component, Default)]
-        struct D;
-
-        let mut world = World::new();
-
-        world.register_required_components::<A, B>();
-        world.register_required_components::<B, C>();
-        world.register_required_components::<C, D>();
-
-        let id = world.spawn(A).id();
-
-        assert!(world.entity(id).get::<D>().is_some());
-    }
-
-    #[test]
-    fn runtime_required_components_deep_require_does_not_override_shallow_require() {
-        #[derive(Component)]
-        struct A;
-        #[derive(Component, Default)]
-        struct B;
-        #[derive(Component, Default)]
-        struct C;
-        #[derive(Component)]
-        struct Counter(i32);
-        #[derive(Component, Default)]
-        struct D;
-
-        let mut world = World::new();
-
-        world.register_required_components::<A, B>();
-        world.register_required_components::<B, C>();
-        world.register_required_components::<C, D>();
-        world.register_required_components_with::<D, Counter>(|| Counter(2));
-        // This should replace the require constructor in A since it is
-        // shallower.
-        world.register_required_components_with::<C, Counter>(|| Counter(1));
-
-        let id = world.spawn(A).id();
-
-        // The "shallower" of the two components is used.
-        assert_eq!(world.entity(id).get::<Counter>().unwrap().0, 1);
-    }
-
-    #[test]
-    fn runtime_required_components_deep_require_does_not_override_shallow_require_deep_subtree_after_shallow(
-    ) {
-        #[derive(Component)]
-        struct A;
-        #[derive(Component, Default)]
-        struct B;
-        #[derive(Component, Default)]
-        struct C;
-        #[derive(Component, Default)]
-        struct D;
-        #[derive(Component, Default)]
-        struct E;
-        #[derive(Component)]
-        struct Counter(i32);
-        #[derive(Component, Default)]
-        struct F;
-
-        let mut world = World::new();
-
-        world.register_required_components::<A, B>();
-        world.register_required_components::<B, C>();
-        world.register_required_components::<C, D>();
-        world.register_required_components::<D, E>();
-        world.register_required_components_with::<E, Counter>(|| Counter(1));
-        world.register_required_components_with::<F, Counter>(|| Counter(2));
-        world.register_required_components::<E, F>();
-
-        let id = world.spawn(A).id();
-
-        // The "shallower" of the two components is used.
-        assert_eq!(world.entity(id).get::<Counter>().unwrap().0, 1);
-    }
-
-    #[test]
-    fn runtime_required_components_existing_archetype() {
-        #[derive(Component)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        let mut world = World::new();
-
-        // Registering required components after the archetype has already been created should panic.
-        // This may change in the future.
-        world.spawn(X);
-        assert!(matches!(
-            world.try_register_required_components::<X, Y>(),
-            Err(RequiredComponentsError::ArchetypeExists(_))
-        ));
-    }
-
-    #[test]
-    fn runtime_required_components_fail_with_duplicate() {
-        #[derive(Component)]
-        #[require(Y)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        let mut world = World::new();
-
-        // This should fail: Tried to register Y as a requirement for X, but the requirement already exists.
-        assert!(matches!(
-            world.try_register_required_components::<X, Y>(),
-            Err(RequiredComponentsError::DuplicateRegistration(_, _))
-        ));
-    }
-
-    #[test]
-    fn required_components_inheritance_depth() {
-        // Test that inheritance depths are computed correctly for requirements.
-        //
-        // Requirements with `require` attribute:
-        //
-        // A -> B -> C
-        //   0    1
-        //
-        // Runtime requirements:
-        //
-        // X -> A -> B -> C
-        //   0    1    2
-        //
-        // X -> Y -> Z -> B -> C
-        //   0    1    2    3
-
-        #[derive(Component, Default)]
-        #[require(B)]
-        struct A;
-
-        #[derive(Component, Default)]
-        #[require(C)]
-        struct B;
-
-        #[derive(Component, Default)]
-        struct C;
-
-        #[derive(Component, Default)]
-        struct X;
-
-        #[derive(Component, Default)]
-        struct Y;
-
-        #[derive(Component, Default)]
-        struct Z;
-
-        let mut world = World::new();
-
-        let a = world.register_component::<A>();
-        let b = world.register_component::<B>();
-        let c = world.register_component::<C>();
-        let y = world.register_component::<Y>();
-        let z = world.register_component::<Z>();
-
-        world.register_required_components::<X, A>();
-        world.register_required_components::<X, Y>();
-        world.register_required_components::<Y, Z>();
-        world.register_required_components::<Z, B>();
-
-        world.spawn(X);
-
-        let required_a = world.get_required_components::<A>().unwrap();
-        let required_b = world.get_required_components::<B>().unwrap();
-        let required_c = world.get_required_components::<C>().unwrap();
-        let required_x = world.get_required_components::<X>().unwrap();
-        let required_y = world.get_required_components::<Y>().unwrap();
-        let required_z = world.get_required_components::<Z>().unwrap();
-
-        /// Returns the component IDs and inheritance depths of the required components
-        /// in ascending order based on the component ID.
-        fn to_vec(required: &RequiredComponents) -> Vec<(ComponentId, u16)> {
-            let mut vec = required
-                .0
-                .iter()
-                .map(|(id, component)| (*id, component.inheritance_depth))
-                .collect::<Vec<_>>();
-            vec.sort_by_key(|(id, _)| *id);
-            vec
-        }
-
-        // 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_x),
-            vec![(a, 0), (b, 1), (c, 2), (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)]);
-    }
-
-    #[test]
-    fn required_components_inheritance_depth_bias() {
-        #[derive(Component, PartialEq, Eq, Clone, Copy, Debug)]
-        struct MyRequired(bool);
-
-        #[derive(Component, Default)]
-        #[require(MyRequired(false))]
-        struct MiddleMan;
-
-        #[derive(Component, Default)]
-        #[require(MiddleMan)]
-        struct ConflictingRequire;
-
-        #[derive(Component, Default)]
-        #[require(MyRequired(true))]
-        struct MyComponent;
-
-        let mut world = World::new();
-        let order_a = world
-            .spawn((ConflictingRequire, MyComponent))
-            .get::<MyRequired>()
-            .cloned();
-        let order_b = world
-            .spawn((MyComponent, ConflictingRequire))
-            .get::<MyRequired>()
-            .cloned();
-
-        assert_eq!(order_a, Some(MyRequired(true)));
-        assert_eq!(order_b, Some(MyRequired(true)));
-    }
-
-    #[test]
-    #[should_panic]
-    fn required_components_recursion_errors() {
-        #[derive(Component, Default)]
-        #[require(B)]
-        struct A;
-
-        #[derive(Component, Default)]
-        #[require(C)]
-        struct B;
-
-        #[derive(Component, Default)]
-        #[require(B)]
-        struct C;
-
-        World::new().register_component::<A>();
-    }
-
-    #[test]
-    #[should_panic]
-    fn required_components_self_errors() {
-        #[derive(Component, Default)]
-        #[require(A)]
-        struct A;
-
-        World::new().register_component::<A>();
-    }
-
     #[derive(Default)]
     struct CaptureMapper(Vec<Entity>);
     impl EntityMapper for CaptureMapper {

release-content/migration-guides/required_components_rework.md

@@ -0,0 +1,15 @@
+---
+title: Required components refactor
+pull_requests: [20110]
+---
+
+The required components feature has been reworked to be more consistent around the priority of the required components and fix some soundness issues. In particular:
+
+- the priority of required components will now always follow a priority given by the depth-first/preorder traversal of the dependency tree. This was mostly the case before with a couple of exceptions that we are now fixing:
+  - when deriving the `Component` trait, sometimes required components at depth 1 had priority over components at depth 2 even if they came after in the depth-first ordering;
+  - registering runtime required components followed a breadth-first ordering and used the wrong inheritance depth for derived required components.
+- uses of the inheritance depth were removed from the `RequiredComponent` struct and from the methods for registering runtime required components, as it's not unused for the depth-first ordering;
+- `Component::register_required_components`, `RequiredComponents::register` and `RequiredComponents::register_by_id` are now `unsafe`;
+- `RequiredComponentConstructor`'s only field is now private for safety reasons.
+
+The `Component::register_required_components` method has also changed signature. It now takes the `ComponentId` of the component currently being registered and a single other parameter `RequiredComponentsRegistrator` which combines the old `components` and `required_components` parameters, since exposing both of them was unsound. As previously discussed the `inheritance_depth` is now useless and has been removed, while the `recursion_check_stack` has been moved into `ComponentsRegistrator` and will be handled automatically.