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

Fix exact choice recipe book clicks #7822

Merged

Conversation

Machine-Maker
Copy link
Member

@Machine-Maker Machine-Maker commented May 14, 2022

Fixes #7821

Anytime a recipe match is going to be computed by the server to try and place the recipe in the crafting area (grid, furnace, etc.), the recipe ingredients are looked at and any ingredients that are marked as "exact" are noted. When the inventory is accounted and unique int ids are created for each item type, any item that matches the exact ingredient tracked before is given an extra separate unique int id. Then that is used to calculate if the recipe is valid.

This also removes the need for the patch added here. This patch fixed shapeless recipes with exact recipe choices due to it uses the same StackedContents system. But now that that system has been integrated with the ExactChoices, we can just use the same thing there and it works.

Caveats

There are some client-side limitations that are unavoidable as far as I can figure. If you have an exact ingredient that has a custom display name, enchantments or a damage value, the client will show the recipe as red even if you can click on it and it will put the ingredients in the correct slots. This is because the client normally ignores items with names, damage, or enchants when calculating the ingredients for a recipe.


Download the paperclip jar for this pull request: paper-7822.zip

@Machine-Maker Machine-Maker requested a review from a team as a code owner May 14, 2022 22:45
@Boy0000
Copy link
Contributor

Boy0000 commented May 27, 2022

Tried building this locally to test myself but couldn't get it to work?
Atleast if its meant to have the autofill work with custom nbt items.

Recipe still only shows as "craftable" when non-nbt variant is in players inventory.
Clicking it with all the NBT items in the inventory does not autofill (manually placing them makes the recipe work ofc)
Unlike before though, the vanilla equivalent items are not autofilled when they are present

Clip below should show everything, triple checked the nbt of items in inv, the recipe ones and result items.
All were identical.

With patch:

nbt.mp4

Without:

2022-05-27.18-30-56.mp4

@Machine-Maker
Copy link
Member Author

Machine-Maker commented May 27, 2022

Items with damage values, enchantments, or custom display names do not work with auto filling. That isn't something this pr tries to fix. If you have custom nbt that isn't that, like custom lore, attributes, or something else it should work.

This now works with exact recipe choices, but note the caveats listed in the pr description.

@Boy0000
Copy link
Contributor

Boy0000 commented May 27, 2022

Ah alright, kinda sad but seems to work atleast without displayname like intended.
Would this only be when the item registered in the recipe has a displayname?
So if i register the recipe with a clone of the itemstack but remove the displayname it would still work?

also when thinking about it yeah displayname and damage makes sense to not be taken into account 💀
forgot anvil renaming and enchanting was a thing apparently

image

@Machine-Maker
Copy link
Member Author

Machine-Maker commented May 27, 2022

So if i register the recipe with a clone of the itemstack but remove the displayname it would still work?

No. When the game tries to find stacks that match the recipe it just ignores itemstacks in your inventory that have a display name, enchants, or damage. It might be possible to address this in the future, but that's not what this PR does.

This now works, see Caveats listed in pr description.

@partydev
Copy link

partydev commented May 29, 2022

I can confirm that this PR fixes #7821.

Previous behavior was that the correct recipe would not be sent to the player if it was marked as craftable. The material for the items in that recipe are sticks, so the recipe is seen as craftable by the client.

image

EDIT:

After 24 hours of testing, including running it on production, I have not seen any side effects from this PR's changes, and that the issue is fixed. I will continue posting updates.

EDIT:

After a a few days of production use, no issues stemming from this.

granny added a commit to PurpurMC/Purpur that referenced this pull request Jul 15, 2022
github-actions bot pushed a commit to SharkurMC/Sharkur that referenced this pull request Jul 15, 2022
Upstream has released updates that appear to apply and compile correctly

Purpur Changes:
PurpurMC/Purpur@db9c5ad port PaperMC/Paper#7822
github-actions bot pushed a commit to PurpurMC/Tentacles that referenced this pull request Jul 15, 2022
Upstream has released updates that appear to apply and compile correctly

Purpur Changes:
PurpurMC/Purpur@db9c5ad port PaperMC/Paper#7822
github-actions bot pushed a commit to TheBigBrainProject/SussyEmerald that referenced this pull request Jul 15, 2022
Upstream has released updates that appear to apply and compile correctly

Purpur Changes:
PurpurMC/Purpur@db9c5ad port PaperMC/Paper#7822
github-actions bot pushed a commit to DaRacci/Tentacles that referenced this pull request Jul 15, 2022
Upstream has released updates that appear to apply and compile correctly

Purpur Changes:
PurpurMC/Purpur@db9c5ad port PaperMC/Paper#7822
SoSeDiK added a commit to SoSeDiKs-Universe/Kiterino that referenced this pull request Jul 20, 2022
Upstream has released updates that appear to apply and compile correctly

Purpur Changes:
PurpurMC/Purpur@0d7f296 [ci-skip] Update readme
PurpurMC/Purpur@7ffeb29 [ci-skip] Update readme (again)
PurpurMC/Purpur@2b07212 [ci-skip] update paper repo url
PurpurMC/Purpur@4044790 Updated Upstream (Paper)
PurpurMC/Purpur@44b914c Add toggle for RNG manipulation (Closes #1051)
PurpurMC/Purpur@3c8861e [ci-skip] Cleanup patch diff
PurpurMC/Purpur@d84fba0 Updated Upstream (Paper)
PurpurMC/Purpur@31b2f4d send the client custom name of block entities
PurpurMC/Purpur@38f2246 Updated Upstream (Paper)
PurpurMC/Purpur@db9c5ad port PaperMC/Paper#7822
PurpurMC/Purpur@8aa5c25 Chat Preview API (#1050)
PurpurMC/Purpur@b5fd044 Updated Upstream (Paper & Pufferfish)
PurpurMC/Purpur@4951792 Updated Upstream (Paper)
PurpurMC/Purpur@278bb53 Fix #1060
PurpurMC/Purpur@713c7b5 dont send empty messages
PurpurMC/Purpur@ebfdfe6 Updated Upstream (Paper)
@stale
Copy link

stale bot commented Jul 31, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@Machine-Maker
Copy link
Member Author

I wonder if there's some better way to track items like this, via some registering system for plugins to say, "This item should be treated separately in recipes". Right now, every unique nbt tag on an itemstack will create a separate id, and that could get rather big as there's really infinite nbt combinations.

@Machine-Maker
Copy link
Member Author

Dramatically refactored this to an, imo, much better and more extensible system for predicate choice coming later. Details of how this works in PR description.

@Machine-Maker Machine-Maker force-pushed the fix/exact-choice-recipe-book branch 2 times, most recently from 1ca96d8 to 19c8790 Compare June 26, 2023 20:35
@Machine-Maker Machine-Maker added the build-pr-jar Enables a workflow to build Paperclip jars on the pull request. label Jul 11, 2023
@Machine-Maker Machine-Maker force-pushed the fix/exact-choice-recipe-book branch 2 times, most recently from 67eb373 to 2d5ec48 Compare July 11, 2023 19:44
@Machine-Maker Machine-Maker force-pushed the fix/exact-choice-recipe-book branch 4 times, most recently from 4d7dc46 to 663f326 Compare July 12, 2023 18:05
@Machine-Maker Machine-Maker merged commit b8a0049 into PaperMC:master Aug 23, 2023
1 check passed
@Machine-Maker Machine-Maker deleted the fix/exact-choice-recipe-book branch August 23, 2023 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-pr-jar Enables a workflow to build Paperclip jars on the pull request.
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

Recipe Book clicking only matches material when placing items into the crafting grid
5 participants