Corporea + Storage Drawers
syntax-error opened this issue ยท 16 comments
When using a Corporea Index to request items out of a Storage Drawer (from the mod of the same name), if you request a full stack or greater, it only delivers a single stack of the item, and empties the drawer, even if you should have had items remaining. If you request 63 or less, it works as expected.
Additionally, the item count appears to be counting up to one stack twice. I'm presuming that is probably something akin to the JABBA issue, where the drawers have an outlet that only reports 1 stack of items, but, unlike JABBA, simultaneously also reports its entire contents.
Does your system work through ISidedInventory? (And can you point out where you inventory handler code is?)
So the answer is "kinda". I see that you are checking for ISidedInventory in your isValid method. But you're not checking if you're allowed to insert or extract through the given slot.
That's a problem, because the slots in Storage Drawers and JABBA are meant to be insert-only or extract-only, and that's enforced through the ISidedInventory interface.
Well, either way, I added the check, so I'm closing this.
aa39b2d
Did you change anything in your code @jaquadro. I can't reproduce this.
I'll have to check later. I didn't make any changes specific to this area, but it's also something that may not repro in simple cases.
@syntax-error, perhaps you could verify how this changes the behavior of both Storage Drawers and JABBA?
I'd be happy to do so. What is the best way to handle testing unreleased changes like this?
Okay. I'll play around with git when I wake up. Only just started using git this year for contract dev work, and it still baffles the hell out of me some days.
After downloading a copy of your repository and teaching myself the basics of forgegradle to enable me to build a copy of botania as it stands now, I have tested.
(I also used the latest released version of Storage Drawers. I used NEI and Waila and any required libraries to facilitate testing. Otherwise no other mods)
The problem has improved. The corporea system now reports only a stacks worth of items are available when more than 1 stack of items are in a drawer (which is survivable, and is what happens with JABBA). Whenever I order less than a stack of items, I get the correct amount in inventory and the drawer loses the correct amount. If I order a stack or more (by using "192", "3 stacks" or "all", for instance), I get functionality similar to JABBA, in that I only gain the stack of the requested item. However, the drawers loses up to 2 stacks. If the drawer contains a number of items between 1 and 2 stacks, the entire lot is lost. If the drawers contain more than 2 stacks, exactly 2 stacks worth of items are lost from the drawer instead of the expected 1.
Steps to reproduce:
- Create a simple corporea system using a chest with a master spark, a 1x2 full drawer with a spark, and a corporea index with a spark.
- Place 6 stacks of grass blocks into one of the drawers, for 384 blocks.
- Request 1+ stacks of grass blocks. E.g. "all grass blocks". If my bug isn't a magical one off case, you should receive 64 and have 256 blocks remain in the drawer.
Gah, that's even worse >_>
I'll have to try it out specifically with the storage drawers mod then.
I'm currently stepping through. The extraction check was crucial, but it's likely any remaining problem is on my end. I'll give you an update what I find something.
Also going to add, if you want to be able to support extracting the full inventory of these kinds of blocks, you can make repeated extraction attempts on the same slot until it no longer yields output.
Here's what I've managed to dig up. This is the piece of code in CorporeaHelper that leads to the double-extraction:
stackAt.stackSize -= rem;
if(stackAt.stackSize == 0)
inv.setInventorySlotContents(i, null);
This is fragile. You're adjusting stack sizes directly without a corresponding call to the inventory's markDirty(). Drawers and barrels use that call to balance their inventories against adjusted stacks, but it's mostly working because they also balance themselves proactively in calls like getStackInSlot. This could easily lead to an edge case failure if you adjusted a stack without any further calls into the inventory.
The additional call to setInventorySlotContents cause a double-extraction in drawers because they balance the outstanding stack changes before performing the set operation, which means the output stack would be reloaded and cleared again. JABBA balances after the set operation. I can probably change the balance order on my side, but I need to carefully test the impact against a dozen other transport systems.
I would advise replacing the above block with a single call to decrStackInSlot, which will let inventories handle their own empty stack cleanup and invalidation. It would wipe out most of the potential failure edge cases in complex inventories. If you check vanilla hopper mechanics, the emptying of a chest does not trigger a setStackInSlot call.
If you think it is all a big pain, you have my sympathy. IInventory is one of the most horridly-designed interfaces in this game.