Skip to content

Change entity disabling/enabling components to be recursive over Children#24158

Merged
alice-i-cecile merged 3 commits into
bevyengine:mainfrom
Freyja-moth:ecursive-entity-disabling-components
May 6, 2026
Merged

Change entity disabling/enabling components to be recursive over Children#24158
alice-i-cecile merged 3 commits into
bevyengine:mainfrom
Freyja-moth:ecursive-entity-disabling-components

Conversation

@Freyja-moth
Copy link
Copy Markdown
Contributor

@Freyja-moth Freyja-moth commented May 6, 2026

Objective

Addresses part of #24157

Solution

Change .try_insert(Disabled) to .insert_recursive::<Children>(Disabled), and .try_remove::<Disabled>() to .remove_recursive::<Children, Disabled>()

Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be cool if this was generic over all relations based on the linked despawn behavior, but I don't see any way to get there immediately.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it A-States App-level states machines labels May 6, 2026
@alice-i-cecile alice-i-cecile added this to the 0.19 milestone May 6, 2026
@alice-i-cecile
Copy link
Copy Markdown
Member

Import is broken :)

@Freyja-moth Freyja-moth changed the title Change entity disabling/enabling components to be recursive over ChildOf Change entity disabling/enabling components to be recursive over Children May 6, 2026
@alice-i-cecile alice-i-cecile enabled auto-merge May 6, 2026 17:54
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 6, 2026
@urben1680
Copy link
Copy Markdown
Contributor

It would be cool if this was generic over all relations based on the linked despawn behavior, but I don't see any way to get there immediately.

This is already possible since ComponentInfo contains that information. I do that in my reversible schedule crate but I am unsure if it makes much sense to share the code as it is still 0.18.

Merged via the queue into bevyengine:main with commit 1554049 May 6, 2026
38 checks passed
@SkiFire13
Copy link
Copy Markdown
Contributor

A couple of observations:

  • this won't remove the entity from the Children instance which can still cause some confusion;
  • this will cause disabling and then enabling an entity to no longer be an idempotent operation: if any descendant was already disabled, after these operations it will become enabled.

@urben1680
Copy link
Copy Markdown
Contributor

urben1680 commented May 6, 2026

Eh whatever here is the 0.18 code from my crate, you pass an entities_set to this which already contains the parent you want to disable and it gets populated with all children and their children which are not yet disabled.

fn add_enablded_children_to_set(
    world: &World,
    parent: EntityRef,
    entities_set: &mut EntityHashSet
) {
    if parent.contains::<Disabled>() {
        // already disabled
        return;
    }
    let children = parent
        .archetype()
        .components()
        .iter()
        .flat_map(|&component_id| {
            world
                .components()
                .get_info(component_id)
                .unwrap() // listed component id should be known to the world
                .relationship_accessor()
                .and_then(|relationship| match *relationship {
                    RelationshipAccessor::RelationshipTarget { iter, linked_spawn: true } => {
                        // should not panic as parent's archetype lists this component id
                        let ptr = parent.get_by_id(component_id).unwrap();
                        // SAFETY: given ComponentId matches the RelationshipAccessor of the
                        // component type
                        unsafe { Some(iter(ptr)) }
                    }
                    _ => None,
                })
                .into_iter()
                .flatten()
        });

    for child in children {
        if !entities_set.insert(child) {
            // already known
            continue;
        }
        let Ok(child_ref) = world.get_entity(child) else {
            // despawned
            entities_set.remove(child);
            continue;
        };
        if child_ref.contains::<Disabled>() {
            // already disabled
            entities_set.remove(child);
            continue;
        }
        add_enablded_children_to_set(world, child_ref, entities_set);
    }
}

Then you'd batch-insert it with the set.

I have no method for enabling them again but you get the gist here to rewrite it for that too. Probably just need another boolean argument and do equality checks with .contains::<Disabled>().

@targrub
Copy link
Copy Markdown
Contributor

targrub commented May 6, 2026

A couple of observations:

* this won't remove the entity from the `Children` instance which can still cause some confusion;

* this will cause disabling and then enabling an entity to no longer be an idempotent operation: if any descendant was already disabled, after these operations it will become enabled.

These operations should have tests, IMHO, to assure clarity of intended function. They can be fragile to changes of code. (See history of visibility operations for examples.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-States App-level states machines C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants