Skip to content

Commit

Permalink
Not working yet, replace with an events for removed components
Browse files Browse the repository at this point in the history
Refactor to use a resource of component ids to events

Removed components as an iterator over events

Clean up unnecessary code in event, update events on clear_trackers

Correct documentation

Clarify documentation

Fix test to expect despawned entity as well

Fix some CI and docs

Fix links in docs

Rename to match previous type names

Rename in docs/tests

Unused import

Fix removal detection example

Fix example docs

Fix up rebase issue, make panic message more in line with other ones

Fix docs for RemovedComponents
  • Loading branch information
Aceeri committed Dec 21, 2022
1 parent 2938792 commit a034dae
Show file tree
Hide file tree
Showing 7 changed files with 256 additions and 146 deletions.
96 changes: 2 additions & 94 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub mod event;
pub mod query;
#[cfg(feature = "bevy_reflect")]
pub mod reflect;
pub mod removal_detection;
pub mod schedule;
pub mod storage;
pub mod system;
Expand All @@ -33,6 +34,7 @@ pub mod prelude {
entity::Entity,
event::{EventReader, EventWriter, Events},
query::{Added, AnyOf, ChangeTrackers, Changed, Or, QueryState, With, Without},
removal_detection::{RemovedComponentEvents, RemovedComponents},
schedule::{
IntoSystemDescriptor, RunCriteria, RunCriteriaDescriptorCoercion, RunCriteriaLabel,
Schedule, Stage, StageLabel, State, SystemLabel, SystemSet, SystemStage,
Expand Down Expand Up @@ -708,100 +710,6 @@ mod tests {
assert!(i32_query.get(&world, a).is_err());
}

#[test]
fn query_get_works_across_sparse_removal() {
// Regression test for: https://github.com/bevyengine/bevy/issues/6623
let mut world = World::new();
let a = world.spawn((TableStored("abc"), SparseStored(123))).id();
let b = world.spawn((TableStored("def"), SparseStored(456))).id();
let c = world
.spawn((TableStored("ghi"), SparseStored(789), B(1)))
.id();

let mut query = world.query::<&TableStored>();
assert_eq!(query.get(&world, a).unwrap(), &TableStored("abc"));
assert_eq!(query.get(&world, b).unwrap(), &TableStored("def"));
assert_eq!(query.get(&world, c).unwrap(), &TableStored("ghi"));

world.entity_mut(b).remove::<SparseStored>();
world.entity_mut(c).remove::<SparseStored>();

assert_eq!(query.get(&world, a).unwrap(), &TableStored("abc"));
assert_eq!(query.get(&world, b).unwrap(), &TableStored("def"));
assert_eq!(query.get(&world, c).unwrap(), &TableStored("ghi"));
}

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

let a = world.spawn((SparseStored(0), A(123))).id();
let b = world.spawn((SparseStored(1), A(123))).id();

world.entity_mut(a).despawn();
assert_eq!(
world.removed::<A>().collect::<Vec<_>>(),
&[a],
"despawning results in 'removed component' state for table components"
);
assert_eq!(
world.removed::<SparseStored>().collect::<Vec<_>>(),
&[a],
"despawning results in 'removed component' state for sparse set components"
);

world.entity_mut(b).insert(B(1));
assert_eq!(
world.removed::<A>().collect::<Vec<_>>(),
&[a],
"archetype moves does not result in 'removed component' state"
);

world.entity_mut(b).remove::<A>();
assert_eq!(
world.removed::<A>().collect::<Vec<_>>(),
&[a, b],
"removing a component results in a 'removed component' state"
);

world.clear_trackers();
assert_eq!(
world.removed::<A>().collect::<Vec<_>>(),
&[],
"clearning trackers clears removals"
);
assert_eq!(
world.removed::<SparseStored>().collect::<Vec<_>>(),
&[],
"clearning trackers clears removals"
);
assert_eq!(
world.removed::<B>().collect::<Vec<_>>(),
&[],
"clearning trackers clears removals"
);

// TODO: uncomment when world.clear() is implemented
// let c = world.spawn(("abc", 123)).id();
// let d = world.spawn(("abc", 123)).id();
// world.clear();
// assert_eq!(
// world.removed::<i32>(),
// &[c, d],
// "world clears result in 'removed component' states"
// );
// assert_eq!(
// world.removed::<&'static str>(),
// &[c, d, b],
// "world clears result in 'removed component' states"
// );
// assert_eq!(
// world.removed::<f64>(),
// &[b],
// "world clears result in 'removed component' states"
// );
}

#[test]
fn added_tracking() {
let mut world = World::new();
Expand Down
217 changes: 217 additions & 0 deletions crates/bevy_ecs/src/removal_detection.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
//! Removal event types.

use crate as bevy_ecs;
use crate::component::{Component, ComponentId, Components};
use crate::entity::Entity;
use crate::event::{EventId, Events, ManualEventReader};
use crate::storage::SparseSet;
use crate::system::{Local, Res, Resource, SystemParam};
use std::marker::PhantomData;

/// 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 [`RemovedComponentEvents`] events will not be automatically updated/cleared for you,
/// and will need to be manually flushed using [`crate::world::World::clear_trackers`] or [`RemovedComponentEvents::update`]
///
/// For users of `bevy` itself, this is automatically done in a system added by `MinimalPlugins`
/// or `DefaultPlugins` at the end of each pass of the game loop during the `CoreStage::Last`
/// stage.
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// # use bevy_ecs::component::Component;
/// # use bevy_ecs::system::IntoSystem;
/// # use bevy_ecs::removal_detection::RemovedComponents;
/// #
/// # #[derive(Component)]
/// # struct MyComponent;
///
/// fn react_on_removal(mut removed: RemovedComponents<MyComponent>) {
/// removed.iter().for_each(|removed_entity| println!("{:?}", removed_entity));
/// }
///
/// # bevy_ecs::system::assert_is_system(react_on_removal);
/// ```
#[derive(SystemParam)]
pub struct RemovedComponents<'w, 's, T: Component> {
components: &'w Components,
events: Res<'w, RemovedComponentEvents>,
readers: Local<'s, RemovedComponentReaders>,
#[system_param(ignore)]
phantom: PhantomData<&'s T>,
}

impl<'w, 's, T: Component> RemovedComponents<'w, 's, T> {
pub fn component_id(&self) -> ComponentId {
self.components
.component_id::<T>()
.expect("component does not exist in the world")
}

pub fn iter(&mut self) -> impl Iterator<Item = Entity> + '_ {
self.iter_with_id().map(|(e, _)| e)
}

pub fn iter_with_id(&mut self) -> impl Iterator<Item = (Entity, EventId<Entity>)> + '_ {
let component_id = self.component_id();
let events = self.events.get(component_id);
let reader = self.readers.get_mut(component_id);
events
.map(|events| reader.iter_with_id(events))
.into_iter()
.flatten()
.map(|(e, id)| (*e, id))
}

pub fn len(&self) -> usize {
let component_id = self.component_id();
let events = self.events.get(component_id);
let reader = self.readers.get(component_id);
match (reader, events) {
(Some(reader), Some(events)) => reader.len(events),
(None, Some(events)) => events.len(),
_ => 0,
}
}

pub fn is_empty(&self) -> bool {
self.len() == 0
}
}

