[BUG] New implementations on InputIngredient error on null stacks

[idcppl]

Describe the bug Whenever you call the method recipe.inputs and the stack is null it'll error.

Versions GTCE: Latest

Setup Errors on load up with CraftTweaker scripts.

Steps To Reproduce wrote ```import mods.gregtech.recipe.RecipeMap;

for machine in RecipeMap.getRecipeMaps() { for recipe in machine.recipes { var inputs = recipe.inputs; if(inputs.length > 0) { for input in inputs { print(input.commandString); } } } }```

And the log is,

java.lang.IllegalArgumentException: stack cannot be null
	at crafttweaker.mc1120.item.MCItemStack.<init>(MCItemStack.java:88)
	at crafttweaker.mc1120.item.MCItemStack.withAmount(MCItemStack.java:229)
	at crafttweaker.mc1120.item.MCItemStack.amount(MCItemStack.java:327)
	at crafttweaker.mc1120.item.MCItemStack.amount(MCItemStack.java:57)
	at gregtech.api.recipes.crafttweaker.InputIngredient.<init>(InputIngredient.java:23)
	at gregtech.api.recipes.crafttweaker.CTRecipe$$Lambda$568/1355890075.apply(Unknown Source)
	at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
	at java.util.Iterator.forEachRemaining(Iterator.java:116)
	at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:512)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:502)
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
	at gregtech.api.recipes.crafttweaker.CTRecipe.getInputs(CTRecipe.java:37)
	at RecipeGetter.__script__(RecipeGetter.zs:5)
	at __ZenMain__.run(RecipeGetter)
	at crafttweaker.runtime.CrTTweaker.loadScript(CrTTweaker.java:228)
	at crafttweaker.runtime.CrTTweaker.loadScript(CrTTweaker.java:105)
	at crafttweaker.mc1120.events.CommonEventHandler.registerRecipes(CommonEventHandler.java:73)
	at net.minecraftforge.fml.common.eventhandler.ASMEventHandler_75_CommonEventHandler_registerRecipes_Register.invoke(.dynamic)
	at net.minecraftforge.fml.common.eventhandler.ASMEventHandler.invoke(ASMEventHandler.java:90)
	at net.minecraftforge.fml.common.eventhandler.EventBus$1.invoke(EventBus.java:144)
	at net.minecraftforge.fml.common.eventhandler.EventBus.post(EventBus.java:182)
	at net.minecraftforge.registries.GameData.fireRegistryEvents(GameData.java:857)
	at net.minecraftforge.common.crafting.CraftingHelper.loadRecipes(CraftingHelper.java:636)
	at net.minecraftforge.fml.common.Loader.initializeMods(Loader.java:747)
	at net.minecraftforge.fml.client.FMLClientHandler.finishMinecraftLoading(FMLClientHandler.java:336)
	at net.minecraft.client.Minecraft.func_71384_a(Minecraft.java:535)
	at net.minecraft.client.Minecraft.func_99999_d(Minecraft.java:378)
	at net.minecraft.client.main.Main.main(SourceFile:123)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at net.minecraft.launchwrapper.Launch.launch(Launch.java:135)
	at net.minecraft.launchwrapper.Launch.main(Launch.java:28)```

[eutro]

This might be something to fix on CraftTweaker's end.

[LAGIdiot]

Thank you for informing us about this issue. This is not good and should be fixed.

@eutropius225 Can you please elaborate on this?

[warjort]

The issue is representing nonConsumable ingredients in crafttweaker. In particular the integrated circuits.

GT's InputIngredent class does:

        iingredient = CraftTweakerMC
            .getIIngredient(backingIngredient.getIngredient())
            .amount(backingIngredient.getCount());

But here getCount() is zero because the ingredient is not consumed.

Crafttweaker effectively does a

new ItemStack(originalItemStack, 0);

Then validates it with a call to ItemStack.isEmpty() which throws that misleading error about it being null.

There are 3 possbile workarounds we can do in GT that I can see:

1. represent these items with a count of 1 in crafttweaker which is obviously wrong
2. use the CraftTweakerMC.getIItemStackWildcard() to represent an undefined amount of the item, this would require doing a lot extra work for things like matchingStacks that is currently done in getIIngredient()
3. Like (2) but just implement our own NonConsumableIIngredient, But having 0 in the amount might confuse other parts of crafttweaker

[eutro]

Well canonically unconsumed ingredients are represented as having a count of 1 and just not being consumed.

Having an ingredient with a count of 0 sounds absurd, why is it even there if 0 is required from it?

[Archengius]

because it needs to be there, just like items that are not consumed? Ingredient with zero count represents non-consumable item, that's it.

[eutro]

Of course, unconsumed ingredients need to be represented somehow. I'm just saying that using count=0 for that is illogical, and inconsistent with how it's handled elsewhere. It causes bugs like this when the special case is overlooked.

What does it mean to say "this recipe takes as input 0 cobblestone"? Is that actually, well, zero cobblestone, so it doesn't require cobblestone? Does it need at least one cobblestone as a catalyst? Does it require a stack of it? In this case it's the second, you need one, but it's not consumed.

Why can't there just be an extra flag on CountableIngredient for whether it's consumed or not, instead of being an undocumented special value?

[Archengius]

Because CountableIngredient is literally IIngredient with a count, that's it? It doesn't care how you interpret count or ingredient object in question, it's just a simple data holder class that you can use for whatever reason. It was never intended to be mapped to CT ingredient representation or back, neither it was ever supposed to be operated on directly. It's just an internal representation recipe system is using, there are wrapper methods like notConsumable which abstract it away.

Besides, original Ingredient implementation in Minecraft does NOT define any standards or utilities for counting ingredients or checking their amounts. CraftTweaker's ability to set amount of ingredients is purely it's own extension to the standard system, and you should NOT expect it to be compatible with anything else if you just blindly map one data object to another.

[warjort]

For reference, CT represents the wildcard ingredient as -1, i.e. negative 1 items :-)

[hron84]

Actually, other mods (non-GregTech) are solving this problem on some way and maintaining compabibility with CraftTweaker. There is a lot of mods that use catalytic items in ingredients. I am not a mod developer, but I am curious if others can solve this issue without breaking the compatibility, what could prevent GregTech to do same? I am not trolling, I am genuiely curious.

Also, Minecraft itself also has a replaceable ingredient (where a bucket of milk become a bucket), what if you replace the non-consuming item with itself?