Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removing a bundle from an entity when one of the component has already been removed crashes #698

Closed
mockersf opened this issue Oct 18, 2020 · 1 comment · Fixed by #719
Closed
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior

Comments

@mockersf
Copy link
Member

On bevy master, , when removing a bundle from an entity when one of the component has already been removed, it crashes with this unwrap: https://github.com/bevyengine/bevy/blob/master/crates/bevy_ecs/src/system/commands.rs#L150

When using remove_one instead of remove to remove a component, it never crashes even if the component is not present on the entity. This is because remove_one checks for the presence of the component before removing it: https://github.com/bevyengine/bevy/blob/master/crates/bevy_ecs/src/system/commands.rs#L129

In the spirit of #649 and #651, I was thinking of removing the unwrap with a log, but that would mean that the other remaining components from the bundle won't be removed which may be an unexpected behaviour for the user.

The other way around this would be, when a bundle fails to remove, to try to remove each component of the bundle one by one, but I'm not sure if that's a good fix...

What do you think?

example code to reproduce the issue

remove_tag1 will never fail even though it just removes Tag1 from every entity, remove_tag12 will crash one Tag1 are removed.

use bevy::prelude::*;

struct Tag1;
struct Tag2;

fn main() {
    App::build()
        .add_default_plugins()
        .add_startup_system(startup_system.system())
        .add_system(count.system())
        .add_system(remove_tag1.system())
        .add_system(remove_tag12.system())
        .run();
}

fn startup_system(mut commands: Commands) {
    println!("setup");
    commands.spawn((Tag1, Tag2));
    commands.spawn((Tag1,));
}

fn count(mut tag1_query: Query<&Tag1>, mut tag2_query: Query<&Tag2>) {
    println!("counting tags");
    println!("- tag1 count: {}", tag1_query.iter().iter().len());
    println!("- tag2 count: {}", tag2_query.iter().iter().len());
}

fn remove_tag1(mut commands: Commands, mut entity_query: Query<Entity>) {
    let mut removed = 0;
    for entity in &mut entity_query.iter() {
        commands.remove_one::<Tag1>(entity);
        removed += 1;
    }
    println!("removed {} Tag1", removed);
}

fn remove_tag12(mut commands: Commands, mut entity_query: Query<(Entity, &Tag2)>) {
    let mut removed = 0;
    for (entity, _tag2) in &mut entity_query.iter() {
        commands.remove::<(Tag1, Tag2)>(entity);
        removed += 1;
    }
    println!("removed {} Tag1 & Tag2", removed);
}
@mockersf mockersf added the C-Bug An unexpected or incorrect behavior label Oct 18, 2020
@karroffel karroffel added the A-ECS Entities, components, systems, and events label Oct 20, 2020
@CleanCut
Copy link
Member

Here's my take at fixing the situation: #710

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior
Projects
None yet
3 participants