Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.

Feedback Unity.Properties Visitors should visit Private Fields

Discussion in 'Entity Component System' started by Kleptine, Dec 4, 2022.

  1. Kleptine

    Kleptine

    Joined:
    Dec 23, 2013
    Posts:
    202
    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!
     
    Last edited: Dec 4, 2022
    PeppeJ2 and Jes28 like this.
  2. Kleptine

    Kleptine

    Joined:
    Dec 23, 2013
    Posts:
    202
    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!
     
  3. OndrejP

    OndrejP

    Joined:
    Jul 19, 2017
    Posts:
    241
    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
     
  4. Kleptine

    Kleptine

    Joined:
    Dec 23, 2013
    Posts:
    202
    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.