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

Asset will wrongly unload while strong handle is alive under certain circumstances #12344

Closed
Litttlefish opened this issue Mar 6, 2024 · 1 comment · Fixed by #12459
Closed
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior
Milestone

Comments

@Litttlefish
Copy link
Contributor

Litttlefish commented Mar 6, 2024

Bevy version

0.13

What you did

In my tests the app has two states(A and B), and an entity with a sprite. when switching states, this entity will be despawned in OnExit(State::A), and spawn a new one entity is identical to the prior one in OnEnter(State::B).

What went wrong

When respawning it, the sprite will disappear(actually it is there, but the sprite is empty).

This didn't happen in 0.12 version.

Additional information

Here is my test code(need a custom image as test.png):

use bevy::prelude::*;
use bevy::render::camera::ScalingMode;

#[derive(Component)]
struct Marker;

#[derive(States, Default, Debug, PartialEq, Eq, Hash, Clone, Copy)]
enum State {
    #[default]
    A,
    B,
}

fn main() {
    App::new()
        .add_plugins((DefaultPlugins.set(WindowPlugin {
            primary_window: Some(Window {
                resolution: (960.0, 540.0).into(),
                resizable: false,
                ..default()
            }),
            ..default()
        }),))
        .init_state::<State>()
        .add_systems(Startup, setup)
        .add_systems(OnExit(State::A), exit_a)
        .add_systems(OnEnter(State::B), enter_b)
        .add_systems(Update, switch)
        //.add_plugin(EditorPlugin::default())
        .run();
}

fn setup(mut cmd: Commands, asset_server: Res<AssetServer>) {
    cmd.spawn(Camera2dBundle {
        projection: OrthographicProjection {
            scaling_mode: ScalingMode::AutoMin {
                min_width: 960.,
                min_height: 540.,
            },
            ..Camera2dBundle::default().projection
        },
        ..default()
    });
    cmd.spawn((
        SpriteBundle {
            texture: asset_server.load("sprites/test.png"),
            ..default()
        },
        Marker,
    ));
}

fn switch(mouse: Res<ButtonInput<MouseButton>>, mut next_state: ResMut<NextState<State>>) {
    if mouse.just_pressed(MouseButton::Left) {
        next_state.set(State::B)
    }
}

fn exit_a(mut cmd: Commands, q: Query<Entity, With<Marker>>) {
    cmd.entity(q.single()).despawn();
}

fn enter_b(mut cmd: Commands, asset_server: Res<AssetServer>) {
    cmd.spawn((SpriteBundle {
        sprite: Sprite {
            custom_size: Some((960., 540.).into()),
            ..default()
        },
        texture: asset_server.load("sprites/test.png"),
        ..default()
    },));
}

After click state will switch, and the sprite entity will "disappear".

My assumption is that image asset unloaded the sprite in OnExit(State::A), but it didn't load it again in OnEnter(State::B).

@Litttlefish Litttlefish added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 6, 2024
@TrialDragon TrialDragon added A-Assets Load files from disk to use for things like images, models, and sounds and removed S-Needs-Triage This issue needs to be labelled labels Mar 6, 2024
@Litttlefish
Copy link
Contributor Author

Litttlefish commented Mar 7, 2024

Addendum:
The "unloaded" asset actually has a strong handle(you can get it with asset_server.get_handle()), but the image itself is empty.
If you add the handle to a resource(like lock it between state changes), then the image is fine.

So I'll rename this issue

@Litttlefish Litttlefish changed the title AssetServer won't load assets if the handle was unloaded in state change Asset will wrongly unload while strong handle is active under certain circumstances Mar 7, 2024
@Litttlefish Litttlefish changed the title Asset will wrongly unload while strong handle is active under certain circumstances Asset will wrongly unload while strong handle is alive under certain circumstances Mar 7, 2024
@JMS55 JMS55 added this to the 0.13.1 milestone Mar 8, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 17, 2024
# Objective

fix #12344

## Solution

use existing machinery in track_assets to determine if the asset is
unused before firing Asset::Unused event

~~most extract functions use `AssetEvent::Removed` to schedule deletion
of render world resources. `RenderAssetPlugin` was using
`AssetEvent::Unused` instead.
`Unused` fires when the last strong handle is dropped, even if a new one
is created. `Removed` only fires when a new one is not created.
as far as i can see, `Unused` is the same as `Removed` except for this
"feature", and that it also fires early if all handles for a loading
asset are dropped (`Removed` fires after the loading completes). note
that in that case, processing based on `Loaded` won't have been done
anyway.
i think we should get rid of `Unused` completely, it is not currently
used anywhere (except here, previously) and i think using it is probably
always a mistake.
i also am not sure why we keep loading assets that have been dropped
while loading, we should probably drop the loader task as well and
remove immediately.~~
mockersf pushed a commit that referenced this issue Mar 18, 2024
fix #12344

use existing machinery in track_assets to determine if the asset is
unused before firing Asset::Unused event

~~most extract functions use `AssetEvent::Removed` to schedule deletion
of render world resources. `RenderAssetPlugin` was using
`AssetEvent::Unused` instead.
`Unused` fires when the last strong handle is dropped, even if a new one
is created. `Removed` only fires when a new one is not created.
as far as i can see, `Unused` is the same as `Removed` except for this
"feature", and that it also fires early if all handles for a loading
asset are dropped (`Removed` fires after the loading completes). note
that in that case, processing based on `Loaded` won't have been done
anyway.
i think we should get rid of `Unused` completely, it is not currently
used anywhere (except here, previously) and i think using it is probably
always a mistake.
i also am not sure why we keep loading assets that have been dropped
while loading, we should probably drop the loader task as well and
remove immediately.~~
Wabtey added a commit to Fabinistere/cats_destroyer_2000 that referenced this issue Aug 8, 2024
- prevent events handler/removalDetectors to crash by running `Commands::entity()` or `Query::get().unwrap()` after a level change
- chain systems
- FIXME: as issued [here](bevyengine/bevy#12344), the asset will wrongly unload while strong handle is alive after a State Change. It has been fixed in `13.1`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants