Entt: Clone function breaks move-only components

[Kerndog73]

The clone function was added recently in c986a6c4dd91c8729670738e5eee7401c487c652 . The clone function prevents the sparse set from instantiating if the component doesn’t have a copy constructor.

This breaks previously working code that used move-only components.

[skypjack]

I'd add that, in EnTT, being move constructible is the only requirement for a component. Good point.
Unless we find a way to allow cloning an entity without breaking this requirement, I'll remove the functionality.
Any suggestion?

[Kerndog73]

Components should only need a copy constructor if they need to be cloned. If you can find some way of only instantiating the clone function when the component has a copy constructor then this problem will be solved.

[Kerndog73]

Prototype also requires that the components have a copy constructor. Components should only need a copy constructor if clone or prototype is used.

[skypjack]

Prototype is an external tool that doesn't affect the functionalities of the registry. If your components aren't copy constructible, it just fails to compile as from the documentation, so it's fine.
On the other side, you're right in saying that the registry must work also with components that aren't copy constructible. I'm not that sure there is a way to have the cake and eat it in this case, but I'll have a try to keep clone with us.

[skypjack]

See branch issue_90. With a trick that involves a template function we can do this:

• In case the component is copy-constructible, everything works just fine
• In case the component isn't copy-constructible:
  
  
  
  • An assertion terminates the execution in debug mode in case the user tries to clone it
  
  
  • Nothing happens in release mode at runtime, cloning is a no-op operation for such a component
  
  

This is the best we can do with a virtual method in the base class of the sparse set. The other solution around is to get rid of the clone functionality and that's all. Maybe the latter is the _best_ solution somehow, mainly because the requirements for the registry are that components must be move-constructible and not copy-constructible.

What about?

[Kerndog73]

Ideally, trying to clone a move-only component would be a compile-time error but there’s no easy way of achieving that. Your solution is the best we can do without losing the clone function.

[skypjack]

As long as clone is virtual, we cannot have the cake and eat it. The sole solution I see is to play with instantiation, but it introduces a potential unexpected result if users don't catch the error in debug. This is the best compromise we can reach.

The other way around is to get rid of the clone functionality and implement it externally if required.
Probably we should go along this path before to let users write potentially wrong code that uses clone.
I'm not that sure that keeping it on board is a good idea. After all, Prototype can be used more or less to do the same thing and it's an external tool. Moreover, it would trigger an error at compile-time.

[Kerndog73]

I agree. Since clone was only implemented 12 days ago and v2.6.0 was only released 4 days ago. I don’t think it’s too late to remove it completely. Prototype does more-or-less the same thing anyway.

This change might be a good candidate for v2.6.1. Maybe?

[skypjack]

Yep, definitely.

[skypjack]

v2.6.1 upstream, can you updated homebrew-entt? Is it only a matter of running the bash script you wrote now?

[Kerndog73]

Yep. Easy. The Homebrew formula is up to date!

[BBI-YggyKing]

I'd add that, in EnTT, being move constructible is the only requirement for a component.

@skypjack is this still the case? I looked for documentation on component type requirements, but I didn't find anything.

[skypjack]

@bbi-yggy yep, there is also a test for that. :+1:
Not sure if I ever mentioned it in the documentation actually. What section are you referring to? I can update the doc if you point it out. Thanks.

[skypjack]

@bbi-yggy nevermind, I've just checked the doc and it's there:

the sole requirement of a component type is that it must be both move constructible and move assignable.

Actually, being move constructible isn't enough, types must be also move assignable. It's kinda of expected for a move constructible type, but fortunately the doc goes a bit further on my comment in this thread. :wink:

[BBI-YggyKing]

Oh, awesome, thanks! I tried to search the documentation but I guess the search only finds page titles? Anyway, thanks for pointing that out.

I'm experimenting with integration of EnTT into Unreal. I attempted to attach an Unreal UActorComponent type as an EnTT component, unfortunately Unreal requires that actor components must not be move constructible, so I need to find an alternate way to specify components at design time.

[BBI-YggyKing]

(sorry for the double-comment - Github told me "you can't comment at this time" and left me in the editor, so I assumed it hadn't actually posted the comment, sigh)

[skypjack]

@bbi-yggy Oh, yeah, doxygen puts the wiki pages under the menu item _Related pages_. Not exactly intuitive, I agree.
Moreover I've no idea what's taken in consideration by the _search_ functionality. As you can guess, I don't use it very much. Nice to know that's broken, thanks for pointing that out!
I'll experiment a bit and eventually open an issue to doxygen. :+1:

Regarding EnTT and UE4, have you read the post blogs from @vblanco20-1 ? You can find them on his blog.
He experimented quite a lot with EnTT v2 and UE4 and he's still doing something with the v3 as far as I know.

To be honest, I'm definitely interested in working to provide a better and easy integration with UE4.
Feel free to ping me either here or privately for that. I think it would be interesting and could help quite a lot of users.
Count me in if you think that joining our forces to reach the goal is a good idea!

---

Side note: don't worry for the double-comment, I've deleted one of them. :wink:

[BBI-YggyKing]

I saw @vblanco20-1's space battle post, but hadn't found his blog yet, thanks for the link. I'll let you know how things go with Unreal - I have ideas that seem promising, but need to try them out first.

[skypjack]

@bbi-yggy It would be great. I'm really interested in such an integration. Ping me if you find that some changes or feature can make the integration easier and we can discuss them. :+1: