8000 Fix #1822: Return default remainder when recipe is null by Dqu1J · Pull Request #12367 · PaperMC/Paper · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix #1822: Return default remainder when recipe is null #12367

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix #1822: Return default remainder when recipe is null #12367

wants to merge 1 commit into from

Conversation

Dqu1J
Copy link
Contributor
@Dqu1J Dqu1J commented Mar 31, 2025 •
edited
Loading

The bug

If the result slot of a crafting grid is taken, and no recipe exists for the craft, the input materials get duplicated.

This happens when, for example, PrepareItemCraftEvent is used to override the result slot using inventory.setResult(). That is useful for creating dynamic recipes, like dye mixing, something that isn't yet possible to do with regular json recipes.

Screen.Recording.mov

Why it happens

The issue is in the ResultSlot#getRemainingItems method, which gets called when the craft result is taken. It is used to add remainder items to the crafting grid (e.g. water bucket -> bucket).

private NonNullList<ItemStack> getRemainingItems(CraftingInput input, Level level) {
    return level instanceof ServerLevel serverLevel
        ? serverLevel.recipeAccess()
            .getRecipeFor(RecipeType.CRAFTING, input, serverLevel, this.craftSlots.getCurrentRecipe()) // Paper - Perf: Improve mass crafting; check last recipe used first
            .map(recipe -> recipe.value().getRemainingItems(input))
            .orElseGet(() -> copyAllInputItems(input))
        : CraftingRecipe.defaultCraftingReminder(input);
}

That's where the dupe happens. A craft is possible, since a result slot was set in PrepareItemCraftEvent. However there's no actual recipe, so getRemainingItems returns a copy of the input items.

In the onTake method, those remainder items get added to the crafting grid. If there's already an item with the same components, it will simply add to its quantity, resulting it duplication.

And I honestly have no idea why it returns a copy of input items in that case. As far as I know, in vanilla that would never be the case. It's not possible to take an item from the result slot when there is no recipe.

How this PR fixes it

Instead of returning the input items when recipe is null, the default crafting remainder for the items should be returned.

-            .orElseGet(() -> copyAllInputItems(input))
+            .orElseGet(() -> CraftingRecipe.defaultCraftingReminder(input))
Screen.Recording.1.mov

fixes #1822

thank you

@Dqu1J Dqu1J requested a review from a team as a code owner March 31, 2025 13:26
@github-project-automation github-project-automation bot moved this to Awaiting review in Paper PR Queue Mar 31, 2025
@Dqu1J Dqu1J changed the title Fix #1822: Return default remainder when recipe is null Fix #1822 : Return default remainder when recipe is null Mar 31, 2025
@Dqu1J Dqu1J changed the title Fix #1822 : Return default remainder when recipe is null Fix #1822: Return default remainder when recipe is null Mar 31, 2025
@notTamion notTamion added the type: bug Something doesn't work as it was intended to. label Mar 31, 2025
@Owen1212055
Copy link
Member

I'm not quite sure about this, this makes sense in the case that there is no recipe but wouldn't this affect normal people using the crafting table?

Cause there is no "crafting" occurring if there is no recipe but changing this logic would make it seem like one is happening.

I am not sure what the best way to fix this is, maybe to insert some kind of fake recipe? idk.

@Dqu1J
Copy link
Contributor Author
Dqu1J commented May 1, 2025 •
edited
Loading

@Owen1212055 Sorry, I didn't really understand you. This doesn't affect people using the crafting table:

  • Remainder is calculated only when the craft can happen — in vanilla, only when there is a recipe —> vanilla behaviour under this PR
  • Event used to override result slot — craft can occur, BUT no recipe exists —> PR returns default remainder (remainder is stuff like converting water bucket -> bucket) rather than the slot contents (resulting in duplication) as the remainder

It does not make it seem like "there was a craft even though there wasn't". The PR only modifies the remainder returned when recipe is null, not the whole logic behind it and crafting?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
type: bug Something doesn't work as it was intended to.
Projects
Paper PR Queue
Status: Awaiting review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Items in crafting table get duplicated if result slot is set manually
3 participants
@Dqu1J @Owen1212055 @notTamion
0