Skip to content

Commit 49661b9

Browse files
authored
Remove extra call to clear_trackers (#13762)
Fixes #13758. # Objective Calling `update` on the main app already calls `clear_trackers`. Calling it again in `SubApps::update` caused RemovedCompenet Events to be cleared earlier than they should be. ## Solution - Don't call clear_trackers an extra time. ## Testing I manually tested the fix with this unit test: ``` #[cfg(test)] mod test { use crate::core::{FrameCount, FrameCountPlugin}; use crate::prelude::*; #[test] fn test_next_frame_removal() { #[derive(Component)] struct Foo; #[derive(Resource)] struct RemovedCount(usize); let mut app = App::new(); app.add_plugins(FrameCountPlugin); app.add_systems(Startup, |mut commands: Commands| { for _ in 0..100 { commands.spawn(Foo); } commands.insert_resource(RemovedCount(0)); }); app.add_systems(First, |counter: Res<FrameCount>| { println!("Frame {}:", counter.0) }); fn detector_system( mut removals: RemovedComponents<Foo>, foos: Query<Entity, With<Foo>>, mut removed_c: ResMut<RemovedCount>, ) { for e in removals.read() { println!(" Detected removed Foo component for {e:?}"); removed_c.0 += 1; } let c = foos.iter().count(); println!(" Total Foos: {}", c); assert_eq!(c + removed_c.0, 100); } fn deleter_system(foos: Query<Entity, With<Foo>>, mut commands: Commands) { foos.iter().next().map(|e| { commands.entity(e).remove::<Foo>(); }); } app.add_systems(Update, (detector_system, deleter_system).chain()); app.update(); app.update(); app.update(); app.update(); } } ```
1 parent 2c5959a commit 49661b9

File tree

4 files changed

+87
-17
lines changed

4 files changed

+87
-17
lines changed

crates/bevy_app/src/app.rs

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -919,14 +919,21 @@ impl Termination for AppExit {
919919

920920
#[cfg(test)]
921921
mod tests {
922-
use std::sync::Mutex;
923-
use std::{marker::PhantomData, mem};
924-
925-
use bevy_ecs::prelude::{Resource, World};
926-
use bevy_ecs::world::FromWorld;
927-
use bevy_ecs::{event::EventWriter, schedule::ScheduleLabel, system::Commands};
928-
929-
use crate::{App, AppExit, Plugin, Update};
922+
use std::{iter, marker::PhantomData, mem, sync::Mutex};
923+
924+
use bevy_ecs::{
925+
change_detection::{DetectChanges, ResMut},
926+
component::Component,
927+
entity::Entity,
928+
event::EventWriter,
929+
query::With,
930+
removal_detection::RemovedComponents,
931+
schedule::{IntoSystemConfigs, ScheduleLabel},
932+
system::{Commands, Query, Resource},
933+
world::{FromWorld, World},
934+
};
935+
936+
use crate::{App, AppExit, Plugin, SubApp, Update};
930937

931938
struct PluginA;
932939
impl Plugin for PluginA {
@@ -1129,6 +1136,62 @@ mod tests {
11291136
);
11301137
}
11311138

1139+
#[test]
1140+
fn test_update_clears_trackers_once() {
1141+
#[derive(Component, Copy, Clone)]
1142+
struct Foo;
1143+
1144+
let mut app = App::new();
1145+
app.world_mut().spawn_batch(iter::repeat(Foo).take(5));
1146+
1147+
fn despawn_one_foo(mut commands: Commands, foos: Query<Entity, With<Foo>>) {
1148+
if let Some(e) = foos.iter().next() {
1149+
commands.entity(e).despawn();
1150+
};
1151+
}
1152+
fn check_despawns(mut removed_foos: RemovedComponents<Foo>) {
1153+
let mut despawn_count = 0;
1154+
for _ in removed_foos.read() {
1155+
despawn_count += 1;
1156+
}
1157+
1158+
assert_eq!(despawn_count, 2);
1159+
}
1160+
1161+
app.add_systems(Update, despawn_one_foo);
1162+
app.update(); // Frame 0
1163+
app.update(); // Frame 1
1164+
app.add_systems(Update, check_despawns.after(despawn_one_foo));
1165+
app.update(); // Should see despawns from frames 1 & 2, but not frame 0
1166+
}
1167+
1168+
#[test]
1169+
fn test_extract_sees_changes() {
1170+
use super::AppLabel;
1171+
use crate::{self as bevy_app};
1172+
1173+
#[derive(AppLabel, Clone, Copy, Hash, PartialEq, Eq, Debug)]
1174+
struct MySubApp;
1175+
1176+
#[derive(Resource)]
1177+
struct Foo(usize);
1178+
1179+
let mut app = App::new();
1180+
app.world_mut().insert_resource(Foo(0));
1181+
app.add_systems(Update, |mut foo: ResMut<Foo>| {
1182+
foo.0 += 1;
1183+
});
1184+
1185+
let mut sub_app = SubApp::new();
1186+
sub_app.set_extract(|main_world, _sub_world| {
1187+
assert!(main_world.get_resource_ref::<Foo>().unwrap().is_changed());
1188+
});
1189+
1190+
app.insert_sub_app(MySubApp, sub_app);
1191+
1192+
app.update();
1193+
}
1194+
11321195
#[test]
11331196
fn runner_returns_correct_exit_code() {
11341197
fn raise_exits(mut exits: EventWriter<AppExit>) {

crates/bevy_app/src/sub_app.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,21 @@ impl SubApp {
125125
}
126126

127127
/// Runs the default schedule.
128-
pub fn update(&mut self) {
128+
///
129+
/// Does not clear internal trackers used for change detection.
130+
pub fn run_default_schedule(&mut self) {
129131
if self.is_building_plugins() {
130132
panic!("SubApp::update() was called while a plugin was building.");
131133
}
132134

133135
if let Some(label) = self.update_schedule {
134136
self.world.run_schedule(label);
135137
}
138+
}
139+
140+
/// Runs the default schedule and updates internal component trackers.
141+
pub fn update(&mut self) {
142+
self.run_default_schedule();
136143
self.world.clear_trackers();
137144
}
138145

@@ -421,7 +428,7 @@ impl SubApps {
421428
{
422429
#[cfg(feature = "trace")]
423430
let _bevy_frame_update_span = info_span!("main app").entered();
424-
self.main.update();
431+
self.main.run_default_schedule();
425432
}
426433
for (_label, sub_app) in self.sub_apps.iter_mut() {
427434
#[cfg(feature = "trace")]

crates/bevy_ecs/src/removal_detection.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,11 @@ impl RemovedComponentEvents {
116116
///
117117
/// If you are using `bevy_ecs` as a standalone crate,
118118
/// note that the `RemovedComponents` list will not be automatically cleared for you,
119-
/// and will need to be manually flushed using [`World::clear_trackers`](World::clear_trackers)
119+
/// and will need to be manually flushed using [`World::clear_trackers`](World::clear_trackers).
120120
///
121-
/// For users of `bevy` and `bevy_app`, this is automatically done in `bevy_app::App::update`.
122-
/// For the main world, [`World::clear_trackers`](World::clear_trackers) is run after the main schedule is run and after
123-
/// `SubApp`'s have run.
121+
/// For users of `bevy` and `bevy_app`, [`World::clear_trackers`](World::clear_trackers) is
122+
/// automatically called by `bevy_app::App::update` and `bevy_app::SubApp::update`.
123+
/// For the main world, this is delayed until after all `SubApp`s have run.
124124
///
125125
/// # Examples
126126
///

crates/bevy_ecs/src/world/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,9 +1113,9 @@ impl World {
11131113
/// By clearing this internal state, the world "forgets" about those changes, allowing a new round
11141114
/// of detection to be recorded.
11151115
///
1116-
/// When using `bevy_ecs` as part of the full Bevy engine, this method is added as a system to the
1117-
/// main app, to run during `Last`, so you don't need to call it manually. When using `bevy_ecs`
1118-
/// as a separate standalone crate however, you need to call this manually.
1116+
/// When using `bevy_ecs` as part of the full Bevy engine, this method is called automatically
1117+
/// by `bevy_app::App::update` and `bevy_app::SubApp::update`, so you don't need to call it manually.
1118+
/// When using `bevy_ecs` as a separate standalone crate however, you do need to call this manually.
11191119
///
11201120
/// ```
11211121
/// # use bevy_ecs::prelude::*;

0 commit comments

Comments
 (0)