Various routing node improvements/fixes #1678
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This matches the name ingame
ItemStack copies (through ItemHandler::copy() or ItemHandler::extractItem()) are a rather expensive operation. This version only does item extraction when the item matches both the input and output filter's conditions. Ideally, this should mean roughly 1 copy per itemstack moved, although it could be more if the input/output inventories fail to extract or insert. This commit also adds a "canSkip()" heuristic function to item filters, allowing filters that can neither accept nor provide items to be skipped.
transferThroughInputFilter was returning the amount it could still transfer, while TileMasterRoutingNode expected to receive the amount actually transferred.
ItemStacks with count <= 0 are empty, causing them to be treated as a stack of air. Many comparisons with empty stacks will directly use air as the item INSTEAD of the ItemStack's actual item. Input filtering nodes were using negative stack sizes to get the amount of items that can be pulled, causing incorrect comparisons.
Previously, they would drain the stack given by the input filter and ignore their own output limits.
Author
keraldi
approved these changes
Dec 30, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi, here are 3 routing nodes bug fixes and one performance optimization:
Fluid output filters would try to extract the amount given to them by the input filter, ignoring their own limits. This caused odd behavior where inputs limited to amounts such as 50 mb would insert 1000mb in the tank.
All the input item filters derived from PreciseItemFilter had odd behavior since empty item stacks are treated like air in many comparisons.
see related issue
Changes the item filter code to make less ItemStack copies and do less simulated ItemStack extractions, improving performance in large routing systems. Also adds a canSkip heuristic to prune filters that won't be used.
I know the contributing guidelines say not to do unnecessary file renames, but I feel this one is useful since it better matches the ingame item name. Also, I think the old name was meant to be temporary ;). I can remove this commit though.
The commit messages have more information as well! I tested the routing out on a sample world w/one input and one output of each type (including default filters) and several tanks. It seems to work