From 21131068f515535514872971b341050c48692414 Mon Sep 17 00:00:00 2001 From: Chris Juchem Date: Sat, 8 Jun 2024 19:21:00 -0400 Subject: [PATCH 1/8] Remove extra call to clear_trackers --- crates/bevy_app/src/sub_app.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index 472490e398830..4e074d035c372 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -429,8 +429,6 @@ impl SubApps { sub_app.extract(&mut self.main.world); sub_app.update(); } - - self.main.world.clear_trackers(); } /// Returns an iterator over the sub-apps (starting with the main one). From 8bb57f59b4bb3e78b0717ab02890b38aac20052f Mon Sep 17 00:00:00 2001 From: Chris Juchem Date: Sat, 8 Jun 2024 22:56:06 -0400 Subject: [PATCH 2/8] Added unit test --- crates/bevy_app/src/app.rs | 41 +++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 2e5ba17a0edbc..7476c93585884 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -916,9 +916,17 @@ impl Termination for AppExit { #[cfg(test)] mod tests { - use std::{marker::PhantomData, mem}; - - use bevy_ecs::{event::EventWriter, schedule::ScheduleLabel, system::Commands}; + use std::{iter, marker::PhantomData, mem}; + + use bevy_ecs::{ + component::Component, + entity::Entity, + event::EventWriter, + query::With, + removal_detection::RemovedComponents, + schedule::{IntoSystemConfigs, ScheduleLabel}, + system::{Commands, Query}, + }; use crate::{App, AppExit, Plugin, Update}; @@ -1123,6 +1131,33 @@ mod tests { ); } + #[test] + fn test_update_clears_trackers_once() { + #[derive(Component, Copy, Clone)] + struct Foo; + + let mut app = App::new(); + app.world_mut().spawn_batch(iter::repeat(Foo).take(5)); + + fn despawn_one_foo(mut commands: Commands, foos: Query>) { + foos.iter().next().map(|e| commands.entity(e).despawn()); + } + fn check_despawns(mut removed_foos: RemovedComponents) { + let mut despawn_count = 0; + for _ in removed_foos.read() { + despawn_count += 1; + } + + assert_eq!(despawn_count, 2); + } + + app.add_systems(Update, despawn_one_foo); + app.update(); // Frame 0 + app.update(); // Frame 1 + app.add_systems(Update, check_despawns.after(despawn_one_foo)); + app.update(); // Should see despawns from frames 1 & 2, but not frame 0 + } + #[test] fn runner_returns_correct_exit_code() { fn raise_exits(mut exits: EventWriter) { From d607f611e452358a9b283435d2e2584c62487ffe Mon Sep 17 00:00:00 2001 From: Chris Juchem Date: Sat, 8 Jun 2024 23:05:33 -0400 Subject: [PATCH 3/8] Clippy --- crates/bevy_app/src/app.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 7476c93585884..ef4e4462754d7 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -1140,7 +1140,9 @@ mod tests { app.world_mut().spawn_batch(iter::repeat(Foo).take(5)); fn despawn_one_foo(mut commands: Commands, foos: Query>) { - foos.iter().next().map(|e| commands.entity(e).despawn()); + if let Some(e) = foos.iter().next() { + commands.entity(e).despawn() + }; } fn check_despawns(mut removed_foos: RemovedComponents) { let mut despawn_count = 0; From 5888bf276daa1959eeb62671d1a8d6dd230eb6db Mon Sep 17 00:00:00 2001 From: Chris Juchem Date: Sat, 8 Jun 2024 23:14:51 -0400 Subject: [PATCH 4/8] Clippy again --- crates/bevy_app/src/app.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index ef4e4462754d7..759421e6f67a0 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -1141,7 +1141,7 @@ mod tests { fn despawn_one_foo(mut commands: Commands, foos: Query>) { if let Some(e) = foos.iter().next() { - commands.entity(e).despawn() + commands.entity(e).despawn(); }; } fn check_despawns(mut removed_foos: RemovedComponents) { From 4d448589a71f18ab249245e034a191731d482819 Mon Sep 17 00:00:00 2001 From: Chris Juchem Date: Sun, 9 Jun 2024 19:41:22 -0400 Subject: [PATCH 5/8] Delay clearing trackers on main world until after subapps are extracted and run --- crates/bevy_app/src/sub_app.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index 4e074d035c372..59ad4eca8b105 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -125,7 +125,9 @@ impl SubApp { } /// Runs the default schedule. - pub fn update(&mut self) { + /// + /// Does not clear internal trackers used for change detection. + pub fn run_default_schedule(&mut self) { if self.is_building_plugins() { panic!("SubApp::update() was called while a plugin was building."); } @@ -133,6 +135,11 @@ impl SubApp { if let Some(label) = self.update_schedule { self.world.run_schedule(label); } + } + + /// Runs the default schedule and updates internal component trackers. + pub fn update(&mut self) { + self.run_default_schedule(); self.world.clear_trackers(); } @@ -421,7 +428,7 @@ impl SubApps { { #[cfg(feature = "trace")] let _bevy_frame_update_span = info_span!("main app").entered(); - self.main.update(); + self.main.run_default_schedule(); } for (_label, sub_app) in self.sub_apps.iter_mut() { #[cfg(feature = "trace")] @@ -429,6 +436,8 @@ impl SubApps { sub_app.extract(&mut self.main.world); sub_app.update(); } + + self.main.world.clear_trackers(); } /// Returns an iterator over the sub-apps (starting with the main one). From 3d3b99d4b21ca109e7fa70a7fa9b5299de002d0e Mon Sep 17 00:00:00 2001 From: Chris Juchem Date: Sun, 9 Jun 2024 19:47:19 -0400 Subject: [PATCH 6/8] update docs --- crates/bevy_ecs/src/removal_detection.rs | 8 ++++---- crates/bevy_ecs/src/world/mod.rs | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/removal_detection.rs b/crates/bevy_ecs/src/removal_detection.rs index da780e9e8f199..2e009de3d76ec 100644 --- a/crates/bevy_ecs/src/removal_detection.rs +++ b/crates/bevy_ecs/src/removal_detection.rs @@ -116,11 +116,11 @@ impl RemovedComponentEvents { /// /// If you are using `bevy_ecs` as a standalone crate, /// note that the `RemovedComponents` list will not be automatically cleared for you, -/// and will need to be manually flushed using [`World::clear_trackers`](World::clear_trackers) +/// and will need to be manually flushed using [`World::clear_trackers`](World::clear_trackers). /// -/// For users of `bevy` and `bevy_app`, this is automatically done in `bevy_app::App::update`. -/// For the main world, [`World::clear_trackers`](World::clear_trackers) is run after the main schedule is run and after -/// `SubApp`'s have run. +/// For users of `bevy` and `bevy_app`, [`World::clear_trackers`](World::clear_trackers) is +/// automatically called by `bevy_app::App::update` and `bevy_app::SubApp::update`. +/// For the main world, this is delayed until after all `SubApp`s have run. /// /// # Examples /// diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 6d3210244becc..354da63ea88eb 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1113,9 +1113,9 @@ impl World { /// By clearing this internal state, the world "forgets" about those changes, allowing a new round /// of detection to be recorded. /// - /// When using `bevy_ecs` as part of the full Bevy engine, this method is added as a system to the - /// main app, to run during `Last`, so you don't need to call it manually. When using `bevy_ecs` - /// as a separate standalone crate however, you need to call this manually. + /// When using `bevy_ecs` as part of the full Bevy engine, this method is called automatically + /// by `bevy_app::App::update` and `bevy_app::SubApp::update`, so you don't need to call it manually. + /// When using `bevy_ecs` as a separate standalone crate however, you do need to call this manually. /// /// ``` /// # use bevy_ecs::prelude::*; From afba3c4f63f926ce21ccf61c59ce96d288242807 Mon Sep 17 00:00:00 2001 From: Chris Juchem Date: Sun, 9 Jun 2024 20:03:52 -0400 Subject: [PATCH 7/8] add another unit test for extraction --- crates/bevy_app/src/app.rs | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 759421e6f67a0..91bff44c34c01 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -919,16 +919,17 @@ mod tests { use std::{iter, marker::PhantomData, mem}; use bevy_ecs::{ + change_detection::{DetectChanges, ResMut}, component::Component, entity::Entity, event::EventWriter, query::With, removal_detection::RemovedComponents, schedule::{IntoSystemConfigs, ScheduleLabel}, - system::{Commands, Query}, + system::{Commands, Query, Resource}, }; - use crate::{App, AppExit, Plugin, Update}; + use crate::{App, AppExit, Plugin, SubApp, Update}; struct PluginA; impl Plugin for PluginA { @@ -1160,6 +1161,33 @@ mod tests { app.update(); // Should see despawns from frames 1 & 2, but not frame 0 } + #[test] + fn test_extract_sees_changes() { + use super::AppLabel; + use crate::{self as bevy_app}; + + #[derive(AppLabel, Clone, Copy, Hash, PartialEq, Eq, Debug)] + struct MySubApp; + + #[derive(Resource)] + struct Foo(usize); + + let mut app = App::new(); + app.world_mut().insert_resource(Foo(0)); + app.add_systems(Update, |mut foo: ResMut| { + (*foo).0 += 1; + }); + + let mut sub_app = SubApp::new(); + sub_app.set_extract(|main_world, _sub_world| { + assert!(main_world.get_resource_ref::().unwrap().is_changed()); + }); + + app.insert_sub_app(MySubApp, sub_app); + + app.update(); + } + #[test] fn runner_returns_correct_exit_code() { fn raise_exits(mut exits: EventWriter) { From a0e1ad5102bafbb45e67d0aba5994b8559b70b88 Mon Sep 17 00:00:00 2001 From: Chris Juchem Date: Sun, 9 Jun 2024 20:25:54 -0400 Subject: [PATCH 8/8] clippy --- crates/bevy_app/src/app.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 91bff44c34c01..3ae4e57eb341a 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -1175,7 +1175,7 @@ mod tests { let mut app = App::new(); app.world_mut().insert_resource(Foo(0)); app.add_systems(Update, |mut foo: ResMut| { - (*foo).0 += 1; + foo.0 += 1; }); let mut sub_app = SubApp::new();