Feedback Unity.Properties Visitors should visit Private Fields

When copying, instantiating, or serializing (baking) components, un-managed and managed components are treated differently, in a very non-intuitive way:
- Unmanaged components are copied byte-for-byte. This copies private field data!
- Managed components are copied via Unity.Properties. This only copies public field data!

I can see how this came about. It would be odd for a property reflection system to reflect private properties. Pretty much every other major property reflection system (ie. Java beans, Unity's SerializedProperty, etc) only acts on public fields. And this makes sense when your goal is to serialize things to Json, or store them for Scene loading.

But it's very weird for operations like Instantiate or CopyEntities. I expect both of these operations to faithfully copy entities 1-1. And largely, they do, when data is unmanaged. It's just the managed components that seem to be forgotten about.

I'd suggest the following change:
- Allow Unity.Properties and PropertyBag to reflect private fields.
- Visit private fields by default when doing copying operations (Instantiate, CopyEntities) and possibly SerializeWorld.

Additional justification: Unity's managed components deal with this issue by habitually requiring all fields in a managed component to be public. If there's already this unspoken requirement, why not just encode it in the engine? I could see this being a very frustrating paper-cut when folks are learning to use managed components.

Anyway, I've been enjoying using ECS, and this has just been something that's constantly nagging at me at the back of my head. I think it's worth fixing, given how well the unmanaged side of things is designed!

 

Wanted to bump this, as I think it's quite important. It's a blocker because we won't be able to replace/modify Unity.Properties when it moves into the engine!

 

I've read somewhere you can implement IClonable to provide custom implementation for instantiating managed components. This seems reasonable.

I'm surprised there's some "default" implementation which copies public fields, that seems quite "random" and I'd expect it to fail for a lot of cases where managed components are used.

There's the link for documentation about ICloneable and IDisposable
https://docs.unity3d.com/Packages/com.unity.entities@1.0/manual/components-managed-create.html

 

Good point. Yeah, at least when copying entities, ICloneable seems to work. Assuming the components actually implement it.

Yeah, it's still a very weird default choice.