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

Light doesn't go away when its entity is despawned #18

Closed
drawk-cab opened this issue Jul 26, 2024 · 9 comments · Fixed by #19 or #25
Closed

Light doesn't go away when its entity is despawned #18

drawk-cab opened this issue Jul 26, 2024 · 9 comments · Fixed by #19 or #25
Labels

Comments

@drawk-cab
Copy link

drawk-cab commented Jul 26, 2024

I'm trying to use bevy_light_2d to implement glowing entities, but despawning the entities doesn't remove the light.

Here's an altered version of the basic example that demonstrates the issue:

use bevy::prelude::*;
use bevy_light_2d::prelude::*;

#[derive(Component)]
struct Mobile;

fn main() {
    App::new()
        .add_plugins((DefaultPlugins, Light2dPlugin))
        .add_systems(Startup, setup)
        .add_systems(Update, go)
        .run();
}

fn go(
    mut commands: Commands,
    mut query: Query<(Entity, &mut Transform), With<Mobile>>
) {
    for (e, mut transform) in query.iter_mut() {
        transform.translation.x += 1.0;
        if transform.translation.x > 400.0 {
            commands.entity(e).despawn_recursive()
        }
    }
}

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

    commands.spawn((
        PointLight2dBundle {
            point_light: PointLight2d {
                intensity: 3.0,
                radius: 100.0,
                ..default()
            },
            ..default()
        },
        Mobile,
    ));
}

I thought this code would cause the light to disappear when the entity reaches x = 400.0, but instead it just stops moving.
Similarly in my game, I build up lights where entities die, and I can't see a way to remove them.

@jgayfer
Copy link
Owner

jgayfer commented Jul 28, 2024

Thanks for the report + example!

I will take a look when I'm able.

@jgayfer
Copy link
Owner

jgayfer commented Jul 28, 2024

@drawk-cab This should be fixed now. Many thanks for the example. Patch published to crates.io.

@drawk-cab
Copy link
Author

drawk-cab commented Jul 29, 2024

The example is fixed but I'm still getting lights persisting in my game. Even if I don't despawn all lights, occasionally one gets stuck on. But I haven't worked out which ones or why yet.
Will have to figure out another example :)

If it's any help, the lights that get stuck on do now tend to go away as I spawn more lights, as if there was a system of slots where a newly spawned light has a chance to be assigned an old one's slot and replace it. This is an improvement on before where a despawn/replace cycle always added new lights, which eventually slowed down the game.

@jgayfer
Copy link
Owner

jgayfer commented Jul 29, 2024

@drawk-cab Thanks for letting me know. How many lights are we talking approximately?

@drawk-cab
Copy link
Author

drawk-cab commented Jul 31, 2024

About 20 in the game.

Here's an example that I think replicates the issue:

use bevy::prelude::*;
use bevy_light_2d::prelude::*;

#[derive(Component)]
struct Mobile;

#[derive(Resource)]
struct SpawnPoint(Vec3);

fn main() {
    App::new()
        .add_plugins((DefaultPlugins, Light2dPlugin))
        .insert_resource(SpawnPoint( Vec3::new(-200.0, -200.0, 0.0)))
        .add_systems(Startup, setup)
        .add_systems(PreUpdate, despawn)
        .add_systems(Update, spawn)
        .run();
}

fn despawn(
    mut commands: Commands,
    spawn_point: Res<SpawnPoint>,
    query: Query<Entity, With<Mobile>>,
) {
    if spawn_point.0.x > -50.0 {
        for e in query.iter() {
            commands.entity(e).despawn_recursive();
        }
    }
}

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

fn spawn(mut commands: Commands,
    mut spawn_point: ResMut<SpawnPoint>) {
    commands.spawn((
        PointLight2dBundle {
            transform: Transform::from_translation(spawn_point.0),
            point_light: PointLight2d {
                intensity: 3.0,
                radius: 10.0,
              falloff: 1.0,
                ..default()
            },
            ..default()
        },
        Mobile,
    ));
    if spawn_point.0.x > 200.0 {
        spawn_point.0.x = -200.0;
        spawn_point.0.y += 10.0;
    } else {
        spawn_point.0.x += 10.0;
    }
}

This code moves through columns and rows spawning a light every frame, then despawning everything, except for the first 50 units of travel each row when the despawning is skipped. So what I'd expect is a row of lights building up gradually, then getting despawned, and a single light seeming to continue (actually a different entity every frame)

But instead most of the lights remain on until the app moves to the next row. The single light, on the other hand is successfully despawned each frame.

I don't think using the PreUpdate schedule is the problem, it still works if I put the despawning into Update. Just in my game I like to be sure I'm not trying to do work on entities that get despawned anyway.

You can see how as each light in the long row is spawned, it replaces the one immediately below it.

@jgayfer
Copy link
Owner

jgayfer commented Aug 6, 2024

Going to re-open so as to not let this get lost.

@jgayfer jgayfer reopened this Aug 6, 2024
@jgayfer
Copy link
Owner

jgayfer commented Aug 11, 2024

Took a closer look, and can confirm something whacky is going on.

I'm still not sure what is causing the issue. Everything on the extraction + prepare side lines up the the expected number of lights. I'm suspicious something is going on with how we're managing memory on the GPU (which could end up pointing back to GPUArrayBuffer, a Bevy helper we're using).

@jgayfer
Copy link
Owner

jgayfer commented Aug 15, 2024

I think I've cracked it. Will try and get a PR up soon.

@jgayfer jgayfer mentioned this issue Aug 18, 2024
2 tasks
@jgayfer jgayfer added the T-Bug label Aug 18, 2024
@drawk-cab
Copy link
Author

This has fixed the issue in my game too, thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants