From de875fdc4ca06fa30440fd13c680f3d18f230987 Mon Sep 17 00:00:00 2001 From: Brezak Date: Mon, 22 Apr 2024 18:48:18 +0200 Subject: [PATCH] Make `AppExit` more specific about exit reason. (#13022) # Objective Closes #13017. ## Solution - Make `AppExit` a enum with a `Success` and `Error` variant. - Make `App::run()` return a `AppExit` if it ever returns. - Make app runners return a `AppExit` to signal if they encountered a error. --- ## Changelog ### Added - [`App::should_exit`](https://example.org/) - [`AppExit`](https://docs.rs/bevy/latest/bevy/app/struct.AppExit.html) to the `bevy` and `bevy_app` preludes, ### Changed - [`AppExit`](https://docs.rs/bevy/latest/bevy/app/struct.AppExit.html) is now a enum with 2 variants (`Success` and `Error`). - The app's [runner function](https://docs.rs/bevy/latest/bevy/app/struct.App.html#method.set_runner) now has to return a `AppExit`. - [`App::run()`](https://docs.rs/bevy/latest/bevy/app/struct.App.html#method.run) now also returns the `AppExit` produced by the runner function. ## Migration Guide - Replace all usages of [`AppExit`](https://docs.rs/bevy/latest/bevy/app/struct.AppExit.html) with `AppExit::Success` or `AppExit::Failure`. - Any custom app runners now need to return a `AppExit`. We suggest you return a `AppExit::Error` if any `AppExit` raised was a Error. You can use the new [`App::should_exit`](https://example.org/) method. - If not exiting from `main` any other way. You should return the `AppExit` from `App::run()` so the app correctly returns a error code if anything fails e.g. ```rust fn main() -> AppExit { App::new() //Your setup here... .run() } ``` --------- Co-authored-by: Alice Cecile --- crates/bevy_app/src/app.rs | 157 ++++++++++++++++-- crates/bevy_app/src/lib.rs | 2 +- crates/bevy_app/src/schedule_runner.rs | 67 +++++--- crates/bevy_dev_tools/src/ci_testing.rs | 2 +- crates/bevy_render/src/pipelined_rendering.rs | 2 +- crates/bevy_window/src/system.rs | 4 +- crates/bevy_winit/src/lib.rs | 38 ++++- examples/app/custom_loop.rs | 23 ++- examples/ecs/ecs_guide.rs | 4 +- examples/games/desk_toy.rs | 2 +- examples/games/game_menu.rs | 2 +- examples/time/time.rs | 5 +- 12 files changed, 248 insertions(+), 60 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 65c8f0196c..3dd2d7b037 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -4,7 +4,7 @@ use crate::{ }; pub use bevy_derive::AppLabel; use bevy_ecs::{ - event::event_update_system, + event::{event_update_system, ManualEventReader}, intern::Interned, prelude::*, schedule::{ScheduleBuildSettings, ScheduleLabel}, @@ -13,8 +13,14 @@ use bevy_ecs::{ #[cfg(feature = "trace")] use bevy_utils::tracing::info_span; use bevy_utils::{tracing::debug, HashMap}; -use std::fmt::Debug; -use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe}; +use std::{ + fmt::Debug, + process::{ExitCode, Termination}, +}; +use std::{ + num::NonZeroU8, + panic::{catch_unwind, resume_unwind, AssertUnwindSafe}, +}; use thiserror::Error; bevy_ecs::define_label!( @@ -151,7 +157,7 @@ impl App { /// # Panics /// /// Panics if not all plugins have been built. - pub fn run(&mut self) { + pub fn run(&mut self) -> AppExit { #[cfg(feature = "trace")] let _bevy_app_run_span = info_span!("bevy_app").entered(); if self.is_building_plugins() { @@ -160,7 +166,7 @@ impl App { let runner = std::mem::replace(&mut self.runner, Box::new(run_once)); let app = std::mem::replace(self, App::empty()); - (runner)(app); + (runner)(app) } /// Sets the function that will be called when the app is run. @@ -177,17 +183,20 @@ impl App { /// ``` /// # use bevy_app::prelude::*; /// # - /// fn my_runner(mut app: App) { + /// fn my_runner(mut app: App) -> AppExit { /// loop { /// println!("In main loop"); /// app.update(); + /// if let Some(exit) = app.should_exit() { + /// return exit; + /// } /// } /// } /// /// App::new() /// .set_runner(my_runner); /// ``` - pub fn set_runner(&mut self, f: impl FnOnce(App) + 'static) -> &mut Self { + pub fn set_runner(&mut self, f: impl FnOnce(App) -> AppExit + 'static) -> &mut Self { self.runner = Box::new(f); self } @@ -849,11 +858,44 @@ impl App { self.main_mut().ignore_ambiguity(schedule, a, b); self } + + /// Attempts to determine if an [`AppExit`] was raised since the last update. + /// + /// Will attempt to return the first [`Error`](AppExit::Error) it encounters. + /// This should be called after every [`update()`](App::update) otherwise you risk + /// dropping possible [`AppExit`] events. + pub fn should_exit(&self) -> Option { + let mut reader = ManualEventReader::default(); + + self.should_exit_manual(&mut reader) + } + + /// Several app runners in this crate keep their own [`ManualEventReader`]. + /// This exists to accommodate them. + pub(crate) fn should_exit_manual( + &self, + reader: &mut ManualEventReader, + ) -> Option { + let events = self.world().get_resource::>()?; + + let mut events = reader.read(events); + + if events.len() != 0 { + return Some( + events + .find(|exit| exit.is_error()) + .cloned() + .unwrap_or(AppExit::Success), + ); + } + + None + } } -type RunnerFn = Box; +type RunnerFn = Box AppExit>; -fn run_once(mut app: App) { +fn run_once(mut app: App) -> AppExit { while app.plugins_state() == PluginsState::Adding { #[cfg(not(target_arch = "wasm32"))] bevy_tasks::tick_global_task_pools_on_main_thread(); @@ -862,27 +904,99 @@ fn run_once(mut app: App) { app.cleanup(); app.update(); + + let mut exit_code_reader = ManualEventReader::default(); + if let Some(app_exit_events) = app.world().get_resource::>() { + if exit_code_reader + .read(app_exit_events) + .any(AppExit::is_error) + { + return AppExit::error(); + } + } + + AppExit::Success } -/// An event that indicates the [`App`] should exit. If one or more of these are present at the -/// end of an update, the [runner](App::set_runner) will end and ([maybe](App::run)) return -/// control to the caller. +/// An event that indicates the [`App`] should exit. If one or more of these are present at the end of an update, +/// the [runner](App::set_runner) will end and ([maybe](App::run)) return control to the caller. /// /// This event can be used to detect when an exit is requested. Make sure that systems listening /// for this event run before the current update ends. -#[derive(Event, Debug, Clone, Default)] -pub struct AppExit; +/// +/// # Portability +/// This type is roughly meant to map to a standard definition of a process exit code (0 means success, not 0 means error). Due to portability concerns +/// (see [`ExitCode`](https://doc.rust-lang.org/std/process/struct.ExitCode.html) and [`process::exit`](https://doc.rust-lang.org/std/process/fn.exit.html#)) +/// we only allow error codes between 1 and [255](u8::MAX). +#[derive(Event, Debug, Clone, Default, PartialEq, Eq)] +pub enum AppExit { + /// [`App`] exited without any problems. + #[default] + Success, + /// The [`App`] experienced an unhandleable error. + /// Holds the exit code we expect our app to return. + Error(NonZeroU8), +} + +impl AppExit { + /// Creates a [`AppExit::Error`] with a error code of 1. + #[must_use] + pub const fn error() -> Self { + Self::Error(NonZeroU8::MIN) + } + + /// Returns `true` if `self` is a [`AppExit::Success`]. + #[must_use] + pub const fn is_success(&self) -> bool { + matches!(self, AppExit::Success) + } + + /// Returns `true` if `self` is a [`AppExit::Error`]. + #[must_use] + pub const fn is_error(&self) -> bool { + matches!(self, AppExit::Error(_)) + } + + /// Creates a [`AppExit`] from a code. + /// + /// When `code` is 0 a [`AppExit::Success`] is constructed otherwise a + /// [`AppExit::Error`] is constructed. + #[must_use] + pub const fn from_code(code: u8) -> Self { + match NonZeroU8::new(code) { + Some(code) => Self::Error(code), + None => Self::Success, + } + } +} + +impl From for AppExit { + #[must_use] + fn from(value: u8) -> Self { + Self::from_code(value) + } +} + +impl Termination for AppExit { + fn report(self) -> std::process::ExitCode { + match self { + AppExit::Success => ExitCode::SUCCESS, + // We leave logging an error to our users + AppExit::Error(value) => ExitCode::from(value.get()), + } + } +} #[cfg(test)] mod tests { - use std::marker::PhantomData; + use std::{marker::PhantomData, mem}; use bevy_ecs::{ schedule::{OnEnter, States}, system::Commands, }; - use crate::{App, Plugin}; + use crate::{App, AppExit, Plugin}; struct PluginA; impl Plugin for PluginA { @@ -1089,13 +1203,15 @@ mod tests { #[derive(Resource)] struct MyState {} - fn my_runner(mut app: App) { + fn my_runner(mut app: App) -> AppExit { let my_state = MyState {}; app.world_mut().insert_resource(my_state); for _ in 0..5 { app.update(); } + + AppExit::Success } fn my_system(_: Res) { @@ -1108,4 +1224,11 @@ mod tests { .add_systems(PreUpdate, my_system) .run(); } + + #[test] + fn app_exit_size() { + // There wont be many of them so the size isn't a issue but + // it's nice they're so small let's keep it that way. + assert_eq!(mem::size_of::(), mem::size_of::()); + } } diff --git a/crates/bevy_app/src/lib.rs b/crates/bevy_app/src/lib.rs index 8d4fbcfc63..e338a45664 100644 --- a/crates/bevy_app/src/lib.rs +++ b/crates/bevy_app/src/lib.rs @@ -28,7 +28,7 @@ pub use sub_app::*; pub mod prelude { #[doc(hidden)] pub use crate::{ - app::App, + app::{App, AppExit}, main_schedule::{ First, FixedFirst, FixedLast, FixedPostUpdate, FixedPreUpdate, FixedUpdate, Last, Main, PostStartup, PostUpdate, PreStartup, PreUpdate, SpawnScene, Startup, StateTransition, diff --git a/crates/bevy_app/src/schedule_runner.rs b/crates/bevy_app/src/schedule_runner.rs index 0e04c51d2b..bbe8cc4476 100644 --- a/crates/bevy_app/src/schedule_runner.rs +++ b/crates/bevy_app/src/schedule_runner.rs @@ -3,7 +3,7 @@ use crate::{ plugin::Plugin, PluginsState, }; -use bevy_ecs::event::{Events, ManualEventReader}; +use bevy_ecs::event::ManualEventReader; use bevy_utils::{Duration, Instant}; #[cfg(target_arch = "wasm32")] @@ -84,7 +84,15 @@ impl Plugin for ScheduleRunnerPlugin { let mut app_exit_event_reader = ManualEventReader::::default(); match run_mode { - RunMode::Once => app.update(), + RunMode::Once => { + app.update(); + + if let Some(exit) = app.should_exit_manual(&mut app_exit_event_reader) { + return exit; + } + + AppExit::Success + } RunMode::Loop { wait } => { let mut tick = move |app: &mut App, wait: Option| @@ -93,14 +101,9 @@ impl Plugin for ScheduleRunnerPlugin { app.update(); - if let Some(app_exit_events) = - app.world_mut().get_resource_mut::>() - { - if let Some(exit) = app_exit_event_reader.read(&app_exit_events).last() - { - return Err(exit.clone()); - } - } + if let Some(exit) = app.should_exit_manual(&mut app_exit_event_reader) { + return Err(exit); + }; let end_time = Instant::now(); @@ -116,40 +119,54 @@ impl Plugin for ScheduleRunnerPlugin { #[cfg(not(target_arch = "wasm32"))] { - while let Ok(delay) = tick(&mut app, wait) { - if let Some(delay) = delay { - std::thread::sleep(delay); + loop { + match tick(&mut app, wait) { + Ok(Some(delay)) => std::thread::sleep(delay), + Ok(None) => continue, + Err(exit) => return exit, } } } #[cfg(target_arch = "wasm32")] { - fn set_timeout(f: &Closure, dur: Duration) { + fn set_timeout(callback: &Closure, dur: Duration) { web_sys::window() .unwrap() .set_timeout_with_callback_and_timeout_and_arguments_0( - f.as_ref().unchecked_ref(), + callback.as_ref().unchecked_ref(), dur.as_millis() as i32, ) .expect("Should register `setTimeout`."); } let asap = Duration::from_millis(1); - let mut rc = Rc::new(app); - let f = Rc::new(RefCell::new(None)); - let g = f.clone(); + let exit = Rc::new(RefCell::new(AppExit::Success)); + let closure_exit = exit.clone(); - let c = move || { - let app = Rc::get_mut(&mut rc).unwrap(); + let mut app = Rc::new(app); + let moved_tick_closure = Rc::new(RefCell::new(None)); + let base_tick_closure = moved_tick_closure.clone(); + + let tick_app = move || { + let app = Rc::get_mut(&mut app).unwrap(); let delay = tick(app, wait); - if let Ok(delay) = delay { - set_timeout(f.borrow().as_ref().unwrap(), delay.unwrap_or(asap)); + match delay { + Ok(delay) => set_timeout( + moved_tick_closure.borrow().as_ref().unwrap(), + delay.unwrap_or(asap), + ), + Err(code) => { + closure_exit.replace(code); + } } }; - *g.borrow_mut() = Some(Closure::wrap(Box::new(c) as Box)); - set_timeout(g.borrow().as_ref().unwrap(), asap); - }; + *base_tick_closure.borrow_mut() = + Some(Closure::wrap(Box::new(tick_app) as Box)); + set_timeout(base_tick_closure.borrow().as_ref().unwrap(), asap); + + exit.take() + } } } }); diff --git a/crates/bevy_dev_tools/src/ci_testing.rs b/crates/bevy_dev_tools/src/ci_testing.rs index 88dc2fc48d..7a8bd8ecb8 100644 --- a/crates/bevy_dev_tools/src/ci_testing.rs +++ b/crates/bevy_dev_tools/src/ci_testing.rs @@ -37,7 +37,7 @@ fn ci_testing_exit_after( ) { if let Some(exit_after) = ci_testing_config.exit_after { if *current_frame > exit_after { - app_exit_events.send(AppExit); + app_exit_events.send(AppExit::Success); info!("Exiting after {} frames. Test successful!", exit_after); } } diff --git a/crates/bevy_render/src/pipelined_rendering.rs b/crates/bevy_render/src/pipelined_rendering.rs index d1a7d33081..d0f31ad605 100644 --- a/crates/bevy_render/src/pipelined_rendering.rs +++ b/crates/bevy_render/src/pipelined_rendering.rs @@ -193,7 +193,7 @@ fn renderer_extract(app_world: &mut World, _world: &mut World) { render_channels.send_blocking(render_app); } else { // Renderer thread panicked - world.send_event(AppExit); + world.send_event(AppExit::error()); } }); }); diff --git a/crates/bevy_window/src/system.rs b/crates/bevy_window/src/system.rs index 690ff63a77..ac92c77408 100644 --- a/crates/bevy_window/src/system.rs +++ b/crates/bevy_window/src/system.rs @@ -13,7 +13,7 @@ use bevy_ecs::prelude::*; pub fn exit_on_all_closed(mut app_exit_events: EventWriter, windows: Query<&Window>) { if windows.is_empty() { bevy_utils::tracing::info!("No windows are open, exiting"); - app_exit_events.send(AppExit); + app_exit_events.send(AppExit::Success); } } @@ -28,7 +28,7 @@ pub fn exit_on_primary_closed( ) { if windows.is_empty() { bevy_utils::tracing::info!("Primary window was closed, exiting"); - app_exit_events.send(AppExit); + app_exit_events.send(AppExit::Success); } } diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 9de6073649..db66f81ea1 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -19,6 +19,8 @@ mod winit_config; pub mod winit_event; mod winit_windows; +use std::sync::{Arc, Mutex}; + use approx::relative_eq; use bevy_a11y::AccessibilityRequested; use bevy_utils::Instant; @@ -263,7 +265,7 @@ type UserEvent = RequestRedraw; /// /// Overriding the app's [runner](bevy_app::App::runner) while using `WinitPlugin` will bypass the /// `EventLoop`. -pub fn winit_runner(mut app: App) { +pub fn winit_runner(mut app: App) -> AppExit { if app.plugins_state() == PluginsState::Ready { app.finish(); app.cleanup(); @@ -279,6 +281,10 @@ pub fn winit_runner(mut app: App) { let mut runner_state = WinitAppRunnerState::default(); + // TODO: AppExit is effectively a u8 we could use a AtomicU8 here instead of a mutex. + let mut exit_status = Arc::new(Mutex::new(AppExit::Success)); + let handle_exit_status = exit_status.clone(); + // prepare structures to access data in the world let mut app_exit_event_reader = ManualEventReader::::default(); let mut redraw_event_reader = ManualEventReader::::default(); @@ -298,6 +304,8 @@ pub fn winit_runner(mut app: App) { let mut winit_events = Vec::default(); // set up the event loop let event_handler = move |event, event_loop: &EventLoopWindowTarget| { + let mut exit_status = handle_exit_status.lock().unwrap(); + handle_winit_event( &mut app, &mut app_exit_event_reader, @@ -307,6 +315,7 @@ pub fn winit_runner(mut app: App) { &mut focused_windows_state, &mut redraw_event_reader, &mut winit_events, + &mut exit_status, event, event_loop, ); @@ -317,6 +326,12 @@ pub fn winit_runner(mut app: App) { if let Err(err) = event_loop.run(event_handler) { error!("winit event loop returned an error: {err}"); } + + // We should be the only ones holding this `Arc` since the event loop exiting cleanly + // should drop the event handler. if this is not the case something funky is happening. + Arc::get_mut(&mut exit_status) + .map(|mutex| mutex.get_mut().unwrap().clone()) + .unwrap_or(AppExit::error()) } #[allow(clippy::too_many_arguments /* TODO: probs can reduce # of args */)] @@ -334,6 +349,7 @@ fn handle_winit_event( focused_windows_state: &mut SystemState<(Res, Query<&Window>)>, redraw_event_reader: &mut ManualEventReader, winit_events: &mut Vec, + exit_status: &mut AppExit, event: Event, event_loop: &EventLoopWindowTarget, ) { @@ -350,8 +366,14 @@ fn handle_winit_event( } runner_state.redraw_requested = true; + // TODO: Replace with `App::should_exit()` if let Some(app_exit_events) = app.world().get_resource::>() { - if app_exit_event_reader.read(app_exit_events).last().is_some() { + let mut exit_events = app_exit_event_reader.read(app_exit_events); + if exit_events.len() != 0 { + *exit_status = exit_events + .find(|exit| exit.is_error()) + .cloned() + .unwrap_or(AppExit::Success); event_loop.exit(); return; } @@ -411,6 +433,7 @@ fn handle_winit_event( app_exit_event_reader, redraw_event_reader, winit_events, + exit_status, ); if runner_state.active != ActiveState::Suspended { event_loop.set_control_flow(ControlFlow::Poll); @@ -638,6 +661,7 @@ fn handle_winit_event( app_exit_event_reader, redraw_event_reader, winit_events, + exit_status, ); } _ => {} @@ -738,6 +762,7 @@ fn run_app_update_if_should( app_exit_event_reader: &mut ManualEventReader, redraw_event_reader: &mut ManualEventReader, winit_events: &mut Vec, + exit_status: &mut AppExit, ) { runner_state.reset_on_update(); @@ -797,9 +822,16 @@ fn run_app_update_if_should( } } + // TODO: Replace with `App::should_exit()` if let Some(app_exit_events) = app.world().get_resource::>() { - if app_exit_event_reader.read(app_exit_events).last().is_some() { + let mut exit_events = app_exit_event_reader.read(app_exit_events); + if exit_events.len() != 0 { + *exit_status = exit_events + .find(|exit| exit.is_error()) + .cloned() + .unwrap_or(AppExit::Success); event_loop.exit(); + return; } } } diff --git a/examples/app/custom_loop.rs b/examples/app/custom_loop.rs index 3d76bed2df..61796d1c57 100644 --- a/examples/app/custom_loop.rs +++ b/examples/app/custom_loop.rs @@ -1,13 +1,13 @@ //! This example demonstrates you can create a custom runner (to update an app manually). It reads //! lines from stdin and prints them from within the ecs. -use bevy::prelude::*; +use bevy::{app::AppExit, prelude::*}; use std::io; #[derive(Resource)] struct Input(String); -fn my_runner(mut app: App) { +fn my_runner(mut app: App) -> AppExit { println!("Type stuff into the console"); for line in io::stdin().lines() { { @@ -15,17 +15,30 @@ fn my_runner(mut app: App) { input.0 = line.unwrap(); } app.update(); + + if let Some(exit) = app.should_exit() { + return exit; + } } + + AppExit::Success } fn print_system(input: Res) { println!("You typed: {}", input.0); } -fn main() { +fn exit_system(input: Res, mut exit_event: EventWriter) { + if input.0 == "exit" { + exit_event.send(AppExit::Success); + } +} + +// AppExit implements `Termination` so we can return it from main. +fn main() -> AppExit { App::new() .insert_resource(Input(String::new())) .set_runner(my_runner) - .add_systems(Update, print_system) - .run(); + .add_systems(Update, (print_system, exit_system)) + .run() } diff --git a/examples/ecs/ecs_guide.rs b/examples/ecs/ecs_guide.rs index e0836b8632..be77af1cc8 100644 --- a/examples/ecs/ecs_guide.rs +++ b/examples/ecs/ecs_guide.rs @@ -130,10 +130,10 @@ fn game_over_system( ) { if let Some(ref player) = game_state.winning_player { println!("{player} won the game!"); - app_exit_events.send(AppExit); + app_exit_events.send(AppExit::Success); } else if game_state.current_round == game_rules.max_rounds { println!("Ran out of rounds. Nobody wins!"); - app_exit_events.send(AppExit); + app_exit_events.send(AppExit::Success); } } diff --git a/examples/games/desk_toy.rs b/examples/games/desk_toy.rs index c79eacd7a9..47520aa10a 100644 --- a/examples/games/desk_toy.rs +++ b/examples/games/desk_toy.rs @@ -334,7 +334,7 @@ fn quit( .distance(cursor_world_pos) < BEVY_LOGO_RADIUS { - app_exit.send(AppExit); + app_exit.send(AppExit::Success); } } diff --git a/examples/games/game_menu.rs b/examples/games/game_menu.rs index b6f1f533de..60d8d5e349 100644 --- a/examples/games/game_menu.rs +++ b/examples/games/game_menu.rs @@ -789,7 +789,7 @@ mod menu { if *interaction == Interaction::Pressed { match menu_button_action { MenuButtonAction::Quit => { - app_exit_events.send(AppExit); + app_exit_events.send(AppExit::Success); } MenuButtonAction::Play => { game_state.set(GameState::Game); diff --git a/examples/time/time.rs b/examples/time/time.rs index c473f9fa21..bf8ebfc9ed 100644 --- a/examples/time/time.rs +++ b/examples/time/time.rs @@ -1,5 +1,6 @@ //! An example that illustrates how Time is handled in ECS. +use bevy::app::AppExit; use bevy::prelude::*; use std::io::{self, BufRead}; @@ -30,7 +31,7 @@ fn help() { println!(" u: Unpause"); } -fn runner(mut app: App) { +fn runner(mut app: App) -> AppExit { banner(); help(); let stdin = io::stdin(); @@ -78,6 +79,8 @@ fn runner(mut app: App) { } } } + + AppExit::Success } fn print_real_time(time: Res>) {