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

Constraints on input stack size for recipe builder #222

Merged

Conversation

Wizzerinus
Copy link
Contributor

@Wizzerinus Wizzerinus commented Aug 14, 2024

This pull request adds error messages that happen when a recipe becomes invalid/gives unexpected results with ItemStack input of size > 1. It turns out that the absolute majority of mods don't care about the input stack size, which can give results ranging from "recipe is cheaper than intended" to "this cannot be executed at all". I went over nearly every compat in the mod (the exceptions are Mekanism, Thermal Expansion, IC2 exp/c, and Tinker's Complement, I believe the 4 first ones do properly support input stack sizes), nearly every machine in that mod, and checked whether that machine supports multi-inputs. This behavior can be disabled in the config if the pack developer has a mixin to fix the offending mod or such.

I've also found some other bugs/weird behaviors while working on this branch, listed on the bottom of the list. Some with GS, some with other mods (LMAO).

The following machines have multi-input recipes by default. Not all of them were tested:

  • Alchemistry: Combiner
  • Arcane Archives: Gem Crafting Table
  • Avaritia: Compressor (applied differently)
  • Better with Mods: multiple
  • EnderIO: Alloy Smelter, Enchanter (catalyst and input slots)
  • Essentialcraft 4: Mithrilline Furnace, Demon Trade
  • Extended Crafting: Compressor (applied differently)
  • Extrautilities: Enchanter, Generators
  • Forestry: Squeezer
  • Immersive Engineering: Arc Furnace, Squeezer, Metal Press, Engineer's Table
  • Mekanism: some machines (i.e. Enrichment Chamber)
  • Nature's Aura: Offering to the Gods (max size 16)
  • Pneumaticcraft: Amadron, Pressure Chamber
  • Thermal Expansion: some machines (i.e. Compactor)

The following machines don't have them by default but support them:

  • Alchemistry: Dissolver
  • Applied Energistics: Quartz Grindstone
  • Advanced Mortars
  • Extrautilities: Furnace, Resonator, Crusher
  • Immersive Engineering: Fermenter, Alloy Furnace, Blast Furnace, Coke Oven, Crusher, Mixer
  • Pneumaticcraft: TPPP
  • ProdigyTech: Solderer
  • Mekanism: the rest of machines probably (I've tested Chemical Oxidizer)
  • Thermal Expansion: the rest of machines probably (I've tested Alchemical Imbuer)
  • Draconic Evolution: Fusion
  • Nature's Aura: Spawning Ritual (I think??? I literally forgot what was the status there)

The following machines inhibit weird behavior when multi-input is provided. I've opted to "ban" that use case:

  • Immersive Engineering: Blast Furnace Fuel
  • Pneumaticcraft: Plastic Mixer, Cooling Frame
  • ProdigyTech: most machines

The following machines either ignore the input stack size or fail to execute their recipe. Note that these recipes show up in JEI as though they worked. I banned these:

  • Vanilla: Crafting table, Furnace
  • Actually Additions: all
  • Aether: all
  • Applied Energistics: Inscriber, Cannon Ammo
  • Astral Sorcery: all
  • Avaritia: crafting table
  • Better with Mods: multiple
  • Blood Magic: all
  • Botania: all
  • Calculator: all
  • Chisel: all
  • EnderIO: SAG mill, Enchanter (book slot), SAG mill grinding ball, Soul binder, Slice & Splice, Tank, Vat
  • Essentialcraft 4: Magician Table, Magmatic Smeltery, Radiating Chamber, Wind Rune
  • Evilcraft: all
  • Extended Crafting: tables, combination crafting
  • Forestry: all except for Squeezer
  • Immersive Engineering: Bottling Machine
  • Industrial Foregoing: all
  • Inspirations: Cauldron
  • Integrated Dynamics: all
  • Lazy AE2: all
  • Nature's Aura: Natural Altar, Ritual of the Forest
  • Pyrotech: all
  • Roots: all
  • Rustic: all
  • Aurorian: all
  • Tinker's Construct: all
  • Woot: Anvil

The following mods were untested because they didn't exist in GS when I started the branch 😔

  • Atuum 2
  • Botanic Additions
  • Extra Botany
  • Cyclic
  • Primaltech ( I think )

Here's the bugs I found:

  • Moistener's bottom input slot doesn't work properly (Forestry, unsure if it's their issue or GrS issue)
  • Fermenter doesn't work properly (Forestry, same)
  • Oil Generator recipes don't work properly (GrS's side)
  • Guidebook isn't needed to launch Arcane Archives (GrS's side, fixed)
  • Minor issues with SAG mill and Hellfire Forge

⚰️ do with this as you will

Copy link
Collaborator

@WaitingIdly WaitingIdly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in a number of places, GroovyScriptConfig is imported but unused. imports should be cleaned up.

@Wizzerinus
Copy link
Contributor Author

done

Copy link
Collaborator

@WaitingIdly WaitingIdly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noted a few more places to apply previously commented changes

@Wizzerinus
Copy link
Contributor Author

Something like this, maybe

@brachy84 brachy84 added the enhancement New feature or request label Aug 31, 2024
@@ -86,6 +87,18 @@ public static boolean isEmpty(@Nullable NBTTagCompound nbt) {
return nbt == null || nbt.isEmpty();
}

public static boolean overMaxSize(@Nullable IIngredient ingredient, int maxSize) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about adding methods to directly add to the msg and calling those?
additionally, IResourceStack should probably be used here instead of IIngredient, since IResourceStack is the interface that has getAmount.

public static void overMaxSize(GroovyLog.Msg msg, Collection<IIngredient> ingredients, int maxSize) {
    if (!GroovyScriptConfig.compat.checkInputStackCounts) return;
    for (var ingredient : ingredients) {
        msg.add(IngredientHelper.overMaxSize(ingredient, maxSize), "Expected stack size of {} for {}, got {}", maxSize, ingredient, ingredient.getAmount());
    }
}

public static void overMaxSize(GroovyLog.Msg msg, IIngredient ingredient, int maxSize) {
    if (!GroovyScriptConfig.compat.checkInputStackCounts) return;
    msg.add(IngredientHelper.overMaxSize(ingredient, maxSize), "Expected stack size of {} for {}, got {}", maxSize, ingredient, ingredient.getAmount());
}

Copy link
Member

@brachy84 brachy84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

# Conflicts:
#	src/main/java/com/cleanroommc/groovyscript/GroovyScriptConfig.java
#	src/main/java/com/cleanroommc/groovyscript/compat/mods/bloodmagic/TartaricForge.java
#	src/main/java/com/cleanroommc/groovyscript/compat/mods/enderio/SagMill.java
#	src/main/java/com/cleanroommc/groovyscript/compat/mods/enderio/SagMillGrinding.java
#	src/main/java/com/cleanroommc/groovyscript/compat/mods/industrialforegoing/BioReactor.java
#	src/main/java/com/cleanroommc/groovyscript/compat/mods/industrialforegoing/Extractor.java
#	src/main/java/com/cleanroommc/groovyscript/compat/mods/industrialforegoing/OreRaw.java
#	src/main/java/com/cleanroommc/groovyscript/compat/mods/industrialforegoing/OreSieve.java
#	src/main/java/com/cleanroommc/groovyscript/compat/mods/industrialforegoing/ProteinReactor.java
#	src/main/java/com/cleanroommc/groovyscript/helper/recipe/AbstractRecipeBuilder.java
# Conflicts:
#	src/main/java/com/cleanroommc/groovyscript/compat/mods/ModSupport.java
@brachy84 brachy84 merged commit 5d177a5 into CleanroomMC:master Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants