Forge class IngredientNBT doesn't respect meta wildcards

[democat3457]

Issue Description

The Pyrotech anvils don't respect NBT for input items.

What Happens

1. Use the script.
2. Attempt to place a dragon skull on the Granite Anvil.
3. Notice that you are unable to place it on.

What You Expect to Happen

For the skull to be placeable and return the correct items when smashed.

Script

(it's really short)

import mods.pyrotech.GraniteAnvil;

GraniteAnvil.addRecipe("DragonHead1",<mod_lavacow:sharptooth>*3, <iceandfire:dragon_skull:*>.withTag({Stage: 1}),3,"hammer", true);
GraniteAnvil.addRecipe("DragonHead2",<mod_lavacow:sharptooth>*7, <iceandfire:dragon_skull:*>.withTag({Stage: 2}),6,"hammer", true);
GraniteAnvil.addRecipe("DragonHead3",<mod_lavacow:sharptooth>*12, <iceandfire:dragon_skull:*>.withTag({Stage: 3}),9,"hammer", true);
GraniteAnvil.addRecipe("DragonHead4",<mod_lavacow:sharptooth>*17, <iceandfire:dragon_skull:*>.withTag({Stage: 4}),12,"hammer", true);
GraniteAnvil.addRecipe("DragonHead5",<mod_lavacow:sharptooth>*25, <iceandfire:dragon_skull:*>.withTag({Stage: 5}),15,"hammer", true);

Affected Versions

Do not use latest; please supply accurate version numbers.

• Minecraft: 1.12.2
• Forge: 14.23.5.2847
• CraftTweaker: 4.1.20.588
• Dropt: 1.18.0
• Pyrotech: 1.4.33
• Athenaeum: 1.19.1

[codetaylor]

Unable to reproduce.

Used the following to produce an item with NBT and consume that item:

import mods.pyrotech.GraniteAnvil;

GraniteAnvil.addRecipe("dfgsdf", <minecraft:dirt>.withTag({Stage: 1}), <minecraft:grass>, 3, "hammer", true);
GraniteAnvil.addRecipe("pasnldf", <minecraft:stone>, <minecraft:dirt>.withTag({Stage: 1}), 3, "hammer", true);

Ensure there are no other tags on the dragon skull. It uses a strict NBT match.

[democat3457]

Ahhh. Is there a way to not have a strict NBT match, because the skulls have an NBT tag that would be impossible to do recipes for all of them (dragon age)?

[codetaylor]

Is there a way to not have a strict NBT match

Unfortunately, no. It uses the vanilla NBT matching logic which is strictly all or nothing.

[democat3457]

Hm, I just tried it with a valid dragon skull that should be placeable but isn't - the output of /ct hand is <iceandfire:dragon_skull>.withTag({Stage: 3 as long})

[hron84]

@democat3457 {Stage: 3} and {Stage: 3 as long} aren't same NBT tags. Copy/Paste /ct hand values into a variable and use that in recipes. It's much easier than include tags in all recipes.

[democat3457]

Adding as long doesn't work either.

[democat3457]

Any update? Or is it just stuck in github "can't repro" limbo

[codetaylor]

I was able to reproduce the problem using the following script:

import mods.pyrotech.GraniteAnvil;

GraniteAnvil.addRecipe("dfgsdf", <minecraft:log:3>.withTag({Stage: 3 as long}), <minecraft:grass>, 3, "hammer", true);
GraniteAnvil.addRecipe("pasnldf", <minecraft:stone>, <minecraft:log:*>.withTag({Stage: 3 as long}), 3, "hammer", true);

The issue is the meta wildcard *. The Forge class IngredientNBT tests absolute meta values and does not allow for meta wildcard values. Here is the snippet of Forge code responsible for this behavior:

    @Override
    public boolean apply(@Nullable ItemStack input)
    {
        if (input == null)
            return false;
        //Can't use areItemStacksEqualUsingNBTShareTag because it compares stack size as well
        return this.stack.getItem() == input.getItem() && this.stack.getItemDamage() == input.getItemDamage() && ItemStack.areItemStackShareTagsEqual(this.stack, input);
    }

As you can see, it attempts to strictly match the ingredient's meta value against the input item's meta value. The meta value of the wildcard * is 32767 and won't match any other value in this method.

CraftTweaker supplies a utility class, CraftTweakerMC which contains the method getIngredient(IIngredient) to convert an IIngredient to an Ingredient. This is the method that Pyrotech uses in all of its CraftTweaker integration classes and it is responsible for supplying the Forge IngredientNBT class as you can see here:

    public static Ingredient getIngredient(IIngredient ingredient) {
        if(ingredient == null)
            return Ingredient.EMPTY;
        if(ingredient instanceof IOreDictEntry)
            return new OreIngredient(((IOreDictEntry) ingredient).getName());
        if(ingredient instanceof IItemStack)
            if(((IItemStack) ingredient).hasTag())
                return new IngredientNBT(getItemStack(ingredient)){};
            else
                return Ingredient.fromStacks(getItemStack((IItemStack) ingredient));

        return new VanillaIngredient(ingredient);
    }

The solution is to write a custom subclass of IngredientNBT that overrides the apply(ItemStack) method with a method that respects the wildcard, write a custom static utility method like getIngredient(IIngredient) that returns the custom subclass, then evaluate and potentially replace each of the 52 occurrences of the method call in Pyrotech's CraftTweaker integration classes.

I don't know if / when I will get to this, so plan accordingly.

[democat3457]

Ok, thanks for the reply! Would manually specifying each meta of the item work then?

[codetaylor]

It would be tedious if you have a lot of meta values, but yes it should work fine.

[hron84]

@democat3457 try to check if the items already a part of an oredict entry. If yes, you can iterate through them with e.g. <ore:skullDragon>.items iterator. If not, you should definitely add them to an oredict entry (<ore:skullDragon>.add(<iceandfire:dragon_skull:0>)) and then iterate through them when you need to use it (but a lot of RecipeHandlers works well with oredict IIngredients.

(Note: <ore:skullDragon> in these examples is purely made from the thin air, potentially does not exists)