Mirrors/bevy
2
0
Fork 0
mirror of https://github.com/bevyengine/bevy synced 2024-11-10 07:04:33 +00:00
Code Issues Projects Releases Packages Wiki Activity

Removed Type Parameters from Observer (#15151)

Browse source 
# Objective

- Remove any ambiguity around how multiple `Observer` components work on
a single `Entity` by completely removing the concept.
- Fixes #15122

## Solution

- Removed type parameters from `Observer`, relying on a function pointer
to provide type information into the relevant aspects of running an
observer.

## Testing

- Ran CI locally.
- Checked `observers.rs` example continued to function as expected.

## Notes

This communicates to users of observers that only a single `Observer`
can be inserted onto an entity at a time within the established type
system. This has been achieved by erasing the type information from the
stored `ObserverSystem` and retrieving it again using a function
pointer. This has the downside of increasing the size of the `Observer`
component and increases the complexity of the observer runner. However,
this complexity was already present, and is in my opinion a worthwhile
tradeoff for the clearer user experience.

The other notable benefit is users no longer need to use the
`ObserverState` component to filter for `Observer` entities, and can
instead use `Observer` directly.

Technically this is a breaking change, since the type signature for
`Observer` has changed. However, it was so cumbersome to use that I
don't believe there are any instances in the wild of users directly
naming `Observer` types, instead relying on `ObserverState`, and the
methods provided by `App` and `World`. As can be seen in the diff, this
change had very little knock-on effects across Bevy.

## Migration Guide

If you filtered for observers using `Observer<A, B>`, instead filter for
an `Observer`.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
This commit is contained in:
Zachary Harrold 2024-09-11 11:14:28 +10:00 committed by GitHub
parent 37316c7706
commit be35cba801
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 81 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

141
crates/bevy_ecs/src/observer/runner.rs
View file

@ -1,5 +1,7 @@
use std::any::Any;
use crate::{
component::{ComponentHooks, ComponentId, StorageType},
component::{ComponentHook, ComponentHooks, ComponentId, StorageType},
observer::{ObserverDescriptor, ObserverTrigger},
prelude::*,
query::DebugCheckedUnwrap,
@ -257,22 +259,23 @@ pub type ObserverRunner = fn(DeferredWorld, ObserverTrigger, PtrMut, propagate:
/// You can call [`Observer::watch_entity`] more than once, which allows you to watch multiple entities with the same [`Observer`].
///
/// When first added, [`Observer`] will also create an [`ObserverState`] component, which registers the observer with the [`World`] and
/// serves as the "source of truth" of the observer. [`ObserverState`] can be used to filter for [`Observer`] [`Entity`]s, e.g.
/// [`With<ObserverState>`].
/// serves as the "source of truth" of the observer.
///
/// [`SystemParam`]: crate::system::SystemParam
pub struct Observer<T: 'static, B: Bundle> {
system: BoxedObserverSystem<T, B>,
pub struct Observer {
system: Box<dyn Any + Send + Sync + 'static>,
descriptor: ObserverDescriptor,
hook_on_add: ComponentHook,
}
impl<E: Event, B: Bundle> Observer<E, B> {
impl Observer {
/// Creates a new [`Observer`], which defaults to a "global" observer. This means it will run whenever the event `E` is triggered
/// for _any_ entity (or no entity).
pub fn new<M>(system: impl IntoObserverSystem<E, B, M>) -> Self {
pub fn new<E: Event, B: Bundle, M, I: IntoObserverSystem<E, B, M>>(system: I) -> Self {
Self {
system: Box::new(IntoObserverSystem::into_system(system)),
descriptor: Default::default(),
hook_on_add: hook_on_add::<E, B, I::System>,
}
}
@ -308,54 +311,20 @@ impl<E: Event, B: Bundle> Observer<E, B> {
}
}
impl<E: Event, B: Bundle> Component for Observer<E, B> {
impl Component for Observer {
const STORAGE_TYPE: StorageType = StorageType::SparseSet;
fn register_component_hooks(hooks: &mut ComponentHooks) {
hooks.on_add(|mut world, entity, _| {
world.commands().add(move |world: &mut World| {
let event_type = world.init_component::<E>();
let mut components = Vec::new();
B::component_ids(&mut world.components, &mut world.storages, &mut |id| {
components.push(id);
});
let mut descriptor = ObserverDescriptor {
events: vec![event_type],
components,
..Default::default()
};
// Initialize System
let system: *mut dyn ObserverSystem<E, B> =
if let Some(mut observe) = world.get_mut::<Self>(entity) {
descriptor.merge(&observe.descriptor);
&mut *observe.system
} else {
return;
};
// SAFETY: World reference is exclusive and initialize does not touch system, so references do not alias
unsafe {
(*system).initialize(world);
}
{
let mut entity = world.entity_mut(entity);
if let crate::world::Entry::Vacant(entry) = entity.entry::<ObserverState>() {
entry.insert(ObserverState {
descriptor,
runner: observer_system_runner::<E, B>,
..Default::default()
});
}
}
});
hooks.on_add(|world, entity, _id| {
let Some(observe) = world.get::<Self>(entity) else {
return;
};
let hook = observe.hook_on_add;
hook(world, entity, _id);
});
}
}
/// Equivalent to [`BoxedSystem`](crate::system::BoxedSystem) for [`ObserverSystem`].
pub type BoxedObserverSystem<E = (), B = ()> = Box<dyn ObserverSystem<E, B>>;
fn observer_system_runner<E: Event, B: Bundle>(
fn observer_system_runner<E: Event, B: Bundle, S: ObserverSystem<E, B>>(
mut world: DeferredWorld,
observer_trigger: ObserverTrigger,
ptr: PtrMut,
@ -395,23 +364,75 @@ fn observer_system_runner<E: Event, B: Bundle>(
// This transmute is obviously not ideal, but it is safe. Ideally we can remove the
// static constraint from ObserverSystem, but so far we have not found a way.
let trigger: Trigger<'static, E, B> = unsafe { std::mem::transmute(trigger) };
// SAFETY: Observer was triggered so must have an `Observer` component.
let system = unsafe {
&mut observer_cell
.get_mut::<Observer<E, B>>()
.debug_checked_unwrap()
.system
// SAFETY:
// - observer was triggered so must have an `Observer` component.
// - observer cannot be dropped or mutated until after the system pointer is already dropped.
let system: *mut dyn ObserverSystem<E, B> = unsafe {
let mut observe = observer_cell.get_mut::<Observer>().debug_checked_unwrap();
let system = observe.system.downcast_mut::<S>().unwrap();
&mut *system
};
system.update_archetype_component_access(world);
// SAFETY:
// - `update_archetype_component_access` was just called
// - `update_archetype_component_access` is called first
// - there are no outstanding references to world except a private component
// - system is an `ObserverSystem` so won't mutate world beyond the access of a `DeferredWorld`
// - system is the same type erased system from above
unsafe {
system.run_unsafe(trigger, world);
system.queue_deferred(world.into_deferred());
(*system).update_archetype_component_access(world);
(*system).run_unsafe(trigger, world);
(*system).queue_deferred(world.into_deferred());
}
}
/// A [`ComponentHook`] used by [`Observer`] to handle its [`on-add`](`ComponentHooks::on_add`).
///
/// This function exists separate from [`Observer`] to allow [`Observer`] to have its type parameters
/// erased.
///
/// The type parameters of this function _must_ match those used to create the [`Observer`].
/// As such, it is recommended to only use this function within the [`Observer::new`] method to
/// ensure type parameters match.
fn hook_on_add<E: Event, B: Bundle, S: ObserverSystem<E, B>>(
mut world: DeferredWorld<'_>,
entity: Entity,
_: ComponentId,
) {
world.commands().add(move |world: &mut World| {
let event_type = world.init_component::<E>();
let mut components = Vec::new();
B::component_ids(&mut world.components, &mut world.storages, &mut |id| {
components.push(id);
});
let mut descriptor = ObserverDescriptor {
events: vec![event_type],
components,
..Default::default()
};
// Initialize System
let system: *mut dyn ObserverSystem<E, B> =
if let Some(mut observe) = world.get_mut::<Observer>(entity) {
descriptor.merge(&observe.descriptor);
let system = observe.system.downcast_mut::<S>().unwrap();
&mut *system
} else {
return;
};
// SAFETY: World reference is exclusive and initialize does not touch system, so references do not alias
unsafe {
(*system).initialize(world);
}
{
let mut entity = world.entity_mut(entity);
if let crate::world::Entry::Vacant(entry) = entity.entry::<ObserverState>() {
entry.insert(ObserverState {
descriptor,
runner: observer_system_runner::<E, B, S>,
..Default::default()
});
}
}
});
}