Paper/patches/api/0467-Fix-issues-with-recipe-API.patch

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jake Potrebic <jake.m.potrebic@gmail.com>
Date: Sun, 12 May 2024 10:42:42 -0700
Subject: [PATCH] Fix issues with recipe API

Improves the validation when creating recipes
and RecipeChoices to closer match what is
allowed by the Codecs and StreamCodecs internally.

Adds RecipeChoice#empty which is allowed in specific
recipes and ingredient slots.

Also fixes some issues regarding mutability of both ItemStack
and implementations of RecipeChoice.

Also adds some validation regarding Materials passed to RecipeChoice
being items.

diff --git a/src/main/java/org/bukkit/inventory/CookingRecipe.java b/src/main/java/org/bukkit/inventory/CookingRecipe.java
index f7fa79393aef40027446b78bac8e9490cfafd8bc..07906ca1a9b39fcc6774870daa498402f7f37917 100644
--- a/src/main/java/org/bukkit/inventory/CookingRecipe.java
+++ b/src/main/java/org/bukkit/inventory/CookingRecipe.java
@@ -44,10 +44,10 @@ public abstract class CookingRecipe<T extends CookingRecipe> implements Recipe,
      * @param cookingTime The cooking time (in ticks)
      */
     public CookingRecipe(@NotNull NamespacedKey key, @NotNull ItemStack result, @NotNull RecipeChoice input, float experience, int cookingTime) {
-        Preconditions.checkArgument(result.getType() != Material.AIR, "Recipe must have non-AIR result.");
+        Preconditions.checkArgument(!result.isEmpty(), "Recipe cannot have an empty result."); // Paper
         this.key = key;
         this.output = new ItemStack(result);
-        this.ingredient = input;
+        this.ingredient = input.validate(false).clone(); // Paper
         this.experience = experience;
         this.cookingTime = cookingTime;
     }
@@ -84,7 +84,7 @@ public abstract class CookingRecipe<T extends CookingRecipe> implements Recipe,
      */
     @NotNull
     public T setInputChoice(@NotNull RecipeChoice input) {
-        this.ingredient = input;
+        this.ingredient = input.validate(false).clone(); // Paper
         return (T) this;
     }
 
diff --git a/src/main/java/org/bukkit/inventory/CraftingRecipe.java b/src/main/java/org/bukkit/inventory/CraftingRecipe.java
index 1b7b07715067014bf3d35002ae1655793248b426..5bf55b40fbf6ec708f37d90bd0853fe7dd8fffd9 100644
--- a/src/main/java/org/bukkit/inventory/CraftingRecipe.java
+++ b/src/main/java/org/bukkit/inventory/CraftingRecipe.java
@@ -99,7 +99,7 @@ public abstract class CraftingRecipe implements Recipe, Keyed {
     @ApiStatus.Internal
     @NotNull
     protected static ItemStack checkResult(@NotNull ItemStack result) {
-        Preconditions.checkArgument(result.getType() != Material.AIR, "Recipe must have non-AIR result.");
+        Preconditions.checkArgument(!result.isEmpty(), "Recipe cannot have an empty result."); // Paper
         return result;
     }
 }
