Move old armour to the new looks and keep item data through refreshes

pom.xml

@@ -17,6 +17,23 @@
     </properties>
 
     <dependencies>
+        <!-- NBT-API's NBT class references authlib; mocking it in tests needs the type. -->
+        <dependency>
+            <groupId>com.mojang</groupId>
+            <artifactId>authlib</artifactId>
+            <version>7.0.61</version>
+            <scope>test</scope>
+            <exclusions>
+                <exclusion><groupId>*</groupId><artifactId>*</artifactId></exclusion>
+            </exclusions>
+        </dependency>
+        <!-- NBTAPI plugin: the ItemsAdder auto-update opt-out; TLibs' ItemSkinPreserver uses it too. -->
+        <dependency>
+            <groupId>local</groupId>
+            <artifactId>item-nbt-api-plugin</artifactId>
+            <version>2.16.1-tfmc-c571502e7686</version>
+            <scope>provided</scope>
+        </dependency>
         <!-- ItemsAdder loads FastNBT dynamically on a server; mocks need its API types. -->
         <dependency>
             <groupId>beer.devs</groupId>
@@ -99,6 +116,10 @@
     </dependencies>
 
     <repositories>
+        <repository>
+            <id>minecraft-libraries</id>
+            <url>https://libraries.minecraft.net/</url>
+        </repository>
         <repository>
             <id>papermc</id>
             <url>https://repo.papermc.io/repository/maven-public/</url>

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<String, List<String>> legacyModels = new HashMap<>();
+
 	public static boolean showIngredientStats = true;
 
 	/** Divisors applied to bucket-averaged stats before template factors (e.g. movement_speed: 100). */

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 <recipe>(<model>)");
+				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();

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;
 		}

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) {

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);
 	}
 }

src/main/java/net/tfminecraft/advancedcrafting/utils/ArmourLookMigrator.java

@@ -0,0 +1,136 @@
+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, and pieces with an
+ * ItemsAdder look are opted out of ItemsAdder's auto_update (see {@link IaAutoUpdate}).
+ */
+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);
+		}
+		if (IaAutoUpdate.isExposed(result)) {
+			if (result == item) {
+				result = item.clone();
+			}
+			IaAutoUpdate.protect(result);
+		}
+		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<String> 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;
+		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;
+	}
+
+	/** 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: <namespace>.<id>}; 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");
+	}
+}

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);

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);
+		});
+	}
+}

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.<material>.<model>} for vanilla items, {@code ia.<namespace>:<id>} 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;
+	}
+}