Fix NPE when non-BlockEntity calls `AbstractFurnaceBlockEntity#craftR…

…ecipe` (#339)

* Workaround for non-BEs calling static furnace craftRecipe method

* Don't accept recipe output if a remainder can't be dropped

* Fix threadlocal not being unset after usage

* Address requested changes

* Fix off by one

---------

Co-authored-by: Ennui Langeweile <[email protected]>

library/item/item_setting/src/main/java/org/quiltmc/qsl/item/setting/api/RecipeRemainderLogicHandler.java

@@ -16,6 +16,8 @@
 
 package org.quiltmc.qsl.item.setting.api;
 
+import java.util.function.Consumer;
+
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.Nullable;
@@ -24,6 +26,7 @@
 import net.minecraft.item.ItemStack;
 import net.minecraft.recipe.Recipe;
 import net.minecraft.screen.slot.Slot;
+import net.minecraft.util.ItemScatterer;
 import net.minecraft.util.collection.DefaultedList;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.world.World;
@@ -67,7 +70,23 @@ static ItemStack getRemainder(ItemStack original, @Nullable Recipe<?> recipe) {
 	 */
 	@Contract(mutates = "param1, param4, param6")
 	static void handleRemainderForNonPlayerCraft(ItemStack input, int amount, @Nullable Recipe<?> recipe, DefaultedList<ItemStack> inventory, int index, World world, BlockPos location) {
-		RecipeRemainderLogicHandlerImpl.handleRemainderForNonPlayerCraft(input, amount, recipe, inventory, index, world, location);
+		handleRemainderForNonPlayerCraft(input, amount, recipe, inventory, index, remainder -> ItemScatterer.spawn(world, location.getX(), location.getY(), location.getZ(), remainder));
+	}
+
+	/**
+	 * Handles the recipe remainder logic for crafts without a {@link PlayerEntity player} present.
+	 * Excess items that cannot be returned to a slot are handled by the provided {@link Consumer consumer}.
+	 *
+	 * @param input the original item stack
+	 * @param amount the amount by which to decrease the stack
+	 * @param recipe the recipe being used
+	 * @param inventory the inventory
+	 * @param index the index of the original stack in the inventory
+	 * @param failure callback that is run if excess items could not be returned to a slot
+	 */
+	@Contract(mutates = "param1, param4, param6")
+	static void handleRemainderForNonPlayerCraft(ItemStack input, int amount, @Nullable Recipe<?> recipe, DefaultedList<ItemStack> inventory, int index, Consumer<ItemStack> failure) {
+		RecipeRemainderLogicHandlerImpl.handleRemainderForNonPlayerCraft(input, amount, recipe, inventory, index, failure);
 	}
 
 	/**

library/item/item_setting/src/main/java/org/quiltmc/qsl/item/setting/impl/RecipeRemainderLogicHandlerImpl.java

@@ -16,6 +16,8 @@
 
 package org.quiltmc.qsl.item.setting.impl;
 
+import java.util.function.Consumer;
+
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.Nullable;
@@ -24,7 +26,6 @@
 import net.minecraft.item.ItemStack;
 import net.minecraft.recipe.Recipe;
 import net.minecraft.screen.slot.Slot;
-import net.minecraft.util.ItemScatterer;
 import net.minecraft.util.collection.DefaultedList;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.world.World;
@@ -92,11 +93,11 @@ private static ItemStack decrementWithRemainder(ItemStack original, int amount,
 	}
 
 	@Contract(mutates = "param1, param4, param6")
-	public static void handleRemainderForNonPlayerCraft(ItemStack input, int amount, @Nullable Recipe<?> recipe, DefaultedList<ItemStack> inventory, int index, World world, BlockPos location) {
+	public static void handleRemainderForNonPlayerCraft(ItemStack input, int amount, @Nullable Recipe<?> recipe, DefaultedList<ItemStack> inventory, int index, Consumer<ItemStack> failure) {
 		ItemStack remainder = decrementWithRemainder(input, amount, recipe);
 
 		if (!tryReturnItemToInventory(remainder, inventory, index)) {
-			ItemScatterer.spawn(world, location.getX(), location.getY(), location.getZ(), remainder);
+			failure.accept(remainder);
 		}
 	}
 

library/item/item_setting/src/main/java/org/quiltmc/qsl/item/setting/mixin/recipe_remainder/AbstractFurnaceBlockEntityMixin.java

@@ -24,6 +24,7 @@
 import org.spongepowered.asm.mixin.injection.At;
 import org.spongepowered.asm.mixin.injection.Inject;
 import org.spongepowered.asm.mixin.injection.Redirect;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
 import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
 
 import net.minecraft.block.BlockState;
@@ -37,6 +38,7 @@
 import net.minecraft.recipe.Recipe;
 import net.minecraft.recipe.RecipeManager;
 import net.minecraft.registry.DynamicRegistryManager;
+import net.minecraft.util.ItemScatterer;
 import net.minecraft.util.collection.DefaultedList;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.world.World;
@@ -70,6 +72,29 @@ private void setThreadLocalBlockEntity(CallbackInfoReturnable<Boolean> cir) {
 		quilt$THREAD_LOCAL_BLOCK_ENTITY.set((AbstractFurnaceBlockEntity) (BlockEntity) this);
 	}
 
+	// prevent additional smelting if remainder item overflow would have no location to be dropped into the world
+	@SuppressWarnings("ConstantConditions")
+	@Inject(method = "canAcceptRecipeOutput", at = @At("RETURN"), cancellable = true)
+	private static void checkMismatchedRemaindersCanDrop(DynamicRegistryManager registryManager, @Nullable Recipe<?> recipe, DefaultedList<ItemStack> inventory, int count, CallbackInfoReturnable<Boolean> cir) {
+		if (cir.getReturnValue() && quilt$THREAD_LOCAL_BLOCK_ENTITY.get() == null) {
+			ItemStack original = inventory.get(INPUT_SLOT).copy();
+
+			if (!original.isEmpty()) {
+				ItemStack remainder = RecipeRemainderLogicHandler.getRemainder(original, recipe).copy();
+				original.decrement(1);
+
+				if (!remainder.isEmpty() && ItemStack.canCombine(original, remainder)) {
+					int toTake = Math.min(original.getMaxCount() - original.getCount(), remainder.getCount());
+					remainder.decrement(toTake);
+
+					if (!remainder.isEmpty()) {
+						cir.setReturnValue(false);
+					}
+				}
+			}
+		}
+	}
+
 	@SuppressWarnings("ConstantConditions")
 	@Redirect(method = "tick", at = @At(value = "INVOKE", target = "Lnet/minecraft/item/ItemStack;decrement(I)V"))
 	private static void setFuelRemainder(ItemStack fuelStack, int amount, World world, BlockPos pos, BlockState state, AbstractFurnaceBlockEntity blockEntity) {
@@ -106,8 +131,17 @@ private static void setInputRemainder(ItemStack inputStack, int amount, DynamicR
 				recipe,
 				inventory,
 				INPUT_SLOT,
-				quilt$THREAD_LOCAL_BLOCK_ENTITY.get().getWorld(),
-				quilt$THREAD_LOCAL_BLOCK_ENTITY.get().getPos()
+				remainder -> { // consumer only called when there are excess remainder items that can be dropped into the world
+					// block entity guaranteed not to be null here thanks to checks in canAcceptRecipeOutput injection above
+					AbstractFurnaceBlockEntity be = quilt$THREAD_LOCAL_BLOCK_ENTITY.get();
+					BlockPos location = be.getPos();
+					ItemScatterer.spawn(be.getWorld(), location.getX(), location.getY(), location.getZ(), remainder);
+				}
 		);
 	}
+
+	@Inject(method = "tick", at = @At("RETURN"))
+	private static void resetThreadLocalBlockEntity(World world, BlockPos pos, BlockState state, AbstractFurnaceBlockEntity blockEntity, CallbackInfo ci) {
+		quilt$THREAD_LOCAL_BLOCK_ENTITY.remove();
+	}
 }