Fix #1822: Return default remainder when recipe is null
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.
https://github.com/user-attachments/assets/e44e56f6-334d-459b-8283-7364af82f55f
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))
https://github.com/user-attachments/assets/265c962b-ddb8-489b-a4cf-b4c155e3f45c
fixes #1822
thank you
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.
@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?