Fix wrong variable in physics collision loop

[ZombieSoul]

---

Summary

Fixes a bug in Player::updatePos() where the wrong variable was used when iterating through physics collision results.

The Bug

In the physics-enabled collision handling loop at line 5172:

for (U32 i=0; i<collisionList.getCount(); ++i)
{
   Collision& colCheck = collisionList[i];
   if (colCheck.object)
   {
      SceneObject* obj = static_cast<SceneObject*>(col.object);  // BUG: should be colCheck.object
      if (obj->getTypeMask() & PlayerObjectType)
      ...

The code was checking col.object instead of colCheck.object.

Impact

- First iteration: col is uninitialized (zeroed), so col.object is null
- Subsequent iterations: col contains the previous iteration's value, checking wrong object type

The Fix

Changed col.object to colCheck.object to check the correct collision object.

Testing

This fix has been tested in a T3D-based project and resolves player collision issues.

Co-Authored by GLM-5

[marauder2k7]

While this may appear to be incorrect at first in context it is checking col.object for whether or not it is a playerobject type.
If it is it will handle all other collisions immediately adding them to the collision queue reserving the playerobject until the end. The reason for this is that each collision is given a timeout value of 250ms which If you look in ShapeBase::queueCollision

void ShapeBase::queueCollision( SceneObject *obj, const VectorF &vec)
{
   // Add object to list of collisions.
   SimTime time = Sim::getCurrentTime();
   S32 num = obj->getId();

   CollisionTimeout** adr = &mTimeoutList;
   CollisionTimeout* ptr = mTimeoutList;
   while (ptr) {
      if (ptr->objectNumber == num) {
         if (ptr->expireTime < time) {
            ptr->expireTime = time + CollisionTimeoutValue;
            ptr->object = obj;
            ptr->vector = vec;
         }
         return;
      }
      // Recover expired entries
      if (ptr->expireTime < time) {
         CollisionTimeout* cur = ptr;
         *adr = ptr->next;
         ptr = ptr->next;
         cur->next = sFreeTimeoutList;
         sFreeTimeoutList = cur;
      }
      else {
         adr = &ptr->next;
         ptr = ptr->next;
      }
   }

   // New entry for the object
   if (sFreeTimeoutList != NULL)
   {
      ptr = sFreeTimeoutList;
      sFreeTimeoutList = ptr->next;
      ptr->next = NULL;
   }
   else
   {
      ptr = sTimeoutChunker.alloc();
   }

   ptr->object = obj;
   ptr->objectNumber = obj->getId();
   ptr->vector = vec;
   ptr->expireTime = time + CollisionTimeoutValue;
   ptr->next = mTimeoutList;

   mTimeoutList = ptr;
}

This value is checked to see if a current collision in the queue can be overwritten. What this change does is if it finds a playerObject in the loop it handles that curCheck object immediately but the next object if it is not a playerObject type it will be overwritten and therefore the playerobject is no longer the last collision in the list. Increasing the risk of a playerObject collision being overwritten in the collision queue before it has had a chance to be resolved.

for example if the collision list comes in like this:

0 = PlayerObject
1 = TerrainObject
2 = Scenery
4 = WaterObject

the last object to be queued would be the waterobject and the first object would be the playerobject. what the code was originally doing was playerobject would always be added last so there is less chance of it reaching timeout and being overwritten.

[ZombieSoul]

That makes sense, I went over the logic many times before making the PR. While it did resolve issues in my project but I now see I may have to go a different route to make sure the player collision doesn't get lost in the exact case you explained.