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 an 'empty' RecipeChoice for certain recipes and ingredient slots #10710

Merged

Conversation

Machine-Maker
Copy link
Member

@Machine-Maker Machine-Maker commented May 12, 2024

Adds RecipeChoice#empty which is allowed in specific recipes and ingredient slots.


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

@Machine-Maker Machine-Maker requested a review from a team as a code owner May 12, 2024 22:48
@Machine-Maker Machine-Maker force-pushed the feature/RecipeChoice#empty branch 2 times, most recently from 7effa1c to c12cad7 Compare May 12, 2024 23:09
@MiniDigger MiniDigger added the build-pr-jar Enables a workflow to build Paperclip jars on the pull request. label May 13, 2024
@tal5
Copy link
Contributor

tal5 commented May 13, 2024

There's still an issue with the Preconditions.checkArgument(!result.isEmpty(), "Recipe cannot have an empty result.") check in the SmithingRecipe constructor failing for some vanilla recipes, which can be replicated by just going over them, e.g. getServer().recipeIterator().forEachRemaining(recipe -> getLogger().info("")).
Not super familiar with the recipe system, but from what I can see only SmithingTransformRecipes actually have a result, and SmithingTrimRecipes just pass in an air ItemStack which triggers that check (I.e. can probably just move the check into SmithingTransformRecipe?)

@Machine-Maker
Copy link
Member Author

There's still an issue with the Preconditions.checkArgument(!result.isEmpty(), "Recipe cannot have an empty result.")

Yeah, fixed that directly on master.

It did make me think about another possible error, and it turns out that upstream was just gonna insert null into RecipeChoice parameters in constructors if the nms Ingredient was empty. And that was gonna error. Now no vanilla recipes do that, but recipes added via datapacks surely could. So I've updated this PR to correctly handle converting an nms Ingredient back into an empty RecipeChoice. I also added a test that just runs the conversion for all recipes on the server which should catch changes like this in the future.

masmc05 added a commit to masmc05/Paper that referenced this pull request May 13, 2024
patches/api/0479-Fix-issues-with-recipe-API.patch Outdated Show resolved Hide resolved
patches/api/0479-Fix-issues-with-recipe-API.patch Outdated Show resolved Hide resolved
@Machine-Maker Machine-Maker merged commit 7d2e5c3 into PaperMC:master May 20, 2024
3 checks passed
@Machine-Maker Machine-Maker deleted the feature/RecipeChoice#empty branch May 20, 2024 14: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.

None yet

4 participants