remove Clone requirement on Types that Drop. Panics when you duplicat…

…e components and do not implement Clone

flecs_ecs/src/core/c_types.rs

@@ -369,6 +369,8 @@ impl NotEmptyComponent for EcsComponent {}
 impl ComponentInfo for EcsComponent {
     const IS_ENUM: bool = false;
     const IS_TAG: bool = false;
+    const IMPLS_CLONE: bool = true;
+    const IMPLS_DEFAULT: bool = true;
 }
 
 impl ComponentType<Struct> for EcsComponent {}
@@ -413,6 +415,8 @@ impl ComponentId for EcsComponent {
 impl ComponentInfo for Poly {
     const IS_ENUM: bool = false;
     const IS_TAG: bool = false;
+    const IMPLS_CLONE: bool = true;
+    const IMPLS_DEFAULT: bool = true;
 }
 
 impl NotEmptyComponent for Poly {}
@@ -460,6 +464,8 @@ impl ComponentId for Poly {
 impl ComponentInfo for TickSource {
     const IS_TAG: bool = false;
     const IS_ENUM: bool = false;
+    const IMPLS_CLONE: bool = true;
+    const IMPLS_DEFAULT: bool = true;
 }
 
 #[cfg(feature = "flecs_system")]
@@ -503,6 +509,8 @@ impl ComponentId for TickSource {
 impl ComponentInfo for EntityId {
     const IS_ENUM: bool = false;
     const IS_TAG: bool = false;
+    const IMPLS_CLONE: bool = true;
+    const IMPLS_DEFAULT: bool = false;
 }
 
 impl ComponentId for EntityId {

flecs_ecs/src/core/component_registration/helpers.rs

@@ -27,16 +27,14 @@ where
     } else {
         0
     };
-
-    let hooks = if size != 0 && T::NEEDS_DROP {
+    let mut hooks = Default::default();
+    if size != 0 && T::NEEDS_DROP {
         // Register lifecycle callbacks, but only if the component has a
         // size and requires initialization of heap memory / needs drop.
         // Components that don't have a size are tags, and tags don't
         // require construction/destruction/copy/move's.
-        T::__register_lifecycle_hooks()
-    } else {
-        Default::default()
-    };
+        T::__register_lifecycle_hooks(&mut hooks);
+    }
 
     let type_info: flecs_ecs_sys::ecs_type_info_t = flecs_ecs_sys::ecs_type_info_t {
         size: size as i32,

flecs_ecs/src/core/component_registration/mod.rs

@@ -1,9 +1,9 @@
 mod helpers;
 mod registration;
 pub mod registration_traits;
-pub mod types;
+pub mod registration_types;
 
 pub(crate) use helpers::*;
 pub use registration::*;
 pub use registration_traits::*;
-pub use types::*;
+pub use registration_types::*;

flecs_ecs/src/core/component_registration/registration_traits.rs

@@ -1,8 +1,8 @@
 use std::{ffi::CStr, sync::OnceLock};
 
 use crate::core::{
-    ConditionalTypeSelector, DefaultCloneDummy, Entity, EntityT, Enum, IdComponent, IdT, IntoWorld,
-    Struct, TypeHooksT,
+    ConditionalTypeSelector, Entity, EntityT, Enum, FlecsNoneCloneDummy, FlecsNoneDefaultDummy,
+    IdComponent, IdT, IntoWorld, Struct, TypeHooksT,
 };
 
 use super::{
@@ -108,15 +108,15 @@ pub trait ComponentId: Sized + ComponentInfo {
 
     // Not public API.
     #[doc(hidden)]
-    fn __register_lifecycle_hooks() -> TypeHooksT {
-        Default::default()
-    }
+    fn __register_lifecycle_hooks(mut _type_hooks: &mut TypeHooksT) {}
 }
 
 pub trait ComponentInfo: Sized {
     const IS_ENUM: bool;
     const IS_TAG: bool;
     const NEEDS_DROP: bool = std::mem::needs_drop::<Self>();
+    const IMPLS_CLONE: bool;
+    const IMPLS_DEFAULT: bool;
 }
 
 pub trait CachedEnumData: ComponentType<Enum> + ComponentId {
@@ -195,11 +195,15 @@ pub trait CachedEnumData: ComponentType<Enum> + ComponentId {
 impl<T: ComponentInfo> ComponentInfo for &T {
     const IS_ENUM: bool = T::IS_ENUM;
     const IS_TAG: bool = T::IS_TAG;
+    const IMPLS_CLONE: bool = T::IMPLS_CLONE;
+    const IMPLS_DEFAULT: bool = T::IMPLS_DEFAULT;
 }
 
 impl<T: ComponentInfo> ComponentInfo for &mut T {
     const IS_ENUM: bool = T::IS_ENUM;
     const IS_TAG: bool = T::IS_TAG;
+    const IMPLS_CLONE: bool = T::IMPLS_CLONE;
+    const IMPLS_DEFAULT: bool = T::IMPLS_DEFAULT;
 }
 
 impl<T: ComponentId> ComponentId for &T {
@@ -222,17 +226,32 @@ impl<T: ComponentId> ComponentId for &mut T {
     type UnderlyingEnumType = T::UnderlyingEnumType;
 }
 
-pub trait DefaultCloneType {
-    type Type: Default + Clone;
+pub trait FlecsDefaultType {
+    type Type: Default;
+}
+
+pub trait FlecsCloneType {
+    type Type: Clone;
+}
+
+impl<T> FlecsDefaultType for ConditionalTypeSelector<false, T> {
+    type Type = FlecsNoneDefaultDummy;
+}
+
+impl<T> FlecsDefaultType for ConditionalTypeSelector<true, T>
+where
+    T: Default,
+{
+    type Type = T;
 }
 
-impl<T> DefaultCloneType for ConditionalTypeSelector<false, T> {
-    type Type = DefaultCloneDummy;
+impl<T> FlecsCloneType for ConditionalTypeSelector<false, T> {
+    type Type = FlecsNoneCloneDummy;
 }
 
-impl<T> DefaultCloneType for ConditionalTypeSelector<true, T>
+impl<T> FlecsCloneType for ConditionalTypeSelector<true, T>
 where
-    T: Default + Clone,
+    T: Clone,
 {
     type Type = T;
 }

flecs_ecs/src/core/component_registration/types.rs → flecs_ecs/src/core/component_registration/registration_types.rs

@@ -13,11 +13,14 @@ pub struct Struct;
 
 #[derive(Component)]
 pub enum NoneEnum {
-    None,
+    None = 1,
 }
 
 #[derive(Default, Clone)]
-pub struct DefaultCloneDummy;
+pub struct FlecsNoneDefaultDummy;
+
+#[derive(Clone)]
+pub struct FlecsNoneCloneDummy;
 
 pub struct ConditionalTypeSelector<const B: bool, T> {
     phantom: std::marker::PhantomData<T>,

flecs_ecs/src/core/flecs.rs

@@ -17,6 +17,8 @@ macro_rules! create_pre_registered_component {
         impl ComponentInfo for $struct_name {
             const IS_ENUM: bool = false;
             const IS_TAG: bool = true;
+            const IMPLS_CLONE: bool = false;
+            const IMPLS_DEFAULT: bool = false;
         }
 
         impl EmptyComponent for $struct_name {}

flecs_ecs/src/core/lifecycle_traits.rs

@@ -50,24 +50,23 @@ use crate::{core::c_types::TypeHooksT, ecs_assert, sys::ecs_type_info_t};
 use std::{ffi::c_void, mem::MaybeUninit, ptr};
 
 #[allow(clippy::not_unsafe_ptr_arg_deref)]
-pub fn create_lifecycle_actions<T: Clone + Default>() -> TypeHooksT {
-    TypeHooksT {
-        ctor: Some(generic_ctor::<T>),
-        dtor: Some(generic_dtor::<T>),
-        copy: Some(generic_copy::<T>),
-        move_: Some(generic_move::<T>),
-        copy_ctor: Some(generic_copy::<T>), //same implementation as copy
-        move_ctor: Some(generic_move::<T>), //same implementation as move
-        ctor_move_dtor: Some(generic_ctor_move_dtor::<T>),
-        move_dtor: Some(generic_ctor_move_dtor::<T>), //same implementation as ctor_move_dtor
-        on_add: None,
-        on_set: None,
-        on_remove: None,
-        ctx: std::ptr::null_mut(),
-        binding_ctx: std::ptr::null_mut(),
-        ctx_free: None,
-        binding_ctx_free: None,
-    }
+pub fn register_lifecycle_actions<T: Default>(type_hooks: &mut TypeHooksT) {
+    type_hooks.ctor = Some(generic_ctor::<T>);
+    type_hooks.dtor = Some(generic_dtor::<T>);
+    type_hooks.move_ = Some(generic_move::<T>);
+    type_hooks.move_ctor = Some(generic_move::<T>); //same implementation as move
+    type_hooks.ctor_move_dtor = Some(generic_ctor_move_dtor::<T>);
+    type_hooks.move_dtor = Some(generic_ctor_move_dtor::<T>); //same implementation as ctor_move_dtor
+}
+
+pub fn register_copy_lifecycle_action<T: Clone>(type_hooks: &mut TypeHooksT) {
+    type_hooks.copy = Some(generic_copy::<T>);
+    type_hooks.copy_ctor = Some(generic_copy::<T>); //same implementation as copy
+}
+
+pub fn register_copy_lifecycle_panic_action<T>(type_hooks: &mut TypeHooksT) {
+    type_hooks.copy = Some(generic_copy_panic::<T>);
+    type_hooks.copy_ctor = Some(generic_copy_panic::<T>); //same implementation as copy
 }
 
 /// This is the generic constructor for trivial types
@@ -133,7 +132,7 @@ extern "C" fn generic_dtor<T>(ptr: *mut c_void, count: i32, _type_info: *const e
 ///
 /// * C++ API: `copy_impl`
 #[doc(alias = "copy_impl")]
-extern "C" fn generic_copy<T: Default + Clone>(
+extern "C" fn generic_copy<T: Clone>(
     dst_ptr: *mut c_void,
     src_ptr: *const c_void,
     count: i32,
@@ -155,6 +154,22 @@ extern "C" fn generic_copy<T: Default + Clone>(
     }
 }
 
+/// This is the generic copy for trivial types
+/// It will copy the memory
+///
+/// # See also
+///
+/// * C++ API: `copy_impl`
+#[doc(alias = "copy_impl")]
+extern "C" fn generic_copy_panic<T>(
+    _dst_ptr: *mut c_void,
+    _src_ptr: *const c_void,
+    _count: i32,
+    _type_info: *const ecs_type_info_t,
+) {
+    panic!("Clone is not implemented for type {} and it's being used in a copy / duplicate operation such as component overriding or duplicating entities / components", std::any::type_name::<T>());
+}
+
 /// This is the generic move for non-trivial types
 /// It will move the memory
 ///
@@ -194,7 +209,7 @@ extern "C" fn generic_move<T: Default>(
 ///
 /// * C++ API: `move_ctor_impl`
 #[doc(alias = "move_ctor_impl")]
-extern "C" fn generic_ctor_move_dtor<T: Default + Clone>(
+extern "C" fn generic_ctor_move_dtor<T: Default>(
     dst_ptr: *mut c_void,
     src_ptr: *mut c_void,
     count: i32,

flecs_ecs/src/core/utility/mod.rs

@@ -2,7 +2,7 @@ mod errors;
 mod functions;
 mod log;
 pub mod traits;
-mod types;
+pub mod types;
 
 pub use errors::*;
 pub use functions::*;

flecs_ecs/src/core/utility/traits/mod.rs

@@ -14,6 +14,8 @@ pub use into_world::*;
 pub use iter::*;
 pub use reactor::*;
 
+use super::{ImplementsClone, ImplementsDefault};
+
 #[doc(hidden)]
 pub mod private {
     use std::{ffi::c_void, ptr};
@@ -313,3 +315,17 @@ pub mod private {
         }
     }
 }
+
+pub trait DoesNotImpl {
+    const IMPLS: bool = false;
+}
+
+impl<T> DoesNotImpl for T {}
+
+impl<T: Clone> ImplementsClone<T> {
+    pub const IMPLS: bool = true;
+}
+
+impl<T: Default> ImplementsDefault<T> {
+    pub const IMPLS: bool = true;
+}

flecs_ecs/src/core/utility/types.rs

@@ -4,7 +4,7 @@ use crate::core::{Entity, IdT, World};
 
 pub type FTime = f32;
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Default)]
 pub struct EntityId(pub IdT);
 
 impl EntityId {
@@ -223,3 +223,6 @@ impl ObserverEntityBindingCtx {
         }
     }
 }
+
+pub struct ImplementsClone<T>(std::marker::PhantomData<T>);
+pub struct ImplementsDefault<T>(std::marker::PhantomData<T>);

flecs_ecs/tests/clone_default_impl.rs

@@ -0,0 +1,69 @@
+use flecs_ecs::core::{ComponentInfo, World};
+use flecs_ecs_derive::Component;
+
+// normal structs
+#[derive(Component)]
+struct NoneCloneDefault;
+
+#[derive(Default, Clone, Component)]
+struct CloneDefault;
+
+#[derive(Clone, Component)]
+struct CloneNoDefault;
+
+#[derive(Default, Component)]
+struct DefaultNoClone;
+
+// drop structs
+#[derive(Component, Default)]
+struct DefaultNoCloneDrop {
+    _data: String,
+}
+
+#[derive(Default, Clone, Component)]
+struct CloneDefaultDrop {
+    data: String,
+}
+
+#[test]
+fn compile_time_check_impls_clone_default() {
+    // we do it this way to avoid the warning of constant bools getting optimized away from clippy in test cases.
+    let none_clone_default = !NoneCloneDefault::IMPLS_CLONE && !NoneCloneDefault::IMPLS_DEFAULT;
+    let clone_default = CloneDefault::IMPLS_CLONE && CloneDefault::IMPLS_DEFAULT;
+    let clone_no_default = CloneNoDefault::IMPLS_CLONE && !CloneNoDefault::IMPLS_DEFAULT;
+    let default_no_clone = !DefaultNoClone::IMPLS_CLONE && DefaultNoClone::IMPLS_DEFAULT;
+
+    assert!(none_clone_default);
+    assert!(clone_default);
+    assert!(clone_no_default);
+    assert!(default_no_clone);
+}
+
+#[test]
+fn copy_hook_implemented_for_drop_types() {
+    let world = World::new();
+    let e_orig = world.new_entity().set(CloneDefaultDrop {
+        data: "data".to_string(),
+    });
+
+    let entity_cloned = e_orig.duplicate(true);
+    let data_orig = &e_orig.get::<CloneDefaultDrop>().unwrap().data;
+    let data_cloned = &entity_cloned.get::<CloneDefaultDrop>().unwrap().data;
+
+    assert!(*data_orig == *data_cloned);
+}
+
+#[test]
+#[should_panic(
+    expected = "DefaultNoClone does not implement Clone and with a duplicate operation it will panic"
+)]
+#[ignore = "C asserts that world didn't properly end deferring and aborts 
+the test & thus the test not registering the panic and does not get marked as passed"]
+fn copy_hook_not_implemented_for_drop_types() {
+    let world = World::new();
+    let e_orig = world.new_entity().set(DefaultNoCloneDrop {
+        _data: "data".to_string(),
+    });
+
+    let _entity_cloned = e_orig.duplicate(true); // PANICS
+}