ComponentPool ISortable.Sort - Wrong if- / else-if conditions

[KaiHelli]

Hi Doraku,

I noticed a problem in our game where entities with components assigned with SetSameAs<T>() suddenly took the wrong component after running world.Optimize().

After some debugging, I think I found the cause of this. In the ComponentPool class there is a function ISortable.Sort which contains the following code in lines 334 - 353:

if (_links[_sortedIndex].ReferenceCount > 1 || tempLink.ReferenceCount > 1)
 {
     for (int i = 0; i < _mapping.Length; ++i)
     {
         if (_mapping[i] == minEntityId) // <--- Wrong condition.
         {
             _mapping[i] = _sortedIndex;
         }
         else if (_mapping[i] == tempLink.EntityId) // <--- Wrong condition.
         {
             _mapping[i] = minIndex;
         }
     }
 }
 else
 {
     _mapping[minEntityId] = _sortedIndex;
     _mapping[tempLink.EntityId] = minIndex;
 }

The else branch is correct, so sorting components without additional references is handled correctly. However, if there are multiple references, you are iterating over the entire mapping table. There you want to update the outdated references into the component array. The if and else-if clause for this is wrong. I think the corrected code should read:

if (_links[_sortedIndex].ReferenceCount > 1 || tempLink.ReferenceCount > 1)
 {
     for (int i = 0; i < _mapping.Length; ++i)
     {
         if (_mapping[i] == minIndex) // <--- The mapping table stores the index into the component array and not the entity id.
         {
             _mapping[i] = _sortedIndex;
         }
         else if (_mapping[i] == _sortedIndex) // <--- Same issue here.
         {
             _mapping[i] = minIndex;
         }
     }
 }
 else
 {
     _mapping[minEntityId] = _sortedIndex;
     _mapping[tempLink.EntityId] = minIndex;
 }

I hope you can have a look at it. Thanks a lot for your ECS system, we really enjoy working with it. Keep up the good work!

[Doraku]

Hello, thanks for reporting this. You are totally right it seems I completely mixed up the indices :/ I will add a test to show the problem and fix it as soon as I can!

[KaiHelli]

Perfect, that would be great. I wrote up a minimal example, so you are able to reproduce the issue.

World world = new World();

Entity entity1 = world.CreateEntity();
Entity entity2 = world.CreateEntity();
Entity entity3 = world.CreateEntity();

entity3.Set("Something else");
entity2.Set("Hi there!");
entity1.SetSameAs<string>(entity2);

Debug.Print(entity1 + ": " + entity1.Get<string>());
Debug.Print(entity2 + ": " + entity2.Get<string>());
Debug.Print(entity3 + ": " + entity3.Get<string>());

Debug.Print("--------");
world.Optimize();

Debug.Print(entity1 + ": " + entity1.Get<string>());
Debug.Print(entity2 + ": " + entity2.Get<string>());
Debug.Print(entity3 + ": " + entity3.Get<string>());

Which produces the output:

Entity 1:1.0: Hi there!
Entity 1:2.0: Hi there!
Entity 1:3.0: Something else
--------
Entity 1:1.0: Something else
Entity 1:2.0: Something else
Entity 1:3.0: Hi there!

[KaiHelli]

Thank you very much for fixing this issue!

Do you have any recommendation in getting this version into our project? Will there be a new NuGet release in the near future or should we include the source code as a separate project directly into our solution?

[Doraku]

I should create a new version soon, I just want to do some documentation update before to close a couple more issues (no promise but hopefully tonight or tomorrow).
In the meantime you could use the latest CI version from the github registry https://github.com/Doraku/DefaultEcs/packages/26448 if you prefer it instead of directly including the project :)

[KaiHelli]

Ah perfect, thanks for pointing out the CI version. I didn't knew that, that's great :)