Make upgrade recipe requirements a little more lax

- Move some common upgrade code to IUpgradeBase. 99% sure this this
   preserves binary compatibility (on the JVM at least).

 - Instead of requiring the share tag to match, allow upgrades to
   specify their own predicate. IMO this is a little ugly, but required
   to fix #614 as other mods chuck their own NBT on items.

src/main/java/dan200/computercraft/api/IUpgradeBase.java

@@ -0,0 +1,81 @@
+package dan200.computercraft.api;
+
+import dan200.computercraft.api.pocket.IPocketUpgrade;
+import dan200.computercraft.api.turtle.ITurtleUpgrade;
+import net.minecraft.item.ItemStack;
+import net.minecraft.nbt.CompoundNBT;
+import net.minecraft.util.ResourceLocation;
+
+import javax.annotation.Nonnull;
+
+/**
+ * Common functionality between {@link ITurtleUpgrade} and {@link IPocketUpgrade}.
+ */
+public interface IUpgradeBase
+{
+    /**
+     * Gets a unique identifier representing this type of turtle upgrade. eg: "computercraft:wireless_modem"
+     * or "my_mod:my_upgrade".
+     *
+     * You should use a unique resource domain to ensure this upgrade is uniquely identified.
+     * The upgrade will fail registration if an already used ID is specified.
+     *
+     * @return The unique ID for this upgrade.
+     */
+    @Nonnull
+    ResourceLocation getUpgradeID();
+
+    /**
+     * Return an unlocalised string to describe this type of computer in item names.
+     *
+     * Examples of built-in adjectives are "Wireless", "Mining" and "Crafty".
+     *
+     * @return The localisation key for this upgrade's adjective.
+     */
+    @Nonnull
+    String getUnlocalisedAdjective();
+
+    /**
+     * Return an item stack representing the type of item that a computer must be crafted
+     * with to create a version which holds this upgrade. This item stack is also used
+     * to determine the upgrade given by {@code turtle.equipLeft()} or {@code pocket.equipBack()}
+     *
+     * This should be constant over a session (or at least a datapack reload). It is recommended
+     * that you cache the stack too, in order to prevent constructing it every time the method
+     * is called.
+     *
+     * @return The item stack to craft with, or {@link ItemStack#EMPTY} if it cannot be crafted.
+     */
+    @Nonnull
+    ItemStack getCraftingItem();
+
+    /**
+     * Determine if an item is suitable for being used for this upgrade.
+     *
+     * When un-equipping an upgrade, we return {@link #getCraftingItem()} rather than
+     * the original stack. In order to prevent people losing items with enchantments (or
+     * repairing items with non-0 damage), we impose additional checks on the item.
+     *
+     * The default check requires that any non-capability NBT is exactly the same as the
+     * crafting item, but this may be relaxed for your upgrade.
+     *
+     * @param stack The stack to check. This is guaranteed to be non-empty and have the same item as
+     *              {@link #getCraftingItem()}.
+     * @return If this stack may be used to equip this upgrade.
+     * @see net.minecraftforge.common.crafting.NBTIngredient#test(ItemStack) For the implementation of the default
+     * check.
+     */
+    default boolean isItemSuitable( @Nonnull ItemStack stack )
+    {
+        ItemStack crafting = getCraftingItem();
+
+        // A more expanded form of ItemStack.areShareTagsEqual, but allowing an empty tag to be equal to a
+        // null one.
+        CompoundNBT shareTag = stack.getItem().getShareTag( stack );
+        CompoundNBT craftingShareTag = crafting.getItem().getShareTag( crafting );
+        if( shareTag == craftingShareTag ) return true;
+        if( shareTag == null ) return craftingShareTag.isEmpty();
+        if( craftingShareTag == null ) return shareTag.isEmpty();
+        return shareTag.equals( craftingShareTag );
+    }
+}

src/main/java/dan200/computercraft/api/pocket/IPocketUpgrade.java

