From 0daa6c510b1cc16d60b7df03be8c661456a06a55 Mon Sep 17 00:00:00 2001
From: Carter Anderson <mcanders1@gmail.com>
Date: Fri, 21 Jun 2024 11:31:01 -0700
Subject: [PATCH] Make Observer::with_event (and other variants) unsafe
 (#13954)

# Objective

`with_event` will result in unsafe casting of event data of the given
type to the type expected by the Observer system. This is inherently
unsafe.

## Solution

Flag `Observer::with_event` and `ObserverDescriptor::with_events` as
unsafe. This will not affect normal workflows as `with_event` is
intended for very specific (largely internal) use cases.

This _should_ be backported to 0.14 before release.

---

## Changelog

- `Observer::with_event` is now unsafe.
- Rename `ObserverDescriptor::with_triggers` to
`ObserverDescriptor::with_events` and make it unsafe.
---
 crates/bevy_ecs/src/observer/mod.rs    | 21 ++++++++++++++-------
 crates/bevy_ecs/src/observer/runner.rs |  5 ++++-
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs
index b1876afddd..b3d689d449 100644
--- a/crates/bevy_ecs/src/observer/mod.rs
+++ b/crates/bevy_ecs/src/observer/mod.rs
@@ -72,9 +72,12 @@ pub struct ObserverDescriptor {
 }
 
 impl ObserverDescriptor {
-    /// Add the given `triggers` to the descriptor.
-    pub fn with_triggers(mut self, triggers: Vec<ComponentId>) -> Self {
-        self.events = triggers;
+    /// Add the given `events` to the descriptor.
+    /// # Safety
+    /// The type of each [`ComponentId`] in `events` _must_ match the actual value
+    /// of the event passed into the observer.
+    pub unsafe fn with_events(mut self, events: Vec<ComponentId>) -> Self {
+        self.events = events;
         self
     }
 
@@ -518,8 +521,11 @@ mod tests {
         world.init_resource::<R>();
         let on_remove = world.init_component::<OnRemove>();
         world.spawn(
-            Observer::new(|_: Trigger<OnAdd, A>, mut res: ResMut<R>| res.0 += 1)
-                .with_event(on_remove),
+            // SAFETY: OnAdd and OnRemove are both unit types, so this is safe
+            unsafe {
+                Observer::new(|_: Trigger<OnAdd, A>, mut res: ResMut<R>| res.0 += 1)
+                    .with_event(on_remove)
+            },
         );
 
         let entity = world.spawn(A).id();
@@ -641,7 +647,8 @@ mod tests {
         let event_a = world.init_component::<EventA>();
 
         world.spawn(ObserverState {
-            descriptor: ObserverDescriptor::default().with_triggers(vec![event_a]),
+            // SAFETY: we registered `event_a` above and it matches the type of TriggerA
+            descriptor: unsafe { ObserverDescriptor::default().with_events(vec![event_a]) },
             runner: |mut world, _trigger, _ptr| {
                 world.resource_mut::<R>().0 += 1;
             },
@@ -649,7 +656,7 @@ mod tests {
         });
 
         world.commands().add(
-            // SAFETY: we registered `trigger` above and it matches the type of TriggerA
+            // SAFETY: we registered `event_a` above and it matches the type of TriggerA
             unsafe { EmitDynamicTrigger::new_with_id(event_a, EventA, ()) },
         );
         world.flush();
diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs
index f4d5384348..40092be553 100644
--- a/crates/bevy_ecs/src/observer/runner.rs
+++ b/crates/bevy_ecs/src/observer/runner.rs
@@ -298,7 +298,10 @@ impl<E: Event, B: Bundle> Observer<E, B> {
 
     /// Observe the given `event`. This will cause the [`Observer`] to run whenever an event with the given [`ComponentId`]
     /// is triggered.
-    pub fn with_event(mut self, event: ComponentId) -> Self {
+    /// # Safety
+    /// The type of the `event` [`ComponentId`] _must_ match the actual value
+    /// of the event passed into the observer system.
+    pub unsafe fn with_event(mut self, event: ComponentId) -> Self {
         self.descriptor.events.push(event);
         self
     }