Adding PredicateChoice to Paper API #9996

derverdox · 2023-12-03T12:37:02Z

3 days ago I posted a pull request #9991 about adding CustomModelChoice. However, we came to the conclusion that we need a more generalized approach.
As a result, I began working on what is called PredicateChoice.
To make it work the StackedContents logic had to be edited.
In the ExtraContentsMap we don't store the exact ingredient items of every recipe ingredient. Instead, whenever we account a new stack in StackedContents logic we compare it to the provided predicates and store them as "ingredient items" if they fulfill at least one predicate.

We act like the items in the inventory of the player are the exact choices when any ingredient predicate returns true.

It gets more apparent when looking at the changes.
The first tests showed that it works like a charm.

Pros

With predicate choice one can implement his own version of ItemPredicate. This allows for the programmer to make the recipe check for whatever he wants.
They still work as intended if you want to use MaterialChoice or ExactChoice.

Cons

If you use PredicateChoice the predicate tests Bukkit ItemStacks. Consequently, NMS stacks need to instantiate their Craftbukkit mirror to make the checks happen.

Let me know your thoughts! :) Maybe I have missed something.

Machine-Maker

Missing all sorts of // Paper comments, but that's not a big deal.

Overall, this works with my initial testing. I haven't yet done any spark profiles yet to see if the performance is significantly worse or anything.

My main concern with this is the unnecessary replacement of MaterialChoice and ExactChoice. We should just be able to add a 3rd (PredicateChoice) to that lineup without needing to replace the others.

patches/api/0449-Adding-PredicateChoice-to-Paper-API.patch

patches/server/1055-Adding-PredicateChoice-to-Paper-API.patch

Machine-Maker · 2023-12-03T18:07:32Z

This is just a quick mockup of what I had in mind. Ignore the API, it is just quicker to make it not an interface+record. I didn't exhaustively test it, but the test in the test-plugin work so far.

derverdox · 2023-12-03T18:46:46Z

This is just a quick mockup of what I had in mind. Ignore the API, it is just quicker to make it not an interface+record. I didn't exhaustively test it, but the test in the test plugin work so far.

Looks good to me! :) My PR was more of a proof of concept. I am fine with your implementation.
Do you want me to fix my PR? Or do we just pick your's instead? I think it doesn't make a difference right?

Machine-Maker · 2023-12-03T18:59:59Z

You can go ahead and fixup yours

derverdox · 2023-12-03T19:16:37Z

You can go ahead and fixup yours

Allright

derverdox

I created two new patches. Your implementation heavily inspires them. It is way cleaner to use the logic when a predicate is used.
Even fewer code changes than before. I tested things myself, and it works like a charm :)

Since we know that the extra map logic works, we use working code and add more item contents to its storage. In this case, the items that match any predicate choice.

It is a simple solution that brings many new possibilities to the table.
Hope there is no fatal flaw in this idea.

Machine-Maker · 2023-12-13T03:06:12Z

patches/server/1056-Adding-PredicateChoice-to-Paper-API.patch

+                boolean isStackTypeRegistered = this.exactChoiceIds.getInt(stack) > -1;
+                final int id = this.registerExact(stack);
+                // We only want to add the stacking id to the list one time
+                if(!isStackTypeRegistered)


You should just be able to check the id returned by registerExact if it is -1, it's already been added to the exactChoice map.

Machine-Maker · 2023-12-13T03:07:10Z

patches/api/0462-Adding-PredicateChoice-to-Paper-API.patch

