Inverter card preventing multiple types being imported
VoidTest opened this issue ยท 6 comments
Describe the bug
When an import bus is configured with an inverter card for a liquid it cant pull items from the storage.
How to reproduce the bug
Put an inverter card in an import bus and blacklist a liquid then try pulling an item from a chest.
Expected behavior
The export bus pulls everything but the blacklisted liquid even items.
Additional details
This happened when blacklisting blood and trying to pull the item from the blood infuser from evilcraft.
Which minecraft version are you using?
1.21
On which mod loaders does it happen?
NeoForge
Crash log
n/a
A workaround for your specific use case might be to work without an inverter card and an explicit blacklist for the blood liquid and rather forbid any fluids to be imported by only allowing items in the submenu reached via the 2nd button on the left called "Configure Imported Types".
As for the bug itself: I'd say you're right, something is not right there. I'm not sure yet what causes it and I've so far seen three components that might have an influence there:
- The filter list itself and the event of updating it (by removing or adding an item)
- The presence of the inverter card and the event of adding or removing it
- The "Configure Imported Types" setting and the event of changing it
I've seen it start to import items after changing some of those settings and have not yet figured out a pattern.
what I think might be happening is that the filter is evaluated per resource type, so the item filter is effectively empty.
empty filters have the quirk that they are flipped (and empty whitelist allows everything, and an empty blacklist blocks everything)
and I think since the item filter is empty, it won't import anything
one thing you can try to confirm this theory (and work around this issue) is placing one item you don't need to import in the filter
Let's deep dive into the code here:
ImportBusPart.doBusWork
calls importStrategy.transfer(context)
, where importStrategy
is an array of StorageImportStrategy
, one for each resource type (items and fluids in my test scenario) and context
holds the information of isInverted
(i.e. the presence of the inverter card).
Within importStrategy.transfer(context)
we have a for
loop over each strategy and calls strategy.transfer(context)
on them. If the return value is true
the for loop is aborted and the other strategies would be ignored. Spoiler: true is never returned by the actual implementation.
Within strategy.transfer(context)
is the check if that key type (i.e. items or fluids) is enabled and the targeted block has a handler for that kind of transfer. Then we iterate over all inventory slots in the adjacent block and skip any that are empty or satisfy context.isInFilter(resource.what()) == context.isInverted()
which is ultimately applying the filter. context.isInverted()
is true as we have a inverter card installed, so if the item is in the filter we skip it. We now go down to StackTransferContextImpl.isInFilter()
which is implemented as return filter.isEmpty() || filter.isListed(key);
If the filter is empty (which it would be if no item was filtered as originally stated) the return value would be true and the item would be skipped. Thus Mari023's suggestion of adding a random irrelevant item to your blacklist would fix the problem.
The code for StackTransferContextImpl.isInFilter()
which is implemented as return filter.isEmpty() || filter.isListed(key);
mixes two information parts (list is empty and does the item match the filter) and the result is then compared to the inverter card presence information context.isInverted()
. This is however non-trivial, as it is implemented as return !filter.isEmpty() && isInverted;
thus also having a special case for empty lists. I'll have to check the truth tables for that to see if it makes sense in all cases...
Plot twist: We don't even get to that point: We leave here because of if (!context.isKeyTypeEnabled(conversion.getKeyType()))
if there is no single key for that key type as the list of keyTypes
is initialized based on the actually configured filters and not on the "Configure Imported Types" from the import bus.
Quick test: returning a hard true
in isKeyTypeEnabled
correctly imports the items without having a single item in the filter list.
The last thing I want to add to this analysis: A brief history:
- Import bus code had a major rewrite in December 2021 -> 44bc92f6
- Inverter card support was added in April 2022 -> 250c676c
- Manual selection of key types is the most recent addition in February 2024 -> ada06e10
The third step did not account for the already existing keyType check in the first step. I cannot tell if it is a conscious decision or an oversight and I am therefore hesitant to decide whether and if how a fix should be made. Several places where a fix might be placed have other uses than just import bus behavior that needs to be considered and that is beyond my understanding of the code.
I see a potential conflict between what the manual key type selection does and the implicit key type check based on what is in the list, since you may add entries to the filter list of a key type you forbid in the manual key type selection. My gut feeling is that once you introduce manual key type selection you should stick to it all the way and not have a second, hidden mechanism that in some cases contradicts your manual selection. I've experimented with two approaches for a fix:
- Remove the
if (!context.isKeyTypeEnabled(conversion.getKeyType())) { return false; }
from StorageImportStrategy.java - Override the key types automatically determined in the
StackTransferContextImpl
constructor when creating it inImportBusPart.doBusWork()
with the list obtained fromkeyTypeSelection
.
Since either may have a broad range of side effects I'm not confident enough to propose a PR for this problem myself, especially when considering edge cases such as a filter list with entries that belong to a masked out key type. Technically the list would not be empty for the relevant checks in the code, even if it semantially probably should be considered empty.