From 6741e01dfa8876a6113adb79bdd475982e4d3e80 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Thu, 21 Nov 2024 16:52:17 -0800 Subject: [PATCH] Fix adding a subtree of required components to an existing tree replacing shallower required component constructors (#16441) # Objective - Fixes #16406 even more. The previous implementation did not take into account the depth of the requiree when setting the depth relative to the required_by component. ## Solution - Add the depth of the requiree! ## Testing - Added a test. --------- Co-authored-by: Carter Anderson --- crates/bevy_ecs/src/component.rs | 9 ++++++--- crates/bevy_ecs/src/lib.rs | 34 ++++++++++++++++++++++++++++++++ crates/bevy_ecs/src/world/mod.rs | 2 +- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index d3b72f5a3ad1d..b915f8b5cad09 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -1029,8 +1029,8 @@ impl Components { /// registration will be used. pub(crate) unsafe fn register_required_components( &mut self, - required: ComponentId, requiree: ComponentId, + required: ComponentId, constructor: fn() -> R, ) -> Result<(), RequiredComponentsError> { // SAFETY: The caller ensures that the `requiree` is valid. @@ -1083,14 +1083,17 @@ impl Components { for (component_id, component) in inherited_requirements.iter() { // Register the required component. - // The inheritance depth is increased by `1` since this is a component required by the original required component. + // The inheritance depth of inherited components is whatever the requiree's + // depth is relative to `required_by_id`, plus the inheritance depth of the + // inherited component relative to the requiree, plus 1 to account for the + // requiree in between. // SAFETY: Component ID and constructor match the ones on the original requiree. // The original requiree is responsible for making sure the registration is safe. unsafe { required_components.register_dynamic( *component_id, component.constructor.clone(), - component.inheritance_depth + 1, + component.inheritance_depth + depth + 1, ); }; } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 4eab4d00be690..3a2e24d904a32 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -2399,6 +2399,40 @@ mod tests { assert_eq!(world.entity(id).get::().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::(); + world.register_required_components::(); + world.register_required_components::(); + world.register_required_components::(); + world.register_required_components_with::(|| Counter(1)); + world.register_required_components_with::(|| Counter(2)); + world.register_required_components::(); + + let id = world.spawn(A).id(); + + // The "shallower" of the two components is used. + assert_eq!(world.entity(id).get::().unwrap().0, 1); + } + #[test] fn runtime_required_components_existing_archetype() { #[derive(Component)] diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index eae555a0d56c4..63a3025eebc9b 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -515,7 +515,7 @@ impl World { // SAFETY: We just created the `required` and `requiree` components. unsafe { self.components - .register_required_components::(required, requiree, constructor) + .register_required_components::(requiree, required, constructor) } }