Skip to content

Commit 67826b2

Browse files
committed
Replace RemovedComponents<T> backing with Events<Entity> (#5680)
# Objective Removal events are unwieldy and require some knowledge of when to put systems that need to catch events for them, it is very easy to end up missing one and end up with memory leak-ish issues where you don't clean up after yourself. ## Solution Consolidate removals with the benefits of `Events<...>` (such as double buffering and per system ticks for reading the events) and reduce the special casing of it, ideally I was hoping to move the removals to a `Resource` in the world, but that seems a bit more rough to implement/maintain because of double mutable borrowing issues. This doesn't go the full length of change detection esque removal detection a la bevyengine/rfcs#44. Just tries to make the current workflow a bit more user friendly so detecting removals isn't such a scheduling nightmare. --- ## Changelog - RemovedComponents<T> is now backed by an `Events<Entity>` for the benefits of double buffering. ## Migration Guide - Add a `mut` for `removed: RemovedComponents<T>` since we are now modifying an event reader internally. - Iterating over removed components now requires `&mut removed_components` or `removed_components.iter()` instead of `&removed_components`.
1 parent 12f30f5 commit 67826b2

File tree

10 files changed

+273
-132
lines changed

10 files changed

+273
-132
lines changed

crates/bevy_ecs/src/component.rs

+48-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
use crate::{
44
change_detection::MAX_CHANGE_AGE,
55
storage::{SparseSetIndex, Storages},
6-
system::Resource,
6+
system::{Local, Resource},
7+
world::{FromWorld, World},
78
};
89
pub use bevy_ecs_macros::Component;
910
use bevy_ptr::{OwningPtr, UnsafeCellDeref};
@@ -12,6 +13,7 @@ use std::{
1213
alloc::Layout,
1314
any::{Any, TypeId},
1415
borrow::Cow,
16+
marker::PhantomData,
1517
mem::needs_drop,
1618
};
1719

@@ -698,3 +700,48 @@ impl ComponentTicks {
698700
self.changed.set_changed(change_tick);
699701
}
700702
}
703+
704+
/// Initialize and fetch a [`ComponentId`] for a specific type.
705+
///
706+
/// # Example
707+
/// ```rust
708+
/// # use bevy_ecs::{system::Local, component::{Component, ComponentId, ComponentIdFor}};
709+
/// #[derive(Component)]
710+
/// struct Player;
711+
/// fn my_system(component_id: Local<ComponentIdFor<Player>>) {
712+
/// let component_id: ComponentId = component_id.into();
713+
/// // ...
714+
/// }
715+
/// ```
716+
pub struct ComponentIdFor<T: Component> {
717+
component_id: ComponentId,
718+
phantom: PhantomData<T>,
719+
}
720+
721+
impl<T: Component> FromWorld for ComponentIdFor<T> {
722+
fn from_world(world: &mut World) -> Self {
723+
Self {
724+
component_id: world.init_component::<T>(),
725+
phantom: PhantomData,
726+
}
727+
}
728+
}
729+
730+
impl<T: Component> std::ops::Deref for ComponentIdFor<T> {
731+
type Target = ComponentId;
732+
fn deref(&self) -> &Self::Target {
733+
&self.component_id
734+
}
735+
}
736+
737+
impl<T: Component> From<ComponentIdFor<T>> for ComponentId {
738+
fn from(to_component_id: ComponentIdFor<T>) -> ComponentId {
739+
*to_component_id
740+
}
741+
}
742+
743+
impl<'s, T: Component> From<Local<'s, ComponentIdFor<T>>> for ComponentId {
744+
fn from(to_component_id: Local<ComponentIdFor<T>>) -> ComponentId {
745+
**to_component_id
746+
}
747+
}

