From 834cf16623f39843c6a343337c9c95d83e35ebe6 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 14 Nov 2026 00:59:19 +0100 Subject: [PATCH 1/2] Make crafted items follow their template's required class Crafted pieces keep the MMOITEMS_REQUIRED_CLASS list stamped at craft time, and the refresh rebuilds them from their own NBT, so a class added to a template (Warden on CUSTOM_WARHAMMER) never reached old pieces. The item refresh (join, character session, opened storage, clicks) now compares a crafted MMOItems item's class list with its current template as a set (order, spacing and repeats ignored). On a mismatch it sets the REQUIRED_CLASS stat to the template list in template order, or removes it when the template has none, and rebuilds through MMOItems' builder so the lore line follows; wear, look, name, PDC and the tier line are kept like the stat refresh does. With a class stat history only its original layer changes. Matching items, unknown templates and non-crafted items are left untouched without a rebuild. Co-Authored-By: Claude Opus 5.5 --- .../utils/AcItemRefresher.java | 15 +- .../utils/RequiredClassSync.java | 111 ++++++++ .../RequiredClassSyncTest.java | 267 ++++++++++++++++++ 3 files changed, 387 insertions(+), 6 deletions(-) create mode 100644 src/main/java/net/tfminecraft/advancedcrafting/utils/RequiredClassSync.java create mode 100644 src/test/java/net/tfminecraft/advancedcrafting/RequiredClassSyncTest.java diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java index d143cb4..7b0db99 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java @@ -30,14 +30,17 @@ public static ItemStack refreshIfOutdated(ItemStack item) { if (item == null || item.getType().isAir()) { return item; } + ItemStack current = item; CraftStatRefresher.RefreshResult crafted = CraftStatRefresher.refreshIfOutdated(item); if (crafted.isChanged()) { - return ArmourLookMigrator.migrate(crafted.getItem()); + current = crafted.getItem(); + } else { + AcItemLoreRefresher.RefreshResult lore = AcItemLoreRefresher.refreshIfOutdated(item); + if (lore.isChanged()) { + current = lore.getItem(); + } } - AcItemLoreRefresher.RefreshResult lore = AcItemLoreRefresher.refreshIfOutdated(item); - if (lore.isChanged()) { - return ArmourLookMigrator.migrate(lore.getItem()); - } - return ArmourLookMigrator.migrate(item); + // After the stat refresh, which rebuilds from the item's own (possibly old) class list. + return ArmourLookMigrator.migrate(RequiredClassSync.sync(current)); } } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/RequiredClassSync.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/RequiredClassSync.java new file mode 100644 index 0000000..8727c87 --- /dev/null +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/RequiredClassSync.java @@ -0,0 +1,111 @@ +package net.tfminecraft.advancedcrafting.utils; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +import org.bukkit.Bukkit; +import org.bukkit.inventory.ItemStack; +import org.bukkit.persistence.PersistentDataType; + +import io.lumine.mythic.lib.api.item.NBTItem; +import net.Indyuce.mmoitems.ItemStats; +import net.Indyuce.mmoitems.MMOItems; +import net.Indyuce.mmoitems.api.item.mmoitem.LiveMMOItem; +import net.Indyuce.mmoitems.api.item.mmoitem.MMOItem; +import net.Indyuce.mmoitems.api.item.template.MMOItemTemplate; +import net.Indyuce.mmoitems.stat.data.StringListData; +import net.Indyuce.mmoitems.stat.type.StatHistory; +import net.tfminecraft.advancedcrafting.objects.CraftStack; + +/** + * A crafted piece keeps the required-class list MMOItems stamped on it when it was made, and every refresh + * rebuilds it from its own data. This makes the list follow the item's current MMOItems template, so a class + * added to or removed from a weapon or armour template reaches pieces that already exist. Only the class stat + * changes; the rebuild keeps everything else the same way a stat refresh does. + */ +public final class RequiredClassSync { + private static final String TAG = "MMOITEMS_REQUIRED_CLASS"; + private static final String HISTORY_TAG = "HSTRY_REQUIRED_CLASS"; + + private RequiredClassSync() { + } + + /** The same item when its classes already match its template or it has no template, otherwise a rebuilt copy. */ + public static ItemStack sync(ItemStack item) { + if (item == null || item.getType().isAir() || !new CraftStack(item).isCrafted()) { + return item; + } + NBTItem nbt = NBTItem.get(item); + if (!nbt.hasType()) { + return item; + } + MMOItemTemplate template = MMOItems.plugin.getTemplates().getTemplate(nbt); + if (template == null) { + return item; + } + List wanted = new ArrayList<>(classSet(listOf(template.getBaseItemData().get(ItemStats.REQUIRED_CLASS)))); + // The cheap check covers almost every item; only a stat history needs the full MMOItems read. + if (!nbt.hasTag(HISTORY_TAG) && classSet(Arrays.asList(nbt.getString(TAG).split(","))).equals(Set.copyOf(wanted))) { + return item; + } + LiveMMOItem mmo = new LiveMMOItem(nbt); + List before = update(mmo, wanted); + if (before == null) { + return item; + } + RefreshKeeper.keepWear(nbt, mmo); + ItemStack rebuilt = RefreshKeeper.keepAppearance(item, mmo.newBuilder().build()); + rebuilt.setAmount(item.getAmount()); + Integer tier = item.getItemMeta().getPersistentDataContainer().get(PDCKeys.craftMajorityTier(), + PersistentDataType.INTEGER); + if (tier != null) { + CraftTierLore.applyTierLine(rebuilt, tier); + } + Bukkit.getLogger().info("[AC][ClassSync] " + nbt.getType() + "." + nbt.getString("MMOITEMS_ITEM_ID") + ": " + + before + " -> " + wanted); + return rebuilt; + } + + /** + * Gives the item the wanted classes, in that order, and touches no other stat. With a stat history only its + * original layer changes, so gem and external layers stay. Returns the classes it had, or null when they + * already matched (as sets: order, spacing and repeats do not count). + */ + public static List update(MMOItem mmo, List wanted) { + StatHistory history = mmo.getStatHistory(ItemStats.REQUIRED_CLASS); + List before = listOf(history != null ? history.getOriginalData() : mmo.getData(ItemStats.REQUIRED_CLASS)); + if (classSet(before).equals(classSet(wanted))) { + return null; + } + StringListData classes = new StringListData(wanted); + if (history != null) { + history.setOriginalData(classes.clone()); + } + if (wanted.isEmpty()) { + mmo.removeData(ItemStats.REQUIRED_CLASS); + } else { + mmo.setData(ItemStats.REQUIRED_CLASS, classes); + } + return before; + } + + /** Trimmed names without blanks or repeats, in their first order. */ + public static Set classSet(Collection names) { + Set result = new LinkedHashSet<>(); + for (String name : names) { + String trimmed = name.trim(); + if (!trimmed.isEmpty()) { + result.add(trimmed); + } + } + return result; + } + + private static List listOf(Object data) { + return data instanceof StringListData list ? list.getList() : List.of(); + } +} diff --git a/src/test/java/net/tfminecraft/advancedcrafting/RequiredClassSyncTest.java b/src/test/java/net/tfminecraft/advancedcrafting/RequiredClassSyncTest.java new file mode 100644 index 0000000..b24fc2a --- /dev/null +++ b/src/test/java/net/tfminecraft/advancedcrafting/RequiredClassSyncTest.java @@ -0,0 +1,267 @@ +package net.tfminecraft.advancedcrafting; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import io.lumine.mythic.lib.api.item.NBTItem; +import java.util.*; +import net.Indyuce.mmoitems.ItemStats; +import net.Indyuce.mmoitems.MMOItems; +import net.Indyuce.mmoitems.api.Type; +import net.Indyuce.mmoitems.api.item.mmoitem.LiveMMOItem; +import net.Indyuce.mmoitems.api.item.mmoitem.MMOItem; +import net.Indyuce.mmoitems.api.item.template.MMOItemTemplate; +import net.Indyuce.mmoitems.stat.data.DoubleData; +import net.Indyuce.mmoitems.stat.data.StringListData; +import net.Indyuce.mmoitems.stat.data.random.RandomStatData; +import net.Indyuce.mmoitems.stat.type.ItemStat; +import net.Indyuce.mmoitems.stat.type.StatHistory; +import net.tfminecraft.advancedcrafting.objects.data.CraftProvenance; +import net.tfminecraft.advancedcrafting.utils.AcItemRefresher; +import net.tfminecraft.advancedcrafting.utils.CraftTierLore; +import net.tfminecraft.advancedcrafting.utils.IaAutoUpdate; +import net.tfminecraft.advancedcrafting.utils.PDCKeys; +import net.tfminecraft.advancedcrafting.utils.RequiredClassSync; +import org.bukkit.Material; +import org.bukkit.inventory.ItemStack; +import org.bukkit.persistence.PersistentDataType; +import org.junit.jupiter.api.Test; +import org.mockito.MockedConstruction; + +class RequiredClassSyncTest extends CoverageSupport { + static final List OLD = List.of("Crusader", "Templar", "Cleric", "Juggernaut"); + static final List NOW = List.of("Crusader", "Templar", "Cleric", "Juggernaut", "Warden"); + + // --- update(): the stat change on a real MMOItem --- + + @Test + void oldListBecomesTemplateListInTemplateOrder() throws Exception { + var mmo = warhammer(); + mmo.setData(ItemStats.REQUIRED_CLASS, new StringListData(OLD)); + assertEquals(OLD, update(mmo, NOW)); + assertEquals(NOW, classes(mmo)); + } + + @Test + void matchingSetsIgnoreOrderSpacingAndRepeats() throws Exception { + var mmo = warhammer(); + var data = new StringListData(List.of(" Warden", "Juggernaut ", "Cleric", "Templar", "Crusader", "Cleric")); + mmo.setData(ItemStats.REQUIRED_CLASS, data); + assertNull(update(mmo, NOW)); + assertSame(data, mmo.getData(ItemStats.REQUIRED_CLASS)); + assertEquals(Set.of("Warden", "Cleric"), RequiredClassSync.classSet(List.of(" Warden ", "", "Cleric", "Warden"))); + } + + @Test + void templateWithoutClassesRemovesTheRestriction() throws Exception { + var mmo = warhammer(); + mmo.setData(ItemStats.REQUIRED_CLASS, new StringListData(OLD)); + assertEquals(OLD, update(mmo, List.of())); + assertFalse(mmo.hasData(ItemStats.REQUIRED_CLASS)); + // An item without classes and a template without classes already match. + assertNull(update(mmo, List.of())); + } + + @Test + void otherStatsAndGemHistoriesStayTheSame() throws Exception { + var mmo = warhammer(); + var damage = new DoubleData(12); + mmo.setData(ItemStats.ATTACK_DAMAGE, damage); + var damageHistory = new StatHistory(mmo, ItemStats.ATTACK_DAMAGE, new DoubleData(9)); + var gem = UUID.randomUUID(); + var gemDamage = new DoubleData(3); + damageHistory.registerGemstoneData(gem, gemDamage); + mmo.setStatHistory(ItemStats.ATTACK_DAMAGE, damageHistory); + mmo.setData(ItemStats.REQUIRED_CLASS, new StringListData(OLD)); + var classHistory = new StatHistory(mmo, ItemStats.REQUIRED_CLASS, new StringListData(OLD)); + var gemClass = new StringListData(List.of("Bard")); + classHistory.registerGemstoneData(gem, gemClass); + mmo.setStatHistory(ItemStats.REQUIRED_CLASS, classHistory); + + assertEquals(OLD, update(mmo, NOW)); + + // Only the class history's original layer changes; its gem layer stays. + assertEquals(NOW, ((StringListData) classHistory.getOriginalData()).getList()); + assertSame(gemClass, classHistory.getGemstoneData(gem)); + assertEquals(NOW, classes(mmo)); + assertSame(damage, mmo.getData(ItemStats.ATTACK_DAMAGE)); + assertSame(damageHistory, mmo.getStatHistory(ItemStats.ATTACK_DAMAGE)); + assertSame(gemDamage, damageHistory.getGemstoneData(gem)); + assertEquals(9, ((DoubleData) damageHistory.getOriginalData()).getValue()); + assertEquals(Set.of(ItemStats.ATTACK_DAMAGE, ItemStats.REQUIRED_CLASS), mmo.getStats()); + // The history's original layer is what is compared, so the next refresh leaves it alone. + assertNull(update(mmo, NOW)); + } + + // --- sync(): the item refresh --- + + @Test + void oldWarhammerGainsWardenThroughAnMmoItemsRebuild() throws Exception { + var item = crafted(2); + CraftTierLore.applyPdc(item, 0, 3); + item.editMeta(meta -> meta.getPersistentDataContainer().set(PDCKeys.alloyId(), PersistentDataType.STRING, "kept")); + var nbt = nbt("Crusader, Templar, Cleric, Juggernaut", false); + template(nbt, NOW); + var rebuilt = new ItemStack(Material.IRON_AXE); + try (var nbts = mockStatic(NBTItem.class); + var skins = mockStatic(net.tfminecraft.tlibs.objects.api.subapi.ItemSkinPreserver.class); + var models = mockStatic(net.tfminecraft.advancedcrafting.util.LegacyModelData.class); + var autoUpdate = mockStatic(IaAutoUpdate.class); + var mmos = live(new StringListData(OLD), null, rebuilt)) { + nbts.when(() -> NBTItem.get(any(ItemStack.class))).thenReturn(nbt); + skins + .when( + () -> + net.tfminecraft.tlibs.objects.api.subapi.ItemSkinPreserver.apply( + any(ItemStack.class), any(ItemStack.class))) + .thenAnswer(call -> call.getArgument(1)); + + var result = RequiredClassSync.sync(item); + + assertSame(rebuilt, result); + assertEquals(2, result.getAmount()); + var tags = result.getItemMeta().getPersistentDataContainer(); + assertEquals(3, tags.get(PDCKeys.craftMajorityTier(), PersistentDataType.INTEGER)); + assertEquals("kept", tags.get(PDCKeys.alloyId(), PersistentDataType.STRING)); + assertNotNull(CraftProvenance.readFrom(result)); + assertTrue(result.getItemMeta().hasLore()); + var mmo = mmos.constructed().get(0); + verify(mmo).setData(eq(ItemStats.REQUIRED_CLASS), argThat(d -> NOW.equals(((StringListData) d).getList()))); + verify(mmo, times(1)).setData(any(), any()); + verify(mmo, never()).removeData(any()); + } + } + + @Test + void templateWithoutClassesRebuildsWithoutTheRestriction() throws Exception { + var item = crafted(1); + var nbt = nbt("Crusader, Templar", false); + template(nbt, null); + var rebuilt = new ItemStack(Material.IRON_AXE); + try (var nbts = mockStatic(NBTItem.class); + var skins = mockStatic(net.tfminecraft.tlibs.objects.api.subapi.ItemSkinPreserver.class); + var models = mockStatic(net.tfminecraft.advancedcrafting.util.LegacyModelData.class); + var autoUpdate = mockStatic(IaAutoUpdate.class); + var mmos = live(new StringListData(List.of("Crusader", "Templar")), null, rebuilt)) { + nbts.when(() -> NBTItem.get(any(ItemStack.class))).thenReturn(nbt); + skins + .when( + () -> + net.tfminecraft.tlibs.objects.api.subapi.ItemSkinPreserver.apply( + any(ItemStack.class), any(ItemStack.class))) + .thenAnswer(call -> call.getArgument(1)); + + var result = RequiredClassSync.sync(item); + + assertSame(rebuilt, result); + assertNull( + result.getItemMeta().getPersistentDataContainer().get(PDCKeys.craftMajorityTier(), PersistentDataType.INTEGER)); + var mmo = mmos.constructed().get(0); + verify(mmo).removeData(ItemStats.REQUIRED_CLASS); + verify(mmo, never()).setData(any(), any()); + } + } + + @Test + void matchingItemIsNotReadOrRebuilt() throws Exception { + var item = crafted(1); + var nbt = nbt("Juggernaut,Crusader, Templar , Cleric, Warden", false); + template(nbt, NOW); + try (var nbts = mockStatic(NBTItem.class); var mmos = live(null, null, null)) { + nbts.when(() -> NBTItem.get(any(ItemStack.class))).thenReturn(nbt); + assertSame(item, RequiredClassSync.sync(item)); + assertSame(item, AcItemRefresher.refreshIfOutdated(item)); + assertTrue(mmos.constructed().isEmpty()); + } + } + + @Test + void matchingHistoryIsReadButNotRebuilt() throws Exception { + var item = crafted(1); + var nbt = nbt("Crusader, Templar, Cleric, Juggernaut, Warden, Bard", true); + template(nbt, NOW); + var history = mock(StatHistory.class); + when(history.getOriginalData()).thenReturn(new StringListData(NOW)); + try (var nbts = mockStatic(NBTItem.class); var mmos = live(null, history, null)) { + nbts.when(() -> NBTItem.get(any(ItemStack.class))).thenReturn(nbt); + assertSame(item, RequiredClassSync.sync(item)); + var mmo = mmos.constructed().get(0); + // The stubbed builder would return null, so reaching the rebuild would fail the sync. + verify(mmo, never()).setData(any(), any()); + verify(mmo, never()).removeData(any()); + verify(history, never()).setOriginalData(any()); + } + } + + @Test + void itemsWithoutATemplateToFollowAreLeftAlone() throws Exception { + assertNull(RequiredClassSync.sync(null)); + var air = new ItemStack(Material.AIR); + assertSame(air, RequiredClassSync.sync(air)); + var item = crafted(1); + var unknown = nbt("Crusader", false); + when(MMOItems.plugin.getTemplates().getTemplate(unknown)).thenReturn(null); + var vanilla = mock(NBTItem.class); + try (var nbts = mockStatic(NBTItem.class); var mmos = live(null, null, null)) { + // Not an AdvancedCrafting craft: never read. + var plain = new ItemStack(Material.IRON_AXE); + assertSame(plain, RequiredClassSync.sync(plain)); + nbts.verifyNoInteractions(); + // Unknown or removed MMOItems template. + nbts.when(() -> NBTItem.get(any(ItemStack.class))).thenReturn(unknown); + assertSame(item, RequiredClassSync.sync(item)); + // Not an MMOItems item at all. + nbts.when(() -> NBTItem.get(any(ItemStack.class))).thenReturn(vanilla); + assertSame(item, RequiredClassSync.sync(item)); + assertTrue(mmos.constructed().isEmpty()); + } + } + + private static MMOItem warhammer() { + return new MMOItem(mock(Type.class), "CUSTOM_WARHAMMER"); + } + + private static List classes(MMOItem mmo) { + return ((StringListData) mmo.getData(ItemStats.REQUIRED_CLASS)).getList(); + } + + private static List update(MMOItem mmo, List wanted) { + return RequiredClassSync.update(mmo, wanted); + } + + private static ItemStack crafted(int amount) { + var item = new ItemStack(Material.IRON_AXE, amount); + new CraftProvenance("warhammer", "fine", List.of(), 0).applyTo(item); + return item; + } + + private static NBTItem nbt(String classes, boolean history) { + var nbt = mock(NBTItem.class); + when(nbt.hasType()).thenReturn(true); + when(nbt.getType()).thenReturn("CUSTOM_WARHAMMER"); + when(nbt.getString("MMOITEMS_ITEM_ID")).thenReturn("PARSUSITE"); + when(nbt.getString("MMOITEMS_REQUIRED_CLASS")).thenReturn(classes); + when(nbt.hasTag("HSTRY_REQUIRED_CLASS")).thenReturn(history); + return nbt; + } + + private static void template(NBTItem nbt, List classes) { + var template = mock(MMOItemTemplate.class); + Map base = new HashMap<>(); + if (classes != null) base.put(ItemStats.REQUIRED_CLASS, new StringListData(classes)); + when(template.getBaseItemData()).thenReturn(base); + when(MMOItems.plugin.getTemplates().getTemplate(nbt)).thenReturn(template); + } + + private static MockedConstruction live( + StringListData classes, StatHistory history, ItemStack rebuilt) { + return mockConstruction( + LiveMMOItem.class, + withSettings().defaultAnswer(RETURNS_DEEP_STUBS), + (mock, ctx) -> { + when(mock.getStatHistory(any())).thenReturn(history); + when(mock.getData(ItemStats.REQUIRED_CLASS)).thenReturn(classes); + when(mock.newBuilder().build()).thenReturn(rebuilt); + }); + } +} From 06886293fe6d6045eac40aeb4d953a32917e8d4a Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 14 Nov 2026 01:10:11 +0100 Subject: [PATCH 2/2] Keep the stat refresh when the class sync fails, and match class names ignoring case Review follow-ups: a class sync error no longer throws away the stat refresh done before it (it is logged and the item kept), and a case-only template change (warden -> Warden) is no change, as MMOItems matches class names ignoring case. The update() javadoc now says a template without classes drops the whole stat. Co-Authored-By: Claude Opus 5.5 --- .../utils/AcItemRefresher.java | 13 ++++++++-- .../utils/RequiredClassSync.java | 24 +++++++++++++++---- .../RequiredClassSyncTest.java | 14 +++++++++++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java index 7b0db99..5665851 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java @@ -1,5 +1,6 @@ package net.tfminecraft.advancedcrafting.utils; +import org.bukkit.Bukkit; import org.bukkit.inventory.ItemStack; import net.tfminecraft.advancedcrafting.objects.CraftStack; @@ -40,7 +41,15 @@ public static ItemStack refreshIfOutdated(ItemStack item) { current = lore.getItem(); } } - // After the stat refresh, which rebuilds from the item's own (possibly old) class list. - return ArmourLookMigrator.migrate(RequiredClassSync.sync(current)); + // After the stat refresh, which rebuilds from the item's own (possibly old) class list. A failure here keeps + // the stat refresh already done. + ItemStack synced; + try { + synced = RequiredClassSync.sync(current); + } catch (RuntimeException e) { + Bukkit.getLogger().warning("[AC][ClassSync] could not update the classes of an item, kept as is: " + e); + synced = current; + } + return ArmourLookMigrator.migrate(synced); } } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/RequiredClassSync.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/RequiredClassSync.java index 8727c87..d1a6288 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/RequiredClassSync.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/RequiredClassSync.java @@ -3,8 +3,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; +import java.util.Locale; import java.util.Set; import org.bukkit.Bukkit; @@ -49,7 +51,7 @@ public static ItemStack sync(ItemStack item) { } List wanted = new ArrayList<>(classSet(listOf(template.getBaseItemData().get(ItemStats.REQUIRED_CLASS)))); // The cheap check covers almost every item; only a stat history needs the full MMOItems read. - if (!nbt.hasTag(HISTORY_TAG) && classSet(Arrays.asList(nbt.getString(TAG).split(","))).equals(Set.copyOf(wanted))) { + if (!nbt.hasTag(HISTORY_TAG) && sameClasses(Arrays.asList(nbt.getString(TAG).split(",")), wanted)) { return item; } LiveMMOItem mmo = new LiveMMOItem(nbt); @@ -72,13 +74,14 @@ public static ItemStack sync(ItemStack item) { /** * Gives the item the wanted classes, in that order, and touches no other stat. With a stat history only its - * original layer changes, so gem and external layers stay. Returns the classes it had, or null when they - * already matched (as sets: order, spacing and repeats do not count). + * original layer changes, so gem and external layers stay; when the template has no classes the whole class + * stat goes, history included. Returns the classes it had, or null when they already matched (as sets: case, + * order, spacing and repeats do not count). */ public static List update(MMOItem mmo, List wanted) { StatHistory history = mmo.getStatHistory(ItemStats.REQUIRED_CLASS); List before = listOf(history != null ? history.getOriginalData() : mmo.getData(ItemStats.REQUIRED_CLASS)); - if (classSet(before).equals(classSet(wanted))) { + if (sameClasses(before, wanted)) { return null; } StringListData classes = new StringListData(wanted); @@ -105,6 +108,19 @@ public static Set classSet(Collection names) { return result; } + /** Same classes as sets, ignoring case as MMOItems does when it checks a player's class. */ + public static boolean sameClasses(Collection a, Collection b) { + return lowerCase(classSet(a)).equals(lowerCase(classSet(b))); + } + + private static Set lowerCase(Set names) { + Set result = new HashSet<>(); + for (String name : names) { + result.add(name.toLowerCase(Locale.ROOT)); + } + return result; + } + private static List listOf(Object data) { return data instanceof StringListData list ? list.getList() : List.of(); } diff --git a/src/test/java/net/tfminecraft/advancedcrafting/RequiredClassSyncTest.java b/src/test/java/net/tfminecraft/advancedcrafting/RequiredClassSyncTest.java index b24fc2a..6f45f59 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/RequiredClassSyncTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/RequiredClassSyncTest.java @@ -50,6 +50,11 @@ void matchingSetsIgnoreOrderSpacingAndRepeats() throws Exception { assertNull(update(mmo, NOW)); assertSame(data, mmo.getData(ItemStats.REQUIRED_CLASS)); assertEquals(Set.of("Warden", "Cleric"), RequiredClassSync.classSet(List.of(" Warden ", "", "Cleric", "Warden"))); + // MMOItems matches class names ignoring case, so a case-only difference is no change. + assertTrue(RequiredClassSync.sameClasses(List.of("warden", "CLERIC"), List.of("Cleric", "Warden"))); + assertFalse(RequiredClassSync.sameClasses(List.of("Warden"), List.of("Warden", "Cleric"))); + mmo.setData(ItemStats.REQUIRED_CLASS, new StringListData(List.of("crusader", "templar", "cleric", "juggernaut", "warden"))); + assertNull(update(mmo, NOW)); } @Test @@ -193,6 +198,15 @@ void matchingHistoryIsReadButNotRebuilt() throws Exception { } } + @Test + void aFailedClassSyncKeepsTheItem() { + var item = new ItemStack(Material.PAPER); + try (var sync = mockStatic(RequiredClassSync.class)) { + sync.when(() -> RequiredClassSync.sync(any())).thenThrow(new IllegalStateException("broken stat")); + assertSame(item, AcItemRefresher.refreshIfOutdated(item)); + } + } + @Test void itemsWithoutATemplateToFollowAreLeftAlone() throws Exception { assertNull(RequiredClassSync.sync(null));