#[derive(Default, Debug, Resource)]
pub struct RemovedComponentEvents {
events: SparseSet<ComponentId, Events<Entity>>,
}

impl RemovedComponentEvents {
pub fn get(&self, component_id: ComponentId) -> Option<&Events<Entity>> {
self.events.get(component_id)
}

pub fn get_mut(&mut self, component_id: ComponentId) -> &mut Events<Entity> {
self.events
.get_or_insert_with(component_id, Default::default)
}

pub fn extend(
&mut self,
component_id: ComponentId,
entities: impl IntoIterator<Item = Entity>,
) {
self.get_mut(component_id).extend(entities);
}

pub fn push(&mut self, component_id: ComponentId, entity: Entity) {
self.get_mut(component_id).send(entity);
}

pub fn update(&mut self) {
for events in self.events.values_mut() {
events.update();
}
}
}

#[derive(Default, Debug, Resource)]
pub struct RemovedComponentReaders {
readers: SparseSet<ComponentId, ManualEventReader<Entity>>,
}

impl RemovedComponentReaders {
pub fn get(&self, component_id: ComponentId) -> Option<&ManualEventReader<Entity>> {
self.readers.get(component_id)
}

pub fn get_mut(&mut self, component_id: ComponentId) -> &mut ManualEventReader<Entity> {
self.readers
.get_or_insert_with(component_id, Default::default)
}
}

