Repository: Ralith/hecs
Pull request #212 (2021-11-28 15:21:15)

Add EntityBuilderClone::add_bundle and tests
This fixes issue #211 without any apparent breakages in the public API, allowing for an immediate patch release.

The commit also adds appropriate tests for the cloneable entity builder which were previously missing.

ten3roberts: All tests pass after introducing the changes

Patch:
diff --git a/src/bundle.rs b/src/bundle.rs
index b9a256c..7f8bc61 100644
--- a/src/bundle.rs
+++ b/src/bundle.rs
@@ -6,6 +6,7 @@
 // copied, modified, or distributed except according to those terms.
 
 use crate::alloc::{vec, vec::Vec};
+use crate::entity_builder::Cloneable;
 use core::any::{type_name, TypeId};
 use core::ptr::NonNull;
 use core::{fmt, mem};
@@ -57,6 +58,17 @@ pub unsafe trait Bundle: DynamicBundle {
         Self: Sized;
 }
 
+/// A dynamically typed collection of cloneable components
+pub unsafe trait DynamicBundleClone: DynamicBundle {
+    /// Allow a callback to move all components out of the bundle
+    ///
+    /// Must invoke `f` only with a valid pointer and the pointee's type and size. `put` may only be
+    /// called at most once on any given value. Provides helper type for cloning
+    /// each type.
+    #[doc(hidden)]
+    unsafe fn put_with_clone(self, f: impl FnMut(*mut u8, TypeInfo, Cloneable));
+}
+
 /// Error indicating that an entity did not have a required component
 #[derive(Debug, Clone, Eq, PartialEq, Hash)]
 pub struct MissingComponent(&'static str);
@@ -106,6 +118,23 @@ macro_rules! tuple_impl {
             }
         }
 
+        unsafe impl<$($name: Component + Clone),*> DynamicBundleClone for ($($name,)*) {
+            // Compiler false positive warnings
+            #[allow(unused_variables, unused_mut)]
+            unsafe fn put_with_clone(self, mut f: impl FnMut(*mut u8, TypeInfo, Cloneable)) {
+                #[allow(non_snake_case)]
+                let ($(mut $name,)*) = self;
+                $(
+                    f(
+                        (&mut $name as *mut $name).cast::<u8>(),
+                        TypeInfo::of::<$name>(),
+                        Cloneable::new::<$name>()
+                    );
+                    mem::forget($name);
+                )*
+            }
+        }
+
         unsafe impl<$($name: Component),*> Bundle for ($($name,)*) {
             fn with_static_ids<T>(f: impl FnOnce(&[TypeId]) -> T) -> T {
                 const N: usize = count!($($name),*);
diff --git a/src/entity_builder.rs b/src/entity_builder.rs
index 480b910..7e1822a 100644
--- a/src/entity_builder.rs
+++ b/src/entity_builder.rs
@@ -7,6 +7,7 @@
 
 use crate::alloc::alloc::{alloc, dealloc, Layout};
 use crate::alloc::vec::Vec;
+use crate::bundle::DynamicBundleClone;
 use core::any::TypeId;
 use core::ptr::{self, NonNull};
 
@@ -173,6 +174,18 @@ impl EntityBuilderClone {
         self
     }
 
+    /// Add all components in `bundle` to the entity.
+    ///
+    /// If the bundle contains any component which matches the type of a component
+    /// already in the `EntityBuilder`, the newly added component from the bundle
+    /// will replace the old component and the old component will be dropped.
+    pub fn add_bundle(&mut self, bundle: impl DynamicBundleClone) -> &mut Self {
+        unsafe {
+            bundle.put_with_clone(|ptr, ty, cloneable| self.inner.add(ptr, ty, cloneable));
+        }
+        self
+    }
+
     /// Convert into a value whose shared references are [`DynamicBundle`]s suitable for repeated
     /// spawning
     pub fn build(self) -> BuiltEntityClone {
@@ -227,7 +240,7 @@ unsafe impl DynamicBundle for &'_ BuiltEntityClone {
     unsafe fn put(self, mut f: impl FnMut(*mut u8, TypeInfo)) {
         for &(_, offset, ref clone) in &self.0.info {
             let ptr = self.0.storage.as_ptr().add(offset);
-            (clone.0)(ptr, &mut f);
+            (clone.func)(ptr, &mut f);
         }
     }
 }
@@ -374,15 +387,20 @@ impl<M> Default for Common<M> {
 }
 
 #[derive(Clone)]
-struct Cloneable(unsafe fn(*const u8, &mut dyn FnMut(*mut u8, TypeInfo)));
+/// Struct providing dynamic clone via callback
+pub struct Cloneable {
+    func: unsafe fn(*const u8, &mut dyn FnMut(*mut u8, TypeInfo)),
+}
 
 impl Cloneable {
-    fn new<T: Component + Clone>() -> Self {
-        Self(|src, f| unsafe {
-            let mut tmp = (*src.cast::<T>()).clone();
-            f((&mut tmp as *mut T).cast(), TypeInfo::of::<T>());
-            core::mem::forget(tmp);
-        })
+    pub(crate) fn new<T: Component + Clone>() -> Self {
+        Self {
+            func: |src, f| unsafe {
+                let mut tmp = (*src.cast::<T>()).clone();
+                f((&mut tmp as *mut T).cast(), TypeInfo::of::<T>());
+                core::mem::forget(tmp);
+            },
+        }
     }
 }
 
@@ -398,7 +416,7 @@ impl Clone for Common<Cloneable> {
                 indices: self.indices.clone(),
             };
             for &(_, offset, ref clone) in &self.info {
-                (clone.0)(self.storage.as_ptr().add(offset), &mut |src, ty| {
+                (clone.func)(self.storage.as_ptr().add(offset), &mut |src, ty| {
                     result
                         .storage
                         .as_ptr()


Test patch:
diff --git a/tests/tests.rs b/tests/tests.rs
index b533b06..9dc7048 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -5,6 +5,8 @@
 // http://opensource.org/licenses/MIT>, at your option. This file may not be
 // copied, modified, or distributed except according to those terms.
 
+use std::borrow::Cow;
+
 use hecs::*;
 
 #[test]
@@ -200,6 +202,36 @@ fn build_entity() {
     assert_eq!(*world.get::<i32>(f).unwrap(), 789);
 }
 
+#[test]
+fn build_entity_clone() {
+    let mut world = World::new();
+    let mut entity = EntityBuilderClone::new();
+    entity.add("def");
+    entity.add([0u8; 1024]);
+    entity.add(456);
+    entity.add(789);
+    entity.add_bundle(("yup", 67_usize));
+    entity.add_bundle((5.0_f32, String::from("Foo")));
+    entity.add_bundle((7.0_f32, String::from("Bar"), 42_usize));
+    let entity = entity.build();
+    let e = world.spawn(&entity);
+    let f = world.spawn(&entity);
+    let g = world.spawn(&entity);
+    world
+        .insert_one(g, Cow::<'static, str>::from("after"))
+        .unwrap();
+
+    for e in [e, f, g] {
+        assert_eq!(*world.get::<&str>(e).unwrap(), "yup");
+        assert_eq!(*world.get::<i32>(e).unwrap(), 789);
+        assert_eq!(*world.get::<usize>(e).unwrap(), 42);
+        assert_eq!(*world.get::<f32>(e).unwrap(), 7.0);
+        assert_eq!(*world.get::<String>(e).unwrap(), "Bar");
+    }
+
+    assert_eq!(*world.get::<Cow<'static, str>>(g).unwrap(), "after");
+}
+
 #[test]
 fn access_builder_components() {
     let mut world = World::new();