fix run-once runners (#10195)

# Objective

- After #9826, there are issues on "run once runners"
- example `without_winit` crashes:
```
2023-10-19T22:06:01.810019Z  INFO bevy_render::renderer: AdapterInfo { name: "llvmpipe (LLVM 15.0.7, 256 bits)", vendor: 65541, device: 0, device_type: Cpu, driver: "llvmpipe", driver_info: "Mesa 23.2.1 - kisak-mesa PPA (LLVM 15.0.7)", backend: Vulkan }
2023-10-19T22:06:02.860331Z  WARN bevy_audio::audio_output: No audio device found.
2023-10-19T22:06:03.215154Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "Linux 22.04 Ubuntu", kernel: "6.2.0-1014-azure", cpu: "Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz", core_count: "2", memory: "6.8 GiB" }
thread 'main' panicked at crates/bevy_render/src/pipelined_rendering.rs:91:14:
Unable to get RenderApp. Another plugin may have removed the RenderApp before PipelinedRenderingPlugin
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
- example `headless` runs the app twice with the `run_once` schedule

## Solution

- Expose a more complex state of an app than just "ready"
- Also block adding plugins to an app after it has finished or cleaned
up its plugins as that wouldn't work anyway

## Migration Guide

* `app.ready()` has been replaced by `app.plugins_state()` which will
return more details on the current state of plugins in the app
This commit is contained in:
François 2023-10-23 14:25:02 +02:00 committed by GitHub
parent b28525b772
commit 8fb5c99347
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 63 additions and 25 deletions

View File

@ -76,6 +76,7 @@ pub struct App {
plugin_name_added: HashSet<String>, plugin_name_added: HashSet<String>,
/// A private counter to prevent incorrect calls to `App::run()` from `Plugin::build()` /// A private counter to prevent incorrect calls to `App::run()` from `Plugin::build()`
building_plugin_depth: usize, building_plugin_depth: usize,
plugins_state: PluginsState,
} }
impl Debug for App { impl Debug for App {
@ -194,6 +195,19 @@ impl Default for App {
} }
} }
/// Plugins state in the application
#[derive(PartialEq, Eq, Debug, Clone, Copy)]
pub enum PluginsState {
/// Plugins are being added.
Adding,
/// All plugins already added are ready.
Ready,
/// Finish has been executed for all plugins added.
Finished,
/// Cleanup has been executed for all plugins added.
Cleaned,
}
// Dummy plugin used to temporary hold the place in the plugin registry // Dummy plugin used to temporary hold the place in the plugin registry
struct PlaceholderPlugin; struct PlaceholderPlugin;
impl Plugin for PlaceholderPlugin { impl Plugin for PlaceholderPlugin {
@ -221,6 +235,7 @@ impl App {
plugin_name_added: Default::default(), plugin_name_added: Default::default(),
main_schedule_label: Box::new(Main), main_schedule_label: Box::new(Main),
building_plugin_depth: 0, building_plugin_depth: 0,
plugins_state: PluginsState::Adding,
} }
} }
@ -288,7 +303,7 @@ impl App {
panic!("App::run() was called from within Plugin::build(), which is not allowed."); panic!("App::run() was called from within Plugin::build(), which is not allowed.");
} }
if app.ready() { if app.plugins_state() == PluginsState::Ready {
// If we're already ready, we finish up now and advance one frame. // If we're already ready, we finish up now and advance one frame.
// This prevents black frames during the launch transition on iOS. // This prevents black frames during the launch transition on iOS.
app.finish(); app.finish();
@ -300,20 +315,25 @@ impl App {
(runner)(app); (runner)(app);
} }
/// Check that [`Plugin::ready`] of all plugins returns true. This is usually called by the /// Check the state of all plugins already added to this app. This is usually called by the
/// event loop, but can be useful for situations where you want to use [`App::update`] /// event loop, but can be useful for situations where you want to use [`App::update`]
pub fn ready(&self) -> bool { #[inline]
for plugin in &self.plugin_registry { pub fn plugins_state(&self) -> PluginsState {
if !plugin.ready(self) { match self.plugins_state {
return false; PluginsState::Adding => {
for plugin in &self.plugin_registry {
if !plugin.ready(self) {
return PluginsState::Adding;
}
}
PluginsState::Ready
} }
state => state,
} }
true
} }
/// Run [`Plugin::finish`] for each plugin. This is usually called by the event loop once all /// Run [`Plugin::finish`] for each plugin. This is usually called by the event loop once all
/// plugins are [`App::ready`], but can be useful for situations where you want to use /// plugins are ready, but can be useful for situations where you want to use [`App::update`].
/// [`App::update`].
pub fn finish(&mut self) { pub fn finish(&mut self) {
// temporarily remove the plugin registry to run each plugin's setup function on app. // temporarily remove the plugin registry to run each plugin's setup function on app.
let plugin_registry = std::mem::take(&mut self.plugin_registry); let plugin_registry = std::mem::take(&mut self.plugin_registry);
@ -321,6 +341,7 @@ impl App {
plugin.finish(self); plugin.finish(self);
} }
self.plugin_registry = plugin_registry; self.plugin_registry = plugin_registry;
self.plugins_state = PluginsState::Finished;
} }
/// Run [`Plugin::cleanup`] for each plugin. This is usually called by the event loop after /// Run [`Plugin::cleanup`] for each plugin. This is usually called by the event loop after
@ -332,6 +353,7 @@ impl App {
plugin.cleanup(self); plugin.cleanup(self);
} }
self.plugin_registry = plugin_registry; self.plugin_registry = plugin_registry;
self.plugins_state = PluginsState::Cleaned;
} }
/// Adds [`State<S>`] and [`NextState<S>`] resources, [`OnEnter`] and [`OnExit`] schedules /// Adds [`State<S>`] and [`NextState<S>`] resources, [`OnEnter`] and [`OnExit`] schedules
@ -696,6 +718,14 @@ impl App {
/// [`PluginGroup`]:super::PluginGroup /// [`PluginGroup`]:super::PluginGroup
#[track_caller] #[track_caller]
pub fn add_plugins<M>(&mut self, plugins: impl Plugins<M>) -> &mut Self { pub fn add_plugins<M>(&mut self, plugins: impl Plugins<M>) -> &mut Self {
if matches!(
self.plugins_state(),
PluginsState::Cleaned | PluginsState::Finished
) {
panic!(
"Plugins cannot be added after App::cleanup() or App::finish() has been called."
);
}
plugins.add_to_app(self); plugins.add_to_app(self);
self self
} }
@ -947,14 +977,20 @@ impl App {
} }
fn run_once(mut app: App) { fn run_once(mut app: App) {
while !app.ready() { let plugins_state = app.plugins_state();
#[cfg(not(target_arch = "wasm32"))] if plugins_state != PluginsState::Cleaned {
bevy_tasks::tick_global_task_pools_on_main_thread(); while app.plugins_state() == PluginsState::Adding {
#[cfg(not(target_arch = "wasm32"))]
bevy_tasks::tick_global_task_pools_on_main_thread();
}
app.finish();
app.cleanup();
} }
app.finish();
app.cleanup();
app.update(); // if plugins where cleaned before the runner start, an update already ran
if plugins_state != PluginsState::Cleaned {
app.update();
}
} }
/// An event that indicates the [`App`] should exit. This will fully exit the app process at the /// An event that indicates the [`App`] should exit. This will fully exit the app process at the

View File

@ -1,6 +1,7 @@
use crate::{ use crate::{
app::{App, AppExit}, app::{App, AppExit},
plugin::Plugin, plugin::Plugin,
PluginsState,
}; };
use bevy_ecs::event::{Events, ManualEventReader}; use bevy_ecs::event::{Events, ManualEventReader};
use bevy_utils::{Duration, Instant}; use bevy_utils::{Duration, Instant};
@ -71,8 +72,9 @@ impl Plugin for ScheduleRunnerPlugin {
fn build(&self, app: &mut App) { fn build(&self, app: &mut App) {
let run_mode = self.run_mode; let run_mode = self.run_mode;
app.set_runner(move |mut app: App| { app.set_runner(move |mut app: App| {
if !app.ready() { let plugins_state = app.plugins_state();
while !app.ready() { if plugins_state != PluginsState::Cleaned {
while app.plugins_state() == PluginsState::Adding {
#[cfg(not(target_arch = "wasm32"))] #[cfg(not(target_arch = "wasm32"))]
bevy_tasks::tick_global_task_pools_on_main_thread(); bevy_tasks::tick_global_task_pools_on_main_thread();
} }
@ -83,7 +85,10 @@ impl Plugin for ScheduleRunnerPlugin {
let mut app_exit_event_reader = ManualEventReader::<AppExit>::default(); let mut app_exit_event_reader = ManualEventReader::<AppExit>::default();
match run_mode { match run_mode {
RunMode::Once => { RunMode::Once => {
app.update(); // if plugins where cleaned before the runner start, an update already ran
if plugins_state != PluginsState::Cleaned {
app.update();
}
} }
RunMode::Loop { wait } => { RunMode::Loop { wait } => {
let mut tick = move |app: &mut App, let mut tick = move |app: &mut App,

View File

@ -20,7 +20,7 @@ use system::{changed_windows, create_windows, despawn_windows, CachedWindow};
pub use winit_config::*; pub use winit_config::*;
pub use winit_windows::*; pub use winit_windows::*;
use bevy_app::{App, AppExit, Last, Plugin}; use bevy_app::{App, AppExit, Last, Plugin, PluginsState};
use bevy_ecs::event::{Events, ManualEventReader}; use bevy_ecs::event::{Events, ManualEventReader};
use bevy_ecs::prelude::*; use bevy_ecs::prelude::*;
use bevy_ecs::system::{SystemParam, SystemState}; use bevy_ecs::system::{SystemParam, SystemState};
@ -378,8 +378,6 @@ pub fn winit_runner(mut app: App) {
ResMut<CanvasParentResizeEventChannel>, ResMut<CanvasParentResizeEventChannel>,
)> = SystemState::from_world(&mut app.world); )> = SystemState::from_world(&mut app.world);
let mut finished_and_setup_done = app.ready();
// setup up the event loop // setup up the event loop
let event_handler = move |event: Event<()>, let event_handler = move |event: Event<()>,
event_loop: &EventLoopWindowTarget<()>, event_loop: &EventLoopWindowTarget<()>,
@ -387,14 +385,13 @@ pub fn winit_runner(mut app: App) {
#[cfg(feature = "trace")] #[cfg(feature = "trace")]
let _span = bevy_utils::tracing::info_span!("winit event_handler").entered(); let _span = bevy_utils::tracing::info_span!("winit event_handler").entered();
if !finished_and_setup_done { if app.plugins_state() != PluginsState::Cleaned {
if !app.ready() { if app.plugins_state() != PluginsState::Ready {
#[cfg(not(target_arch = "wasm32"))] #[cfg(not(target_arch = "wasm32"))]
tick_global_task_pools_on_main_thread(); tick_global_task_pools_on_main_thread();
} else { } else {
app.finish(); app.finish();
app.cleanup(); app.cleanup();
finished_and_setup_done = true;
} }
if let Some(app_exit_events) = app.world.get_resource::<Events<AppExit>>() { if let Some(app_exit_events) = app.world.get_resource::<Events<AppExit>>() {
@ -775,7 +772,7 @@ pub fn winit_runner(mut app: App) {
} }
}; };
if finished_and_setup_done && should_update { if app.plugins_state() == PluginsState::Cleaned && should_update {
// reset these on each update // reset these on each update
runner_state.wait_elapsed = false; runner_state.wait_elapsed = false;
runner_state.window_event_received = false; runner_state.window_event_received = false;