Issue #49. Falling inventory may fall out of world and be displayed at wrong location client-side

[Masterkent]

When a player takes a copy of a respawnable inventory, the respawnable inventory changes its state from Pickup to Sleeping. Leaving the state Pickup invokes the function Inventory.Pickup.EndState:

Code: Select all

      function EndState()
      {
            bCollideWorld = false;
            bSleepTouch = false;
      }

This implementation disables collision with the world geometry regardless of whether the inventory is falling or not. If the inventory is falling (e.g. when it's spawned by a destroyed decoration), it may fall out of world and be destroyed by the call to Actor.FellOutOfWorld(). I think, this code should be changed to

Code: Select all

      function EndState()
      {
            if (Physics != PHYS_Falling)
                  bCollideWorld = false;
            bSleepTouch = false;
      }

so touching a respawnable inventory before it reached the ground would not lead to its destruction. This would also affect some UPakHeath actors which are supposed to be respawnable (even in single-player game) but sometimes may fall out of world due to Physics == PHYS_Falling being always true.

When a weapon is dropped by a player, Inventory.DropFrom is called:

Code: Select all

function DropFrom(vector StartLocation)
{
      if ( !SetLocation(StartLocation) )
            return;
      RespawnTime = 0.0; //don't respawn
      SetPhysics(PHYS_Falling);
      RemoteRole = ROLE_DumbProxy;
      BecomePickup();
      NetPriority = 6;
      bCollideWorld = true;
      if ( Pawn(Owner) != None )
            Pawn(Owner).DeleteInventory(self);
      GotoState('PickUp', 'Dropped');
}

Setting RemoteRole to ROLE_DumbProxy enables replication of Location from server to clients, so the weapon's Location can be properly updated client-side. Then, when Timer() is invoked, it changes RemoteRole from ROLE_DumbProxy to ROLE_SimulatedProxy:

Code: Select all

      function Timer()
      {
            if ( RemoteRole != ROLE_SimulatedProxy )
            {
                  NetPriority = 2;
                  RemoteRole = ROLE_SimulatedProxy;
                  if ( bHeldItem )
                        SetTimer(40.0, false);
                  return;
            }

            if ( bHeldItem )
                  Destroy();
      }

and then replication of Location does not take place anymore. Normally, for a dropped weapon, the timer should be activated by the call to Landed() when the weapon landed on the ground:

Code: Select all

      function Landed(Vector HitNormal)
      {
            local rotator newRot;
            newRot = Rotation;
            newRot.pitch = 0;
            SetRotation(newRot);
            SetTimer(2.0, false);
      }

However, some weapons (e.g. Eightball) may activate the timer before they were dropped and for different purposes, then the call to Timer may happen before the weapon reaches the ground, that leads to missing updates of actual Location client-side (the weapon is observed as hanging somewhere in air).

Theoretically, such a situation may also occur when the weapon has bHeldItem == true and it falls during more than 45 seconds. This is how I would fix the current implementation:

Code: Select all

      function Timer()
      {
            if ( RemoteRole != ROLE_SimulatedProxy )
            {
                  NetPriority = 2;
                  RemoteRole = ROLE_SimulatedProxy;
+                  if (Physics == PHYS_Falling)
+                        bSimulatedPawnRep = true;
                  if ( bHeldItem )
                        SetTimer(40.0, false);
                  return;
            }

            if ( bHeldItem )
                  Destroy();
      }

      function BeginState()
      {
            BecomePickup();
            bCollideWorld = true;
            if ( bHeldItem )
                  SetTimer(45, false);
+            else
+                  SetTimer(0, false);
      }

Although adding

Code: Select all

                  if (Physics == PHYS_Falling)
                        bSimulatedPawnRep = true;

alone would solve the problem with replication, I think that for a non-held inventory the timer should be stopped anyway. For example, Eightball.Idle.Timer obviously serves a different purpose than controlling RemoteRole or interval before automatic self-destruction, and the current influence of the Eightball-specific use of the timer on such things takes place due to a coincidence that was overlooked.

Engine/Classes/Inventory.uc

[Smirftsch]

implemented as suggested. Thanks