Fluids can be voided when autocrafting with multiple fluids
MAKtheMortal opened this issue · 5 comments
Describe the bug
If you run an autocrafting recipe with multiple fluids, and the destination inventory only has one tank available, the additional fluids will be voided.
For example, if the crafting recipe uses fluid A and B, and the crafter points at a Functional Storage fluid drawer 1x2, this would normally work just fine. Even if there are some A and B in the drawer already. Even if there isn't enough room in one of the tanks, the autocrafting system will properly detect this and block until there is room. The bug comes in if there is fluid C in one of the two drawers. Fluid A goes into the empty tank and fluid B gets voided. You'll see the following log warning:
[Server] Server thread/WARN [co.re.re.ap.au.ICraftingPatternContainer/]: Inventory unexpectedly didn't accept all of material.B, the remainder has been voided!
I have tracked this down in the code, and it looks to be a problem with ICraftingPatternContainer.insertFluidsIntoInventory(). By analogy with ICraftingPatternContainer.insertItemsIntoInventory(), the code should go tank-by-tank, and remove tanks from consideration as fluids get sim-inserted. It looks like the Forge API doesn't support inserting to a specific tank, which makes this problematic. Perhaps there is some way to calculate if the fluid could possibly fit, regardless of how the fluid handler is implemented. In the example above, no implementation of IFluidHandler could fit both A and B if it already has C and only two tanks. This could be an additional check if the existing simulation check passes. Ideally Forge should change the API to mirror IItemHandler, but the use case of simultaneously inserting multiple fluids in simulation mode may be a bit niche.
How can we reproduce this bug or crash?
- Point a crafter at a Functional Storage fluid drawer 1x2, or any other container that accepts two fluids.
- Put some fluid in one of the two fluid drawers.
- Create a recipe that has two fluid ingredients, not including the fluid you already inserted.
- Put the recipe in the crafter and craft it.
- Observe that one of your ingredients is voided and there is a log warning about the voiding.
What Minecraft version is this happening on?
Minecraft 1.20.1
What Forge version is this happening on?
47.2.20
What Refined Storage version is this happening on?
1.12.4
Relevant log output
[Server] Server thread/WARN [co.re.re.ap.au.ICraftingPatternContainer/]: Inventory unexpectedly didn't accept all of material.gtceu.duranium, the remainder has been voided!
I'm new to modding and open source in general, but I'm not new to programming.
I've locally added the following code to insertFluidsIntoInventory() just before the final return true, and it seems to be working well for my use case (Functional Storage fluid drawers and GregTech machines). It obviously can't be guaranteed to match whatever is hiding in IFluidHandler, but still it's better than what it's currently doing.
// Perform extra check during simulation to prevent voiding fluids.
// This is inspired by the way insertItemsIntoInventory() does it.
if (action == Action.SIMULATE) {
Deque<FluidStack> stacks = new ArrayDeque<>(toInsert);
FluidStack current = stacks.poll();
int amountToInsert = current != null ? current.getAmount() : 0;
List<Integer> availableTanks = IntStream.range(0, dest.getTanks()).boxed().collect(Collectors.toList());
while (current != null && !availableTanks.isEmpty()) {
for (int i = 0; i < availableTanks.size(); ++i) {
int tank = availableTanks.get(i);
int capacity = dest.getTankCapacity(tank);
FluidStack existingFluid = dest.getFluidInTank(tank);
int existingAmount = existingFluid.getAmount();
if ((existingAmount == 0 || existingFluid.isFluidEqual(current)) && existingAmount < capacity) {
amountToInsert -= capacity - existingAmount;
availableTanks.remove(i);
break;
}
}
if (amountToInsert <= 0) { // If we inserted successfully, get a next stack.
current = stacks.poll();
amountToInsert = current != null ? current.getAmount() : 0;
} else if (current.getAmount() == amountToInsert) { // If we didn't insert anything over ALL these tanks, stop here.
break;
} // If we didn't insert all, continue with other tanks
}
return current == null && stacks.isEmpty();
}
I haven't learned all the contribution procedures, and I don't want to step on anyone's toes. But perhaps this code will help. :)
Still not perfect... I get a rare voided fluid when running complex recipe chains. Specifically, I lost a small amount of water while crafting some gelatin mixture in a GregTech mixer. This block has two input tanks, and the recipe has two input fluids. I can't reproduce the problem with a simple setup, and I can't imagine what went wrong. The recipe chain does use this mixer for other recipes in parallel, so it is likely that at least one of the mixer input tanks had something in it.
Yeah, losing water isn't a big deal, but it clogs up the machine with the other inputs and stalls the complex autocraft until I go and manually insert the missing water. It also implies my algorithm doesn't quite match what gregtech is doing when filling the IFluidHandler.
Github bug reporter that actually tracks down the exact line of code responsible? Your service is greatly appreciated, we need more of you in the world
I've encountered this issue recently while going through Greg Tech. Moreso with the single-block machines like the Chemical Reactor, but I've run into the problem with my setup for the multiblock machines as well.
I am using a crafter pointed to an AE2 interface (which can accept both items and fluids) with storage buses on the input parts of the machines. There are 9 input slots in the interface and my single recipe had 4 item inputs and 2 fluid inputs, so there should have been enough room in the interface to store these, but one of the fluids was voided. I can only assume that it tried to put one fluid where an item was already placed.