Bug in ChangeEntityLocationRaw

The logic for pulling an entity out of its old entity block has a bug that I believe was the root cause of the multiple Sword entity bug found in Day066.

Towards the end of that session, the solution implemented by Casey was to guard against attempts to re-add entities that already have a reference index or some such thing.

The following statement in ChangeEntityLocationRaw will fail to remove an entity from its old entity block when the Hash location of the entity is the last entry of FirstBlock.

1
2
   Block->LowEntityIndex[Index] =
      FirstBlock->LowEntityIndex[--FirstBlock->EntityCount];


The remainder of ChangeEntityLocationRaw then goes on to successfully add the entity to its new block, resulting in the entity existing in more than one block.

Subsequent calls to BeginSim will encounter the same entity across different blocks and pull in each occurrence into SimRegion had Casey not implemented the guard against re-adding solution.

This situation can compound or "balloon" out to result the same entity being Hashed across many blocks.

I'm not following Casey's implementation verbatim, and in-fact not even in C/C++ so I don't have a C/C++ solution, but the following is what worked for me.

1
2
3
4
5
6
7
8
9
   if (Index === Block.LowEntityIndex.length - 1 && Block === FirstBlock) {
                                    
      FirstBlock.LowEntityIndex.pop();

   } else {
                                    
      Block.LowEntityIndex[Index] = FirstBlock.LowEntityIndex.pop();

   }
I haven't looked at this myself yet, but I'm not sure I follow you. The code

1
2
   Block->LowEntityIndex[Index] =
      FirstBlock->LowEntityIndex[--FirstBlock->EntityCount];


would work fine if it's the last entity, wouldn't it? Because if Index is already equal to (EntityCount - 1), which is the case to which you are referring, then no copy occurs (it just leaves the value where it was) and then the entity count is decremented, which means that entity is now "off the end of the array" and no longer counted.

Know what I mean?

So, again, haven't looked at this in detail, so you may be right that there's a bug in that line, but it still looks correct to me so I might need a bit more explanation as to what you think the bug is...

- Casey
Ah true, the use of EntityCount to indicate the end of the arrays content was lost on me.

I see what you mean, as long as the counter is respected in all dealings with the array, then this line isn't a problem.

Sorry to waste your time Casey. I'll refrain from suggesting bugs, especially since I'm interrupting the logic under a ported implementation.
Nah, ain't no thang! Better to be safe than sorry, I think. I'd much rather have someone report a non-bug than not report an actual bug :)

- Casey