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();
    }
}
```
This commit is contained in:
Chris Juchem 2024-06-10 14:06:05 -04:00 committed by GitHub
parent 2c5959a29d
commit 49661b99fe
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 85 additions and 15 deletions

View file

@ -919,14 +919,21 @@ impl Termination for AppExit {
#[cfg(test)]
mod tests {
use std::sync::Mutex;
use std::{marker::PhantomData, mem};
use std::{iter, marker::PhantomData, mem, sync::Mutex};
use bevy_ecs::prelude::{Resource, World};
use bevy_ecs::world::FromWorld;
use bevy_ecs::{event::EventWriter, schedule::ScheduleLabel, system::Commands};
use bevy_ecs::{
change_detection::{DetectChanges, ResMut},
component::Component,
entity::Entity,
event::EventWriter,
query::With,
removal_detection::RemovedComponents,
schedule::{IntoSystemConfigs, ScheduleLabel},
system::{Commands, Query, Resource},
world::{FromWorld, World},
};
use crate::{App, AppExit, Plugin, Update};
use crate::{App, AppExit, Plugin, SubApp, Update};
struct PluginA;
impl Plugin for PluginA {
@ -1129,6 +1136,62 @@ 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<Entity, With<Foo>>) {
if let Some(e) = foos.iter().next() {
commands.entity(e).despawn();
};
}
fn check_despawns(mut removed_foos: RemovedComponents<Foo>) {
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 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>| {
foo.0 += 1;
});
let mut sub_app = SubApp::new();
sub_app.set_extract(|main_world, _sub_world| {
assert!(main_world.get_resource_ref::<Foo>().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<AppExit>) {

View file

@ -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")]

View file

@ -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
///

View file

@ -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::*;