Skip to content

Commit

Permalink
fix run-once runners (#10195)
Browse files Browse the repository at this point in the history
# 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
  • Loading branch information
mockersf authored Oct 23, 2023
1 parent b28525b commit 8fb5c99
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 25 deletions.
66 changes: 51 additions & 15 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ pub struct App {
plugin_name_added: HashSet<String>,
/// A private counter to prevent incorrect calls to `App::run()` from `Plugin::build()`
building_plugin_depth: usize,
plugins_state: PluginsState,
}

impl Debug for App {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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();
Expand All @@ -300,27 +315,33 @@ 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);
for plugin in &plugin_registry {
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
Expand All @@ -332,6 +353,7 @@ impl App {
plugin.cleanup(self);
}
self.plugin_registry = plugin_registry;
self.plugins_state = PluginsState::Cleaned;
}

/// Adds [`State<S>`] and [`NextState<S>`] resources, [`OnEnter`] and [`OnExit`] schedules
Expand Down Expand Up @@ -696,6 +718,14 @@ impl App {
/// [`PluginGroup`]:super::PluginGroup
#[track_caller]
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);
self
}
Expand Down Expand Up @@ -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
Expand Down
11 changes: 8 additions & 3 deletions crates/bevy_app/src/schedule_runner.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{
app::{App, AppExit},
plugin::Plugin,
PluginsState,
};
use bevy_ecs::event::{Events, ManualEventReader};
use bevy_utils::{Duration, Instant};
Expand Down Expand Up @@ -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();
}
Expand All @@ -83,7 +85,10 @@ impl Plugin for ScheduleRunnerPlugin {
let mut app_exit_event_reader = ManualEventReader::<AppExit>::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,
Expand Down
11 changes: 4 additions & 7 deletions crates/bevy_winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -378,23 +378,20 @@ pub fn winit_runner(mut app: App) {
ResMut<CanvasParentResizeEventChannel>,
)> = 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<()>,
control_flow: &mut ControlFlow| {
#[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::<Events<AppExit>>() {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 8fb5c99

Please sign in to comment.