From b1c48138026432be94a1a73e8ff3f3bcab28ace3 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Mon, 1 Apr 2024 18:42:51 +0200 Subject: [PATCH 1/8] add iter_resources and iter_resources_mut methods on world --- crates/bevy_ecs/src/world/mod.rs | 184 +++++++++++++++++++++++++++++++ 1 file changed, 184 insertions(+) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d245df8241930..5c172eb67e841 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2136,6 +2136,190 @@ impl World { } } + /// Iterates over all resources in the world. + /// + /// # Examples + /// + /// ## Printing the size of all resources + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # #[derive(Resource)] + /// # struct A(u32); + /// # #[derive(Resource)] + /// # struct B(u32); + /// # + /// # let mut world = World::new(); + /// # world.insert_resource(A(1)); + /// # world.insert_resource(B(2)); + /// let mut total = 0; + /// for (info, _) in world.iter_resources() { + /// println!("Resource: {}", info.name()); + /// println!("Size: {} bytes", info.layout().size()); + /// total += info.layout().size(); + /// } + /// println!("----------------"); + /// println!("Total size: {} bytes", total); + /// # assert_eq!(total, std::mem::size_of::() + std::mem::size_of::()); + /// ``` + /// + /// ## Dynamically running closures for resources matching specific `TypeId`s + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # use std::collections::HashMap; + /// # use std::any::TypeId; + /// # use bevy_ptr::Ptr; + /// # #[derive(Resource)] + /// # struct A(u32); + /// # #[derive(Resource)] + /// # struct B(u32); + /// # + /// # let mut world = World::new(); + /// # world.insert_resource(A(1)); + /// # world.insert_resource(B(2)); + /// # + /// // In this example, `A` and `B` are resources. We deliberately do not use the + /// // `bevy_reflect` crate here to showcase the low-level `Ptr` usage. You should + /// // probably use something like `ReflectFromPtr` in a real-world scenario. + /// + /// // Create the hash map that will store the closures for each resource type + /// let mut closures: HashMap)>> = HashMap::new(); + /// + /// // Add closure for `A` + /// closures.insert(TypeId::of::(), Box::new(|ptr| { + /// let a = unsafe { &ptr.deref::() }; + /// # assert_eq!(a.0, 1); + /// // ... do something with `a` here + /// })); + /// + /// // Add closure for `B` + /// closures.insert(TypeId::of::(), Box::new(|ptr| { + /// let b = unsafe { &ptr.deref::() }; + /// # assert_eq!(b.0, 2); + /// // ... do something with `b` here + /// })); + /// + /// // Iterate all resources, in order to run the closures for each matching resource type + /// for (info, ptr) in world.iter_resources() { + /// let Some(type_id) = info.type_id() else { + /// // It's possible for resources to not have a `TypeId` (e.g. non-Rust resources + /// // dynamically inserted via a scripting language) in which case we can't match them. + /// continue; + /// }; + /// + /// let Some(closure) = closures.get(&type_id) else { + /// // No closure for this resource type, skip it. + /// continue; + /// }; + /// + /// // Run the closure for the resource + /// closure(&ptr); + /// } + /// ``` + #[inline] + pub fn iter_resources(&self) -> impl Iterator)> { + self.storages + .resources + .iter() + .filter_map(|(component_id, data)| { + let component_info = self.components.get_info(component_id)?; + Some((component_info, data.get_data()?)) + }) + } + + /// Mutably iterates over all resources in the world. + /// + /// # Example + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # use bevy_ecs::change_detection::MutUntyped; + /// # use std::collections::HashMap; + /// # use std::any::TypeId; + /// # #[derive(Resource)] + /// # struct A(u32); + /// # #[derive(Resource)] + /// # struct B(u32); + /// # + /// # let mut world = World::new(); + /// # world.insert_resource(A(1)); + /// # world.insert_resource(B(2)); + /// # + /// // In this example, `A` and `B` are resources. We deliberately do not use the + /// // `bevy_reflect` crate here to showcase the low-level `MutUntyped` usage. You should + /// // probably use something like `ReflectFromPtr` in a real-world scenario. + /// + /// // Create the hash map that will store the mutator closures for each resource type + /// let mut mutators: HashMap)>> = HashMap::new(); + /// + /// // Add mutator closure for `A` + /// mutators.insert(TypeId::of::(), Box::new(|mut_untyped| { + /// // Note: `MutUntyped::as_mut()` automatically marks the resource as changed + /// // for ECS change detection, and gives us a `PtrMut` we can use to mutate the resource. + /// let a = unsafe { &mut mut_untyped.as_mut().deref_mut::() }; + /// # a.0 += 1; + /// // ... mutate `a` here + /// })); + /// + /// // Add mutator closure for `B` + /// mutators.insert(TypeId::of::(), Box::new(|mut_untyped| { + /// let b = unsafe { &mut mut_untyped.as_mut().deref_mut::() }; + /// # b.0 += 1; + /// // ... mutate `b` here + /// })); + /// + /// // Iterate all resources, in order to run the mutator closures for each matching resource type + /// for (info, mut mut_untyped) in world.iter_resources_mut() { + /// let Some(type_id) = info.type_id() else { + /// // It's possible for resources to not have a `TypeId` (e.g. non-Rust resources + /// // dynamically inserted via a scripting language) in which case we can't match them. + /// continue; + /// }; + /// + /// let Some(mutator) = mutators.get(&type_id) else { + /// // No mutator closure for this resource type, skip it. + /// continue; + /// }; + /// + /// // Run the mutator closure for the resource + /// mutator(&mut mut_untyped); + /// } + /// # assert_eq!(world.resource::().0, 2); + /// # assert_eq!(world.resource::().0, 3); + /// ``` + #[inline] + pub fn iter_resources_mut(&mut self) -> impl Iterator)> { + self.storages + .resources + .iter() + .filter_map(|(component_id, data)| { + let component_info = self.components.get_info(component_id)?; + let (ptr, ticks) = data.get_with_ticks()?; + + // SAFETY: + // - We have exclusive access to the world, so no other code can be aliasing the `TickCells` + // - We only hold one `TicksMut` at a time, and we let go of it before getting the next oned + let ticks = unsafe { + TicksMut::from_tick_cells( + ticks, + self.last_change_tick(), + self.read_change_tick(), + ) + }; + + let mut_untyped = MutUntyped { + // SAFETY: + // - We have exclusive access to the world, so no other code can be aliasing the `Ptr` + // - We iterate one resource at a time, and we let go of each `PtrMut` before getting the next one + value: unsafe { ptr.assert_unique() }, + ticks, + }; + + Some((component_info, mut_untyped)) + }) + } + /// Gets a `!Send` resource to the resource with the id [`ComponentId`] if it exists. /// The returned pointer must not be used to modify the resource, and must not be /// dereferenced after the immutable borrow of the [`World`] ends. From 058884ba159540e9b6c68a3d9ab6cf16b0510458 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Mon, 1 Apr 2024 18:56:11 +0200 Subject: [PATCH 2/8] add tests for iter_resources and iter_resources_mut --- crates/bevy_ecs/src/world/mod.rs | 66 ++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 5c172eb67e841..d0a9a2dae7ea7 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2738,6 +2738,12 @@ mod tests { #[derive(Resource)] struct TestResource(u32); + #[derive(Resource)] + struct TestResource2(String); + + #[derive(Resource)] + struct TestResource3; + #[test] fn get_resource_by_id() { let mut world = World::new(); @@ -2778,6 +2784,66 @@ mod tests { assert_eq!(resource.0, 43); } + #[test] + fn iter_resources() { + let mut world = World::new(); + world.insert_resource(TestResource(42)); + world.insert_resource(TestResource2("Hello, world!".to_string())); + world.insert_resource(TestResource3); + world.remove_resource::(); + + let mut iter = world.iter_resources(); + + let (info, ptr) = iter.next().unwrap(); + assert_eq!(info.name(), std::any::type_name::()); + // SAFETY: We know that the resource is of type `TestResource` + assert_eq!(unsafe { ptr.deref::().0 }, 42); + + let (info, ptr) = iter.next().unwrap(); + assert_eq!(info.name(), std::any::type_name::()); + // SAFETY: We know that the resource is of type `TestResource2` + assert_eq!( + unsafe { &ptr.deref::().0 }, + &"Hello, world!".to_string() + ); + + assert!(iter.next().is_none()); + } + + #[test] + fn iter_resources_mut() { + let mut world = World::new(); + world.insert_resource(TestResource(42)); + world.insert_resource(TestResource2("Hello, world!".to_string())); + world.insert_resource(TestResource3); + world.remove_resource::(); + + let mut iter = world.iter_resources_mut(); + + let (info, mut mut_untyped) = iter.next().unwrap(); + assert_eq!(info.name(), std::any::type_name::()); + // SAFETY: We know that the resource is of type `TestResource` + unsafe { + mut_untyped.as_mut().deref_mut::().0 = 43; + }; + + let (info, mut mut_untyped) = iter.next().unwrap(); + assert_eq!(info.name(), std::any::type_name::()); + // SAFETY: We know that the resource is of type `TestResource2` + unsafe { + mut_untyped.as_mut().deref_mut::().0 = "Hello, world?".to_string(); + }; + + assert!(iter.next().is_none()); + std::mem::drop(iter); + + assert_eq!(world.resource::().0, 43); + assert_eq!( + world.resource::().0, + "Hello, world?".to_string() + ); + } + #[test] fn custom_resource_with_layout() { static DROP_COUNT: AtomicU32 = AtomicU32::new(0); From cb555a09e1822ee42fa2ae39196115c9c9dcd733 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Mon, 1 Apr 2024 19:13:25 +0200 Subject: [PATCH 3/8] move safety comment --- crates/bevy_ecs/src/world/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d0a9a2dae7ea7..f4147d00a93fd 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2801,8 +2801,8 @@ mod tests { let (info, ptr) = iter.next().unwrap(); assert_eq!(info.name(), std::any::type_name::()); - // SAFETY: We know that the resource is of type `TestResource2` assert_eq!( + // SAFETY: We know that the resource is of type `TestResource2` unsafe { &ptr.deref::().0 }, &"Hello, world!".to_string() ); From 0174d386156ef97245babc0148b058a40c1d4ec1 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Mon, 1 Apr 2024 23:00:53 +0200 Subject: [PATCH 4/8] add doc link Co-authored-by: Alice Cecile --- crates/bevy_ecs/src/world/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index f4147d00a93fd..0643e8eebdeb8 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2180,7 +2180,7 @@ impl World { /// # world.insert_resource(B(2)); /// # /// // In this example, `A` and `B` are resources. We deliberately do not use the - /// // `bevy_reflect` crate here to showcase the low-level `Ptr` usage. You should + /// // `bevy_reflect` crate here to showcase the low-level [`Ptr`] usage. You should /// // probably use something like `ReflectFromPtr` in a real-world scenario. /// /// // Create the hash map that will store the closures for each resource type From 4f2feda0248ce92dc1466e133d78cbd0dc0ea991 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Tue, 2 Apr 2024 10:19:44 +0200 Subject: [PATCH 5/8] improve docs Co-authored-by: James Liu Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com> --- crates/bevy_ecs/src/world/mod.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 0643e8eebdeb8..cfe68f2612a96 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2138,6 +2138,9 @@ impl World { /// Iterates over all resources in the world. /// + /// The returned iterator provides lifetimed, but type-unsafe pointers. Actually reading the contents + /// of each resource will require the use of unsafe code. + /// /// # Examples /// /// ## Printing the size of all resources @@ -2158,7 +2161,6 @@ impl World { /// println!("Size: {} bytes", info.layout().size()); /// total += info.layout().size(); /// } - /// println!("----------------"); /// println!("Total size: {} bytes", total); /// # assert_eq!(total, std::mem::size_of::() + std::mem::size_of::()); /// ``` @@ -2188,6 +2190,7 @@ impl World { /// /// // Add closure for `A` /// closures.insert(TypeId::of::(), Box::new(|ptr| { + /// // SAFETY: We assert ptr is the same type of A with TypeId of A /// let a = unsafe { &ptr.deref::() }; /// # assert_eq!(a.0, 1); /// // ... do something with `a` here @@ -2230,6 +2233,9 @@ impl World { /// Mutably iterates over all resources in the world. /// + /// The returned iterator provides lifetimed, but type-unsafe pointers. Actually reading from or writing + /// to the contents of each resource will require the use of unsafe code. + /// /// # Example /// /// ``` @@ -2299,7 +2305,7 @@ impl World { // SAFETY: // - We have exclusive access to the world, so no other code can be aliasing the `TickCells` - // - We only hold one `TicksMut` at a time, and we let go of it before getting the next oned + // - We only hold one `TicksMut` at a time, and we let go of it before getting the next one let ticks = unsafe { TicksMut::from_tick_cells( ticks, From 009535dbc89680d1787349d3cd0209b7df2aece2 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Tue, 2 Apr 2024 10:20:10 +0200 Subject: [PATCH 6/8] use `debug_checked_unwrap` Co-authored-by: James Liu --- crates/bevy_ecs/src/world/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index cfe68f2612a96..b0233cd667d0f 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2226,7 +2226,8 @@ impl World { .resources .iter() .filter_map(|(component_id, data)| { - let component_info = self.components.get_info(component_id)?; + // SAFETY: If a resource has been initialized, a corresponding ComponentInfo must exist with it's ID. + let component_info = unsafe { self.components.get_info(component_id).debug_checked_unwrap() }; Some((component_info, data.get_data()?)) }) } @@ -2300,7 +2301,8 @@ impl World { .resources .iter() .filter_map(|(component_id, data)| { - let component_info = self.components.get_info(component_id)?; + // SAFETY: If a resource has been initialized, a corresponding ComponentInfo must exist with it's ID. + let component_info = unsafe { self.components.get_info(component_id).debug_checked_unwrap() }; let (ptr, ticks) = data.get_with_ticks()?; // SAFETY: From 99a0b46b1613e7aa2f879e4ba23990d3c8690b53 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Tue, 2 Apr 2024 10:23:23 +0200 Subject: [PATCH 7/8] format --- crates/bevy_ecs/src/world/mod.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index b0233cd667d0f..a98a80a69b1e6 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2190,7 +2190,7 @@ impl World { /// /// // Add closure for `A` /// closures.insert(TypeId::of::(), Box::new(|ptr| { - /// // SAFETY: We assert ptr is the same type of A with TypeId of A + /// // SAFETY: We assert ptr is the same type of A with TypeId of A /// let a = unsafe { &ptr.deref::() }; /// # assert_eq!(a.0, 1); /// // ... do something with `a` here @@ -2227,7 +2227,11 @@ impl World { .iter() .filter_map(|(component_id, data)| { // SAFETY: If a resource has been initialized, a corresponding ComponentInfo must exist with it's ID. - let component_info = unsafe { self.components.get_info(component_id).debug_checked_unwrap() }; + let component_info = unsafe { + self.components + .get_info(component_id) + .debug_checked_unwrap() + }; Some((component_info, data.get_data()?)) }) } @@ -2302,7 +2306,11 @@ impl World { .iter() .filter_map(|(component_id, data)| { // SAFETY: If a resource has been initialized, a corresponding ComponentInfo must exist with it's ID. - let component_info = unsafe { self.components.get_info(component_id).debug_checked_unwrap() }; + let component_info = unsafe { + self.components + .get_info(component_id) + .debug_checked_unwrap() + }; let (ptr, ticks) = data.get_with_ticks()?; // SAFETY: From 03d64d8a1db4a8df7ecab39780c78a48c9d88aa2 Mon Sep 17 00:00:00 2001 From: Mateusz Wachowiak Date: Tue, 2 Apr 2024 10:29:52 +0200 Subject: [PATCH 8/8] safety comments --- crates/bevy_ecs/src/world/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index a98a80a69b1e6..ed6781ffe3596 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2198,6 +2198,7 @@ impl World { /// /// // Add closure for `B` /// closures.insert(TypeId::of::(), Box::new(|ptr| { + /// // SAFETY: We assert ptr is the same type of B with TypeId of B /// let b = unsafe { &ptr.deref::() }; /// # assert_eq!(b.0, 2); /// // ... do something with `b` here @@ -2268,6 +2269,7 @@ impl World { /// mutators.insert(TypeId::of::(), Box::new(|mut_untyped| { /// // Note: `MutUntyped::as_mut()` automatically marks the resource as changed /// // for ECS change detection, and gives us a `PtrMut` we can use to mutate the resource. + /// // SAFETY: We assert ptr is the same type of A with TypeId of A /// let a = unsafe { &mut mut_untyped.as_mut().deref_mut::() }; /// # a.0 += 1; /// // ... mutate `a` here @@ -2275,6 +2277,7 @@ impl World { /// /// // Add mutator closure for `B` /// mutators.insert(TypeId::of::(), Box::new(|mut_untyped| { + /// // SAFETY: We assert ptr is the same type of B with TypeId of B /// let b = unsafe { &mut mut_untyped.as_mut().deref_mut::() }; /// # b.0 += 1; /// // ... mutate `b` here