crates/bevy_ecs/src/lib.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ pub mod event;
1313
pub mod query;
1414
#[cfg(feature = "bevy_reflect")]
1515
pub mod reflect;
16+
pub mod removal_detection;
1617
pub mod schedule;
1718
pub mod schedule_v3;
1819
pub mod storage;
@@ -34,6 +35,7 @@ pub mod prelude {
3435
entity::Entity,
3536
event::{EventReader, EventWriter, Events},
3637
query::{Added, AnyOf, ChangeTrackers, Changed, Or, QueryState, With, Without},
38+
removal_detection::RemovedComponents,
3739
schedule::{
3840
IntoSystemDescriptor, RunCriteria, RunCriteriaDescriptorCoercion, RunCriteriaLabel,
3941
Schedule, Stage, StageLabel, State, SystemLabel, SystemSet, SystemStage,
@@ -42,7 +44,7 @@ pub mod prelude {
4244
adapter as system_adapter,
4345
adapter::{dbg, error, ignore, info, unwrap, warn},
4446
Commands, In, IntoPipeSystem, IntoSystem, Local, NonSend, NonSendMut, ParallelCommands,
45-
ParamSet, Query, RemovedComponents, Res, ResMut, Resource, System, SystemParamFunction,
47+
ParamSet, Query, Res, ResMut, Resource, System, SystemParamFunction,
4648
},
4749
world::{FromWorld, World},
4850
};
+169
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
//! Alerting events when a component is removed from an entity.
2+
3+
use crate::{
4+
self as bevy_ecs,
5+
component::{Component, ComponentId, ComponentIdFor},
6+
entity::Entity,
7+
event::{Events, ManualEventIterator, ManualEventReader},
8+
prelude::Local,
9+
storage::SparseSet,
10+
system::{ReadOnlySystemParam, SystemMeta, SystemParam},
11+
world::World,
12+
};
13+
14+
use std::{
15+
fmt::Debug,
16+
iter,
17+
marker::PhantomData,
18+
ops::{Deref, DerefMut},
19+
option,
20+
};
21+
22+
/// Wrapper around a [`ManualEventReader<Entity>`] so that we
23+
/// can differentiate events between components.
24+
#[derive(Debug)]
25+
pub struct RemovedComponentReader<T>
26+
where
27+
T: Component,
28+
{
29+
reader: ManualEventReader<Entity>,
30+
marker: PhantomData<T>,
31+
}
32+
33+
impl<T: Component> Default for RemovedComponentReader<T> {
34+
fn default() -> Self {
35+
Self {
36+
reader: Default::default(),
37+
marker: PhantomData,
38+
}
39+
}
40+
}
41+
42+
impl<T: Component> Deref for RemovedComponentReader<T> {
43+
type Target = ManualEventReader<Entity>;
44+
fn deref(&self) -> &Self::Target {
45+
&self.reader
46+
}
47+
}
48+
49+
impl<T: Component> DerefMut for RemovedComponentReader<T> {
50+
fn deref_mut(&mut self) -> &mut Self::Target {
51+
&mut self.reader
52+
}
53+
}
54+
55+
/// Wrapper around a map of components to [`Events<Entity>`].
56+
/// So that we can find the events without naming the type directly.
57+
#[derive(Default, Debug)]
58+
pub struct RemovedComponentEvents {
59+
event_sets: SparseSet<ComponentId, Events<Entity>>,
60+
}
61+
62+
impl RemovedComponentEvents {
63+
pub fn new() -> Self {
64+
Self::default()
65+
}
66+
67+
pub fn update(&mut self) {
68+
for (_component_id, events) in self.event_sets.iter_mut() {
69+
events.update();
70+
}
71+
}
72+
73+
pub fn get(&self, component_id: impl Into<ComponentId>) -> Option<&Events<Entity>> {
74+
self.event_sets.get(component_id.into())
75+
}
76+
77+
pub fn send(&mut self, component_id: impl Into<ComponentId>, entity: Entity) {
78+
self.event_sets
79+
.get_or_insert_with(component_id.into(), Default::default)
80+
.send(entity);
81+
}
82+
}
83+
84+
/// A [`SystemParam`] that grants access to the entities that had their `T` [`Component`] removed.
85+
///
86+
/// Note that this does not allow you to see which data existed before removal.
87+
/// If you need this, you will need to track the component data value on your own,
88+
/// using a regularly scheduled system that requests `Query<(Entity, &T), Changed<T>>`
89+
/// and stores the data somewhere safe to later cross-reference.
90+
///
91+
/// If you are using `bevy_ecs` as a standalone crate,
92+
/// note that the `RemovedComponents` list will not be automatically cleared for you,
93+
/// and will need to be manually flushed using [`World::clear_trackers`](crate::world::World::clear_trackers)
94+
///
95+
/// For users of `bevy` and `bevy_app`, this is automatically done in `bevy_app::App::update`.
96+
/// For the main world, [`World::clear_trackers`](crate::world::World::clear_trackers) is run after the main schedule is run and after
97+
/// `SubApp`'s have run.
98+
///
99+
/// # Examples
100+
///
101+
/// Basic usage:
102+
///
103+
/// ```
104+
/// # use bevy_ecs::component::Component;
105+
/// # use bevy_ecs::system::IntoSystem;
106+
/// # use bevy_ecs::removal_detection::RemovedComponents;
107+
/// #
108+
/// # #[derive(Component)]
109+
/// # struct MyComponent;
110+
/// fn react_on_removal(mut removed: RemovedComponents<MyComponent>) {
111+
/// removed.iter().for_each(|removed_entity| println!("{:?}", removed_entity));
112+
/// }
113+
/// # bevy_ecs::system::assert_is_system(react_on_removal);
114+
/// ```
115+
#[derive(SystemParam)]
116+
pub struct RemovedComponents<'w, 's, T: Component> {
117+
component_id: Local<'s, ComponentIdFor<T>>,
118+
reader: Local<'s, RemovedComponentReader<T>>,
119+
event_sets: &'w RemovedComponentEvents,
120+
}
121+
122+
/// Iterator over entities that had a specific component removed.
123+
///
124+
/// See [`RemovedComponents`].
125+
pub type RemovedIter<'a> =
126+
iter::Flatten<option::IntoIter<iter::Cloned<ManualEventIterator<'a, Entity>>>>;
127+
128+
impl<'w, 's, T: Component> RemovedComponents<'w, 's, T> {
129+
pub fn iter(&mut self) -> RemovedIter<'_> {
130+
self.event_sets
131+
.get(**self.component_id)
132+
.map(|events| self.reader.iter(events).cloned())
133+
.into_iter()
134+
.flatten()
135+
}
136+
}
137+
138+
impl<'a, 'w, 's: 'a, T> IntoIterator for &'a mut RemovedComponents<'w, 's, T>
139+
where
140+
T: Component,
141+
{
142+
type Item = Entity;
143+
type IntoIter = RemovedIter<'a>;
144+
fn into_iter(self) -> Self::IntoIter {
145+
self.iter()
146+
}
147+
}
148+
149+
// SAFETY: Only reads World removed component events
150+
unsafe impl<'a> ReadOnlySystemParam for &'a RemovedComponentEvents {}
151+
152+
// SAFETY: no component value access, removed component events can be read in parallel and are
153+
// never mutably borrowed during system execution
154+
unsafe impl<'a> SystemParam for &'a RemovedComponentEvents {
155+
type State = ();
156+
type Item<'w, 's> = &'w RemovedComponentEvents;
157+
158+
fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {}
159+
160+
#[inline]
161+
unsafe fn get_param<'w, 's>(
162+
_state: &'s mut Self::State,
163+
_system_meta: &SystemMeta,
164+
world: &'w World,
165+
_change_tick: u32,
166+
) -> Self::Item<'w, 's> {
167+
world.removed_components()
168+
}
169+
}

