From 1a4e83f45349a918d28ef46f6b0302c0e70ca3d7 Mon Sep 17 00:00:00 2001 From: TelepathicGrunt <40846040+TelepathicGrunt@users.noreply.github.com> Date: Sun, 7 Jul 2024 10:01:36 -0400 Subject: [PATCH] Split BuildCreativeModeTabContentsEvent internals to operate on two separate sets (#1156) --- .../InsertableLinkedOpenCustomHashSet.java | 155 +++++++++++++ .../common/util/MutableHashedLinkedMap.java | 40 +--- .../common/util/strategy/BasicStrategy.java | 28 +++ .../util/strategy/IdentityStrategy.java | 27 +++ .../common/util/strategy/package-info.java | 13 ++ .../BuildCreativeModeTabContentsEvent.java | 149 ++++++++++++- .../neoforged/neoforge/event/EventHooks.java | 44 ++-- .../unittest/CreativeTabOrderTest.java | 205 ++++++++++++++++-- ...InsertableLinkedOpenCustomHashSetTest.java | 80 +++++++ .../neoforge/oldtest/CreativeModeTabTest.java | 29 +-- 10 files changed, 668 insertions(+), 102 deletions(-) create mode 100644 src/main/java/net/neoforged/neoforge/common/util/InsertableLinkedOpenCustomHashSet.java create mode 100644 src/main/java/net/neoforged/neoforge/common/util/strategy/BasicStrategy.java create mode 100644 src/main/java/net/neoforged/neoforge/common/util/strategy/IdentityStrategy.java create mode 100644 src/main/java/net/neoforged/neoforge/common/util/strategy/package-info.java create mode 100644 tests/src/junit/java/net/neoforged/neoforge/unittest/InsertableLinkedOpenCustomHashSetTest.java diff --git a/src/main/java/net/neoforged/neoforge/common/util/InsertableLinkedOpenCustomHashSet.java b/src/main/java/net/neoforged/neoforge/common/util/InsertableLinkedOpenCustomHashSet.java new file mode 100644 index 0000000000..c3049af8b1 --- /dev/null +++ b/src/main/java/net/neoforged/neoforge/common/util/InsertableLinkedOpenCustomHashSet.java @@ -0,0 +1,155 @@ +/* + * Copyright (c) NeoForged and contributors + * SPDX-License-Identifier: LGPL-2.1-only + */ + +package net.neoforged.neoforge.common.util; + +import it.unimi.dsi.fastutil.Hash; +import it.unimi.dsi.fastutil.HashCommon; +import it.unimi.dsi.fastutil.objects.ObjectLinkedOpenCustomHashSet; +import net.neoforged.neoforge.common.util.strategy.BasicStrategy; +import net.neoforged.neoforge.common.util.strategy.IdentityStrategy; + +/** + * Special linked hash set that allow changing the order of its entries and is strict to throw if attempting to add an entry that already exists. + * Requires a strategy for the hashing behavior. Use {@link BasicStrategy#BASIC} or {@link IdentityStrategy#IDENTITY} if no special hashing needed. + */ +public class InsertableLinkedOpenCustomHashSet extends ObjectLinkedOpenCustomHashSet { + /** The number of bits which take up the space for the previous or next position. */ + private static final long LINK_BIT_SPACE = 32L; + /** The bitmask for the next element's position. */ + private static final long NEXT_LINK = 0x00000000FFFFFFFFL; + /** The bitmask for the previous element's position. */ + private static final long PREV_LINK = 0xFFFFFFFF00000000L; + + /** + * Constructs a new {@link InsertableLinkedOpenCustomHashSet} with a {@link BasicStrategy}. + */ + public InsertableLinkedOpenCustomHashSet() { + super(BasicStrategy.BASIC); + } + + /** + * Constructs a new {@link InsertableLinkedOpenCustomHashSet} with the given {@link Hash.Strategy}. + * + * @param strategy The strategy to use for adding and getting elements from the set. + */ + public InsertableLinkedOpenCustomHashSet(Hash.Strategy strategy) { + super(strategy); + } + + /** + * This method will attempt to add {@code element} after the given element {@code insertAfter} in the set. If an + * element matching {@code insertAfter} cannot be found with this set's {@link Hash.Strategy}, then {@code element} + * will be added in insertion order. If {#code element} already exists in the set, then the set is not modified. + * + * @param insertAfter The element to insert {@code element} after. + * @param element The element to add into this set. + * @return {@code true} if the element was added to the set. + */ + public boolean addAfter(T insertAfter, T element) { + int afterPos; + // Use the default return if afterPos == last. Otherwise, special checks would have to be done. + if (contains(insertAfter) && (last != (afterPos = getPos(insertAfter)))) { + int pos = HashCommon.mix(strategy.hashCode(element)) & mask; + T curr = key[pos]; + // If an element exists in this pos, shift index until an empty space is found or return false if it matches. + if (curr != null) { + do { + if (strategy.equals(curr, element)) { + return false; + } + } while (!((curr = key[pos = (pos + 1) & mask]) == null)); + } + key[pos] = element; + + // This chunk inserts the new pos in-between insertAfter and it's next link. + long nextPos = link[afterPos] & NEXT_LINK; + // Fix the previous link for the next element to point back to this pos. + link[(int) nextPos] ^= (link[(int) nextPos] ^ ((pos & NEXT_LINK) << LINK_BIT_SPACE)) & PREV_LINK; + // Fix the next link for insertAfter to point forward to this pos. + link[afterPos] ^= (link[afterPos] ^ (pos & NEXT_LINK)) & NEXT_LINK; + // Set this pos to point back to insertAfter and forward to the next element. + link[pos] = ((afterPos & NEXT_LINK) << LINK_BIT_SPACE) | nextPos; + + if (size++ >= maxFill) { + rehash(HashCommon.arraySize(size + 1, f)); + } + return true; + } + return add(element); + } + + /** + * This method will attempt to add {@code element} before the given element {@code insertBefore} in the set. If an + * element matching {@code insertBefore} cannot be found with this set's {@link Hash.Strategy}, then {@code element} + * will be added in insertion order. If {#code element} already exists in the set, then the set is not modified. + * + * @param insertBefore The element to insert {@code element} before. + * @param element The element to add into this set. + * @return {@code true} if the element was added to the set. + */ + public boolean addBefore(T insertBefore, T element) { + if (contains(insertBefore)) { + int beforePos = getPos(insertBefore); + // Use this method instead so special logic isn't needed for handling invalid "previous" links. + if (beforePos == first) { + if (contains(element)) { + return false; + } + return addAndMoveToFirst(element); + } + int pos = HashCommon.mix(strategy.hashCode(element)) & mask; + T curr = key[pos]; + // If an element exists in this pos, shift index until an empty space is found or return false if it matches. + if (curr != null) { + do { + if (strategy.equals(curr, element)) { + return false; + } + } while (!((curr = key[pos = (pos + 1) & mask]) == null)); + } + key[pos] = element; + + // This chunk inserts the new pos in-between insertBefore and it's previous link. + long prevPos = ((link[beforePos] & PREV_LINK) >> LINK_BIT_SPACE); + // Fix the next link for the previous element to point forward to this pos. + link[(int) prevPos] ^= (link[(int) prevPos] ^ (pos & NEXT_LINK)) & NEXT_LINK; + // Fix the previous link for insertBefore to point back to this pos. + link[beforePos] ^= (link[beforePos] ^ ((pos & NEXT_LINK) << LINK_BIT_SPACE)) & PREV_LINK; + // Set this pos to point back to the previous element and forward to insertBefore. + link[pos] = (prevPos << LINK_BIT_SPACE) | (beforePos & NEXT_LINK); + + if (size++ >= maxFill) { + rehash(HashCommon.arraySize(size + 1, f)); + } + return true; + } + return add(element); + } + + @Override + public void addFirst(T element) { + addAndMoveToFirst(element); + } + + @Override + public void addLast(T element) { + addAndMoveToLast(element); + } + + /** + * Requires that {@code existingElement} exists in the set already. + */ + private int getPos(T existingElement) { + int pos = HashCommon.mix(strategy.hashCode(existingElement)) & mask; + T curr = key[pos]; + do { + if (strategy.equals(curr, existingElement)) { + break; + } + } while (!((curr = key[pos = (pos + 1) & mask]) == null)); + return pos; + } +} diff --git a/src/main/java/net/neoforged/neoforge/common/util/MutableHashedLinkedMap.java b/src/main/java/net/neoforged/neoforge/common/util/MutableHashedLinkedMap.java index da2026b48d..84e6da496a 100644 --- a/src/main/java/net/neoforged/neoforge/common/util/MutableHashedLinkedMap.java +++ b/src/main/java/net/neoforged/neoforge/common/util/MutableHashedLinkedMap.java @@ -11,8 +11,8 @@ import java.util.Iterator; import java.util.Map; import java.util.NoSuchElementException; -import java.util.Objects; import java.util.function.BiPredicate; +import net.neoforged.neoforge.common.util.strategy.BasicStrategy; import org.jetbrains.annotations.Nullable; /** @@ -22,16 +22,6 @@ * @param the type of mapped values */ public class MutableHashedLinkedMap implements Iterable> { - /** - * A strategy that uses {@link Objects#hashCode(Object)} and {@link Object#equals(Object)}. - */ - public static final Strategy BASIC = new BasicStrategy(); - - /** - * A strategy that uses {@link System#identityHashCode(Object)} and {@code a == b} comparisons. - */ - public static final Strategy IDENTITY = new IdentityStrategy(); - private final Strategy strategy; private final Map entries; private final MergeFunction merge; @@ -45,10 +35,10 @@ public class MutableHashedLinkedMap implements Iterable> { private transient int changes = 0; /** - * Creates a new instance using the {@link #BASIC} strategy. + * Creates a new instance using the {@link BasicStrategy#BASIC} strategy. */ public MutableHashedLinkedMap() { - this(BASIC); + this(BasicStrategy.BASIC); } /** @@ -392,28 +382,4 @@ public int hashCode() { (value == null ? 0 : value.hashCode()); } } - - private static class BasicStrategy implements Strategy { - @Override - public int hashCode(Object o) { - return Objects.hashCode(o); - } - - @Override - public boolean equals(Object a, Object b) { - return Objects.equals(a, b); - } - } - - private static class IdentityStrategy implements Strategy { - @Override - public int hashCode(Object o) { - return System.identityHashCode(o); - } - - @Override - public boolean equals(Object a, Object b) { - return a == b; - } - } } diff --git a/src/main/java/net/neoforged/neoforge/common/util/strategy/BasicStrategy.java b/src/main/java/net/neoforged/neoforge/common/util/strategy/BasicStrategy.java new file mode 100644 index 0000000000..30498bca72 --- /dev/null +++ b/src/main/java/net/neoforged/neoforge/common/util/strategy/BasicStrategy.java @@ -0,0 +1,28 @@ +/* + * Copyright (c) NeoForged and contributors + * SPDX-License-Identifier: LGPL-2.1-only + */ + +package net.neoforged.neoforge.common.util.strategy; + +import it.unimi.dsi.fastutil.Hash; +import java.util.Objects; + +/** + * A strategy that uses {@link Objects#hashCode(Object)} and {@link Object#equals(Object)}. + */ +public class BasicStrategy implements Hash.Strategy { + public static final Hash.Strategy BASIC = new BasicStrategy(); + + private BasicStrategy() {} + + @Override + public int hashCode(Object o) { + return Objects.hashCode(o); + } + + @Override + public boolean equals(Object a, Object b) { + return Objects.equals(a, b); + } +} diff --git a/src/main/java/net/neoforged/neoforge/common/util/strategy/IdentityStrategy.java b/src/main/java/net/neoforged/neoforge/common/util/strategy/IdentityStrategy.java new file mode 100644 index 0000000000..1381184d65 --- /dev/null +++ b/src/main/java/net/neoforged/neoforge/common/util/strategy/IdentityStrategy.java @@ -0,0 +1,27 @@ +/* + * Copyright (c) NeoForged and contributors + * SPDX-License-Identifier: LGPL-2.1-only + */ + +package net.neoforged.neoforge.common.util.strategy; + +import it.unimi.dsi.fastutil.Hash; + +/** + * A strategy that uses {@link System#identityHashCode(Object)} and {@code a == b} comparisons. + */ +public class IdentityStrategy implements Hash.Strategy { + public static final Hash.Strategy IDENTITY = new IdentityStrategy(); + + private IdentityStrategy() {} + + @Override + public int hashCode(Object o) { + return System.identityHashCode(o); + } + + @Override + public boolean equals(Object a, Object b) { + return a == b; + } +} diff --git a/src/main/java/net/neoforged/neoforge/common/util/strategy/package-info.java b/src/main/java/net/neoforged/neoforge/common/util/strategy/package-info.java new file mode 100644 index 0000000000..8f0691c9b2 --- /dev/null +++ b/src/main/java/net/neoforged/neoforge/common/util/strategy/package-info.java @@ -0,0 +1,13 @@ +/* + * Copyright (c) NeoForged and contributors + * SPDX-License-Identifier: LGPL-2.1-only + */ + +@FieldsAreNonnullByDefault +@MethodsReturnNonnullByDefault +@ParametersAreNonnullByDefault +package net.neoforged.neoforge.common.util.strategy; + +import javax.annotation.ParametersAreNonnullByDefault; +import net.minecraft.FieldsAreNonnullByDefault; +import net.minecraft.MethodsReturnNonnullByDefault; diff --git a/src/main/java/net/neoforged/neoforge/event/BuildCreativeModeTabContentsEvent.java b/src/main/java/net/neoforged/neoforge/event/BuildCreativeModeTabContentsEvent.java index f7b099cdbc..cee33ee915 100644 --- a/src/main/java/net/neoforged/neoforge/event/BuildCreativeModeTabContentsEvent.java +++ b/src/main/java/net/neoforged/neoforge/event/BuildCreativeModeTabContentsEvent.java @@ -5,13 +5,15 @@ package net.neoforged.neoforge.event; +import it.unimi.dsi.fastutil.objects.ObjectSortedSet; +import it.unimi.dsi.fastutil.objects.ObjectSortedSets; import net.minecraft.resources.ResourceKey; import net.minecraft.world.flag.FeatureFlagSet; import net.minecraft.world.item.CreativeModeTab; import net.minecraft.world.item.ItemStack; import net.neoforged.bus.api.Event; import net.neoforged.fml.event.IModBusEvent; -import net.neoforged.neoforge.common.util.MutableHashedLinkedMap; +import net.neoforged.neoforge.common.util.InsertableLinkedOpenCustomHashSet; import org.jetbrains.annotations.ApiStatus; /** @@ -24,15 +26,17 @@ public final class BuildCreativeModeTabContentsEvent extends Event implements IModBusEvent, CreativeModeTab.Output { private final CreativeModeTab tab; private final CreativeModeTab.ItemDisplayParameters parameters; - private final MutableHashedLinkedMap entries; + private final InsertableLinkedOpenCustomHashSet parentEntries; + private final InsertableLinkedOpenCustomHashSet searchEntries; private final ResourceKey tabKey; @ApiStatus.Internal - public BuildCreativeModeTabContentsEvent(CreativeModeTab tab, ResourceKey tabKey, CreativeModeTab.ItemDisplayParameters parameters, MutableHashedLinkedMap entries) { + public BuildCreativeModeTabContentsEvent(CreativeModeTab tab, ResourceKey tabKey, CreativeModeTab.ItemDisplayParameters parameters, InsertableLinkedOpenCustomHashSet parentEntries, InsertableLinkedOpenCustomHashSet searchEntries) { this.tab = tab; this.tabKey = tabKey; this.parameters = parameters; - this.entries = entries; + this.parentEntries = parentEntries; + this.searchEntries = searchEntries; } /** @@ -61,12 +65,141 @@ public boolean hasPermissions() { return this.parameters.hasPermissions(); } - public MutableHashedLinkedMap getEntries() { - return this.entries; + /** + * The current immutable ordered set of the parent tab entries in the order to be added to the Creative Menu. + * Purely for querying to see what in it. Please use the other event methods for modifications. + */ + public ObjectSortedSet getParentEntries() { + return ObjectSortedSets.unmodifiable(this.parentEntries); + } + + /** + * The current immutable ordered set of the search tab entries in the order to be added to the Creative Menu. + * Purely for querying to see what in it. Please use the other event methods for modifications. + */ + public ObjectSortedSet getSearchEntries() { + return ObjectSortedSets.unmodifiable(this.searchEntries); } + /** + * Inserts the new stack at the end of the given tab at this point in time. + * + * @throws IllegalArgumentException if the new itemstack's count is not 1 or entry already was added to the tab previously. + */ @Override - public void accept(ItemStack stack, CreativeModeTab.TabVisibility visibility) { - getEntries().put(stack, visibility); + public void accept(ItemStack newEntry, CreativeModeTab.TabVisibility visibility) { + assertStackCount(newEntry); + + if (isParentTab(visibility)) { + assertNewEntryDoesNotAlreadyExists(parentEntries, newEntry); + parentEntries.add(newEntry); + } + + if (isSearchTab(visibility)) { + assertNewEntryDoesNotAlreadyExists(searchEntries, newEntry); + searchEntries.add(newEntry); + } + } + + /** + * Inserts the new entry after the specified existing entry. + * + * @throws IllegalArgumentException if the new itemstack's count is not 1 or target does not exist in set + * OR if the existing entry is not found in the tab's lists. + */ + public void insertAfter(ItemStack existingEntry, ItemStack newEntry, CreativeModeTab.TabVisibility visibility) { + assertStackCount(newEntry); + + if (isParentTab(visibility)) { + assertTargetExists(parentEntries, existingEntry); + assertNewEntryDoesNotAlreadyExists(parentEntries, newEntry); + parentEntries.addBefore(existingEntry, newEntry); + } + + if (isSearchTab(visibility)) { + assertTargetExists(searchEntries, existingEntry); + assertNewEntryDoesNotAlreadyExists(searchEntries, newEntry); + searchEntries.addBefore(existingEntry, newEntry); + } + } + + /** + * Inserts the new entry before the specified existing entry. + * + * @throws IllegalArgumentException if the new itemstack's count is not 1 or target does not exist in set + * OR if the existing entry is not found in the tab's lists. + */ + public void insertBefore(ItemStack existingEntry, ItemStack newEntry, CreativeModeTab.TabVisibility visibility) { + assertStackCount(newEntry); + + if (isParentTab(visibility)) { + assertTargetExists(parentEntries, existingEntry); + assertNewEntryDoesNotAlreadyExists(parentEntries, newEntry); + parentEntries.addAfter(existingEntry, newEntry); + } + + if (isSearchTab(visibility)) { + assertTargetExists(searchEntries, existingEntry); + assertNewEntryDoesNotAlreadyExists(searchEntries, newEntry); + searchEntries.addAfter(existingEntry, newEntry); + } + } + + /** + * Inserts the new entry in the front of the tab's content. + * + * @throws IllegalArgumentException if the new itemstack's count is not 1 or entry already was added to the tab previously. + */ + public void insertFirst(ItemStack newEntry, CreativeModeTab.TabVisibility visibility) { + assertStackCount(newEntry); + + if (isParentTab(visibility)) { + assertNewEntryDoesNotAlreadyExists(parentEntries, newEntry); + parentEntries.addFirst(newEntry); + } + + if (isSearchTab(visibility)) { + assertNewEntryDoesNotAlreadyExists(searchEntries, newEntry); + searchEntries.addFirst(newEntry); + } + } + + /** + * Removes an entry from the tab's content. + */ + public void remove(ItemStack existingEntry, CreativeModeTab.TabVisibility visibility) { + if (isParentTab(visibility)) { + parentEntries.remove(existingEntry); + } + + if (isSearchTab(visibility)) { + searchEntries.remove(existingEntry); + } + } + + static boolean isParentTab(CreativeModeTab.TabVisibility visibility) { + return visibility == CreativeModeTab.TabVisibility.PARENT_TAB_ONLY || visibility == CreativeModeTab.TabVisibility.PARENT_AND_SEARCH_TABS; + } + + static boolean isSearchTab(CreativeModeTab.TabVisibility visibility) { + return visibility == CreativeModeTab.TabVisibility.SEARCH_TAB_ONLY || visibility == CreativeModeTab.TabVisibility.PARENT_AND_SEARCH_TABS; + } + + private void assertTargetExists(InsertableLinkedOpenCustomHashSet setToCheck, ItemStack existingEntry) { + if (!setToCheck.contains(existingEntry)) { + throw new IllegalArgumentException("Itemstack " + existingEntry + " does not exist in tab's list"); + } + } + + private void assertNewEntryDoesNotAlreadyExists(InsertableLinkedOpenCustomHashSet setToCheck, ItemStack newEntry) { + if (setToCheck.contains(newEntry)) { + throw new IllegalArgumentException("Itemstack " + newEntry + " already exists in the tab's list"); + } + } + + private static void assertStackCount(ItemStack newEntry) { + if (newEntry.getCount() != 1) { + throw new IllegalArgumentException("The stack count must be 1 for " + newEntry); + } } } diff --git a/src/main/java/net/neoforged/neoforge/event/EventHooks.java b/src/main/java/net/neoforged/neoforge/event/EventHooks.java index 760e395aa0..72bd60a0cd 100644 --- a/src/main/java/net/neoforged/neoforge/event/EventHooks.java +++ b/src/main/java/net/neoforged/neoforge/event/EventHooks.java @@ -9,7 +9,6 @@ import com.mojang.brigadier.CommandDispatcher; import com.mojang.datafixers.util.Either; import com.mojang.serialization.DynamicOps; -import it.unimi.dsi.fastutil.objects.ObjectLinkedOpenCustomHashSet; import java.io.File; import java.util.EnumSet; import java.util.List; @@ -112,7 +111,7 @@ import net.neoforged.neoforge.common.extensions.IFluidStateExtension; import net.neoforged.neoforge.common.extensions.IOwnedSpawner; import net.neoforged.neoforge.common.util.BlockSnapshot; -import net.neoforged.neoforge.common.util.MutableHashedLinkedMap; +import net.neoforged.neoforge.common.util.InsertableLinkedOpenCustomHashSet; import net.neoforged.neoforge.event.brewing.PlayerBrewedPotionEvent; import net.neoforged.neoforge.event.brewing.PotionBrewEvent; import net.neoforged.neoforge.event.enchanting.EnchantmentLevelSetEvent; @@ -1083,34 +1082,31 @@ public static ItemEnchantments getAllEnchantmentLevels(ItemEnchantments enchantm */ @ApiStatus.Internal public static void onCreativeModeTabBuildContents(CreativeModeTab tab, ResourceKey tabKey, CreativeModeTab.DisplayItemsGenerator originalGenerator, CreativeModeTab.ItemDisplayParameters params, CreativeModeTab.Output output) { - final var searchDupes = new ObjectLinkedOpenCustomHashSet(ItemStackLinkedSet.TYPE_AND_TAG); - // The ItemStackLinkedSet.TYPE_AND_TAG strategy cannot be used for the MutableHashedLinkedMap due to vanilla - // adding multiple identical ItemStacks with different TabVisibility values. The values also cannot be merged - // because it does not abide by the intended order. For example, vanilla adds all max enchanted books to the - // "ingredient" tab with "parent only" visibility, then also adds all enchanted books again in increasing order - // to their max values but with the "search only" visibility. Because the parent-only is added first and then - // the search-only entries are added after, the max enchantments would show up first and then the enchantments - // in increasing order up to max-1. - final var entries = new MutableHashedLinkedMap(MutableHashedLinkedMap.BASIC, (stack, tabVisibility) -> { - if (!searchDupes.add(stack) && tabVisibility != CreativeModeTab.TabVisibility.SEARCH_TAB_ONLY) { - throw new IllegalStateException( - "Accidentally adding the same item stack twice " - + stack.getDisplayName().getString() - + " to a Creative Mode Tab: " - + tab.getDisplayName().getString()); - } - return true; - }); + final var parentEntries = new InsertableLinkedOpenCustomHashSet(ItemStackLinkedSet.TYPE_AND_TAG); + final var searchEntries = new InsertableLinkedOpenCustomHashSet(ItemStackLinkedSet.TYPE_AND_TAG); originalGenerator.accept(params, (stack, vis) -> { if (stack.getCount() != 1) throw new IllegalArgumentException("The stack count must be 1"); - entries.put(stack, vis); + + if (BuildCreativeModeTabContentsEvent.isParentTab(vis)) { + parentEntries.add(stack); + } + + if (BuildCreativeModeTabContentsEvent.isSearchTab(vis)) { + searchEntries.add(stack); + } }); - ModLoader.postEvent(new BuildCreativeModeTabContentsEvent(tab, tabKey, params, entries)); - for (var entry : entries) - output.accept(entry.getKey(), entry.getValue()); + ModLoader.postEvent(new BuildCreativeModeTabContentsEvent(tab, tabKey, params, parentEntries, searchEntries)); + + for (var entry : parentEntries) { + output.accept(entry, CreativeModeTab.TabVisibility.PARENT_TAB_ONLY); + } + + for (var entry : searchEntries) { + output.accept(entry, CreativeModeTab.TabVisibility.SEARCH_TAB_ONLY); + } } /** diff --git a/tests/src/junit/java/net/neoforged/neoforge/unittest/CreativeTabOrderTest.java b/tests/src/junit/java/net/neoforged/neoforge/unittest/CreativeTabOrderTest.java index b06f46c0e9..d9ed64be16 100644 --- a/tests/src/junit/java/net/neoforged/neoforge/unittest/CreativeTabOrderTest.java +++ b/tests/src/junit/java/net/neoforged/neoforge/unittest/CreativeTabOrderTest.java @@ -6,16 +6,19 @@ package net.neoforged.neoforge.unittest; import it.unimi.dsi.fastutil.objects.ObjectOpenCustomHashSet; -import java.util.Map; +import it.unimi.dsi.fastutil.objects.ObjectSortedSet; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import java.util.stream.IntStream; -import net.minecraft.core.HolderLookup; import net.minecraft.core.component.DataComponents; import net.minecraft.core.registries.Registries; +import net.minecraft.network.chat.Component; +import net.minecraft.resources.ResourceKey; +import net.minecraft.resources.ResourceLocation; import net.minecraft.server.MinecraftServer; import net.minecraft.tags.ItemTags; import net.minecraft.tags.TagKey; -import net.minecraft.world.flag.FeatureFlagSet; import net.minecraft.world.flag.FeatureFlags; import net.minecraft.world.item.CreativeModeTab; import net.minecraft.world.item.CreativeModeTabs; @@ -26,9 +29,12 @@ import net.minecraft.world.item.Items; import net.minecraft.world.item.enchantment.Enchantment; import net.minecraft.world.item.enchantment.EnchantmentInstance; +import net.minecraft.world.level.ItemLike; +import net.minecraft.world.level.block.Blocks; import net.neoforged.bus.api.IEventBus; import net.neoforged.fml.common.Mod; import net.neoforged.neoforge.event.BuildCreativeModeTabContentsEvent; +import net.neoforged.neoforge.registries.RegisterEvent; import net.neoforged.testframework.junit.EphemeralTestServerProvider; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; @@ -41,6 +47,7 @@ @TestMethodOrder(MethodOrderer.MethodName.class) public class CreativeTabOrderTest { public static final String MOD_ID = "creative_tab_order_test"; + private static final ResourceKey STONE_ORDERING = ResourceKey.create(Registries.CREATIVE_MODE_TAB, ResourceLocation.fromNamespaceAndPath(MOD_ID, "stone_ordering")); private static final Set> ENCHANTABLES = Set.of( ItemTags.FOOT_ARMOR_ENCHANTABLE, ItemTags.LEG_ARMOR_ENCHANTABLE, @@ -61,8 +68,20 @@ public class CreativeTabOrderTest { ItemTags.EQUIPPABLE_ENCHANTABLE, ItemTags.CROSSBOW_ENCHANTABLE, ItemTags.VANISHING_ENCHANTABLE); - public static Iterable> ingredientsTab; - public static Iterable> searchTab; + public static ObjectSortedSet ingredientsTab; + public static ObjectSortedSet searchTab; + public static ObjectSortedSet stoneParentTab; + public static ObjectSortedSet stoneSearchTab; + public static boolean stackCountExceptionForAccept = false; + public static boolean stackCountExceptionForInsertAfter = false; + public static boolean stackCountExceptionForInsertBefore = false; + public static boolean stackCountExceptionForInsertFirst = false; + public static boolean targetDoesNotExistExceptionForInsertAfter = false; + public static boolean targetDoesNotExistExceptionForInsertBefore = false; + public static boolean newEntryExistAlreadyExceptionForAccept = false; + public static boolean newEntryExistAlreadyExceptionForInsertFirst = false; + public static boolean newEntryExistAlreadyExceptionForInsertAfter = false; + public static boolean newEntryExistAlreadyExceptionForInsertBefore = false; @BeforeAll static void testSetupTabs(MinecraftServer server) { @@ -70,7 +89,7 @@ static void testSetupTabs(MinecraftServer server) { } /** - * The local tabEnchantments variable comes from {@link CreativeModeTabs#generateEnchantmentBookTypesOnlyMaxLevel(CreativeModeTab.Output, HolderLookup, Set, CreativeModeTab.TabVisibility, FeatureFlagSet)} + * The local tabEnchantments variable comes from {@link CreativeModeTabs#generateEnchantmentBookTypesOnlyMaxLevel} * * @param server Ephemeral server from extension */ @@ -79,12 +98,9 @@ void testIngredientsEnchantmentExistence(MinecraftServer server) { final Set tabEnchantments = server.registryAccess().lookupOrThrow(Registries.ENCHANTMENT).listElements() .map(enchantment -> EnchantedBookItem.createForEnchantment(new EnchantmentInstance(enchantment, enchantment.value().getMaxLevel()))) .collect(() -> new ObjectOpenCustomHashSet<>(ItemStackLinkedSet.TYPE_AND_TAG), ObjectOpenCustomHashSet::add, ObjectOpenCustomHashSet::addAll); - for (Map.Entry entry : ingredientsTab) { - if (entry.getValue() == CreativeModeTab.TabVisibility.SEARCH_TAB_ONLY) { - continue; - } - if (entry.getKey().getItem() == Items.ENCHANTED_BOOK) { - Assertions.assertTrue(tabEnchantments.remove(entry.getKey()), "Enchanted book present that does not exist in the default set?"); + for (ItemStack entry : ingredientsTab) { + if (entry.is(Items.ENCHANTED_BOOK)) { + Assertions.assertTrue(tabEnchantments.remove(entry), "Enchanted book present that does not exist in the default set?"); } } @@ -92,7 +108,7 @@ void testIngredientsEnchantmentExistence(MinecraftServer server) { } /** - * The local tabEnchantments variable comes from {@link CreativeModeTabs#generateEnchantmentBookTypesAllLevels(CreativeModeTab.Output, HolderLookup, Set, CreativeModeTab.TabVisibility, FeatureFlagSet)} + * The local tabEnchantments variable comes from {@link CreativeModeTabs#generateEnchantmentBookTypesAllLevels} * * @param server Ephemeral server from extension */ @@ -106,11 +122,11 @@ void testSearchEnchantmentOrder(MinecraftServer server) { Enchantment enchantment = null; int level = 0; - for (Map.Entry entry : searchTab) { - if (entry.getKey().getItem() != Items.ENCHANTED_BOOK) { + for (ItemStack entry : searchTab) { + if (!entry.is(Items.ENCHANTED_BOOK)) { continue; } - final var enchantmentEntry = entry.getKey().get(DataComponents.STORED_ENCHANTMENTS).entrySet().iterator().next(); + final var enchantmentEntry = entry.get(DataComponents.STORED_ENCHANTMENTS).entrySet().iterator().next(); final var entryEnchantment = enchantmentEntry.getKey().value(); final var entryEnchantmentLevel = enchantmentEntry.getIntValue(); if (enchantment == null || enchantment != entryEnchantment) { @@ -119,26 +135,177 @@ void testSearchEnchantmentOrder(MinecraftServer server) { } else { Assertions.assertTrue(entryEnchantmentLevel > level); } - Assertions.assertTrue(tabEnchantments.remove(entry.getKey()), "Enchanted book present that does not exist in the default set?"); + Assertions.assertTrue(tabEnchantments.remove(entry), "Enchanted book present that does not exist in the default set?"); level = entryEnchantmentLevel; } Assertions.assertTrue(tabEnchantments.isEmpty(), "Missing enchantments in Search tab."); } + /** + * Verifies that the tab sorting works properly where people can specify what item appears after what. + * + * @param server Ephemeral server from extension + */ + @Test + void testParentStoneOrder(MinecraftServer server) { + List desiredOrder = setupDesiredStoneOrder(); + for (ItemStack entry : stoneParentTab) { + Item currentDesiredItem = desiredOrder.removeFirst(); + if (!entry.is(currentDesiredItem)) { + Assertions.assertFalse(false, entry.getItem() + " is not the desired " + currentDesiredItem + " in the stone parent tab!"); + } + } + Assertions.assertTrue(desiredOrder.isEmpty(), "Not all sorted stones were found in stone parent tab!"); + + desiredOrder = setupDesiredStoneOrder(); + for (ItemStack entry : stoneSearchTab) { + Item currentDesiredItem = desiredOrder.removeFirst(); + if (!entry.is(currentDesiredItem)) { + Assertions.assertFalse(false, entry.getItem() + " is not the desired " + currentDesiredItem + " in the stone search tab!"); + } + } + Assertions.assertTrue(desiredOrder.isEmpty(), "Not all sorted stones were found in stone search tab!"); + } + + /** + * Makes sure the validation checks were triggered properly for problematic inputs into {@link BuildCreativeModeTabContentsEvent} + * + * @param server Ephemeral server from extension + */ + @Test + void testBuildCreativeModeTabContentsEventValidations(MinecraftServer server) { + Assertions.assertTrue(stackCountExceptionForAccept, "Accept method is missing itemstack validation where stack should be 1."); + Assertions.assertTrue(stackCountExceptionForInsertAfter, "Insert After method is missing itemstack validation where stack should be 1."); + Assertions.assertTrue(stackCountExceptionForInsertBefore, "Insert Before method is missing itemstack validation where stack should be 1."); + Assertions.assertTrue(stackCountExceptionForInsertFirst, "Insert First method is missing itemstack validation where stack should be 1."); + Assertions.assertTrue(targetDoesNotExistExceptionForInsertAfter, "Insert After method is missing target itemstack validation where target should exist."); + Assertions.assertTrue(targetDoesNotExistExceptionForInsertBefore, "Insert Before method is missing target itemstack validation where target should exist."); + Assertions.assertTrue(newEntryExistAlreadyExceptionForAccept, "Accept method is missing duplicate itemstack validation where entry should not be added twice."); + Assertions.assertTrue(newEntryExistAlreadyExceptionForInsertFirst, "Insert First method is missing duplicate itemstack validation where entry should not be added twice."); + Assertions.assertTrue(newEntryExistAlreadyExceptionForInsertAfter, "Insert After method is missing duplicate itemstack validation where entry should not be added twice."); + Assertions.assertTrue(newEntryExistAlreadyExceptionForInsertBefore, "Insert Before method is missing duplicate itemstack validation where entry should not be added twice."); + } + + private static List setupDesiredStoneOrder() { + List desiredOrder = new ArrayList<>(); + desiredOrder.add(Items.BASALT); + desiredOrder.add(Items.STONE); + desiredOrder.add(Items.TUFF); + desiredOrder.add(Items.GRANITE); + desiredOrder.add(Items.DIORITE); + desiredOrder.add(Items.BLACKSTONE); + desiredOrder.add(Items.CALCITE); + desiredOrder.add(Items.ANDESITE); + return desiredOrder; + } + @Mod(MOD_ID) public static class CreativeTabOrderTestMod { public CreativeTabOrderTestMod(IEventBus modBus) { + modBus.addListener(this::onCreativeModeTabRegister); modBus.addListener(this::buildCreativeTab); } private void buildCreativeTab(final BuildCreativeModeTabContentsEvent event) { + if (event.getTabKey() == STONE_ORDERING) { + var vis = CreativeModeTab.TabVisibility.PARENT_AND_SEARCH_TABS; + event.insertAfter(i(Blocks.STONE), i(Blocks.TUFF), vis); + event.insertAfter(i(Blocks.DIORITE), i(Blocks.CALCITE), vis); + event.insertBefore(i(Blocks.CALCITE), i(Blocks.BLACKSTONE), vis); + event.accept(i(Blocks.CYAN_CONCRETE), vis); + event.remove(i(Blocks.CYAN_CONCRETE), vis); + event.insertFirst(i(Blocks.BASALT), vis); + + catchSpecificExceptionForAction( + () -> event.accept(new ItemStack(Items.DIRT, 4), vis), + "The stack count must be 1 for", + () -> stackCountExceptionForAccept = true); + + catchSpecificExceptionForAction( + () -> event.insertAfter(i(Blocks.STONE), new ItemStack(Items.DIRT, 4), vis), + "The stack count must be 1 for", + () -> stackCountExceptionForInsertAfter = true); + + catchSpecificExceptionForAction( + () -> event.insertBefore(i(Blocks.STONE), new ItemStack(Items.DIRT, 4), vis), + "The stack count must be 1 for", + () -> stackCountExceptionForInsertBefore = true); + + catchSpecificExceptionForAction( + () -> event.insertFirst(new ItemStack(Items.DIRT, 4), vis), + "The stack count must be 1 for", + () -> stackCountExceptionForInsertFirst = true); + + catchSpecificExceptionForAction( + () -> event.insertAfter(i(Blocks.LECTERN), i(Blocks.DIRT), vis), + "does not exist in tab's list", + () -> targetDoesNotExistExceptionForInsertAfter = true); + + catchSpecificExceptionForAction( + () -> event.insertBefore(i(Blocks.LECTERN), i(Blocks.DIRT), vis), + "does not exist in tab's list", + () -> targetDoesNotExistExceptionForInsertBefore = true); + + catchSpecificExceptionForAction( + () -> event.accept(i(Blocks.STONE), vis), + "already exists in the tab's list", + () -> newEntryExistAlreadyExceptionForAccept = true); + + catchSpecificExceptionForAction( + () -> event.insertFirst(i(Blocks.STONE), vis), + "already exists in the tab's list", + () -> newEntryExistAlreadyExceptionForInsertFirst = true); + + catchSpecificExceptionForAction( + () -> event.insertAfter(i(Blocks.DIORITE), i(Blocks.STONE), vis), + "already exists in the tab's list", + () -> newEntryExistAlreadyExceptionForInsertAfter = true); + + catchSpecificExceptionForAction( + () -> event.insertBefore(i(Blocks.DIORITE), i(Blocks.STONE), vis), + "already exists in the tab's list", + () -> newEntryExistAlreadyExceptionForInsertBefore = true); + + stoneParentTab = event.getParentEntries(); + stoneSearchTab = event.getSearchEntries(); + } + if (event.getTabKey() == CreativeModeTabs.INGREDIENTS) { - ingredientsTab = event.getEntries(); + ingredientsTab = event.getParentEntries(); } if (event.getTabKey() == CreativeModeTabs.SEARCH) { - searchTab = event.getEntries(); + searchTab = event.getSearchEntries(); + } + } + + private static void catchSpecificExceptionForAction(Runnable action, String targetExceptionMessage, Runnable foundExceptionAction) { + try { + action.run(); + } catch (IllegalArgumentException e) { + if (e.getMessage().contains(targetExceptionMessage)) { + foundExceptionAction.run(); + } } } + + private void onCreativeModeTabRegister(RegisterEvent event) { + event.register(Registries.CREATIVE_MODE_TAB, helper -> { + helper.register(STONE_ORDERING, CreativeModeTab.builder().icon(() -> new ItemStack(Blocks.STONE)) + .title(Component.literal("Stone Ordering")) + .withLabelColor(0x0000FF) + .displayItems((params, output) -> { + output.accept(new ItemStack(Blocks.STONE)); + output.accept(new ItemStack(Blocks.GRANITE)); + output.accept(new ItemStack(Blocks.DIORITE)); + output.accept(new ItemStack(Blocks.ANDESITE)); + }) + .build()); + }); + } + } + + private static ItemStack i(ItemLike item) { + return new ItemStack(item); } } diff --git a/tests/src/junit/java/net/neoforged/neoforge/unittest/InsertableLinkedOpenCustomHashSetTest.java b/tests/src/junit/java/net/neoforged/neoforge/unittest/InsertableLinkedOpenCustomHashSetTest.java new file mode 100644 index 0000000000..e000a30be4 --- /dev/null +++ b/tests/src/junit/java/net/neoforged/neoforge/unittest/InsertableLinkedOpenCustomHashSetTest.java @@ -0,0 +1,80 @@ +/* + * Copyright (c) NeoForged and contributors + * SPDX-License-Identifier: LGPL-2.1-only + */ + +package net.neoforged.neoforge.unittest; + +import it.unimi.dsi.fastutil.Hash; +import it.unimi.dsi.fastutil.objects.ObjectLinkedOpenHashSet; +import java.util.stream.IntStream; +import net.neoforged.neoforge.common.util.InsertableLinkedOpenCustomHashSet; +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class InsertableLinkedOpenCustomHashSetTest { + private static final Hash.Strategy CUSTOM_STRATEGY = new Hash.Strategy<>() { + @Override + public int hashCode(String o) { + return Character.hashCode(o.charAt(0)); + } + + @Override + public boolean equals(@Nullable String a, @Nullable String b) { + return a == null ? b == null : b != null && a.charAt(0) == b.charAt(0); + } + }; + + @Test + public void testAddAfter() { + var test = new InsertableLinkedOpenCustomHashSet(); + IntStream.rangeClosed(0, 10).forEach(test::add); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10), test); + test.addAfter(4, 100); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(0, 1, 2, 3, 4, 100, 5, 6, 7, 8, 9, 10), test); + test.addAfter(100, 101); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(0, 1, 2, 3, 4, 100, 101, 5, 6, 7, 8, 9, 10), test); + test.addAfter(10, 102); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(0, 1, 2, 3, 4, 100, 101, 5, 6, 7, 8, 9, 10, 102), test); + test.addAfter(-1, 103); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(0, 1, 2, 3, 4, 100, 101, 5, 6, 7, 8, 9, 10, 102, 103), test); + Assertions.assertFalse(test.addAfter(3, 103), "Set was mutated, despite the element being present"); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(102, 0, 1, 2, 3, 101, 100, 4, 5, 6, 7, 8, 9, 10, 103), test); + } + + @Test + public void testAddBefore() { + var test = new InsertableLinkedOpenCustomHashSet(); + IntStream.rangeClosed(0, 10).forEach(test::add); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10), test); + test.addBefore(4, 100); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(0, 1, 2, 3, 100, 4, 5, 6, 7, 8, 9, 10), test); + test.addBefore(100, 101); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(0, 1, 2, 3, 101, 100, 4, 5, 6, 7, 8, 9, 10), test); + test.addBefore(0, 102); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(102, 0, 1, 2, 3, 101, 100, 4, 5, 6, 7, 8, 9, 10), test); + test.addBefore(-1, 103); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(102, 0, 1, 2, 3, 101, 100, 4, 5, 6, 7, 8, 9, 10, 103), test); + Assertions.assertFalse(test.addBefore(3, 103), "Set was mutated, despite the element being present"); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of(102, 0, 1, 2, 3, 101, 100, 4, 5, 6, 7, 8, 9, 10, 103), test); + } + + @Test + public void testAddAfterCustomStrategy() { + var test = new InsertableLinkedOpenCustomHashSet(CUSTOM_STRATEGY); + test.add("here"); + test.add("is"); + test.add("a"); + test.add("test"); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of("here", "is", "a", "test"), test); + test.addAfter("a", "b"); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of("here", "is", "a", "b", "test"), test); + test.addAfter("b", "c"); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of("here", "is", "a", "b", "c", "test"), test); + test.addAfter("test", "102"); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of("here", "is", "a", "b", "c", "test", "102"), test); + test.addAfter("doesn't exist", "203"); + Assertions.assertEquals(ObjectLinkedOpenHashSet.of("here", "is", "a", "b", "c", "test", "102", "203"), test); + } +} diff --git a/tests/src/main/java/net/neoforged/neoforge/oldtest/CreativeModeTabTest.java b/tests/src/main/java/net/neoforged/neoforge/oldtest/CreativeModeTabTest.java index 6eb7c416aa..8bc4da7d4d 100644 --- a/tests/src/main/java/net/neoforged/neoforge/oldtest/CreativeModeTabTest.java +++ b/tests/src/main/java/net/neoforged/neoforge/oldtest/CreativeModeTabTest.java @@ -117,28 +117,29 @@ private static ItemStack i(ItemLike item) { } private static void onCreativeModeTabBuildContents(BuildCreativeModeTabContentsEvent event) { - var entries = event.getEntries(); var vis = TabVisibility.PARENT_AND_SEARCH_TABS; if (event.getTabKey() == LOGS) { - entries.putAfter(i(Blocks.ACACIA_LOG), i(Blocks.STRIPPED_ACACIA_LOG), vis); - entries.putAfter(i(Blocks.BIRCH_LOG), i(Blocks.STRIPPED_BIRCH_LOG), vis); - entries.putAfter(i(Blocks.DARK_OAK_LOG), i(Blocks.STRIPPED_DARK_OAK_LOG), vis); - entries.putAfter(i(Blocks.JUNGLE_LOG), i(Blocks.STRIPPED_JUNGLE_LOG), vis); - entries.putAfter(i(Blocks.OAK_LOG), i(Blocks.STRIPPED_OAK_LOG), vis); - entries.putAfter(i(Blocks.SPRUCE_LOG), i(Blocks.STRIPPED_SPRUCE_LOG), vis); + event.insertAfter(i(Blocks.ACACIA_LOG), i(Blocks.STRIPPED_ACACIA_LOG), vis); + event.insertAfter(i(Blocks.BIRCH_LOG), i(Blocks.STRIPPED_BIRCH_LOG), vis); + event.insertAfter(i(Blocks.DARK_OAK_LOG), i(Blocks.STRIPPED_DARK_OAK_LOG), vis); + event.insertAfter(i(Blocks.JUNGLE_LOG), i(Blocks.STRIPPED_JUNGLE_LOG), vis); + event.insertAfter(i(Blocks.OAK_LOG), i(Blocks.STRIPPED_OAK_LOG), vis); + event.insertAfter(i(Blocks.SPRUCE_LOG), i(Blocks.STRIPPED_SPRUCE_LOG), vis); } if (event.getTabKey() == STONE) { - entries.putBefore(i(Blocks.STONE), i(Blocks.SMOOTH_STONE), vis); - entries.putBefore(i(Blocks.GRANITE), i(Blocks.POLISHED_GRANITE), vis); - entries.putBefore(i(Blocks.DIORITE), i(Blocks.POLISHED_DIORITE), vis); - entries.putBefore(i(Blocks.ANDESITE), i(Blocks.POLISHED_ANDESITE), vis); + event.insertBefore(i(Blocks.STONE), i(Blocks.SMOOTH_STONE), vis); + event.insertBefore(i(Blocks.GRANITE), i(Blocks.POLISHED_GRANITE), vis); + event.insertBefore(i(Blocks.DIORITE), i(Blocks.POLISHED_DIORITE), vis); + event.insertBefore(i(Blocks.ANDESITE), i(Blocks.POLISHED_ANDESITE), vis); } // Adding this causes a crash (as it should) when opening the creative inventory -// if (event.getTabKey() == DAMAGED_SWORDS) { -// entries.putBefore(i(Items.WOODEN_SWORD), i(Items.WOODEN_SWORD), vis); -// } + if (false) { + if (event.getTabKey() == DAMAGED_SWORDS) { + event.insertBefore(i(Items.WOODEN_SWORD), i(Items.WOODEN_SWORD), vis); + } + } } private static class CreativeModeColorTab extends CreativeModeTab {