From 0c3f52aab4c47e86626cb1ca36ae14b8d310be51 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Mon, 1 Apr 2024 11:10:37 -0400 Subject: [PATCH 1/8] feat: test check in ci --- tools/ci/src/main.rs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tools/ci/src/main.rs b/tools/ci/src/main.rs index 617f96f7f7da8..7b1d59c0ef2c0 100644 --- a/tools/ci/src/main.rs +++ b/tools/ci/src/main.rs @@ -18,6 +18,7 @@ bitflags! { const EXAMPLE_CHECK = 0b10000000; const COMPILE_CHECK = 0b100000000; const CFG_CHECK = 0b1000000000; + const TEST_CHECK = 0b10000000000; } } @@ -56,13 +57,18 @@ fn main() { ("doc", Check::DOC_TEST | Check::DOC_CHECK), ( "compile", - Check::COMPILE_FAIL | Check::BENCH_CHECK | Check::EXAMPLE_CHECK | Check::COMPILE_CHECK, + Check::COMPILE_FAIL + | Check::BENCH_CHECK + | Check::EXAMPLE_CHECK + | Check::COMPILE_CHECK + | Check::TEST_CHECK, ), ("format", Check::FORMAT), ("clippy", Check::CLIPPY), ("compile-fail", Check::COMPILE_FAIL), ("bench-check", Check::BENCH_CHECK), ("example-check", Check::EXAMPLE_CHECK), + ("test-check", Check::TEST_CHECK), ("cfg-check", Check::CFG_CHECK), ("doc-check", Check::DOC_CHECK), ("doc-test", Check::DOC_TEST), @@ -314,6 +320,24 @@ fn main() { ); } + if checks.contains(Check::TEST_CHECK) { + let mut args = vec!["--workspace", "--examples"]; + + if flags.contains(Flag::KEEP_GOING) { + args.push("--keep-going"); + } + + test_suite.insert( + Check::TEST_CHECK, + vec![CITest { + command: cmd!(sh, "cargo check {args...}"), + failure_message: "Please fix compiler examples for tests in output above.", + subdir: None, + env_vars: Vec::new(), + }], + ); + } + // Actually run the tests: let mut failed_checks: Check = Check::empty(); From e9e20c8f6a019f82faeb072d4922fca1deae5dcc Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Sun, 31 Mar 2024 19:34:31 -0400 Subject: [PATCH 2/8] style: reorder `remove_resource` This makes the declaration order consistent with the surrounding methods of `Commands::remove_resource`. This is an opinionated change that I would be fine with reverting. --- crates/bevy_ecs/src/system/commands/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index e4fa7f65520d9..7952966410bd8 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1122,11 +1122,6 @@ fn init_resource(world: &mut World) { world.init_resource::(); } -/// A [`Command`] that removes the [resource](Resource) `R` from the world. -fn remove_resource(world: &mut World) { - world.remove_resource::(); -} - /// A [`Command`] that inserts a [`Resource`] into the world. fn insert_resource(resource: R) -> impl Command { move |world: &mut World| { @@ -1134,6 +1129,11 @@ fn insert_resource(resource: R) -> impl Command { } } +/// A [`Command`] that removes the [resource](Resource) `R` from the world. +fn remove_resource(world: &mut World) { + world.remove_resource::(); +} + /// [`EntityCommand`] to log the components of a given entity. See [`EntityCommands::log_components`]. fn log_components(entity: Entity, world: &mut World) { let debug_infos: Vec<_> = world From 981b70328aede9e3389abba9ba8fbf46a4ca09ff Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Sun, 31 Mar 2024 19:48:53 -0400 Subject: [PATCH 3/8] feat: non-send commands --- crates/bevy_ecs/src/system/commands/mod.rs | 120 +++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 7952966410bd8..ee2de08aa0ac2 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -493,6 +493,102 @@ impl<'w, 's> Commands<'w, 's> { self.queue.push(remove_resource::); } + /// Pushes a [`Command`] to the queue for inserting a non-[`Send`] resource in the [`World`] with an inferred value. + /// + /// See [`World::init_non_send_resource`] for more details. + /// + /// # Example + /// + /// ``` + /// # use bevy::prelude::*; + /// # + /// struct MyNonSend(*const u8); + /// + /// impl Default for MyNonSend { + /// fn default() -> Self { + /// MyNonSend(std::ptr::null()) + /// } + /// } + /// + /// fn create_my_non_send(mut commands: Commands) { + /// commands.init_non_send_resource::(); + /// } + /// # + /// # App::new() + /// # .add_systems(Startup, (create_my_non_send, check).chain()) + /// # .run(); + /// # + /// # fn check(my_non_send: NonSend) { + /// # assert!(my_non_send.0.is_null()); + /// # } + /// ``` + pub fn init_non_send_resource(&mut self) { + self.queue.push(init_non_send_resource::); + } + + /// Pushes a [`Command`] to the queue for inserting a non-[`Send`] resource in the [`World`] with an specific value. + /// + /// Note that this command takes a closure, not a value. This closure is executed on the main thread and should return + /// the value of the non-[`Send`] resource. The closure itself must be [`Send`], but its returned value does not need to be. + /// + /// See [`World::insert_non_send_resource`] for more details. + /// + /// # Example + /// + /// ``` + /// # use bevy::prelude::*; + /// # + /// struct MyNonSend(*const u8); + /// + /// fn create_my_non_send(mut commands: Commands) { + /// // Note that this is a closure: + /// commands.insert_non_send_resource(|| { + /// MyNonSend(std::ptr::null()) + /// }); + /// } + /// # + /// # App::new() + /// # .add_systems(Startup, (create_my_non_send, check).chain()) + /// # .run(); + /// # + /// # fn check(my_non_send: NonSend) { + /// # assert!(my_non_send.0.is_null()); + /// # } + /// ``` + pub fn insert_non_send_resource(&mut self, func: F) + where + F: FnOnce() -> R + Send + 'static, + R: 'static, + { + self.queue.push(insert_non_send_resource(func)); + } + + /// Pushes a [`Command`] to the queue for removing a non-[`Send`] resource from the [`World`]. + /// + /// See [`World::remove_non_send_resource`] for more details. + /// + /// ``` + /// # use bevy::prelude::*; + /// # + /// struct MyNonSend(*const u8); + /// + /// fn remove_my_non_send(mut commands: Commands) { + /// commands.remove_non_send_resource::(); + /// } + /// # + /// # App::new() + /// # .insert_non_send_resource(MyNonSend(std::ptr::null())) + /// # .add_systems(Startup, (remove_my_non_send, check).chain()) + /// # .run(); + /// # + /// # fn check(my_non_send: Option>) { + /// # assert!(my_non_send.is_none()); + /// # } + /// ``` + pub fn remove_non_send_resource(&mut self) { + self.queue.push(remove_non_send_resource::); + } + /// Runs the system corresponding to the given [`SystemId`]. /// Systems are ran in an exclusive and single threaded way. /// Running slow systems can become a bottleneck. @@ -1134,6 +1230,30 @@ fn remove_resource(world: &mut World) { world.remove_resource::(); } +/// A [`Command`] that inserts a non-[`Send`] resource `R` into the world using +/// a value created with the [`FromWorld`] trait. +fn init_non_send_resource(world: &mut World) { + world.init_non_send_resource::(); +} + +/// A [`Command`] that removes a non-[`Send`] resource `R` from the world. +fn remove_non_send_resource(world: &mut World) { + world.remove_non_send_resource::(); +} + +/// A [`Command`] that inserts a non-[`Send`] resource into the world by calling +/// `func` on the main thread and inserting its returned value. +fn insert_non_send_resource(func: F) -> impl Command +where + // `R` is not `Send`, but the function is! + F: FnOnce() -> R + Send + 'static, + R: 'static, +{ + move |world: &mut World| { + world.insert_non_send_resource((func)()); + } +} + /// [`EntityCommand`] to log the components of a given entity. See [`EntityCommands::log_components`]. fn log_components(entity: Entity, world: &mut World) { let debug_infos: Vec<_> = world From e92a8e90f994eeb9f3315cd8e9fbaf705a36dd73 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Sun, 31 Mar 2024 20:57:33 -0400 Subject: [PATCH 4/8] feat: add tests --- crates/bevy_ecs/src/system/commands/mod.rs | 60 ++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index ee2de08aa0ac2..e76510c77f72a 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1444,4 +1444,64 @@ mod tests { assert!(world.contains_resource::>()); assert!(world.contains_resource::>()); } + + mod non_send { + use super::*; + + #[allow(dead_code)] + struct MyNonSend(*const ()); + + impl Default for MyNonSend { + fn default() -> Self { + MyNonSend(std::ptr::null()) + } + } + + #[test] + fn init() { + let mut world = World::default(); + let mut queue = CommandQueue::default(); + + { + let mut commands = Commands::new(&mut queue, &world); + commands.init_non_send_resource::(); + } + + queue.apply(&mut world); + + assert!(world.contains_non_send::()); + } + + #[test] + fn insert() { + let mut world = World::default(); + let mut queue = CommandQueue::default(); + + { + let mut commands = Commands::new(&mut queue, &world); + commands.insert_non_send_resource(|| MyNonSend(std::ptr::from_ref(&()))); + } + + queue.apply(&mut world); + + assert!(world.contains_non_send::()); + } + + #[test] + fn remove() { + let mut world = World::default(); + let mut queue = CommandQueue::default(); + + world.init_non_send_resource::(); + + { + let mut commands = Commands::new(&mut queue, &world); + commands.remove_non_send_resource::(); + } + + queue.apply(&mut world); + + assert!(!world.contains_non_send::()); + } + } } From 3938830eadc824f4bf743b0c71f61bc656c1185d Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Sun, 31 Mar 2024 21:10:35 -0400 Subject: [PATCH 5/8] feat: test that `insert_non_send_resource` does not send resource --- crates/bevy_ecs/src/system/commands/mod.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index e76510c77f72a..06a4e4bfb8abd 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1503,5 +1503,27 @@ mod tests { assert!(!world.contains_non_send::()); } + + #[test] + fn is_in_fact_not_being_sent() { + use std::thread::{self, ThreadId}; + + let mut world = World::default(); + let mut queue = CommandQueue::default(); + + thread::scope(|s| { + s.spawn(|| { + let mut commands = Commands::new(&mut queue, &world); + commands.insert_non_send_resource(|| thread::current().id()); + }); + }); + + queue.apply(&mut world); + + assert_eq!( + *world.non_send_resource::(), + std::thread::current().id() + ); + } } } From 533176885c5635f2306fff1b9ecf21ee7f89a648 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Mon, 1 Apr 2024 11:28:26 -0400 Subject: [PATCH 6/8] fix: tests I incorrectly assumed that bevy was added as a dev-dependency. :) --- crates/bevy_ecs/src/system/commands/mod.rs | 67 +++++++--------------- 1 file changed, 22 insertions(+), 45 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 06a4e4bfb8abd..81c56c230ae41 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -500,27 +500,20 @@ impl<'w, 's> Commands<'w, 's> { /// # Example /// /// ``` - /// # use bevy::prelude::*; + /// # use bevy_ecs::prelude::*; /// # - /// struct MyNonSend(*const u8); - /// - /// impl Default for MyNonSend { - /// fn default() -> Self { - /// MyNonSend(std::ptr::null()) - /// } - /// } - /// - /// fn create_my_non_send(mut commands: Commands) { - /// commands.init_non_send_resource::(); - /// } + /// # struct MyNonSend(*const u8); /// # - /// # App::new() - /// # .add_systems(Startup, (create_my_non_send, check).chain()) - /// # .run(); + /// # impl Default for MyNonSend { + /// # fn default() -> Self { + /// # MyNonSend(std::ptr::null()) + /// # } + /// # } /// # - /// # fn check(my_non_send: NonSend) { - /// # assert!(my_non_send.0.is_null()); + /// # fn init_my_non_send(mut commands: Commands) { + /// commands.init_non_send_resource::(); /// # } + /// # bevy_ecs::system::assert_is_system(init_my_non_send); /// ``` pub fn init_non_send_resource(&mut self) { self.queue.push(init_non_send_resource::); @@ -536,24 +529,16 @@ impl<'w, 's> Commands<'w, 's> { /// # Example /// /// ``` - /// # use bevy::prelude::*; - /// # - /// struct MyNonSend(*const u8); - /// - /// fn create_my_non_send(mut commands: Commands) { - /// // Note that this is a closure: - /// commands.insert_non_send_resource(|| { - /// MyNonSend(std::ptr::null()) - /// }); - /// } + /// # use bevy_ecs::prelude::*; /// # - /// # App::new() - /// # .add_systems(Startup, (create_my_non_send, check).chain()) - /// # .run(); + /// # struct MyNonSend(*const u8); /// # - /// # fn check(my_non_send: NonSend) { - /// # assert!(my_non_send.0.is_null()); + /// # fn insert_my_non_send(mut commands: Commands) { + /// commands.insert_non_send_resource(|| { + /// MyNonSend(std::ptr::null()) + /// }); /// # } + /// # bevy_ecs::system::assert_is_system(insert_my_non_send); /// ``` pub fn insert_non_send_resource(&mut self, func: F) where @@ -568,22 +553,14 @@ impl<'w, 's> Commands<'w, 's> { /// See [`World::remove_non_send_resource`] for more details. /// /// ``` - /// # use bevy::prelude::*; - /// # - /// struct MyNonSend(*const u8); - /// - /// fn remove_my_non_send(mut commands: Commands) { - /// commands.remove_non_send_resource::(); - /// } + /// # use bevy_ecs::prelude::*; /// # - /// # App::new() - /// # .insert_non_send_resource(MyNonSend(std::ptr::null())) - /// # .add_systems(Startup, (remove_my_non_send, check).chain()) - /// # .run(); + /// # struct MyNonSend(*const u8); /// # - /// # fn check(my_non_send: Option>) { - /// # assert!(my_non_send.is_none()); + /// # fn remove_my_non_send(mut commands: Commands) { + /// commands.remove_non_send_resource::(); /// # } + /// # bevy_ecs::system::assert_is_system(remove_my_non_send); /// ``` pub fn remove_non_send_resource(&mut self) { self.queue.push(remove_non_send_resource::); From fadc6833b679c3c73559cc72387800716c47332d Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Mon, 1 Apr 2024 11:36:00 -0400 Subject: [PATCH 7/8] feat: give example of trying to send a non-send Also remove the `is_in_fact_not_being_sent` test. --- crates/bevy_ecs/src/system/commands/mod.rs | 39 ++++++++++------------ 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 81c56c230ae41..bfa9afbd91906 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -540,6 +540,23 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(insert_my_non_send); /// ``` + /// + /// Note that the value must be constructed inside of the closure. Moving it will fail to compile: + /// + /// ```compile_fail,E0277 + /// # use bevy_ecs::prelude::*; + /// # + /// # struct MyNonSend(*const u8); + /// # + /// # fn insert_my_non_send(mut commands: Commands) { + /// let my_non_send = MyNonSend(std::ptr::null()); + /// + /// commands.insert_non_send_resource(move || { + /// my_non_send + /// }); + /// # } + /// # bevy_ecs::system::assert_is_system(insert_my_non_send); + /// ``` pub fn insert_non_send_resource(&mut self, func: F) where F: FnOnce() -> R + Send + 'static, @@ -1480,27 +1497,5 @@ mod tests { assert!(!world.contains_non_send::()); } - - #[test] - fn is_in_fact_not_being_sent() { - use std::thread::{self, ThreadId}; - - let mut world = World::default(); - let mut queue = CommandQueue::default(); - - thread::scope(|s| { - s.spawn(|| { - let mut commands = Commands::new(&mut queue, &world); - commands.insert_non_send_resource(|| thread::current().id()); - }); - }); - - queue.apply(&mut world); - - assert_eq!( - *world.non_send_resource::(), - std::thread::current().id() - ); - } } } From 451c3bee74b795d0c189499a8c4419a8c161c380 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Mon, 1 Apr 2024 11:37:53 -0400 Subject: [PATCH 8/8] refactor: rustfmt --- crates/bevy_ecs/src/system/commands/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index bfa9afbd91906..a05225da5745d 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -540,9 +540,9 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(insert_my_non_send); /// ``` - /// + /// /// Note that the value must be constructed inside of the closure. Moving it will fail to compile: - /// + /// /// ```compile_fail,E0277 /// # use bevy_ecs::prelude::*; /// # @@ -550,7 +550,7 @@ impl<'w, 's> Commands<'w, 's> { /// # /// # fn insert_my_non_send(mut commands: Commands) { /// let my_non_send = MyNonSend(std::ptr::null()); - /// + /// /// commands.insert_non_send_resource(move || { /// my_non_send /// });