Merge pull request #770 from pylonmc/human/furnace-cache-fix

Furance Recipe Cache Fix

Author: LordIdra

nms/src/main/kotlin/io/github/pylonmc/rebar/nms/NmsAccessorImpl.kt

@@ -8,6 +8,7 @@ import io.github.pylonmc.rebar.entity.packet.BlockTextureEntity
 import io.github.pylonmc.rebar.i18n.PlayerTranslationHandler
 import io.github.pylonmc.rebar.i18n.packet.PlayerPacketHandler
 import io.github.pylonmc.rebar.nms.entity.BlockTextureEntityImpl
+import io.github.pylonmc.rebar.nms.recipe.AccessibleCachedCheck
 import io.github.pylonmc.rebar.nms.recipe.HandlerRecipeBookClick
 import io.github.pylonmc.rebar.util.position.BlockPosition
 import io.papermc.paper.adventure.PaperAdventure
@@ -25,14 +26,16 @@ import net.minecraft.server.MinecraftServer
 import net.minecraft.world.inventory.AbstractCraftingMenu
 import net.minecraft.world.inventory.RecipeBookMenu.PostPlaceAction
 import net.minecraft.world.item.Item
+import net.minecraft.world.item.crafting.RecipeManager
+import net.minecraft.world.level.block.entity.AbstractFurnaceBlockEntity
 import net.minecraft.world.level.block.state.properties.Property
 import org.bukkit.Material
+import org.bukkit.NamespacedKey
 import org.bukkit.World
 import org.bukkit.block.Block
 import org.bukkit.craftbukkit.CraftEquipmentSlot
 import org.bukkit.craftbukkit.CraftWorld
 import org.bukkit.craftbukkit.block.CraftBlock
-import org.bukkit.craftbukkit.entity.CraftEntity
 import org.bukkit.craftbukkit.entity.CraftLivingEntity
 import org.bukkit.craftbukkit.entity.CraftPlayer
 import org.bukkit.craftbukkit.inventory.CraftItemStack
@@ -45,6 +48,10 @@ import org.bukkit.entity.Player
 import org.bukkit.inventory.EquipmentSlot
 import org.bukkit.inventory.ItemStack
 import org.bukkit.persistence.PersistentDataContainer
+import java.lang.invoke.MethodHandle
+import java.lang.invoke.MethodHandles
+import java.lang.invoke.VarHandle
+import java.lang.reflect.Field
 import java.util.UUID
 import java.util.concurrent.ConcurrentHashMap
 import kotlin.coroutines.EmptyCoroutineContext
@@ -56,6 +63,25 @@ import net.minecraft.world.entity.EquipmentSlot as NmsEquipmentSlot
 @Suppress("unused")
 object NmsAccessorImpl : NmsAccessor {
 
+    // We use both the field and the handle because the handle will have significantly better performance
+    // getting the field value but cannot be used for setting so we still need the raw field.
+    // (even if we used a VarHandle, because the field is normally final, setting will not work)
+    private val furnaceQuickCheckField: Field
+    private val furnaceQuickCheckHandle: MethodHandle
+
+    init {
+        try {
+            furnaceQuickCheckField = AbstractFurnaceBlockEntity::class.java.getDeclaredField("quickCheck")
+            furnaceQuickCheckField.isAccessible = true
+
+            val methodHandles = MethodHandles.privateLookupIn(AbstractFurnaceBlockEntity::class.java, MethodHandles.lookup())
+            furnaceQuickCheckHandle = methodHandles.unreflectGetter(furnaceQuickCheckField)
+        } catch (e: Throwable) {
+            Rebar.logger.severe("Failed to access furnace quick check: ${e.message}")
+            throw RuntimeException(e)
+        }
+    }
+
     private val players = ConcurrentHashMap<UUID, PlayerPacketHandler>()
 
     override fun damageItem(itemStack: ItemStack, amount: Int, world: World, onBreak: (Material) -> Unit, force: Boolean) {
@@ -199,4 +225,22 @@ object NmsAccessorImpl : NmsAccessor {
         }
         blocks
     }
+
+    override fun setFurnaceRecipeCache(block: Block, recipe: NamespacedKey) {
+        val block = block as CraftBlock
+        val blockEntity = block.level.getBlockEntity(block.position) as? AbstractFurnaceBlockEntity ?: return
+        try {
+            val currentQuickCheck = furnaceQuickCheckHandle.invoke(blockEntity) as? RecipeManager.CachedCheck<*, *> ?: return
+            if (currentQuickCheck is AccessibleCachedCheck<*, *>) {
+                currentQuickCheck.lastRecipe = CraftNamespacedKey.toResourceKey(Registries.RECIPE, recipe)
+            } else {
+                val newQuickCheck = AccessibleCachedCheck(blockEntity.recipeType)
+                newQuickCheck.lastRecipe = CraftNamespacedKey.toResourceKey(Registries.RECIPE, recipe)
+                furnaceQuickCheckField.set(blockEntity, newQuickCheck)
+            }
+        } catch (e: Throwable) {
+            Rebar.logger.severe("Failed to set furnace recipe cache: ${e.message}")
+            e.printStackTrace()
+        }
+    }
 }