#[cfg(test)]
mod tests {
use crate as bevy_ecs;
use crate::{
component::Component,
removal_detection::RemovedComponents,
system::{Resource, SystemState},
world::World,
};

#[derive(Component, Resource, Debug, PartialEq, Eq, Clone, Copy)]
struct A(usize);
#[derive(Component, Debug, PartialEq, Eq, Clone, Copy)]
struct B(usize);
#[derive(Component, Debug, PartialEq, Eq, Clone, Copy)]
struct C;

#[derive(Component, Copy, Clone, PartialEq, Eq, Debug)]
#[component(storage = "Table")]
struct TableStored(&'static str);
#[derive(Component, Copy, Clone, PartialEq, Eq, Debug)]
#[component(storage = "SparseSet")]
struct SparseStored(u32);

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

let a = world.spawn().insert_bundle((SparseStored(0), A(123))).id();
let b = world.spawn().insert_bundle((SparseStored(1),)).id();
let c = world
.spawn()
.insert_bundle((SparseStored(0), A(123), B(1), C))
.id();

let mut a_system: SystemState<RemovedComponents<A>> = SystemState::new(&mut world);
let mut c_system: SystemState<RemovedComponents<C>> = SystemState::new(&mut world);
let mut sparse_system: SystemState<RemovedComponents<SparseStored>> =
SystemState::new(&mut world);
world.entity_mut(a).despawn();
world.entity_mut(b).despawn();
assert_eq!(
a_system.get_mut(&mut world).iter().collect::<Vec<_>>(),
&[a],
"despawning results in 'removed component' state for table components"
);
assert_eq!(
sparse_system.get_mut(&mut world).iter().collect::<Vec<_>>(),
&[a, b],
"despawning results in 'removed component' state for sparse set components"
);

// When we check again the event iterator should be ahead now.
assert_eq!(
a_system.get_mut(&mut world).iter().collect::<Vec<_>>(),
&[],
"reading from the same system state will result in no new removal events"
);
assert_eq!(
sparse_system.get_mut(&mut world).iter().collect::<Vec<_>>(),
&[],
"reading from the same system state will result in no new removal events"
);

world.entity_mut(c).remove::<C>();
assert_eq!(
c_system.get_mut(&mut world).iter().collect::<Vec<_>>(),
&[c],
"removing specific component should result in removal event"
);
assert_eq!(
c_system.get_mut(&mut world).iter().collect::<Vec<_>>(),
&[],
"reading from same system state will result in no new removal events"
);
}
}
10 changes: 6 additions & 4 deletions crates/bevy_ecs/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@
//! - [`NonSend`] and `Option<NonSend>`
//! - [`NonSendMut`] and `Option<NonSendMut>`
//! - [`&World`](crate::world::World)
//! - [`RemovedComponents`]
//! - [`SystemName`]
//! - [`RemovedComponents`](crate::removal_detection::RemovedComponents)
//! - [`SystemChangeTick`]
//! - [`Archetypes`](crate::archetype::Archetypes) (Provides Archetype metadata)
//! - [`Bundles`](crate::bundle::Bundles) (Provides Bundles metadata)
Expand Down Expand Up @@ -114,6 +114,7 @@ mod tests {
entity::{Entities, Entity},
prelude::AnyOf,
query::{Added, Changed, Or, With, Without},
removal_detection::RemovedComponents,
schedule::{Schedule, Stage, SystemStage},
system::{
Commands, IntoSystem, Local, NonSend, NonSendMut, ParamSet, Query, QueryComponentError,
Expand Down Expand Up @@ -577,7 +578,7 @@ mod tests {
world.entity_mut(spurious_entity).despawn();

fn validate_despawn(
removed_i32: RemovedComponents<W<i32>>,
mut removed_i32: RemovedComponents<W<i32>>,
despawned: Res<Despawned>,
mut n_systems: ResMut<NSystems>,
) {
Expand All @@ -602,13 +603,14 @@ mod tests {
world.entity_mut(entity_to_remove_w_from).remove::<W<i32>>();

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

Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,7 @@ unsafe impl<T: FromWorld + Send + 'static> SystemParamState for LocalState<T> {
}
}

<<<<<<< HEAD
/// 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.
Expand Down Expand Up @@ -906,6 +907,8 @@ unsafe impl<T: Component> SystemParamState for RemovedComponentsState<T> {
}
}

=======
>>>>>>> 0a7b6378e (Not working yet, replace with an events for removed components)
/// Shared borrow of a non-[`Send`] resource.
///
/// Only `Send` resources may be accessed with the [`Res`] [`SystemParam`]. In case that the
Expand Down
Loading

0 comments on commit a034dae

Please sign in to comment.