Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add checkDamage Boolean for li.cil.oc.util.InventoryUtils#haveSameItemType #3706

Open
wants to merge 3 commits into
base: master-MC1.7.10
Choose a base branch
from

Conversation

zeng-github01
Copy link

@zeng-github01 zeng-github01 commented Apr 14, 2024

maybe fix #3288

Need to test

@asiekierka
Copy link
Contributor

While this PR may help in a specific case, I am worried other mods may be patching the villager trade code to check damage for their specific items - as such, fixing this could lead to more devastating issues in cross-mod interaction.

In my opinion, the best way would be to try and re-use the same codepath vanilla uses for executing villager trades, but as I outlined in the initial comment - this would probably not be too easy.

@zeng-github01
Copy link
Author

While this PR may help in a specific case, I am worried other mods may be patching the villager trade code to check damage for their specific items - as such, fixing this could lead to more devastating issues in cross-mod interaction.

In my opinion, the best way would be to try and re-use the same codepath vanilla uses for executing villager trades, but as I outlined in the initial comment - this would probably not be too easy.

You mean there might be other mods that modify the OC code? That would cause glitches?

@zeng-github01
Copy link
Author

zeng-github01 commented Apr 16, 2024

@asiekierka I have a new idea, can you help me test it? I will submit a new commit about it soon

Test branch

Considering that this patch is very irregular, I chose to submit it to the test branch
I found the vanilla transaction code by querying useRecipe, the above code is an attempt to manually build the relevant class

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect villager trade emulation in Trading Upgrade
2 participants