PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.67k stars 2.25k forks source link

Crafting doesn't work with 'usingConvertsTo' property #11165

Open Three-of-Ten-201 opened 1 month ago

Three-of-Ten-201 commented 1 month ago

Expected behavior

Shaped and shapeless crafting recipes output their corresponding result regardless of the 'usingConvertsTo' properties of the ingredients.

Observed/Actual behavior

Shaped and shapeless crafting recipes register and show up correctly in the recipe book. However, putting the ingredients into the crafting table doesn't give an output item at all if one of the ingredients contains a food component with a 'usingConvertsTo' property.

Steps/models to reproduce

  1. Create a crafting recipe (shaped or shapeless) with one of the ingredients being an item that has a usingConvertsTo property.
  2. Try to input the recipe into a crafting table

Example:

  1. Create an ItemStack of material Material.PAPER
  2. Retrieve a food component from the itemstack using ItemMeta#getFood()
  3. Set an ItemStack of material Material.DIRT as the usingConvertsTo using FoodComponent#setUsingConvertsTo(ItemStack var1)
  4. Create a recipe using the first itemstack as an ingredient (with RecipeChoice.ExactChoice) and a new ItemStack with _Material.COARSEDIRT as the result item.
  5. Try to input the recipe into a crafting table (no result should pop up in the output slot)
@Override
    public boolean onCommand(@NotNull CommandSender sender, @NotNull Command command, @NotNull String s, @NotNull String[] args) {
        if (sender instanceof Player player) {

            ItemStack ingredient = new ItemStack(Material.PAPER);

            ItemMeta itemMeta = ingredient.getItemMeta();

            FoodComponent food = itemMeta.getFood();
            food.setNutrition(1);
            food.setSaturation(0.5F);
            food.setUsingConvertsTo(new ItemStack(Material.DIRT));
            itemMeta.setFood(food);
            ingredient.setItemMeta(itemMeta);

            Bukkit.addRecipe(new ShapelessRecipe(
                    new NamespacedKey("plugin", "test_recipe"),
                    new ItemStack(Material.COARSE_DIRT))
                    .addIngredient(new RecipeChoice.ExactChoice(ingredient)), true);

            player.getInventory().addItem(ingredient);
        }
        return true;
    }

Plugin and Datapack List

Datapacks:

Plugins:

Paper version

[18:32:54 INFO]: Checking version, please wait...
[18:32:55 INFO]: This server is running Paper version 1.21-109-master@5a5035b (2024-07-23T08:16:30Z) (Implementing API version 1.21-R0.1-SNAPSHOT)
You are running the latest version
Previous version: 1.21-105-7e91a2c (MC: 1.21)

Other

My plugin (the only one on the server) doesn't change or interfere with the behaviour of the crafting table or the 'usingConvertsTo' property in any way. I just discovered this issue while messing around with custom food items.

Furthermore, I tried to use a rabbit stew item as an ingredient, which should also have a 'usingConvertsTo' property (converts to minecraft:bowl) and this worked. So, it seems to be only for custom items.

Machine-Maker commented 1 month ago

Wait, I thought usingConvertsTo was only for when you eat the food. Not anything to do with crafting. Can you confirm this is a thing in vanilla by trying out items with custom usingConvertsTo?

Three-of-Ten-201 commented 1 month ago

Yes, the usingConvertsTo is the item that you get for eating the food. It has indeed nothing to do with crafting. However, as soon as a custom item has a food component with a usingConvertsTo property, you can no longer use it to craft things.

Boy0000 commented 1 month ago

usingConvertsTo does not seem to be relevant, it is just that the item has a food-component and the recipe does not Im guessing this is a similar issue as to when items have enchantments, lost durability and so on Another example where the PredicateChoice PR would be useful

no food component, as recipe intends:

food component with/without usingConvertsTo:( no result)

Three-of-Ten-201 commented 1 month ago

Okay, I did some more tests on that. Here's what I found:

  1. When you create a recipe and tell it to use an item that has a food component via RecipeChoice.ExactChoice then it will give the correct output (assuming that the input item has the correct food properties of course)
  2. When you create a recipe and tell it to use an item that has a food component with a usingConvertsTo property, it won't give you an output even if the input item has exactly the usingConvertsTo defined in the recipe (and the other properties are also correct)
  3. When you create a recipe that just uses a material and no RecipeChoice.ExactChoice, the crafting table will output the correct result even if the input has a food component and a usingConvertsTo property

So we have two "variables":

  1. The recipe expects the input item to have usingConvertsTo
  2. The input item actually HAS usingConvertsTo

The test results are:

  1. If expected and provided, it doesn't craft
  2. If expected and not provided, it doesn't craft (works as expected)
  3. If not expected, but provided, it crafts
  4. If not expected and not provided, it crafts (works as expected)

