Fix push/pullItems losing items from furnace output slot

This was happening because the output slot of a furnace is protected
from all insertions. We were greedily extracting items at the beginning
of the transfer and couldn't place the remainder back into the protected
slot. Instead, simulate the transaction first, count how many items
successfully transfer, then actually mutate the inventories by that
amount. The ItemStorage wrappers already supported simulating
transactions, so I just exposed the option in InventoryUtil.

Also, because it's likely there's still some edge cases where things
will go wrong (especially in the presence of modded blocks), I added
some detailed logging in the case that we detect that items have been
lost.

Fixes cc-tweaked#122

src/main/java/dan200/computercraft/shared/peripheral/generic/methods/InventoryMethods.java

@@ -302,39 +302,30 @@ else if( object instanceof Container container )
     private static int moveItem( ItemStorage from, int fromSlot, ItemStorage to, int toSlot, final int limit )
     {
         // Moving nothing is easy
-        if( limit == 0 )
-        {
-            return 0;
-        }
+        if( limit == 0 ) return 0;
 
-        // Get stack to move
-        ItemStack stack = InventoryUtil.takeItems( limit, from, fromSlot, 1, fromSlot );
-        if( stack.isEmpty() )
-        {
-            return 0;
-        }
-        int stackCount = stack.getCount();
+        // Simulate transaction
+        ItemStack simulatedStack = InventoryUtil.takeItems( limit, from, fromSlot, 1, fromSlot, true );
+        ItemStack simulatedRemainder = toSlot < 0
+            ? InventoryUtil.storeItems( simulatedStack, to, true )
+            : InventoryUtil.storeItems( simulatedStack, to, toSlot, 1, toSlot, true );
 
-        // Move items in
-        ItemStack remainder;
-        if( toSlot < 0 )
-        {
-            remainder = InventoryUtil.storeItems( stack, to );
-        }
-        else
-        {
-            remainder = InventoryUtil.storeItems( stack, to, toSlot, 1, toSlot );
-        }
+        // Count how many items were successfully transferred
+        int transferCount = simulatedStack.getCount() - simulatedRemainder.getCount();
 
-        // Calculate items moved
-        int count = stackCount - remainder.getCount();
+        // Execute the transaction
+        ItemStack stack = InventoryUtil.takeItems( transferCount, from, fromSlot, 1, fromSlot, false );
+        ItemStack remainder = toSlot < 0
+            ? InventoryUtil.storeItems( stack, to, false )
+            : InventoryUtil.storeItems( stack, to, toSlot, 1, toSlot, false );
 
         if( !remainder.isEmpty() )
         {
-            // Put the remainder back
-            InventoryUtil.storeItems( remainder, from, fromSlot, 1, fromSlot );
+            ComputerCraft.log.error( "Items were lost during a generic inventory transfer!" );
+            ComputerCraft.log.error( String.format( "from=%s fromSlot=%d to=%s toSlot=%d limit=%d", from, fromSlot, to, toSlot, limit ) );
+            ComputerCraft.log.error( "Please report this at https://github.com/cc-tweaked/cc-restitched/issues" );
         }
 
-        return count;
+        return transferCount;
     }
 }

src/main/java/dan200/computercraft/shared/pocket/apis/PocketAPI.java

