Use slot-by-slot insertion to resolve Fluid Tank transfer issue with Item Pipes

[epiccoleman]

Description

This PR attempts to address #705

As described in the issue, when multiple filled Portable Tanks (or Portable tanks with duplicate fullness values) are transferred into an inventory via an Item Pipe, the source chest will receive only one of the Portable Tanks, with the subsequently transferred Portable Tanks disappearing.

It looks like Fabric's Storage.insert() method does not respect the item's DataComponents.MAX_STACK_SIZE when merging into existing slots in vanilla inventories. This may imply that the problem exists for other items as well, though I did not find any other cases during testing.

The fix adds a slot-by-slot transfer for SlottedStorage inventories in the FabricItemApi wrapper class, to ensure that the inserted item's MaxStackSize is respected. For non SlottedStorage inventories, the original behavior is preserved.

Full transparency: I used Claude Code to help make this PR.

Further disclaimer: I know very little about Minecraft mod dev - so I am not sure that this is the correct approach to the bugfix or if this approach might carry other implications.

Fixes #705

How Has This Been Tested?

This was tested in game on a fresh instance of Minecraft 1.21.1 with only the Fabric loader and Oritech 1.0.1 installed. I used the same test setup as described in #705 - two chests next to each other, with a source chest being extracted to the target chest. I filled several tanks of both Turbofuel and Sulfuric Acid and verified that the target chest received each tank as expected.

It is worth noting that this problem is not specific to Portable Tanks, but rather seems to be an issue in any case where an item uses DataComponents.MAX_STACK_SIZE to change from the default stack size. I also tested the fix with some diamonds given via /give @s diamond[max_stack_size=1], which reproduced the problem behavior in an unmodified Oritech install, and which also behaves appropriately after applying this PR.

[epiccoleman]

I was thinking about this a little more during the work day - I wonder if the fix for this issue really should target Fabric, since it does seem to be a bug in their API more so than in Oritech specifically (I'd think that Oritech should be able to expect that Storage.insert won't just disappear items, instead of special casing SlottedStorage which I assume is the most common type).

I'll take a glance at the Fabric issue tracker and see if there's anything in there that seems related, and report back. Probably still a case to be made for addressing it in Oritech in the interim?

[epiccoleman]

OK, further investigation. I think I've found the issue in Fabric itself.

I think what's happening here is that InventorySlotWrapper.getCapacity is using the Fluid Tank item definition's default, unfilled stack size of 64. When a Fluid Tank is filled its maximum stack size is changed via DataComponents, and so Fabric is unaware that the tank can't stack, so it tries to push it into the same slot as the other tank. I would need to spend a bit more time than I have to trace right now to trace the whole chain and debug it.

	@Override
	public int getCapacity(ItemVariant variant) {
		// Special case to limit buckets to 1 in furnace fuel inputs.
		if (storage.inventory instanceof AbstractFurnaceBlockEntity && slot == 1 && variant.isOf(Items.BUCKET)) {
			return 1;
		}

		// Special case to limit brewing stand "bottle inputs" to 1.
		if (storage.inventory instanceof BrewingStandBlockEntity && slot < 3) {
			return 1;
		}

		return Math.min(storage.inventory.getMaxCountPerStack(), variant.getItem().getMaxCount());
	}

relevant Fabric source

I've built a tweaked Fabric which changes that last line to:

return Math.min(storage.inventory.getMaxCountPerStack(), variant.toStack().getMaxCount());

Which I think will respect the specific item instance's max stack size. We'll see how far I get, I did not expect to be building a new copy of Fabric today :)

[epiccoleman]

Alright. Built a fresh copy of Fabric API with my change. That does indeed fix the issue.

I'll get an issue opened up on the Fabric repo with a PR over there, and link here when that's done. @Rearth - I'll leave this issue open over here for now - guess it's up to you whether you'd want Oritech to fix this in the Fabric wrapper code or wait for a lower-level fix in Fabric.

[epiccoleman]

I was able to confirm that this issue can be fixed in Fabric API. I've filed an issue and a PR over on the Fabric API, which explains the problematic chain of calls in the Fabric's Storage API and proposes a fix.

[epiccoleman]

The bugfix associated with this for Fabric has been merged, so this PR is probably no longer needed.