So you can have custom food items AND custom recipes with them, it just doesn't work when the ingredient has usingConvertsTo?

Boy0000 commented 1 month ago

Not able to reproduce the part where if the recipe expects an ingredient with food-component that has usingConvertsTo it will never work On my end it seems to work fine

https://github.com/user-attachments/assets/c345b050-cc74-4cb3-9c62-75e93e0a4a8f

All it would do is compare itemmeta 1:1, and since these match, it is correct Still PredicateChoice or some other form of way to specify an item in a recipe is heavily needed Custom Items in recipes are painful to deal with because of the need for ExactChoice, thus breaking with enchantments, durability and other such component/meta differences

Three-of-Ten-201 commented 1 month ago

Not able to reproduce the part where if the recipe expects an ingredient with food-component that has usingConvertsTo it will never work On my end it seems to work fine

Still doesn't work for me. For context: I have an item (cooked rice) that converts to a bowl when eaten (because the cooked rice is crafted with a bowl). I wanted to create a recipe for sushi that uses the cooked rice, dried kelp and cooked salmon. I created the recipe using ExactChoice, it showed up correctly in the recipe book, but when I input the items, it doesn't give me an output at all. I know that this is almost exactly the same you showed in the video (apart from the fact that it worked there), but I wonder what I did wrong/what is wrong with the recipe or the items. Crafting with my other food items that don't have usingConvertsTo works absolutely fine. Also, when I remove the usingConvertsTo from the cooked rice item, it works just fine.

lynxplay commented 1 month ago

So this is a rather annoying issue. The equality check (far far down the line) compares two Optional instances with each other. ItemStack#equals does not exist. They are supposed to be compared with alternative methods. The two different ItemStack instances in there fail to compare, so the item fails.

diff --git a/src/main/java/net/minecraft/world/food/FoodProperties.java b/src/main/java/net/minecraft/world/food/FoodProperties.java
index c60cdb3ef4..57347bcb43 100644
--- a/src/main/java/net/minecraft/world/food/FoodProperties.java
+++ b/src/main/java/net/minecraft/world/food/FoodProperties.java
@@ -16,6 +16,20 @@ import net.minecraft.world.level.ItemLike;
 public record FoodProperties(
     int nutrition, float saturation, boolean canAlwaysEat, float eatSeconds, Optional<ItemStack> usingConvertsTo, List<FoodProperties.PossibleEffect> effects
 ) {
+    // Paper start - correct equality check
+    @Override
+    public boolean equals(final Object o) {
+        if (this == o) return true;
+        if (!(o instanceof final FoodProperties that)) return false;
+        return nutrition == that.nutrition &&
+            Float.compare(saturation, that.saturation) == 0 &&
+            Float.compare(eatSeconds, that.eatSeconds) == 0 &&
+            canAlwaysEat == that.canAlwaysEat &&
+            ItemStack.isSameItemSameComponentsOptional(usingConvertsTo, that.usingConvertsTo) &&
+            java.util.Objects.equals(effects, that.effects);
+    }
+    // Paper end - correct equality check
+
     public static final float DEFAULT_EAT_SECONDS = 1.6F;
     public static final Codec<FoodProperties> DIRECT_CODEC = RecordCodecBuilder.create(
         instance -> instance.group(
diff --git a/src/main/java/net/minecraft/world/item/ItemStack.java b/src/main/java/net/minecraft/world/item/ItemStack.java
index 2c312c0b74..60a5f02353 100644
--- a/src/main/java/net/minecraft/world/item/ItemStack.java
+++ b/src/main/java/net/minecraft/world/item/ItemStack.java
@@ -878,6 +878,13 @@ public final class ItemStack implements DataComponentHolder {
         return !stack.is(otherStack.getItem()) ? false : (stack.isEmpty() && otherStack.isEmpty() ? true : Objects.equals(stack.components, otherStack.components));
     }

+    // Paper start - isSameItemSameComponentsOptional
+    public static boolean isSameItemSameComponentsOptional(Optional<ItemStack> stack, Optional<ItemStack> otherStack) {
+        if (stack.isEmpty() && otherStack.isEmpty()) return true;
+        return stack.isPresent() && otherStack.isPresent() && isSameItemSameComponents(stack.get(), otherStack.get());
+    }
+    // Paper end - isSameItemSameComponentsOptional
+
     public static MapCodec<ItemStack> lenientOptionalFieldOf(String fieldName) {
         return ItemStack.CODEC.lenientOptionalFieldOf(fieldName).xmap((optional) -> {
             return (ItemStack) optional.orElse(ItemStack.EMPTY);

would e.g. fix this.