From 56f8fdbaa98bcfd64d048cd4ac5c1282ed2bed15 Mon Sep 17 00:00:00 2001
From: payonel <payonel@hotmail.com>
Date: Sat, 9 Nov 2019 22:46:31 -0800
Subject: [PATCH] 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
---
 .../scala/li/cil/oc/server/agent/Player.scala     | 10 +++++-----
 .../scala/li/cil/oc/server/component/Agent.scala  | 15 ++++++++++++---
 .../scala/li/cil/oc/util/InventoryUtils.scala     |  4 ++--
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/main/scala/li/cil/oc/server/agent/Player.scala b/src/main/scala/li/cil/oc/server/agent/Player.scala
index 100760bc0..796d50597 100644
--- a/src/main/scala/li/cil/oc/server/agent/Player.scala
+++ b/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)
+      }
     }
   }
 
diff --git a/src/main/scala/li/cil/oc/server/component/Agent.scala b/src/main/scala/li/cil/oc/server/component/Agent.scala
index 88be38d19..658fb27bd 100644
--- a/src/main/scala/li/cil/oc/server/component/Agent.scala
+++ b/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()
   }
diff --git a/src/main/scala/li/cil/oc/util/InventoryUtils.scala b/src/main/scala/li/cil/oc/util/InventoryUtils.scala
index 6089f221e..fd1c2d9b3 100644
--- a/src/main/scala/li/cil/oc/util/InventoryUtils.scala
+++ b/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)
       }
     }