Trade Logic Fixes - #47
Merged
Merged
Conversation
UltraProdigy
left a comment
Member
There was a problem hiding this comment.
Only saw one functional problem... as far as the bug maybe you could make a local mutabe copy of the inputSlotCache and decrement it as each requirement “reserves” items during the check so it can't double count?
UltraProdigy
approved these changes
Dec 16, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Supersedes #37.
Fixes #36.
Fixes GTNewHorizons/GT-New-Horizons-Modpack#22713.
Changes:
Developer Comments:
Full disclosure:
Technically, a bug can occur if a trade requires multiple inputs which can be satisfied with a single item. eg. if a trade wanted 1 oredict:logWood + 1 Oak log. Since the required items are checked separately, having 1 oak log in the input slots would satisfy both these conditions. However, when the trade is actually performed, it would void the oak log and fail the trade. This bug does not affect the pack currently as we do not have trades that fit this condition.
I'm not sure if there's an elegant solution to address this, especially if we look at the general case of checking whether a set of items with overlapping oredicts can be mix-and-matched to fulfill an arbitrary set of oredict/non-oredict requirements (intuitively it sounds like a np-complete problem). We can possibly deal with the voiding issue by tracking what items were consumed and returning them to the output slot if the trade fails while trying to pull the required items.
Tested in SP+MP daily 214.