From f289a455e78362d54d37fb4cd78944be11a4d459 Mon Sep 17 00:00:00 2001 From: Jason Mitchell Date: Mon, 28 Sep 2026 22:40:05 -0700 Subject: [PATCH] Fix ME terminal lag from re-sorting on every inventory update (#1665) --- .../client/me/ItemRepoFunctionalTest.java | 265 ++++++++++++++++++ .../gui/implementations/GuiMEMonitorable.java | 10 +- src/main/java/appeng/client/me/ItemRepo.java | 94 ++++++- src/main/java/appeng/util/ItemSorters.java | 47 +++- .../java/appeng/util/ItemSortersTest.java | 26 ++ 5 files changed, 423 insertions(+), 19 deletions(-) create mode 100644 src/functionalTest/java/appeng/test/client/me/ItemRepoFunctionalTest.java create mode 100644 src/test/java/appeng/util/ItemSortersTest.java diff --git a/src/functionalTest/java/appeng/test/client/me/ItemRepoFunctionalTest.java b/src/functionalTest/java/appeng/test/client/me/ItemRepoFunctionalTest.java new file mode 100644 index 000000000..790a5bc29 --- /dev/null +++ b/src/functionalTest/java/appeng/test/client/me/ItemRepoFunctionalTest.java @@ -0,0 +1,265 @@ +package appeng.test.client.me; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import net.minecraft.init.Blocks; +import net.minecraft.init.Items; +import net.minecraft.item.Item; +import net.minecraft.item.ItemStack; + +import org.junit.jupiter.api.Test; + +import appeng.api.config.SortDir; +import appeng.api.config.SortOrder; +import appeng.api.config.ViewItems; +import appeng.api.storage.data.IAEItemStack; +import appeng.api.storage.data.IAEStack; +import appeng.client.gui.widgets.ISortSource; +import appeng.client.me.ItemRepo; +import appeng.me.cache.ItemFlowGridCache.FlowRate; +import appeng.util.ItemSorters; +import appeng.util.item.AEItemStack; + +public class ItemRepoFunctionalTest { + + private static class MutableSortSource implements ISortSource { + + Enum sortBy = SortOrder.NAME; + Enum sortDir = SortDir.ASCENDING; + Enum display = ViewItems.ALL; + + @Override + public Enum getSortBy() { + return sortBy; + } + + @Override + public Enum getSortDir() { + return sortDir; + } + + @Override + public Enum getSortDisplay() { + return display; + } + } + + private static class Fixture { + + final MutableSortSource sort = new MutableSortSource(); + final ItemRepo repo = new ItemRepo(() -> 0, sort); + } + + private static Fixture newFixture() { + final Fixture fixture = new Fixture(); + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Items.stick, 5))); + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Items.apple, 3))); + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Blocks.dirt, 1))); + fixture.repo.updateView(); + return fixture; + } + + private static List> viewList(final ItemRepo repo) { + final List> view = new ArrayList<>(); + for (int i = 0; i < repo.size(); i++) { + view.add(repo.getReferenceStack(i)); + } + return view; + } + + private static IAEStack findItem(final ItemRepo repo, final Item item) { + for (int i = 0; i < repo.size(); i++) { + final IAEStack stack = repo.getReferenceStack(i); + if (stack instanceof IAEItemStack ais && ais.getItem() == item) { + return stack; + } + } + return null; + } + + private static void assertNameSorted(final ItemRepo repo) { + final List> view = viewList(repo); + final List> expected = new ArrayList<>(view); + expected.sort(ItemSorters.CONFIG_BASED_SORT_BY_NAME); + assertEquals(expected, view); + } + + @Test + void nameSortMatchesUncachedComparator() { + final Fixture fixture = newFixture(); + + assertNameSorted(fixture.repo); + } + + @Test + void countOnlyUpdateSkipsRebuild() { + final Fixture fixture = newFixture(); + + final IAEStack stickBefore = findItem(fixture.repo, Items.stick); + + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Items.stick, 7))); + assertFalse(fixture.repo.updateViewIfChanged()); + + final IAEStack stickAfter = findItem(fixture.repo, Items.stick); + assertSame(stickBefore, stickAfter); + assertEquals(7, stickAfter.getStackSize()); + } + + @Test + void visibilityChangesRebuild() { + final Fixture fixture = newFixture(); + + final AEItemStack zeroStick = AEItemStack.create(new ItemStack(Items.stick, 1)); + zeroStick.setStackSize(0); + fixture.repo.postUpdate(zeroStick); + assertTrue(fixture.repo.updateViewIfChanged()); + assertNull(findItem(fixture.repo, Items.stick)); + + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Items.bread, 2))); + assertTrue(fixture.repo.updateViewIfChanged()); + assertNotNull(findItem(fixture.repo, Items.bread)); + + assertNameSorted(fixture.repo); + } + + @Test + void amountSortRebuildsOnCountChange() { + final Fixture fixture = newFixture(); + fixture.sort.sortBy = SortOrder.AMOUNT; + fixture.repo.updateView(); + + final ItemStack dirtStack = new ItemStack(Blocks.dirt, 10); + fixture.repo.postUpdate(AEItemStack.create(dirtStack)); + assertTrue(fixture.repo.updateViewIfChanged()); + + final IAEStack first = fixture.repo.getReferenceStack(0); + assertTrue(first instanceof IAEItemStack ais && ais.getItemStack().isItemEqual(dirtStack)); + } + + @Test + void craftableModeAlwaysRebuilds() { + final Fixture fixture = newFixture(); + fixture.sort.display = ViewItems.CRAFTABLE; + fixture.repo.updateView(); + + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Items.stick, 7))); + assertTrue(fixture.repo.updateViewIfChanged()); + } + + @Test + void flowRatesRebuildOnlyInFlowingMode() { + final Fixture fixture = newFixture(); + + final Map, FlowRate> rates = new HashMap<>(); + rates.put(AEItemStack.create(new ItemStack(Items.apple, 1)), new FlowRate(1, 0)); + fixture.repo.updateFlowRates(rates); + assertFalse(fixture.repo.updateViewIfChanged()); + + fixture.sort.display = ViewItems.FLOWING; + fixture.repo.updateView(); + assertEquals(1, fixture.repo.size()); + assertNotNull(findItem(fixture.repo, Items.apple)); + + fixture.repo.updateFlowRates(new HashMap<>()); + assertTrue(fixture.repo.updateViewIfChanged()); + assertEquals(0, fixture.repo.size()); + } + + @Test + void pausedUpdateAppendsWithoutResort() { + final Fixture fixture = newFixture(); + + final List> before = viewList(fixture.repo); + fixture.repo.setPaused(true); + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Items.bread, 2))); + assertTrue(fixture.repo.updateViewIfChanged()); + + final List> after = viewList(fixture.repo); + assertEquals(before, after.subList(0, 3)); + final IAEStack last = after.get(after.size() - 1); + assertTrue(last instanceof IAEItemStack ais && ais.getItem() == Items.bread); + } + + @Test + void sortKeysPrunedAfterChurn() { + final Fixture fixture = newFixture(); + + for (int meta = 1; meta <= 600; meta++) { + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Items.stick, 1, meta))); + } + fixture.repo.updateView(); + assertEquals(603, fixture.repo.getSortKeyCount()); + + for (int meta = 1; meta <= 600; meta++) { + final AEItemStack gone = AEItemStack.create(new ItemStack(Items.stick, 1, meta)); + gone.setStackSize(0); + fixture.repo.postUpdate(gone); + } + fixture.repo.updateView(); + assertEquals(3, fixture.repo.getSortKeyCount()); + assertNotNull(fixture.repo.peekSortKey(AEItemStack.create(new ItemStack(Items.apple, 1)))); + + for (int meta = 601; meta <= 1200; meta++) { + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Items.stick, 1, meta))); + } + fixture.repo.updateView(); + assertEquals(603, fixture.repo.getSortKeyCount()); + assertNameSorted(fixture.repo); + } + + @Test + void bouncingItemKeepsSortKey() { + final Fixture fixture = newFixture(); + final AEItemStack stick = AEItemStack.create(new ItemStack(Items.stick, 1)); + final Object keyBefore = fixture.repo.peekSortKey(stick); + assertNotNull(keyBefore); + + final IAEItemStack zeroStick = stick.copy(); + zeroStick.setStackSize(0); + fixture.repo.postUpdate(zeroStick); + fixture.repo.updateView(); + assertNull(findItem(fixture.repo, Items.stick)); + + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Items.stick, 4))); + fixture.repo.updateView(); + assertSame(keyBefore, fixture.repo.peekSortKey(stick)); + } + + @Test + void pruneKeepsKeysOfFilteredLiveStacks() { + final Fixture fixture = newFixture(); + final AEItemStack apple = AEItemStack.create(new ItemStack(Items.apple, 1)); + final Object appleKey = fixture.repo.peekSortKey(apple); + assertNotNull(appleKey); + + final IAEItemStack craftableApple = apple.copy(); + craftableApple.setStackSize(0); + craftableApple.setCraftable(true); + fixture.repo.postUpdate(craftableApple); + fixture.sort.display = ViewItems.STORED; + for (int meta = 1; meta <= 600; meta++) { + fixture.repo.postUpdate(AEItemStack.create(new ItemStack(Items.stick, 1, meta))); + } + fixture.repo.updateView(); + assertNull(findItem(fixture.repo, Items.apple)); + + for (int meta = 1; meta <= 600; meta++) { + final AEItemStack gone = AEItemStack.create(new ItemStack(Items.stick, 1, meta)); + gone.setStackSize(0); + fixture.repo.postUpdate(gone); + } + fixture.repo.updateView(); + assertEquals(3, fixture.repo.getSortKeyCount()); + assertSame(appleKey, fixture.repo.peekSortKey(apple)); + } +} diff --git a/src/main/java/appeng/client/gui/implementations/GuiMEMonitorable.java b/src/main/java/appeng/client/gui/implementations/GuiMEMonitorable.java index 9274c6c56..913cfd698 100644 --- a/src/main/java/appeng/client/gui/implementations/GuiMEMonitorable.java +++ b/src/main/java/appeng/client/gui/implementations/GuiMEMonitorable.java @@ -56,7 +56,6 @@ import appeng.api.storage.data.AEStackTypeRegistry; import appeng.api.storage.data.IAEItemStack; import appeng.api.storage.data.IAEStack; import appeng.api.storage.data.IAEStackType; -import appeng.api.storage.data.IDisplayRepo; import appeng.api.util.IConfigManager; import appeng.api.util.IConfigurableObject; import appeng.client.ActionKey; @@ -119,7 +118,7 @@ public class GuiMEMonitorable extends AEBaseGui public static final int keyBindPickBlockAction = 1_000_101; private static String memoryText = ""; - private final IDisplayRepo repo; + private final ItemRepo repo; protected int offsetRepoX = 9; protected int offsetRepoY = 18; private final int MAGIC_HEIGHT_NUMBER = 114 + 1; @@ -154,6 +153,7 @@ public class GuiMEMonitorable extends AEBaseGui public final boolean hasPinHost; private boolean enableShiftPause = true; private boolean needsViewUpdate = false; + private int scrollBarRepoSize = -1; protected VirtualMEPinSlot[] pinSlots = null; protected VirtualMEMonitorableSlot[] monitorableSlots = null; @@ -198,7 +198,6 @@ public class GuiMEMonitorable extends AEBaseGui final String text = getText(); repo.setSearchString(text.trim()); repo.updateView(); - setScrollBar(); if (AEConfig.instance.preserveSearchBar) monitorableContainer.saveSearchString(this.getText()); } @@ -225,6 +224,7 @@ public class GuiMEMonitorable extends AEBaseGui } private void setScrollBar() { + this.scrollBarRepoSize = this.repo.size(); this.getScrollBar().setTop(this.offsetRepoY).setLeft(166 + this.offsetRepoX).setHeight(this.rows * 18 - 2); int totalPinRows = craftingPinsRows.ordinal() + playerPinsRows.ordinal(); this.getScrollBar().setRange( @@ -1094,9 +1094,9 @@ public class GuiMEMonitorable extends AEBaseGui public void drawScreen(final int mouseX, final int mouseY, final float btn) { if (this.needsViewUpdate) { this.needsViewUpdate = false; - this.repo.updateView(); - this.setScrollBar(); + this.repo.updateViewIfChanged(); } + if (this.repo.size() != this.scrollBarRepoSize) this.setScrollBar(); handleTooltip(mouseX, mouseY, searchField); diff --git a/src/main/java/appeng/client/me/ItemRepo.java b/src/main/java/appeng/client/me/ItemRepo.java index 0c5516439..015e02040 100644 --- a/src/main/java/appeng/client/me/ItemRepo.java +++ b/src/main/java/appeng/client/me/ItemRepo.java @@ -11,6 +11,7 @@ package appeng.client.me; import java.util.ArrayList; +import java.util.Comparator; import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -22,8 +23,11 @@ import java.util.regex.PatternSyntaxException; import javax.annotation.Nonnull; +import net.minecraft.item.Item; import net.minecraft.item.ItemStack; +import com.google.common.annotations.VisibleForTesting; + import appeng.api.AEApi; import appeng.api.config.SearchBoxMode; import appeng.api.config.Settings; @@ -47,10 +51,13 @@ import appeng.util.Platform; import appeng.util.item.OreHelper; import appeng.util.item.OreReference; import appeng.util.prioitylist.IPartitionList; +import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import it.unimi.dsi.fastutil.objects.Reference2BooleanMap; public class ItemRepo implements IDisplayRepo { + private static final int SORT_KEY_PRUNE_FLOOR = 256; + private final IItemList> list = AEApi.instance().storage().createAEStackList(); private IAEStack[] pinsRepo = new IAEStack[0]; private int visibleCraftingRows = 0; @@ -67,6 +74,13 @@ public class ItemRepo implements IDisplayRepo { private IPartitionList myPartitionList; private boolean hasPower; private boolean paused = false; + private Object2ObjectOpenHashMap, SortKey> sortKeys = new Object2ObjectOpenHashMap<>(); + private final Comparator> sortByName = ItemSorters.sortByName(this::getSortKey, k -> k.name); + private final Comparator> sortByMod = ItemSorters + .sortByMod(this::getSortKey, SortKey::getModId, k -> k.name); + private boolean viewStale; + private boolean sizesChanged; + private boolean flowRatesChanged; public ItemRepo(final IScrollSource src, final ISortSource sortSrc) { this.src = src; @@ -184,10 +198,20 @@ public class ItemRepo implements IDisplayRepo { } if (st != null) { + final boolean wasMeaningful = st.isMeaningful(); + final boolean wasCraftable = st.isCraftable(); + final long oldSize = st.getStackSize(); st.reset(); st.add(is); + final long newSize = st.getStackSize(); + if (st.isMeaningful() != wasMeaningful || st.isCraftable() != wasCraftable + || (oldSize == 0) != (newSize == 0)) { + this.viewStale = true; + } + if (newSize != oldSize) this.sizesChanged = true; } else { this.list.add(is); + this.viewStale = true; } } @@ -197,8 +221,24 @@ public class ItemRepo implements IDisplayRepo { this.updateView(); } + /** Rebuilds only when a pending update can change visibility or order; returns whether it rebuilt. */ + public boolean updateViewIfChanged() { + final Enum display = this.sortSrc.getSortDisplay(); + final Enum sortBy = this.sortSrc.getSortBy(); + if (this.viewStale || display == ViewItems.CRAFTABLE + || (this.flowRatesChanged && display == ViewItems.FLOWING) + || (this.sizesChanged && (sortBy == SortOrder.AMOUNT || sortBy == SortOrder.INVTWEAKS))) { + this.updateView(); + return true; + } + return false; + } + @Override public void updateView() { + this.viewStale = false; + this.sizesChanged = false; + this.flowRatesChanged = false; IItemList> visiblePins = getPinsCache(!AEConfig.instance.showCraftingPinsItemsInMainView, true); if (this.paused) { for (int i = this.view.size() - 1; i >= 0; i--) { @@ -236,17 +276,44 @@ public class ItemRepo implements IDisplayRepo { ItemSorters.setDirection((appeng.api.config.SortDir) SortDir); if (SortBy == SortOrder.MOD) { - this.view.sort(ItemSorters.CONFIG_BASED_SORT_BY_MOD); + this.view.sort(this.sortByMod); } else if (SortBy == SortOrder.AMOUNT) { this.view.sort(ItemSorters.CONFIG_BASED_SORT_BY_SIZE); } else if (SortBy == SortOrder.INVTWEAKS) { this.view.sort(ItemSorters.CONFIG_BASED_SORT_BY_INV_TWEAKS); } else { - this.view.sort(ItemSorters.CONFIG_BASED_SORT_BY_NAME); + this.view.sort(this.sortByName); } + + this.pruneSortKeys(); } } + private void pruneSortKeys() { + final int live = this.list.size(); + if (this.sortKeys.size() <= Math.max(SORT_KEY_PRUNE_FLOOR, 2 * live)) return; + final Object2ObjectOpenHashMap, SortKey> kept = new Object2ObjectOpenHashMap<>(live); + for (final IAEStack is : this.list) { + final SortKey key = this.sortKeys.get(is); + if (key != null) kept.put(is, key); + } + this.sortKeys = kept; + } + + @VisibleForTesting + public int getSortKeyCount() { + return this.sortKeys.size(); + } + + @VisibleForTesting + public Object peekSortKey(final IAEStack stack) { + return this.sortKeys.get(stack); + } + + private SortKey getSortKey(final IAEStack stack) { + return this.sortKeys.computeIfAbsent(stack, SortKey::new); + } + private void addEntriesToView(Iterable> entries, IItemList> visiblePins) { final Enum viewMode = this.sortSrc.getSortDisplay(); Reference2BooleanMap> typeFilters = this.sortSrc.getTypeFilter(); @@ -296,8 +363,8 @@ public class ItemRepo implements IDisplayRepo { if (viewMode == ViewItems.FLOWING && !this.flowRates.containsKey(is)) continue; if (is instanceof IAEItemStack ais) { - if (registry.isBlacklisted(ais.getItemStack().getItem()) - || registry.isBlacklisted(ais.getItemStack().getItem().getClass())) { + final Item item = ais.getItem(); + if (registry.isBlacklisted(item) || registry.isBlacklisted(item.getClass())) { continue; } } @@ -375,6 +442,7 @@ public class ItemRepo implements IDisplayRepo { @Override public void clear() { this.list.resetStatus(); + this.viewStale = true; } @Override @@ -443,5 +511,23 @@ public class ItemRepo implements IDisplayRepo { @Override public void updateFlowRates(final Map, FlowRate> rates) { this.flowRates = rates; + this.flowRatesChanged = true; + } + + private static final class SortKey { + + private final IAEStack stack; + private final String name; + private String modId; + + private SortKey(final IAEStack stack) { + this.stack = stack; + this.name = ItemSorters.getSortName(stack); + } + + private String getModId() { + if (this.modId == null) this.modId = this.stack.getModId(); + return this.modId; + } } } diff --git a/src/main/java/appeng/util/ItemSorters.java b/src/main/java/appeng/util/ItemSorters.java index 1dc785f50..e8cd382ae 100644 --- a/src/main/java/appeng/util/ItemSorters.java +++ b/src/main/java/appeng/util/ItemSorters.java @@ -11,7 +11,7 @@ package appeng.util; import java.util.Comparator; -import java.util.regex.Pattern; +import java.util.function.Function; import com.gtnewhorizon.gtnhlib.util.font.FontRendering; @@ -20,16 +20,19 @@ import appeng.api.storage.data.IAEStack; public class ItemSorters { - private static final Pattern FORMATTING_PATTERN = Pattern.compile("(?s)" + '\u00a7' + "."); - private static SortDir direction = SortDir.ASCENDING; - public static final Comparator> CONFIG_BASED_SORT_BY_NAME = Comparator - .comparing(ItemSorters::getSortName, (a, b) -> a.compareToIgnoreCase(b) * direction.sortHint); + private static final Comparator DIRECTED_IGNORE_CASE = (a, b) -> a.compareToIgnoreCase(b) + * direction.sortHint; - public static final Comparator> CONFIG_BASED_SORT_BY_MOD = Comparator - .comparing((IAEStack stack) -> stack.getModId(), (a, b) -> a.compareToIgnoreCase(b) * direction.sortHint) - .thenComparing(ItemSorters::getSortName); + public static final Comparator> CONFIG_BASED_SORT_BY_NAME = sortByName( + Function.identity(), + ItemSorters::getSortName); + + public static final Comparator> CONFIG_BASED_SORT_BY_MOD = sortByMod( + Function.identity(), + IAEStack::getModId, + ItemSorters::getSortName); public static final Comparator> CONFIG_BASED_SORT_BY_SIZE = Comparator .comparing(IAEStack::getStackSize, (a, b) -> Long.compare(b, a) * direction.sortHint); @@ -47,9 +50,33 @@ public class ItemSorters { } }; + public static Comparator> sortByName(final Function, K> key, + final Function sortName) { + return Comparator.comparing(key, Comparator.comparing(sortName, DIRECTED_IGNORE_CASE)); + } + + public static Comparator> sortByMod(final Function, K> key, + final Function modId, final Function sortName) { + return Comparator.comparing(key, Comparator.comparing(modId, DIRECTED_IGNORE_CASE).thenComparing(sortName)); + } + /** Display name without format codes, so &-styled or colored names sort by their visible text. */ - private static String getSortName(final IAEStack stack) { - return FORMATTING_PATTERN.matcher(FontRendering.preprocessText(stack.getDisplayName())).replaceAll(""); + public static String getSortName(final IAEStack stack) { + return stripFormatting(FontRendering.preprocessText(stack.getDisplayName())); + } + + public static String stripFormatting(final String s) { + int i = s.indexOf('\u00a7'); + if (i < 0) return s; + final int len = s.length(); + final StringBuilder sb = new StringBuilder(len); + int last = 0; + while (i >= 0 && i + 1 < len) { + sb.append(s, last, i); + last = i + 1 + Character.charCount(s.codePointAt(i + 1)); + i = s.indexOf('\u00a7', last); + } + return sb.append(s, last, len).toString(); } public static int compareInt(final int a, final int b) { diff --git a/src/test/java/appeng/util/ItemSortersTest.java b/src/test/java/appeng/util/ItemSortersTest.java new file mode 100644 index 000000000..e05d783c2 --- /dev/null +++ b/src/test/java/appeng/util/ItemSortersTest.java @@ -0,0 +1,26 @@ +package appeng.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; + +import java.util.regex.Pattern; + +import org.junit.Test; + +public class ItemSortersTest { + + @Test + public void stripFormattingMatchesRegex() { + final Pattern formattingPattern = Pattern.compile("(?s)\u00a7."); + + final String noFormatting = "Iron Ingot"; + assertSame(noFormatting, ItemSorters.stripFormatting(noFormatting)); + + final String[] inputs = { "Iron Ingot", "\u00a7aRed", "\u00a7\u00a7a", "abc\u00a7", "\u00a7", "a\u00a7\nb", + "\u00a7\ud83d\ude00x", "\u00a7x\u00a7f\u00a7f\u00a70\u00a70\u00a70\u00a70Hex", "" }; + + for (final String input : inputs) { + assertEquals(formattingPattern.matcher(input).replaceAll(""), ItemSorters.stripFormatting(input)); + } + } +}