Skip to content

Commit

Permalink
Fix adding a subtree of required components to an existing tree repla…
Browse files Browse the repository at this point in the history
…cing 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 <mcanders1@gmail.com>
  • Loading branch information
andriyDev and cart authored Nov 22, 2024
1 parent 8a3a8b5 commit 6741e01
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 4 deletions.
9 changes: 6 additions & 3 deletions crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,8 +1029,8 @@ impl Components {
/// registration will be used.
pub(crate) unsafe fn register_required_components<R: Component>(
&mut self,
required: ComponentId,
requiree: ComponentId,
required: ComponentId,
constructor: fn() -> R,
) -> Result<(), RequiredComponentsError> {
// SAFETY: The caller ensures that the `requiree` is valid.
Expand Down Expand Up @@ -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,
);
};
}
Expand Down
34 changes: 34 additions & 0 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2399,6 +2399,40 @@ mod tests {
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)]
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ impl World {
// SAFETY: We just created the `required` and `requiree` components.
unsafe {
self.components
.register_required_components::<R>(required, requiree, constructor)
.register_required_components::<R>(requiree, required, constructor)
}
}

Expand Down

0 comments on commit 6741e01

Please sign in to comment.