diff --git a/src/main/java/com/smashingmods/alchemylib/api/item/IngredientStack.java b/src/main/java/com/smashingmods/alchemylib/api/item/IngredientStack.java index b8c97fa..84d0a27 100644 --- a/src/main/java/com/smashingmods/alchemylib/api/item/IngredientStack.java +++ b/src/main/java/com/smashingmods/alchemylib/api/item/IngredientStack.java @@ -10,6 +10,7 @@ import net.minecraft.world.item.ItemStack; import net.minecraft.world.item.crafting.Ingredient; import net.minecraft.world.level.ItemLike; +import net.neoforged.neoforge.common.crafting.ICustomIngredient; import java.util.Arrays; import java.util.List; @@ -26,15 +27,49 @@ @SuppressWarnings("unused") public class IngredientStack { + /** + * Stand-in registry name for an Ingredient that declares no values at all (e.g. {@code Ingredient.of()}). It is + * namespaced to the mod so it can never collide with a real item or tag id, keeping {@link #getRegistryName()} + * non-null for that degenerate case instead of indexing into an empty values array. + */ + private static final ResourceLocation EMPTY = ResourceLocation.fromNamespaceAndPath("alchemylib", "empty"); + + /** + * Stand-in registry name for a NeoForge {@linkplain Ingredient#isCustom() custom ingredient} + * ({@link ICustomIngredient}). A custom ingredient declares no item or tag values -- its contents are only + * knowable by resolving items, which the constructor must never do (see + * {@link #IngredientStack(Ingredient, int)}) -- so this mod-namespaced stand-in keeps + * {@link #getRegistryName()} non-null without colliding with a real item or tag id. + */ + private static final ResourceLocation CUSTOM = ResourceLocation.fromNamespaceAndPath("alchemylib", "custom"); + private final Ingredient ingredient; private final int count; private final ResourceLocation registryName; + /** + * The equality key for this stack: the sorted, unmodifiable list of kind-prefixed declared value locations + * ({@code "tag:"} for tag values, {@code "item:"} for item values) for a vanilla + * ingredient, or the {@link ICustomIngredient} itself for a custom one. The kind prefix keeps a tag and an + * item that share a location distinct -- with bare locations they compared equal and hash-based + * recipe-input sets silently merged them. NeoForge requires custom ingredients to implement + * {@code equals}/{@code hashCode} (its own, like {@code CompoundIngredient}, are records with structural + * equality); a third-party custom that skips that contract degrades to instance identity, which still keeps + * separately decoded ingredients distinct. + */ + private final Object identity; /** * All other constructors reference this main constructor for creating a new IngredientStack. * - *

{@link IngredientStack#registryName} is taken from the 0th entry of the Ingredient's values array. If that - * entry is an item value the item's registry name is used; if it is a tag value the tag's location is used.

+ *

Both {@link #registryName} and {@link #identity} are derived from what the Ingredient declares, + * never from the items it resolves to: recipes are decoded during {@code RecipeManager.apply}, before registry + * tags are bound to that reload, so {@link Ingredient#getItems()} would substitute -- and permanently memoize -- + * NeoForge's barrier "Empty Tag" placeholder stacks. A {@linkplain Ingredient#isCustom() custom ingredient} + * gets the {@link #CUSTOM} stand-in name and is identified by its {@link ICustomIngredient}; a vanilla + * ingredient is identified by its full declared value set (each value's location prefixed with its kind, + * {@code tag:}/{@code item:}, sorted) with the first value's location as the representative + * {@link #registryName}; an ingredient with no values at all (e.g. {@code Ingredient.of()}) gets the + * {@link #EMPTY} stand-in and an empty identity.

* * @param pIngredient {@link Ingredient} * @param pCount The count for how items are in this stack. Only a max of 64 is valid, similar to ItemStack. @@ -42,14 +77,46 @@ public class IngredientStack { public IngredientStack(Ingredient pIngredient, int pCount) { this.ingredient = pIngredient; this.count = Math.min(pCount, 64); - Ingredient.Value value = pIngredient.values[0]; - if (value instanceof Ingredient.TagValue tagValue) { - this.registryName = tagValue.tag().location(); - } else if (value instanceof Ingredient.ItemValue itemValue) { - this.registryName = BuiltInRegistries.ITEM.getKey(itemValue.item().getItem()); + if (pIngredient.isCustom()) { + this.identity = pIngredient.getCustomIngredient(); + this.registryName = CUSTOM; } else { - throw new IllegalArgumentException("Ingredient value is neither an item nor a tag value."); + this.identity = Arrays.stream(pIngredient.values) + .map(IngredientStack::valueIdentity) + .sorted() + .collect(Collectors.toUnmodifiableList()); + this.registryName = pIngredient.values.length > 0 ? valueLocation(pIngredient.values[0]) : EMPTY; + } + } + + /** + * The identity entry an Ingredient value contributes: its {@linkplain #valueLocation(Ingredient.Value) + * declared location} prefixed with the value's kind ({@code "tag:"} or {@code "item:"}). A tag and an item + * may legally share a location, so the bare location is not enough to keep them distinct. + * + * @param pValue {@link Ingredient.Value} + * @return the kind-prefixed declared location, never {@code null} + */ + private static String valueIdentity(Ingredient.Value pValue) { + String kind = pValue instanceof Ingredient.TagValue ? "tag" : "item"; + return kind + ":" + valueLocation(pValue); + } + + /** + * The location an Ingredient value declares: a tag value yields the tag's location, an item value yields the + * item's registry name. Neither requires resolving the ingredient's items, so this is safe at recipe-decode + * time. + * + * @param pValue {@link Ingredient.Value} + * @return the declared {@link ResourceLocation}, never {@code null} + */ + private static ResourceLocation valueLocation(Ingredient.Value pValue) { + if (pValue instanceof Ingredient.TagValue tagValue) { + return tagValue.tag().location(); + } else if (pValue instanceof Ingredient.ItemValue itemValue) { + return BuiltInRegistries.ITEM.getKey(itemValue.item().getItem()); } + throw new IllegalArgumentException("Ingredient value is neither an item nor a tag value."); } public IngredientStack(Ingredient pIngredient) { @@ -163,7 +230,10 @@ public boolean isEmpty() { } /** - * Determines object equality of this IngredientStack against another object based on {@link ResourceLocation#equals(Object)}. + * Determines object equality of this IngredientStack against another object. Two IngredientStacks are equal + * when they share the same {@link #getCount() count} and the same {@link #identity full ingredient identity}, + * so two multi-item ingredients that merely share a first item are not equal, and two custom ingredients are + * only equal when their {@link ICustomIngredient}s are. * * @param pObject Object * @return boolean @@ -174,18 +244,19 @@ public boolean equals(Object pObject) { if (!(pObject instanceof IngredientStack that)) return false; if (getCount() != that.getCount()) return false; - return getRegistryName().equals(that.getRegistryName()); + return identity.equals(that.identity); } /** - * Calculates the hash code for this IngredientStack based on its {@link ResourceLocation registryName} hash code. + * Calculates the hash code for this IngredientStack based on its {@link #getCount() count} and its + * {@link #identity full ingredient identity}. * * @return int */ @Override public int hashCode() { int result = getCount(); - result = 31 * result + getRegistryName().hashCode(); + result = 31 * result + identity.hashCode(); return result; } diff --git a/src/test/java/com/smashingmods/alchemylib/api/item/IngredientStackTest.java b/src/test/java/com/smashingmods/alchemylib/api/item/IngredientStackTest.java index df8ef00..3409feb 100644 --- a/src/test/java/com/smashingmods/alchemylib/api/item/IngredientStackTest.java +++ b/src/test/java/com/smashingmods/alchemylib/api/item/IngredientStackTest.java @@ -1,16 +1,24 @@ package com.smashingmods.alchemylib.api.item; import com.smashingmods.alchemylib.testsupport.BootstrappedTest; +import net.minecraft.core.registries.Registries; import net.minecraft.network.RegistryFriendlyByteBuf; import net.minecraft.resources.ResourceLocation; import net.minecraft.tags.ItemTags; +import net.minecraft.tags.TagKey; +import net.minecraft.world.item.Item; import net.minecraft.world.item.ItemStack; import net.minecraft.world.item.Items; import net.minecraft.world.item.crafting.Ingredient; +import net.minecraft.world.level.ItemLike; +import net.neoforged.neoforge.common.crafting.CompoundIngredient; import org.junit.jupiter.api.Test; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -19,14 +27,25 @@ /** * Tests for {@link IngredientStack}: the constructor reads the access-transformer-opened * {@code Ingredient.values} field, so these need the Minecraft classpath and a populated item registry - * (hence {@link BootstrappedTest}). All ingredients are built from vanilla {@link Items} and {@link ItemTags}. + * (hence {@link BootstrappedTest}). All ingredients are built from vanilla {@link Items} and {@link ItemTags} + * (plus NeoForge's {@link CompoundIngredient} for the custom-ingredient paths). * They also pin the {@code toNetwork}/{@code fromNetwork} round trip -- the persistence seam the recipe packets * depend on -- and that {@code toStacks} does not mutate the Ingredient's shared cached stacks. * - *

The constructor's {@code else throw IllegalArgumentException} branch (value neither item nor tag) is - * unreachable for real ingredients -- an empty {@code Ingredient.of()} has an empty {@code values} array, so - * {@code values[0]} is an {@code ArrayIndexOutOfBoundsException} rather than that exception -- so it is left - * untested on purpose.

+ *

NeoForge custom ingredients and the empty {@code Ingredient.of()} both have an empty {@code values} array, + * so the constructor must not read {@code values[0]} blindly -- doing so threw an + * {@code ArrayIndexOutOfBoundsException} while recipes were being decoded. Nor may it fall back to resolving + * {@code Ingredient.getItems()}: at decode time registry tags are not yet bound, so that would memoize NeoForge's + * barrier "Empty Tag" placeholders. Both shapes instead map to mod-namespaced stand-in registry names, pinned + * below. The {@code throw IllegalArgumentException} branch (a declared value that is neither item nor tag) + * stays unreachable for real ingredients, so it is left untested on purpose.

+ * + *

Equality is keyed on the count plus the Ingredient's full identity (the declared value locations, each + * prefixed with its kind -- {@code tag:}/{@code item:} -- and sorted, or the {@code ICustomIngredient} itself), + * so the multi-item cases below pin that two ingredients sharing only a first item are not equal while the same + * item set in any order is, the tag-vs-item case pins that a tag and an item sharing a location stay distinct, + * and the custom cases pin that two different custom ingredients stay distinct -- with first-value-only equality + * they collapsed to one stand-in name and hash-based recipe-input sets silently dropped inputs.

*/ class IngredientStackTest extends BootstrappedTest { @@ -65,6 +84,37 @@ void getRegistryName_tagValue_isTagLocation() { assertEquals(ResourceLocation.fromNamespaceAndPath("minecraft", "planks"), stack.getRegistryName()); } + @Test + void constructor_emptyIngredient_doesNotThrow() { + // Regression for the ArrayIndexOutOfBoundsException at IngredientStack.: an empty Ingredient has an + // empty values[] array (the shape NeoForge custom ingredients also take), which crashed recipe decode. + assertDoesNotThrow(() -> new IngredientStack(Ingredient.of())); + } + + @Test + void getRegistryName_emptyIngredient_isEmptyStandIn() { + // Pins the same empty-values shape as the no-throw test above: the registry name must be the + // mod-namespaced stand-in, not anything resolved through getItems() at decode time. + IngredientStack stack = new IngredientStack(Ingredient.of()); + + assertEquals(ResourceLocation.fromNamespaceAndPath("alchemylib", "empty"), stack.getRegistryName()); + } + + @Test + void constructor_customIngredient_doesNotThrow() { + // Regression for the ArrayIndexOutOfBoundsException at IngredientStack. (#12): a NeoForge custom + // ingredient constructs with values = new Value[0], the exact shape that crashed world creation in packs + // whose recipes use compound/custom ingredients. + assertDoesNotThrow(() -> new IngredientStack(compoundOf(Items.STONE, Items.DIRT))); + } + + @Test + void getRegistryName_customIngredient_isCustomStandIn() { + IngredientStack stack = new IngredientStack(compoundOf(Items.STONE, Items.DIRT)); + + assertEquals(ResourceLocation.fromNamespaceAndPath("alchemylib", "custom"), stack.getRegistryName()); + } + @Test void equalsAndHashCode_sameCountAndRegistryName_areEqual() { IngredientStack first = new IngredientStack(Ingredient.of(Items.STONE), 4); @@ -90,6 +140,69 @@ void equals_differentRegistryName_areNotEqual() { assertNotEquals(first, second); } + @Test + void equals_tagAndItemSharingLocation_areNotEqual() { + // Kind-discriminator regression: a tag and an item may legally share a ResourceLocation. Identity entries + // are kind-prefixed ("tag:"/"item:") so these two must not compare equal -- with bare locations they + // collided and hash-based recipe-input sets silently merged distinct inputs. + TagKey stoneTag = TagKey.create(Registries.ITEM, ResourceLocation.fromNamespaceAndPath("minecraft", "stone")); + IngredientStack tagBacked = new IngredientStack(Ingredient.of(stoneTag), 4); + IngredientStack itemBacked = new IngredientStack(Ingredient.of(Items.STONE), 4); + + assertNotEquals(tagBacked, itemBacked); + } + + @Test + void equals_multiItemSharingFirstItem_areNotEqual() { + // Identity must reflect the whole declared value set, not just the first item: these two share IRON_INGOT + // as their first item but differ on the second, so they must not collide in hash-based dedup/lookup. + IngredientStack first = new IngredientStack(Ingredient.of(Items.IRON_INGOT, Items.GOLD_INGOT), 4); + IngredientStack second = new IngredientStack(Ingredient.of(Items.IRON_INGOT, Items.COPPER_INGOT), 4); + + assertNotEquals(first, second); + } + + @Test + void equalsAndHashCode_sameItemSetAndCount_areEqual() { + // Same full item set (regardless of declared order) and same count: equal, with matching hash codes. + IngredientStack first = new IngredientStack(Ingredient.of(Items.IRON_INGOT, Items.GOLD_INGOT), 4); + IngredientStack second = new IngredientStack(Ingredient.of(Items.GOLD_INGOT, Items.IRON_INGOT), 4); + + assertEquals(first, second); + assertEquals(first.hashCode(), second.hashCode()); + } + + @Test + void equalsAndHashCode_structurallySameCustomIngredients_areEqual() { + // Custom ingredients are identified by their ICustomIngredient. NeoForge's CompoundIngredient is a record, + // so two separately built but structurally identical compounds compare equal. + IngredientStack first = new IngredientStack(compoundOf(Items.STONE, Items.DIRT), 4); + IngredientStack second = new IngredientStack(compoundOf(Items.STONE, Items.DIRT), 4); + + assertEquals(first, second); + assertEquals(first.hashCode(), second.hashCode()); + } + + @Test + void equals_differentCustomIngredients_areNotEqual() { + IngredientStack first = new IngredientStack(compoundOf(Items.IRON_INGOT, Items.GOLD_INGOT), 4); + IngredientStack second = new IngredientStack(compoundOf(Items.STONE, Items.DIRT), 4); + + assertNotEquals(first, second); + } + + @Test + void linkedHashSet_differentCustomIngredients_keepsBoth() { + // Recipe-input-drop regression: recipes collect their inputs into a LinkedHashSet. With + // equality keyed on a single resolved name, every unresolvable custom ingredient collapsed to the same + // stand-in and the set silently dropped all but the first input of such a recipe. + Set inputs = new LinkedHashSet<>(); + inputs.add(new IngredientStack(compoundOf(Items.IRON_INGOT, Items.GOLD_INGOT), 4)); + inputs.add(new IngredientStack(compoundOf(Items.STONE, Items.DIRT), 4)); + + assertEquals(2, inputs.size()); + } + @Test void toStacks_appliesCountWithoutMutatingIngredientCache() { Ingredient ingredient = Ingredient.of(Items.STONE); @@ -128,4 +241,15 @@ private static IngredientStack encodeDecodeNetwork(IngredientStack stack) { stack.toNetwork(buffer); return IngredientStack.fromNetwork(buffer); } + + /** + * A real NeoForge custom ingredient: {@link CompoundIngredient#of} with two children wraps a + * {@code CompoundIngredient} in a vanilla {@code Ingredient} whose {@code values} array is empty and whose + * {@code isCustom()} is true -- the exact shape recipe decode hands to the IngredientStack constructor. + * Only construction is exercised here; serializing it would need the neoforge:ingredient_type registry, + * which the bare bootstrap does not populate. + */ + private static Ingredient compoundOf(ItemLike pFirst, ItemLike pSecond) { + return CompoundIngredient.of(Ingredient.of(pFirst), Ingredient.of(pSecond)); + } }