NiklasEi / bevy_asset_loader

Bevy plugin helping with asset loading and organization
Apache License 2.0
485 stars 54 forks source link

load-bearing `track_fake_long_task` system in `stageless_progress` example #54

Closed ChristopherBiscardi closed 2 years ago

ChristopherBiscardi commented 2 years ago

In the new iyes_loopless stageless_progress example, the track_fake_long_task system is load-bearing. If removed, the assets fail to be available.

If you remove the system in the current example, the current progress is logged out as done:0, total: 0.

2022-06-01T01:44:05.670996Z  INFO stageless_progress: Current progress: Progress { done: 0, total: 0 }

and if you add a requirement for the TextureAssets in quit, you get a crash because Res<TextureAssets> is not available.

fn quit(
    mut quit: EventWriter<AppExit>,
    // textures: Res<TextureAssets>,
) {
    info!("quitting");
    quit.send(AppExit);
}
2022-06-01T01:40:13.660762Z  WARN bevy_ecs::schedule::graph_utils: bevy_asset_loader::asset_loader::stageless::systems::run_loading_state<stageless_progress::MyStates> wants to be after unknown label: Preparation
2022-06-01T01:40:13.662065Z  INFO stageless_progress: Current progress: Progress { done: 0, total: 0 }
thread 'Compute Task Pool (3)' panicked at 'Resource requested by stageless_progress::quit does not exist: stageless_progress::TextureAssets', /Users/chris/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_ecs-0.7.0/src/system/system_param.rs:319:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at 'task has failed', /Users/chris/.cargo/registry/src/github.com-1ecc6299db9ec823/async-task-4.2.0/src/task.rs:425:45
thread 'main' panicked at 'Task thread panicked while executing.: Any { .. }', /Users/chris/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_tasks-0.7.0/src/task_pool.rs:77:21

There's a possibly unrelated warning as well relating to the Preparation label.

2022-06-01T01:40:13.660762Z  WARN bevy_ecs::schedule::graph_utils: bevy_asset_loader::asset_loader::stageless::systems::run_loading_state<stageless_progress::MyStates> wants to be after unknown label: Preparation

I think this is because of a race condition relating to time.seconds_since_startup(). With the delay, it's fine, without the delay it fails.

ChristopherBiscardi commented 2 years ago

The lack of loading also seems to occur in the regular progress_tracking example if you remove the 1s initial delay. Although this example doesn't crash, the progress only ever shows done: 0, total: 5.

2022-06-01T02:41:42.260194Z  INFO progress_tracking: Current progress: Progress { done: 0, total: 5 }

edit: I think this may actually be because the 1s delay causes more logs, thus we see 0/6 and 5/6 until 6/6 happens, so may not be related to the original report

NiklasEi commented 2 years ago

I can reproduce the issue for the stageless_progress example, but the "normal" progress_tracking example behaves correctly for me. I extended both examples with asserts on main and tested them without the long running fake task.

The problem in the stageless example seems to come from a system ordering issue. I was not able to fix the ordering itself.

There is a simple workaround though: let bevy_asset_loader decide when to change the state and not iyes_progress. Since in the first frame the progress can currently be { done: 0, total: 0 }, iyes_progress thinks everything is done and continues to the next state. The example on main uses the workaround for now.

NiklasEi commented 2 years ago

I will close this for now as I don't think it's worth investing much more time in a proper fix here. iyes_loopless Is a stopgap solution until stageless Bevy comes and the workaround seems good enough for that in my opinion. With stageless Bevy, this should be resolved properly.

ChristopherBiscardi commented 2 years ago

makes sense to me! Thanks for taking a look at it, the workaround fix has held up in my usage so far.

NiklasEi commented 1 year ago

I verified that the next version coming with Bevy 0.10 does not have this issue.