jgayfer / bevy_light_2d

General purpose 2D lighting for the Bevy game engine.
MIT License
170 stars 7 forks source link

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

Closed drawk-cab closed 2 months ago

drawk-cab commented 3 months ago

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 commented 3 months ago

Thanks for the report + example!

I will take a look when I'm able.

jgayfer commented 3 months ago

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

drawk-cab commented 3 months ago

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 commented 3 months ago

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

drawk-cab commented 3 months ago

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 commented 3 months ago

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

jgayfer commented 3 months ago

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 commented 3 months ago

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

drawk-cab commented 2 months ago

This has fixed the issue in my game too, thanks :+1: