From 5aab12f0b6516033aa4480594f0c93c3dba551d3 Mon Sep 17 00:00:00 2001 From: B_head Date: Tue, 27 Jun 2023 06:58:13 +0900 Subject: [PATCH] Replace `App::update()` to `World::run_schedule()` Moved the responsibility for running top-level schedules from `App` struct to the runner function. --- crates/bevy_app/src/app.rs | 54 +++++++++---------- crates/bevy_app/src/schedule_runner.rs | 46 ++++++++++++++-- crates/bevy_asset/src/asset_server.rs | 10 ++-- crates/bevy_asset/src/debug_asset_server.rs | 4 +- crates/bevy_core/src/lib.rs | 2 +- crates/bevy_render/src/lib.rs | 13 ++--- crates/bevy_render/src/view/visibility/mod.rs | 4 +- crates/bevy_transform/src/systems.rs | 6 +-- crates/bevy_winit/src/lib.rs | 33 +++++++++++- crates/bevy_winit/src/winit_config.rs | 13 ++++- examples/app/custom_loop.rs | 2 +- tests/how_to_test_systems.rs | 10 ++-- 12 files changed, 136 insertions(+), 61 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 6cf2df68eacc2f..dbd63ff2dff60e 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -64,13 +64,12 @@ pub struct App { /// the application's event loop and advancing the [`Schedule`]. /// Typically, it is not configured manually, but set by one of Bevy's built-in plugins. /// See `bevy::winit::WinitPlugin` and [`ScheduleRunnerPlugin`](crate::schedule_runner::ScheduleRunnerPlugin). - pub runner: Box, - /// The schedule that systems are added to by default. /// - /// The schedule that runs the main loop of schedule execution. + /// # Note /// - /// This is initially set to [`Main`]. - pub main_schedule_label: BoxedScheduleLabel, + /// Inside the runner function, `World::clear_trackers()` must be called periodically. + /// If that isn't called on a world, it may lead to memory leaks in `RemovedComponents`. + pub runner: Box, sub_apps: HashMap, plugin_registry: Vec>, plugin_name_added: HashSet, @@ -135,6 +134,11 @@ pub struct SubApp { /// The [`SubApp`]'s instance of [`App`] pub app: App, + /// The schedule to run by default. + /// + /// This is initially set to [`Main`]. + pub main_schedule_label: BoxedScheduleLabel, + /// A function that allows access to both the main [`App`] [`World`] and the [`SubApp`]. This is /// useful for moving data between the sub app and the main app. extract: Box, @@ -150,13 +154,14 @@ impl SubApp { pub fn new(app: App, extract: impl Fn(&mut World, &mut App) + Send + Sync + 'static) -> Self { Self { app, + main_schedule_label: Box::new(Main), extract: Box::new(extract), } } /// Runs the [`SubApp`]'s default schedule. pub fn run(&mut self) { - self.app.world.run_schedule(&*self.app.main_schedule_label); + self.app.world.run_schedule(&*self.main_schedule_label); self.app.world.clear_trackers(); } @@ -219,38 +224,19 @@ impl App { sub_apps: HashMap::default(), plugin_registry: Vec::default(), plugin_name_added: Default::default(), - main_schedule_label: Box::new(Main), building_plugin_depth: 0, } } - /// Advances the execution of the [`Schedule`] by one cycle. - /// - /// This method also updates sub apps. + /// Update sub apps. /// See [`insert_sub_app`](Self::insert_sub_app) for more details. - /// - /// The schedule run by this method is determined by the [`main_schedule_label`](App) field. - /// By default this is [`Main`]. - /// - /// # Panics - /// - /// The active schedule of the app must be set before this method is called. - pub fn update(&mut self) { - #[cfg(feature = "trace")] - let _bevy_update_span = info_span!("update").entered(); - { - #[cfg(feature = "trace")] - let _bevy_main_update_span = info_span!("main app").entered(); - self.world.run_schedule(&*self.main_schedule_label); - } + pub fn update_sub_apps(&mut self) { for (_label, sub_app) in self.sub_apps.iter_mut() { #[cfg(feature = "trace")] let _sub_app_span = info_span!("sub app", name = ?_label).entered(); sub_app.extract(&mut self.world); sub_app.run(); } - - self.world.clear_trackers(); } /// Starts the application by calling the app's [runner function](Self::set_runner). @@ -636,6 +622,11 @@ impl App { /// The runner function is usually not set manually, but by Bevy integrated plugins /// (e.g. `WinitPlugin`). /// + /// # Note + /// + /// Inside the runner function, `World::clear_trackers()` must be called periodically. + /// If that isn't called on a world, it may lead to memory leaks in `RemovedComponents`. + /// /// # Examples /// /// ``` @@ -644,7 +635,7 @@ impl App { /// fn my_runner(mut app: App) { /// loop { /// println!("In main loop"); - /// app.update(); + /// app.world.run_schedule(Main); /// } /// } /// @@ -970,7 +961,12 @@ fn run_once(mut app: App) { app.finish(); app.cleanup(); - app.update(); + { + #[cfg(feature = "trace")] + let _main_schedule_span = info_span!("main schedule", name = ?Main).entered(); + app.world.run_schedule(Main); + } + app.update_sub_apps(); } /// 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 c143f75c42f532..0faabde8b77dec 100644 --- a/crates/bevy_app/src/schedule_runner.rs +++ b/crates/bevy_app/src/schedule_runner.rs @@ -1,10 +1,17 @@ use crate::{ app::{App, AppExit}, plugin::Plugin, + Main, +}; +use bevy_ecs::{ + event::{Events, ManualEventReader}, + schedule::BoxedScheduleLabel, }; -use bevy_ecs::event::{Events, ManualEventReader}; use bevy_utils::{Duration, Instant}; +#[cfg(feature = "trace")] +use bevy_utils::tracing::info_span; + #[cfg(target_arch = "wasm32")] use std::{cell::RefCell, rc::Rc}; #[cfg(target_arch = "wasm32")] @@ -43,10 +50,13 @@ impl Default for RunMode { /// typically, the `winit` event loop /// (see [`WinitPlugin`](https://docs.rs/bevy/latest/bevy/winit/struct.WinitPlugin.html)) /// executes the schedule making [`ScheduleRunnerPlugin`] unnecessary. -#[derive(Default)] pub struct ScheduleRunnerPlugin { /// Determines whether the [`Schedule`](bevy_ecs::schedule::Schedule) is run once or repeatedly. pub run_mode: RunMode, + /// The schedule to run by default. + /// + /// This is initially set to [`Main`]. + pub main_schedule_label: BoxedScheduleLabel, } impl ScheduleRunnerPlugin { @@ -54,6 +64,7 @@ impl ScheduleRunnerPlugin { pub fn run_once() -> Self { ScheduleRunnerPlugin { run_mode: RunMode::Once, + main_schedule_label: Box::new(Main), } } @@ -63,6 +74,16 @@ impl ScheduleRunnerPlugin { run_mode: RunMode::Loop { wait: Some(wait_duration), }, + main_schedule_label: Box::new(Main), + } + } +} + +impl Default for ScheduleRunnerPlugin { + fn default() -> Self { + ScheduleRunnerPlugin { + run_mode: RunMode::Loop { wait: None }, + main_schedule_label: Box::new(Main), } } } @@ -70,11 +91,21 @@ impl ScheduleRunnerPlugin { impl Plugin for ScheduleRunnerPlugin { fn build(&self, app: &mut App) { let run_mode = self.run_mode; + let main_schedule_label = self.main_schedule_label.clone(); app.set_runner(move |mut app: App| { + // Prevent panic when schedules do not exist + app.init_schedule(main_schedule_label.clone()); + let mut app_exit_event_reader = ManualEventReader::::default(); match run_mode { RunMode::Once => { - app.update(); + { + #[cfg(feature = "trace")] + let _main_schedule_span = + info_span!("main schedule", name = ?main_schedule_label).entered(); + app.world.run_schedule(main_schedule_label); + } + app.update_sub_apps(); } RunMode::Loop { wait } => { let mut tick = move |app: &mut App, @@ -91,7 +122,14 @@ impl Plugin for ScheduleRunnerPlugin { } } - app.update(); + { + #[cfg(feature = "trace")] + let _main_schedule_span = + info_span!("main schedule", name = ?main_schedule_label).entered(); + app.world.run_schedule(&main_schedule_label); + } + app.update_sub_apps(); + app.world.clear_trackers(); if let Some(app_exit_events) = app.world.get_resource_mut::>() diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index d97f206bbdc36e..12b843424c68a5 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -645,7 +645,7 @@ pub fn free_unused_assets_system(asset_server: Res) { mod test { use super::*; use crate::{loader::LoadedAsset, update_asset_storage_system}; - use bevy_app::{App, Update}; + use bevy_app::{App, Main, Update}; use bevy_ecs::prelude::*; use bevy_reflect::{TypePath, TypeUuid}; use bevy_utils::BoxedFuture; @@ -896,19 +896,19 @@ mod test { // asset is loading assert_eq!(LoadState::Loading, get_load_state(&handle, &app.world)); - app.update(); + app.world.run_schedule(Main); // asset should exist and be loaded at this point assert_eq!(LoadState::Loaded, get_load_state(&handle, &app.world)); assert!(get_asset(&handle, &app.world).is_some()); // after dropping the handle, next call to `tick` will prepare the assets for removal. drop(handle); - app.update(); + app.world.run_schedule(Main); assert_eq!(LoadState::Loaded, get_load_state(&weak_handle, &app.world)); assert!(get_asset(&weak_handle, &app.world).is_some()); // second call to tick will actually remove the asset. - app.update(); + app.world.run_schedule(Main); assert_eq!( LoadState::Unloaded, get_load_state(&weak_handle, &app.world) @@ -918,7 +918,7 @@ mod test { // finally, reload the asset let handle = load_asset(path.clone(), &app.world).typed(); assert_eq!(LoadState::Loading, get_load_state(&handle, &app.world)); - app.update(); + app.world.run_schedule(Main); assert_eq!(LoadState::Loaded, get_load_state(&handle, &app.world)); assert!(get_asset(&handle, &app.world).is_some()); } diff --git a/crates/bevy_asset/src/debug_asset_server.rs b/crates/bevy_asset/src/debug_asset_server.rs index 6c2e3d4815102a..94330b7192ccb9 100644 --- a/crates/bevy_asset/src/debug_asset_server.rs +++ b/crates/bevy_asset/src/debug_asset_server.rs @@ -2,7 +2,7 @@ //! //! Internal assets (e.g. shaders) are bundled directly into an application and can't be hot //! reloaded using the conventional API. -use bevy_app::{App, Plugin, Update}; +use bevy_app::{App, Main, Plugin, Update}; use bevy_ecs::{prelude::*, system::SystemState}; use bevy_tasks::{IoTaskPool, TaskPoolBuilder}; use bevy_utils::{Duration, HashMap}; @@ -81,7 +81,7 @@ impl Plugin for DebugAssetServerPlugin { } fn run_debug_asset_app(mut debug_asset_app: NonSendMut) { - debug_asset_app.0.update(); + debug_asset_app.0.world.run_schedule(Main); } pub(crate) fn sync_debug_assets( diff --git a/crates/bevy_core/src/lib.rs b/crates/bevy_core/src/lib.rs index a9b10b1e81f2b3..709a0173e59a1a 100644 --- a/crates/bevy_core/src/lib.rs +++ b/crates/bevy_core/src/lib.rs @@ -203,7 +203,7 @@ mod tests { TypeRegistrationPlugin::default(), FrameCountPlugin::default(), )); - app.update(); + app.world.run_schedule(Main); let frame_count = app.world.resource::(); assert_eq!(1, frame_count.0); diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index 8df96b72922590..e2e39c248edd6d 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -266,7 +266,6 @@ impl Plugin for RenderPlugin { app.init_resource::(); let mut render_app = App::empty(); - render_app.main_schedule_label = Box::new(Render); let mut extract_schedule = Schedule::new(); extract_schedule.set_apply_final_deferred(false); @@ -296,14 +295,14 @@ impl Plugin for RenderPlugin { app.insert_resource(receiver); render_app.insert_resource(sender); - app.insert_sub_app(RenderApp, SubApp::new(render_app, move |main_world, render_app| { + let mut sub_app = SubApp::new(render_app, move |main_world, render_app| { #[cfg(feature = "trace")] - let _render_span = bevy_utils::tracing::info_span!("extract main app to render subapp").entered(); + let _render_span = + bevy_utils::tracing::info_span!("extract main app to render subapp").entered(); { #[cfg(feature = "trace")] let _stage_span = - bevy_utils::tracing::info_span!("reserve_and_flush") - .entered(); + bevy_utils::tracing::info_span!("reserve_and_flush").entered(); // reserve all existing main world entities for use in render_app // they can only be spawned using `get_or_spawn()` @@ -326,7 +325,9 @@ impl Plugin for RenderPlugin { // run extract schedule extract(main_world, render_app); - })); + }); + sub_app.main_schedule_label = Box::new(Render); + app.insert_sub_app(RenderApp, sub_app); } app.add_plugins(( diff --git a/crates/bevy_render/src/view/visibility/mod.rs b/crates/bevy_render/src/view/visibility/mod.rs index 4c72a5abb5f05e..b28c84c7758419 100644 --- a/crates/bevy_render/src/view/visibility/mod.rs +++ b/crates/bevy_render/src/view/visibility/mod.rs @@ -516,7 +516,7 @@ mod test { .entity_mut(root2_child2) .push_children(&[root2_child2_grandchild1]); - app.update(); + app.world.run_schedule(Main); let is_visible = |e: Entity| { app.world @@ -613,7 +613,7 @@ mod test { .entity_mut(root1_child2) .push_children(&[root1_child2_grandchild1]); - app.update(); + app.world.run_schedule(Main); let is_visible = |e: Entity| { app.world diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs index 46629eca57db9b..ca9a9edb913a67 100644 --- a/crates/bevy_transform/src/systems.rs +++ b/crates/bevy_transform/src/systems.rs @@ -424,13 +424,13 @@ mod test { }) .id(); - app.update(); + app.world.run_schedule(Main); // check the `Children` structure is spawned assert_eq!(&**app.world.get::(parent).unwrap(), &[child]); assert_eq!(&**app.world.get::(child).unwrap(), &[grandchild]); // Note that at this point, the `GlobalTransform`s will not have updated yet, due to `Commands` delay - app.update(); + app.world.run_schedule(Main); let mut state = app.world.query::<&GlobalTransform>(); for global in state.iter(&app.world) { @@ -474,6 +474,6 @@ mod test { &mut *temp.get_mut::(grandchild).unwrap(), ); - app.update(); + app.world.run_schedule(Main); } } diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 15f9254a5ad877..0dc91146299cf6 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -24,7 +24,7 @@ use system::{changed_window, create_window, despawn_window, CachedWindow}; pub use winit_config::*; pub use winit_windows::*; -use bevy_app::{App, AppExit, Last, Main, Plugin, Render}; +use bevy_app::{App, AppExit, Last, Plugin}; use bevy_ecs::event::{Events, ManualEventReader}; use bevy_ecs::prelude::*; use bevy_input::{ @@ -45,6 +45,9 @@ use bevy_window::{ WindowScaleFactorChanged, WindowThemeChanged, }; +#[cfg(feature = "trace")] +use bevy_utils::tracing::info_span; + #[cfg(target_os = "android")] pub use winit::platform::android::activity::AndroidApp; @@ -335,6 +338,16 @@ pub fn winit_runner(mut app: App) { ResMut, )> = SystemState::from_world(&mut app.world); + { + let (winit_config, _) = focused_window_state.get(&app.world); + let main_schedule_label = winit_config.main_schedule_label.clone(); + let render_schedule_label = winit_config.render_schedule_label.clone(); + + // Prevent panic when schedules do not exist + app.init_schedule(main_schedule_label); + app.init_schedule(render_schedule_label); + } + let mut finished_and_setup_done = false; let event_handler = move |event: Event<()>, @@ -690,7 +703,23 @@ pub fn winit_runner(mut app: App) { if update && finished_and_setup_done { winit_state.last_update = Instant::now(); - app.update(); + let main_schedule_label = winit_config.main_schedule_label.clone(); + let render_schedule_label = winit_config.render_schedule_label.clone(); + + { + #[cfg(feature = "trace")] + let _main_schedule_span = + info_span!("main schedule", name = ?main_schedule_label).entered(); + app.world.run_schedule(main_schedule_label); + } + { + #[cfg(feature = "trace")] + let _render_schedule_span = + info_span!("render schedule", name = ?render_schedule_label).entered(); + app.world.run_schedule(render_schedule_label); + } + app.update_sub_apps(); + app.world.clear_trackers(); } } Event::RedrawEventsCleared => { diff --git a/crates/bevy_winit/src/winit_config.rs b/crates/bevy_winit/src/winit_config.rs index ec2ff83127488c..cae89efa981df5 100644 --- a/crates/bevy_winit/src/winit_config.rs +++ b/crates/bevy_winit/src/winit_config.rs @@ -1,4 +1,5 @@ -use bevy_ecs::system::Resource; +use bevy_app::{Main, Render}; +use bevy_ecs::{schedule::BoxedScheduleLabel, system::Resource}; use bevy_utils::Duration; /// A resource for configuring usage of the [`winit`] library. @@ -31,6 +32,14 @@ pub struct WinitSettings { pub focused_mode: UpdateMode, /// Configures how the winit event loop updates while the window is *not* focused. pub unfocused_mode: UpdateMode, + /// The main schedule to run by default. + /// + /// This is initially set to [`Main`]. + pub main_schedule_label: BoxedScheduleLabel, + /// The render schedule to run by default. + /// + /// This is initially set to [`Render`]. + pub render_schedule_label: BoxedScheduleLabel, } impl WinitSettings { /// Configure winit with common settings for a game. @@ -65,6 +74,8 @@ impl Default for WinitSettings { return_from_run: false, focused_mode: UpdateMode::Continuous, unfocused_mode: UpdateMode::Continuous, + main_schedule_label: Box::new(Main), + render_schedule_label: Box::new(Render), } } } diff --git a/examples/app/custom_loop.rs b/examples/app/custom_loop.rs index 17f70ae9f6d389..597befdd46c04d 100644 --- a/examples/app/custom_loop.rs +++ b/examples/app/custom_loop.rs @@ -14,7 +14,7 @@ fn my_runner(mut app: App) { let mut input = app.world.resource_mut::(); input.0 = line.unwrap(); } - app.update(); + app.world.run_schedule(Main); } } diff --git a/tests/how_to_test_systems.rs b/tests/how_to_test_systems.rs index 192aff7cd2f397..3b65ac400b06e2 100644 --- a/tests/how_to_test_systems.rs +++ b/tests/how_to_test_systems.rs @@ -70,7 +70,7 @@ fn did_hurt_enemy() { .id(); // Run systems - app.update(); + app.world.run_schedule(Main); // Check resulting changes assert!(app.world.get::(enemy_id).is_some()); @@ -101,7 +101,7 @@ fn did_despawn_enemy() { .id(); // Run systems - app.update(); + app.world.run_schedule(Main); // Check enemy was despawned assert!(app.world.get::(enemy_id).is_none()); @@ -129,7 +129,7 @@ fn spawn_enemy_using_input_resource() { app.insert_resource(input); // Run systems - app.update(); + app.world.run_schedule(Main); // Check resulting changes, one entity has been spawned with `Enemy` component assert_eq!(app.world.query::<&Enemy>().iter(&app.world).len(), 1); @@ -138,7 +138,7 @@ fn spawn_enemy_using_input_resource() { app.world.resource_mut::>().clear(); // Run systems - app.update(); + app.world.run_schedule(Main); // Check resulting changes, no new entity has been spawned assert_eq!(app.world.query::<&Enemy>().iter(&app.world).len(), 1); @@ -164,7 +164,7 @@ fn update_score_on_event() { .send(EnemyDied(3)); // Run systems - app.update(); + app.world.run_schedule(Main); // Check resulting changes assert_eq!(app.world.resource::().0, 3);