+            return Objects.equals(itemPredicate, that.itemPredicate);
+        }
+
+        public static interface ItemPredicate extends Predicate<ItemStack> {


This type shouldn't exist

Machine-Maker · 2023-12-13T03:07:29Z

patches/api/0462-Adding-PredicateChoice-to-Paper-API.patch

+    /**
+     * Represents a choice that matches when the item predicate is fulfilled.
+     */
+    public static class PredicateChoice implements RecipeChoice {


This should be an interface that is backed by a package-private record implementation with static methods to create instances of it.

Machine-Maker · 2023-12-13T03:08:21Z

patches/server/1056-Adding-PredicateChoice-to-Paper-API.patch

+
+     public static final Ingredient EMPTY = new Ingredient(Stream.empty());
+     private final Ingredient.Value[] values;
+    @Nullable private RecipeChoice.PredicateChoice.ItemPredicate itemPredicate; // Paper


Just have a public field for Predicate

Machine-Maker · 2023-12-13T03:08:35Z

patches/server/1056-Adding-PredicateChoice-to-Paper-API.patch

+    // Paper start - Adding PredicateChoice
+    public Ingredient(RecipeChoice.PredicateChoice.ItemPredicate itemPredicate) {
+        List<org.bukkit.inventory.ItemStack> bukkitChoices = itemPredicate.recipeBookExamples();
+        this.itemPredicate = itemPredicate;
+        this.values = bukkitChoices.stream().map(CraftItemStack::asNMSCopy).map(ItemValue::new).toArray(Value[]::new);
+    }
+    // Paper end - Adding PredicateChoice


No need for a custom constructor, do all the logic in CraftRecipe

Machine-Maker · 2023-12-13T03:09:01Z

patches/server/1056-Adding-PredicateChoice-to-Paper-API.patch

+    // Paper start - Adding PredicateChoice
+    @Nullable
+    public RecipeChoice.PredicateChoice.ItemPredicate getItemPredicate() {
+        return itemPredicate;
+    }
+    // Paper end - Adding PredicateChoice


with a public field, no need for a getter

Machine-Maker · 2023-12-13T03:09:26Z

patches/server/1056-Adding-PredicateChoice-to-Paper-API.patch

+        }
+        // Paper start - Adding PredicateChoice
+        else if(bukkit instanceof RecipeChoice.PredicateChoice predicateChoice){
+            stack = new Ingredient(predicateChoice.getItemPredicate());


Just use the regular constructor and pass in the recipe book examples as itemstacks and then assign the predicate field on the next line.

PaperMC/Paper#9996 I merged this change, added @NotNull tags to get the API to build, and deleted the existing Purpur item predicate API since it was conflicting.

partydev · 2024-01-09T01:36:24Z

Upon testing this found that clicking the recipe book item does not move the items in your inventory to the crafting grid.

Ingredients have custom model data, lore, and attributes set. The recipe does work, just not the recipe books behavior. This may be more to blame on the recipe book patch than this patch.

JakeGBLP · 2024-03-07T17:28:56Z

Is this still being worked on?

derverdox · 2024-03-20T22:19:02Z

Is this still being worked on?

Hello! I was working on my bachelor thesis lately so i was not able to fully work on this.
Will continue soon!

- Fixed RecipeBook click not working

derverdox · 2024-03-21T15:58:26Z

Upon testing this found that clicking the recipe book item does not move the items in your inventory to the crafting grid. Ingredients have custom model data, lore, and attributes set. The recipe does work, just not the recipe books behavior. This may be more to blame on the recipe book patch than this patch.

Hey. I just uploaded a fix to the recipe book problem. The issue seems to be resolved. It would be nice if you could test as well! :)

Machine-Maker

Some of the diff in the server patch can be moved back to the patch that created it, such as simple renames, like the hasExactIngredients but that doesn't have to be done now, can be done before merging.

We also need to make it clear that mutating the stack within the Predicate provided in the create method does not support mutating the stack in an way. I don't want to open that can of worms, and I also don't want to copy the itemstack each time that predicate might be called.

Machine-Maker · 2024-03-21T23:45:01Z

patches/api/0469-Adding-PredicateChoice.patch

+
+    @Override
+    public @NotNull RecipeChoice clone() {
+        return new PredicateChoiceImpl(predicate, recipeBookExamples());


This also has to clone each itemstack in the recipe book examples because ItemStack is a mutable type.

Machine-Maker · 2024-03-21T23:45:43Z

patches/api/0469-Adding-PredicateChoice.patch

+        static PredicateChoice create(@NotNull Predicate<ItemStack> predicate, ItemStack... recipeBookExamples){
+            Objects.requireNonNull(predicate, "The item predicate cannot be null!");
+            Objects.requireNonNull(predicate, "The mustHaveRecipeBookExample cannot be null!");
+            if(recipeBookExamples.length == 0)
+                throw new IllegalArgumentException("Please provide at least one recipe book example item!");
+            return new PredicateChoiceImpl(predicate, List.of(recipeBookExamples));
+        }
+
+        static PredicateChoice create(@NotNull Predicate<ItemStack> predicate, java.util.Collection<ItemStack> recipeBookExamples){
+            Objects.requireNonNull(predicate, "The item predicate cannot be null!");
+            Objects.requireNonNull(predicate, "The mustHaveRecipeBookExample cannot be null!");
+            if(recipeBookExamples.isEmpty())
+                throw new IllegalArgumentException("Please provide at least one recipe book example item!");
+            return new PredicateChoiceImpl(predicate, List.copyOf(recipeBookExamples));
+        }


I would move these methods up 1 level to be on RecipeChoice itself and named createPredicateChoice

An alternative would be to just make PredicateChoice a top-level class next to RecipeChoice and have create methods in it. Not a fan of the static factory methods inside an inner class.

dawon · 2024-08-14T07:16:46Z

patches/api/0469-Adding-PredicateChoice.patch

+    interface PredicateChoice extends RecipeChoice {
+        static PredicateChoice create(@NotNull Predicate<ItemStack> predicate, ItemStack... recipeBookExamples){
+            Objects.requireNonNull(predicate, "The item predicate cannot be null!");
+            Objects.requireNonNull(predicate, "The mustHaveRecipeBookExample cannot be null!");


This line checks the exactly same thing as the one above... I think here should be recipeBookExamples?

dawon · 2024-08-14T07:16:54Z

patches/api/0469-Adding-PredicateChoice.patch

+
+        static PredicateChoice create(@NotNull Predicate<ItemStack> predicate, java.util.Collection<ItemStack> recipeBookExamples){
+            Objects.requireNonNull(predicate, "The item predicate cannot be null!");
+            Objects.requireNonNull(predicate, "The mustHaveRecipeBookExample cannot be null!");


This line checks the exactly same thing as the one above... I think here should be recipeBookExamples?

Adding PredicateChoice to Paper API

db062b8

derverdox requested a review from a team as a code owner December 3, 2023 12:37

derverdox mentioned this pull request Dec 3, 2023

Adding CustomModelChoice to bukkit recipe system #9991

Closed

Machine-Maker requested changes Dec 3, 2023

View reviewed changes

joshuaprince mentioned this pull request Dec 4, 2023

RecipeChoice.ExactChoice setPredicate is ignored for Shapeless Recipes PurpurMC/Purpur#1460

Open

4 tasks

Better implementation of PredicateChoice for Paper API

904e97c

derverdox commented Dec 5, 2023

View reviewed changes

Machine-Maker requested changes Dec 13, 2023

View reviewed changes

derverdox and others added 2 commits March 20, 2024 23:35

Merge branch 'PaperMC:master' into feature/predicate_choice

eedca5e

- Small code changes

135b0d5

- Fixed RecipeBook click not working

derverdox requested a review from Machine-Maker March 21, 2024 17:16

Machine-Maker requested changes Mar 21, 2024

View reviewed changes

dawon suggested changes Aug 14, 2024

View reviewed changes

Fusezion mentioned this pull request Aug 18, 2024

[SUGGESTION] Recipe input item match from only NBT in custom_data tag ShaneBeee/SkBee#704

Closed

MiniDigger added the pre-softspoon label Dec 21, 2024

CPieter mentioned this pull request Jan 26, 2025

Adding PredicateChoice to Paper API (updated version) #12017

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding PredicateChoice to Paper API #9996

Adding PredicateChoice to Paper API #9996

derverdox commented Dec 3, 2023 •

edited

Loading

Machine-Maker left a comment

Machine-Maker commented Dec 3, 2023

derverdox commented Dec 3, 2023 •

edited

Loading

Machine-Maker commented Dec 3, 2023

derverdox commented Dec 3, 2023

derverdox left a comment •

edited

Loading

Machine-Maker Dec 13, 2023

Machine-Maker Dec 13, 2023

Machine-Maker Dec 13, 2023

Machine-Maker Dec 13, 2023

Machine-Maker Dec 13, 2023

Machine-Maker Dec 13, 2023

Machine-Maker Dec 13, 2023

partydev commented Jan 9, 2024 •

edited

Loading

JakeGBLP commented Mar 7, 2024

derverdox commented Mar 20, 2024

derverdox commented Mar 21, 2024

Machine-Maker left a comment

Machine-Maker Mar 21, 2024

Machine-Maker Mar 21, 2024

dawon Aug 14, 2024

dawon Aug 14, 2024

Adding PredicateChoice to Paper API #9996

Are you sure you want to change the base?

Adding PredicateChoice to Paper API #9996

Conversation

derverdox commented Dec 3, 2023 • edited Loading

Machine-Maker left a comment

Choose a reason for hiding this comment

Machine-Maker commented Dec 3, 2023

derverdox commented Dec 3, 2023 • edited Loading

Machine-Maker commented Dec 3, 2023

derverdox commented Dec 3, 2023

derverdox left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

partydev commented Jan 9, 2024 • edited Loading

JakeGBLP commented Mar 7, 2024

derverdox commented Mar 20, 2024

derverdox commented Mar 21, 2024

Machine-Maker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derverdox commented Dec 3, 2023 •

edited

Loading

derverdox commented Dec 3, 2023 •

edited

Loading

derverdox left a comment •

edited

Loading

partydev commented Jan 9, 2024 •

edited

Loading