From 17107110e2895a7506e57f30e8433d59018bb93b Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:48:51 +0100 Subject: [PATCH 1/3] perf: refresh pickup lore only on items that can carry it, and set only changed slots Each pickup (once per player per tick since #75) ran both lore writers over all 41 inventory slots and set every slot back. For an ordinary item that meant about eight copies of its meta, its lore read twice and run through regexes (ArtifactLore.readAura), all to change nothing, and then a slot packet per slot. On Main at 15+ players that was ~3 s of main thread per hour. VesselLore now reads an item's meta once to tell whether a writer can touch it: aura or charge-tier data, a charge item match, or lore with a "/" (an aura line that readAura could read back). Other items come back unchanged, as both writers left them before. The inventory refresh sets a slot again only when its item changed; unchanged slots were only resent to the client. Lore output is the same: artifacts and charges still go through the same writers. Co-Authored-By: Claude Opus 5.5 --- .../magic/artifact/aura/VesselLore.java | 71 +++++++++-- .../magic/artifact/aura/VesselLoreTest.java | 115 ++++++++++++++++++ 2 files changed, 173 insertions(+), 13 deletions(-) create mode 100644 src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java diff --git a/src/main/java/net/tfminecraft/magic/artifact/aura/VesselLore.java b/src/main/java/net/tfminecraft/magic/artifact/aura/VesselLore.java index 8d0eba5..76cf027 100644 --- a/src/main/java/net/tfminecraft/magic/artifact/aura/VesselLore.java +++ b/src/main/java/net/tfminecraft/magic/artifact/aura/VesselLore.java @@ -3,10 +3,15 @@ import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.PlayerInventory; +import org.bukkit.inventory.meta.ItemMeta; +import org.bukkit.persistence.PersistentDataContainer; +import net.tfminecraft.magic.artifact.ArtifactKeys; import net.tfminecraft.magic.artifact.ArtifactLore; import net.tfminecraft.magic.charge.ChargeIds; +import net.tfminecraft.magic.charge.ChargeKeys; import net.tfminecraft.magic.charge.ChargeLore; +import net.tfminecraft.magic.charge.ChargeRegistry; /** Sends an item to whichever lore writer owns it. */ public final class VesselLore { @@ -14,34 +19,74 @@ public final class VesselLore { private VesselLore() {} public static ItemStack updateItem(ItemStack stack) { - if (stack == null || stack.getType().isAir()) { + if (stack == null || stack.getType().isAir() || !mayHaveVesselLore(stack)) { return stack; } + return write(stack); + } + + private static ItemStack write(ItemStack stack) { if (ChargeIds.isCharge(stack)) { return ChargeLore.updateItem(stack); } return ArtifactLore.updateItem(stack); } + /** + * False only for an item neither lore writer touches, found from one copy of its meta. A charge has its tier key + * or matches a charge item (ChargeIds.isCharge); an artifact has aura data (AuraData.fromPersistentData) or aura + * lines its lore can be read back from, and each of those has a "/" (ArtifactLore.readAura). For anything else + * both writers leave the item exactly as it was, so it isn't read again for each of them. + */ + // Keep the existing legacy text representation, formatting, and exact-string comparisons. + @SuppressWarnings("deprecation") + static boolean mayHaveVesselLore(ItemStack stack) { + ItemMeta meta = stack.getItemMeta(); + PersistentDataContainer pdc = meta.getPersistentDataContainer(); + if (pdc.has(ArtifactKeys.auraCap()) || pdc.has(ArtifactKeys.auraFill()) || pdc.has(ArtifactKeys.auraData()) + || pdc.has(ChargeKeys.chargeTier()) || ChargeRegistry.match(stack) != null) { + return true; + } + if (!meta.hasLore()) { + return false; + } + for (String line : meta.getLore()) { + if (line.indexOf('/') >= 0) { + return true; + } + } + return false; + } + + /** + * Rewrites the vessel lore of every item the player carries. Only slots whose item the writers changed are set + * again; setting an unchanged item gave the same inventory and only resent the slot to the client. + */ public static void updateInventory(Player player) { if (player == null) { return; } PlayerInventory inventory = player.getInventory(); ItemStack[] storage = inventory.getStorageContents(); - for (int i = 0; i < storage.length; i++) { - storage[i] = updateItem(storage[i]); - } - inventory.setStorageContents(storage); ItemStack[] armor = inventory.getArmorContents(); - for (int i = 0; i < armor.length; i++) { - armor[i] = updateItem(armor[i]); - } - inventory.setArmorContents(armor); - ItemStack[] extra = inventory.getExtraContents(); - for (int i = 0; i < extra.length; i++) { - extra[i] = updateItem(extra[i]); + refresh(inventory, storage, 0); + refresh(inventory, armor, storage.length); + refresh(inventory, inventory.getExtraContents(), storage.length + armor.length); + } + + /** Refreshes items, which fill the inventory's slots from first on (storage, then armor, then off hand). */ + private static void refresh(PlayerInventory inventory, ItemStack[] items, int first) { + for (int i = 0; i < items.length; i++) { + ItemStack item = items[i]; + if (item == null || item.getType().isAir() || !mayHaveVesselLore(item)) { + continue; + } + // A copy from before: the writers change the item in place, and it may be the inventory's own. + ItemStack before = item.clone(); + ItemStack after = write(item); + if (!after.equals(before)) { + inventory.setItem(first + i, after); + } } - inventory.setExtraContents(extra); } } diff --git a/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java b/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java new file mode 100644 index 0000000..d5e1571 --- /dev/null +++ b/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java @@ -0,0 +1,115 @@ +package net.tfminecraft.magic.artifact.aura; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import java.util.List; +import net.tfminecraft.magic.Cache; +import net.tfminecraft.magic.Magic; +import net.tfminecraft.magic.artifact.ArtifactKeys; +import net.tfminecraft.magic.charge.ChargeDef; +import net.tfminecraft.magic.charge.ChargeKeys; +import net.tfminecraft.magic.charge.ChargeLore; +import net.tfminecraft.magic.charge.ChargeRegistry; +import net.tfminecraft.magic.charge.TierBands; +import net.tfminecraft.magic.registry.ElementRegistry; +import org.bukkit.Material; +import org.bukkit.NamespacedKey; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; +import org.bukkit.persistence.PersistentDataType; +import org.junit.jupiter.api.*; +import org.mockbukkit.mockbukkit.MockBukkit; + +class VesselLoreTest { + + @BeforeEach + void setup() { + MockBukkit.mock(); + Magic.plugin = mock(Magic.class); + when(Magic.plugin.namespace()).thenReturn("magic"); + ChargeRegistry.clear(); + TierBands.clear(); + TierBands.register("default", 1, 5); + ElementRegistry.clear(); + Cache.artifactGlint = true; + } + + @AfterEach + void cleanup() { + Magic.plugin = null; + MockBukkit.unmock(); + ChargeRegistry.clear(); + ElementRegistry.clear(); + TierBands.clear(); + } + + static ItemStack withLore(String... lines) { + var item = new ItemStack(Material.STONE); + var meta = item.getItemMeta(); + meta.setLore(List.of(lines)); + item.setItemMeta(meta); + return item; + } + + static ItemStack withKey(NamespacedKey key) { + var item = new ItemStack(Material.STONE); + var meta = item.getItemMeta(); + meta.getPersistentDataContainer().set(key, PersistentDataType.INTEGER, 1); + item.setItemMeta(meta); + return item; + } + + @Test + void onlyItemsWithVesselKeysAChargeMatchOrSlashedLoreAreLookedAt() { + assertFalse(VesselLore.mayHaveVesselLore(new ItemStack(Material.STONE))); + assertFalse(VesselLore.mayHaveVesselLore(withLore("Only lore", "A sword, well kept"))); + assertTrue(VesselLore.mayHaveVesselLore(withLore("Only lore", "Fire 3 / 10"))); + assertTrue(VesselLore.mayHaveVesselLore(withKey(ArtifactKeys.auraCap()))); + assertTrue(VesselLore.mayHaveVesselLore(withKey(ArtifactKeys.auraFill()))); + assertTrue(VesselLore.mayHaveVesselLore(withKey(ArtifactKeys.auraData()))); + assertTrue(VesselLore.mayHaveVesselLore(withKey(ChargeKeys.chargeTier()))); + var shard = new ItemStack(Material.AMETHYST_SHARD); + try (var registry = mockStatic(ChargeRegistry.class)) { + registry.when(() -> ChargeRegistry.match(shard)).thenReturn(new ChargeDef(1, "v.AMETHYST_SHARD", 20)); + assertTrue(VesselLore.mayHaveVesselLore(shard)); + } + } + + @Test + void itemsNoWriterTouchesComeBackUnreadAndUnchanged() { + var plain = withLore("Only lore"); + var before = plain.clone(); + try (var charge = mockStatic(ChargeLore.class)) { + assertSame(plain, VesselLore.updateItem(plain)); + charge.verifyNoInteractions(); + } + assertEquals(before, plain); + // Lore an artifact could be read back from still goes to the artifact writer, which leaves this one alone. + var slashed = withLore("Ten / twenty"); + assertSame(slashed, VesselLore.updateItem(slashed)); + assertEquals(withLore("Ten / twenty"), slashed); + } + + @Test + void inventoryRefreshSetsOnlyTheSlotsWhoseItemChanged() { + var blank = withKey(ChargeKeys.chargeTier()); + var current = withKey(ChargeKeys.chargeTier()); + ChargeLore.apply(current); + var player = mock(Player.class); + var inventory = mock(PlayerInventory.class); + when(player.getInventory()).thenReturn(inventory); + when(inventory.getStorageContents()) + .thenReturn(new ItemStack[] {withLore("Only lore"), null, new ItemStack(Material.AIR), blank}); + when(inventory.getArmorContents()).thenReturn(new ItemStack[] {null, current.clone()}); + var offHand = withKey(ChargeKeys.chargeTier()); + when(inventory.getExtraContents()).thenReturn(new ItemStack[] {offHand}); + VesselLore.updateInventory(player); + verify(inventory).setItem(3, blank); + verify(inventory).setItem(6, offHand); + verify(inventory, times(2)).setItem(anyInt(), any()); + assertEquals(current, blank, "the blank charge got its lore"); + } +} From 1315a42f00422f09c22100eb6c064220a3f9fc69 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 5 Nov 2026 12:07:16 +0100 Subject: [PATCH 2/3] Skip items without meta before any lookup, and test an artifact slot Review: both writers skip an item without meta, so do that first instead of looking up keys and charge matches on it. Co-Authored-By: Claude Opus 5.5 --- .../magic/artifact/aura/VesselLore.java | 13 +++++--- .../magic/artifact/aura/VesselLoreTest.java | 33 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/tfminecraft/magic/artifact/aura/VesselLore.java b/src/main/java/net/tfminecraft/magic/artifact/aura/VesselLore.java index 76cf027..6f22e65 100644 --- a/src/main/java/net/tfminecraft/magic/artifact/aura/VesselLore.java +++ b/src/main/java/net/tfminecraft/magic/artifact/aura/VesselLore.java @@ -1,5 +1,6 @@ package net.tfminecraft.magic.artifact.aura; +import org.bukkit.Bukkit; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.PlayerInventory; @@ -33,15 +34,19 @@ private static ItemStack write(ItemStack stack) { } /** - * False only for an item neither lore writer touches, found from one copy of its meta. A charge has its tier key - * or matches a charge item (ChargeIds.isCharge); an artifact has aura data (AuraData.fromPersistentData) or aura - * lines its lore can be read back from, and each of those has a "/" (ArtifactLore.readAura). For anything else - * both writers leave the item exactly as it was, so it isn't read again for each of them. + * False only for an item neither lore writer touches, found from one copy of its meta. Both writers skip an item + * without meta (hasItemMeta). A charge has its tier key or matches a charge item (ChargeIds.isCharge); an artifact + * has aura data (AuraData.fromPersistentData) or aura lines its lore can be read back from, and each of those has + * a "/" (ArtifactLore.readAura). For anything else both writers leave the item exactly as it was, so it isn't + * read again for each of them. */ // Keep the existing legacy text representation, formatting, and exact-string comparisons. @SuppressWarnings("deprecation") static boolean mayHaveVesselLore(ItemStack stack) { ItemMeta meta = stack.getItemMeta(); + if (Bukkit.getItemFactory().equals(meta, null)) { + return false; + } PersistentDataContainer pdc = meta.getPersistentDataContainer(); if (pdc.has(ArtifactKeys.auraCap()) || pdc.has(ArtifactKeys.auraFill()) || pdc.has(ArtifactKeys.auraData()) || pdc.has(ChargeKeys.chargeTier()) || ChargeRegistry.match(stack) != null) { diff --git a/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java b/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java index d5e1571..ed86018 100644 --- a/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java +++ b/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java @@ -7,15 +7,19 @@ import java.util.List; import net.tfminecraft.magic.Cache; import net.tfminecraft.magic.Magic; +import net.tfminecraft.magic.artifact.Artifact; +import net.tfminecraft.magic.artifact.ArtifactIds; import net.tfminecraft.magic.artifact.ArtifactKeys; import net.tfminecraft.magic.charge.ChargeDef; import net.tfminecraft.magic.charge.ChargeKeys; import net.tfminecraft.magic.charge.ChargeLore; import net.tfminecraft.magic.charge.ChargeRegistry; import net.tfminecraft.magic.charge.TierBands; +import net.tfminecraft.magic.model.ElementDef; import net.tfminecraft.magic.registry.ElementRegistry; import org.bukkit.Material; import org.bukkit.NamespacedKey; +import org.bukkit.configuration.file.YamlConfiguration; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.PlayerInventory; @@ -65,6 +69,11 @@ static ItemStack withKey(NamespacedKey key) { @Test void onlyItemsWithVesselKeysAChargeMatchOrSlashedLoreAreLookedAt() { assertFalse(VesselLore.mayHaveVesselLore(new ItemStack(Material.STONE))); + var named = new ItemStack(Material.STONE); + var meta = named.getItemMeta(); + meta.setDisplayName("Named stone"); + named.setItemMeta(meta); + assertFalse(VesselLore.mayHaveVesselLore(named)); assertFalse(VesselLore.mayHaveVesselLore(withLore("Only lore", "A sword, well kept"))); assertTrue(VesselLore.mayHaveVesselLore(withLore("Only lore", "Fire 3 / 10"))); assertTrue(VesselLore.mayHaveVesselLore(withKey(ArtifactKeys.auraCap()))); @@ -72,6 +81,9 @@ void onlyItemsWithVesselKeysAChargeMatchOrSlashedLoreAreLookedAt() { assertTrue(VesselLore.mayHaveVesselLore(withKey(ArtifactKeys.auraData()))); assertTrue(VesselLore.mayHaveVesselLore(withKey(ChargeKeys.chargeTier()))); var shard = new ItemStack(Material.AMETHYST_SHARD); + var shardMeta = shard.getItemMeta(); + shardMeta.setDisplayName("Charge"); + shard.setItemMeta(shardMeta); try (var registry = mockStatic(ChargeRegistry.class)) { registry.when(() -> ChargeRegistry.match(shard)).thenReturn(new ChargeDef(1, "v.AMETHYST_SHARD", 20)); assertTrue(VesselLore.mayHaveVesselLore(shard)); @@ -112,4 +124,25 @@ void inventoryRefreshSetsOnlyTheSlotsWhoseItemChanged() { verify(inventory, times(2)).setItem(anyInt(), any()); assertEquals(current, blank, "the blank charge got its lore"); } + + @Test + void anArtifactInTheInventoryGetsItsAuraLoreAndOnlyItsSlotIsSet() { + ElementRegistry.register(new ElementDef("fire", new YamlConfiguration())); + var artifact = new ItemStack(Material.STONE); + ArtifactIds.writeNew(artifact); + var aura = Artifact.create(); + aura.setCap("fire", 10); + aura.setFill("fire", 5); + aura.persistPdc(artifact); + var player = mock(Player.class); + var inventory = mock(PlayerInventory.class); + when(player.getInventory()).thenReturn(inventory); + when(inventory.getStorageContents()).thenReturn(new ItemStack[] {new ItemStack(Material.DIRT), artifact}); + when(inventory.getArmorContents()).thenReturn(new ItemStack[0]); + when(inventory.getExtraContents()).thenReturn(new ItemStack[0]); + VesselLore.updateInventory(player); + verify(inventory).setItem(1, artifact); + verify(inventory, times(1)).setItem(anyInt(), any()); + assertTrue(artifact.getItemMeta().getLore().stream().anyMatch(line -> line.contains("/"))); + } } From cd97318c040aec9f7180da60d8c8f43a5c5fb764 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 5 Nov 2026 12:24:24 +0100 Subject: [PATCH 3/3] Prove skipped items reach neither writer, and cover the no-meta skip Review: the plain-item test could not tell the skip from the old path, and MockBukkit's item factory never reports an empty meta. Co-Authored-By: Claude Opus 5.5 --- .../magic/artifact/aura/VesselLoreTest.java | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java b/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java index ed86018..d46a6c4 100644 --- a/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java +++ b/src/test/java/net/tfminecraft/magic/artifact/aura/VesselLoreTest.java @@ -10,6 +10,8 @@ import net.tfminecraft.magic.artifact.Artifact; import net.tfminecraft.magic.artifact.ArtifactIds; import net.tfminecraft.magic.artifact.ArtifactKeys; +import net.tfminecraft.magic.artifact.ArtifactLore; +import net.tfminecraft.magic.charge.ChargeIds; import net.tfminecraft.magic.charge.ChargeDef; import net.tfminecraft.magic.charge.ChargeKeys; import net.tfminecraft.magic.charge.ChargeLore; @@ -17,10 +19,12 @@ import net.tfminecraft.magic.charge.TierBands; import net.tfminecraft.magic.model.ElementDef; import net.tfminecraft.magic.registry.ElementRegistry; +import org.bukkit.Bukkit; import org.bukkit.Material; import org.bukkit.NamespacedKey; import org.bukkit.configuration.file.YamlConfiguration; import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemFactory; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.PlayerInventory; import org.bukkit.persistence.PersistentDataType; @@ -94,15 +98,31 @@ void onlyItemsWithVesselKeysAChargeMatchOrSlashedLoreAreLookedAt() { void itemsNoWriterTouchesComeBackUnreadAndUnchanged() { var plain = withLore("Only lore"); var before = plain.clone(); - try (var charge = mockStatic(ChargeLore.class)) { + try (var ids = mockStatic(ChargeIds.class); + var charge = mockStatic(ChargeLore.class); + var artifact = mockStatic(ArtifactLore.class)) { assertSame(plain, VesselLore.updateItem(plain)); + ids.verifyNoInteractions(); charge.verifyNoInteractions(); + artifact.verifyNoInteractions(); + // Lore an artifact could be read back from still goes to the artifact writer. + var slashed = withLore("Ten / twenty"); + artifact.when(() -> ArtifactLore.updateItem(slashed)).thenReturn(slashed); + assertSame(slashed, VesselLore.updateItem(slashed)); + artifact.verify(() -> ArtifactLore.updateItem(slashed)); } assertEquals(before, plain); - // Lore an artifact could be read back from still goes to the artifact writer, which leaves this one alone. - var slashed = withLore("Ten / twenty"); - assertSame(slashed, VesselLore.updateItem(slashed)); - assertEquals(withLore("Ten / twenty"), slashed); + } + + @Test + void itemsWithoutMetaAreSkippedLikeBothWritersSkipThem() { + // On Paper the item factory calls an item without meta equal to no meta at all; MockBukkit's never does. + var factory = mock(ItemFactory.class); + when(factory.equals(any(), isNull())).thenReturn(true); + try (var bukkit = mockStatic(Bukkit.class, CALLS_REAL_METHODS)) { + bukkit.when(Bukkit::getItemFactory).thenReturn(factory); + assertFalse(VesselLore.mayHaveVesselLore(withKey(ChargeKeys.chargeTier()))); + } } @Test