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

Tentatively allow for the market to include items in adjacent inventories #11

Open
wants to merge 2 commits into
base: 1.12
Choose a base branch
from

Conversation

noobanidus
Copy link
Contributor

This is unfortunately quite a large refactoring.

  1. As default vanilla chest inventories are not accessible to the client via item capability/TileEntity (the contents are only synced via EntityPlayerMP::sendAllContents), the logic for determining how many of a trade you could make had to be moved to the server side.
  2. This resulted in an overhaul of the packets sent from the server to the client to include the max number of times a trade can be made (for UI purposes) instead of calculating it on the client.

I've been testing it quite extensively on my server both with and without an attached inventory (for the past 2 weeks) and it appears to function a) identical to how it did previously, but b) now with the ability to place items in a chest or inventory next to it, and have those included when interacting.

The results of trades are, obviously, still placed in your own inventory.

Review/feedback etc much appreciated. I've also taken the liberty of fixing some of the package-private methods to public/private where relevant.

@noobanidus
Copy link
Contributor Author

@faceofcat This has been thoroughly tested and it can be merged into the main branch. The Codacy complaint is mainly about pre-existing code that I've modified.

@faceofcat
Copy link
Collaborator

ok... added to my calendar to test this and see if I can remember how to upload a new version to curse 😄 ... thanks!

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.

2 participants