@@ -82,7 +82,7 @@ public final Object[] equipBack()
             ItemStack stack = previousUpgrade.getCraftingItem();
             if( !stack.isEmpty() )
             {
-                stack = InventoryUtil.storeItems( stack, ItemStorage.wrap( inventory ), inventory.selected );
+                stack = InventoryUtil.storeItems( stack, ItemStorage.wrap( inventory ), inventory.selected, false );
                 if( !stack.isEmpty() )
                 {
                     WorldUtil.dropItemStack( stack, player.getCommandSenderWorld(), player.position() );
@@ -118,7 +118,7 @@ public final Object[] unequipBack()
         ItemStack stack = previousUpgrade.getCraftingItem();
         if( !stack.isEmpty() )
         {
-            stack = InventoryUtil.storeItems( stack, ItemStorage.wrap( inventory ), inventory.selected );
+            stack = InventoryUtil.storeItems( stack, ItemStorage.wrap( inventory ), inventory.selected, false );
             if( stack.isEmpty() )
             {
                 WorldUtil.dropItemStack( stack, player.getCommandSenderWorld(), player.position() );

src/main/java/dan200/computercraft/shared/turtle/FurnaceRefuelHandler.java

@@ -38,7 +38,7 @@ public int refuel( @Nonnull ITurtleAccess turtle, @Nonnull ItemStack currentStac
         Item replacementStack = stack.getItem().getCraftingRemainingItem();
         if( replacementStack != null )
         {
-            ItemStack remainder = InventoryUtil.storeItems( new ItemStack( replacementStack ), turtle.getItemHandler(), turtle.getSelectedSlot() );
+            ItemStack remainder = InventoryUtil.storeItems( new ItemStack( replacementStack ), turtle.getItemHandler(), turtle.getSelectedSlot(), false );
             if( !remainder.isEmpty() )
             {
                 WorldUtil.dropItemStack( remainder, turtle.getLevel(), turtle.getPosition(), turtle.getDirection().getOpposite() );

src/main/java/dan200/computercraft/shared/turtle/core/TurtleCraftCommand.java

@@ -38,7 +38,7 @@ public TurtleCommandResult execute( @Nonnull ITurtleAccess turtle )
         // Store or drop any remainders
         for( ItemStack stack : results )
         {
-            ItemStack remainder = InventoryUtil.storeItems( stack, turtle.getItemHandler(), turtle.getSelectedSlot() );
+            ItemStack remainder = InventoryUtil.storeItems( stack, turtle.getItemHandler(), turtle.getSelectedSlot(), false );
             if( !remainder.isEmpty() )
             {
                 WorldUtil.dropItemStack( remainder, turtle.getLevel(), turtle.getPosition(), turtle.getDirection() );

src/main/java/dan200/computercraft/shared/turtle/core/TurtleDropCommand.java

@@ -46,7 +46,7 @@ public TurtleCommandResult execute( @Nonnull ITurtleAccess turtle )
         Direction direction = this.direction.toWorldDir( turtle );
 
         // Get things to drop
-        ItemStack stack = InventoryUtil.takeItems( quantity, turtle.getItemHandler(), turtle.getSelectedSlot(), 1, turtle.getSelectedSlot() );
+        ItemStack stack = InventoryUtil.takeItems( quantity, turtle.getItemHandler(), turtle.getSelectedSlot(), 1, turtle.getSelectedSlot(), false );
         if( stack.isEmpty() )
         {
             return TurtleCommandResult.failure( "No items to drop" );
@@ -63,11 +63,11 @@ public TurtleCommandResult execute( @Nonnull ITurtleAccess turtle )
         if( inventory != null )
         {
             // Drop the item into the inventory
-            ItemStack remainder = InventoryUtil.storeItems( stack, ItemStorage.wrap( inventory, side ) );
+            ItemStack remainder = InventoryUtil.storeItems( stack, ItemStorage.wrap( inventory, side ), false );
             if( !remainder.isEmpty() )
             {
                 // Put the remainder back in the turtle
-                InventoryUtil.storeItems( remainder, turtle.getItemHandler(), turtle.getSelectedSlot() );
+                InventoryUtil.storeItems( remainder, turtle.getItemHandler(), turtle.getSelectedSlot(), false );
             }
 
             // Return true if we stored anything

src/main/java/dan200/computercraft/shared/turtle/core/TurtleEquipCommand.java

@@ -63,12 +63,12 @@ public TurtleCommandResult execute( @Nonnull ITurtleAccess turtle )
         if( newUpgradeStack != null )
         {
             // Consume new upgrades item
-            InventoryUtil.takeItems( 1, ItemStorage.wrap( inventory ), turtle.getSelectedSlot(), 1, turtle.getSelectedSlot() );
+            InventoryUtil.takeItems( 1, ItemStorage.wrap( inventory ), turtle.getSelectedSlot(), 1, turtle.getSelectedSlot(), false );
         }
         if( oldUpgradeStack != null )
         {
             // Store old upgrades item
-            ItemStack remainder = InventoryUtil.storeItems( oldUpgradeStack, ItemStorage.wrap( inventory ), turtle.getSelectedSlot() );
+            ItemStack remainder = InventoryUtil.storeItems( oldUpgradeStack, ItemStorage.wrap( inventory ), turtle.getSelectedSlot(), false );
             if( !remainder.isEmpty() )
             {
                 // If there's no room for the items, drop them

src/main/java/dan200/computercraft/shared/turtle/core/TurtlePlaceCommand.java

@@ -131,7 +131,7 @@ private static boolean deployOnEntity( @Nonnull ItemStack stack, final ITurtleAc
         Vec3 hitPos = hit.getValue();
 
         ItemStorage itemHandler = ItemStorage.wrap( turtlePlayer.getInventory() );
-        DropConsumer.set( hitEntity, drop -> InventoryUtil.storeItems( drop, itemHandler, 1 ) );
+        DropConsumer.set( hitEntity, drop -> InventoryUtil.storeItems( drop, itemHandler, 1, false ) );
 
         boolean placed = doDeployOnEntity( stack, turtlePlayer, hitEntity, hitPos );
 

src/main/java/dan200/computercraft/shared/turtle/core/TurtlePlayer.java

@@ -185,7 +185,7 @@ public void unloadInventory( ITurtleAccess turtle )
         int totalSize = getInventory().getContainerSize();
         for( int i = slots; i < totalSize; i++ )
         {
-            ItemStack remainder = InventoryUtil.storeItems( getInventory().getItem( i ), turtle.getItemHandler(), turtle.getSelectedSlot() );
+            ItemStack remainder = InventoryUtil.storeItems( getInventory().getItem( i ), turtle.getItemHandler(), turtle.getSelectedSlot(), false );
             if( !remainder.isEmpty() )
             {
                 WorldUtil.dropItemStack( remainder, turtle.getLevel(), dropPosition, dropDirection );

src/main/java/dan200/computercraft/shared/turtle/core/TurtleSuckCommand.java

@@ -61,15 +61,15 @@ public TurtleCommandResult execute( @Nonnull ITurtleAccess turtle )
             ItemStorage inventory = ItemStorage.wrap( inventoryContainer );
 
             // Take from inventory of thing in front
-            ItemStack stack = InventoryUtil.takeItems( quantity, inventory );
+            ItemStack stack = InventoryUtil.takeItems( quantity, inventory, false );
             if( stack.isEmpty() ) return TurtleCommandResult.failure( "No items to take" );
 
             // Try to place into the turtle
-            ItemStack remainder = InventoryUtil.storeItems( stack, turtle.getItemHandler(), turtle.getSelectedSlot() );
+            ItemStack remainder = InventoryUtil.storeItems( stack, turtle.getItemHandler(), turtle.getSelectedSlot(), false );
             if( !remainder.isEmpty() )
             {
                 // Put the remainder back in the inventory
-                InventoryUtil.storeItems( remainder, inventory );
+                InventoryUtil.storeItems( remainder, inventory, false );
             }
 
             // Return true if we consumed anything
@@ -111,7 +111,7 @@ public TurtleCommandResult execute( @Nonnull ITurtleAccess turtle )
                     leaveStack = ItemStack.EMPTY;
                 }
 
-                ItemStack remainder = InventoryUtil.storeItems( storeStack, turtle.getItemHandler(), turtle.getSelectedSlot() );
+                ItemStack remainder = InventoryUtil.storeItems( storeStack, turtle.getItemHandler(), turtle.getSelectedSlot(), false );
 
                 if( remainder != storeStack )
                 {

src/main/java/dan200/computercraft/shared/turtle/core/TurtleTransferToCommand.java

@@ -30,19 +30,19 @@ public TurtleTransferToCommand( int slot, int limit )
     public TurtleCommandResult execute( @Nonnull ITurtleAccess turtle )
     {
         // Take stack
-        ItemStack stack = InventoryUtil.takeItems( quantity, turtle.getItemHandler(), turtle.getSelectedSlot(), 1, turtle.getSelectedSlot() );
+        ItemStack stack = InventoryUtil.takeItems( quantity, turtle.getItemHandler(), turtle.getSelectedSlot(), 1, turtle.getSelectedSlot(), false );
         if( stack.isEmpty() )
         {
             turtle.playAnimation( TurtleAnimation.WAIT );
             return TurtleCommandResult.success();
         }
 
         // Store stack
-        ItemStack remainder = InventoryUtil.storeItems( stack, turtle.getItemHandler(), slot, 1, slot );
+        ItemStack remainder = InventoryUtil.storeItems( stack, turtle.getItemHandler(), slot, 1, slot, false );
         if( !remainder.isEmpty() )
         {
             // Put the remainder back
-            InventoryUtil.storeItems( remainder, turtle.getItemHandler(), turtle.getSelectedSlot(), 1, turtle.getSelectedSlot() );
+            InventoryUtil.storeItems( remainder, turtle.getItemHandler(), turtle.getSelectedSlot(), 1, turtle.getSelectedSlot(), false );
         }
 
         // Return true if we moved anything

src/main/java/dan200/computercraft/shared/turtle/upgrades/TurtleTool.java

@@ -270,7 +270,7 @@ private TurtleCommandResult dig( ITurtleAccess turtle, Direction direction )
 
     private static Function<ItemStack, ItemStack> turtleDropConsumer( BlockEntity tile, ITurtleAccess turtle )
     {
-        return drop -> tile.isRemoved() ? drop : InventoryUtil.storeItems( drop, turtle.getItemHandler(), turtle.getSelectedSlot() );
+        return drop -> tile.isRemoved() ? drop : InventoryUtil.storeItems( drop, turtle.getItemHandler(), turtle.getSelectedSlot(), false );
     }
 
     private static void stopConsuming( BlockEntity tile, ITurtleAccess turtle )

src/main/java/dan200/computercraft/shared/util/InventoryUtil.java

@@ -103,19 +103,19 @@ public static Container getInventory( BlockEntity tileEntity )
     // Methods for placing into inventories:
 
     @Nonnull
-    public static ItemStack storeItems( @Nonnull ItemStack itemstack, ItemStorage inventory, int begin )
+    public static ItemStack storeItems( @Nonnull ItemStack itemstack, ItemStorage inventory, int begin, boolean simulate )
     {
-        return storeItems( itemstack, inventory, 0, inventory.size(), begin );
+        return storeItems( itemstack, inventory, 0, inventory.size(), begin, simulate );
     }
 
     @Nonnull
-    public static ItemStack storeItems( @Nonnull ItemStack itemstack, ItemStorage inventory )
+    public static ItemStack storeItems( @Nonnull ItemStack itemstack, ItemStorage inventory, boolean simulate )
     {
-        return storeItems( itemstack, inventory, 0, inventory.size(), 0 );
+        return storeItems( itemstack, inventory, 0, inventory.size(), 0, simulate );
     }
 
     @Nonnull
-    public static ItemStack storeItems( @Nonnull ItemStack stack, ItemStorage inventory, int start, int range, int begin )
+    public static ItemStack storeItems( @Nonnull ItemStack stack, ItemStorage inventory, int start, int range, int begin, boolean simulate )
     {
         if( stack.isEmpty() ) return ItemStack.EMPTY;
 
@@ -125,27 +125,27 @@ public static ItemStack storeItems( @Nonnull ItemStack stack, ItemStorage invent
         {
             int slot = start + (i + begin - start) % range;
             if( remainder.isEmpty() ) break;
-            remainder = inventory.store( slot, remainder, false );
+            remainder = inventory.store( slot, remainder, simulate );
         }
         return areItemsEqual( stack, remainder ) ? stack : remainder;
     }
 
     // Methods for taking out of inventories
 
     @Nonnull
-    public static ItemStack takeItems( int count, ItemStorage inventory, int begin )
+    public static ItemStack takeItems( int count, ItemStorage inventory, int begin, boolean simulate )
     {
-        return takeItems( count, inventory, 0, inventory.size(), begin );
+        return takeItems( count, inventory, 0, inventory.size(), begin, simulate );
     }
 
     @Nonnull
-    public static ItemStack takeItems( int count, ItemStorage inventory )
+    public static ItemStack takeItems( int count, ItemStorage inventory, boolean simulate )
     {
-        return takeItems( count, inventory, 0, inventory.size(), 0 );
+        return takeItems( count, inventory, 0, inventory.size(), 0, simulate );
     }
 
     @Nonnull
-    public static ItemStack takeItems( int count, ItemStorage inventory, int start, int range, int begin )
+    public static ItemStack takeItems( int count, ItemStorage inventory, int start, int range, int begin, boolean simulate )
     {
         // Combine multiple stacks from inventory into one if necessary
         ItemStack partialStack = ItemStack.EMPTY;
@@ -157,7 +157,7 @@ public static ItemStack takeItems( int count, ItemStorage inventory, int start,
             if( count <= 0 ) break;
 
             // If this doesn't slot, abort.
-            ItemStack extracted = inventory.take( slot, count, partialStack, false );
+            ItemStack extracted = inventory.take( slot, count, partialStack, simulate );
             if( extracted.isEmpty() )
             {
                 continue;

src/main/java/dan200/computercraft/shared/util/ItemStorage.java

@@ -197,6 +197,12 @@ else if( areStackable( stack, existing ) )
                 return stack;
             }
         }
+
+        @Override
+        public String toString()
+        {
+            return String.format( "%s{type=%s}", getClass().getSimpleName(), inventory.getClass().getSimpleName() );
+        }
     }
 
     class SidedInventoryWrapper extends InventoryWrapper
@@ -248,6 +254,12 @@ public ItemStack store( int slot, @Nonnull ItemStack stack, boolean simulate )
             }
             return super.store( mappedSlot, stack, simulate );
         }
+
+        @Override
+        public String toString()
+        {
+            return String.format( "%s{type=%s, facing=%s}", getClass().getSimpleName(), inventory.getClass().getSimpleName(), facing );
+        }
     }
 
     class View implements ItemStorage
@@ -317,5 +329,11 @@ public ItemStorage view( int start, int size )
         {
             return new View( parent, this.start + start, size );
         }
+
+        @Override
+        public String toString()
+        {
+            return String.format( "%s{parent=%s, start=%d, size=%d}", getClass().getSimpleName(), parent, start, size );
+        }
     }
 }