feat(ecs): emit Removed events before component removal

4 files changed, 120 insertions, 72 deletions

diff --git a/ecs/src/actions.rs b/ecs/src/actions.rs
index ba3ced5..f8a59fa 100644
--- a/ecs/src/actions.rs
+++ b/ecs/src/actions.rs
@@ -1,7 +1,10 @@
+use std::any::type_name;
 use std::marker::PhantomData;
 use std::rc::{Rc, Weak};
 
 use crate::component::{Parts as ComponentParts, Sequence as ComponentSequence};
+use crate::event::component::Removed;
+use crate::pair::Pair;
 use crate::system::{Metadata as SystemMetadata, Param as SystemParam};
 use crate::uid::{Kind as UidKind, Uid};
 use crate::{ActionQueue, World};
@@ -11,10 +14,10 @@ use crate::{ActionQueue, World};
 pub struct Actions<'world>
 {
     action_queue: &'world ActionQueue,
-    action_queue_weak: Weak<ActionQueue>,
+    world: Option<&'world World>,
 }
 
-impl<'world> Actions<'world>
+impl Actions<'_>
 {
     /// Queues up a entity to spawn at the end of the current tick, returning the [`Uid`]
     /// that the entity will have.
@@ -35,6 +38,31 @@ impl<'world> Actions<'world>
     {
         debug_assert_eq!(entity_uid.kind(), UidKind::Entity);
 
+        let Some(world) = self.world else {
+            self.action_queue.push(Action::Despawn(entity_uid));
+            return;
+        };
+
+        let Some(ent) = world.get_entity(entity_uid) else {
+            tracing::warn!("Cannot entity that doesn't exist");
+            return;
+        };
+
+        // TODO: Submit all events with a single function call to reduce overhead
+        for comp_id in ent.component_ids() {
+            if comp_id.kind() == UidKind::Pair {
+                continue;
+            }
+
+            world.event_submitter().submit_event(
+                &Pair::builder()
+                    .relation::<Removed>()
+                    .target_id(comp_id)
+                    .build(),
+                entity_uid,
+            );
+        }
+
         self.action_queue.push(Action::Despawn(entity_uid));
     }
 
@@ -56,6 +84,7 @@ impl<'world> Actions<'world>
     }
 
     /// Queues up removing component(s) from a entity at the end of the current tick.
+    #[tracing::instrument(skip(self, component_ids))]
     pub fn remove_components(
         &mut self,
         entity_uid: Uid,
@@ -70,10 +99,44 @@ impl<'world> Actions<'world>
             return;
         }
 
-        self.action_queue.push(Action::RemoveComponents(
-            entity_uid,
-            component_ids.collect(),
-        ));
+        let Some(world) = self.world else {
+            self.action_queue.push(Action::RemoveComponents(
+                entity_uid,
+                component_ids.collect(),
+            ));
+            return;
+        };
+
+        let Some(ent) = world.get_entity(entity_uid) else {
+            tracing::warn!("Cannot remove components from entity that doesn't exist");
+            return;
+        };
+
+        let component_ids = component_ids
+            .filter(|comp_id| ent.has_component(*comp_id))
+            .collect::<Vec<_>>();
+
+        if component_ids.is_empty() {
+            return;
+        }
+
+        // TODO: Submit all events with a single function call to reduce overhead
+        for comp_id in &component_ids {
+            if comp_id.kind() == UidKind::Pair {
+                continue;
+            }
+
+            world.event_submitter().submit_event(
+                &Pair::builder()
+                    .relation::<Removed>()
+                    .target_id(*comp_id)
+                    .build(),
+                entity_uid,
+            );
+        }
+
+        self.action_queue
+            .push(Action::RemoveComponents(entity_uid, component_ids));
     }
 
     /// Stops the [`World`]. The world will finish the current tick and that tick will be
@@ -86,19 +149,22 @@ impl<'world> Actions<'world>
     /// Returns a struct which holds a weak reference to the [`World`] that `Actions`
     /// references and that can be used to aquire a new `Actions` instance if the
     /// referenced [`World`] is still alive.
+    ///
+    /// # Panics
+    /// This function will panic if `self` was retrieved from a [`WeakRef`].
     #[must_use]
     pub fn to_weak_ref(&self) -> WeakRef
     {
-        WeakRef {
-            action_queue: self.action_queue_weak.clone(),
-        }
-    }
+        let world = self.world.unwrap_or_else(|| {
+            panic!(
+                "This function cannot be called if the {} was retrieved from a {}",
+                type_name::<Self>(),
+                type_name::<WeakRef>()
+            )
+        });
 
-    fn new(action_queue: &'world Rc<ActionQueue>) -> Self
-    {
-        Self {
-            action_queue,
-            action_queue_weak: Rc::downgrade(action_queue),
+        WeakRef {
+            action_queue: Rc::downgrade(&world.data.action_queue),
         }
     }
 }
@@ -109,7 +175,10 @@ impl<'world> SystemParam<'world> for Actions<'world>
 
     fn new(world: &'world World, _system_metadata: &SystemMetadata) -> Self
     {
-        Self::new(&world.data.action_queue)
+        Self {
+            action_queue: &world.data.action_queue,
+            world: Some(world),
+        }
     }
 }
 
@@ -148,7 +217,10 @@ impl Ref<'_>
     #[must_use]
     pub fn to_actions(&self) -> Actions<'_>
     {
-        Actions::new(&self.action_queue)
+        Actions {
+            action_queue: &self.action_queue,
+            world: None,
+        }
     }
 }
 
