From 2636465f9290a8b5fc9459ab36958485b09fb6d1 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Sun, 29 Dec 2024 16:18:53 -0600 Subject: [PATCH] Move some structs that `impl Command` to methods on `World` and `EntityWorldMut` (#16999) ## Objective Commands were previously limited to structs that implemented `Command`. Now there are blanket implementations for closures, which (in my opinion) are generally preferable. Internal commands within `commands/mod.rs` have been switched from structs to closures, but there are a number of internal commands in other areas of the engine that still use structs. I'd like to tidy these up by moving their implementations to methods on `World`/`EntityWorldMut` and changing `Commands` to use those methods through closures. This PR handles the following: - `TriggerEvent` and `EmitDynamicTrigger` double as commands and helper structs, and can just be moved to `World` methods. - Four structs that enabled insertion/removal of components via reflection. This functionality shouldn't be exclusive to commands, and can be added to `EntityWorldMut`. - Five structs that mostly just wrapped `World` methods, and can be replaced with closures that do the same thing. ## Solution - __Observer Triggers__ (`observer/trigger_event.rs` and `observer/mod.rs`) - Moved the internals of `TriggerEvent` to the `World` methods that used it. - Replaced `EmitDynamicTrigger` with two `World` methods: - `trigger_targets_dynamic` - `trigger_targets_dynamic_ref` - `TriggerTargets` was now the only thing in `observer/trigger_event.rs`, so it's been moved to `observer/mod.rs` and `trigger_event.rs` was deleted. - __Reflection Insert/Remove__ (`reflect/entity_commands.rs`) - Replaced the following `Command` impls with equivalent methods on `EntityWorldMut`: - `InsertReflect` -> `insert_reflect` - `InsertReflectWithRegistry` -> `insert_reflect_with_registry` - `RemoveReflect` -> `remove_reflect` - `RemoveReflectWithRegistry` -> `remove_reflect_with_registry` - __System Registration__ (`system/system_registry.rs`) - The following `Command` impls just wrapped a `World` method and have been replaced with closures: - `RunSystemWith` - `UnregisterSystem` - `RunSystemCachedWith` - `UnregisterSystemCached` - `RegisterSystem` called a helper function that basically worked as a constructor for `RegisteredSystem` and made sure it came with a marker component. That helper function has been replaced with `RegisteredSystem::new` and a `#[require]`. ## Possible Addition The extension trait that adds the reflection commands, `ReflectCommandExt`, isn't strictly necessary; we could just `impl EntityCommands`. We could even move them to the same files as the main impls and put it behind a `#[cfg]`. The PR that added it [had a similar conversation](https://github.com/bevyengine/bevy/pull/8895#discussion_r1234713671) and decided to stick with the trait, but we could revisit it here if so desired. --- crates/bevy_ecs/src/observer/mod.rs | 194 +++++++++++-- crates/bevy_ecs/src/observer/trigger_event.rs | 196 ------------- .../bevy_ecs/src/reflect/entity_commands.rs | 266 ++++++++++-------- crates/bevy_ecs/src/system/commands/mod.rs | 52 +++- crates/bevy_ecs/src/system/system_registry.rs | 229 ++------------- crates/bevy_ecs/src/world/entity_ref.rs | 2 +- 6 files changed, 390 insertions(+), 549 deletions(-) delete mode 100644 crates/bevy_ecs/src/observer/trigger_event.rs diff --git a/crates/bevy_ecs/src/observer/mod.rs b/crates/bevy_ecs/src/observer/mod.rs index c78484c6a6ff5..2f690973481f3 100644 --- a/crates/bevy_ecs/src/observer/mod.rs +++ b/crates/bevy_ecs/src/observer/mod.rs @@ -2,11 +2,9 @@ mod entity_observer; mod runner; -mod trigger_event; pub use entity_observer::CloneEntityWithObserversExt; pub use runner::*; -pub use trigger_event::*; use crate::{ archetype::ArchetypeFlags, @@ -168,6 +166,98 @@ impl<'w, E, B: Bundle> DerefMut for Trigger<'w, E, B> { } } +/// Represents a collection of targets for a specific [`Trigger`] of an [`Event`]. Targets can be of type [`Entity`] or [`ComponentId`]. +/// +/// When a trigger occurs for a given event and [`TriggerTargets`], any [`Observer`] that watches for that specific event-target combination +/// will run. +pub trait TriggerTargets { + /// The components the trigger should target. + fn components(&self) -> &[ComponentId]; + + /// The entities the trigger should target. + fn entities(&self) -> &[Entity]; +} + +impl TriggerTargets for () { + fn components(&self) -> &[ComponentId] { + &[] + } + + fn entities(&self) -> &[Entity] { + &[] + } +} + +impl TriggerTargets for Entity { + fn components(&self) -> &[ComponentId] { + &[] + } + + fn entities(&self) -> &[Entity] { + core::slice::from_ref(self) + } +} + +impl TriggerTargets for Vec { + fn components(&self) -> &[ComponentId] { + &[] + } + + fn entities(&self) -> &[Entity] { + self.as_slice() + } +} + +impl TriggerTargets for [Entity; N] { + fn components(&self) -> &[ComponentId] { + &[] + } + + fn entities(&self) -> &[Entity] { + self.as_slice() + } +} + +impl TriggerTargets for ComponentId { + fn components(&self) -> &[ComponentId] { + core::slice::from_ref(self) + } + + fn entities(&self) -> &[Entity] { + &[] + } +} + +impl TriggerTargets for Vec { + fn components(&self) -> &[ComponentId] { + self.as_slice() + } + + fn entities(&self) -> &[Entity] { + &[] + } +} + +impl TriggerTargets for [ComponentId; N] { + fn components(&self) -> &[ComponentId] { + self.as_slice() + } + + fn entities(&self) -> &[Entity] { + &[] + } +} + +impl TriggerTargets for &Vec { + fn components(&self) -> &[ComponentId] { + &[] + } + + fn entities(&self) -> &[Entity] { + self.as_slice() + } +} + /// A description of what an [`Observer`] observes. #[derive(Default, Clone)] pub struct ObserverDescriptor { @@ -431,16 +521,20 @@ impl World { /// While event types commonly implement [`Copy`], /// those that don't will be consumed and will no longer be accessible. /// If you need to use the event after triggering it, use [`World::trigger_ref`] instead. - pub fn trigger(&mut self, event: impl Event) { - TriggerEvent { event, targets: () }.trigger(self); + pub fn trigger(&mut self, mut event: E) { + let event_id = self.register_component::(); + // SAFETY: We just registered `event_id` with the type of `event` + unsafe { self.trigger_targets_dynamic_ref(event_id, &mut event, ()) }; } /// Triggers the given [`Event`] as a mutable reference, which will run any [`Observer`]s watching for it. /// /// Compared to [`World::trigger`], this method is most useful when it's necessary to check /// or use the event after it has been modified by observers. - pub fn trigger_ref(&mut self, event: &mut impl Event) { - TriggerEvent { event, targets: () }.trigger_ref(self); + pub fn trigger_ref(&mut self, event: &mut E) { + let event_id = self.register_component::(); + // SAFETY: We just registered `event_id` with the type of `event` + unsafe { self.trigger_targets_dynamic_ref(event_id, event, ()) }; } /// Triggers the given [`Event`] for the given `targets`, which will run any [`Observer`]s watching for it. @@ -448,8 +542,10 @@ impl World { /// While event types commonly implement [`Copy`], /// those that don't will be consumed and will no longer be accessible. /// If you need to use the event after triggering it, use [`World::trigger_targets_ref`] instead. - pub fn trigger_targets(&mut self, event: impl Event, targets: impl TriggerTargets) { - TriggerEvent { event, targets }.trigger(self); + pub fn trigger_targets(&mut self, mut event: E, targets: impl TriggerTargets) { + let event_id = self.register_component::(); + // SAFETY: We just registered `event_id` with the type of `event` + unsafe { self.trigger_targets_dynamic_ref(event_id, &mut event, targets) }; } /// Triggers the given [`Event`] as a mutable reference for the given `targets`, @@ -457,8 +553,74 @@ impl World { /// /// Compared to [`World::trigger_targets`], this method is most useful when it's necessary to check /// or use the event after it has been modified by observers. - pub fn trigger_targets_ref(&mut self, event: &mut impl Event, targets: impl TriggerTargets) { - TriggerEvent { event, targets }.trigger_ref(self); + pub fn trigger_targets_ref(&mut self, event: &mut E, targets: impl TriggerTargets) { + let event_id = self.register_component::(); + // SAFETY: We just registered `event_id` with the type of `event` + unsafe { self.trigger_targets_dynamic_ref(event_id, event, targets) }; + } + + /// Triggers the given [`Event`] for the given `targets`, which will run any [`Observer`]s watching for it. + /// + /// While event types commonly implement [`Copy`], + /// those that don't will be consumed and will no longer be accessible. + /// If you need to use the event after triggering it, use [`World::trigger_targets_dynamic_ref`] instead. + /// + /// # Safety + /// + /// Caller must ensure that `event_data` is accessible as the type represented by `event_id`. + pub unsafe fn trigger_targets_dynamic( + &mut self, + event_id: ComponentId, + mut event_data: E, + targets: Targets, + ) { + // SAFETY: `event_data` is accessible as the type represented by `event_id` + unsafe { + self.trigger_targets_dynamic_ref(event_id, &mut event_data, targets); + }; + } + + /// Triggers the given [`Event`] as a mutable reference for the given `targets`, + /// which will run any [`Observer`]s watching for it. + /// + /// Compared to [`World::trigger_targets_dynamic`], this method is most useful when it's necessary to check + /// or use the event after it has been modified by observers. + /// + /// # Safety + /// + /// Caller must ensure that `event_data` is accessible as the type represented by `event_id`. + pub unsafe fn trigger_targets_dynamic_ref( + &mut self, + event_id: ComponentId, + event_data: &mut E, + targets: Targets, + ) { + let mut world = DeferredWorld::from(self); + if targets.entities().is_empty() { + // SAFETY: `event_data` is accessible as the type represented by `event_id` + unsafe { + world.trigger_observers_with_data::<_, E::Traversal>( + event_id, + Entity::PLACEHOLDER, + targets.components(), + event_data, + false, + ); + }; + } else { + for target in targets.entities() { + // SAFETY: `event_data` is accessible as the type represented by `event_id` + unsafe { + world.trigger_observers_with_data::<_, E::Traversal>( + event_id, + *target, + targets.components(), + event_data, + E::AUTO_PROPAGATE, + ); + }; + } + } } /// Register an observer to the cache, called when an observer is created @@ -590,7 +752,7 @@ mod tests { use crate as bevy_ecs; use crate::component::ComponentId; use crate::{ - observer::{EmitDynamicTrigger, Observer, ObserverDescriptor, ObserverState, OnReplace}, + observer::{Observer, ObserverDescriptor, ObserverState, OnReplace}, prelude::*, traversal::Traversal, }; @@ -996,7 +1158,7 @@ mod tests { let event_a = world.register_component::(); world.spawn(ObserverState { - // SAFETY: we registered `event_a` above and it matches the type of TriggerA + // SAFETY: we registered `event_a` above and it matches the type of EventA descriptor: unsafe { ObserverDescriptor::default().with_events(vec![event_a]) }, runner: |mut world, _trigger, _ptr, _propagate| { world.resource_mut::().observed("event_a"); @@ -1004,10 +1166,10 @@ mod tests { ..Default::default() }); - world.commands().queue( - // SAFETY: we registered `event_a` above and it matches the type of TriggerA - unsafe { EmitDynamicTrigger::new_with_id(event_a, EventA, ()) }, - ); + world.commands().queue(move |world: &mut World| { + // SAFETY: we registered `event_a` above and it matches the type of EventA + unsafe { world.trigger_targets_dynamic(event_a, EventA, ()) }; + }); world.flush(); assert_eq!(vec!["event_a"], world.resource::().0); } diff --git a/crates/bevy_ecs/src/observer/trigger_event.rs b/crates/bevy_ecs/src/observer/trigger_event.rs deleted file mode 100644 index bf84e57bae301..0000000000000 --- a/crates/bevy_ecs/src/observer/trigger_event.rs +++ /dev/null @@ -1,196 +0,0 @@ -use crate::{ - component::ComponentId, - entity::Entity, - event::Event, - world::{Command, DeferredWorld, World}, -}; -use alloc::vec::Vec; - -/// A [`Command`] that emits a given trigger for a given set of targets. -pub struct TriggerEvent { - /// The event to trigger. - pub event: E, - - /// The targets to trigger the event for. - pub targets: Targets, -} - -impl TriggerEvent { - pub(super) fn trigger(mut self, world: &mut World) { - let event_type = world.register_component::(); - trigger_event(world, event_type, &mut self.event, self.targets); - } -} - -impl TriggerEvent<&mut E, Targets> { - pub(super) fn trigger_ref(self, world: &mut World) { - let event_type = world.register_component::(); - trigger_event(world, event_type, self.event, self.targets); - } -} - -impl Command - for TriggerEvent -{ - fn apply(self, world: &mut World) { - self.trigger(world); - } -} - -/// Emit a trigger for a dynamic component id. This is unsafe and must be verified manually. -pub struct EmitDynamicTrigger { - event_type: ComponentId, - event_data: T, - targets: Targets, -} - -impl EmitDynamicTrigger { - /// Sets the event type of the resulting trigger, used for dynamic triggers - /// # Safety - /// Caller must ensure that the component associated with `event_type` is accessible as E - pub unsafe fn new_with_id(event_type: ComponentId, event_data: E, targets: Targets) -> Self { - Self { - event_type, - event_data, - targets, - } - } -} - -impl Command - for EmitDynamicTrigger -{ - fn apply(mut self, world: &mut World) { - trigger_event(world, self.event_type, &mut self.event_data, self.targets); - } -} - -#[inline] -fn trigger_event( - world: &mut World, - event_type: ComponentId, - event_data: &mut E, - targets: Targets, -) { - let mut world = DeferredWorld::from(world); - if targets.entities().is_empty() { - // SAFETY: T is accessible as the type represented by self.trigger, ensured in `Self::new` - unsafe { - world.trigger_observers_with_data::<_, E::Traversal>( - event_type, - Entity::PLACEHOLDER, - targets.components(), - event_data, - false, - ); - }; - } else { - for target in targets.entities() { - // SAFETY: T is accessible as the type represented by self.trigger, ensured in `Self::new` - unsafe { - world.trigger_observers_with_data::<_, E::Traversal>( - event_type, - *target, - targets.components(), - event_data, - E::AUTO_PROPAGATE, - ); - }; - } - } -} - -/// Represents a collection of targets for a specific [`Trigger`] of an [`Event`]. Targets can be of type [`Entity`] or [`ComponentId`]. -/// -/// When a trigger occurs for a given event and [`TriggerTargets`], any [`Observer`] that watches for that specific event-target combination -/// will run. -/// -/// [`Trigger`]: crate::observer::Trigger -/// [`Observer`]: crate::observer::Observer -pub trait TriggerTargets { - /// The components the trigger should target. - fn components(&self) -> &[ComponentId]; - - /// The entities the trigger should target. - fn entities(&self) -> &[Entity]; -} - -impl TriggerTargets for () { - fn components(&self) -> &[ComponentId] { - &[] - } - - fn entities(&self) -> &[Entity] { - &[] - } -} - -impl TriggerTargets for Entity { - fn components(&self) -> &[ComponentId] { - &[] - } - - fn entities(&self) -> &[Entity] { - core::slice::from_ref(self) - } -} - -impl TriggerTargets for Vec { - fn components(&self) -> &[ComponentId] { - &[] - } - - fn entities(&self) -> &[Entity] { - self.as_slice() - } -} - -impl TriggerTargets for [Entity; N] { - fn components(&self) -> &[ComponentId] { - &[] - } - - fn entities(&self) -> &[Entity] { - self.as_slice() - } -} - -impl TriggerTargets for ComponentId { - fn components(&self) -> &[ComponentId] { - core::slice::from_ref(self) - } - - fn entities(&self) -> &[Entity] { - &[] - } -} - -impl TriggerTargets for Vec { - fn components(&self) -> &[ComponentId] { - self.as_slice() - } - - fn entities(&self) -> &[Entity] { - &[] - } -} - -impl TriggerTargets for [ComponentId; N] { - fn components(&self) -> &[ComponentId] { - self.as_slice() - } - - fn entities(&self) -> &[Entity] { - &[] - } -} - -impl TriggerTargets for &Vec { - fn components(&self) -> &[ComponentId] { - &[] - } - - fn entities(&self) -> &[Entity] { - self.as_slice() - } -} diff --git a/crates/bevy_ecs/src/reflect/entity_commands.rs b/crates/bevy_ecs/src/reflect/entity_commands.rs index 4bb77d9d8fc95..0d77e4b4b873d 100644 --- a/crates/bevy_ecs/src/reflect/entity_commands.rs +++ b/crates/bevy_ecs/src/reflect/entity_commands.rs @@ -3,11 +3,10 @@ use crate::{ prelude::Mut, reflect::{AppTypeRegistry, ReflectBundle, ReflectComponent}, system::{EntityCommands, Resource}, - world::{Command, World}, + world::{EntityWorldMut, World}, }; use alloc::{borrow::Cow, boxed::Box}; use bevy_reflect::{PartialReflect, TypeRegistry}; -use core::marker::PhantomData; /// An extension trait for [`EntityCommands`] for reflection related functions pub trait ReflectCommandExt { @@ -170,48 +169,183 @@ pub trait ReflectCommandExt { impl ReflectCommandExt for EntityCommands<'_> { fn insert_reflect(&mut self, component: Box) -> &mut Self { - self.commands.queue(InsertReflect { - entity: self.entity, - component, + self.queue(move |entity: Entity, world: &mut World| { + if let Ok(mut entity) = world.get_entity_mut(entity) { + entity.insert_reflect(component); + } + }) + } + + fn insert_reflect_with_registry>( + &mut self, + component: Box, + ) -> &mut Self { + self.queue(move |entity: Entity, world: &mut World| { + if let Ok(mut entity) = world.get_entity_mut(entity) { + entity.insert_reflect_with_registry::(component); + } + }) + } + + fn remove_reflect(&mut self, component_type_path: impl Into>) -> &mut Self { + let component_type_path: Cow<'static, str> = component_type_path.into(); + self.queue(move |entity: Entity, world: &mut World| { + if let Ok(mut entity) = world.get_entity_mut(entity) { + entity.remove_reflect(component_type_path); + } + }) + } + + fn remove_reflect_with_registry>( + &mut self, + component_type_path: impl Into>, + ) -> &mut Self { + let component_type_path: Cow<'static, str> = component_type_path.into(); + self.queue(move |entity: Entity, world: &mut World| { + if let Ok(mut entity) = world.get_entity_mut(entity) { + entity.remove_reflect_with_registry::(component_type_path); + } + }) + } +} + +impl<'w> EntityWorldMut<'w> { + /// Adds the given boxed reflect component or bundle to the entity using the reflection data in + /// [`AppTypeRegistry`]. + /// + /// This will overwrite any previous component(s) of the same type. + /// + /// # Panics + /// + /// - If the entity has been despawned while this `EntityWorldMut` is still alive. + /// - If [`AppTypeRegistry`] does not have the reflection data for the given + /// [`Component`](crate::component::Component) or [`Bundle`](crate::bundle::Bundle). + /// - If the component or bundle data is invalid. See [`PartialReflect::apply`] for further details. + /// - If [`AppTypeRegistry`] is not present in the [`World`]. + /// + /// # Note + /// + /// Prefer to use the typed [`EntityWorldMut::insert`] if possible. Adding a reflected component + /// is much slower. + pub fn insert_reflect(&mut self, component: Box) -> &mut Self { + self.assert_not_despawned(); + let entity_id = self.id(); + self.world_scope(|world| { + world.resource_scope(|world, registry: Mut| { + let type_registry = ®istry.as_ref().read(); + insert_reflect_with_registry_ref(world, entity_id, type_registry, component); + }); + world.flush(); }); + self.update_location(); self } - fn insert_reflect_with_registry>( + /// Same as [`insert_reflect`](EntityWorldMut::insert_reflect), but using + /// the `T` resource as type registry instead of [`AppTypeRegistry`]. + /// + /// This will overwrite any previous component(s) of the same type. + /// + /// # Panics + /// + /// - If the entity has been despawned while this `EntityWorldMut` is still alive. + /// - If the given [`Resource`] does not have the reflection data for the given + /// [`Component`](crate::component::Component) or [`Bundle`](crate::bundle::Bundle). + /// - If the component or bundle data is invalid. See [`PartialReflect::apply`] for further details. + /// - If the given [`Resource`] is not present in the [`World`]. + pub fn insert_reflect_with_registry>( &mut self, component: Box, ) -> &mut Self { - self.commands.queue(InsertReflectWithRegistry:: { - entity: self.entity, - _t: PhantomData, - component, + self.assert_not_despawned(); + let entity_id = self.id(); + self.world_scope(|world| { + world.resource_scope(|world, registry: Mut| { + let type_registry = registry.as_ref().as_ref(); + insert_reflect_with_registry_ref(world, entity_id, type_registry, component); + }); + world.flush(); }); + self.update_location(); self } - fn remove_reflect(&mut self, component_type_path: impl Into>) -> &mut Self { - self.commands.queue(RemoveReflect { - entity: self.entity, - component_type_path: component_type_path.into(), + /// Removes from the entity the component or bundle with the given type name registered in [`AppTypeRegistry`]. + /// + /// If the type is a bundle, it will remove any components in that bundle regardless if the entity + /// contains all the components. + /// + /// Does nothing if the type is a component and the entity does not have a component of the same type, + /// if the type is a bundle and the entity does not contain any of the components in the bundle, + /// or if [`AppTypeRegistry`] does not contain the reflection data for the given component. + /// + /// # Panics + /// + /// - If the entity has been despawned while this `EntityWorldMut` is still alive. + /// - If [`AppTypeRegistry`] is not present in the [`World`]. + /// + /// # Note + /// + /// Prefer to use the typed [`EntityCommands::remove`] if possible. Removing a reflected component + /// is much slower. + pub fn remove_reflect(&mut self, component_type_path: Cow<'static, str>) -> &mut Self { + self.assert_not_despawned(); + let entity_id = self.id(); + self.world_scope(|world| { + world.resource_scope(|world, registry: Mut| { + let type_registry = ®istry.as_ref().read(); + remove_reflect_with_registry_ref( + world, + entity_id, + type_registry, + component_type_path, + ); + }); + world.flush(); }); + self.update_location(); self } - fn remove_reflect_with_registry>( + /// Same as [`remove_reflect`](EntityWorldMut::remove_reflect), but using + /// the `T` resource as type registry instead of `AppTypeRegistry`. + /// + /// If the given type is a bundle, it will remove any components in that bundle regardless if the entity + /// contains all the components. + /// + /// Does nothing if the type is a component and the entity does not have a component of the same type, + /// if the type is a bundle and the entity does not contain any of the components in the bundle, + /// or if [`AppTypeRegistry`] does not contain the reflection data for the given component. + /// + /// # Panics + /// + /// - If the entity has been despawned while this `EntityWorldMut` is still alive. + /// - If [`AppTypeRegistry`] is not present in the [`World`]. + pub fn remove_reflect_with_registry>( &mut self, - component_type_name: impl Into>, + component_type_path: Cow<'static, str>, ) -> &mut Self { - self.commands.queue(RemoveReflectWithRegistry:: { - entity: self.entity, - _t: PhantomData, - component_type_name: component_type_name.into(), + self.assert_not_despawned(); + let entity_id = self.id(); + self.world_scope(|world| { + world.resource_scope(|world, registry: Mut| { + let type_registry = registry.as_ref().as_ref(); + remove_reflect_with_registry_ref( + world, + entity_id, + type_registry, + component_type_path, + ); + }); + world.flush(); }); + self.update_location(); self } } /// Helper function to add a reflect component or bundle to a given entity -fn insert_reflect( +fn insert_reflect_with_registry_ref( world: &mut World, entity: Entity, type_registry: &TypeRegistry, @@ -238,48 +372,8 @@ fn insert_reflect( } } -/// A [`Command`] that adds the boxed reflect component or bundle to an entity using the data in -/// [`AppTypeRegistry`]. -/// -/// See [`ReflectCommandExt::insert_reflect`] for details. -pub struct InsertReflect { - /// The entity on which the component will be inserted. - pub entity: Entity, - /// The reflect [`Component`](crate::component::Component) or [`Bundle`](crate::bundle::Bundle) - /// that will be added to the entity. - pub component: Box, -} - -impl Command for InsertReflect { - fn apply(self, world: &mut World) { - let registry = world.get_resource::().unwrap().clone(); - insert_reflect(world, self.entity, ®istry.read(), self.component); - } -} - -/// A [`Command`] that adds the boxed reflect component or bundle to an entity using the data in the provided -/// [`Resource`] that implements [`AsRef`]. -/// -/// See [`ReflectCommandExt::insert_reflect_with_registry`] for details. -pub struct InsertReflectWithRegistry> { - /// The entity on which the component will be inserted. - pub entity: Entity, - pub _t: PhantomData, - /// The reflect [`Component`](crate::component::Component) that will be added to the entity. - pub component: Box, -} - -impl> Command for InsertReflectWithRegistry { - fn apply(self, world: &mut World) { - world.resource_scope(|world, registry: Mut| { - let registry: &TypeRegistry = registry.as_ref().as_ref(); - insert_reflect(world, self.entity, registry, self.component); - }); - } -} - /// Helper function to remove a reflect component or bundle from a given entity -fn remove_reflect( +fn remove_reflect_with_registry_ref( world: &mut World, entity: Entity, type_registry: &TypeRegistry, @@ -298,54 +392,6 @@ fn remove_reflect( } } -/// A [`Command`] that removes the component or bundle of the same type as the given type name from -/// the provided entity. -/// -/// See [`ReflectCommandExt::remove_reflect`] for details. -pub struct RemoveReflect { - /// The entity from which the component will be removed. - pub entity: Entity, - /// The [`Component`](crate::component::Component) or [`Bundle`](crate::bundle::Bundle) - /// type name that will be used to remove a component - /// of the same type from the entity. - pub component_type_path: Cow<'static, str>, -} - -impl Command for RemoveReflect { - fn apply(self, world: &mut World) { - let registry = world.get_resource::().unwrap().clone(); - remove_reflect( - world, - self.entity, - ®istry.read(), - self.component_type_path, - ); - } -} - -/// A [`Command`] that removes the component or bundle of the same type as the given type name from -/// the provided entity using the provided [`Resource`] that implements [`AsRef`]. -/// -/// See [`ReflectCommandExt::remove_reflect_with_registry`] for details. -pub struct RemoveReflectWithRegistry> { - /// The entity from which the component will be removed. - pub entity: Entity, - pub _t: PhantomData, - /// The [`Component`](crate::component::Component) or [`Bundle`](crate::bundle::Bundle) - /// type name that will be used to remove a component - /// of the same type from the entity. - pub component_type_name: Cow<'static, str>, -} - -impl> Command for RemoveReflectWithRegistry { - fn apply(self, world: &mut World) { - world.resource_scope(|world, registry: Mut| { - let registry: &TypeRegistry = registry.as_ref().as_ref(); - remove_reflect(world, self.entity, registry, self.component_type_name); - }); - } -} - #[cfg(test)] mod tests { use crate::{ diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index d6646bca55fef..b96d2baba9a6b 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1,13 +1,10 @@ #[cfg(feature = "std")] mod parallel_scope; -use alloc::vec::Vec; +use alloc::{boxed::Box, vec::Vec}; use core::{marker::PhantomData, panic::Location}; -use super::{ - Deferred, IntoObserverSystem, IntoSystem, RegisterSystem, Resource, RunSystemCachedWith, - UnregisterSystem, UnregisterSystemCached, -}; +use super::{Deferred, IntoObserverSystem, IntoSystem, RegisteredSystem, Resource}; use crate::{ self as bevy_ecs, bundle::{Bundle, InsertMode}, @@ -15,16 +12,16 @@ use crate::{ component::{Component, ComponentId, ComponentInfo, Mutable}, entity::{Entities, Entity, EntityCloneBuilder}, event::{Event, SendEvent}, - observer::{Observer, TriggerEvent, TriggerTargets}, + observer::{Observer, TriggerTargets}, schedule::ScheduleLabel, - system::{input::SystemInput, RunSystemWith, SystemId}, + system::{input::SystemInput, SystemId}, world::{ command_queue::RawCommandQueue, unsafe_world_cell::UnsafeWorldCell, Command, CommandQueue, EntityWorldMut, FromWorld, SpawnBatchIter, World, }, }; use bevy_ptr::OwningPtr; -use log::{error, info}; +use log::{error, info, warn}; #[cfg(feature = "std")] pub use parallel_scope::*; @@ -878,7 +875,11 @@ impl<'w, 's> Commands<'w, 's> { where I: SystemInput: Send> + 'static, { - self.queue(RunSystemWith::new_with_input(id, input)); + self.queue(move |world: &mut World| { + if let Err(error) = world.run_system_with(id, input) { + warn!("{error}"); + } + }); } /// Registers a system and returns a [`SystemId`] so it can later be called by [`World::run_system`]. @@ -939,7 +940,12 @@ impl<'w, 's> Commands<'w, 's> { O: Send + 'static, { let entity = self.spawn_empty().id(); - self.queue(RegisterSystem::new(system, entity)); + let system = RegisteredSystem::new(Box::new(IntoSystem::into_system(system))); + self.queue(move |world: &mut World| { + if let Ok(mut entity) = world.get_entity_mut(entity) { + entity.insert(system); + } + }); SystemId::from_entity(entity) } @@ -951,7 +957,11 @@ impl<'w, 's> Commands<'w, 's> { I: SystemInput + Send + 'static, O: Send + 'static, { - self.queue(UnregisterSystem::new(system_id)); + self.queue(move |world: &mut World| { + if let Err(error) = world.unregister_system(system_id) { + warn!("{error}"); + } + }); } /// Removes a system previously registered with [`World::register_system_cached`]. @@ -966,7 +976,11 @@ impl<'w, 's> Commands<'w, 's> { &mut self, system: S, ) { - self.queue(UnregisterSystemCached::new(system)); + self.queue(move |world: &mut World| { + if let Err(error) = world.unregister_system_cached(system) { + warn!("{error}"); + } + }); } /// Similar to [`Self::run_system`], but caching the [`SystemId`] in a @@ -990,7 +1004,11 @@ impl<'w, 's> Commands<'w, 's> { M: 'static, S: IntoSystem + Send + 'static, { - self.queue(RunSystemCachedWith::new(system, input)); + self.queue(move |world: &mut World| { + if let Err(error) = world.run_system_cached_with(system, input) { + warn!("{error}"); + } + }); } /// Sends a "global" [`Trigger`] without any targets. This will run any [`Observer`] of the `event` that @@ -998,7 +1016,9 @@ impl<'w, 's> Commands<'w, 's> { /// /// [`Trigger`]: crate::observer::Trigger pub fn trigger(&mut self, event: impl Event) { - self.queue(TriggerEvent { event, targets: () }); + self.queue(move |world: &mut World| { + world.trigger(event); + }); } /// Sends a [`Trigger`] for the given targets. This will run any [`Observer`] of the `event` that @@ -1010,7 +1030,9 @@ impl<'w, 's> Commands<'w, 's> { event: impl Event, targets: impl TriggerTargets + Send + Sync + 'static, ) { - self.queue(TriggerEvent { event, targets }); + self.queue(move |world: &mut World| { + world.trigger_targets(event, targets); + }); } /// Spawns an [`Observer`] and returns the [`EntityCommands`] associated diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index f924618a7182c..03ac04698b4c8 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -2,14 +2,13 @@ use crate::reflect::ReflectComponent; use crate::{ self as bevy_ecs, - bundle::Bundle, change_detection::Mut, entity::Entity, system::{input::SystemInput, BoxedSystem, IntoSystem, System}, - world::{Command, World}, + world::World, }; use alloc::boxed::Box; -use bevy_ecs_macros::{Component, Resource}; +use bevy_ecs_macros::{require, Component, Resource}; #[cfg(feature = "bevy_reflect")] use bevy_reflect::Reflect; use core::marker::PhantomData; @@ -17,13 +16,23 @@ use thiserror::Error; /// A small wrapper for [`BoxedSystem`] that also keeps track whether or not the system has been initialized. #[derive(Component)] -struct RegisteredSystem { +#[require(SystemIdMarker)] +pub(crate) struct RegisteredSystem { initialized: bool, system: BoxedSystem, } +impl RegisteredSystem { + pub fn new(system: BoxedSystem) -> Self { + RegisteredSystem { + initialized: false, + system, + } + } +} + /// Marker [`Component`](bevy_ecs::component::Component) for identifying [`SystemId`] [`Entity`]s. -#[derive(Component)] +#[derive(Component, Default)] #[cfg_attr(feature = "bevy_reflect", derive(Reflect))] #[cfg_attr(feature = "bevy_reflect", reflect(Component))] pub struct SystemIdMarker; @@ -120,17 +129,6 @@ impl core::fmt::Debug for SystemId { #[derive(Resource)] pub struct CachedSystemId(pub SystemId); -/// Creates a [`Bundle`] for a one-shot system entity. -fn system_bundle(system: BoxedSystem) -> impl Bundle { - ( - RegisteredSystem { - initialized: false, - system, - }, - SystemIdMarker, - ) -} - impl World { /// Registers a system and returns a [`SystemId`] so it can later be called by [`World::run_system`]. /// @@ -163,7 +161,7 @@ impl World { I: SystemInput + 'static, O: 'static, { - let entity = self.spawn(system_bundle(system)).id(); + let entity = self.spawn(RegisteredSystem::new(system)).id(); SystemId::from_entity(entity) } @@ -401,7 +399,9 @@ impl World { self.resource_scope(|world, mut id: Mut>| { if let Ok(mut entity) = world.get_entity_mut(id.0.entity()) { if !entity.contains::>() { - entity.insert(system_bundle(Box::new(IntoSystem::into_system(system)))); + entity.insert(RegisteredSystem::new(Box::new(IntoSystem::into_system( + system, + )))); } } else { id.0 = world.register_system(system); @@ -456,199 +456,6 @@ impl World { } } -/// The [`Command`] type for [`World::run_system`] or [`World::run_system_with`]. -/// -/// This command runs systems in an exclusive and single threaded way. -/// Running slow systems can become a bottleneck. -/// -/// If the system needs an [`In<_>`](crate::system::In) input value to run, it must -/// be provided as part of the command. -/// -/// There is no way to get the output of a system when run as a command, because the -/// execution of the system happens later. To get the output of a system, use -/// [`World::run_system`] or [`World::run_system_with`] instead of running the system as a command. -#[derive(Debug, Clone)] -pub struct RunSystemWith { - system_id: SystemId, - input: I::Inner<'static>, -} - -/// The [`Command`] type for [`World::run_system`]. -/// -/// This command runs systems in an exclusive and single threaded way. -/// Running slow systems can become a bottleneck. -/// -/// If the system needs an [`In<_>`](crate::system::In) input value to run, use the -/// [`RunSystemWith`] type instead. -/// -/// There is no way to get the output of a system when run as a command, because the -/// execution of the system happens later. To get the output of a system, use -/// [`World::run_system`] or [`World::run_system_with`] instead of running the system as a command. -pub type RunSystem = RunSystemWith<()>; - -impl RunSystem { - /// Creates a new [`Command`] struct, which can be added to [`Commands`](crate::system::Commands). - pub fn new(system_id: SystemId) -> Self { - Self::new_with_input(system_id, ()) - } -} - -impl RunSystemWith { - /// Creates a new [`Command`] struct, which can be added to [`Commands`](crate::system::Commands) - /// in order to run the specified system with the provided [`In<_>`](crate::system::In) input value. - pub fn new_with_input(system_id: SystemId, input: I::Inner<'static>) -> Self { - Self { system_id, input } - } -} - -impl Command for RunSystemWith -where - I: SystemInput: Send> + 'static, -{ - #[inline] - fn apply(self, world: &mut World) { - _ = world.run_system_with(self.system_id, self.input); - } -} - -/// The [`Command`] type for registering one shot systems from [`Commands`](crate::system::Commands). -/// -/// This command needs an already boxed system to register, and an already spawned entity. -pub struct RegisterSystem { - system: BoxedSystem, - entity: Entity, -} - -impl RegisterSystem -where - I: SystemInput + 'static, - O: 'static, -{ - /// Creates a new [`Command`] struct, which can be added to [`Commands`](crate::system::Commands). - pub fn new + 'static>(system: S, entity: Entity) -> Self { - Self { - system: Box::new(IntoSystem::into_system(system)), - entity, - } - } -} - -impl Command for RegisterSystem -where - I: SystemInput + Send + 'static, - O: Send + 'static, -{ - fn apply(self, world: &mut World) { - if let Ok(mut entity) = world.get_entity_mut(self.entity) { - entity.insert(system_bundle(self.system)); - } - } -} - -/// The [`Command`] type for unregistering one-shot systems from [`Commands`](crate::system::Commands). -pub struct UnregisterSystem { - system_id: SystemId, -} - -impl UnregisterSystem -where - I: SystemInput + 'static, - O: 'static, -{ - /// Creates a new [`Command`] struct, which can be added to [`Commands`](crate::system::Commands). - pub fn new(system_id: SystemId) -> Self { - Self { system_id } - } -} - -impl Command for UnregisterSystem -where - I: SystemInput + 'static, - O: 'static, -{ - fn apply(self, world: &mut World) { - let _ = world.unregister_system(self.system_id); - } -} - -/// The [`Command`] type for unregistering one-shot systems from [`Commands`](crate::system::Commands). -pub struct UnregisterSystemCached -where - I: SystemInput + 'static, - S: IntoSystem + Send + 'static, -{ - system: S, - _phantom: PhantomData (I, O, M)>, -} - -impl UnregisterSystemCached -where - I: SystemInput + 'static, - S: IntoSystem + Send + 'static, -{ - /// Creates a new [`Command`] struct, which can be added to [`Commands`](crate::system::Commands). - pub fn new(system: S) -> Self { - Self { - system, - _phantom: PhantomData, - } - } -} - -impl Command for UnregisterSystemCached -where - I: SystemInput + 'static, - O: 'static, - M: 'static, - S: IntoSystem + Send + 'static, -{ - fn apply(self, world: &mut World) { - let _ = world.unregister_system_cached(self.system); - } -} - -/// The [`Command`] type for running a cached one-shot system from -/// [`Commands`](crate::system::Commands). -/// -/// See [`World::register_system_cached`] for more information. -pub struct RunSystemCachedWith -where - I: SystemInput, - S: IntoSystem, -{ - system: S, - input: I::Inner<'static>, - _phantom: PhantomData<(fn() -> O, fn() -> M)>, -} - -impl RunSystemCachedWith -where - I: SystemInput, - S: IntoSystem, -{ - /// Creates a new [`Command`] struct, which can be added to - /// [`Commands`](crate::system::Commands). - pub fn new(system: S, input: I::Inner<'static>) -> Self { - Self { - system, - input, - _phantom: PhantomData, - } - } -} - -impl Command for RunSystemCachedWith -where - I: SystemInput: Send> + Send + 'static, - O: Send + 'static, - S: IntoSystem + Send + 'static, - M: 'static, -{ - fn apply(self, world: &mut World) { - let _ = world.run_system_cached_with(self.system, self.input); - } -} - /// An operation with stored systems failed. #[derive(Error)] pub enum RegisteredSystemError { diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index f8b8e55f59f37..fa96a00de1081 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -986,7 +986,7 @@ impl<'w> EntityWorldMut<'w> { #[inline(always)] #[track_caller] - fn assert_not_despawned(&self) { + pub(crate) fn assert_not_despawned(&self) { if self.location.archetype_id == ArchetypeId::INVALID { self.panic_despawned(); }