RecipeChoice.ExactChoice setPredicate is ignored for Shapeless Recipes

[joshuaprince]

Spark link

https://spark.lucko.me/YRCuU3Kvfy

Expected behavior

Purpur adds a setPredicate method to RecipeChoice.ExactChoice to allow plugins to add more precise control over crafting ingredients in recipes. When a predicate is set, it should be called whenever the recipe is a candidate for preparing an item craft.

Observed/Actual behavior

The predicate is ignored and never called. When calling getPredicate on the recipe later, it returns null.

Only Shapeless recipes are affected. ShapedRecipe seems to work as expected.

Steps/models to reproduce

A simple test plugin with nothing else installed shows the issue:

    @Override
    public void onEnable() {
        this.getServer().getPluginManager().registerEvents(this, this);

        NamespacedKey key = new NamespacedKey(this, "enchanted-slimeball-to-diamond");

        ItemStack enchantedSlimeball = new ItemStack(Material.SLIME_BALL);

        RecipeChoice.ExactChoice choice = new RecipeChoice.ExactChoice(enchantedSlimeball);
        choice.setPredicate(itemStack -> {
            // Only slime balls with an enchantment on them should be craftable into diamonds.
            this.getLogger().info("Called predicate");
            return itemStack.hasEnchants();
        });

        ShapelessRecipe recipe = new ShapelessRecipe(key, new ItemStack(Material.DIAMOND));
        recipe.addIngredient(choice);
        Bukkit.addRecipe(recipe);
    }

    @EventHandler(ignoreCancelled = false, priority = EventPriority.MONITOR)
    public void onPrepareCraft(PrepareItemCraftEvent event) {
        Recipe recipe = event.getRecipe();
        if (!(recipe instanceof ShapelessRecipe shapelessRecipe)) {
            return; // ignore everything except shapeless recipes
        }

        if (!(!shapelessRecipe.getChoiceList().isEmpty() && shapelessRecipe.getChoiceList().get(0) instanceof RecipeChoice.ExactChoice choice)) {
            return; // ignore shapeless recipes that do not have an exact choice
        }

        getLogger().info("recipe key %s - predicate %s".formatted(shapelessRecipe.getKey(), choice.getPredicate() == null ? "NULL" : choice));
    }

With the plugin installed, place a normal slime ball on the crafting grid. Expected behavior is that the crafting is blocked, the console prints "Called predicate" and a string for the predicate function. Instead, only the following is printed:

[PurpurItemPredicate] recipe key purpuritempredicate:enchanted-slimeball-to-diamond - predicate NULL

Purpur version

Current: git-Purpur-2091 (MC: 1.20.2)*
* You are running the latest version

Agreements

• I am running the latest version of Purpur available from https://purpurmc.org/downloads.
• I have searched for and ensured there isn't already an open issue regarding this.
• I ticked all the boxes without actually reading them
• My version of Minecraft is supported by Purpur.

Other

This bug was introduced in Purpur 1.20.1-2044, which involved upstream changes. On 2043 and earlier, the test plugin above works as expected. It was probably changes to the ShapelessRecipe class hierarchy in that patch that caused this.

[granny]

It broke after this PR was merged: PaperMC/Paper#7822. Technically, the current implementation shouldn't have even worked on Shapeless Recipes. The only reason it did is because of an old workaround that aikar added to allow Shapeless recipes to work with the Bukkit RecipeChoice API by using the same test logic as Shaped in Shapeless. While this allowed the API to work with shapeless recipes, it didn't work well with the recipe book. Machine-Maker's proper fix allows exact choice recipes to properly integrate with the recipe book.

The current implementation would have to change in some way to let you set a Predicate that gets checked for both Shaped and Shapeless recipes. Since it doesn't even integrate well with the recipe book I'm debating on just ripping it out completely. Machine-Maker talks here about creating a new PredicateChoice RecipeChoice, which would probably be the better way to go instead of trying to fix this broken implementation. If I get around to it before he does, then I'll PR it to Paper and deprecate the setPredicate API for removal to the closest minecraft version release.

[joshuaprince]

Upstream has an active PR to add PredicateChoice: PaperMC/Paper#9996