Fixes an issue where a robot without an inventory was deleting the

drops. The core problem here actually is that OC agents try to
pull items directly out of killed/destroyed blocks via capturedDrops,
and the robots also pick up items from collision.
The collision code was written to always destroy the drops.
The code that tries to pull items out of capturedDrops spawns items in
world when there is no room in the robot.
Mob drops cause collision pickup AND run the capturedDrops code. When
collision doesn't take anything, and kills the drops, the items are lost.

The best fix would be to stop taking items directly from capturedDrops,
but this code has been here for some time, and we'll leave it for now.

The next best fix is to not always destroy drops on collision, but let
mc code handle collision cleanup as it was designed. Then, when taking
capturedDrops directly, first check if there is an inventory.

This solution leaves one last loop hole, collision doesn't find a place
in inventory and thus has valid world drops.

We can't ignore collision for attacks because the mobs are putting items
in the world. But instead, we need to stop capturedDrops action from
spawning in the world.

closes #3172

src/main/scala/li/cil/oc/server/agent/Player.scala

@@ -187,11 +187,11 @@ class Player(val agent: internal.Agent) extends FakePlayer(agent.world.asInstanc
   private def collectDroppedItems(itemsBefore: Iterable[EntityItem]) {
     val itemsAfter = adjacentItems
     val itemsDropped = itemsAfter -- itemsBefore
-    for (drop <- itemsDropped) {
-      
-      drop.delayBeforeCanPickup = 0
-      drop.onCollideWithPlayer(this)
-      drop.setDead()
+    if (itemsDropped.nonEmpty) {
+      for (drop <- itemsDropped) {
+        drop.delayBeforeCanPickup = 0
+        drop.onCollideWithPlayer(this)
+      }
     }
   }
 

src/main/scala/li/cil/oc/server/component/Agent.scala

@@ -288,11 +288,20 @@ trait Agent extends traits.WorldControl with traits.InventoryControl with traits
     entity.captureDrops = true
   }
 
+
   protected def endConsumeDrops(player: Player, entity: Entity) {
     entity.captureDrops = false
-    for (drop <- entity.capturedDrops) {
-      val stack = drop.getEntityItem
-      InventoryUtils.addToPlayerInventory(stack, player)
+    // this inventory size check is a HACK to preserve old behavior that a agent can suck items out
+    // of the capturedDrops. Ideally, we'd only pick up items off the ground. We could clear the
+    // capturedDrops when Player.attackTargetEntityWithCurrentItem() is called
+    // But this felt slightly less hacky, slightly
+    if (player.inventory.getSizeInventory > 0) {
+      for (drop <- entity.capturedDrops) {
+        if (!drop.isDead) {
+          val stack = drop.getEntityItem
+          InventoryUtils.addToPlayerInventory(stack, player, spawnInWorld = false)
+        }
+      }
     }
     entity.capturedDrops.clear()
   }

src/main/scala/li/cil/oc/util/InventoryUtils.scala

@@ -369,7 +369,7 @@ object InventoryUtils {
   /**
    * Try inserting an item stack into a player inventory. If that fails, drop it into the world.
    */
-  def addToPlayerInventory(stack: ItemStack, player: EntityPlayer): Unit = {
+  def addToPlayerInventory(stack: ItemStack, player: EntityPlayer, spawnInWorld: Boolean = true): Unit = {
     if (stack != null) {
       if (player.inventory.addItemStackToInventory(stack)) {
         player.inventory.markDirty()
@@ -377,7 +377,7 @@ object InventoryUtils {
           player.openContainer.detectAndSendChanges()
         }
       }
-      if (stack.stackSize > 0) {
+      if (stack.stackSize > 0 && spawnInWorld) {
         player.dropPlayerItemWithRandomChoice(stack, false)
       }
     }