Industrial Foregoing Version: 1.16.5-3.2.11-55cb112

Titanium Version: 1.16.5-3.2.8.2.jar

Other Versions:
Forge: 1.16.5-36.1.4
Mekanism: 1.16.5-10.0.21.448
JEI: 1.16.5-7.6.4.86
Patchouli: 1.16.4-50

Crashlog If Applicable (Upload to pastebin/gist): N/A

To Reproduce

1. Place down a Bio Reactor (leaving it unpowered).
2. Fill a chest with 9 different stacks of bio-reactor compatible items, e.g.: 9 dyes (or combination of Carrots, Potatoes, Seeds, etc...).
3. Use a Mekanism Ultimate Transporter (or other item transfer that can move stacks of items), set to move from chest to the Bio Reactor.
4. Wait for the stacks to be transfered in.
5. Remove some items from the Bio-Reactor, and putting them back into the chest.
6. Observe that the items are not going back into the Bio-Reactor.
7. Test with a hopper and see that items aren't being pulled in.
8. Place a chest adjacent to the Bio Reactor and set it to pull into the input 3x3 from the chest.
9. Put the items into the chest.
10. Observe it properly fills the Bio Reactor.

Code in question

Description

net.minecraftforge.items.IItemHandler#isItemValid requires that implementation should not consider the current state of the inventory (the input filter insert predicate is used for that method).

I was going to do a PR to make the change but I think you want to preserve the "only insertable into existing slot or if another slot does not contain the same item" and inventory state should not be taken into account and more, explained further below.

Potential Fix 1

In order to do this properly, a larger structural change to Titanium lib's InventoryComponent whould probably need to be done.

What is currently referred to as "input filter / insert predicate" could probably be renamed to "slot filter / slot predicate" since it is used for implementing IItemHandler#isItemValid. I'm suggesting the rename because in most implementations of an "input filter" properly don't take inventory state into account and just filter by slot and item stack alone.

Then, re-adding the original "input filter / insert predicate", which is allowed to take the state of the inventory into account, and having that be used in the implementation of IItemHandler#insertItem (as well as checking the slot filter for sanity perhaps).

I'm not sure if this change would mess with the the auto-pull feature though...?
It's also a breaking api change if it was made.

Potential Fix 2

A quicker / hacky fix would be to just always return true in IItemHandler#isItemValid and then move the predicate check to the IItemHandler#insertItem method, but IMO it is not as proper as the first suggested change. If going with this, IItemHandler#isItemValid should probably be checked in the IItemHandler#insertItem implementation.

Related Change: Remainer Insertion

Updating the code in question to be:

    private boolean canInsert(int slot, ItemStack stack) {
        for (int i = 0; i < input.getInventory().getSlots(); i++) {
            if (input.getInventory().getStackInSlot(i).isItemEqual(stack)) {
                return i == slot;
            }
        }
        for (ITag<Item> itemTag : VALID) {
            if (itemTag.contains(stack.getItem())) return true; //contains
        }
        return false;
    }

This change breaks the contract so doesn't really fix the root problem. I don't recommend doing this on it's own, but I put this here because I noticed a bug with the code in question (assuming that it was okay to take into account inventory state for IItemHandler#isItemValid), where if the item stack trying to be inserted was more than the remaining space in the stack, it would not top-off the stack. I think the remaining space check should be left to the IItemHandler#insertItem implementation in InventoryComponent instead.

So combining this with either of the potential fixes would be good. For Potential Fix 1, the first loop would be for the insert filter, and the second for loop would be for the slot filter.

Related Change: Bio Reactor tag (off-topic, ignore this)

Also, I think for more modpack maker configurability, you could make all the tags other than BIOREACTOR_INPUT be part of BIOREACTOR_INPUT. That way a modpack maker could decide to remove a tag from BIOREACTOR_INPUT if they want to keep the tag but not have it be used in the Bio Reactor. As a bonus you'd only have to check a single tag in the code instead of looping through multiple. The JEI plugin will need to be updated too I think, since it'll only be a single entry otherwise, but maybe this is okay?

Currently I think all the items for the Bioreactor don't give varying amounts, it's just based on the number of filled slots?
Not sure if you wanted to change this to use recipes or something instead of a tag... Yeah this is super off topic, just ignore this one haha.

End Note

Let me know if either of the two related changes should be moved to another issue. Especially the last one, it's probably not be that relevant to this issue. I wouldn't mind doing a PR for that one since it is a smaller change with less side-effects, but it touches relevant code (removing the for loop), so I decided to mention it here.

I hope it is okay I'm adding a comment to this, because I've been going out of my mind, trying to figure out what I was doing wrong with the Bioreactor. I am using Item/Universal Pipes from Pipez mod, and I simply couldn't get it to fill up the bioreactor correctly.

Good to hear this is known and hopefully a fix is in the pipeline. In the meantime, I will use a chest as relay :)