@@ -6,10 +6,8 @@
 package dan200.computercraft.api.pocket;
 
 import dan200.computercraft.api.ComputerCraftAPI;
+import dan200.computercraft.api.IUpgradeBase;
 import dan200.computercraft.api.peripheral.IPeripheral;
-import dan200.computercraft.api.turtle.ITurtleUpgrade;
-import net.minecraft.item.ItemStack;
-import net.minecraft.util.ResourceLocation;
 import net.minecraft.world.World;
 
 import javax.annotation.Nonnull;
@@ -18,49 +16,10 @@ import javax.annotation.Nullable;
 /**
  * Additional peripherals for pocket computers.
  *
- * This is similar to {@link ITurtleUpgrade}.
- */
-public interface IPocketUpgrade
-{
-
-    /**
-     * Gets a unique identifier representing this type of turtle upgrade. eg: "computercraft:wireless_modem" or
-     * "my_mod:my_upgrade".
-     *
-     * You should use a unique resource domain to ensure this upgrade is uniquely identified. The upgrade will fail
-     * registration if an already used ID is specified.
-     *
-     * @return The upgrade's id.
-     * @see IPocketUpgrade#getUpgradeID()
  * @see ComputerCraftAPI#registerPocketUpgrade(IPocketUpgrade)
  */
-    @Nonnull
+public interface IPocketUpgrade extends IUpgradeBase
-    ResourceLocation getUpgradeID();
+{
-
-    /**
-     * Return an unlocalised string to describe the type of pocket computer this upgrade provides.
-     *
-     * An example of a built-in adjectives is "Wireless" - this is converted to "Wireless Pocket Computer".
-     *
-     * @return The unlocalised adjective.
-     * @see ITurtleUpgrade#getUnlocalisedAdjective()
-     */
-    @Nonnull
-    String getUnlocalisedAdjective();
-
-    /**
-     * Return an item stack representing the type of item that a pocket computer must be crafted with to create a
-     * pocket computer which holds this upgrade. This item stack is also used to determine the upgrade given by
-     * {@code pocket.equip()}/{@code pocket.unequip()}.
-     *
-     * Ideally this should be constant over a session. It is recommended that you cache
-     * the item too, in order to prevent constructing it every time the method is called.
-     *
-     * @return The item stack used for crafting. This can be {@link ItemStack#EMPTY} if crafting is disabled.
-     */
-    @Nonnull
-    ItemStack getCraftingItem();
-
     /**
      * Creates a peripheral for the pocket computer.
      *

src/main/java/dan200/computercraft/api/turtle/ITurtleUpgrade.java

@@ -6,6 +6,7 @@
 package dan200.computercraft.api.turtle;
 
 import dan200.computercraft.api.ComputerCraftAPI;
+import dan200.computercraft.api.IUpgradeBase;
 import dan200.computercraft.api.client.TransformedModel;
 import dan200.computercraft.api.peripheral.IPeripheral;
 import dan200.computercraft.api.turtle.event.TurtleAttackEvent;
@@ -13,7 +14,6 @@ import dan200.computercraft.api.turtle.event.TurtleBlockEvent;
 import net.minecraft.client.renderer.model.ModelResourceLocation;
 import net.minecraft.item.ItemStack;
 import net.minecraft.util.Direction;
-import net.minecraft.util.ResourceLocation;
 import net.minecraftforge.api.distmarker.Dist;
 import net.minecraftforge.api.distmarker.OnlyIn;
 import net.minecraftforge.event.entity.player.AttackEntityEvent;
@@ -28,29 +28,8 @@ import javax.annotation.Nullable;
  *
  * @see ComputerCraftAPI#registerTurtleUpgrade(ITurtleUpgrade)
  */
-public interface ITurtleUpgrade
+public interface ITurtleUpgrade extends IUpgradeBase
 {
-    /**
-     * Gets a unique identifier representing this type of turtle upgrade. eg: "computercraft:wireless_modem" or "my_mod:my_upgrade".
-     * You should use a unique resource domain to ensure this upgrade is uniquely identified.
-     * The turtle will fail registration if an already used ID is specified.
-     *
-     * @return The unique ID for this upgrade.
-     * @see ComputerCraftAPI#registerTurtleUpgrade(ITurtleUpgrade)
-     */
-    @Nonnull
-    ResourceLocation getUpgradeID();
-
-    /**
-     * Return an unlocalised string to describe this type of turtle in turtle item names.
-     *
-     * Examples of built-in adjectives are "Wireless", "Mining" and "Crafty".
-     *
-     * @return The localisation key for this upgrade's adjective.
-     */
-    @Nonnull
-    String getUnlocalisedAdjective();
-
     /**
      * Return whether this turtle adds a tool or a peripheral to the turtle.
      *
@@ -60,19 +39,6 @@ public interface ITurtleUpgrade
     @Nonnull
     TurtleUpgradeType getType();
 
-    /**
-     * Return an item stack representing the type of item that a turtle must be crafted
-     * with to create a turtle which holds this upgrade. This item stack is also used
-     * to determine the upgrade given by {@code turtle.equip()}
-     *
-     * Ideally this should be constant over a session. It is recommended that you cache
-     * the item too, in order to prevent constructing it every time the method is called.
-     *
-     * @return The item stack to craft with, or {@link ItemStack#EMPTY} if it cannot be crafted.
-     */
-    @Nonnull
-    ItemStack getCraftingItem();
-
     /**
      * Will only be called for peripheral upgrades. Creates a peripheral for a turtle being placed using this upgrade.
      *

src/main/java/dan200/computercraft/shared/PocketUpgrades.java

@@ -7,7 +7,6 @@ package dan200.computercraft.shared;
 
 import dan200.computercraft.ComputerCraft;
 import dan200.computercraft.api.pocket.IPocketUpgrade;
-import dan200.computercraft.shared.util.InventoryUtil;
 import net.minecraft.item.ItemStack;
 import net.minecraftforge.fml.ModContainer;
 import net.minecraftforge.fml.ModLoadingContext;
@@ -55,7 +54,7 @@ public final class PocketUpgrades
         for( IPocketUpgrade upgrade : upgrades.values() )
         {
             ItemStack craftingStack = upgrade.getCraftingItem();
-            if( !craftingStack.isEmpty() && InventoryUtil.areItemsSimilar( stack, craftingStack ) )
+            if( !craftingStack.isEmpty() && craftingStack.getItem() == stack.getItem() && upgrade.isItemSuitable( stack ) )
             {
                 return upgrade;
             }

src/main/java/dan200/computercraft/shared/TurtleUpgrades.java

@@ -8,7 +8,6 @@ package dan200.computercraft.shared;
 import dan200.computercraft.ComputerCraft;
 import dan200.computercraft.api.turtle.ITurtleUpgrade;
 import dan200.computercraft.shared.computer.core.ComputerFamily;
-import dan200.computercraft.shared.util.InventoryUtil;
 import net.minecraft.item.ItemStack;
 import net.minecraftforge.fml.ModLoadingContext;
 
@@ -83,7 +82,7 @@ public final class TurtleUpgrades
             if( !wrapper.enabled ) continue;
 
             ItemStack craftingStack = wrapper.upgrade.getCraftingItem();
-            if( !craftingStack.isEmpty() && InventoryUtil.areItemsSimilar( stack, craftingStack ) )
+            if( !craftingStack.isEmpty() && craftingStack.getItem() == stack.getItem() && wrapper.upgrade.isItemSuitable( stack ) )
             {
                 return wrapper.upgrade;
             }

src/main/java/dan200/computercraft/shared/integration/jei/RecipeResolver.java

@@ -6,6 +6,7 @@
 package dan200.computercraft.shared.integration.jei;
 
 import dan200.computercraft.ComputerCraft;
+import dan200.computercraft.api.IUpgradeBase;
 import dan200.computercraft.api.pocket.IPocketUpgrade;
 import dan200.computercraft.api.turtle.ITurtleUpgrade;
 import dan200.computercraft.api.turtle.TurtleSide;
@@ -16,7 +17,6 @@ import dan200.computercraft.shared.pocket.items.ItemPocketComputer;
 import dan200.computercraft.shared.pocket.items.PocketComputerItemFactory;
 import dan200.computercraft.shared.turtle.items.ITurtleItem;
 import dan200.computercraft.shared.turtle.items.TurtleItemFactory;
-import dan200.computercraft.shared.util.InventoryUtil;
 import mezz.jei.api.constants.VanillaRecipeCategoryUid;
 import mezz.jei.api.recipe.IFocus;
 import mezz.jei.api.recipe.advanced.IRecipeManagerPlugin;
@@ -83,7 +83,10 @@ class RecipeResolver implements IRecipeManagerPlugin
         for( UpgradeInfo upgrade : upgrades )
         {
             ItemStack craftingStack = upgrade.stack;
-            if( !craftingStack.isEmpty() && InventoryUtil.areItemsSimilar( stack, craftingStack ) ) return true;
+            if( !craftingStack.isEmpty() && craftingStack.getItem() == stack.getItem() && upgrade.upgrade.isItemSuitable( stack ) )
+            {
+                return true;
+            }
         }
 
         return false;
@@ -194,11 +197,10 @@ class RecipeResolver implements IRecipeManagerPlugin
 
             List<Shaped> recipes = null;
             boolean multiple = false;
-            Ingredient ingredient = fromStacks( stack );
             for( UpgradeInfo upgrade : upgrades )
             {
                 ItemStack craftingStack = upgrade.stack;
-                if( craftingStack.isEmpty() || !InventoryUtil.areItemsSimilar( stack, craftingStack ) )
+                if( !craftingStack.isEmpty() && craftingStack.getItem() == stack.getItem() && upgrade.upgrade.isItemSuitable( stack ) )
                 {
                     continue;
                 }
@@ -332,42 +334,29 @@ class RecipeResolver implements IRecipeManagerPlugin
         }
     }
 
-    private static final class Upgrade<T>
-    {
-        final T upgrade;
-        final ItemStack stack;
-        final Ingredient ingredient;
-
-        private Upgrade( T upgrade, ItemStack stack )
-        {
-            this.upgrade = upgrade;
-            this.stack = stack;
-            ingredient = fromStacks( stack );
-        }
-    }
-
     private static class UpgradeInfo
     {
         final ItemStack stack;
         final Ingredient ingredient;
         final ITurtleUpgrade turtle;
         final IPocketUpgrade pocket;
+        final IUpgradeBase upgrade;
         ArrayList<Shaped> recipes;
 
         UpgradeInfo( ItemStack stack, ITurtleUpgrade turtle )
         {
             this.stack = stack;
-            ingredient = fromStacks( stack );
+            this.ingredient = fromStacks( stack );
-            this.turtle = turtle;
+            this.upgrade = this.turtle = turtle;
-            pocket = null;
+            this.pocket = null;
         }
 
         UpgradeInfo( ItemStack stack, IPocketUpgrade pocket )
         {
             this.stack = stack;
-            ingredient = fromStacks( stack );
+            this.ingredient = fromStacks( stack );
-            turtle = null;
+            this.turtle = null;
-            this.pocket = pocket;
+            this.upgrade = this.pocket = pocket;
         }
 
         List<Shaped> getRecipes()

src/main/java/dan200/computercraft/shared/turtle/upgrades/TurtleTool.java

@@ -28,6 +28,7 @@ import net.minecraft.entity.item.ArmorStandEntity;
 import net.minecraft.fluid.IFluidState;
 import net.minecraft.item.Item;
 import net.minecraft.item.ItemStack;
+import net.minecraft.nbt.CompoundNBT;
 import net.minecraft.tileentity.TileEntity;
 import net.minecraft.util.DamageSource;
 import net.minecraft.util.Direction;
@@ -38,6 +39,7 @@ import net.minecraft.world.World;
 import net.minecraftforge.api.distmarker.Dist;
 import net.minecraftforge.api.distmarker.OnlyIn;
 import net.minecraftforge.common.MinecraftForge;
+import net.minecraftforge.common.util.Constants;
 import net.minecraftforge.event.entity.player.AttackEntityEvent;
 import net.minecraftforge.event.world.BlockEvent;
 import org.apache.commons.lang3.tuple.Pair;
@@ -68,6 +70,24 @@ public class TurtleTool extends AbstractTurtleUpgrade
         this.item = toolItem;
     }
 
+    @Override
+    public boolean isItemSuitable( @Nonnull ItemStack stack )
+    {
+        CompoundNBT tag = stack.getTag();
+        if( tag == null || tag.isEmpty() ) return true;
+
+        // Check we've not got anything vaguely interesting on the item. We allow other mods to add their
+        // own NBT, with the understanding such details will be lost to the mist of time.
+        if( stack.isDamaged() || stack.isEnchanted() || stack.hasDisplayName() ) return false;
+        if( tag.contains( "AttributeModifiers", Constants.NBT.TAG_LIST ) &&
+            !tag.getList( "AttributeModifiers", Constants.NBT.TAG_COMPOUND ).isEmpty() )
+        {
+            return false;
+        }
+
+        return true;
+    }
+
     @Nonnull
     @Override
     @OnlyIn( Dist.CLIENT )

src/main/java/dan200/computercraft/shared/util/InventoryUtil.java

@@ -9,7 +9,6 @@ import net.minecraft.entity.Entity;
 import net.minecraft.inventory.IInventory;
 import net.minecraft.inventory.ISidedInventory;
 import net.minecraft.item.ItemStack;
-import net.minecraft.nbt.CompoundNBT;
 import net.minecraft.tileentity.TileEntity;
 import net.minecraft.util.Direction;
 import net.minecraft.util.math.BlockPos;
@@ -40,35 +39,6 @@ public final class InventoryUtil
         return a == b || ItemHandlerHelper.canItemStacksStack( a, b );
     }
 
-    /**
-     * Determines if two items are "mostly" equivalent. Namely, they have the same item and damage, and identical
-     * share stacks.
-     *
-     * This is largely based on {@link net.minecraftforge.common.crafting.IngredientNBT#test(ItemStack)}. It is
-     * sufficient to ensure basic information (such as enchantments) are the same, while not having to worry about
-     * capabilities.
-     *
-     * @param a The first stack to check
-     * @param b The second stack to check
-     * @return If these items are largely the same.
-     */
-    public static boolean areItemsSimilar( @Nonnull ItemStack a, @Nonnull ItemStack b )
-    {
-        if( a == b ) return true;
-        if( a.isEmpty() ) return !b.isEmpty();
-
-        if( a.getItem() != b.getItem() ) return false;
-
-        // A more expanded form of ItemStack.areShareTagsEqual, but allowing an empty tag to be equal to a
-        // null one.
-        CompoundNBT shareTagA = a.getItem().getShareTag( a );
-        CompoundNBT shareTagB = b.getItem().getShareTag( b );
-        if( shareTagA == shareTagB ) return true;
-        if( shareTagA == null ) return shareTagB.isEmpty();
-        if( shareTagB == null ) return shareTagA.isEmpty();
-        return shareTagA.equals( shareTagB );
-    }
-
     // Methods for finding inventories:
 
     public static IItemHandler getInventory( World world, BlockPos pos, Direction side )