diff --git a/ecs/src/event.rs b/ecs/src/event.rs
index a0b2487..fc3a58b 100644
--- a/ecs/src/event.rs
+++ b/ecs/src/event.rs
@@ -78,6 +78,11 @@ impl NewEvents
     {
         std::mem::take(&mut self.events)
     }
+
+    pub fn is_empty(&self) -> bool
+    {
+        self.events.is_empty()
+    }
 }
 
 #[derive(Debug)]
diff --git a/ecs/src/event/component.rs b/ecs/src/event/component.rs
index e8b9559..96761a6 100644
--- a/ecs/src/event/component.rs
+++ b/ecs/src/event/component.rs
@@ -10,7 +10,7 @@ use crate::Component;
 #[derive(Debug, Component)]
 pub struct Added(Infallible);
 
-/// Pair relation for events emitted when:
+/// Pair relation for events emitted **before**:
 /// a) The target component is removed from a entity.
 /// b) A entity with the target component is despawned.
 #[derive(Debug, Component)]
diff --git a/ecs/src/lib.rs b/ecs/src/lib.rs
index fce780c..48a770e 100644
--- a/ecs/src/lib.rs
+++ b/ecs/src/lib.rs
@@ -19,7 +19,7 @@ use crate::component::{
     Sequence as ComponentSequence,
 };
 use crate::entity::{Declaration as EntityDeclaration, Handle as EntityHandle};
-use crate::event::component::{Added, Removed};
+use crate::event::component::Added;
 use crate::event::{Emitted as EmittedEvent, NewEvents, Submitter as EventSubmitter};
 use crate::extension::{Collector as ExtensionCollector, Extension};
 use crate::lock::Lock;
@@ -408,21 +408,30 @@ impl World
 
     fn emit_new_events(&self)
     {
-        let new_events = self
-            .data
-            .new_events
-            .write_nonblock()
-            .expect("Failed to acquire read-write lock to new events")
-            .take();
-
-        for (event_id, event_matches) in new_events {
-            self.emit_event_observers(
-                event_id,
-                &EmittedEvent {
-                    event: event_id,
-                    match_ids: &event_matches.match_ids,
-                },
-            );
+        loop {
+            let new_events = {
+                let mut new_events_lock = self
+                    .data
+                    .new_events
+                    .write_nonblock()
+                    .expect("Failed to acquire read-write lock to new events");
+
+                if new_events_lock.is_empty() {
+                    break;
+                }
+
+                new_events_lock.take()
+            };
+
+            for (event_id, event_matches) in new_events {
+                self.emit_event_observers(
+                    event_id,
+                    &EmittedEvent {
+                        event: event_id,
+                        match_ids: &event_matches.match_ids,
+                    },
+                );
+            }
         }
     }
 
@@ -456,33 +465,10 @@ impl World
                     );
                 }
                 Action::Despawn(entity_uid) => {
-                    let component_ids = self
-                        .get_entity(entity_uid)
-                        .expect("Not possible")
-                        .component_ids()
-                        .collect::<Vec<_>>();
-
                     if let Err(err) =
                         self.data.component_storage.remove_entity(entity_uid)
                     {
                         tracing::error!("Failed to despawn entity: {err}");
-                        continue;
-                    }
-
-                    let event_submitter = EventSubmitter::new(&self.data.new_events);
-
-                    for comp_id in component_ids {
-                        if comp_id.kind() == UidKind::Pair {
-                            continue;
-                        }
-
-                        event_submitter.submit_event(
-                            &Pair::builder()
-                                .relation::<Removed>()
-                                .target_id(comp_id)
-                                .build(),
-                            entity_uid,
-                        );
                     }
                 }
                 Action::AddComponents(entity_uid, components) => {
@@ -498,7 +484,6 @@ impl World
                         entity_uid,
                         component_ids,
                         &mut self.data.component_storage,
-                        &EventSubmitter::new(&self.data.new_events),
                     );
                 }
                 Action::Stop => {
@@ -548,7 +533,6 @@ impl World
         entity_uid: Uid,
         component_ids: impl IntoIterator<Item = Uid>,
         component_storage: &mut ComponentStorage,
-        event_submitter: &EventSubmitter<'_>,
     )
     {
         let component_id_iter = component_ids.into_iter();
@@ -558,20 +542,7 @@ impl World
                 component_storage.remove_entity_component(entity_uid, component_id)
             {
                 tracing::error!("Failed to remove component to entity: {err}");
-                continue;
-            }
-
-            if component_id.kind() == UidKind::Pair {
-                continue;
             }
-
-            event_submitter.submit_event(
-                &Pair::builder()
-                    .relation::<Removed>()
-                    .target_id(component_id)
-                    .build(),
-                entity_uid,
-            );
         }
     }