nms/src/main/kotlin/io/github/pylonmc/rebar/nms/recipe/AccessibleCachedCheck.kt

@@ -0,0 +1,33 @@
+package io.github.pylonmc.rebar.nms.recipe
+
+import net.minecraft.resources.ResourceKey
+import net.minecraft.server.level.ServerLevel
+import net.minecraft.world.item.crafting.Recipe
+import net.minecraft.world.item.crafting.RecipeHolder
+import net.minecraft.world.item.crafting.RecipeInput
+import net.minecraft.world.item.crafting.RecipeManager
+import net.minecraft.world.item.crafting.RecipeType
+import java.util.Optional
+
+/**
+ * A dedicated class for [RecipeManager.CachedCheck] that allows changing the lastRecipe field
+ * This is effectively identical to the result of [RecipeManager.createCheck] except it's not
+ * an anonymous class
+ *
+ * This is necessary to avoid using laggy reflection on the anonymous class provided by [RecipeManager.createCheck]
+ */
+class AccessibleCachedCheck<I : RecipeInput, T : Recipe<I>>(
+    val type: RecipeType<T>,
+    var lastRecipe: ResourceKey<Recipe<*>>? = null
+) : RecipeManager.CachedCheck<I, T> {
+
+    override fun getRecipeFor(input: I, level: ServerLevel): Optional<RecipeHolder<T>> {
+        val recipeManager = level.recipeAccess()
+        val result = recipeManager.getRecipeFor(type, input, level, this.lastRecipe)
+        if (result.isPresent) {
+            this.lastRecipe = result.get().id
+        }
+        return result
+    }
+
+}

rebar/src/main/kotlin/io/github/pylonmc/rebar/nms/NmsAccessor.kt

@@ -7,6 +7,7 @@ import io.github.pylonmc.rebar.i18n.PlayerTranslationHandler
 import io.github.pylonmc.rebar.util.position.BlockPosition
 import net.kyori.adventure.text.Component
 import org.bukkit.Material
+import org.bukkit.NamespacedKey
 import org.bukkit.World
 import org.bukkit.block.Block
 import org.bukkit.entity.Entity
