From 7fe627c7a3a83dcfbe9a04bf8fb8257c4bc61e00 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 8 Oct 2026 19:05:33 +0200 Subject: [PATCH 1/9] Tag crafted items with the model scheme they wear ArmourShop needs to know which metal a crafted armour piece is, but every crafted piece shares one *_custom_* MMOItems template per weight, so the item id cannot tell steel from mythril. The craft record now also stores advancedcrafting:ac_craft_model_scheme: the scheme whose look the item got (iron, steel, bronze, ...). Alloys use their base ingredient's scheme, so an alloy piece carries its base metal. The scheme choice moved into ModelSchemeResolver, which the station and the craft record share. Join/chest refreshes re-apply the craft record, so refreshed items keep the tag and older crafted items gain it when they refresh. Co-Authored-By: Claude Opus 5.5 --- .../objects/crafting/CraftingStation.java | 16 +---- .../objects/data/CraftProvenance.java | 7 ++ .../utils/ModelSchemeResolver.java | 69 ++++++++++++++++++ .../advancedcrafting/utils/PDCKeys.java | 5 ++ .../ModelSchemeResolverTest.java | 72 +++++++++++++++++++ .../advancedcrafting/StationCoverageTest.java | 14 ++-- 6 files changed, 165 insertions(+), 18 deletions(-) create mode 100644 src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java create mode 100644 src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java diff --git a/src/main/java/net/tfminecraft/advancedcrafting/objects/crafting/CraftingStation.java b/src/main/java/net/tfminecraft/advancedcrafting/objects/crafting/CraftingStation.java index b080ea9..c7ad042 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/objects/crafting/CraftingStation.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/objects/crafting/CraftingStation.java @@ -53,6 +53,7 @@ import net.tfminecraft.advancedcrafting.utils.CraftTierLore; import net.tfminecraft.advancedcrafting.utils.MMOStatApplicator; import net.tfminecraft.advancedcrafting.utils.MajorityTierResolver; +import net.tfminecraft.advancedcrafting.utils.ModelSchemeResolver; import net.tfminecraft.advancedcrafting.utils.ProfessionPermissions; import net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle; @@ -506,20 +507,7 @@ private StationFeedback createItem(Player p, Double forcedQualityPercent, boolea mmo.setData(ItemStats.LORE, lore); ItemStack finalItem = mmo.newBuilder().build(); if(scheme != null) { - // A recipe-specific model on the main material wins over the secondary model ingredient. - if(scheme.getModel(recipe.getId()) == null && !recipe.getModelType().equalsIgnoreCase("none")) { - for(String s : currentMaterials.keySet()) { - String modeltype = s.split("\\.")[0]; - String modelId = s.split("\\.")[1]; - if(modeltype.equalsIgnoreCase("ingredient")) { - Ingredient ing = IngredientLoader.getByString(modelId); - if(ing.getIngredientData().getType().getId().equalsIgnoreCase(recipe.getModelType())) scheme = ing.getIngredientData().getModelScheme(); - } else if(modeltype.equalsIgnoreCase("alloy")) { - Alloy a = AlloyManager.getAlloyById(modelId); - if(a.getData().getType().getId().equalsIgnoreCase(recipe.getModelType())) scheme = a.getData().getModelScheme(); - } - } - } + scheme = ModelSchemeResolver.forModel(recipe, scheme, currentMaterials); finalItem = applyModel(finalItem, scheme); } CraftProvenance provenance = CraftProvenance.from(recipe, currentMaterials, q); diff --git a/src/main/java/net/tfminecraft/advancedcrafting/objects/data/CraftProvenance.java b/src/main/java/net/tfminecraft/advancedcrafting/objects/data/CraftProvenance.java index 6fe6dc3..dfadcdf 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/objects/data/CraftProvenance.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/objects/data/CraftProvenance.java @@ -18,7 +18,9 @@ import net.tfminecraft.advancedcrafting.objects.crafting.CraftingRecipe; import net.tfminecraft.advancedcrafting.objects.crafting.Quality; import net.tfminecraft.advancedcrafting.objects.ingredients.Ingredient; +import net.tfminecraft.advancedcrafting.objects.schemes.ModelScheme; import net.tfminecraft.advancedcrafting.objects.stats.StatTemplate; +import net.tfminecraft.advancedcrafting.utils.ModelSchemeResolver; import net.tfminecraft.advancedcrafting.utils.PDCKeys; public class CraftProvenance { @@ -101,6 +103,11 @@ public void applyTo(ItemStack item) { meta.getPersistentDataContainer().set(PDCKeys.craftInputs(), PersistentDataType.STRING, GSON.toJson(inputs)); meta.getPersistentDataContainer().set(PDCKeys.craftStatTemplateRevision(), PersistentDataType.INTEGER, statTemplateRevision); + ModelScheme scheme = ModelSchemeResolver.resolve(RecipeLoader.getByString(recipeId), inputs); + if (scheme != null) { + meta.getPersistentDataContainer().set(PDCKeys.craftModelScheme(), PersistentDataType.STRING, + scheme.getId()); + } item.setItemMeta(meta); } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java new file mode 100644 index 0000000..a18488c --- /dev/null +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java @@ -0,0 +1,69 @@ +package net.tfminecraft.advancedcrafting.utils; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import net.tfminecraft.advancedcrafting.loaders.IngredientLoader; +import net.tfminecraft.advancedcrafting.managers.AlloyManager; +import net.tfminecraft.advancedcrafting.objects.alloys.Alloy; +import net.tfminecraft.advancedcrafting.objects.crafting.CraftingRecipe; +import net.tfminecraft.advancedcrafting.objects.data.CraftInput; +import net.tfminecraft.advancedcrafting.objects.ingredients.Ingredient; +import net.tfminecraft.advancedcrafting.objects.schemes.ModelScheme; + +/** + * Picks the model scheme whose look a crafted item wears: the main material's scheme, unless that scheme + * has no model for the recipe and a material of the recipe's model type brings its own. Alloys use their + * base's scheme, so an alloy piece wears (and is tagged with) its base metal. + */ +public final class ModelSchemeResolver { + private ModelSchemeResolver() { + } + + /** The scheme of a recorded craft, or null when the recipe or inputs are unknown. */ + public static ModelScheme resolve(CraftingRecipe recipe, List inputs) { + if (recipe == null || inputs == null || inputs.isEmpty()) { + return null; + } + HashMap materials = new HashMap<>(); + for (CraftInput input : inputs) { + materials.put(input.getKind().toLowerCase() + "." + input.getId().toLowerCase(), input.getAmount()); + } + ModelScheme main = schemeOf(MajorityTierResolver.resolveMajorityKey(recipe, materials), null, null); + return forModel(recipe, main, materials); + } + + /** A recipe-specific model on the main material wins over the secondary model ingredient. */ + public static ModelScheme forModel(CraftingRecipe recipe, ModelScheme scheme, Map materials) { + if (scheme == null || scheme.getModel(recipe.getId()) != null + || recipe.getModelType().equalsIgnoreCase("none")) { + return scheme; + } + for (String key : materials.keySet()) { + scheme = schemeOf(key, recipe.getModelType(), scheme); + } + return scheme; + } + + /** The scheme of one material key if it exists and (when given) has the wanted type, else the fallback. */ + private static ModelScheme schemeOf(String key, String wantedType, ModelScheme fallback) { + String[] split = key.split("\\."); + if (split.length < 2) { + return fallback; + } + if (split[0].equalsIgnoreCase("ingredient")) { + Ingredient ing = IngredientLoader.getByString(split[1]); + if (ing != null && (wantedType == null + || ing.getIngredientData().getType().getId().equalsIgnoreCase(wantedType))) { + return ing.getIngredientData().getModelScheme(); + } + } else if (split[0].equalsIgnoreCase("alloy")) { + Alloy a = AlloyManager.getAlloyById(split[1]); + if (a != null && (wantedType == null || a.getData().getType().getId().equalsIgnoreCase(wantedType))) { + return a.getData().getModelScheme(); + } + } + return fallback; + } +} diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/PDCKeys.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/PDCKeys.java index c737a7e..7ca4c77 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/PDCKeys.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/PDCKeys.java @@ -24,6 +24,11 @@ public static NamespacedKey craftStatTemplateRevision() { return new NamespacedKey(AdvancedCrafting.plugin, "ac_craft_stat_template_revision"); } + /** Model scheme the crafted item wears (iron, steel, ...); alloys carry their base's. Read by ArmourShop. */ + public static NamespacedKey craftModelScheme() { + return new NamespacedKey(AdvancedCrafting.plugin, "ac_craft_model_scheme"); + } + public static NamespacedKey scrapBase() { return new NamespacedKey(AdvancedCrafting.plugin, "ac_scrap_base"); } diff --git a/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java b/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java new file mode 100644 index 0000000..4ffdb6d --- /dev/null +++ b/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java @@ -0,0 +1,72 @@ +package net.tfminecraft.advancedcrafting; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.*; +import net.tfminecraft.advancedcrafting.loaders.*; +import net.tfminecraft.advancedcrafting.managers.AlloyManager; +import net.tfminecraft.advancedcrafting.objects.alloys.Alloy; +import net.tfminecraft.advancedcrafting.objects.crafting.CraftingRecipe; +import net.tfminecraft.advancedcrafting.objects.data.*; +import net.tfminecraft.advancedcrafting.objects.ingredients.*; +import net.tfminecraft.advancedcrafting.objects.schemes.ModelScheme; +import net.tfminecraft.advancedcrafting.utils.ModelSchemeResolver; +import org.junit.jupiter.api.Test; + +class ModelSchemeResolverTest extends CoverageSupport { + static CraftInput in(String kind, String id, int amount) { + return new CraftInput(kind, id, amount, 0); + } + + static String scheme(CraftingRecipe recipe, CraftInput... inputs) { + ModelScheme scheme = ModelSchemeResolver.resolve(recipe, List.of(inputs)); + return scheme == null ? null : scheme.getId(); + } + + Alloy alloy(String id, Ingredient base) { + var alloy = new Alloy(id, id, new AlloyData(base, new StatData(), new HashMap<>(), null)); + AlloyManager.addAlloy(alloy); + return alloy; + } + + @Test + void recordedCraftsResolveTheSchemeTheirLookCameFromAndAlloysUseTheirBase() throws Exception { + TypeLoader.map.put("paper", new IngredientType("paper", yaml("name: Paper"))); + SchemeLoader.models.put("iron", new ModelScheme("iron", yaml("models: ['smith(v.iron_sword.1)']"))); + SchemeLoader.models.put("own", new ModelScheme("own", yaml("models: ['sword(v.iron_sword.2)']"))); + SchemeLoader.models.put("cloth", new ModelScheme("cloth", yaml("models: ['smith(v.golden_sword.3)']"))); + ingredient("iron", "model-scheme: iron"); + ingredient("own", "model-scheme: own"); + var cloth = ingredient("cloth", "type: paper\nmodel-scheme: cloth"); + alloy("darksteel", IngredientLoader.getByString("iron")); + alloy("clothsteel", cloth); + var noModel = recipe("recipe: ['metal.1', 'paper.1']\nmodel-type: none"); + var byPaper = recipe("recipe: ['metal.1', 'paper.1']\nmodel-type: paper"); + + assertNull(ModelSchemeResolver.resolve(null, List.of(in("ingredient", "iron", 1)))); + assertNull(ModelSchemeResolver.resolve(noModel, null)); + assertNull(ModelSchemeResolver.resolve(noModel, List.of())); + + // The main material decides; an alloy wears its base metal's scheme. + assertEquals("iron", scheme(noModel, in("ingredient", "iron", 3), in("ingredient", "cloth", 1))); + assertEquals("iron", scheme(noModel, in("alloy", "darksteel", 3))); + // A model-type material takes over when the main scheme has no model for the recipe ... + assertEquals("cloth", scheme(byPaper, in("alloy", "darksteel", 3), in("ingredient", "cloth", 1))); + assertEquals("cloth", scheme(byPaper, in("ingredient", "iron", 3), in("alloy", "clothsteel", 1))); + // ... but not when it has one of its own. + assertEquals("own", scheme(byPaper, in("ingredient", "own", 3), in("ingredient", "cloth", 1))); + // Unknown, malformed and foreign inputs leave the main scheme alone. + assertEquals( + "iron", + scheme( + byPaper, + in("ingredient", "iron", 3), + in("alloy", "darksteel", 1), + in("ingredient", "gone", 1), + in("alloy", "gone", 1), + in("ingredient", "", 1), + in("misc", "x", 1))); + assertNull(scheme(noModel, in("ingredient", "gone", 1))); + assertNull(scheme(noModel, in("alloy", "gone", 1))); + } +} diff --git a/src/test/java/net/tfminecraft/advancedcrafting/StationCoverageTest.java b/src/test/java/net/tfminecraft/advancedcrafting/StationCoverageTest.java index 4557004..e6ecdd3 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/StationCoverageTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/StationCoverageTest.java @@ -404,12 +404,11 @@ void mainMaterialRecipeModelWinsAndSecondaryIngredientRemainsFallback() throws E specific ? Material.DIAMOND_SWORD : modelType.equals("none") ? Material.IRON_SWORD : Material.GOLDEN_SWORD; + var craftRecipe = recipe("recipe: ['metal.1', 'paper.1']\nmodel-type: " + modelType); + RecipeLoader.map.put(craftRecipe.getId(), craftRecipe); var station = new CraftingStation( - loc(), - recipe("recipe: ['metal.1', 'paper.1']\nmodel-type: " + modelType), - new HashMap<>(Map.of(key, 1, secondary, 1)), - new HashMap<>()); + loc(), craftRecipe, new HashMap<>(Map.of(key, 1, secondary, 1)), new HashMap<>()); station.getCurrentMaterials().put("unknown.removed", 1); try (var nbt = mockStatic(NBTItem.class); var models = mockStatic(LegacyModelData.class); @@ -425,6 +424,13 @@ void mainMaterialRecipeModelWinsAndSecondaryIngredientRemainsFallback() throws E var result = station.buildCompletedItem(p, 50.); assertEquals(expectedMaterial, result.getType()); models.verify(() -> LegacyModelData.set(any(), eq(expectedModel))); + // The tag names the scheme whose look the item got; alloys carry their base's. + assertEquals( + expectedModel == 3 ? "cloth-look" : "metal-look", + result + .getItemMeta() + .getPersistentDataContainer() + .get(PDCKeys.craftModelScheme(), PersistentDataType.STRING)); } } } From 4faad34f58223bf622fb9b12e4c0735b8b6bdbdd Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 10 Oct 2026 08:22:36 +0200 Subject: [PATCH 2/9] Address review: drop a stale scheme tag and malformed recorded inputs - applyTo removes ac_craft_model_scheme when the recipe or inputs no longer resolve, so ArmourShop never trusts an old metal. - readFrom drops recorded inputs with a null entry, kind or id, which every refresh consumer (outdated check, stat calculator, majority tier, scheme resolver) reads; the resolver also skips them when called directly. Co-Authored-By: Claude Opus 5.5 --- .../objects/data/CraftProvenance.java | 10 +++++++++- .../utils/ModelSchemeResolver.java | 3 +++ .../ItemMetadataCoverageTest.java | 19 +++++++++++++++++++ .../ModelSchemeResolverTest.java | 7 +++++++ 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/advancedcrafting/objects/data/CraftProvenance.java b/src/main/java/net/tfminecraft/advancedcrafting/objects/data/CraftProvenance.java index dfadcdf..46fb7c8 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/objects/data/CraftProvenance.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/objects/data/CraftProvenance.java @@ -107,6 +107,9 @@ public void applyTo(ItemStack item) { if (scheme != null) { meta.getPersistentDataContainer().set(PDCKeys.craftModelScheme(), PersistentDataType.STRING, scheme.getId()); + } else { + // A recipe or ingredient that no longer resolves must not leave an old metal for ArmourShop to trust. + meta.getPersistentDataContainer().remove(PDCKeys.craftModelScheme()); } item.setItemMeta(meta); } @@ -129,7 +132,12 @@ public static CraftProvenance readFrom(ItemStack item) { if (inputsJson != null) { List parsed = GSON.fromJson(inputsJson, new TypeToken>() {}.getType()); if (parsed != null) { - inputs = parsed; + // Every refresh consumer reads these inputs; a hand-edited or truncated record must not break them. + for (CraftInput input : parsed) { + if (input != null && input.getKind() != null && input.getId() != null) { + inputs.add(input); + } + } } } Integer templateRevision = meta.getPersistentDataContainer().get(PDCKeys.craftStatTemplateRevision(), diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java index a18488c..501db9f 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java @@ -28,6 +28,9 @@ public static ModelScheme resolve(CraftingRecipe recipe, List inputs } HashMap materials = new HashMap<>(); for (CraftInput input : inputs) { + if (input == null || input.getKind() == null || input.getId() == null) { + continue; + } materials.put(input.getKind().toLowerCase() + "." + input.getId().toLowerCase(), input.getAmount()); } ModelScheme main = schemeOf(MajorityTierResolver.resolveMajorityKey(recipe, materials), null, null); diff --git a/src/test/java/net/tfminecraft/advancedcrafting/ItemMetadataCoverageTest.java b/src/test/java/net/tfminecraft/advancedcrafting/ItemMetadataCoverageTest.java index 7ae22fe..5ef0600 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/ItemMetadataCoverageTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/ItemMetadataCoverageTest.java @@ -223,6 +223,25 @@ void craftProvenanceRoundTripsAndDetectsIngredientAndTemplateRevisions() throws meta.getPersistentDataContainer().set(PDCKeys.craftInputs(), PersistentDataType.STRING, "null"); minimal.setItemMeta(meta); assertTrue(CraftProvenance.readFrom(minimal).getInputs().isEmpty()); + // Malformed records (null entries, kinds or ids) are dropped instead of breaking every refresh. + meta = minimal.getItemMeta(); + meta.getPersistentDataContainer() + .set( + PDCKeys.craftInputs(), + PersistentDataType.STRING, + "[null,{\"id\":\"iron\",\"n\":1},{\"k\":\"ingredient\",\"n\":1}," + + "{\"k\":\"ingredient\",\"id\":\"iron\",\"n\":2,\"r\":1}]"); + minimal.setItemMeta(meta); + var kept = CraftProvenance.readFrom(minimal).getInputs(); + assertEquals(1, kept.size()); + assertEquals("iron", kept.get(0).getId()); + // A record whose recipe no longer resolves loses its old scheme tag. + meta = minimal.getItemMeta(); + meta.getPersistentDataContainer().set(PDCKeys.craftModelScheme(), PersistentDataType.STRING, "old"); + minimal.setItemMeta(meta); + CraftProvenance.readFrom(minimal).applyTo(minimal); + assertNull( + minimal.getItemMeta().getPersistentDataContainer().get(PDCKeys.craftModelScheme(), PersistentDataType.STRING)); assertNotNull(new CraftProvenance().getInputs()); } diff --git a/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java b/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java index 4ffdb6d..c65aee8 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java @@ -67,6 +67,13 @@ void recordedCraftsResolveTheSchemeTheirLookCameFromAndAlloysUseTheirBase() thro in("ingredient", "", 1), in("misc", "x", 1))); assertNull(scheme(noModel, in("ingredient", "gone", 1))); + // Malformed inputs are skipped. + assertEquals( + "iron", + ModelSchemeResolver.resolve( + noModel, + Arrays.asList(null, in(null, "iron", 1), in("ingredient", null, 1), in("ingredient", "iron", 3))) + .getId()); assertNull(scheme(noModel, in("alloy", "gone", 1))); } } From 104f427144810b8733b285e97282c63f5ce86542 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:35:43 +0200 Subject: [PATCH 3/9] Move crafted armour still in its old default look to the per-recipe model Pieces crafted before the per-recipe models still wear their metal's shared type model (iron chainmail, steel iron, bronze bronze_*, ...) or the infantry paper / mage dust look. When AcItemRefresher sees a recorded craft (join, ender chest, opened storage, click, equip), ArmourLookMigrator gives it the recipe's current model through ArmorMerger, so its MMOItems data stays. Any other look is a skin and is kept. The old look is saved in ac_previous_model and logged. Every recorded craft also gets its model scheme tag, so ArmourShop's metal skin lines apply to older pieces too. armour-look-migration.enabled is off by default; legacy-models lists the old looks of recipes whose look came from another ingredient. Co-Authored-By: Claude Opus 5.5 --- .../advancedcrafting/cache/Cache.java | 5 + .../loaders/ConfigLoader.java | 13 + .../objects/crafting/CraftingStation.java | 25 +- .../utils/AcItemRefresher.java | 6 +- .../utils/ArmourLookMigrator.java | 124 +++++++ .../advancedcrafting/utils/ModelApplier.java | 39 ++ .../advancedcrafting/utils/PDCKeys.java | 5 + src/main/resources/config.yml | 17 + .../ArmourLookMigratorTest.java | 337 ++++++++++++++++++ 9 files changed, 546 insertions(+), 25 deletions(-) create mode 100644 src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java create mode 100644 src/main/java/net/tfminecraft/advancedcrafting/utils/ModelApplier.java create mode 100644 src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java diff --git a/src/main/java/net/tfminecraft/advancedcrafting/cache/Cache.java b/src/main/java/net/tfminecraft/advancedcrafting/cache/Cache.java index 189c7f5..098abf2 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/cache/Cache.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/cache/Cache.java @@ -39,6 +39,11 @@ public class Cache { public static boolean debugStatRefresh; + /** Crafted pieces still wearing an old default look get their recipe's current model. */ + public static boolean armourLookMigration; + /** Old default looks per recipe id, for recipes whose old look did not come from their metal's type model. */ + public static Map> legacyModels = new HashMap<>(); + public static boolean showIngredientStats = true; /** Divisors applied to bucket-averaged stats before template factors (e.g. movement_speed: 100). */ diff --git a/src/main/java/net/tfminecraft/advancedcrafting/loaders/ConfigLoader.java b/src/main/java/net/tfminecraft/advancedcrafting/loaders/ConfigLoader.java index 1ff236d..fdfe206 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/loaders/ConfigLoader.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/loaders/ConfigLoader.java @@ -79,6 +79,19 @@ public void load(File configFile) { Cache.hitOvershootWarnMessage = overshootMessage; Cache.debugStatRefresh = config.getBoolean("debug-stat-refresh", false); + + Cache.armourLookMigration = config.getBoolean("armour-look-migration.enabled", false); + Cache.legacyModels.clear(); + for (String entry : config.getStringList("armour-look-migration.legacy-models")) { + int open = entry.indexOf('('); + if (open <= 0 || !entry.endsWith(")")) { + AdvancedCrafting.plugin.getLogger().warning("Ignoring armour-look-migration legacy model '" + entry + + "'; expected ()"); + continue; + } + Cache.legacyModels.computeIfAbsent(entry.substring(0, open).trim().toLowerCase(), key -> new ArrayList<>()) + .add(entry.substring(open + 1, entry.length() - 1).trim()); + } Cache.showIngredientStats = config.getBoolean("show-ingredient-stats", true); Cache.globalStatOffsets.clear(); diff --git a/src/main/java/net/tfminecraft/advancedcrafting/objects/crafting/CraftingStation.java b/src/main/java/net/tfminecraft/advancedcrafting/objects/crafting/CraftingStation.java index c7ad042..3764e7a 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/objects/crafting/CraftingStation.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/objects/crafting/CraftingStation.java @@ -1,23 +1,19 @@ package net.tfminecraft.advancedcrafting.objects.crafting; -import net.tfminecraft.advancedcrafting.util.LegacyModelData; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.commons.lang.WordUtils; import org.bukkit.Bukkit; import org.bukkit.Location; -import org.bukkit.Material; import org.bukkit.command.ConsoleCommandSender; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; -import org.bukkit.inventory.meta.ItemMeta; import io.lumine.mythic.lib.api.item.NBTItem; import net.tfminecraft.tlibs.TLibs; @@ -53,6 +49,7 @@ import net.tfminecraft.advancedcrafting.utils.CraftTierLore; import net.tfminecraft.advancedcrafting.utils.MMOStatApplicator; import net.tfminecraft.advancedcrafting.utils.MajorityTierResolver; +import net.tfminecraft.advancedcrafting.utils.ModelApplier; import net.tfminecraft.advancedcrafting.utils.ModelSchemeResolver; import net.tfminecraft.advancedcrafting.utils.ProfessionPermissions; import net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle; @@ -527,29 +524,13 @@ private StationFeedback createItem(Player p, Double forcedQualityPercent, boolea return StationFeedback.SUCCESS; } - // This path mutates the existing ItemStack; replacing it would change aliases held by callers. - @SuppressWarnings("deprecation") private ItemStack applyModel(ItemStack i, ModelScheme scheme) { - // A model named after the recipe (e.g. heavy_helmet) wins over the shared type (helmet). - String path = scheme.getModel(recipe.getId()); - if(path == null) { - path = scheme.getModel(recipe.getType()); - } + String path = ModelApplier.modelFor(scheme, recipe); if(path == null) { Bukkit.getLogger().warning("AC: No model in the scheme "+scheme.getId()+" for the recipe type "+recipe.getType()); return i; } - String type = path.split("\\.")[0]; - if(type.equalsIgnoreCase("v")) { - i.setType(Material.valueOf(path.split("\\.")[1].toUpperCase())); - ItemMeta m = i.getItemMeta(); - LegacyModelData.set(m, Integer.parseInt(path.split("\\.")[2])); - i.setItemMeta(m); - } else if(type.equalsIgnoreCase("ia")) { - ItemAPI api = TLibs.getItemAPI(); - i = api.getArmorMerger().merge(i, Optional.empty(), path); - } - return i; + return ModelApplier.apply(i, path); } public StationFeedback hit(Player p, ItemStack i) { diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java index 95bdef9..d143cb4 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/AcItemRefresher.java @@ -32,12 +32,12 @@ public static ItemStack refreshIfOutdated(ItemStack item) { } CraftStatRefresher.RefreshResult crafted = CraftStatRefresher.refreshIfOutdated(item); if (crafted.isChanged()) { - return crafted.getItem(); + return ArmourLookMigrator.migrate(crafted.getItem()); } AcItemLoreRefresher.RefreshResult lore = AcItemLoreRefresher.refreshIfOutdated(item); if (lore.isChanged()) { - return lore.getItem(); + return ArmourLookMigrator.migrate(lore.getItem()); } - return item; + return ArmourLookMigrator.migrate(item); } } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java new file mode 100644 index 0000000..3515335 --- /dev/null +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java @@ -0,0 +1,124 @@ +package net.tfminecraft.advancedcrafting.utils; + +import java.util.List; +import java.util.Locale; + +import org.bukkit.Bukkit; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; +import org.bukkit.persistence.PersistentDataContainer; +import org.bukkit.persistence.PersistentDataType; + +import io.lumine.mythic.lib.api.item.NBTItem; +import net.tfminecraft.advancedcrafting.cache.Cache; +import net.tfminecraft.advancedcrafting.loaders.RecipeLoader; +import net.tfminecraft.advancedcrafting.objects.crafting.CraftingRecipe; +import net.tfminecraft.advancedcrafting.objects.data.CraftProvenance; +import net.tfminecraft.advancedcrafting.objects.schemes.ModelScheme; +import net.tfminecraft.advancedcrafting.util.LegacyModelData; + +/** + * Brings crafted pieces made before the per-recipe models up to date. A piece that still wears the old default + * look of its recipe (its metal's type model, or a configured legacy model) gets the recipe's current model + * through the same merge skins use, so its MMOItems data stays. Any other look is a skin and is kept. Every + * recorded craft also gets its model scheme tag, which ArmourShop's metal skin lines read. + */ +public final class ArmourLookMigrator { + private ArmourLookMigrator() { + } + + /** The same item when nothing changes, otherwise an updated copy. */ + public static ItemStack migrate(ItemStack item) { + if (!Cache.armourLookMigration || item == null || item.getType().isAir() || !item.hasItemMeta()) { + return item; + } + CraftProvenance provenance = CraftProvenance.readFrom(item); + if (provenance == null) { + return item; + } + CraftingRecipe recipe = RecipeLoader.getByString(provenance.getRecipeId()); + ModelScheme scheme = ModelSchemeResolver.resolve(recipe, provenance.getInputs()); + if (scheme == null) { + return item; + } + ItemStack result = item; + String target = scheme.getModel(recipe.getId()); + if (target != null && !wears(item, target) && wearsLegacyLook(item, recipe, scheme)) { + String before = describe(item); + result = ModelApplier.apply(item.clone(), target); + ItemMeta meta = result.getItemMeta(); + meta.getPersistentDataContainer().set(PDCKeys.previousModel(), PersistentDataType.STRING, before); + result.setItemMeta(meta); + Bukkit.getLogger().info("[AC][LookMigration] " + recipe.getId() + " (" + scheme.getId() + "): " + before + + " -> " + target); + } + PersistentDataContainer tags = result.getItemMeta().getPersistentDataContainer(); + if (!scheme.getId().equals(tags.get(PDCKeys.craftModelScheme(), PersistentDataType.STRING))) { + if (result == item) { + result = item.clone(); + } + ItemMeta meta = result.getItemMeta(); + meta.getPersistentDataContainer().set(PDCKeys.craftModelScheme(), PersistentDataType.STRING, scheme.getId()); + result.setItemMeta(meta); + } + return result; + } + + /** Configured legacy models replace the scheme's type model, e.g. infantry pieces wore the paper look. */ + static boolean wearsLegacyLook(ItemStack item, CraftingRecipe recipe, ModelScheme scheme) { + List legacy = Cache.legacyModels.get(recipe.getId().toLowerCase(Locale.ROOT)); + if (legacy != null) { + for (String path : legacy) { + if (wears(item, path)) { + return true; + } + } + return false; + } + String typeModel = scheme.getModel(recipe.getType()); + return typeModel != null && wears(item, typeModel); + } + + /** Whether the item shows this scheme model: the same ItemsAdder item, or the same vanilla material and model. */ + static boolean wears(ItemStack item, String path) { + String[] parts = path.split("\\."); + String ia = iaId(item); + if (parts[0].equalsIgnoreCase("ia")) { + return ia != null && ia.equalsIgnoreCase(path.substring(3)); + } + if (!parts[0].equalsIgnoreCase("v") || parts.length != 3 || ia != null || hasItemsAdderCompound(item) + || !item.getType().name().equalsIgnoreCase(parts[1])) { + return false; + } + int model = Integer.parseInt(parts[2]); + ItemMeta meta = item.getItemMeta(); + int current = LegacyModelData.has(meta) ? LegacyModelData.get(meta) : 0; + return current == model; + } + + /** The look as a scheme path, for the log and the previous-model tag. */ + static String describe(ItemStack item) { + String ia = iaId(item); + if (ia != null) { + return "ia." + ia; + } + ItemMeta meta = item.getItemMeta(); + int model = LegacyModelData.has(meta) ? LegacyModelData.get(meta) : 0; + return "v." + item.getType().name().toLowerCase(Locale.ROOT) + "." + model; + } + + /** TLibs stamps merged ItemsAdder looks as {@code ia: .}; returned as namespace:id. */ + private static String iaId(ItemStack item) { + NBTItem nbt = NBTItem.get(item); + if (!nbt.hasTag("ia")) { + return null; + } + String ia = nbt.getString("ia"); + int dot = ia.indexOf('.'); + return dot > 0 && dot < ia.length() - 1 ? ia.substring(0, dot) + ":" + ia.substring(dot + 1) : null; + } + + private static boolean hasItemsAdderCompound(ItemStack item) { + return NBTItem.get(item).hasTag("itemsadder"); + } +} diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelApplier.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelApplier.java new file mode 100644 index 0000000..98e6d44 --- /dev/null +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelApplier.java @@ -0,0 +1,39 @@ +package net.tfminecraft.advancedcrafting.utils; + +import java.util.Optional; + +import org.bukkit.Material; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; + +import net.tfminecraft.advancedcrafting.objects.crafting.CraftingRecipe; +import net.tfminecraft.advancedcrafting.objects.schemes.ModelScheme; +import net.tfminecraft.advancedcrafting.util.LegacyModelData; +import net.tfminecraft.tlibs.TLibs; + +/** Model scheme paths: {@code v..} for vanilla items, {@code ia.:} for ItemsAdder. */ +public final class ModelApplier { + private ModelApplier() { + } + + /** A model named after the recipe (e.g. heavy_helmet) wins over the shared type (helmet). */ + public static String modelFor(ModelScheme scheme, CraftingRecipe recipe) { + String path = scheme.getModel(recipe.getId()); + return path != null ? path : scheme.getModel(recipe.getType()); + } + + // This path mutates the existing ItemStack; replacing it would change aliases held by callers. + @SuppressWarnings("deprecation") + public static ItemStack apply(ItemStack i, String path) { + String type = path.split("\\.")[0]; + if(type.equalsIgnoreCase("v")) { + i.setType(Material.valueOf(path.split("\\.")[1].toUpperCase())); + ItemMeta m = i.getItemMeta(); + LegacyModelData.set(m, Integer.parseInt(path.split("\\.")[2])); + i.setItemMeta(m); + } else if(type.equalsIgnoreCase("ia")) { + i = TLibs.getItemAPI().getArmorMerger().merge(i, Optional.empty(), path); + } + return i; + } +} diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/PDCKeys.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/PDCKeys.java index 7ca4c77..672f9bb 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/PDCKeys.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/PDCKeys.java @@ -29,6 +29,11 @@ public static NamespacedKey craftModelScheme() { return new NamespacedKey(AdvancedCrafting.plugin, "ac_craft_model_scheme"); } + /** The old default look a crafted piece wore before the look migration replaced it. */ + public static NamespacedKey previousModel() { + return new NamespacedKey(AdvancedCrafting.plugin, "ac_previous_model"); + } + public static NamespacedKey scrapBase() { return new NamespacedKey(AdvancedCrafting.plugin, "ac_scrap_base"); } diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index 825057a..1fe01fb 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -51,6 +51,23 @@ show-ingredient-stats: true # Log stat template / ingredient revision refreshes to console ([AC][StatRefresh]). debug-stat-refresh: false +# Crafted pieces still wearing their old default look get their recipe's current model (e.g. light_helmet) +# when they are refreshed: on join, in opened chests, on click and on equip. The merge keeps all MMOItems +# data; skinned pieces keep their skin. Every recorded craft also gets its model scheme (metal) tag. +# The old look is the scheme's type model (helmet, ...), unless legacy-models lists the recipe: +# (), for looks that came from another ingredient (infantry paper, mage dust). +armour-look-migration: + enabled: false + legacy-models: + - infantry_helmet(v.carved_pumpkin.1) + - infantry_chestplate(v.golden_chestplate.0) + - infantry_leggings(v.golden_leggings.0) + - infantry_boots(v.golden_boots.0) + - mage_helmet(ia.tfmc_armor:iron_mage_helmet) + - mage_chestplate(ia.tfmc_armor:iron_mage_chestplate) + - mage_leggings(ia.tfmc_armor:iron_mage_leggings) + - mage_boots(ia.tfmc_armor:iron_mage_boots) + # Divide bucket-averaged stat values before template factors (movement_speed config uses ~100x larger numbers than MMOItems). global-stat-offsets: movement_speed: 100 diff --git a/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java b/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java new file mode 100644 index 0000000..b2f0234 --- /dev/null +++ b/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java @@ -0,0 +1,337 @@ +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.nio.file.Files; +import java.util.*; +import net.tfminecraft.advancedcrafting.cache.Cache; +import net.tfminecraft.advancedcrafting.loaders.*; +import net.tfminecraft.advancedcrafting.objects.crafting.CraftingRecipe; +import net.tfminecraft.advancedcrafting.objects.data.*; +import net.tfminecraft.advancedcrafting.objects.schemes.ModelScheme; +import net.tfminecraft.advancedcrafting.util.LegacyModelData; +import net.tfminecraft.advancedcrafting.utils.*; +import org.bukkit.Material; +import org.bukkit.NamespacedKey; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; +import org.bukkit.persistence.PersistentDataType; +import org.junit.jupiter.api.*; +import org.mockito.MockedStatic; + +class ArmourLookMigratorTest extends CoverageSupport { + /** Stands in for TLibs' {@code ia} NBT tag ("namespace.id"). */ + static final NamespacedKey IA = new NamespacedKey("test", "ia"); + /** Stands in for the ItemsAdder {@code itemsadder} compound. */ + static final NamespacedKey COMPOUND = new NamespacedKey("test", "itemsadder"); + /** Stands in for the custom model data component, which MockBukkit does not implement. */ + static final NamespacedKey MODEL = new NamespacedKey("test", "model"); + + MockedStatic nbts; + MockedStatic models; + CraftingRecipe light; + CraftingRecipe infantry; + + @BeforeEach + void world() throws Exception { + models = mockStatic(LegacyModelData.class); + models + .when(() -> LegacyModelData.set(any(ItemMeta.class), any())) + .thenAnswer( + call -> { + ItemMeta meta = call.getArgument(0); + Integer value = call.getArgument(1); + if (value == null) meta.getPersistentDataContainer().remove(MODEL); + else meta.getPersistentDataContainer().set(MODEL, PersistentDataType.INTEGER, value); + return null; + }); + models + .when(() -> LegacyModelData.has(any(ItemMeta.class))) + .thenAnswer(call -> ((ItemMeta) call.getArgument(0)).getPersistentDataContainer().has(MODEL)); + models + .when(() -> LegacyModelData.get(any(ItemMeta.class))) + .thenAnswer( + call -> + ((ItemMeta) call.getArgument(0)) + .getPersistentDataContainer() + .get(MODEL, PersistentDataType.INTEGER)); + nbts = mockStatic(NBTItem.class); + nbts.when(() -> NBTItem.get(any(ItemStack.class))) + .thenAnswer( + call -> { + ItemStack item = call.getArgument(0); + var pdc = item.getItemMeta().getPersistentDataContainer(); + String ia = pdc.get(IA, PersistentDataType.STRING); + var nbt = mock(NBTItem.class); + when(nbt.hasTag("ia")).thenReturn(ia != null); + when(nbt.getString("ia")).thenReturn(ia); + when(nbt.hasTag("itemsadder")).thenReturn(ia != null || pdc.has(COMPOUND)); + return nbt; + }); + // The merge swaps the look and stamps the ItemsAdder id, like ArmorMerger. + when(items.getArmorMerger().merge(any(ItemStack.class), any(), anyString())) + .thenAnswer( + call -> { + ItemStack item = call.getArgument(0); + String id = ((String) call.getArgument(2)).substring(3); + item.setType(Material.LEATHER_HELMET); + var meta = item.getItemMeta(); + meta.getPersistentDataContainer().set(IA, PersistentDataType.STRING, id.replace(':', '.')); + item.setItemMeta(meta); + return item; + }); + SchemeLoader.models.put( + "iron", + new ModelScheme( + "iron", + yaml( + "models: ['helmet(v.chainmail_helmet.0)', 'light_helmet(ia.tfmc_armor:iron_light_helmet)'," + + " 'infantry_helmet(ia.tfmc_armor:base_metal_iron_infantry_helmet)']"))); + SchemeLoader.models.put( + "bronze", + new ModelScheme( + "bronze", + yaml( + "models: ['helmet(ia.tfmc_armor:bronze_helmet)'," + + " 'light_helmet(ia.tfmc_armor:bronze_light_helmet)']"))); + SchemeLoader.models.put("plain", new ModelScheme("plain", yaml("models: []"))); + ingredient("iron", "model-scheme: iron"); + ingredient("bronze", "model-scheme: bronze"); + ingredient("plain", "model-scheme: plain"); + light = armour("light_helmet"); + infantry = armour("infantry_helmet"); + Cache.armourLookMigration = true; + Cache.legacyModels.clear(); + Cache.legacyModels.put("infantry_helmet", List.of("v.carved_pumpkin.1")); + } + + @AfterEach + void closeNbt() { + nbts.close(); + models.close(); + } + + CraftingRecipe armour(String id) throws Exception { + var recipe = + new CraftingRecipe( + id, + yaml( + "name: '%material% Helmet'\ncategory: weapons\ntemplate: HELMETS.TEST\ntype: helmet\n" + + "recipe: ['metal.4']")); + RecipeLoader.map.put(id, recipe); + return recipe; + } + + /** A piece crafted from {@code metal} that wears a vanilla look. */ + ItemStack crafted(String recipe, String metal, Material material, Integer model) { + var item = new ItemStack(material); + var meta = item.getItemMeta(); + LegacyModelData.set(meta, model); + item.setItemMeta(meta); + new CraftProvenance(recipe, "fine", List.of(new CraftInput("ingredient", metal, 4, 0)), 0) + .applyTo(item); + // Older crafts carry no scheme tag. + meta = item.getItemMeta(); + meta.getPersistentDataContainer().remove(PDCKeys.craftModelScheme()); + item.setItemMeta(meta); + return item; + } + + static ItemStack withIa(ItemStack item, String ia) { + var meta = item.getItemMeta(); + meta.getPersistentDataContainer().set(IA, PersistentDataType.STRING, ia); + item.setItemMeta(meta); + return item; + } + + static String tag(ItemStack item, NamespacedKey key) { + return item.getItemMeta().getPersistentDataContainer().get(key, PersistentDataType.STRING); + } + + static String ia(ItemStack item) { + return tag(item, IA); + } + + @Test + void oldDefaultLooksTakeTheRecipeModelAndKeepARecordOfTheOldOne() { + var old = crafted("light_helmet", "iron", Material.CHAINMAIL_HELMET, null); + var migrated = ArmourLookMigrator.migrate(old); + + assertNotSame(old, migrated); + assertEquals(Material.CHAINMAIL_HELMET, old.getType(), "the original stack is left alone"); + assertNull(tag(old, PDCKeys.craftModelScheme())); + assertEquals("tfmc_armor.iron_light_helmet", ia(migrated)); + assertEquals("v.chainmail_helmet.0", tag(migrated, PDCKeys.previousModel())); + assertEquals("iron", tag(migrated, PDCKeys.craftModelScheme())); + assertEquals("light_helmet", CraftProvenance.readFrom(migrated).getRecipeId()); + + // Migrated pieces are left alone from then on. + assertSame(migrated, ArmourLookMigrator.migrate(migrated)); + + // An ItemsAdder type model counts as the old look too. + var bronze = withIa(crafted("light_helmet", "bronze", Material.LEATHER_HELMET, 5), "tfmc_armor.bronze_helmet"); + var newBronze = ArmourLookMigrator.migrate(bronze); + assertEquals("tfmc_armor.bronze_light_helmet", ia(newBronze)); + assertEquals("ia.tfmc_armor:bronze_helmet", tag(newBronze, PDCKeys.previousModel())); + assertEquals("bronze", tag(newBronze, PDCKeys.craftModelScheme())); + } + + @Test + void skinsAndOtherLooksStayButStillGetTheMetalTag() { + var skinned = + withIa( + crafted("light_helmet", "iron", Material.LEATHER_HELMET, 10194), + "tfmc_submissions.geofflive_thalendorian_armor_iron_helmet"); + var tagged = ArmourLookMigrator.migrate(skinned); + assertNotSame(skinned, tagged); + assertEquals("tfmc_submissions.geofflive_thalendorian_armor_iron_helmet", ia(tagged)); + assertEquals(Material.LEATHER_HELMET, tagged.getType()); + assertNull(tag(tagged, PDCKeys.previousModel())); + assertEquals("iron", tag(tagged, PDCKeys.craftModelScheme())); + assertSame(tagged, ArmourLookMigrator.migrate(tagged)); + verify(items.getArmorMerger(), never()).merge(any(ItemStack.class), any(), anyString()); + + // A vanilla skin of another model, a vanilla material carrying an ItemsAdder compound, and a + // stale ia tag on the old material all stay as they are. + var otherModel = ArmourLookMigrator.migrate(crafted("light_helmet", "iron", Material.CHAINMAIL_HELMET, 3)); + assertEquals(Material.CHAINMAIL_HELMET, otherModel.getType()); + var compoundItem = crafted("light_helmet", "iron", Material.CHAINMAIL_HELMET, null); + var meta = compoundItem.getItemMeta(); + meta.getPersistentDataContainer().set(COMPOUND, PersistentDataType.STRING, "x"); + compoundItem.setItemMeta(meta); + assertEquals(Material.CHAINMAIL_HELMET, ArmourLookMigrator.migrate(compoundItem).getType()); + var stale = withIa(crafted("light_helmet", "iron", Material.CHAINMAIL_HELMET, null), "old.skin"); + assertEquals("old.skin", ia(ArmourLookMigrator.migrate(stale))); + var otherMaterial = ArmourLookMigrator.migrate(crafted("light_helmet", "iron", Material.IRON_HELMET, null)); + assertEquals(Material.IRON_HELMET, otherMaterial.getType()); + } + + @Test + void configuredLegacyModelsReplaceTheMetalTypeModel() { + var paperLook = crafted("infantry_helmet", "iron", Material.CARVED_PUMPKIN, 1); + var migrated = ArmourLookMigrator.migrate(paperLook); + assertEquals("tfmc_armor.base_metal_iron_infantry_helmet", ia(migrated)); + assertEquals("v.carved_pumpkin.1", tag(migrated, PDCKeys.previousModel())); + + // An infantry piece in plain chainmail was skinned that way; the metal's type model does not apply. + var chainmail = ArmourLookMigrator.migrate(crafted("infantry_helmet", "iron", Material.CHAINMAIL_HELMET, null)); + assertEquals(Material.CHAINMAIL_HELMET, chainmail.getType()); + assertEquals("iron", tag(chainmail, PDCKeys.craftModelScheme())); + } + + @Test + void piecesWithoutARecipeModelOrARecordAreOnlyTaggedOrSkipped() throws Exception { + // A scheme with no model for the recipe (weapons, other materials) only gets the tag. + var plain = crafted("light_helmet", "plain", Material.CHAINMAIL_HELMET, null); + var tagged = ArmourLookMigrator.migrate(plain); + assertEquals(Material.CHAINMAIL_HELMET, tagged.getType()); + assertEquals("plain", tag(tagged, PDCKeys.craftModelScheme())); + // The plain scheme has no type model either. + armour("plain_cap"); + assertFalse( + invokeLegacy(crafted("plain_cap", "plain", Material.CHAINMAIL_HELMET, null), "plain_cap", "plain")); + + // Unknown recipes and inputs resolve no scheme; items without a record are not crafted pieces. + var unknownRecipe = crafted("gone", "iron", Material.CHAINMAIL_HELMET, null); + assertSame(unknownRecipe, ArmourLookMigrator.migrate(unknownRecipe)); + var unknownMetal = crafted("light_helmet", "gone", Material.CHAINMAIL_HELMET, null); + assertSame(unknownMetal, ArmourLookMigrator.migrate(unknownMetal)); + var premade = new ItemStack(Material.CHAINMAIL_HELMET); + assertSame(premade, ArmourLookMigrator.migrate(premade)); + assertNull(ArmourLookMigrator.migrate(null)); + var air = new ItemStack(Material.AIR); + assertSame(air, ArmourLookMigrator.migrate(air)); + var noMeta = mock(ItemStack.class); + when(noMeta.getType()).thenReturn(Material.CHAINMAIL_HELMET); + assertSame(noMeta, ArmourLookMigrator.migrate(noMeta)); + + // Switched off, nothing changes. + Cache.armourLookMigration = false; + var old = crafted("light_helmet", "iron", Material.CHAINMAIL_HELMET, null); + assertSame(old, ArmourLookMigrator.migrate(old)); + } + + boolean invokeLegacy(ItemStack item, String recipe, String scheme) throws Exception { + var m = + ArmourLookMigrator.class.getDeclaredMethod( + "wearsLegacyLook", ItemStack.class, CraftingRecipe.class, ModelScheme.class); + m.setAccessible(true); + return (boolean) m.invoke(null, item, RecipeLoader.map.get(recipe), SchemeLoader.models.get(scheme)); + } + + @Test + void looksAreMatchedAndDescribedByTheirSchemePath() throws Exception { + var wears = + ArmourLookMigrator.class.getDeclaredMethod("wears", ItemStack.class, String.class); + wears.setAccessible(true); + var describe = ArmourLookMigrator.class.getDeclaredMethod("describe", ItemStack.class); + describe.setAccessible(true); + var plain = new ItemStack(Material.CHAINMAIL_HELMET); + + assertTrue((boolean) wears.invoke(null, plain, "v.chainmail_helmet.0")); + assertFalse((boolean) wears.invoke(null, plain, "v.chainmail_helmet")); + assertFalse((boolean) wears.invoke(null, plain, "m.helmets.light_custom_helmet")); + assertFalse((boolean) wears.invoke(null, plain, "ia.tfmc_armor:iron_light_helmet")); + assertEquals("v.chainmail_helmet.0", describe.invoke(null, plain)); + + var modelled = new ItemStack(Material.CARVED_PUMPKIN); + var meta = modelled.getItemMeta(); + LegacyModelData.set(meta, 1); + modelled.setItemMeta(meta); + assertEquals("v.carved_pumpkin.1", describe.invoke(null, modelled)); + + var skinned = withIa(new ItemStack(Material.LEATHER_HELMET), "tfmc_armor.iron_light_helmet"); + assertTrue((boolean) wears.invoke(null, skinned, "ia.TFMC_ARMOR:iron_light_helmet")); + assertEquals("ia.tfmc_armor:iron_light_helmet", describe.invoke(null, skinned)); + + // Malformed ia tags read as no ItemsAdder id. + for (String broken : List.of("nodot", ".id", "ns.")) { + var item = withIa(new ItemStack(Material.LEATHER_HELMET), broken); + assertEquals("v.leather_helmet.0", describe.invoke(null, item)); + } + } + + @Test + void modelPathsApplyVanillaAndItemsAdderLooks() throws Exception { + var scheme = SchemeLoader.models.get("iron"); + assertEquals("ia.tfmc_armor:iron_light_helmet", ModelApplier.modelFor(scheme, light)); + var heavy = armour("heavy_helmet"); + assertEquals("v.chainmail_helmet.0", ModelApplier.modelFor(scheme, heavy)); + + var vanilla = ModelApplier.apply(new ItemStack(Material.LEATHER_HELMET), "v.iron_helmet.4"); + assertEquals(Material.IRON_HELMET, vanilla.getType()); + assertEquals(4, LegacyModelData.get(vanilla.getItemMeta())); + var ia = ModelApplier.apply(new ItemStack(Material.CHAINMAIL_HELMET), "ia.tfmc_armor:iron_light_helmet"); + assertEquals("tfmc_armor.iron_light_helmet", ia(ia)); + var unknown = new ItemStack(Material.CHAINMAIL_HELMET); + assertSame(unknown, ModelApplier.apply(unknown, "x.something")); + } + + @Test + void theConfigListsLegacyModelsPerRecipe() throws Exception { + var file = temp.resolve("migration.yml"); + Files.writeString( + file, + "armour-look-migration:\n enabled: true\n legacy-models:\n" + + " - Infantry_Helmet(v.carved_pumpkin.1)\n - infantry_helmet( v.golden_helmet.0 )\n" + + " - broken\n - (v.stone.0)\n - open(v.stone.0\n"); + Cache.armourLookMigration = false; + new ConfigLoader().load(file.toFile()); + assertTrue(Cache.armourLookMigration); + assertEquals(Map.of("infantry_helmet", List.of("v.carved_pumpkin.1", "v.golden_helmet.0")), Cache.legacyModels); + + Files.writeString(file, "scrap-path: x\n"); + new ConfigLoader().load(file.toFile()); + assertFalse(Cache.armourLookMigration); + assertTrue(Cache.legacyModels.isEmpty()); + } + + @Test + void theItemRefreshAlsoMigratesLooks() throws Exception { + // Not outdated: only the look migration runs. + var old = crafted("light_helmet", "iron", Material.CHAINMAIL_HELMET, null); + assertEquals("tfmc_armor.iron_light_helmet", ia(AcItemRefresher.refreshIfOutdated(old))); + } +} From 4246169b3e60e2010ac3edc0639654ece90aaa92 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:52:18 +0200 Subject: [PATCH 4/9] Keep wear, skin, name and foreign data when a stat refresh rebuilds an item CraftStatRefresher rebuilt outdated crafted items from their MMOItems data only. On TFMCDev01 that repaired them to full durability, dropped ArmourShop skin names, the itemsadder compound of skins, and other plugins' data such as geminfusion:socket_rarities and tlibs:socket_overrides. RefreshKeeper keeps the damage taken (scaled to the new maximum, never breaking an item), then puts back the skin through TLibs' ItemSkinPreserver, the material, model and leather colour, the display name, and every persistent data key the rebuilt item does not set itself. Co-Authored-By: Claude Opus 5.5 --- pom.xml | 7 + .../utils/CraftStatRefresher.java | 6 +- .../advancedcrafting/utils/RefreshKeeper.java | 65 +++++++ .../advancedcrafting/MmoCoverageTest.java | 8 + .../advancedcrafting/RefreshKeeperTest.java | 163 ++++++++++++++++++ 5 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 src/main/java/net/tfminecraft/advancedcrafting/utils/RefreshKeeper.java create mode 100644 src/test/java/net/tfminecraft/advancedcrafting/RefreshKeeperTest.java diff --git a/pom.xml b/pom.xml index e6296fc..c86c783 100644 --- a/pom.xml +++ b/pom.xml @@ -17,6 +17,13 @@ + + + local + item-nbt-api-plugin + 2.16.1-tfmc-c571502e7686 + test + beer.devs diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/CraftStatRefresher.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/CraftStatRefresher.java index 7074870..cbf502a 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/CraftStatRefresher.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/CraftStatRefresher.java @@ -46,10 +46,12 @@ public static RefreshResult refresh(ItemStack item, boolean force) { StatData newStats = CraftStatCalculator.compute(recipe, provenance.getInputs()); StatRefreshDebug.logBefore(item, provenance, recipe, newStats); - LiveMMOItem mmo = new LiveMMOItem(NBTItem.get(item)); + NBTItem nbt = NBTItem.get(item); + LiveMMOItem mmo = new LiveMMOItem(nbt); MMOStatApplicator.applyExternalLayer(mmo, newStats, CraftStatCalculator.collectManagedStatIds(recipe), true); + RefreshKeeper.keepWear(nbt, mmo); - ItemStack rebuilt = mmo.newBuilder().build(); + ItemStack rebuilt = RefreshKeeper.keepAppearance(item, mmo.newBuilder().build()); rebuilt.setAmount(item.getAmount()); provenance.syncRevisions(); provenance.applyTo(rebuilt); diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/RefreshKeeper.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/RefreshKeeper.java new file mode 100644 index 0000000..b8687f3 --- /dev/null +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/RefreshKeeper.java @@ -0,0 +1,65 @@ +package net.tfminecraft.advancedcrafting.utils; + +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; +import org.bukkit.inventory.meta.LeatherArmorMeta; + +import io.lumine.mythic.lib.api.item.NBTItem; +import net.Indyuce.mmoitems.ItemStats; +import net.Indyuce.mmoitems.api.item.mmoitem.MMOItem; +import net.Indyuce.mmoitems.stat.data.DoubleData; +import net.tfminecraft.advancedcrafting.util.LegacyModelData; +import net.tfminecraft.tlibs.objects.api.subapi.ItemSkinPreserver; + +/** + * A stat refresh rebuilds the item from its MMOItems data, which only covers stats. This puts back what the + * player's item had besides those: its wear, skin or model, display name (ArmourShop names) and other + * plugins' data such as gem socket rarities. + */ +public final class RefreshKeeper { + private static final String DURABILITY = "MMOITEMS_DURABILITY"; + private static final String MAX_DURABILITY = "MMOITEMS_MAX_DURABILITY"; + + private RefreshKeeper() { + } + + /** + * Before the rebuild: keep the damage the item has taken, so a refresh neither repairs nor breaks it. + * MMOItems builds a full item unless its current durability is set. + */ + public static void keepWear(NBTItem old, MMOItem mmo) { + if (!old.hasTag(DURABILITY) || !old.hasTag(MAX_DURABILITY) || !mmo.hasData(ItemStats.MAX_DURABILITY)) { + return; + } + int current = old.getInteger(DURABILITY); + int taken = old.getInteger(MAX_DURABILITY) - current; + int max = (int) ((DoubleData) mmo.getData(ItemStats.MAX_DURABILITY)).getValue(); + int kept = Math.min(max, Math.max(current > 0 ? 1 : 0, max - taken)); + mmo.setData(ItemStats.CUSTOM_DURABILITY, new DoubleData(kept)); + } + + /** After the rebuild: the old look, name and foreign data on the rebuilt item. */ + public static ItemStack keepAppearance(ItemStack old, ItemStack rebuilt) { + ItemStack kept = ItemSkinPreserver.apply(old, rebuilt); + ItemMeta oldMeta = old.getItemMeta(); + if (kept.getType() != old.getType()) { + kept.setType(old.getType()); + } + ItemMeta meta = kept.getItemMeta(); + if (oldMeta == null || meta == null) { + return kept; + } + if (LegacyModelData.has(oldMeta)) { + LegacyModelData.set(meta, LegacyModelData.get(oldMeta)); + } + if (oldMeta instanceof LeatherArmorMeta oldLeather && meta instanceof LeatherArmorMeta leather) { + leather.setColor(oldLeather.getColor()); + } + if (oldMeta.hasDisplayName()) { + meta.displayName(oldMeta.displayName()); + } + oldMeta.getPersistentDataContainer().copyTo(meta.getPersistentDataContainer(), false); + kept.setItemMeta(meta); + return kept; + } +} diff --git a/src/test/java/net/tfminecraft/advancedcrafting/MmoCoverageTest.java b/src/test/java/net/tfminecraft/advancedcrafting/MmoCoverageTest.java index 4d1da98..151bb45 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/MmoCoverageTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/MmoCoverageTest.java @@ -71,12 +71,20 @@ void craftedRefreshPreservesAmountSynchronizesProvenanceAndAppliesTier() throws var rebuilt = new ItemStack(Material.IRON_SWORD); var nbt = mock(NBTItem.class); 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 mmos = mockConstruction( LiveMMOItem.class, withSettings().defaultAnswer(RETURNS_DEEP_STUBS), (mock, ctx) -> when(mock.newBuilder().build()).thenReturn(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 = CraftStatRefresher.refreshIfOutdated(item); assertTrue(result.isChanged()); assertNull(result.getError()); diff --git a/src/test/java/net/tfminecraft/advancedcrafting/RefreshKeeperTest.java b/src/test/java/net/tfminecraft/advancedcrafting/RefreshKeeperTest.java new file mode 100644 index 0000000..cc701ff --- /dev/null +++ b/src/test/java/net/tfminecraft/advancedcrafting/RefreshKeeperTest.java @@ -0,0 +1,163 @@ +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 net.Indyuce.mmoitems.ItemStats; +import net.Indyuce.mmoitems.api.item.mmoitem.MMOItem; +import net.Indyuce.mmoitems.stat.data.DoubleData; +import net.kyori.adventure.text.Component; +import net.tfminecraft.advancedcrafting.util.LegacyModelData; +import net.tfminecraft.advancedcrafting.utils.RefreshKeeper; +import net.tfminecraft.tlibs.objects.api.subapi.ItemSkinPreserver; +import org.bukkit.Color; +import org.bukkit.Material; +import org.bukkit.NamespacedKey; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; +import org.bukkit.inventory.meta.LeatherArmorMeta; +import org.bukkit.persistence.PersistentDataType; +import org.junit.jupiter.api.*; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; + +class RefreshKeeperTest extends CoverageSupport { + static final NamespacedKey MODEL = new NamespacedKey("test", "model"); + static final NamespacedKey GEMS = new NamespacedKey("geminfusion", "socket_rarities"); + + MockedStatic skins; + MockedStatic models; + + @BeforeEach + void mocks() { + skins = mockStatic(ItemSkinPreserver.class); + skins + .when(() -> ItemSkinPreserver.apply(any(ItemStack.class), any(ItemStack.class))) + .thenAnswer(call -> call.getArgument(1)); + // MockBukkit has no custom model data component; a PDC entry stands in for it. + models = mockStatic(LegacyModelData.class); + models + .when(() -> LegacyModelData.set(any(ItemMeta.class), any())) + .thenAnswer( + call -> { + ((ItemMeta) call.getArgument(0)) + .getPersistentDataContainer() + .set(MODEL, PersistentDataType.INTEGER, call.getArgument(1)); + return null; + }); + models + .when(() -> LegacyModelData.has(any(ItemMeta.class))) + .thenAnswer(call -> ((ItemMeta) call.getArgument(0)).getPersistentDataContainer().has(MODEL)); + models + .when(() -> LegacyModelData.get(any(ItemMeta.class))) + .thenAnswer( + call -> + ((ItemMeta) call.getArgument(0)) + .getPersistentDataContainer() + .get(MODEL, PersistentDataType.INTEGER)); + } + + @AfterEach + void close() { + skins.close(); + models.close(); + } + + static NBTItem wear(Integer current, Integer max) { + var nbt = mock(NBTItem.class); + when(nbt.hasTag("MMOITEMS_DURABILITY")).thenReturn(current != null); + when(nbt.hasTag("MMOITEMS_MAX_DURABILITY")).thenReturn(max != null); + when(nbt.getInteger("MMOITEMS_DURABILITY")).thenReturn(current == null ? 0 : current); + when(nbt.getInteger("MMOITEMS_MAX_DURABILITY")).thenReturn(max == null ? 0 : max); + return nbt; + } + + static MMOItem rebuilt(Double max) { + var mmo = mock(MMOItem.class); + when(mmo.hasData(ItemStats.MAX_DURABILITY)).thenReturn(max != null); + if (max != null) when(mmo.getData(ItemStats.MAX_DURABILITY)).thenReturn(new DoubleData(max)); + return mmo; + } + + static Double kept(Integer current, Integer oldMax, Double newMax) { + var mmo = rebuilt(newMax); + RefreshKeeper.keepWear(wear(current, oldMax), mmo); + var captor = ArgumentCaptor.forClass(DoubleData.class); + verify(mmo, atMost(1)).setData(eq(ItemStats.CUSTOM_DURABILITY), captor.capture()); + return captor.getAllValues().isEmpty() ? null : captor.getValue().getValue(); + } + + @Test + void theDamageTakenCarriesOverToTheNewMaximum() { + assertEquals(461.0, kept(478, 500, 483.0)); + assertEquals(483.0, kept(500, 500, 483.0), "an undamaged item stays undamaged"); + assertEquals(520.0, kept(500, 500, 520.0)); + assertEquals(1.0, kept(5, 500, 400.0), "a refresh never breaks an item"); + assertEquals(0.0, kept(0, 500, 400.0), "a broken item stays broken"); + assertNull(kept(null, 500, 483.0)); + assertNull(kept(478, null, 483.0)); + assertNull(kept(478, 500, null)); + } + + @Test + void theLookNameAndForeignDataComeBackOnTheRebuiltItem() { + var old = new ItemStack(Material.LEATHER_HELMET); + var meta = (LeatherArmorMeta) old.getItemMeta(); + meta.setColor(Color.fromRGB(0xFFFFF3)); + meta.displayName(Component.text("Thalendorian Armor Iron Helmet")); + meta.getPersistentDataContainer().set(GEMS, PersistentDataType.STRING, "{\"a\":\"common\"}"); + meta.getPersistentDataContainer().set(PlainKeys.QUALITY, PersistentDataType.STRING, "old"); + LegacyModelData.set(meta, 10194); + old.setItemMeta(meta); + + var fresh = new ItemStack(Material.CHAINMAIL_HELMET); + var freshMeta = fresh.getItemMeta(); + freshMeta.getPersistentDataContainer().set(PlainKeys.QUALITY, PersistentDataType.STRING, "new"); + fresh.setItemMeta(freshMeta); + + var result = RefreshKeeper.keepAppearance(old, fresh); + assertEquals(Material.LEATHER_HELMET, result.getType()); + var kept = (LeatherArmorMeta) result.getItemMeta(); + assertEquals(Color.fromRGB(0xFFFFF3), kept.getColor()); + assertEquals(Component.text("Thalendorian Armor Iron Helmet"), kept.displayName()); + assertEquals(10194, LegacyModelData.get(kept)); + assertEquals("{\"a\":\"common\"}", kept.getPersistentDataContainer().get(GEMS, PersistentDataType.STRING)); + assertEquals( + "new", + kept.getPersistentDataContainer().get(PlainKeys.QUALITY, PersistentDataType.STRING), + "the rebuilt item's own data wins"); + + // A plain item without a model or name keeps what the rebuild made. + var plain = new ItemStack(Material.IRON_SWORD); + var plainResult = RefreshKeeper.keepAppearance(plain, new ItemStack(Material.IRON_SWORD)); + assertFalse(LegacyModelData.has(plainResult.getItemMeta())); + assertFalse(plainResult.getItemMeta().hasDisplayName()); + } + + @Test + void itemsWithoutMetaOrWithMismatchedMetaAreLeftToTheRebuild() { + var noMeta = mock(ItemStack.class); + when(noMeta.getType()).thenReturn(Material.IRON_SWORD); + var fresh = new ItemStack(Material.IRON_SWORD); + assertSame(fresh, RefreshKeeper.keepAppearance(noMeta, fresh)); + + var old = new ItemStack(Material.LEATHER_HELMET); + var rebuiltNoMeta = mock(ItemStack.class); + when(rebuiltNoMeta.getType()).thenReturn(Material.LEATHER_HELMET); + assertSame(rebuiltNoMeta, RefreshKeeper.keepAppearance(old, rebuiltNoMeta)); + + // An old leather look whose rebuilt meta is not leather keeps the rebuilt colour handling. + var plainMeta = new ItemStack(Material.IRON_HELMET).getItemMeta(); + var odd = mock(ItemStack.class); + when(odd.getType()).thenReturn(Material.LEATHER_HELMET); + when(odd.getItemMeta()).thenReturn(plainMeta); + assertSame(odd, RefreshKeeper.keepAppearance(old, odd)); + verify(odd).setItemMeta(plainMeta); + } + + /** A key the rebuilt item sets itself. */ + static final class PlainKeys { + static final NamespacedKey QUALITY = new NamespacedKey("advancedcrafting", "ac_craft_quality"); + } +} From 83a9684dd90405d5a8e246ec3af30136949f0705 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Thu, 8 Oct 2026 21:10:47 +0200 Subject: [PATCH 5/9] Opt crafted pieces with an ItemsAdder look out of ItemsAdder auto_update ItemsAdder's auto_update rebuilds an item from its definition when the definition's base properties hash no longer matches the one it stored for that player slot. The rebuild keeps only amount, durability, persistent data, trim, name and enchantments, so a crafted MMOItems piece wearing an ItemsAdder look loses its stats, gems and lore (the 2026-10-03 incident, and reproduced on TFMCDev01 by editing a definition or a player's inventory offline). ItemsAdder skips items whose itemsadder compound has override_auto_update: true. The look migration sets that flag on every recorded craft with an ItemsAdder look (migrated and skinned pieces), and the stat refresh sets it on rebuilt pieces. NBTAPI becomes a provided dependency and a softdepend. Co-Authored-By: Claude Opus 5.5 --- pom.xml | 18 ++++- .../utils/ArmourLookMigrator.java | 9 ++- .../advancedcrafting/utils/IaAutoUpdate.java | 38 +++++++++++ .../advancedcrafting/utils/RefreshKeeper.java | 3 + src/main/resources/plugin.yml | 1 + .../ArmourLookMigratorTest.java | 33 ++++++++++ .../advancedcrafting/IaAutoUpdateTest.java | 66 +++++++++++++++++++ .../advancedcrafting/MmoCoverageTest.java | 1 + .../advancedcrafting/RefreshKeeperTest.java | 9 +++ 9 files changed, 175 insertions(+), 3 deletions(-) create mode 100644 src/main/java/net/tfminecraft/advancedcrafting/utils/IaAutoUpdate.java create mode 100644 src/test/java/net/tfminecraft/advancedcrafting/IaAutoUpdateTest.java diff --git a/pom.xml b/pom.xml index c86c783..b4a469b 100644 --- a/pom.xml +++ b/pom.xml @@ -17,12 +17,22 @@ - + + + com.mojang + authlib + 7.0.61 + test + + ** + + + local item-nbt-api-plugin 2.16.1-tfmc-c571502e7686 - test + provided @@ -106,6 +116,10 @@ + + minecraft-libraries + https://libraries.minecraft.net/ + papermc https://repo.papermc.io/repository/maven-public/ diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java index 3515335..b4aa57b 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java @@ -21,7 +21,8 @@ * Brings crafted pieces made before the per-recipe models up to date. A piece that still wears the old default * look of its recipe (its metal's type model, or a configured legacy model) gets the recipe's current model * through the same merge skins use, so its MMOItems data stays. Any other look is a skin and is kept. Every - * recorded craft also gets its model scheme tag, which ArmourShop's metal skin lines read. + * recorded craft also gets its model scheme tag, which ArmourShop's metal skin lines read, and pieces with an + * ItemsAdder look are opted out of ItemsAdder's auto_update (see {@link IaAutoUpdate}). */ public final class ArmourLookMigrator { private ArmourLookMigrator() { @@ -61,6 +62,12 @@ public static ItemStack migrate(ItemStack item) { meta.getPersistentDataContainer().set(PDCKeys.craftModelScheme(), PersistentDataType.STRING, scheme.getId()); result.setItemMeta(meta); } + if (IaAutoUpdate.isExposed(result)) { + if (result == item) { + result = item.clone(); + } + IaAutoUpdate.protect(result); + } return result; } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/IaAutoUpdate.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/IaAutoUpdate.java new file mode 100644 index 0000000..c9591c0 --- /dev/null +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/IaAutoUpdate.java @@ -0,0 +1,38 @@ +package net.tfminecraft.advancedcrafting.utils; + +import org.bukkit.inventory.ItemStack; + +import de.tr7zw.nbtapi.NBT; +import de.tr7zw.nbtapi.iface.ReadableNBT; + +/** + * ItemsAdder's auto_update rebuilds an item from its definition when that definition changes (or a player's + * slot hashes no longer match), keeping only amount, durability, persistent data, trim, name and enchantments. + * On a crafted MMOItems piece wearing an ItemsAdder look that wipes its stats, gems and lore (the Forj + * incident). ItemsAdder skips items whose {@code itemsadder} compound has {@code override_auto_update: true}. + */ +public final class IaAutoUpdate { + private static final String COMPOUND = "itemsadder"; + private static final String OVERRIDE = "override_auto_update"; + + private IaAutoUpdate() { + } + + /** Whether the item wears an ItemsAdder look that ItemsAdder may still rebuild. */ + public static boolean isExposed(ItemStack item) { + return NBT.get(item, nbt -> { + if (!nbt.hasTag(COMPOUND)) { + return false; + } + ReadableNBT compound = nbt.getCompound(COMPOUND); + return compound != null && !Boolean.TRUE.equals(compound.getBoolean(OVERRIDE)); + }); + } + + /** Opts the item out of ItemsAdder's auto_update. */ + public static void protect(ItemStack item) { + NBT.modify(item, nbt -> { + nbt.getOrCreateCompound(COMPOUND).setBoolean(OVERRIDE, true); + }); + } +} diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/RefreshKeeper.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/RefreshKeeper.java index b8687f3..50cdcda 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/RefreshKeeper.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/RefreshKeeper.java @@ -60,6 +60,9 @@ public static ItemStack keepAppearance(ItemStack old, ItemStack rebuilt) { } oldMeta.getPersistentDataContainer().copyTo(meta.getPersistentDataContainer(), false); kept.setItemMeta(meta); + if (IaAutoUpdate.isExposed(kept)) { + IaAutoUpdate.protect(kept); + } return kept; } } diff --git a/src/main/resources/plugin.yml b/src/main/resources/plugin.yml index 8bcfe79..0f7cefa 100644 --- a/src/main/resources/plugin.yml +++ b/src/main/resources/plugin.yml @@ -5,6 +5,7 @@ version: ${project.version} author: Drefvelin depend: [TLibs, MMOCore, MMOItems, MythicLib] +softdepend: [NBTAPI] commands: ac: diff --git a/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java b/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java index b2f0234..3956cf7 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java @@ -28,14 +28,35 @@ class ArmourLookMigratorTest extends CoverageSupport { static final NamespacedKey COMPOUND = new NamespacedKey("test", "itemsadder"); /** Stands in for the custom model data component, which MockBukkit does not implement. */ static final NamespacedKey MODEL = new NamespacedKey("test", "model"); + /** Stands in for itemsadder.override_auto_update. */ + static final NamespacedKey PROTECTED = new NamespacedKey("test", "protected"); MockedStatic nbts; MockedStatic models; + MockedStatic autoUpdate; CraftingRecipe light; CraftingRecipe infantry; @BeforeEach void world() throws Exception { + autoUpdate = mockStatic(IaAutoUpdate.class); + autoUpdate + .when(() -> IaAutoUpdate.isExposed(any(ItemStack.class))) + .thenAnswer( + call -> { + var pdc = ((ItemStack) call.getArgument(0)).getItemMeta().getPersistentDataContainer(); + return (pdc.has(IA) || pdc.has(COMPOUND)) && !pdc.has(PROTECTED); + }); + autoUpdate + .when(() -> IaAutoUpdate.protect(any(ItemStack.class))) + .thenAnswer( + call -> { + ItemStack item = call.getArgument(0); + var meta = item.getItemMeta(); + meta.getPersistentDataContainer().set(PROTECTED, PersistentDataType.BYTE, (byte) 1); + item.setItemMeta(meta); + return null; + }); models = mockStatic(LegacyModelData.class); models .when(() -> LegacyModelData.set(any(ItemMeta.class), any())) @@ -111,6 +132,7 @@ void world() throws Exception { void closeNbt() { nbts.close(); models.close(); + autoUpdate.close(); } CraftingRecipe armour(String id) throws Exception { @@ -166,6 +188,7 @@ void oldDefaultLooksTakeTheRecipeModelAndKeepARecordOfTheOldOne() { assertEquals("v.chainmail_helmet.0", tag(migrated, PDCKeys.previousModel())); assertEquals("iron", tag(migrated, PDCKeys.craftModelScheme())); assertEquals("light_helmet", CraftProvenance.readFrom(migrated).getRecipeId()); + assertTrue(migrated.getItemMeta().getPersistentDataContainer().has(PROTECTED)); // Migrated pieces are left alone from then on. assertSame(migrated, ArmourLookMigrator.migrate(migrated)); @@ -190,13 +213,23 @@ void skinsAndOtherLooksStayButStillGetTheMetalTag() { assertEquals(Material.LEATHER_HELMET, tagged.getType()); assertNull(tag(tagged, PDCKeys.previousModel())); assertEquals("iron", tag(tagged, PDCKeys.craftModelScheme())); + assertTrue(tagged.getItemMeta().getPersistentDataContainer().has(PROTECTED)); assertSame(tagged, ArmourLookMigrator.migrate(tagged)); + // A tagged skin that ItemsAdder could still rebuild is protected on its own. + var exposed = tagged.clone(); + var exposedMeta = exposed.getItemMeta(); + exposedMeta.getPersistentDataContainer().remove(PROTECTED); + exposed.setItemMeta(exposedMeta); + var protectedCopy = ArmourLookMigrator.migrate(exposed); + assertNotSame(exposed, protectedCopy); + assertTrue(protectedCopy.getItemMeta().getPersistentDataContainer().has(PROTECTED)); verify(items.getArmorMerger(), never()).merge(any(ItemStack.class), any(), anyString()); // A vanilla skin of another model, a vanilla material carrying an ItemsAdder compound, and a // stale ia tag on the old material all stay as they are. var otherModel = ArmourLookMigrator.migrate(crafted("light_helmet", "iron", Material.CHAINMAIL_HELMET, 3)); assertEquals(Material.CHAINMAIL_HELMET, otherModel.getType()); + assertFalse(otherModel.getItemMeta().getPersistentDataContainer().has(PROTECTED)); var compoundItem = crafted("light_helmet", "iron", Material.CHAINMAIL_HELMET, null); var meta = compoundItem.getItemMeta(); meta.getPersistentDataContainer().set(COMPOUND, PersistentDataType.STRING, "x"); diff --git a/src/test/java/net/tfminecraft/advancedcrafting/IaAutoUpdateTest.java b/src/test/java/net/tfminecraft/advancedcrafting/IaAutoUpdateTest.java new file mode 100644 index 0000000..7063281 --- /dev/null +++ b/src/test/java/net/tfminecraft/advancedcrafting/IaAutoUpdateTest.java @@ -0,0 +1,66 @@ +package net.tfminecraft.advancedcrafting; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import de.tr7zw.nbtapi.NBT; +import de.tr7zw.nbtapi.iface.ReadWriteItemNBT; +import de.tr7zw.nbtapi.iface.ReadWriteNBT; +import de.tr7zw.nbtapi.iface.ReadableItemNBT; +import de.tr7zw.nbtapi.iface.ReadableNBT; +import java.util.function.Consumer; +import java.util.function.Function; +import net.tfminecraft.advancedcrafting.utils.IaAutoUpdate; +import org.bukkit.inventory.ItemStack; +import org.junit.jupiter.api.Test; + +class IaAutoUpdateTest extends CoverageSupport { + static boolean exposed(ReadableItemNBT nbt) { + var item = mock(ItemStack.class); + try (var nbts = mockStatic(NBT.class)) { + nbts.when(() -> NBT.get(eq(item), any(Function.class))) + .thenAnswer(call -> ((Function) call.getArgument(1)).apply(nbt)); + return IaAutoUpdate.isExposed(item); + } + } + + static ReadableItemNBT withCompound(ReadableNBT compound) { + var nbt = mock(ReadableItemNBT.class); + when(nbt.hasTag("itemsadder")).thenReturn(true); + when(nbt.getCompound("itemsadder")).thenReturn(compound); + return nbt; + } + + static ReadableNBT flag(Boolean value) { + var compound = mock(ReadableNBT.class); + when(compound.getBoolean("override_auto_update")).thenReturn(value); + return compound; + } + + @Test + void onlyItemsAdderLooksWithoutTheOptOutAreExposed() { + assertFalse(exposed(mock(ReadableItemNBT.class)), "no ItemsAdder look"); + assertFalse(exposed(withCompound(null))); + assertTrue(exposed(withCompound(flag(null)))); + assertTrue(exposed(withCompound(flag(false)))); + assertFalse(exposed(withCompound(flag(true))), "already opted out"); + } + + @Test + void protectingSetsTheOptOut() { + var item = mock(ItemStack.class); + var nbt = mock(ReadWriteItemNBT.class); + var compound = mock(ReadWriteNBT.class); + when(nbt.getOrCreateCompound("itemsadder")).thenReturn(compound); + try (var nbts = mockStatic(NBT.class)) { + nbts.when(() -> NBT.modify(eq(item), any(Consumer.class))) + .thenAnswer( + call -> { + ((Consumer) call.getArgument(1)).accept(nbt); + return null; + }); + IaAutoUpdate.protect(item); + } + verify(compound).setBoolean("override_auto_update", true); + } +} diff --git a/src/test/java/net/tfminecraft/advancedcrafting/MmoCoverageTest.java b/src/test/java/net/tfminecraft/advancedcrafting/MmoCoverageTest.java index 151bb45..08b9a0c 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/MmoCoverageTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/MmoCoverageTest.java @@ -73,6 +73,7 @@ void craftedRefreshPreservesAmountSynchronizesProvenanceAndAppliesTier() throws 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 = mockConstruction( LiveMMOItem.class, diff --git a/src/test/java/net/tfminecraft/advancedcrafting/RefreshKeeperTest.java b/src/test/java/net/tfminecraft/advancedcrafting/RefreshKeeperTest.java index cc701ff..2834877 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/RefreshKeeperTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/RefreshKeeperTest.java @@ -9,6 +9,7 @@ import net.Indyuce.mmoitems.stat.data.DoubleData; import net.kyori.adventure.text.Component; import net.tfminecraft.advancedcrafting.util.LegacyModelData; +import net.tfminecraft.advancedcrafting.utils.IaAutoUpdate; import net.tfminecraft.advancedcrafting.utils.RefreshKeeper; import net.tfminecraft.tlibs.objects.api.subapi.ItemSkinPreserver; import org.bukkit.Color; @@ -28,9 +29,14 @@ class RefreshKeeperTest extends CoverageSupport { MockedStatic skins; MockedStatic models; + MockedStatic autoUpdate; @BeforeEach void mocks() { + autoUpdate = mockStatic(IaAutoUpdate.class); + autoUpdate + .when(() -> IaAutoUpdate.isExposed(any(ItemStack.class))) + .thenAnswer(call -> ((ItemStack) call.getArgument(0)).getType() == Material.LEATHER_HELMET); skins = mockStatic(ItemSkinPreserver.class); skins .when(() -> ItemSkinPreserver.apply(any(ItemStack.class), any(ItemStack.class))) @@ -62,6 +68,7 @@ void mocks() { void close() { skins.close(); models.close(); + autoUpdate.close(); } static NBTItem wear(Integer current, Integer max) { @@ -127,12 +134,14 @@ void theLookNameAndForeignDataComeBackOnTheRebuiltItem() { "new", kept.getPersistentDataContainer().get(PlainKeys.QUALITY, PersistentDataType.STRING), "the rebuilt item's own data wins"); + autoUpdate.verify(() -> IaAutoUpdate.protect(result)); // A plain item without a model or name keeps what the rebuild made. var plain = new ItemStack(Material.IRON_SWORD); var plainResult = RefreshKeeper.keepAppearance(plain, new ItemStack(Material.IRON_SWORD)); assertFalse(LegacyModelData.has(plainResult.getItemMeta())); assertFalse(plainResult.getItemMeta().hasDisplayName()); + autoUpdate.verify(() -> IaAutoUpdate.protect(plainResult), never()); } @Test From 571d50d6a1d70e9213970d45fa665933491fdf82 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 10 Oct 2026 08:24:53 +0200 Subject: [PATCH 6/9] Keep a join sweep going past a broken item and ignore malformed model paths A recorded craft whose MMOItems data is gone threw in the stat refresh and stopped the whole join sweep on TFMCDev01, so the rest of that inventory was not refreshed or migrated. CraftRefreshListener now logs the item and carries on. A v.. path with a non-numeric model no longer throws in the look migration; it simply does not match. Co-Authored-By: Claude Opus 5.5 --- .../managers/CraftRefreshListener.java | 9 ++++++- .../utils/ArmourLookMigrator.java | 7 ++++- .../ArmourLookMigratorTest.java | 1 + .../RefreshListenerCoverageTest.java | 26 +++++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/CraftRefreshListener.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/CraftRefreshListener.java index 3f7cc1f..fa8c2a1 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/managers/CraftRefreshListener.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/CraftRefreshListener.java @@ -126,7 +126,14 @@ private void tryRefresh(ItemStack item, ItemConsumer writer) { if (item == null || item.getType().isAir() || !AcItemRefresher.isManaged(item)) { return; } - ItemStack refreshed = AcItemRefresher.refreshIfOutdated(item); + ItemStack refreshed; + try { + refreshed = AcItemRefresher.refreshIfOutdated(item); + } catch (RuntimeException e) { + // One broken item (e.g. a recorded craft whose MMOItems data is gone) must not stop the rest of a sweep. + AdvancedCrafting.plugin.getLogger().warning("Could not refresh " + item.getType() + ": " + e); + return; + } if (refreshed == item) { return; } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java index b4aa57b..0b85229 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java @@ -97,7 +97,12 @@ static boolean wears(ItemStack item, String path) { || !item.getType().name().equalsIgnoreCase(parts[1])) { return false; } - int model = Integer.parseInt(parts[2]); + int model; + try { + model = Integer.parseInt(parts[2]); + } catch (NumberFormatException e) { + return false; + } ItemMeta meta = item.getItemMeta(); int current = LegacyModelData.has(meta) ? LegacyModelData.get(meta) : 0; return current == model; diff --git a/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java b/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java index 3956cf7..e51c295 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/ArmourLookMigratorTest.java @@ -305,6 +305,7 @@ void looksAreMatchedAndDescribedByTheirSchemePath() throws Exception { assertTrue((boolean) wears.invoke(null, plain, "v.chainmail_helmet.0")); assertFalse((boolean) wears.invoke(null, plain, "v.chainmail_helmet")); + assertFalse((boolean) wears.invoke(null, plain, "v.chainmail_helmet.x"), "a malformed model never matches"); assertFalse((boolean) wears.invoke(null, plain, "m.helmets.light_custom_helmet")); assertFalse((boolean) wears.invoke(null, plain, "ia.tfmc_armor:iron_light_helmet")); assertEquals("v.chainmail_helmet.0", describe.invoke(null, plain)); diff --git a/src/test/java/net/tfminecraft/advancedcrafting/RefreshListenerCoverageTest.java b/src/test/java/net/tfminecraft/advancedcrafting/RefreshListenerCoverageTest.java index 03446fd..69a5072 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/RefreshListenerCoverageTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/RefreshListenerCoverageTest.java @@ -158,6 +158,32 @@ void joinSweepRefreshesInventoryArmourOffhandAndEnderChestOneTickLater() throws verify(gone, never()).getInventory(); } + @Test + void aBrokenItemIsLoggedAndTheSweepCarriesOn() { + when(plugin.isEnabled()).thenReturn(true); + var p = server.addPlayer(); + var broken = new ItemStack(Material.IRON_HELMET); + var next = new ItemStack(Material.IRON_BOOTS); + var refreshed = new ItemStack(Material.LEATHER_BOOTS); + p.getInventory().setItem(0, broken); + p.getInventory().setItem(1, next); + try (var refresher = mockStatic(AcItemRefresher.class)) { + refresher.when(() -> AcItemRefresher.isManaged(any())).thenReturn(true); + refresher + .when(() -> AcItemRefresher.refreshIfOutdated(argThat(i -> i != null && i.getType() == Material.IRON_HELMET))) + .thenThrow(new NullPointerException("no MMOItems type")); + refresher + .when(() -> AcItemRefresher.refreshIfOutdated(argThat(i -> i != null && i.getType() == Material.IRON_BOOTS))) + .thenReturn(refreshed); + var join = mock(PlayerJoinEvent.class); + when(join.getPlayer()).thenReturn(p); + new CraftRefreshListener().onJoin(join); + server.getScheduler().performOneTick(); + } + assertEquals(Material.IRON_HELMET, p.getInventory().getItem(0).getType()); + assertEquals(Material.LEATHER_BOOTS, p.getInventory().getItem(1).getType()); + } + @Test void openSweepOnlyTouchesWorldStorageAfterTheEvent() throws Exception { when(plugin.isEnabled()).thenReturn(true); From 1c1045e9b73874a8ec34990b5b5efca51bf98aff Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:42:28 +0200 Subject: [PATCH 7/9] Address review: model-type materials need a model, NBTAPI stays optional - forModel only lets a model-type material take over when its scheme has a model for the recipe. A material without a scheme, or one whose unknown scheme fell back to the empty default, no longer replaces a valid scheme and leaves the crafted item without a look. - IaAutoUpdate treats a missing NBTAPI (a soft dependency) as "nothing to opt out" instead of throwing a LinkageError out of every stat refresh. Co-Authored-By: Claude Opus 5.5 --- .../advancedcrafting/utils/IaAutoUpdate.java | 29 ++++++++++++------- .../utils/ModelSchemeResolver.java | 11 +++++-- .../advancedcrafting/IaAutoUpdateTest.java | 11 +++++++ .../ModelSchemeResolverTest.java | 10 +++++++ 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/IaAutoUpdate.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/IaAutoUpdate.java index c9591c0..8df924a 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/IaAutoUpdate.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/IaAutoUpdate.java @@ -20,19 +20,28 @@ private IaAutoUpdate() { /** Whether the item wears an ItemsAdder look that ItemsAdder may still rebuild. */ public static boolean isExposed(ItemStack item) { - return NBT.get(item, nbt -> { - if (!nbt.hasTag(COMPOUND)) { - return false; - } - ReadableNBT compound = nbt.getCompound(COMPOUND); - return compound != null && !Boolean.TRUE.equals(compound.getBoolean(OVERRIDE)); - }); + try { + return NBT.get(item, nbt -> { + if (!nbt.hasTag(COMPOUND)) { + return false; + } + ReadableNBT compound = nbt.getCompound(COMPOUND); + return compound != null && !Boolean.TRUE.equals(compound.getBoolean(OVERRIDE)); + }); + } catch (LinkageError missingNbtApi) { + // NBTAPI is a soft dependency; without it there is no opt-out to set. + return false; + } } /** Opts the item out of ItemsAdder's auto_update. */ public static void protect(ItemStack item) { - NBT.modify(item, nbt -> { - nbt.getOrCreateCompound(COMPOUND).setBoolean(OVERRIDE, true); - }); + try { + NBT.modify(item, nbt -> { + nbt.getOrCreateCompound(COMPOUND).setBoolean(OVERRIDE, true); + }); + } catch (LinkageError missingNbtApi) { + // NBTAPI is a soft dependency; without it the item simply stays as it is. + } } } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java b/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java index 501db9f..f60a225 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/utils/ModelSchemeResolver.java @@ -37,14 +37,21 @@ public static ModelScheme resolve(CraftingRecipe recipe, List inputs return forModel(recipe, main, materials); } - /** A recipe-specific model on the main material wins over the secondary model ingredient. */ + /** + * A recipe-specific model on the main material wins over the secondary model ingredient. A model-type + * material only takes over when its scheme has a model for the recipe, so one without a scheme (or with an + * empty one) never leaves the item without a look. + */ public static ModelScheme forModel(CraftingRecipe recipe, ModelScheme scheme, Map materials) { if (scheme == null || scheme.getModel(recipe.getId()) != null || recipe.getModelType().equalsIgnoreCase("none")) { return scheme; } for (String key : materials.keySet()) { - scheme = schemeOf(key, recipe.getModelType(), scheme); + ModelScheme candidate = schemeOf(key, recipe.getModelType(), null); + if (candidate != null && ModelApplier.modelFor(candidate, recipe) != null) { + scheme = candidate; + } } return scheme; } diff --git a/src/test/java/net/tfminecraft/advancedcrafting/IaAutoUpdateTest.java b/src/test/java/net/tfminecraft/advancedcrafting/IaAutoUpdateTest.java index 7063281..29ed21c 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/IaAutoUpdateTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/IaAutoUpdateTest.java @@ -63,4 +63,15 @@ void protectingSetsTheOptOut() { } verify(compound).setBoolean("override_auto_update", true); } + + @Test + void withoutNbtApiNothingIsExposedOrChanged() { + var item = mock(ItemStack.class); + try (var nbts = mockStatic(NBT.class)) { + nbts.when(() -> NBT.get(eq(item), any(Function.class))).thenThrow(new NoClassDefFoundError("NBT")); + nbts.when(() -> NBT.modify(eq(item), any(Consumer.class))).thenThrow(new NoClassDefFoundError("NBT")); + assertFalse(IaAutoUpdate.isExposed(item)); + assertDoesNotThrow(() -> IaAutoUpdate.protect(item)); + } + } } diff --git a/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java b/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java index c65aee8..0f6051e 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/ModelSchemeResolverTest.java @@ -67,6 +67,16 @@ void recordedCraftsResolveTheSchemeTheirLookCameFromAndAlloysUseTheirBase() thro in("ingredient", "", 1), in("misc", "x", 1))); assertNull(scheme(noModel, in("ingredient", "gone", 1))); + // A model-type material whose scheme has no model for the recipe (an unknown scheme falls back to an + // empty default) or no scheme at all keeps the main scheme. + SchemeLoader.models.put("default", new ModelScheme("default", yaml("models: []"))); + var blank = ingredient("blank", "type: paper\nmodel-scheme: missing"); + alloy("blankalloy", blank); + assertEquals("iron", scheme(byPaper, in("ingredient", "iron", 3), in("ingredient", "blank", 1))); + assertEquals("iron", scheme(byPaper, in("ingredient", "iron", 3), in("alloy", "blankalloy", 1))); + SchemeLoader.models.remove("default"); + ingredient("schemeless", "type: paper\nmodel-scheme: missing"); + assertEquals("iron", scheme(byPaper, in("ingredient", "iron", 3), in("ingredient", "schemeless", 1))); // Malformed inputs are skipped. assertEquals( "iron", From e3ef22dd6a58e6275d37791bb7062562d6bd8cfd Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:56:06 +0200 Subject: [PATCH 8/9] CI: install NBTAPI for the build IaAutoUpdate compiles against NBTAPI (provided), so the build now downloads the pinned NBTAPI-2.16.1.jar from ServerAssets, checks its hash and installs it as local:item-nbt-api-plugin, as ArmourShop does. Co-Authored-By: Claude Opus 5.5 --- .github/dependencies.sha256 | 1 + .github/scripts/install-local-dependencies.sh | 1 + .github/scripts/prepare-release.sh | 1 + 3 files changed, 3 insertions(+) diff --git a/.github/dependencies.sha256 b/.github/dependencies.sha256 index fd19572..f42aed5 100644 --- a/.github/dependencies.sha256 +++ b/.github/dependencies.sha256 @@ -3,3 +3,4 @@ a37f7789fcdcd11c9e5890a2fac742aa4501749588af23182991953e06ee1d17 libs/MMOItems-6.10.1-SNAPSHOT.jar 225aa7f75d4e6e6ae6b3ad87a7ca678c025c027f00374ab1062275d24f3a73c9 libs/MythicLib-1.7.1-SNAPSHOT.jar 5a01b37bd7442fb0f2c809cadac82ba70f75173956e04efac3336595cc070368 libs/ItemsAdder-4.0.18.jar +c571502e768640c7cff10c49b3eef904ccf3fd95bd81803913abfe43cfc09746 libs/NBTAPI-2.16.1.jar diff --git a/.github/scripts/install-local-dependencies.sh b/.github/scripts/install-local-dependencies.sh index fbb1b1c..710f329 100644 --- a/.github/scripts/install-local-dependencies.sh +++ b/.github/scripts/install-local-dependencies.sh @@ -19,3 +19,4 @@ mvn -B --no-transfer-progress org.apache.maven.plugins:maven-install-plugin:3.1. mvn -B --no-transfer-progress org.apache.maven.plugins:maven-install-plugin:3.1.4:install-file \ -Dfile="libs/ItemsAdder-4.0.18.jar" -DgroupId="local" -DartifactId="LoneLibs" \ -Dversion="4.0.18-tfmc-5a01b37bd744" -Dpackaging=jar -DgeneratePom=true "$@" +mvn -B --no-transfer-progress org.apache.maven.plugins:maven-install-plugin:3.1.4:install-file \n -Dfile="libs/NBTAPI-2.16.1.jar" -DgroupId="local" -DartifactId="item-nbt-api-plugin" \n -Dversion="2.16.1-tfmc-c571502e7686" -Dpackaging=jar -DgeneratePom=true "$@" diff --git a/.github/scripts/prepare-release.sh b/.github/scripts/prepare-release.sh index 5250a35..a3a7416 100755 --- a/.github/scripts/prepare-release.sh +++ b/.github/scripts/prepare-release.sh @@ -8,4 +8,5 @@ curl --fail --location --silent --show-error --retry 3 -H "Authorization: Bearer curl --fail --location --silent --show-error --retry 3 -H "Authorization: Bearer $GH_TOKEN" -H "Accept: application/vnd.github.raw+json" "https://api.github.com/repos/TF-Minecraft/ServerAssets/contents/jars/a37f7789fcdc/MMOItems-6.10.1-SNAPSHOT.jar?ref=$ref" > "libs/MMOItems-6.10.1-SNAPSHOT.jar" curl --fail --location --silent --show-error --retry 3 -H "Authorization: Bearer $GH_TOKEN" -H "Accept: application/vnd.github.raw+json" "https://api.github.com/repos/TF-Minecraft/ServerAssets/contents/jars/225aa7f75d4e/MythicLib-1.7.1-SNAPSHOT.jar?ref=$ref" > "libs/MythicLib-1.7.1-SNAPSHOT.jar" curl --fail --location --silent --show-error --retry 3 -H "Authorization: Bearer $GH_TOKEN" -H "Accept: application/vnd.github.raw+json" "https://api.github.com/repos/TF-Minecraft/ServerAssets/contents/jars/5a01b37bd744/ItemsAdder-4.0.18.jar?ref=$ref" > "libs/ItemsAdder-4.0.18.jar" +curl --fail --location --silent --show-error --retry 3 -H "Authorization: Bearer $GH_TOKEN" -H "Accept: application/vnd.github.raw+json" "https://api.github.com/repos/TF-Minecraft/ServerAssets/contents/jars/c571502e7686/NBTAPI-2.16.1.jar?ref=$ref" > "libs/NBTAPI-2.16.1.jar" bash .github/scripts/install-local-dependencies.sh "$@" From 4a08c0195f1eaecdb7ebe298afd0d746835f6a51 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:58:06 +0200 Subject: [PATCH 9/9] CI: fix the NBTAPI install command's line breaks Co-Authored-By: Claude Opus 5.5 --- .github/scripts/install-local-dependencies.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/scripts/install-local-dependencies.sh b/.github/scripts/install-local-dependencies.sh index 710f329..cfbca14 100644 --- a/.github/scripts/install-local-dependencies.sh +++ b/.github/scripts/install-local-dependencies.sh @@ -19,4 +19,6 @@ mvn -B --no-transfer-progress org.apache.maven.plugins:maven-install-plugin:3.1. mvn -B --no-transfer-progress org.apache.maven.plugins:maven-install-plugin:3.1.4:install-file \ -Dfile="libs/ItemsAdder-4.0.18.jar" -DgroupId="local" -DartifactId="LoneLibs" \ -Dversion="4.0.18-tfmc-5a01b37bd744" -Dpackaging=jar -DgeneratePom=true "$@" -mvn -B --no-transfer-progress org.apache.maven.plugins:maven-install-plugin:3.1.4:install-file \n -Dfile="libs/NBTAPI-2.16.1.jar" -DgroupId="local" -DartifactId="item-nbt-api-plugin" \n -Dversion="2.16.1-tfmc-c571502e7686" -Dpackaging=jar -DgeneratePom=true "$@" +mvn -B --no-transfer-progress org.apache.maven.plugins:maven-install-plugin:3.1.4:install-file \ + -Dfile="libs/NBTAPI-2.16.1.jar" -DgroupId="local" -DartifactId="item-nbt-api-plugin" \ + -Dversion="2.16.1-tfmc-c571502e7686" -Dpackaging=jar -DgeneratePom=true "$@"