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.
  2. Unite 2023 Registrations are now LIVE! We are thrilled to announce that Unite 2023 Registration is now live and open for all!
    Dismiss Notice
New Forum User Notice Unite 2023 Registrations are now LIVE!

Bug Add/RemoveComponent with ComponentTypes Behaviour Missmatch (Enities 0.14.0-p18)

Discussion in 'Entity Component System' started by Lieene-Guo, Aug 26, 2020.

  1. Lieene-Guo

    Lieene-Guo

    Joined:
    Aug 20, 2013
    Posts:
    547
    Untitled.png

    Here's the code from EntityManager

    Code (CSharp):
    1.         [StructuralChangeMethod]
    2.         public void AddComponent(EntityQuery entityQuery, ComponentTypes componentTypes)
    3.         {
    4.             var access = GetCheckedEntityDataAccess();
    5.             var ecs = access->EntityComponentStore;
    6.             var queryImpl = entityQuery._GetImpl();
    7.  
    8.             Unity.Entities.EntityComponentStore
    9.             .AssertValidEntityQuery(entityQuery, ecs);
    10.             if (queryImpl->IsEmptyIgnoreFilter)
    11.                 return;
    12.  
    13.             access->AddComponents(
    14.                 queryImpl->_QueryData->MatchingArchetypes, queryImpl->_Filter,
    15.                 componentTypes);
    16.         }
    Code (CSharp):
    1.         [StructuralChangeMethod]
    2.         public void RemoveComponent(EntityQuery entityQuery, ComponentTypes types)
    3.         {
    4.             var access = GetCheckedEntityDataAccess();
    5.             var ecs = access->EntityComponentStore;
    6.  
    7.             Unity.Entities.EntityComponentStore.AssertValidEntityQuery(entityQuery, ecs);
    8.  
    9.             if (entityQuery.IsEmptyIgnoreFilter)
    10.                 return;
    11.  
    12.             if (entityQuery.CalculateEntityCount() == 0)
    13.                 return;
    14.  
    15.  
    16.             // @TODO: Opportunity to do all components in batch on a per chunk basis.
    17.             for (int i = 0; i != types.Length; i++)
    18.                 RemoveComponent(entityQuery, types.GetComponentType(i));
    19.         }
    This part in RemoveComponent:
     for (int i = 0; i != types.Length; i++)  RemoveComponent(entityQuery, types.GetComponentType(i));

    Would only remove the first Component, if the component types to remove are included in the query.
    After the first remove, the query will no longer match.
    But the new function AddComponent solved that problem as it iterates though chunks only once.
    This mismatching behavior is misleading.
     
    Last edited: Aug 26, 2020
    Lieene-Guo, Aug 26, 2020
    #1

  2. Brian_Will

    Brian_Will

    Unity Technologies

    Joined:
    Apr 4, 2020
    Posts:
    12
    Good catch! If I had noticed the issue, I would have noted this change in the changelog.

    While it's possible some users might intentionally or accidentally depend upon the original behavior, we consider that behavior a bug because it's far too surprising and probably not commonly useful.

    In the off chance users still want the old, odd behavior, they can just write the same loop:

    Code (CSharp):
    1. for (int i = 0; i != types.Length; i++)
    2.                 RemoveComponent(entityQuery, types.GetComponentType(i));
    (There's no actual need for all the preamble before it because the RemoveComponent calls in the loop do all those checks. The original loop code was really just a placeholder.)
     
    Brian_Will, Aug 26, 2020
    #2