From 77e9450175c1db374aa9cfdd4576cb4f25f827df Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Mon, 4 May 2026 00:08:38 -0500 Subject: [PATCH 1/4] One-shot `SystemHandle`s --- .../migration-guides/oneshot_handles.md | 58 +++ benches/benches/bevy_ecs/fragmentation.rs | 8 +- benches/benches/bevy_ecs/world/world_get.rs | 16 +- crates/bevy_app/src/app.rs | 41 +- crates/bevy_app/src/sub_app.rs | 4 +- crates/bevy_ecs/Cargo.toml | 2 + .../bevy_ecs/src/system/commands/command.rs | 4 +- crates/bevy_ecs/src/system/commands/mod.rs | 67 +-- crates/bevy_ecs/src/system/system_registry.rs | 389 +++++++++++++----- crates/bevy_remote/src/lib.rs | 67 +-- examples/ecs/callbacks.rs | 12 +- examples/ecs/one_shot_systems.rs | 14 +- examples/showcase/loading_screen.rs | 22 +- 13 files changed, 503 insertions(+), 201 deletions(-) create mode 100644 _release-content/migration-guides/oneshot_handles.md diff --git a/_release-content/migration-guides/oneshot_handles.md b/_release-content/migration-guides/oneshot_handles.md new file mode 100644 index 0000000000000..5bd3b675999b4 --- /dev/null +++ b/_release-content/migration-guides/oneshot_handles.md @@ -0,0 +1,58 @@ +--- +title: "One-shot system functions now return `SystemHandle`s" +pull_requests: [24114] +--- + +One-shot system functions like `World::register_system`, `World::register_boxed_system`, +`App::register_system`, or `SubApp::register_system` now return `SystemHandle`s +instead of `SystemId`s. Consider storing these rather than `SystemId`s in order +to support automatic cleanup of one-shot systems. + +```rust +fn my_system() {} + +// Bevy 0.18 +#[derive(Component)] +pub struct MyCallback(SystemId); + +let id = world.register_system(my_system); +let entity = world.spawn(MyCallback(id)).id(); +world.despawn(entity); // Doesn't automatically cleanup the registered system! + +// Bevy 0.19 +#[derive(Component)] +pub struct MyCallback(SystemHandle); + +let handle = world.register_system(my_system); +let entity = world.spawn(MyCallback(handle)).id(); +world.despawn(entity); +// The registered system entity will be despawned at the next call to the +// `despawn_unused_registered_systems` system, which is in the `Last` schedule +// by default. +``` + +Calling `World::run_system` or `World::run_system_with` with handles may attempt +to take ownership of the handle, which wasn't a problem previously because +`SystemId`s are `Copy`. To avoid this issue, pass the handle by reference: + +```rust +// Bevy 0.18 +let id = world.register_system(my_system); +world.run_system(id); +world.run_system(id); +world.run_system(id); + +// Bevy 0.19 +let handle = world.register_system(my_system); +world.run_system(&handle); +world.run_system(&handle); +world.run_system(&handle); +``` + +`bevy_remote` was migrated from `SystemId`s to `SystemHandle`s in the following ways: + +- `bevy_remote::RemoteMethods` functions now accept and return `RemoteMethodSystemHandle`, + which holds `SystemHandle`s rather than `SystemId`s. If necessary, you may convert + `SystemId`s into weak `SystemHandle`s. Most likely, the changes to the one-shot + system registration functions mean you've probably been switched to `SystemHandle`s + automatically. diff --git a/benches/benches/bevy_ecs/fragmentation.rs b/benches/benches/bevy_ecs/fragmentation.rs index c3e895baa25f3..e4b09b301ad46 100644 --- a/benches/benches/bevy_ecs/fragmentation.rs +++ b/benches/benches/bevy_ecs/fragmentation.rs @@ -29,9 +29,9 @@ fn iter_frag_empty(c: &mut Criterion) { black_box(t); }); }; - let query_id = world.register_system(query); + let query_handle = world.register_system(query); b.iter(|| { - world.run_system(query_id).unwrap(); + world.run_system(&query_handle).unwrap(); }); }); group.bench_function("foreach_sparse", |b| { @@ -44,9 +44,9 @@ fn iter_frag_empty(c: &mut Criterion) { black_box(t); }); }; - let query_id = world.register_system(query); + let query_handle = world.register_system(query); b.iter(|| { - world.run_system(query_id).unwrap(); + world.run_system(&query_handle).unwrap(); }); }); group.finish(); diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index a1ffec7aba0ac..716b77e3beec2 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -278,9 +278,9 @@ pub fn query_get(criterion: &mut Criterion) { } assert_eq!(black_box(count), entity_count); }; - let query_id = world.register_system(query); + let query_handle = world.register_system(query); bencher.iter(|| { - world.run_system(query_id).unwrap(); + world.run_system(&query_handle).unwrap(); }); }); group.bench_function(format!("{entity_count}_entities_sparse"), |bencher| { @@ -299,9 +299,9 @@ pub fn query_get(criterion: &mut Criterion) { } assert_eq!(black_box(count), entity_count); }; - let query_id = world.register_system(query); + let query_handle = world.register_system(query); bencher.iter(|| { - world.run_system(query_id).unwrap(); + world.run_system(&query_handle).unwrap(); }); }); } @@ -334,9 +334,9 @@ pub fn query_get_many(criterion: &mut Criterion) { } assert_eq!(black_box(count), entity_count); }; - let query_id = world.register_system(query); + let query_handle = world.register_system(query); bencher.iter(|| { - world.run_system(query_id).unwrap(); + world.run_system(&query_handle).unwrap(); }); }); group.bench_function(format!("{entity_count}_calls_sparse"), |bencher| { @@ -358,9 +358,9 @@ pub fn query_get_many(criterion: &mut Criterion) { } assert_eq!(black_box(count), entity_count); }; - let query_id = world.register_system(query); + let query_handle = world.register_system(query); bencher.iter(|| { - world.run_system(query_id).unwrap(); + world.run_system(&query_handle).unwrap(); }); }); } diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index b78fbedd1a241..7096a4959473d 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -19,7 +19,7 @@ use bevy_ecs::{ InternedSystemSet, ScheduleBuildSettings, ScheduleCleanupPolicy, ScheduleError, ScheduleLabel, }, - system::{ScheduleSystem, SystemId, SystemInput}, + system::{ScheduleSystem, SystemHandle, SystemInput}, }; use bevy_platform::collections::HashMap; #[cfg(feature = "bevy_reflect")] @@ -130,6 +130,11 @@ impl Default for App { .in_set(bevy_ecs::message::MessageUpdateSystems) .run_if(bevy_ecs::message::message_update_condition), ); + #[cfg(feature = "std")] + app.add_systems( + crate::Last, + bevy_ecs::system::despawn_unused_registered_systems, + ); app.add_message::(); app @@ -362,19 +367,22 @@ impl App { self.main_mut().remove_systems_in_set(schedule, set, policy) } - /// Registers a system and returns a [`SystemId`] so it can later be called by [`World::run_system`]. + /// Registers a system and returns a strong [`SystemHandle`] so it can later + /// be called by [`World::run_system`]. /// - /// It's possible to register the same systems more than once, they'll be stored separately. + /// It's possible to register the same systems more than once, they'll be + /// stored separately. /// /// This is different from adding systems to a [`Schedule`] with [`App::add_systems`], - /// because the [`SystemId`] that is returned can be used anywhere in the [`World`] to run the associated system. - /// This allows for running systems in a push-based fashion. - /// Using a [`Schedule`] is still preferred for most cases - /// due to its better performance and ability to run non-conflicting systems simultaneously. + /// because the [`SystemHandle`] that is returned can be used anywhere in + /// the [`World`] to run the associated system. This allows for running + /// systems in a push-based fashion. Using a [`Schedule`] is still preferred + /// for most cases due to its better performance and ability to run + /// non-conflicting systems simultaneously. pub fn register_system( &mut self, system: impl IntoSystem + 'static, - ) -> SystemId + ) -> SystemHandle where I: SystemInput + 'static, O: 'static, @@ -2086,4 +2094,21 @@ mod tests { assert_eq!(test_events.len(), 2); // Events are double-buffered, so we see 2 + 0 = 2 assert_eq!(test_events.iter_current_update_messages().count(), 0); } + + #[test] + fn auto_despawn_unused_registered_systems() { + let mut app = App::new(); + + fn my_system() {} + + let handle = app.register_system(my_system); + let entity = handle.entity(); + + app.update(); + assert!(app.world().get_entity(entity).is_ok()); + + drop(handle); + app.update(); + assert!(app.world().get_entity(entity).is_err()); + } } diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index 1b20aab2ff5d3..7d8427e37bd67 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -8,7 +8,7 @@ use bevy_ecs::{ InternedScheduleLabel, InternedSystemSet, ScheduleBuildSettings, ScheduleCleanupPolicy, ScheduleError, ScheduleLabel, }, - system::{ScheduleSystem, SystemId, SystemInput}, + system::{ScheduleSystem, SystemHandle, SystemInput}, }; use bevy_platform::collections::{HashMap, HashSet}; use core::fmt::Debug; @@ -239,7 +239,7 @@ impl SubApp { pub fn register_system( &mut self, system: impl IntoSystem + 'static, - ) -> SystemId + ) -> SystemHandle where I: SystemInput + 'static, O: 'static, diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index f3f409289710b..5525a481bad02 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -69,6 +69,7 @@ std = [ "bevy_utils/std", "bitflags/std", "concurrent-queue/std", + "crossbeam-channel/std", "fixedbitset/std", "indexmap/std", "serde?/std", @@ -101,6 +102,7 @@ bevy_platform = { path = "../bevy_platform", version = "0.19.0-dev", default-fea ] } bitflags = { version = "2.3", default-features = false } +crossbeam-channel = { version = "0.5", default-features = false } fixedbitset = { version = "0.5", default-features = false } serde = { version = "1", default-features = false, features = [ "alloc", diff --git a/crates/bevy_ecs/src/system/commands/command.rs b/crates/bevy_ecs/src/system/commands/command.rs index 2f384bdf25a32..fb5902218a74e 100644 --- a/crates/bevy_ecs/src/system/commands/command.rs +++ b/crates/bevy_ecs/src/system/commands/command.rs @@ -202,7 +202,7 @@ where } /// A [`Command`] that runs the given system, -/// caching its [`SystemId`] in a [`CachedSystemId`](crate::system::CachedSystemId) resource. +/// caching its [`SystemHandle`] in a [`CachedSystemHandle`](crate::system::CachedSystemHandle) resource. pub fn run_system_cached(system: S) -> impl Command where M: 'static, @@ -215,7 +215,7 @@ where } /// A [`Command`] that runs the given system with the given input value, -/// caching its [`SystemId`] in a [`CachedSystemId`](crate::system::CachedSystemId) resource. +/// caching its [`SystemHandle`] in a [`CachedSystemHandle`](crate::system::CachedSystemHandle) resource. /// /// To use the supplied input, the system should have a [`SystemInput`] as the first parameter. pub fn run_system_cached_with(system: S, input: I::Inner<'static>) -> impl Command diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index f99b5948c96c6..31bb3de2de07e 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -31,7 +31,8 @@ use crate::{ resource::Resource, schedule::ScheduleLabel, system::{ - Deferred, IntoSystem, RegisteredSystem, SystemId, SystemInput, SystemParamValidationError, + Deferred, IntoSystem, RegisteredSystem, SystemHandle, SystemId, SystemInput, + SystemParamValidationError, }, world::{ command_queue::RawCommandQueue, unsafe_world_cell::UnsafeWorldCell, CommandQueue, @@ -935,8 +936,8 @@ impl<'w, 's> Commands<'w, 's> { /// /// It will internally return a [`RegisteredSystemError`](crate::system::system_registry::RegisteredSystemError), /// which will be handled by [logging the error at the `warn` level](warn). - pub fn run_system(&mut self, id: SystemId) { - self.queue(command::run_system(id).handle_error_with(warn)); + pub fn run_system(&mut self, id: impl Into) { + self.queue(command::run_system(id.into()).handle_error_with(warn)); } /// Runs the system corresponding to the given [`SystemId`] with input. @@ -957,44 +958,49 @@ impl<'w, 's> Commands<'w, 's> { /// /// It will internally return a [`RegisteredSystemError`](crate::system::system_registry::RegisteredSystemError), /// which will be handled by [logging the error at the `warn` level](warn). - pub fn run_system_with(&mut self, id: SystemId, input: I::Inner<'static>) + pub fn run_system_with(&mut self, id: impl Into>, input: I::Inner<'static>) where I: SystemInput: Send> + 'static, { - self.queue(command::run_system_with(id, input).handle_error_with(warn)); + self.queue(command::run_system_with(id.into(), input).handle_error_with(warn)); } - /// Registers a system and returns its [`SystemId`] so it can later be called by + /// Registers a system and returns its [`SystemHandle`] so it can later be called by /// [`Commands::run_system`] or [`World::run_system`]. /// /// This is different from adding systems to a [`Schedule`](crate::schedule::Schedule), - /// because the [`SystemId`] that is returned can be used anywhere in the [`World`] to run the associated system. + /// because the [`SystemHandle`] that is returned can be used anywhere in + /// the [`World`] to run the associated system. /// /// Using a [`Schedule`](crate::schedule::Schedule) is still preferred for most cases /// due to its better performance and ability to run non-conflicting systems simultaneously. /// /// # Note /// + /// This function returns a "weak" handle to the system, so if the handle is + /// dropped the system will **not** be automatically unregistered. You must + /// manually unregister the system with [`Commands::unregister_system`]. + /// /// If the same system is registered more than once, /// each registration will be considered a different system, - /// and they will each be given their own [`SystemId`]. + /// and they will each be given their own [`SystemHandle`]. /// /// If you want to avoid registering the same system multiple times, - /// consider using [`Commands::run_system_cached`] or storing the [`SystemId`] + /// consider using [`Commands::run_system_cached`] or storing the [`SystemHandle`] /// in a [`Local`](crate::system::Local). /// /// # Example /// /// ``` - /// # use bevy_ecs::{prelude::*, world::CommandQueue, system::SystemId}; + /// # use bevy_ecs::{prelude::*, world::CommandQueue, system::SystemHandle}; /// #[derive(Resource)] /// struct Counter(i32); /// /// fn register_system( /// mut commands: Commands, - /// mut local_system: Local>, + /// mut local_system: Local>, /// ) { - /// if let Some(system) = *local_system { + /// if let Some(system) = &*local_system { /// commands.run_system(system); /// } else { /// *local_system = Some(commands.register_system(increment_counter)); @@ -1008,14 +1014,14 @@ impl<'w, 's> Commands<'w, 's> { /// # let mut world = World::default(); /// # world.insert_resource(Counter(0)); /// # let mut queue_1 = CommandQueue::default(); - /// # let systemid = { + /// # let system_handle = { /// # let mut commands = Commands::new(&mut queue_1, &world); /// # commands.register_system(increment_counter) /// # }; /// # let mut queue_2 = CommandQueue::default(); /// # { /// # let mut commands = Commands::new(&mut queue_2, &world); - /// # commands.run_system(systemid); + /// # commands.run_system(system_handle); /// # } /// # queue_1.append(&mut queue_2); /// # queue_1.apply(&mut world); @@ -1025,15 +1031,17 @@ impl<'w, 's> Commands<'w, 's> { pub fn register_system( &mut self, system: impl IntoSystem + 'static, - ) -> SystemId + ) -> SystemHandle where I: SystemInput + Send + 'static, O: Send + 'static, { - let entity = self.spawn_empty().id(); - let system = RegisteredSystem::::new(Box::new(IntoSystem::into_system(system))); - self.entity(entity).insert(system); - SystemId::from_entity(entity) + let entity = self + .spawn(RegisteredSystem::::new(Box::new( + IntoSystem::into_system(system), + ))) + .id(); + SystemHandle::Weak(SystemId::from_entity(entity)) } /// Removes a system previously registered with [`Commands::register_system`] @@ -1066,7 +1074,8 @@ impl<'w, 's> Commands<'w, 's> { /// # Fallible /// /// This command will fail if the given system - /// is not currently cached in a [`CachedSystemId`](crate::system::CachedSystemId) resource. + /// is not currently cached in a + /// [`CachedSystemHandle`](crate::system::CachedSystemHandle) resource. /// /// It will internally return a [`RegisteredSystemError`](crate::system::system_registry::RegisteredSystemError), /// which will be handled by [logging the error at the `warn` level](warn). @@ -1085,10 +1094,10 @@ impl<'w, 's> Commands<'w, 's> { /// Unlike [`Commands::run_system`], this method does not require manual registration. /// /// The first time this method is called for a particular system, - /// it will register the system and store its [`SystemId`] in a - /// [`CachedSystemId`](crate::system::CachedSystemId) resource for later. + /// it will register the system and store its [`SystemHandle`] in a + /// [`CachedSystemHandle`](crate::system::CachedSystemHandle) resource for later. /// - /// If you would rather manage the [`SystemId`] yourself, + /// If you would rather manage the [`SystemHandle`] yourself, /// or register multiple copies of the same system, /// use [`Commands::register_system`] instead. /// @@ -1117,10 +1126,10 @@ impl<'w, 's> Commands<'w, 's> { /// To use the supplied input, the system should have a [`SystemInput`] as the first parameter. /// /// The first time this method is called for a particular system, - /// it will register the system and store its [`SystemId`] in a - /// [`CachedSystemId`](crate::system::CachedSystemId) resource for later. + /// it will register the system and store its [`SystemHandle`] in a + /// [`CachedSystemHandle`](crate::system::CachedSystemHandle) resource for later. /// - /// If you would rather manage the [`SystemId`] yourself, + /// If you would rather manage the [`SystemHandle`] yourself, /// or register multiple copies of the same system, /// use [`Commands::register_system`] instead. /// @@ -2956,15 +2965,15 @@ mod tests { fn nothing() {} let resources = world.iter_resources().count(); - let id = world.register_system_cached(nothing); + let handle = world.register_system_cached(nothing); assert_eq!(world.iter_resources().count(), resources + 1); - assert!(world.get_entity(id.entity).is_ok()); + assert!(world.get_entity(handle.entity()).is_ok()); let mut commands = Commands::new(&mut queue, &world); commands.unregister_system_cached(nothing); queue.apply(&mut world); assert_eq!(world.iter_resources().count(), resources); - assert!(world.get_entity(id.entity).is_err()); + assert!(world.get_entity(handle.entity()).is_err()); } fn is_send() {} diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 819f78c8c9c44..ab81cdbd1de5a 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -11,6 +11,7 @@ use crate::{ }; use alloc::boxed::Box; use bevy_ecs_macros::{Component, Resource}; +use bevy_platform::sync::Arc; use bevy_utils::prelude::DebugName; use core::{any::TypeId, marker::PhantomData}; use thiserror::Error; @@ -32,6 +33,39 @@ impl RegisteredSystem { } } +/// A system that despawns any [`RegisteredSystem`] entities whose [`SystemHandle`] +/// reference count has reached zero. +#[cfg(feature = "std")] +pub fn despawn_unused_registered_systems( + despawner: Option>, + mut commands: crate::system::Commands, +) { + // `RegisteredSystemDespawner` is initialized lazily the first time a system + // is registered, so it's possible that it doesn't exist yet when this system runs. + if let Some(despawner) = despawner { + for entity in despawner.receiver.try_iter() { + commands.entity(entity).despawn(); + } + } +} + +/// A resource that stores the channel for despawning unused [`RegisteredSystem`] +/// entities. +#[derive(Resource)] +#[cfg(feature = "std")] +pub struct RegisteredSystemDespawner { + receiver: crossbeam_channel::Receiver, + sender: crossbeam_channel::Sender, +} + +#[cfg(feature = "std")] +impl Default for RegisteredSystemDespawner { + fn default() -> Self { + let (sender, receiver) = crossbeam_channel::unbounded(); + Self { receiver, sender } + } +} + #[derive(Debug, Clone)] struct TypeIdAndName { type_id: TypeId, @@ -94,6 +128,107 @@ impl RemovedSystem { } } +/// A maybe-strong handle to an entity with a [`RegisteredSystem`] component. +/// Strong handles provide automatic cleanup of registered systems once all clones +/// of the handle are dropped, while weak handles do not. +/// +/// Strong handles are only returned by functions on [`World`], like +/// [`World::register_system`] and [`World::register_system_cached`]. +/// [`Commands::register_system`] can only return weak handles due to the lack of +/// access to the world during command submission. +/// +/// # Cleanup +/// +/// [`RegisteredSystem`] entities are cleaned up by the [`despawn_unused_registered_systems`] +/// system, which is automatically added to the default app by the `bevy_ecs` +/// crate when the "std" feature is enabled. If not using the default app, the +/// "std" feature, or `bevy_app` in general, consider running this system +/// yourself to ensure proper cleanup of registered systems. +/// +/// [`Commands::register_system`]: crate::system::Commands::register_system +pub enum SystemHandle { + /// A strong handle keeps the system entity alive as long as the handle + /// (and any clones of it) exist. + Strong(Arc), + /// A weak handle does not keep the system entity alive, but is [`Copy`]able. + Weak(SystemId), +} + +impl SystemHandle { + /// Returns the [`Entity`] of the registered system associated with this handle. + pub fn entity(&self) -> Entity { + match self { + SystemHandle::Strong(strong) => strong.entity, + SystemHandle::Weak(weak) => weak.entity, + } + } +} + +impl Eq for SystemHandle {} + +// A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. +impl Clone for SystemHandle { + fn clone(&self) -> Self { + match self { + SystemHandle::Strong(strong) => SystemHandle::Strong(Arc::clone(strong)), + SystemHandle::Weak(weak) => SystemHandle::Weak(*weak), + } + } +} + +// A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. +impl PartialEq for SystemHandle { + fn eq(&self, other: &Self) -> bool { + self.entity() == other.entity() + } +} + +impl PartialEq> for SystemHandle { + fn eq(&self, other: &SystemId) -> bool { + self.entity() == other.entity + } +} + +// A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. +impl core::hash::Hash for SystemHandle { + fn hash(&self, state: &mut H) { + self.entity().hash(state); + } +} + +impl core::fmt::Debug for SystemHandle { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + let name = if matches!(self, SystemHandle::Strong(_)) { + "StrongSystemHandle" + } else { + "WeakSystemHandle" + }; + f.debug_tuple(name).field(&self.entity()).finish() + } +} + +impl From> for SystemHandle { + fn from(id: SystemId) -> Self { + SystemHandle::Weak(id) + } +} + +/// A strong handle for a registered system that keeps the system entity alive +/// as long as the handle (and any clones of it) exist. +pub struct StrongSystemHandle { + entity: Entity, + #[cfg(feature = "std")] + drop_sender: crossbeam_channel::Sender, +} + +#[cfg(feature = "std")] +impl Drop for StrongSystemHandle { + fn drop(&mut self) { + // Send the entity to be despawned by the world when the last strong handle is dropped. + let _ = self.drop_sender.send(self.entity); + } +} + /// An identifier for a registered system. /// /// These are opaque identifiers, keyed to a specific [`World`], @@ -141,7 +276,13 @@ impl Clone for SystemId { // A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. impl PartialEq for SystemId { fn eq(&self, other: &Self) -> bool { - self.entity == other.entity && self.marker == other.marker + self.entity == other.entity + } +} + +impl PartialEq> for SystemId { + fn eq(&self, other: &SystemHandle) -> bool { + self.entity == other.entity() } } @@ -158,42 +299,62 @@ impl core::fmt::Debug for SystemId { } } +impl From<&SystemHandle> for SystemId { + fn from(handle: &SystemHandle) -> Self { + match handle { + SystemHandle::Strong(strong) => Self::from_entity(strong.entity), + SystemHandle::Weak(weak) => *weak, + } + } +} + +impl From> for SystemId { + fn from(handle: SystemHandle) -> Self { + (&handle).into() + } +} + /// A cached [`SystemId`] distinguished by the unique function type of its system. /// /// This resource is inserted by [`World::register_system_cached`]. #[derive(Resource)] -pub struct CachedSystemId { - /// The cached `SystemId` as an `Entity`. - pub entity: Entity, +pub struct CachedSystemHandle { + strong: Arc, _marker: PhantomData S>, } -impl CachedSystemId { - /// Creates a new `CachedSystemId` struct given a `SystemId`. - pub fn new(id: SystemId) -> Self { - Self { - entity: id.entity(), - _marker: PhantomData, - } +/// Deprecated alias for [`CachedSystemHandle`]. +#[deprecated( + since = "0.18.0", + note = "This type was renamed to `CachedSystemHandle` for consistency." +)] +pub type CachedSystemId = CachedSystemHandle; + +impl CachedSystemHandle { + /// Returns the [`Entity`] of the cached system associated with this handle. + pub fn entity(&self) -> Entity { + self.strong.entity } } impl World { - /// Registers a system and returns a [`SystemId`] so it can later be called by [`World::run_system`]. + /// Registers a system and returns a strong [`SystemHandle`] so it can later + /// be called by [`World::run_system`]. /// - /// It's possible to register multiple copies of the same system by calling this function - /// multiple times. If that's not what you want, consider using [`World::register_system_cached`] - /// instead. + /// It's possible to register multiple copies of the same system by calling + /// this function multiple times. If that's not what you want, consider + /// using [`World::register_system_cached`] instead. /// /// This is different from adding systems to a [`Schedule`](crate::schedule::Schedule), - /// because the [`SystemId`] that is returned can be used anywhere in the [`World`] to run the associated system. - /// This allows for running systems in a pushed-based fashion. - /// Using a [`Schedule`](crate::schedule::Schedule) is still preferred for most cases - /// due to its better performance and ability to run non-conflicting systems simultaneously. + /// because the [`SystemHandle`] that is returned can be used anywhere in the + /// [`World`] to run the associated system. This allows for running systems + /// in a pushed-based fashion. Using a [`Schedule`](crate::schedule::Schedule) + /// is still preferred for most cases due to its better performance and + /// ability to run non-conflicting systems simultaneously. pub fn register_system( &mut self, system: impl IntoSystem + 'static, - ) -> SystemId + ) -> SystemHandle where I: SystemInput + 'static, O: 'static, @@ -205,29 +366,38 @@ impl World { /// /// This is useful if the [`IntoSystem`] implementor has already been turned into a /// [`System`](crate::system::System) trait object and put in a [`Box`]. - pub fn register_boxed_system(&mut self, system: BoxedSystem) -> SystemId + pub fn register_boxed_system(&mut self, system: BoxedSystem) -> SystemHandle where I: SystemInput + 'static, O: 'static, { let entity = self.spawn(RegisteredSystem::new(system)).id(); - SystemId::from_entity(entity) + #[cfg(feature = "std")] + let despawner = self.get_resource_or_init::(); + + SystemHandle::Strong(Arc::new(StrongSystemHandle { + entity, + #[cfg(feature = "std")] + drop_sender: despawner.sender.clone(), + })) } - /// Removes a registered system and returns the system, if it exists. - /// After removing a system, the [`SystemId`] becomes invalid and attempting to use it afterwards will result in errors. - /// Re-adding the removed system will register it on a new [`SystemId`]. + /// Removes a registered system and returns the system, if it exists. After + /// removing a system, the [`SystemId`] becomes invalid and attempting to use + /// it afterwards will result in errors. Re-adding the removed system will + /// register it on a new [`SystemId`]. /// /// If no system corresponds to the given [`SystemId`], this method returns an error. /// Systems are also not allowed to remove themselves, this returns an error too. pub fn unregister_system( &mut self, - id: SystemId, + id: impl Into>, ) -> Result, RegisteredSystemError> where I: SystemInput + 'static, O: 'static, { + let id = id.into(); match self.get_entity_mut(id.entity) { Ok(mut entity) => { let registered_system = entity @@ -245,10 +415,11 @@ impl World { } } - /// Run stored systems by their [`SystemId`]. - /// Before running a system, it must first be registered. - /// The method [`World::register_system`] stores a given system and returns a [`SystemId`]. - /// This is different from [`RunSystemOnce::run_system_once`](crate::system::RunSystemOnce::run_system_once), + /// Run stored systems by their [`SystemId`]. Before running a system, it + /// must first be registered. The method [`World::register_system`] stores a + /// given system and returns a [`SystemHandle`], which is convertible into a + /// [`SystemId`]. This is different from + /// [`RunSystemOnce::run_system_once`](crate::system::RunSystemOnce::run_system_once), /// because it keeps local state between calls and change detection works correctly. /// /// Also runs any queued-up commands. @@ -269,9 +440,9 @@ impl World { /// let mut world = World::default(); /// let counter_one = world.register_system(increment); /// let counter_two = world.register_system(increment); - /// world.run_system(counter_one); // -> 1 - /// world.run_system(counter_one); // -> 2 - /// world.run_system(counter_two); // -> 1 + /// world.run_system(&counter_one); // -> 1 + /// world.run_system(&counter_one); // -> 2 + /// world.run_system(&counter_two); // -> 1 /// ``` /// /// ## Change detection @@ -292,10 +463,10 @@ impl World { /// }); /// /// // Resources are changed when they are first added - /// let _ = world.run_system(detector); // -> Something happened! - /// let _ = world.run_system(detector); // -> Nothing happened. + /// let _ = world.run_system(&detector); // -> Something happened! + /// let _ = world.run_system(&detector); // -> Nothing happened. /// world.resource_mut::().set_changed(); - /// let _ = world.run_system(detector); // -> Something happened! + /// let _ = world.run_system(&detector); // -> Something happened! /// ``` /// /// ## Getting system output @@ -327,19 +498,20 @@ impl World { /// ]; /// /// for (label, scoring_system) in scoring_systems { - /// println!("{label} has score {}", world.run_system(scoring_system).expect("system succeeded")); + /// println!("{label} has score {}", world.run_system(&scoring_system).expect("system succeeded")); /// } /// ``` pub fn run_system( &mut self, - id: SystemId<(), O>, + id: impl Into>, ) -> Result> { self.run_system_with(id, ()) } /// Run a stored chained system by its [`SystemId`], providing an input value. - /// Before running a system, it must first be registered. - /// The method [`World::register_system`] stores a given system and returns a [`SystemId`]. + /// Before running a system, it must first be registered. The method + /// [`World::register_system`] stores a given system and returns a [`SystemHandle`], + /// which is convertible into a [`SystemId`]. /// /// To use the supplied input, the system should have a [`SystemInput`] as the first parameter. /// Also runs any queued-up commands. @@ -356,21 +528,22 @@ impl World { /// let mut world = World::default(); /// let counter_one = world.register_system(increment); /// let counter_two = world.register_system(increment); - /// assert_eq!(world.run_system_with(counter_one, 1).unwrap(), 1); - /// assert_eq!(world.run_system_with(counter_one, 20).unwrap(), 21); - /// assert_eq!(world.run_system_with(counter_two, 30).unwrap(), 30); + /// assert_eq!(world.run_system_with(&counter_one, 1).unwrap(), 1); + /// assert_eq!(world.run_system_with(&counter_one, 20).unwrap(), 21); + /// assert_eq!(world.run_system_with(&counter_two, 30).unwrap(), 30); /// ``` /// /// See [`World::run_system`] for more examples. pub fn run_system_with( &mut self, - id: SystemId, + id: impl Into>, input: I::Inner<'_>, ) -> Result> where I: SystemInput + 'static, O: 'static, { + let id = id.into(); // Lookup let mut entity = self .get_entity_mut(id.entity) @@ -429,14 +602,14 @@ impl World { Ok(result?) } - /// Registers a system or returns its cached [`SystemId`]. + /// Registers a system or returns its cached [`SystemHandle`]. /// - /// If you want to run the system immediately and you don't need its `SystemId`, see + /// If you want to run the system immediately and you don't need its `SystemHandle`, see /// [`World::run_system_cached`]. /// /// The first time this function is called for a particular system, it will register it and - /// store its [`SystemId`] in a [`CachedSystemId`] resource for later. If you would rather - /// manage the `SystemId` yourself, or register multiple copies of the same system, use + /// store its [`SystemHandle`] in a [`CachedSystemHandle`] resource for later. If you would rather + /// manage the `SystemHandle` yourself, or register multiple copies of the same system, use /// [`World::register_system`] instead. /// /// # Limitations @@ -448,7 +621,7 @@ impl World { /// If you want to access values from the environment within a system, consider passing them in /// as inputs via [`World::run_system_cached_with`]. If that's not an option, consider /// [`World::register_system`] instead. - pub fn register_system_cached(&mut self, system: S) -> SystemId + pub fn register_system_cached(&mut self, system: S) -> SystemHandle where I: SystemInput + 'static, O: 'static, @@ -461,27 +634,36 @@ impl World { ); } - if !self.contains_resource::>() { - let id = self.register_system(system); - self.insert_resource(CachedSystemId::::new(id)); - return id; + if !self.contains_resource::>() { + let handle = self.register_system(system); + self.insert_resource(CachedSystemHandle:: { + strong: match &handle { + SystemHandle::Strong(strong) => strong.clone(), + SystemHandle::Weak(_) => unreachable!(), + }, + _marker: PhantomData, + }); + return handle; } - self.resource_scope(|world, mut id: Mut>| { - if let Ok(mut entity) = world.get_entity_mut(id.entity) { + self.resource_scope(|world, mut handle: Mut>| { + if let Ok(mut entity) = world.get_entity_mut(handle.strong.entity) { if !entity.contains::>() { entity.insert(RegisteredSystem::new(Box::new(IntoSystem::into_system( system, )))); } } else { - id.entity = world.register_system(system).entity(); + handle.strong = match world.register_system(system) { + SystemHandle::Strong(strong) => strong, + SystemHandle::Weak(_) => unreachable!(), + }; } - SystemId::from_entity(id.entity) + SystemHandle::Strong(Arc::clone(&handle.strong)) }) } - /// Removes a cached system and its [`CachedSystemId`] resource. + /// Removes a cached system and its [`CachedSystemHandle`] resource. /// /// See [`World::register_system_cached`] for more information. pub fn unregister_system_cached( @@ -494,9 +676,9 @@ impl World { S: IntoSystem + 'static, { let id = self - .remove_resource::>() + .remove_resource::>() .ok_or(RegisteredSystemError::SystemNotCached)?; - self.unregister_system(SystemId::::from_entity(id.entity)) + self.unregister_system(SystemHandle::::Strong(id.strong)) } /// Runs a cached system, registering it if necessary. @@ -604,7 +786,9 @@ mod tests { use crate::{ prelude::*, - system::{RegisteredSystemError, SystemId}, + system::{ + despawn_unused_registered_systems, RegisteredSystemError, SystemHandle, SystemId, + }, }; #[derive(Resource, Default, PartialEq, Debug)] @@ -629,15 +813,15 @@ mod tests { world.init_resource::(); assert_eq!(*world.resource::(), Counter(0)); // Resources are changed when they are first added. - let id = world.register_system(count_up_iff_changed); - world.run_system(id).expect("system runs successfully"); + let handle = world.register_system(count_up_iff_changed); + world.run_system(&handle).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(1)); // Nothing changed - world.run_system(id).expect("system runs successfully"); + world.run_system(&handle).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(1)); // Making a change world.resource_mut::().set_changed(); - world.run_system(id).expect("system runs successfully"); + world.run_system(&handle).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(2)); } @@ -652,14 +836,14 @@ mod tests { let mut world = World::new(); world.insert_resource(Counter(1)); assert_eq!(*world.resource::(), Counter(1)); - let id = world.register_system(doubling); - world.run_system(id).expect("system runs successfully"); + let handle = world.register_system(doubling); + world.run_system(&handle).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(1)); - world.run_system(id).expect("system runs successfully"); + world.run_system(&handle).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(2)); - world.run_system(id).expect("system runs successfully"); + world.run_system(&handle).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(4)); - world.run_system(id).expect("system runs successfully"); + world.run_system(&handle).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(8)); } @@ -674,29 +858,29 @@ mod tests { let mut world = World::new(); - let id = world.register_system(increment_sys); + let handle = world.register_system(increment_sys); // Insert the resource after registering the system. world.insert_resource(Counter(1)); assert_eq!(*world.resource::(), Counter(1)); world - .run_system_with(id, NonCopy(1)) + .run_system_with(&handle, NonCopy(1)) .expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(2)); world - .run_system_with(id, NonCopy(1)) + .run_system_with(&handle, NonCopy(1)) .expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(3)); world - .run_system_with(id, NonCopy(20)) + .run_system_with(&handle, NonCopy(20)) .expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(23)); world - .run_system_with(id, NonCopy(1)) + .run_system_with(&handle, NonCopy(1)) .expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(24)); } @@ -714,17 +898,17 @@ mod tests { let mut world = World::new(); - let id = world.register_system(increment_sys); + let handle = world.register_system(increment_sys); // Insert the resource after registering the system. world.insert_resource(Counter(1)); assert_eq!(*world.resource::(), Counter(1)); - let output = world.run_system(id).expect("system runs successfully"); + let output = world.run_system(&handle).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(2)); assert_eq!(output, NonCopy(2)); - let output = world.run_system(id).expect("system runs successfully"); + let output = world.run_system(&handle).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(3)); assert_eq!(output, NonCopy(3)); } @@ -737,8 +921,8 @@ mod tests { } let mut world = World::new(); - let fallible_system_id = world.register_system(sys); - let output = world.run_system(fallible_system_id); + let fallible_system_handle = world.register_system(sys); + let output = world.run_system(&fallible_system_handle); assert!(matches!(output, Ok(Err(_)))); } @@ -755,14 +939,12 @@ mod tests { #[test] fn nested_systems() { - use crate::system::SystemId; - #[derive(Component)] - struct Callback(SystemId); + struct Callback(SystemHandle); fn nested(query: Query<&Callback>, mut commands: Commands) { for callback in query.iter() { - commands.run_system(callback.0); + commands.run_system(&callback.0); } } @@ -785,14 +967,12 @@ mod tests { #[test] fn nested_systems_with_inputs() { - use crate::system::SystemId; - #[derive(Component)] - struct Callback(SystemId>, u8); + struct Callback(SystemHandle>, u8); fn nested(query: Query<&Callback>, mut commands: Commands) { for callback in query.iter() { - commands.run_system_with(callback.0, callback.1); + commands.run_system_with(&callback.0, callback.1); } } @@ -805,7 +985,7 @@ mod tests { }); let nested_id = world.register_system(nested); - world.spawn(Callback(increment_by, 2)); + world.spawn(Callback(increment_by.clone(), 2)); world.spawn(Callback(increment_by, 3)); let _ = world.run_system(nested_id); assert_eq!(*world.resource::(), Counter(5)); @@ -829,7 +1009,7 @@ mod tests { let new = world.register_system_cached(four); assert_ne!(old, new); - let output = world.run_system(old); + let output = world.run_system(&old); assert!(matches!( output, Err(RegisteredSystemError::SystemIdNotRegistered(x)) if x == old, @@ -991,11 +1171,11 @@ mod tests { let post_system = world.register_system(post); let mut event = MyEvent { cancelled: false }; - world.run_system_with(post_system, &mut event).unwrap(); + world.run_system_with(&post_system, &mut event).unwrap(); assert!(!event.cancelled); world.resource_mut::().0 = 1; - world.run_system_with(post_system, &mut event).unwrap(); + world.run_system_with(&post_system, &mut event).unwrap(); assert!(event.cancelled); } @@ -1042,9 +1222,9 @@ mod tests { } let mut world = World::new(); - let id = world.register_system(system); - SYSTEM_ID.set(Some(id)); - world.run_system(id).unwrap(); + let handle = world.register_system(system); + SYSTEM_ID.set(Some((&handle).into())); + world.run_system(handle).unwrap(); assert_eq!(INVOCATIONS_LEFT.get(), 0); } @@ -1074,4 +1254,23 @@ mod tests { Err(err) => panic!("Failed with wrong error. `{:?}`", err), } } + + #[test] + fn despawn_unused() { + let mut world = World::new(); + + fn system() {} + + let handle = world.register_system(system); + let entity = handle.entity(); + drop(handle); + + assert!(world.get_entity(entity).is_ok()); + + let mut cleanup = IntoSystem::into_system(despawn_unused_registered_systems); + cleanup.initialize(&mut world); + cleanup.run((), &mut world).unwrap(); + + assert!(world.get_entity(entity).is_err()); + } } diff --git a/crates/bevy_remote/src/lib.rs b/crates/bevy_remote/src/lib.rs index ec1f9b3b2c57f..007c378c88ff0 100644 --- a/crates/bevy_remote/src/lib.rs +++ b/crates/bevy_remote/src/lib.rs @@ -545,7 +545,7 @@ use bevy_ecs::{ InternedScheduleLabel, IntoScheduleConfigs, ScheduleBuildMetadata, ScheduleBuilt, ScheduleLabel, SystemSet, }, - system::{Commands, In, IntoSystem, ResMut, System, SystemId}, + system::{Commands, In, IntoSystem, ResMut, System, SystemHandle}, world::World, }; use bevy_platform::collections::HashMap; @@ -811,10 +811,10 @@ impl Plugin for RemotePlugin { remote_methods.insert( name.clone(), match handler { - RemoteMethodHandler::Instant(system) => RemoteMethodSystemId::Instant( + RemoteMethodHandler::Instant(system) => RemoteMethodSystemHandle::Instant( app.main_mut().world_mut().register_boxed_system(system), ), - RemoteMethodHandler::Watching(system) => RemoteMethodSystemId::Watching( + RemoteMethodHandler::Watching(system) => RemoteMethodSystemHandle::Watching( app.main_mut().world_mut().register_boxed_system(system), ), }, @@ -868,12 +868,14 @@ impl Plugin for RemotePlugin { render_remote_methods.insert( name, match handler { - RemoteMethodHandler::Instant(system) => RemoteMethodSystemId::Instant( - render_app.world_mut().register_boxed_system(system), - ), - RemoteMethodHandler::Watching(system) => RemoteMethodSystemId::Watching( + RemoteMethodHandler::Instant(system) => RemoteMethodSystemHandle::Instant( render_app.world_mut().register_boxed_system(system), ), + RemoteMethodHandler::Watching(system) => { + RemoteMethodSystemHandle::Watching( + render_app.world_mut().register_boxed_system(system), + ) + } }, ); } @@ -930,16 +932,18 @@ pub enum RemoteMethodHandler { Watching(Box>, Out = BrpResult>>>), } -/// The [`SystemId`] of a function that implements a remote instant method (`world.get_components`, `world.query`, etc.) +/// The [`SystemHandle`] of a function that implements a remote instant method +/// (`world.get_components`, `world.query`, etc.) /// /// The first parameter is the JSON value of the `params`. Typically, an /// implementation will deserialize these as the first thing they do. /// /// The returned JSON value will be returned as the response. Bevy will /// automatically populate the `id` field before sending. -pub type RemoteInstantMethodSystemId = SystemId>, BrpResult>; +pub type RemoteInstantMethodSystemHandle = SystemHandle>, BrpResult>; -/// The [`SystemId`] of a function that implements a remote watching method (`world.get_components+watch`, `world.list_components+watch`, etc.) +/// The [`SystemHandle`] of a function that implements a remote watching method +/// (`world.get_components+watch`, `world.list_components+watch`, etc.) /// /// The first parameter is the JSON value of the `params`. Typically, an /// implementation will deserialize these as the first thing they do. @@ -947,22 +951,23 @@ pub type RemoteInstantMethodSystemId = SystemId>, BrpResult>; /// The optional returned JSON value will be sent as a response. If no /// changes were detected this should be [`None`]. Re-running of this /// handler is done in the [`RemotePlugin`]. -pub type RemoteWatchingMethodSystemId = SystemId>, BrpResult>>; +pub type RemoteWatchingMethodSystemHandle = + SystemHandle>, BrpResult>>; -/// The [`SystemId`] of a function that can be used as a remote method. -#[derive(Debug, Clone, Copy)] -pub enum RemoteMethodSystemId { +/// The [`SystemHandle`] of a function that can be used as a remote method. +#[derive(Debug, Clone)] +pub enum RemoteMethodSystemHandle { /// A handler that only runs once and returns one response. - Instant(RemoteInstantMethodSystemId), + Instant(RemoteInstantMethodSystemHandle), /// A handler that watches for changes and response when a change is detected. - Watching(RemoteWatchingMethodSystemId), + Watching(RemoteWatchingMethodSystemHandle), } /// Holds all implementations of methods known to the server. /// /// Custom methods can be added to this list using [`RemoteMethods::insert`]. #[derive(Debug, Resource, Default)] -pub struct RemoteMethods(HashMap); +pub struct RemoteMethods(HashMap); impl RemoteMethods { /// Creates a new [`RemoteMethods`] resource with no methods registered in it. @@ -976,13 +981,13 @@ impl RemoteMethods { pub fn insert( &mut self, method_name: impl Into, - handler: RemoteMethodSystemId, - ) -> Option { + handler: RemoteMethodSystemHandle, + ) -> Option { self.0.insert(method_name.into(), handler) } - /// Get a [`RemoteMethodSystemId`] with its method name. - pub fn get(&self, method: &str) -> Option<&RemoteMethodSystemId> { + /// Get a [`RemoteMethodSystemHandle`] with its method name. + pub fn get(&self, method: &str) -> Option<&RemoteMethodSystemHandle> { self.0.get(method) } @@ -994,7 +999,7 @@ impl RemoteMethods { /// Holds the [`BrpMessage`]'s of all ongoing watching requests along with their handlers. #[derive(Debug, Resource, Default)] -pub struct RemoteWatchingRequests(Vec<(BrpMessage, RemoteWatchingMethodSystemId)>); +pub struct RemoteWatchingRequests(Vec<(BrpMessage, RemoteWatchingMethodSystemHandle)>); /// A single request from a Bevy Remote Protocol client to the server, /// serialized in JSON. @@ -1387,7 +1392,11 @@ fn process_remote_requests(world: &mut World) { while let Ok(message) = world.resource_mut::().try_recv() { // Fetch the handler for the method. If there's no such handler // registered, return an error. - let Some(&handler) = world.resource::().get(&message.method) else { + let Some(handler) = world + .resource::() + .get(&message.method) + .cloned() + else { let _ = message.sender.force_send(Err(BrpError { code: error_codes::METHOD_NOT_FOUND, message: format!("Method `{}` not found", message.method), @@ -1397,8 +1406,8 @@ fn process_remote_requests(world: &mut World) { }; match handler { - RemoteMethodSystemId::Instant(id) => { - let result = match world.run_system_with(id, message.params) { + RemoteMethodSystemHandle::Instant(handle) => { + let result = match world.run_system_with(handle, message.params) { Ok(result) => result, Err(error) => { let _ = message.sender.force_send(Err(BrpError { @@ -1412,11 +1421,11 @@ fn process_remote_requests(world: &mut World) { let _ = message.sender.force_send(result); } - RemoteMethodSystemId::Watching(id) => { + RemoteMethodSystemHandle::Watching(handle) => { world .resource_mut::() .0 - .push((message, id)); + .push((message, handle)); } } } @@ -1445,10 +1454,10 @@ fn process_ongoing_watching_requests(world: &mut World) { fn process_single_ongoing_watching_request( world: &mut World, message: &BrpMessage, - system_id: &RemoteWatchingMethodSystemId, + system_handle: &RemoteWatchingMethodSystemHandle, ) -> BrpResult> { world - .run_system_with(*system_id, message.params.clone()) + .run_system_with(system_handle, message.params.clone()) .map_err(|error| BrpError { code: error_codes::INTERNAL_ERROR, message: format!("Failed to run method handler: {error}"), diff --git a/examples/ecs/callbacks.rs b/examples/ecs/callbacks.rs index cf1586bf6619e..324e038674ec3 100644 --- a/examples/ecs/callbacks.rs +++ b/examples/ecs/callbacks.rs @@ -4,7 +4,7 @@ //! This pattern trades some performance for flexibility and works well for things like cutscenes, scripted events, //! or one-off UI-driven interactions that don't need to run every frame. -use bevy::{ecs::system::SystemId, prelude::*}; +use bevy::{ecs::system::SystemHandle, prelude::*}; fn main() { let mut app = App::new(); @@ -15,25 +15,25 @@ fn main() { #[derive(Component)] struct Callback { - system_id: SystemId<(), ()>, + system_handle: SystemHandle<(), ()>, } fn setup_callbacks(mut commands: Commands) { let trivial_callback = Callback { - system_id: commands.register_system(|| { + system_handle: commands.register_system(|| { println!("This is the trivial callback system"); }), }; let ordinary_system_callback = Callback { - system_id: commands.register_system(|query: Query<&Callback>| { + system_handle: commands.register_system(|query: Query<&Callback>| { let n_callbacks = query.iter().len(); println!("This is the ordinary callback system. There are currently {n_callbacks} callbacks in the world."); }), }; let exclusive_callback = Callback { - system_id: commands.register_system(|world: &mut World| { + system_handle: commands.register_system(|world: &mut World| { let n_entities = world.entities().len(); println!("This is the exclusive callback system. There are currently {n_entities} entities in the world."); }), @@ -48,6 +48,6 @@ fn setup_callbacks(mut commands: Commands) { // triggering the callback in response to some entity-event! fn run_callbacks(mut commands: Commands, query: Query<&Callback>) { for callback in query.iter() { - commands.run_system(callback.system_id); + commands.run_system(&callback.system_handle); } } diff --git a/examples/ecs/one_shot_systems.rs b/examples/ecs/one_shot_systems.rs index 627e7dea44a3f..a9b6c322a4d41 100644 --- a/examples/ecs/one_shot_systems.rs +++ b/examples/ecs/one_shot_systems.rs @@ -9,7 +9,7 @@ //! docs for more details. use bevy::{ - ecs::system::{RunSystemOnce, SystemId}, + ecs::system::{RunSystemOnce, SystemHandle}, prelude::*, }; @@ -29,7 +29,7 @@ fn main() { } #[derive(Component)] -struct Callback(SystemId); +struct Callback(SystemHandle); #[derive(Component)] struct Triggered; @@ -40,16 +40,16 @@ struct A; struct B; fn setup_with_commands(mut commands: Commands) { - let system_id = commands.register_system(system_a); - commands.spawn((Callback(system_id), A)); + let system_handle = commands.register_system(system_a); + commands.spawn((Callback(system_handle), A)); } fn setup_with_world(world: &mut World) { // We can run it once manually world.run_system_once(system_b).unwrap(); // Or with a Callback - let system_id = world.register_system(system_b); - world.spawn((Callback(system_id), B)); + let system_handle = world.register_system(system_b); + world.spawn((Callback(system_handle), B)); } /// Tag entities that have callbacks we want to run with the `Triggered` component. @@ -74,7 +74,7 @@ fn trigger_system( /// This could be done in an exclusive system rather than using `Commands` if preferred. fn evaluate_callbacks(query: Query<(Entity, &Callback), With>, mut commands: Commands) { for (entity, callback) in query.iter() { - commands.run_system(callback.0); + commands.run_system(&callback.0); commands.entity(entity).remove::(); } } diff --git a/examples/showcase/loading_screen.rs b/examples/showcase/loading_screen.rs index af9dc94498b2e..dc78a2140d965 100644 --- a/examples/showcase/loading_screen.rs +++ b/examples/showcase/loading_screen.rs @@ -1,5 +1,5 @@ //! Shows how to create a loading screen that waits for assets to load and render. -use bevy::{ecs::system::SystemId, prelude::*}; +use bevy::{ecs::system::SystemHandle, prelude::*}; use pipelines_ready::*; @@ -64,16 +64,16 @@ impl LoadingData { // This resource will hold the level related systems ID for later use. #[derive(Resource)] struct LevelData { - unload_level_id: SystemId, - level_1_id: SystemId, - level_2_id: SystemId, + unload_level_handle: SystemHandle, + level_1_handle: SystemHandle, + level_2_handle: SystemHandle, } fn setup(mut commands: Commands) { let level_data = LevelData { - unload_level_id: commands.register_system(unload_current_level), - level_1_id: commands.register_system(load_level_1), - level_2_id: commands.register_system(load_level_2), + unload_level_handle: commands.register_system(unload_current_level), + level_1_handle: commands.register_system(load_level_1), + level_2_handle: commands.register_system(load_level_2), }; commands.insert_resource(level_data); @@ -104,11 +104,11 @@ fn level_selection( // Only trigger a load if the current level is fully loaded. if let LoadingState::LevelReady = loading_state.as_ref() { if keyboard.just_pressed(KeyCode::Digit1) { - commands.run_system(level_data.unload_level_id); - commands.run_system(level_data.level_1_id); + commands.run_system(&level_data.unload_level_handle); + commands.run_system(&level_data.level_1_handle); } else if keyboard.just_pressed(KeyCode::Digit2) { - commands.run_system(level_data.unload_level_id); - commands.run_system(level_data.level_2_id); + commands.run_system(&level_data.unload_level_handle); + commands.run_system(&level_data.level_2_handle); } } } From 36cf72a582697bfa065aec20c92cacdb5602489b Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Mon, 4 May 2026 02:36:58 -0500 Subject: [PATCH 2/4] Adjust docs --- crates/bevy_ecs/src/system/system_registry.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index ab81cdbd1de5a..442b0fd992654 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -150,7 +150,9 @@ pub enum SystemHandle { /// A strong handle keeps the system entity alive as long as the handle /// (and any clones of it) exist. Strong(Arc), - /// A weak handle does not keep the system entity alive, but is [`Copy`]able. + /// A weak handle does not keep the system entity alive. If a weak handle is + /// returned by a registration function, the system entity must be + /// manually despawned. Weak(SystemId), } From e8b51cd28dfd64b9a80f1c07ef55e662b1b5982d Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Mon, 4 May 2026 02:57:50 -0500 Subject: [PATCH 3/4] adjust docs and fix ci --- crates/bevy_ecs/src/system/commands/command.rs | 6 ++++-- crates/bevy_ecs/src/system/commands/mod.rs | 3 ++- crates/bevy_ecs/src/system/system_registry.rs | 14 +++++++++----- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/command.rs b/crates/bevy_ecs/src/system/commands/command.rs index fb5902218a74e..4e1291f32411e 100644 --- a/crates/bevy_ecs/src/system/commands/command.rs +++ b/crates/bevy_ecs/src/system/commands/command.rs @@ -202,7 +202,8 @@ where } /// A [`Command`] that runs the given system, -/// caching its [`SystemHandle`] in a [`CachedSystemHandle`](crate::system::CachedSystemHandle) resource. +/// caching its [`SystemHandle`](crate::system::SystemHandle) in a +/// [`CachedSystemHandle`](crate::system::CachedSystemHandle) resource. pub fn run_system_cached(system: S) -> impl Command where M: 'static, @@ -215,7 +216,8 @@ where } /// A [`Command`] that runs the given system with the given input value, -/// caching its [`SystemHandle`] in a [`CachedSystemHandle`](crate::system::CachedSystemHandle) resource. +/// caching its [`SystemHandle`](crate::system::SystemHandle) in a +/// [`CachedSystemHandle`](crate::system::CachedSystemHandle) resource. /// /// To use the supplied input, the system should have a [`SystemInput`] as the first parameter. pub fn run_system_cached_with(system: S, input: I::Inner<'static>) -> impl Command diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 31bb3de2de07e..e1679c7b15021 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -2964,7 +2964,8 @@ mod tests { fn nothing() {} - let resources = world.iter_resources().count(); + // +1 accounts for `RegisteredSystemDespawner` resource + let resources = world.iter_resources().count() + 1; let handle = world.register_system_cached(nothing); assert_eq!(world.iter_resources().count(), resources + 1); assert!(world.get_entity(handle.entity()).is_ok()); diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 442b0fd992654..a5731ad62c3a0 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -33,7 +33,7 @@ impl RegisteredSystem { } } -/// A system that despawns any [`RegisteredSystem`] entities whose [`SystemHandle`] +/// A system that despawns any registered system entities whose [`SystemHandle`] /// reference count has reached zero. #[cfg(feature = "std")] pub fn despawn_unused_registered_systems( @@ -49,7 +49,7 @@ pub fn despawn_unused_registered_systems( } } -/// A resource that stores the channel for despawning unused [`RegisteredSystem`] +/// A resource that stores the channel for despawning unused registered system /// entities. #[derive(Resource)] #[cfg(feature = "std")] @@ -128,9 +128,13 @@ impl RemovedSystem { } } -/// A maybe-strong handle to an entity with a [`RegisteredSystem`] component. +/// A maybe-strong handle to an entity acting as a registered system. +/// /// Strong handles provide automatic cleanup of registered systems once all clones -/// of the handle are dropped, while weak handles do not. +/// of the handle are dropped, while weak handles do not. However, the **existence +/// of a strong handle does not prevent the registered system entity from being +/// despawned manually**, like with [`World::unregister_system`] or +/// [`World::unregister_system_cached`]. /// /// Strong handles are only returned by functions on [`World`], like /// [`World::register_system`] and [`World::register_system_cached`]. @@ -139,7 +143,7 @@ impl RemovedSystem { /// /// # Cleanup /// -/// [`RegisteredSystem`] entities are cleaned up by the [`despawn_unused_registered_systems`] +/// Registered system entities are cleaned up by the [`despawn_unused_registered_systems`] /// system, which is automatically added to the default app by the `bevy_ecs` /// crate when the "std" feature is enabled. If not using the default app, the /// "std" feature, or `bevy_app` in general, consider running this system From a2b6b4066e90a2c21e61207b6ccf77deb87ddaf2 Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Mon, 4 May 2026 03:28:51 -0500 Subject: [PATCH 4/4] try_despawn to avoid unnecessary warnings --- crates/bevy_ecs/src/system/system_registry.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index a5731ad62c3a0..62711b78f06df 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -44,7 +44,8 @@ pub fn despawn_unused_registered_systems( // is registered, so it's possible that it doesn't exist yet when this system runs. if let Some(despawner) = despawner { for entity in despawner.receiver.try_iter() { - commands.entity(entity).despawn(); + // In case the entity was already despawned manually, we ignore the error here. + commands.entity(entity).try_despawn(); } } }