crates/bevy_ecs/src/system/mod.rs

+9-5
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
//! - [`NonSend`] and `Option<NonSend>`
8585
//! - [`NonSendMut`] and `Option<NonSendMut>`
8686
//! - [`&World`](crate::world::World)
87-
//! - [`RemovedComponents`]
87+
//! - [`RemovedComponents`](crate::removal_detection::RemovedComponents)
8888
//! - [`SystemName`]
8989
//! - [`SystemChangeTick`]
9090
//! - [`Archetypes`](crate::archetype::Archetypes) (Provides Archetype metadata)
@@ -139,10 +139,11 @@ mod tests {
139139
entity::{Entities, Entity},
140140
prelude::{AnyOf, StageLabel},
141141
query::{Added, Changed, Or, With, Without},
142+
removal_detection::RemovedComponents,
142143
schedule::{Schedule, Stage, SystemStage},
143144
system::{
144145
Commands, IntoSystem, Local, NonSend, NonSendMut, ParamSet, Query, QueryComponentError,
145-
RemovedComponents, Res, ResMut, Resource, System, SystemState,
146+
Res, ResMut, Resource, System, SystemState,
146147
},
147148
world::{FromWorld, World},
148149
};
@@ -602,7 +603,7 @@ mod tests {
602603
world.entity_mut(spurious_entity).despawn();
603604

604605
fn validate_despawn(
605-
removed_i32: RemovedComponents<W<i32>>,
606+
mut removed_i32: RemovedComponents<W<i32>>,
606607
despawned: Res<Despawned>,
607608
mut n_systems: ResMut<NSystems>,
608609
) {
@@ -627,13 +628,16 @@ mod tests {
627628
world.entity_mut(entity_to_remove_w_from).remove::<W<i32>>();
628629

629630
fn validate_remove(
630-
removed_i32: RemovedComponents<W<i32>>,
631+
mut removed_i32: RemovedComponents<W<i32>>,
632+
despawned: Res<Despawned>,
631633
removed: Res<Removed>,
632634
mut n_systems: ResMut<NSystems>,
633635
) {
636+
// The despawned entity from the previous frame was
637+
// double buffered so we now have it in this system as well.
634638
assert_eq!(
635639
removed_i32.iter().collect::<Vec<_>>(),
636-
&[removed.0],
640+
&[despawned.0, removed.0],
637641
"removing a component causes the correct entity to show up in the 'RemovedComponent' system parameter."
638642
);
639643

0 commit comments

Comments
 (0)