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

Filter RemovedComponents for entities that no longer exist #510

Merged
merged 5 commits into from
Feb 11, 2024

Conversation

thebluefish
Copy link
Contributor

@thebluefish thebluefish commented Feb 2, 2024

Fixes #509

Here is an MVCE demonstrating a crash:

use bevy::prelude::*;
use bevy_ecs_tilemap::prelude::*;

fn main() {
    App::new()
        .add_plugins((DefaultPlugins, TilemapPlugin))
        .add_state::<ExampleState>()
        .add_systems(Startup, setup)
        .add_systems(Update, transition.run_if(in_state(ExampleState::State1)))
        .add_systems(OnExit(ExampleState::State1), despawn)
        .add_systems(OnEnter(ExampleState::State2), spawn)
        .run();
}

#[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Hash, States)]
enum ExampleState {
    #[default]
    State1,
    State2,
}

fn setup(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());
    commands.spawn(TilemapBundle::default());
}

fn transition(mut next_state: ResMut<NextState<ExampleState>>) {
    println!("transitioning");
    next_state.set(ExampleState::State2);
}

fn despawn(mut commands: Commands, query: Query<Entity, With<TileStorage>>) {
    for entity in &query {
        println!("despawning {entity:?}");
        commands.entity(entity).despawn();
    }
}

fn spawn(mut commands: Commands) {
    let entity = commands.spawn_empty().id();
    println!("spawning {entity:?}");
    commands.entity(entity).insert(SpriteBundle::default());
}

transitioning
despawning 2v0
spawning 2v1
2024-02-02T18:54:12.858004Z ERROR bevy_ecs::system::commands: Failed to 'insert or spawn' bundle of type bevy_sprite::render::SpriteBatch into the following invalid entities: [2v1]
thread '' panicked at C:\Users\s33n.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_render-0.12.1\src\render_phase\draw.rs:275:67:
called Result::unwrap() on an Err value: NoSuchEntity(2v1)

I believe the root problem is that we are erroneously inserting an Entity 2v0 into the render world via ExtractedRemovedMapBundle when that entity in our current world is now 2v1. This appears to interfere with systems that interact with that entity in the render world.

@thebluefish
Copy link
Contributor Author

Digging into this, it seemed that my original fix was a band-aid solution that failed to understand what was being accomplished. It resolved the original problem, but simply by breaking the logic. The new solution fixes what appears to be an inconsistency when extracting, by keeping the same Entity between main and render worlds for these temp cleanup components.

@rparrett
Copy link
Collaborator

rparrett commented Feb 2, 2024

Have been following. This new fix makes much more sense to me too.

Confirmed all examples still function.

@rparrett
Copy link
Collaborator

rparrett commented Feb 4, 2024

CI is failing on fmt and clippy.

@StarArawn
Copy link
Owner

@thebluefish Can you run fmt/clippy? If not I'll branch off of your code and open a new PR. 👍 We're planning on rolling out a release tomorrow.

@thebluefish
Copy link
Contributor Author

Ah apologies, I did not see a notification for the prior messages. Should be good now, but I'm not sure how to restart the CI.

Copy link
Collaborator

@rparrett rparrett left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@StarArawn StarArawn merged commit 92fc406 into StarArawn:main Feb 11, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.12 bug: despawn + spawn during state change, without spawning texture, panics
3 participants