diff --git a/fabric-item-group-api-v1/src/client/java/net/fabricmc/fabric/mixin/itemgroup/client/CreativeInventoryScreenMixin.java b/fabric-item-group-api-v1/src/client/java/net/fabricmc/fabric/mixin/itemgroup/client/CreativeInventoryScreenMixin.java index ce4054182..67c771f90 100644 --- a/fabric-item-group-api-v1/src/client/java/net/fabricmc/fabric/mixin/itemgroup/client/CreativeInventoryScreenMixin.java +++ b/fabric-item-group-api-v1/src/client/java/net/fabricmc/fabric/mixin/itemgroup/client/CreativeInventoryScreenMixin.java @@ -93,13 +93,10 @@ public abstract class CreativeInventoryScreenMixin<T extends ScreenHandler> exte private void fabric_updateSelection() { if (!fabric_isGroupVisible(selectedTab)) { - ItemGroups.getGroups().stream() + ItemGroups.getGroups() + .stream() .filter(this::fabric_isGroupVisible) - .min((a, b) -> { - if (a.isSpecial() && !b.isSpecial()) return 1; - if (!a.isSpecial() && b.isSpecial()) return -1; - return 0; - }) + .min((a, b) -> Boolean.compare(a.isSpecial(), b.isSpecial())) .ifPresent(this::setSelectedTab); } } @@ -144,7 +141,7 @@ public abstract class CreativeInventoryScreenMixin<T extends ScreenHandler> exte } private boolean fabric_isGroupVisible(ItemGroup itemGroup) { - return fabric_currentPage == fabric_getPage(itemGroup); + return itemGroup.shouldDisplay() && fabric_currentPage == fabric_getPage(itemGroup); } private static int fabric_getPage(ItemGroup itemGroup) { diff --git a/fabric-item-group-api-v1/src/main/java/net/fabricmc/fabric/mixin/itemgroup/ItemGroupsMixin.java b/fabric-item-group-api-v1/src/main/java/net/fabricmc/fabric/mixin/itemgroup/ItemGroupsMixin.java index b68a86976..d381d638e 100644 --- a/fabric-item-group-api-v1/src/main/java/net/fabricmc/fabric/mixin/itemgroup/ItemGroupsMixin.java +++ b/fabric-item-group-api-v1/src/main/java/net/fabricmc/fabric/mixin/itemgroup/ItemGroupsMixin.java @@ -45,6 +45,7 @@ import net.minecraft.item.ItemGroup; import net.minecraft.item.ItemGroups; import net.minecraft.registry.Registries; import net.minecraft.registry.RegistryKey; +import net.minecraft.registry.entry.RegistryEntry; import net.fabricmc.fabric.impl.itemgroup.FabricItemGroup; @@ -54,21 +55,40 @@ public class ItemGroupsMixin { private static final int TABS_PER_PAGE = FabricItemGroup.TABS_PER_PAGE; @Inject(method = "collect", at = @At("HEAD"), cancellable = true) - private static void collect(CallbackInfo ci) { + private static void deferDuplicateCheck(CallbackInfo ci) { + /* + * Defer the duplication checks to when fabric performs them (see mixin below). + * It is preserved just in case, but fabric's pagination logic should prevent any from happening anyway. + */ + ci.cancel(); + } + + @Inject(method = "updateEntries", at = @At("TAIL")) + private static void paginateGroups(CallbackInfo ci) { final List<RegistryKey<ItemGroup>> vanillaGroups = List.of(BUILDING_BLOCKS, COLORED_BLOCKS, NATURAL, FUNCTIONAL, REDSTONE, HOTBAR, SEARCH, TOOLS, COMBAT, FOOD_AND_DRINK, INGREDIENTS, SPAWN_EGGS, OPERATOR, INVENTORY); int count = 0; - // Sort the item groups to ensure they are in a deterministic order. - final List<RegistryKey<ItemGroup>> sortedItemGroups = Registries.ITEM_GROUP.getKeys().stream() - .sorted(Comparator.comparing(RegistryKey::getValue)) + Comparator<RegistryEntry.Reference<ItemGroup>> entryComparator = (e1, e2) -> { + // Non-displayable groups should come last for proper pagination + int displayCompare = Boolean.compare(e1.value().shouldDisplay(), e2.value().shouldDisplay()); + + if (displayCompare != 0) { + return -displayCompare; + } else { + // Ensure a deterministic order + return e1.registryKey().getValue().compareTo(e2.registryKey().getValue()); + } + }; + final List<RegistryEntry.Reference<ItemGroup>> sortedItemGroups = Registries.ITEM_GROUP.streamEntries() + .sorted(entryComparator) .toList(); - for (RegistryKey<ItemGroup> registryKey : sortedItemGroups) { - final ItemGroup itemGroup = Registries.ITEM_GROUP.getOrThrow(registryKey); + for (RegistryEntry.Reference<ItemGroup> reference : sortedItemGroups) { + final ItemGroup itemGroup = reference.value(); final FabricItemGroup fabricItemGroup = (FabricItemGroup) itemGroup; - if (vanillaGroups.contains(registryKey)) { + if (vanillaGroups.contains(reference.registryKey())) { // Vanilla group goes on the first page. fabricItemGroup.setPage(0); continue; @@ -99,7 +119,5 @@ public class ItemGroupsMixin { throw new IllegalArgumentException("Duplicate position: (%s) for item groups %s vs %s".formatted(position, displayName, existingName)); } } - - ci.cancel(); } } diff --git a/fabric-item-group-api-v1/src/testmod/java/net/fabricmc/fabric/test/item/group/ItemGroupTest.java b/fabric-item-group-api-v1/src/testmod/java/net/fabricmc/fabric/test/item/group/ItemGroupTest.java index 338bf03d2..f92bc187a 100644 --- a/fabric-item-group-api-v1/src/testmod/java/net/fabricmc/fabric/test/item/group/ItemGroupTest.java +++ b/fabric-item-group-api-v1/src/testmod/java/net/fabricmc/fabric/test/item/group/ItemGroupTest.java @@ -69,6 +69,10 @@ public class ItemGroupTest implements ModInitializer { // Add a differently damaged pickaxe to all groups ItemGroupEvents.MODIFY_ENTRIES_ALL.register((group, content) -> { + if (group.getIcon() == ItemStack.EMPTY) { // Leave the empty groups empty + return; + } + ItemStack minDmgPickaxe = new ItemStack(Items.DIAMOND_PICKAXE); minDmgPickaxe.setDamage(1); content.prepend(minDmgPickaxe); @@ -78,6 +82,17 @@ public class ItemGroupTest implements ModInitializer { content.add(maxDmgPickaxe); }); + // Regression test for #3566 + for (int j = 0; j < 20; j++) { + Registry.register( + Registries.ITEM_GROUP, + new Identifier(MOD_ID, "empty_group_" + j), + FabricItemGroup.builder() + .displayName(Text.literal("Empty Item Group: " + j)) + .build() + ); + } + for (int i = 0; i < 100; i++) { final int index = i;