Skip to content

fix: guard IngredientStack against empty Ingredient.values (#12)#13

Merged
Dark-Arcana merged 3 commits into
SmashingMods:1.21.1from
F0x06:fix/12-ingredientstack-empty-values-aioobe
Jun 11, 2026
Merged

fix: guard IngredientStack against empty Ingredient.values (#12)#13
Dark-Arcana merged 3 commits into
SmashingMods:1.21.1from
F0x06:fix/12-ingredientstack-empty-values-aioobe

Conversation

@F0x06

@F0x06 F0x06 commented Jun 10, 2026

Copy link
Copy Markdown

Summary

Fixes #12 — a client crash (ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0) when creating a new world.

IngredientStack's main constructor read pIngredient.values[0] directly, assuming every Ingredient carries at least one value. That holds for plain item/tag ingredients, but NeoForge custom/compound ingredients (and the empty ingredient) leave the vanilla values[]
array empty and expose their contents through getItems() instead. When a recipe using such an ingredient is decoded during the resource reload at world creation, values[0] throws and takes down the client.

Crash

java.lang.ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0
at com.smashingmods.alchemylib.api.item.IngredientStack.(IngredientStack.java:45)
...
at net.minecraft.world.item.crafting.RecipeManager.apply(RecipeManager.java:60)

Fix

registryName is now derived through a new resolveRegistryName(Ingredient) helper:

  1. values non-empty → unchanged behaviour (item value → item registry name, tag value → tag location).
  2. values empty → fall back to the first item the ingredient resolves to (getItems()[0]).
  3. resolves to nothing → item registry default key (minecraft:air).

This keeps registryName non-null (required by equals/hashCode) and preserves the existing happy-path semantics.

Tests

  • constructor_emptyIngredient_doesNotThrow — direct regression on the AIOOBE.
  • getRegistryName_emptyIngredient_isItemRegistryDefaultKey — empty ingredient resolves to minecraft:air.
  • Updated the class doc; the empty-ingredient path is now covered instead of "left untested on purpose".

./gradlew test --tests "*IngredientStackTest" → green.

SmashingMods#12)

The main constructor read pIngredient.values[0] directly, assuming every
Ingredient carries at least one value. NeoForge custom/compound ingredients
(and the empty ingredient) leave the vanilla values[] array empty and expose
their contents through getItems(), so decoding such a recipe at world creation
threw ArrayIndexOutOfBoundsException and crashed the client.

Resolve registryName via a new resolveRegistryName helper: keep the existing
values[0] path when present, otherwise fall back to the first item the
ingredient resolves to, and finally to the item registry default key
(minecraft:air). Add regression tests for the empty-ingredient path.
@F0x06 F0x06 changed the base branch from 1.19.x to 1.21.1 June 10, 2026 19:42
…, never resolving items at decode time

The previous fix for SmashingMods#12 stopped the ArrayIndexOutOfBoundsException but fell
back to Ingredient.getItems()[0] when the values array is empty. Recipes are
decoded during RecipeManager.apply, before registry tags are bound to that
reload, so getItems() resolves tag-backed children to NeoForge's barrier
"Empty Tag" placeholder stacks and memoizes that snapshot for the
Ingredient's lifetime (barrier ghosts in JEI and the recipe selector). Worse,
every unresolvable custom ingredient collapsed to the same fallback name
(minecraft:barrier or minecraft:air), and with equality keyed on count plus
that single name, recipes collecting inputs into a LinkedHashSet silently
dropped any second custom-ingredient input.

Never resolve items in the constructor. Instead key both registryName and the
new identity field on what the Ingredient declares:

- custom ingredient (Ingredient.isCustom()): stand-in name alchemylib:custom,
  identity = the ICustomIngredient itself. NeoForge mandates equals/hashCode
  on ICustomIngredient implementations (its own, like CompoundIngredient, are
  records); third-party customs that skip the contract degrade to instance
  identity, which still keeps separately decoded ingredients distinct.
- no declared values (Ingredient.of()): stand-in name alchemylib:empty,
  empty identity.
- declared values: identity = the whole sorted value set (tag locations and
  item registry names), registryName = the first value's location as before.

equals/hashCode now compare count plus the full identity, which also brings
the 1.21.4 whole-ingredient-identity fix (b06042f) to 1.21.1: two multi-item
ingredients sharing only a first item are no longer equal.

Keeps the existing empty-ingredient regression tests (the stand-in expectation
replaces minecraft:air) and adds coverage for the custom-ingredient
constructor path, custom and multi-item equality, and the LinkedHashSet
input-drop regression.
…k identity

The identity introduced in 1275a86 is a sorted list of bare declared
locations, mixing tag locations and item registry names. A tag and an item
may legally share a ResourceLocation, so Ingredient.of(#mymod:x) and
Ingredient.of(mymod:x item) compared equal at equal counts -- reproducing,
for that pair, the exact silent LinkedHashSet recipe-input merge the
identity exists to prevent.

Prefix each identity entry with its value kind: tag values become
"tag:<location>", item values "item:<registry name>". The custom branch
(identity = the ICustomIngredient itself) and registryName semantics are
unchanged. Adds a regression test pinning that a tag-backed and an
item-backed ingredient sharing minecraft:stone at equal counts are not
equal.
@Dark-Arcana Dark-Arcana merged commit 3f785e1 into SmashingMods:1.21.1 Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ATM10 + Alchemistry v1.21.1

2 participants