Search Unity

Feedback This should be changed: EntityManager.Instantiate(NativeArray<Entity> src, NativeArray<Entity> out)

Discussion in 'Entity Component System' started by msfredb7, Jun 1, 2023.

  1. msfredb7

    msfredb7

    Joined:
    Nov 1, 2012
    Posts:
    163
    EntityManager has this method:
    Instantiate(NativeArray<Entity> srcEntities, NativeArray<Entity> outputEntities)

    Its signature suggests that it behaves identically to Entity Instantiate(entity), but operates on an array of entities (just like DestroyEntity(Entity) and DestroyEntity(NativeArray<Entity>)).

    I think the vast majority of people would assume that, in the following scenario, A and B are logically identical.
    Code (CSharp):
    1. NativeArray<Entity> srcEntities = ...;
    2. NativeArray<Entity> dstEntities = new(srcEntities.Length, Allocator);
    3.  
    4. // A
    5. for (int i = 0; i < srcEntities.Length; i++)
    6. {
    7.     dstEntities[i]= EntityManager.Instantiate(srcEntities[i]);
    8. }
    9.  
    10. // B
    11. EntityManager.Instantiate(srcEntities, dstEntities);
    However, this is not the case. This overload of Instantiate is the only one that does not clone the children entities (LinkedEntityGroup). This is very error prone and has already caused 2 bugs on our project.

    I suggest you rename the method, change its behavior, or add InstantiateFlags so this behavior is explicit.
     
    Last edited: Jun 3, 2023
    msfredb7, Jun 1, 2023
    #1
    Enzi, WAYNGames and vectorized-runner like this.

  2. cort_of_unity

    cort_of_unity

    Unity Technologies

    Joined:
    Aug 15, 2018
    Posts:
    98
    Thank you for bringing this to our attention.
    The behavior of these Instantiate methods re: LinkedEntityGroup is explicitly covered in their API documentation, so it is at least the intended behavior. That doesn't make it any less surprising and error-prone, though. We'll discuss internally and figure out what change would make the most sense.
     
    cort_of_unity, Jun 2, 2023
    #2
    msfredb7 likes this.

  3. TheOtherMonarch

    TheOtherMonarch

    Joined:
    Jul 28, 2012
    Posts:
    866
    Maybe a new name is in order.
     
    TheOtherMonarch, Jun 3, 2023
    #3

  4. cort_of_unity

    cort_of_unity

    Unity Technologies

    Joined:
    Aug 15, 2018
    Posts:
    98
    A new name is the most likely outcome, yes. My main question is whether this method has a real-world use case, given the clearly intentional semantics of ignoring the LinkedEntityGroup buffer on the input entities. Is anybody currently using it to great effect?
    If all you want is to instantiate N unrelated entities and get an array of their "roots", we're probably not going to be able to do much better than the code from scenario A above, which can trivially be implemented as an extension method on the public API if necessary.
     
    cort_of_unity, Jun 5, 2023
    #4

  5. msfredb7

    msfredb7

    Joined:
    Nov 1, 2012
    Posts:
    163
    On our project, we do not use this "skip LinkedEntityGroup" behavior. So we usually code scenario A.

    The main benefit I see from providing a Instantiate(NativeArray src, NativeArray dst) inside the Entities package is that it opens the door for potential optimizations later on, without people needing to update their code.
     
    msfredb7, Jun 5, 2023
    #5