From 8fb5c99347964da55dc0e54e9e471132a7d9d3a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Mon, 23 Oct 2023 14:25:02 +0200 Subject: [PATCH] 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 --- crates/bevy_app/src/app.rs | 66 ++++++++++++++++++++------ crates/bevy_app/src/schedule_runner.rs | 11 +++-- crates/bevy_winit/src/lib.rs | 11 ++--- 3 files changed, 63 insertions(+), 25 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index db43d534ab4ea..3b206ce2deaea 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -76,6 +76,7 @@ pub struct App { plugin_name_added: HashSet, /// A private counter to prevent incorrect calls to `App::run()` from `Plugin::build()` building_plugin_depth: usize, + plugins_state: PluginsState, } 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 struct PlaceholderPlugin; impl Plugin for PlaceholderPlugin { @@ -221,6 +235,7 @@ impl App { plugin_name_added: Default::default(), main_schedule_label: Box::new(Main), 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."); } - if app.ready() { + if app.plugins_state() == PluginsState::Ready { // If we're already ready, we finish up now and advance one frame. // This prevents black frames during the launch transition on iOS. app.finish(); @@ -300,20 +315,25 @@ impl 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`] - pub fn ready(&self) -> bool { - for plugin in &self.plugin_registry { - if !plugin.ready(self) { - return false; + #[inline] + pub fn plugins_state(&self) -> PluginsState { + match self.plugins_state { + 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 - /// plugins are [`App::ready`], but can be useful for situations where you want to use - /// [`App::update`]. + /// plugins are ready, but can be useful for situations where you want to use [`App::update`]. pub fn finish(&mut self) { // temporarily remove the plugin registry to run each plugin's setup function on app. let plugin_registry = std::mem::take(&mut self.plugin_registry); @@ -321,6 +341,7 @@ impl App { plugin.finish(self); } 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 @@ -332,6 +353,7 @@ impl App { plugin.cleanup(self); } self.plugin_registry = plugin_registry; + self.plugins_state = PluginsState::Cleaned; } /// Adds [`State`] and [`NextState`] resources, [`OnEnter`] and [`OnExit`] schedules @@ -696,6 +718,14 @@ impl App { /// [`PluginGroup`]:super::PluginGroup #[track_caller] pub fn add_plugins(&mut self, plugins: impl Plugins) -> &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); self } @@ -947,14 +977,20 @@ impl App { } fn run_once(mut app: App) { - while !app.ready() { - #[cfg(not(target_arch = "wasm32"))] - bevy_tasks::tick_global_task_pools_on_main_thread(); + let plugins_state = app.plugins_state(); + if plugins_state != PluginsState::Cleaned { + 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 diff --git a/crates/bevy_app/src/schedule_runner.rs b/crates/bevy_app/src/schedule_runner.rs index e14687c92ff59..18b2f0b61fb55 100644 --- a/crates/bevy_app/src/schedule_runner.rs +++ b/crates/bevy_app/src/schedule_runner.rs @@ -1,6 +1,7 @@ use crate::{ app::{App, AppExit}, plugin::Plugin, + PluginsState, }; use bevy_ecs::event::{Events, ManualEventReader}; use bevy_utils::{Duration, Instant}; @@ -71,8 +72,9 @@ impl Plugin for ScheduleRunnerPlugin { fn build(&self, app: &mut App) { let run_mode = self.run_mode; app.set_runner(move |mut app: App| { - if !app.ready() { - while !app.ready() { + let plugins_state = app.plugins_state(); + if plugins_state != PluginsState::Cleaned { + while app.plugins_state() == PluginsState::Adding { #[cfg(not(target_arch = "wasm32"))] bevy_tasks::tick_global_task_pools_on_main_thread(); } @@ -83,7 +85,10 @@ impl Plugin for ScheduleRunnerPlugin { let mut app_exit_event_reader = ManualEventReader::::default(); match run_mode { 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 } => { let mut tick = move |app: &mut App, diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 38f7d9b6e6cf1..ad77d95269877 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -20,7 +20,7 @@ use system::{changed_windows, create_windows, despawn_windows, CachedWindow}; pub use winit_config::*; 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::prelude::*; use bevy_ecs::system::{SystemParam, SystemState}; @@ -378,8 +378,6 @@ pub fn winit_runner(mut app: App) { ResMut, )> = SystemState::from_world(&mut app.world); - let mut finished_and_setup_done = app.ready(); - // setup up the event loop let event_handler = move |event: Event<()>, event_loop: &EventLoopWindowTarget<()>, @@ -387,14 +385,13 @@ pub fn winit_runner(mut app: App) { #[cfg(feature = "trace")] let _span = bevy_utils::tracing::info_span!("winit event_handler").entered(); - if !finished_and_setup_done { - if !app.ready() { + if app.plugins_state() != PluginsState::Cleaned { + if app.plugins_state() != PluginsState::Ready { #[cfg(not(target_arch = "wasm32"))] tick_global_task_pools_on_main_thread(); } else { app.finish(); app.cleanup(); - finished_and_setup_done = true; } if let Some(app_exit_events) = app.world.get_resource::>() { @@ -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 runner_state.wait_elapsed = false; runner_state.window_event_received = false;