From 776a786e1bb46b0f1fbba742f8c334a299e1b180 Mon Sep 17 00:00:00 2001 From: SquidDev Date: Tue, 27 Nov 2018 17:17:39 +0000 Subject: [PATCH] Fix InventoryUtil ignoring the stack limit when extracting items Using turtle.suck on an inventory filled with tools would fill the entire chest with said item, rather than extracting a single item. In order to avoid that, we clamp the extract limit to the max stack size when first extracting an item. This also inlines the makeSlotList logic, which means we can avoid creating an array for each inventory operation. This probably won't have any meaninful performance impact (even on large inventories), but is a nice optimisation to make. --- .../shared/util/InventoryUtil.java | 117 ++++++------------ 1 file changed, 36 insertions(+), 81 deletions(-) diff --git a/src/main/java/dan200/computercraft/shared/util/InventoryUtil.java b/src/main/java/dan200/computercraft/shared/util/InventoryUtil.java index f7e808a449..17feaa6fd1 100644 --- a/src/main/java/dan200/computercraft/shared/util/InventoryUtil.java +++ b/src/main/java/dan200/computercraft/shared/util/InventoryUtil.java @@ -95,122 +95,77 @@ else if( tileEntity instanceof IInventory ) // Methods for placing into inventories: - @Nonnull - public static ItemStack storeItems( @Nonnull ItemStack itemstack, IItemHandler inventory, int start, int range, int begin ) - { - int[] slots = makeSlotList( start, range, begin ); - return storeItems( itemstack, inventory, slots ); - } - @Nonnull public static ItemStack storeItems( @Nonnull ItemStack itemstack, IItemHandler inventory, int begin ) { - int[] slots = makeSlotList( 0, inventory.getSlots(), begin ); - return storeItems( itemstack, inventory, slots ); + return storeItems( itemstack, inventory, 0, inventory.getSlots(), begin ); } @Nonnull public static ItemStack storeItems( @Nonnull ItemStack itemstack, IItemHandler inventory ) { - int[] slots = makeSlotList( 0, inventory.getSlots(), 0 ); // TODO: optimise this out? - return storeItems( itemstack, inventory, slots ); + return storeItems( itemstack, inventory, 0, inventory.getSlots(), 0 ); } - // Methods for taking out of inventories - @Nonnull - public static ItemStack takeItems( int count, IItemHandler inventory, int start, int range, int begin ) + public static ItemStack storeItems( @Nonnull ItemStack stack, IItemHandler inventory, int start, int range, int begin ) { - int[] slots = makeSlotList( start, range, begin ); - return takeItems( count, inventory, slots ); + if( stack.isEmpty() ) return ItemStack.EMPTY; + + // Inspect the slots in order and try to find empty or stackable slots + ItemStack remainder = stack.copy(); + for( int i = 0; i < range; i++ ) + { + int slot = start + ((i + (begin - start)) % range); + if( remainder.isEmpty() ) break; + remainder = inventory.insertItem( slot, remainder, false ); + } + return areItemsEqual( stack, remainder ) ? stack : remainder; } + // Methods for taking out of inventories + @Nonnull public static ItemStack takeItems( int count, IItemHandler inventory, int begin ) { - int[] slots = makeSlotList( 0, inventory.getSlots(), begin ); - return takeItems( count, inventory, slots ); + return takeItems( count, inventory, 0, inventory.getSlots(), begin ); } @Nonnull public static ItemStack takeItems( int count, IItemHandler inventory ) { - int[] slots = makeSlotList( 0, inventory.getSlots(), 0 ); - return takeItems( count, inventory, slots ); - } - - // Private methods - - private static int[] makeSlotList( int start, int range, int begin ) - { - if( start < 0 || range == 0 ) - { - return null; - } - - int[] slots = new int[ range ]; - for( int n = 0; n < slots.length; ++n ) - { - slots[ n ] = start + ((n + (begin - start)) % range); - } - return slots; - } - - @Nonnull - private static ItemStack storeItems( @Nonnull ItemStack stack, IItemHandler inventory, int[] slots ) - { - if( slots == null || slots.length == 0 ) - { - return stack; - } - if( stack.isEmpty() ) - { - return ItemStack.EMPTY; - } - - // Inspect the slots in order and try to find empty or stackable slots - ItemStack remainder = stack.copy(); - for( int slot : slots ) - { - if( remainder.isEmpty() ) break; - remainder = inventory.insertItem( slot, remainder, false ); - } - return areItemsEqual( stack, remainder ) ? stack : remainder; + return takeItems( count, inventory, 0, inventory.getSlots(), 0 ); } @Nonnull - private static ItemStack takeItems( int count, IItemHandler inventory, int[] slots ) + public static ItemStack takeItems( int count, IItemHandler inventory, int start, int range, int begin ) { - if( slots == null ) - { - return ItemStack.EMPTY; - } - // Combine multiple stacks from inventory into one if necessary ItemStack partialStack = ItemStack.EMPTY; - int countRemaining = count; - for( int slot : slots ) + for( int i = 0; i < range; i++ ) { - if( countRemaining <= 0 ) break; + int slot = start + ((i + (begin - start)) % range); + + if( count <= 0 ) break; ItemStack stack = inventory.getStackInSlot( slot ); - if( !stack.isEmpty() ) + if( !stack.isEmpty() && (partialStack.isEmpty() || areItemsStackable( stack, partialStack )) ) { - if( partialStack.isEmpty() || areItemsStackable( stack, partialStack ) ) + ItemStack extracted = inventory.extractItem( slot, count, false ); + if( !extracted.isEmpty() ) { - ItemStack extracted = inventory.extractItem( slot, countRemaining, false ); - if( !extracted.isEmpty() ) + if( partialStack.isEmpty() ) { - countRemaining -= extracted.getCount(); - if( partialStack.isEmpty() ) - { - partialStack = extracted; - } - else - { - partialStack.grow( extracted.getCount() ); - } + // If we've extracted for this first time, then limit the count to the maximum stack size. + partialStack = extracted; + count = Math.min( count, extracted.getMaxStackSize() ); } + else + { + partialStack.grow( extracted.getCount() ); + } + + count -= extracted.getCount(); } } }