Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] New implementations on InputIngredient error on null stacks #1439

Open
idcppl opened this issue Feb 2, 2021 · 9 comments · May be fixed by #1728
Open

[BUG] New implementations on InputIngredient error on null stacks #1439

idcppl opened this issue Feb 2, 2021 · 9 comments · May be fixed by #1728
Labels
status: unverified type: bug Something isn't working

Comments

@idcppl
Copy link
Contributor

idcppl commented Feb 2, 2021

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)```
@idcppl idcppl added status: unverified type: bug Something isn't working labels Feb 2, 2021
@eutro
Copy link
Contributor

eutro commented Feb 2, 2021

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

@LAGIdiot
Copy link
Member

LAGIdiot commented Feb 6, 2021

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

@eutropius225 Can you please elaborate on this?

@warjort
Copy link
Contributor

warjort commented Feb 16, 2021

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
Copy link
Contributor

eutro commented Feb 16, 2021

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
Copy link
Member

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
Copy link
Contributor

eutro commented Feb 16, 2021

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
Copy link
Member

Archengius commented Feb 16, 2021

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
Copy link
Contributor

warjort commented Feb 16, 2021

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

TheDarkDnKTv added a commit to TheDarkDnKTv/GregTech that referenced this issue Oct 28, 2021
@hron84
Copy link

hron84 commented Sep 9, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: unverified type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants