Automatic cleanup of one-shot systems

_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.

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();

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<const N: usize>(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<const N: usize>(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();
             });
         });
     }

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::<AppExit>();
 
         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<I, O, M>(
         &mut self,
         system: impl IntoSystem<I, O, M> + 'static,
-    ) -> SystemId<I, O>
+    ) -> SystemHandle<I, O>
     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());
+    }
 }

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<I, O, M>(
         &mut self,
         system: impl IntoSystem<I, O, M> + 'static,
-    ) -> SystemId<I, O>
+    ) -> SystemHandle<I, O>
     where
         I: SystemInput + 'static,
         O: 'static,

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",

crates/bevy_ecs/src/system/commands/command.rs

@@ -202,7 +202,8 @@ where
 }
 
 /// A [`Command`] that runs the given system,
-/// caching its [`SystemId`] in a [`CachedSystemId`](crate::system::CachedSystemId) resource.
+/// caching its [`SystemHandle`](crate::system::SystemHandle) in a
+/// [`CachedSystemHandle`](crate::system::CachedSystemHandle) resource.
 pub fn run_system_cached<M, S>(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 [`SystemId`] in a [`CachedSystemId`](crate::system::CachedSystemId) 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<I, M, S>(system: S, input: I::Inner<'static>) -> impl Command