-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Adding PredicateChoice to Paper API (updated version) #12017
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to paper 🎉
Thank you for updating the PR. I don'T have time for a indepth review rn, but I left some initial easy nitpicks around comments 👍
Will get to a review next week probably
paper-server/patches/sources/net/minecraft/world/item/crafting/Ingredient.java.patch
Outdated
Show resolved
Hide resolved
paper-server/src/main/java/org/bukkit/craftbukkit/inventory/CraftRecipe.java
Outdated
Show resolved
Hide resolved
Hi and welcome to the Paper PR world (well this more a thing can say a Paper Team Member xd) The paper comments currently just apply to code in patches, its not required in the api,craftbukkit,paper classes. |
I just noticed there is already a PredicateRecipeChoice for potions, with the addition of the PredicateChoice this could be a bit confusing. Maybe it should be renamed to something more clear like a PotionRecipeChoice/PotionPredicateChoice? |
well rename sounds a little breaking i think, maybe this new PredicateChoice can be named ItemPredicateChoice (? or add an extension of RecipeChoice for Potion and move things to that for make clear the diff... but not really sure. |
marked stackPredicate field in Ingredient as Nullable
Renamed field predicate to stackPredicate
This PR is an updated version of derverdox's PredicateChoice PR #9996. Since the branch in the old PR was quite outdated I reïmplemented the changes in a new branch, but most of the changes remain the same. Credit of the code should go to derverdox and Machine-Maker.
The feature seems to work fine from my testing but I'm unfamiliar with the Paper API so I please let me know if I missed something.