diff --git a/src/main/java/org/bukkit/inventory/EmptyRecipeChoice.java b/src/main/java/org/bukkit/inventory/EmptyRecipeChoice.java
new file mode 100644
index 0000000000000000000000000000000000000000..ed0ab6163f47ec843ba4f7ea4a98bb2fa315eaa1
--- /dev/null
+++ b/src/main/java/org/bukkit/inventory/EmptyRecipeChoice.java
@@ -0,0 +1,33 @@
+package org.bukkit.inventory;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.framework.qual.DefaultQualifier;
+import org.jetbrains.annotations.ApiStatus;
+
+@ApiStatus.Internal
+@DefaultQualifier(NonNull.class)
+record EmptyRecipeChoice() implements RecipeChoice {
+
+    static final RecipeChoice INSTANCE = new EmptyRecipeChoice();
+    @Override
+    public ItemStack getItemStack() {
+        throw new UnsupportedOperationException("This is an empty RecipeChoice");
+    }
+
+    @SuppressWarnings("MethodDoesntCallSuperMethod")
+    @Override
+    public RecipeChoice clone() {
+        return this;
+    }
+
+    @Override
+    public boolean test(final ItemStack itemStack) {
+        return false;
+    }
+
+    @Override
+    public RecipeChoice validate(final boolean allowEmptyRecipes) {
+        if (allowEmptyRecipes) return this;
+        throw new IllegalArgumentException("empty RecipeChoice isn't allowed here");
+    }
+}
diff --git a/src/main/java/org/bukkit/inventory/MerchantRecipe.java b/src/main/java/org/bukkit/inventory/MerchantRecipe.java
index 39f9766a03d420340d79841197f75c8b1dd49f4a..4e59f5176fd6cf92457ad750081c253a58790b61 100644
--- a/src/main/java/org/bukkit/inventory/MerchantRecipe.java
+++ b/src/main/java/org/bukkit/inventory/MerchantRecipe.java
@@ -79,6 +79,7 @@ public class MerchantRecipe implements Recipe {
         this(result, uses, maxUses, experienceReward, villagerExperience, priceMultiplier, 0, 0, ignoreDiscounts);
     }
     public MerchantRecipe(@NotNull ItemStack result, int uses, int maxUses, boolean experienceReward, int villagerExperience, float priceMultiplier, int demand, int specialPrice, boolean ignoreDiscounts) {
+        Preconditions.checkArgument(!result.isEmpty(), "Recipe cannot have an empty result."); // Paper
         this.ignoreDiscounts = ignoreDiscounts;
         // Paper end
         this.result = result;
@@ -101,11 +102,12 @@ public class MerchantRecipe implements Recipe {
     @NotNull
     @Override
     public ItemStack getResult() {
-        return result;
+        return result.clone(); // Paper
     }
 
     public void addIngredient(@NotNull ItemStack item) {
         Preconditions.checkState(ingredients.size() < 2, "MerchantRecipe can only have maximum 2 ingredients");
+        Preconditions.checkArgument(!item.isEmpty(), "Recipe cannot have an empty itemstack ingredient."); // Paper
         ingredients.add(item.clone());
     }
 
@@ -117,6 +119,7 @@ public class MerchantRecipe implements Recipe {
         Preconditions.checkState(ingredients.size() <= 2, "MerchantRecipe can only have maximum 2 ingredients");
         this.ingredients = new ArrayList<ItemStack>();
         for (ItemStack item : ingredients) {
+            Preconditions.checkArgument(!item.isEmpty(), "Recipe cannot have an empty itemstack ingredient."); // Paper
             this.ingredients.add(item.clone());
         }
     }
diff --git a/src/main/java/org/bukkit/inventory/RecipeChoice.java b/src/main/java/org/bukkit/inventory/RecipeChoice.java
index 91bfeffcdbe47208c7d0ddbe013cd0f11fddfa32..f1aa67997f904953742e8895e49341c2f73d44a2 100644
--- a/src/main/java/org/bukkit/inventory/RecipeChoice.java
+++ b/src/main/java/org/bukkit/inventory/RecipeChoice.java
@@ -22,6 +22,19 @@ import org.jetbrains.annotations.NotNull;
  */
 public interface RecipeChoice extends Predicate<ItemStack>, Cloneable {
 
+    // Paper start - add "empty" choice
+    /**
+     * An "empty" recipe choice. Only valid as a recipe choice in
+     * specific places. Check the javadocs of a method before using it
+     * to be sure it's valid for that recipe and ingredient type.
+     *
+     * @return the empty recipe choice
+     */
+    static @NotNull RecipeChoice empty() {
+        return EmptyRecipeChoice.INSTANCE;
+    }
+    // Paper end
+
     /**
      * Gets a single item stack representative of this stack choice.
      *
@@ -38,6 +51,13 @@ public interface RecipeChoice extends Predicate<ItemStack>, Cloneable {
     @Override
     boolean test(@NotNull ItemStack itemStack);
 
+    // Paper start - check valid ingredients
+    @org.jetbrains.annotations.ApiStatus.Internal
+    default @NotNull RecipeChoice validate(final boolean allowEmptyRecipes) {
+        return this;
+    }
+    // Paper end - check valid ingredients
+
     /**
      * Represents a choice of multiple matching Materials.
      */
@@ -60,8 +80,7 @@ public interface RecipeChoice extends Predicate<ItemStack>, Cloneable {
          * @param choices the tag
          */
         public MaterialChoice(@NotNull Tag<Material> choices) {
-            Preconditions.checkArgument(choices != null, "choices");
-            this.choices = new ArrayList<>(choices.getValues());
+            this(new ArrayList<>(java.util.Objects.requireNonNull(choices, "Cannot create a material choice with null tag").getValues())); // Paper - delegate to list ctor to make sure all checks are called
         }
 
         public MaterialChoice(@NotNull List<Material> choices) {
@@ -78,6 +97,7 @@ public interface RecipeChoice extends Predicate<ItemStack>, Cloneable {
                 }
 
                 Preconditions.checkArgument(!choice.isAir(), "Cannot have empty/air choice");
+                Preconditions.checkArgument(choice.isItem(), "Cannot have non-item choice %s", choice); // Paper - validate material choice input to items
                 this.choices.add(choice);
             }
         }
@@ -152,6 +172,16 @@ public interface RecipeChoice extends Predicate<ItemStack>, Cloneable {
         public String toString() {
             return "MaterialChoice{" + "choices=" + choices + '}';
         }
+
+        // Paper start - check valid ingredients
+        @Override
+        public @NotNull RecipeChoice validate(final boolean allowEmptyRecipes) {
+            if (this.choices.stream().anyMatch(Material::isAir)) {
+                throw new IllegalArgumentException("RecipeChoice.MaterialChoice cannot contain air");
+            }
+            return this;
+        }
+        // Paper end - check valid ingredients
     }
 
     /**
@@ -197,7 +227,12 @@ public interface RecipeChoice extends Predicate<ItemStack>, Cloneable {
         public ExactChoice clone() {
             try {
                 ExactChoice clone = (ExactChoice) super.clone();
-                clone.choices = new ArrayList<>(choices);
+                // Paper start - properly clone
+                clone.choices = new ArrayList<>(this.choices.size());
+                for (ItemStack choice : this.choices) {
+                    clone.choices.add(choice.clone());
+                }
+                // Paper end - properly clone
                 return clone;
             } catch (CloneNotSupportedException ex) {
                 throw new AssertionError(ex);
@@ -244,5 +279,15 @@ public interface RecipeChoice extends Predicate<ItemStack>, Cloneable {
         public String toString() {
             return "ExactChoice{" + "choices=" + choices + '}';
         }
+
+        // Paper start - check valid ingredients
+        @Override
+        public @NotNull RecipeChoice validate(final boolean allowEmptyRecipes) {
+            if (this.choices.stream().anyMatch(s -> s.getType().isAir())) {
+                throw new IllegalArgumentException("RecipeChoice.ExactChoice cannot contain air");
+            }
+            return this;
+        }
+        // Paper end - check valid ingredients
     }
 }
diff --git a/src/main/java/org/bukkit/inventory/ShapedRecipe.java b/src/main/java/org/bukkit/inventory/ShapedRecipe.java
index 295d82dd73b600e9436d2bbec0e11dbeaf78bbf4..c0105d716985acef497d60b5c631a56b4ca5847b 100644
--- a/src/main/java/org/bukkit/inventory/ShapedRecipe.java
+++ b/src/main/java/org/bukkit/inventory/ShapedRecipe.java
@@ -178,14 +178,15 @@ public class ShapedRecipe extends CraftingRecipe {
         Preconditions.checkArgument(key != ' ', "Space in recipe shape must represent no ingredient");
         Preconditions.checkArgument(ingredients.containsKey(key), "Symbol does not appear in the shape:", key);
 
-        ingredients.put(key, ingredient);
+        ingredients.put(key, ingredient.validate(false).clone()); // Paper
         return this;
     }
 
     // Paper start
     @NotNull
     public ShapedRecipe setIngredient(char key, @NotNull ItemStack item) {
-        return setIngredient(key, new RecipeChoice.ExactChoice(item));
+        Preconditions.checkArgument(!item.getType().isAir(), "Item cannot be air"); // Paper
+        return setIngredient(key, new RecipeChoice.ExactChoice(item.clone())); // Paper
     }
     // Paper end
 
diff --git a/src/main/java/org/bukkit/inventory/ShapelessRecipe.java b/src/main/java/org/bukkit/inventory/ShapelessRecipe.java
index 3bf5064cd6ceb05ea98b18993da46c67be140115..79db6dbc0367de2eaa397674624c765d5aeb8fa5 100644
--- a/src/main/java/org/bukkit/inventory/ShapelessRecipe.java
+++ b/src/main/java/org/bukkit/inventory/ShapelessRecipe.java
@@ -132,7 +132,7 @@ public class ShapelessRecipe extends CraftingRecipe {
     public ShapelessRecipe addIngredient(@NotNull RecipeChoice ingredient) {
         Preconditions.checkArgument(ingredients.size() + 1 <= 9, "Shapeless recipes cannot have more than 9 ingredients");
 
-        ingredients.add(ingredient);
+        ingredients.add(ingredient.validate(false).clone()); // Paper
         return this;
     }
 
@@ -145,6 +145,8 @@ public class ShapelessRecipe extends CraftingRecipe {
     @NotNull
     public ShapelessRecipe addIngredient(int count, @NotNull ItemStack item) {
         Preconditions.checkArgument(ingredients.size() + count <= 9, "Shapeless recipes cannot have more than 9 ingredients");
+        Preconditions.checkArgument(!item.getType().isAir(), "Item cannot be air"); // Paper
+        item = item.clone(); // Paper
         while (count-- > 0) {
             ingredients.add(new RecipeChoice.ExactChoice(item));
         }
diff --git a/src/main/java/org/bukkit/inventory/SmithingRecipe.java b/src/main/java/org/bukkit/inventory/SmithingRecipe.java
index 1ef9a715a2736e88a16083c6873803a8bd6bcf29..3072858dd4413129ec1737572838c2ea5ffd84bc 100644
--- a/src/main/java/org/bukkit/inventory/SmithingRecipe.java
+++ b/src/main/java/org/bukkit/inventory/SmithingRecipe.java
@@ -44,12 +44,13 @@ public class SmithingRecipe implements Recipe, Keyed {
      */
     @Deprecated
     public SmithingRecipe(@NotNull NamespacedKey key, @NotNull ItemStack result, @NotNull RecipeChoice base, @NotNull RecipeChoice addition, boolean copyDataComponents) {
+        com.google.common.base.Preconditions.checkArgument(!result.isEmpty() || this instanceof ComplexRecipe, "Recipe cannot have an empty result."); // Paper
         this.copyDataComponents = copyDataComponents;
         // Paper end
         this.key = key;
         this.result = result;
-        this.base = base;
-        this.addition = addition;
+        this.base = base.validate(true).clone(); // Paper
+        this.addition = addition.validate(true).clone(); // Paper
     }
 
     /**
diff --git a/src/main/java/org/bukkit/inventory/SmithingTransformRecipe.java b/src/main/java/org/bukkit/inventory/SmithingTransformRecipe.java
index 68e7132d77151b7b8312638d8bb79ea59e2fa5a6..60bfdd6b8814be8e3ffdfaef8a5ac7eeff9a5830 100644
--- a/src/main/java/org/bukkit/inventory/SmithingTransformRecipe.java
+++ b/src/main/java/org/bukkit/inventory/SmithingTransformRecipe.java
@@ -15,13 +15,13 @@ public class SmithingTransformRecipe extends SmithingRecipe {
      *
      * @param key The unique recipe key
      * @param result The item you want the recipe to create.
-     * @param template The template item.
-     * @param base The base ingredient
-     * @param addition The addition ingredient
+     * @param template The template item ({@link RecipeChoice#empty()} can be used)
+     * @param base The base ingredient ({@link RecipeChoice#empty()} can be used)
+     * @param addition The addition ingredient ({@link RecipeChoice#empty()} can be used)
      */
     public SmithingTransformRecipe(@NotNull NamespacedKey key, @NotNull ItemStack result, @NotNull RecipeChoice template, @NotNull RecipeChoice base, @NotNull RecipeChoice addition) {
         super(key, result, base, addition);
-        this.template = template;
+        this.template = template.validate(true).clone(); // Paper
     }
     // Paper start
     /**
@@ -29,14 +29,14 @@ public class SmithingTransformRecipe extends SmithingRecipe {
      *
      * @param key The unique recipe key
      * @param result The item you want the recipe to create.
-     * @param template The template item.
-     * @param base The base ingredient
-     * @param addition The addition ingredient
+     * @param template The template item ({@link RecipeChoice#empty()} can be used)
+     * @param base The base ingredient ({@link RecipeChoice#empty()} can be used)
+     * @param addition The addition ingredient ({@link RecipeChoice#empty()} can be used)
      * @param copyDataComponents whether to copy the data components from the input base item to the output
      */
     public SmithingTransformRecipe(@NotNull NamespacedKey key, @NotNull ItemStack result, @NotNull RecipeChoice template, @NotNull RecipeChoice base, @NotNull RecipeChoice addition, boolean copyDataComponents) {
         super(key, result, base, addition, copyDataComponents);
-        this.template = template;
+        this.template = template.validate(true).clone();
     }
     // Paper end
 
diff --git a/src/main/java/org/bukkit/inventory/SmithingTrimRecipe.java b/src/main/java/org/bukkit/inventory/SmithingTrimRecipe.java
index ce36bb5b030f17e11f74e987235be143c1925aa7..9ac27cbe7dcff0403ef34727d0461b8201aca6f1 100644
--- a/src/main/java/org/bukkit/inventory/SmithingTrimRecipe.java
+++ b/src/main/java/org/bukkit/inventory/SmithingTrimRecipe.java
@@ -15,27 +15,27 @@ public class SmithingTrimRecipe extends SmithingRecipe implements ComplexRecipe
      * Create a smithing recipe to produce the specified result ItemStack.
      *
      * @param key The unique recipe key
-     * @param template The template item.
-     * @param base The base ingredient
-     * @param addition The addition ingredient
+     * @param template The template item ({@link RecipeChoice#empty()} can be used)
+     * @param base The base ingredient ({@link RecipeChoice#empty()} can be used)
+     * @param addition The addition ingredient ({@link RecipeChoice#empty()} can be used)
      */
     public SmithingTrimRecipe(@NotNull NamespacedKey key, @NotNull RecipeChoice template, @NotNull RecipeChoice base, @NotNull RecipeChoice addition) {
         super(key, new ItemStack(Material.AIR), base, addition);
-        this.template = template;
+        this.template = template.validate(true).clone(); // Paper
     }
     // Paper start
     /**
      * Create a smithing recipe to produce the specified result ItemStack.
      *
      * @param key The unique recipe key
-     * @param template The template item.
-     * @param base The base ingredient
-     * @param addition The addition ingredient
+     * @param template The template item. ({@link RecipeChoice#empty()} can be used)
+     * @param base The base ingredient ({@link RecipeChoice#empty()} can be used)
+     * @param addition The addition ingredient ({@link RecipeChoice#empty()} can be used)
      * @param copyDataComponents whether to copy the data components from the input base item to the output
      */
     public SmithingTrimRecipe(@NotNull NamespacedKey key, @NotNull RecipeChoice template, @NotNull RecipeChoice base, @NotNull RecipeChoice addition, boolean copyDataComponents) {
         super(key, new ItemStack(Material.AIR), base, addition, copyDataComponents);
-        this.template = template;
+        this.template = template.validate(true).clone(); // Paper
     }
     // Paper end
 
diff --git a/src/main/java/org/bukkit/inventory/StonecuttingRecipe.java b/src/main/java/org/bukkit/inventory/StonecuttingRecipe.java
index bc3440eb72127824b3961fbdae583bb61385f65e..17b33f8e6e3dc6a22686a498fa944382e8767077 100644
--- a/src/main/java/org/bukkit/inventory/StonecuttingRecipe.java
+++ b/src/main/java/org/bukkit/inventory/StonecuttingRecipe.java
@@ -35,10 +35,10 @@ public class StonecuttingRecipe implements Recipe, Keyed {
      * @param input The input choices.
      */
     public StonecuttingRecipe(@NotNull NamespacedKey key, @NotNull ItemStack result, @NotNull RecipeChoice input) {
-        Preconditions.checkArgument(result.getType() != Material.AIR, "Recipe must have non-AIR result.");
+        Preconditions.checkArgument(!result.isEmpty(), "Recipe cannot have an empty result."); // Paper
         this.key = key;
         this.output = new ItemStack(result);
-        this.ingredient = input;
+        this.ingredient = input.validate(false).clone(); // Paper
     }
 
     /**
@@ -73,7 +73,7 @@ public class StonecuttingRecipe implements Recipe, Keyed {
      */
     @NotNull
     public StonecuttingRecipe setInputChoice(@NotNull RecipeChoice input) {
-        this.ingredient = input;
+        this.ingredient = input.validate(false).clone(); // Paper
         return (StonecuttingRecipe) this;
     }