[MC1.12.1-1.7.0.11] Robot crafting destroys item recipe after crafting

[SirDavidLudwig]

If you craft an item with a robot, that recipe's output item will be replaced with air, breaking the recipe until the server/client is restarted. I tested this with Forge 1.12.1-2485 and with the latest dev build of OpenComputers MC1.12.1-1.7.0.11 and no other mods.

[stridervc]

I just encountered this bug as well. Minecraft 1.12.2, OpenComputers-MC1.12.1-1.7.0.20.
The craft works once, after that, the recipe is invalid, even for players.

[SirDavidLudwig]

I ended up going into the code and adding a cheap fix for it myself. Hopefully they'll get around to properly fixing it soon.

[payonel]

@SirDavidLudwig did you want to share you fix?

[SirDavidLudwig]

@payonel I can share my fix, but I'm not certain if what I did is proper or not as I'm not much of a mod developer.

[payonel]

@SirDavidLudwig I'd still like to see your fix! Thanks :)

[SirDavidLudwig]

@payonel Sorry for the late reply, been busy with University. My fix is simply creating a copy of the item stack when passing it to the other methods to prevent any manipulating the direct reference.

Specifically, this is what I changed in ~/src/main/scala/li/cil/oc/server/component/UpgradeCrafting.scala:

// Line 66 down I think
while (countCrafted < wantedCount) {
    val result = CraftingManager.findMatchingRecipe(CraftingInventory, host.world)
    if (result == null || result.getRecipeOutput.getCount < 1) break ()
    countCrafted += result.getRecipeOutput.getCount
    // Create the copy here, you don't necessarily have to store it, 
    // I just didn't want to create multiple copies
    var output = result.getRecipeOutput.copy()
    FMLCommonHandler.instance.firePlayerCraftingEvent(host.player, output, this) // Use it here
    val surplus = mutable.ArrayBuffer.empty[ItemStack]
    for (slot < -0 until getSizeInventory) {
        val stack = getStackInSlot(slot)
        if (!stack.isEmpty) {
            decrStackSize(slot, 1)
            val item = stack.getItem
            if (item.hasContainerItem(stack)) {
                val container = item.getContainerItem(stack)
                if (container.isItemStackDamageable && container.getItemDamage > container.getMaxDamage) {
                    MinecraftForge.EVENT_BUS.post(new PlayerDestroyItemEvent(host.player, container, null))
                } else if (!getStackInSlot(slot).isEmpty) {
                    surplus += container
                } else {
                    setInventorySlotContents(slot, container)
                }
            }
        }
    }
    save()
    InventoryUtils.addToPlayerInventory(output, host.player) // And here
    for (stack < -surplus) {
        InventoryUtils.addToPlayerInventory(stack, host.player)
    }
    load()
}

[payonel]

Thank you

[neumond]

Thanks @SirDavidLudwig, fix seems to be working.

Regarding the bug itself, probably it breaks only with recipes which produce multiple items, like iron nuggets out of an ingot.

[payonel]

@neumond are you saying this fix breaks crafting iron->iron nuggets?

[neumond]

No, reverse. Fix makes it work.

Some recipes do work even with unpatched mod, but I can clearly remember that I got problems with recipes producing multiple outputs, like chips, nuggets. First craft goes successfully, but next crafts yield no result. OC API says that craft was successful and produced 0 items (true, 0). Then, as @stridervc noticed, even player can't craft that recipe.

After applying fix I've made several successful crafts of iron->iron nuggets, using robot and player inventory, and it still works fine.

[payonel]

generally problems we see in 1.12 repro in 1.11
if anyone has time could you test repro on earlier versions? (we only support 1.7.10, 1.10, 1.11, and 1.12)

[thiakil]

@payonel yeah net.minecraft.item.crafting.IRecipe#getCraftingResult should always be copied (all versions) if you're doing anything more than checking what it is; most implementations just return the instance's field directly.

[payonel]

fixed in #2683