Skip to content

[Merged by Bors] - Removal detection cleanup #3010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 51 additions & 12 deletions crates/bevy_ecs/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,32 +450,71 @@ mod tests {
}

#[test]
fn remove_tracking() {
fn removal_tracking() {
let mut world = World::new();

let entity_to_despawn = world.spawn().insert(W(1)).id();
let entity_to_remove_w_from = world.spawn().insert(W(2)).id();
let spurious_entity = world.spawn().id();

// Track which entities we want to operate on
struct Despawned(Entity);
let a = world.spawn().insert_bundle((W("abc"), W(123))).id();
world.spawn().insert_bundle((W("abc"), W(123)));
world.insert_resource(false);
world.insert_resource(Despawned(a));
world.insert_resource(Despawned(entity_to_despawn));
struct Removed(Entity);
world.insert_resource(Removed(entity_to_remove_w_from));

world.entity_mut(a).despawn();
// Verify that all the systems actually ran
#[derive(Default)]
struct NSystems(usize);
world.insert_resource(NSystems::default());

fn validate_removed(
// First, check that removal detection is triggered if and only if we despawn an entity with the correct component
world.entity_mut(entity_to_despawn).despawn();
world.entity_mut(spurious_entity).despawn();

fn validate_despawn(
removed_i32: RemovedComponents<W<i32>>,
despawned: Res<Despawned>,
mut ran: ResMut<bool>,
mut n_systems: ResMut<NSystems>,
) {
assert_eq!(
removed_i32.iter().collect::<Vec<_>>(),
&[despawned.0],
"despawning results in 'removed component' state"
"despawning causes the correct entity to show up in the 'RemovedComponent' system parameter."
);

*ran = true;
n_systems.0 += 1;
}

run_system(&mut world, validate_removed);
assert!(*world.get_resource::<bool>().unwrap(), "system ran");
run_system(&mut world, validate_despawn);

// Reset the trackers to clear the buffer of removed components
// Ordinarily, this is done in a system added by MinimalPlugins
world.clear_trackers();

// Then, try removing a component
world.spawn().insert(W(3)).id();
world.spawn().insert(W(4)).id();
world.entity_mut(entity_to_remove_w_from).remove::<W<i32>>();

fn validate_remove(
removed_i32: RemovedComponents<W<i32>>,
removed: Res<Removed>,
mut n_systems: ResMut<NSystems>,
) {
assert_eq!(
removed_i32.iter().collect::<Vec<_>>(),
&[removed.0],
"removing a component causes the correct entity to show up in the 'RemovedComponent' system parameter."
);

n_systems.0 += 1;
}

run_system(&mut world, validate_remove);

// Verify that both systems actually ran
assert_eq!(world.get_resource::<NSystems>().unwrap().0, 2);
}

#[test]
Expand Down
12 changes: 12 additions & 0 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,18 @@ impl<'w, 's, T: Resource + FromWorld> SystemParamFetch<'w, 's> for LocalState<T>

/// A [`SystemParam`] that grants access to the entities that had their `T` [`Component`] removed.
///
/// Note that this does not allow you to see which data existed before removal.
/// If you need this, you will need to track the component data value on your own,
/// using a regularly scheduled system that requests `Query<(Entity, &T), Changed<T>>`
/// and stores the data somewhere safe to later cross-reference.
///
/// 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`]
///
/// For users of `bevy` itself, this is automatically done in a system added by `MinimalPlugins`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be mentioned that it's also added by DefaultPlugins, not just by MinimalPlugins

/// or `DefaultPlugins` at the end of each pass of the game loop.
///
/// # Examples
///
/// Basic usage:
Expand Down