bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.22k stars 3.57k forks source link

Add regression test for spawning entities during remove hooks #16260

Open alice-i-cecile opened 1 week ago

alice-i-cecile commented 1 week ago

I have made a test for this, that fails at 0.14.2 but passes in current bevy main, so it should cover from this regression

#[test]
fn test_spawn_entities_during_on_remove_hook() {
    #[derive(Component)]
    struct MyComponent;

    App::new()
        .add_plugins(MinimalPlugins)
        .add_systems(Startup, |world: &mut World| {
            world.register_component_hooks::<MyComponent>().on_remove(
                |mut world, _entity, _component_id| {
                    world.commands().spawn_empty();
                },
            );
        })
        .add_systems(
            Update,
            |mut commands: Commands,
                my_component_q: Query<Entity, With<MyComponent>>,
                mut exit_event: EventWriter<AppExit>,
                mut after_first_frame: Local<bool>| {
                for entity in &my_component_q {
                    commands.entity(entity).despawn();
                }

                commands.spawn(MyComponent);

                if *after_first_frame {
                    exit_event.send(AppExit::Success);
                }

                *after_first_frame = true;
            },
        )
        .run();
}

It's a bit of an unusual test (I'm not even sure if MinimalPlugins can be used in a test like this). It's more an integration test rather than a unit test. I don't know if there is some place inside Bevy repo where more of these tests exist. If so, I can open a PR to add it.

Originally posted by @Maximetinu in https://github.com/bevyengine/bevy/issues/16219#issuecomment-2458452806

Maximetinu commented 1 week ago

Good!

I can open a PR for this as soon as I know where to place this test

I will dig through the codebase or ask in discord about it

alice-i-cecile commented 1 week ago

I think we can probably swap this over to a simpler unit test using World and run_system_once :) Then it can go in the hooks part of bevy_ecs, which is much simpler.