@@ -57,6 +58,28 @@ interface NmsAccessor {
 
     fun blocksBetween(from: BlockPosition, to: BlockPosition): List<Block>
 
+    /**
+     * Furnaces have a recipe cache of the last recipe smelted, this is great
+     * for performance but has an unexpected side effect once rebar items are introduced.
+     *
+     * Lets say you have a furnace with nothing cached and you put in a raw tin (rebar item) to smelt.
+     * The furnace is going to search for a recipe that matches and there are 2 possible choices
+     * 1. the raw tin -> tin ingot recipe, supplied by rebar using exact choice
+     * 2. the raw iron -> iron ingot recipe, supplied by Minecraft, using material choice, so it validates even if the item is a rebar item
+     *
+     * Even if it picks the vanilla recipe, when it starts smelting & finishes smelting, rebar will ensure
+     * the rebar recipe is used instead.
+     *
+     * However, the furnace itself has no way of knowing that we changed which recipe was used, so if the vanilla recipe was used
+     * it will cache the vanilla recipe. Then, when it tries to start smelting another raw tin, it will check that it can fit
+     * the result of the next valid recipe, which is evaluated to the cached vanilla recipe, and it will find that it can't, because a tin ingot
+     * is in the result slot, not the iron ingot. This then deadlocks the furnace in a feedback loop of use the vanilla recipe, oh can't fit it.
+     *
+     * In order to avoid this, whenever we override the recipe being used, we set the recipe in the recipe cache so that next time
+     * the furnace smelts, it uses the correct recipe and doesn't get deadlocked.
+     */
+    fun setFurnaceRecipeCache(block: Block, recipe: NamespacedKey)
+
     companion object {
         val instance = Class.forName("io.github.pylonmc.rebar.nms.NmsAccessorImpl")
             .getDeclaredField("INSTANCE")

rebar/src/main/kotlin/io/github/pylonmc/rebar/recipe/RecipeListener.kt

@@ -4,9 +4,11 @@ import io.github.pylonmc.rebar.item.RebarItem
 import io.github.pylonmc.rebar.item.RebarItemSchema
 import io.github.pylonmc.rebar.item.base.*
 import io.github.pylonmc.rebar.item.research.Research.Companion.canCraft
+import io.github.pylonmc.rebar.nms.NmsAccessor
 import io.github.pylonmc.rebar.recipe.vanilla.CookingRecipeWrapper
 import io.github.pylonmc.rebar.recipe.vanilla.ShapedRecipeType
 import io.github.pylonmc.rebar.recipe.vanilla.VanillaRecipeType
+import io.github.pylonmc.rebar.recipe.vanilla.recipeType
 import io.github.pylonmc.rebar.util.isRebarAndIsNot
 import io.github.pylonmc.rebar.util.plainText
 import io.papermc.paper.datacomponent.DataComponentTypes
@@ -189,13 +191,55 @@ internal object RebarRecipeListener : Listener {
         }
     }
 
+    @EventHandler(priority = EventPriority.LOWEST)
+    private fun onStartCook(e: FurnaceStartSmeltEvent) {
+        if (RebarItemSchema.fromStack(e.source) == null) return
+
+        val originalRecipe = e.recipe
+        if (originalRecipe.key !in VanillaRecipeType.nonRebarRecipes) {
+            return
+        }
+
+        val originalType = originalRecipe.recipeType
+        if (originalType == null) {
+            e.totalCookTime = 0 // instantly complete so that it doesn't show progress bar, this will get canceled in BlockCookEvent
+            return
+        }
+
+        for (recipe in originalType.recipes) {
+            if (recipe is CookingRecipeWrapper && recipe.key !in VanillaRecipeType.nonRebarRecipes && recipe.recipe.inputChoice.test(e.source)) {
+                e.totalCookTime = recipe.recipe.cookingTime
+                NmsAccessor.instance.setFurnaceRecipeCache(e.block, recipe.key)
+                break
+            }
+        }
+    }
+
     @EventHandler(priority = EventPriority.LOWEST)
     private fun onCook(e: BlockCookEvent) {
         if (RebarItemSchema.fromStack(e.source) == null) return
 
-        for (recipe in RecipeType.vanillaCookingRecipes()) {
-            if (recipe.key !in VanillaRecipeType.nonRebarRecipes && recipe.recipe.inputChoice.test(e.source)) {
-                e.result = recipe.recipe.result.clone()
+        val originalRecipe = e.recipe
+        if (originalRecipe == null) {
+            e.isCancelled = true
+            return
+        }
+
+        if (originalRecipe.key !in VanillaRecipeType.nonRebarRecipes) {
+            // already handled correctly
+            return
+        }
+
+        val originalType = originalRecipe.recipeType
+        if (originalType == null) {
+            e.isCancelled = true
+            return
+        }
+
+        for (recipe in originalType.recipes) {
+            if (recipe is CookingRecipeWrapper && recipe.key !in VanillaRecipeType.nonRebarRecipes && recipe.recipe.inputChoice.test(e.source)) {
+                e.result = recipe.recipe.result
+                NmsAccessor.instance.setFurnaceRecipeCache(e.block, recipe.key)
                 break
             }
         }

rebar/src/main/kotlin/io/github/pylonmc/rebar/recipe/vanilla/VanillaRecipeType.kt

@@ -5,11 +5,17 @@ import io.github.pylonmc.rebar.item.ItemTypeWrapper
 import io.github.pylonmc.rebar.recipe.ConfigurableRecipeType
 import io.github.pylonmc.rebar.recipe.RebarRecipe
 import io.github.pylonmc.rebar.recipe.RecipeInput
+import io.github.pylonmc.rebar.recipe.RecipeType
 import org.bukkit.Bukkit
 import org.bukkit.NamespacedKey
 import org.bukkit.event.Listener
+import org.bukkit.inventory.BlastingRecipe
+import org.bukkit.inventory.CampfireRecipe
+import org.bukkit.inventory.CookingRecipe
+import org.bukkit.inventory.FurnaceRecipe
 import org.bukkit.inventory.Recipe
 import org.bukkit.inventory.RecipeChoice
+import org.bukkit.inventory.SmokingRecipe
 
 sealed interface VanillaRecipeWrapper : RebarRecipe {
     val recipe: Recipe
@@ -68,3 +74,13 @@ internal fun RecipeChoice.asRecipeInput(): RecipeInput {
 internal fun RecipeInput.Item.asRecipeChoice(): RecipeChoice {
     return RecipeChoice.ExactChoice(items.map { it.createItemStack().asQuantity(amount) })
 }
+
+@get:JvmSynthetic
+val CookingRecipe<*>.recipeType: RecipeType<*>?
+    get() = when (this) {
+        is BlastingRecipe -> BlastingRecipeType
+        is CampfireRecipe -> CampfireRecipeType
+        is FurnaceRecipe -> FurnaceRecipeType
+        is SmokingRecipe -> SmokingRecipeType
+        else -> null
+    }