From f9ad319ad5005ca11b55990e268705d893049df0 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Tue, 3 Nov 2026 16:19:47 +0100 Subject: [PATCH 1/6] Let players see the recipes of alloys they have forged /alloy recipes lists the alloys this account has forged, each name clickable. /alloy recipe , or /alloy recipe with an alloy in hand, shows its base and catalysts. Hits, stats and forge odds stay hidden. An alloy the player hasn't forged gets "You don't know the recipe" whether or not it exists, and tab completion offers only their own alloys. A new alloy whose naming prompt timed out was never recorded for its discoverer, so they could not see its recipe. The timeout now records it. Co-Authored-By: Claude Opus 5.5 --- .../lifecycle/CraftLifecycle.java | 10 + .../lifecycle/PlayerAlloyForgeTracker.java | 18 ++ .../managers/AlloyManager.java | 10 + .../managers/AlloyRecipeCommand.java | 159 +++++++++++ .../managers/CommandManager.java | 24 +- src/main/resources/plugin.yml | 4 +- .../AlloyRecipeCommandTest.java | 262 ++++++++++++++++++ .../advancedcrafting/CommandCoverageTest.java | 4 +- 8 files changed, 484 insertions(+), 7 deletions(-) create mode 100644 src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java create mode 100644 src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java diff --git a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java index be0ed25..575dcab 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java @@ -40,6 +40,16 @@ public static void fireAlloyOutcome(Player player, String alloyId) { } } + /** + * Records a forge without firing events, for a player who has gone offline + * (a new alloy whose naming prompt timed out after they left). + */ + public static void recordForge(UUID playerUuid, String alloyId) { + if (tracker != null) { + tracker.recordForge(playerUuid, alloyId); + } + } + public static void fireItemCrafted(Player player, String recipeId, String categoryId) { if (player == null || recipeId == null || recipeId.isBlank()) { return; diff --git a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java index 71b370e..9883d19 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java @@ -7,8 +7,10 @@ import java.io.PrintWriter; import java.util.HashSet; import java.util.ArrayList; +import java.util.Collections; import java.util.Locale; import java.util.Set; +import java.util.TreeSet; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -46,6 +48,22 @@ public boolean recordForge(UUID playerUuid, String alloyId) { return true; } + /** True once this account has forged the alloy (a real alloy, not scrap). */ + public boolean hasForged(UUID playerUuid, String alloyId) { + if (playerUuid == null || alloyId == null || alloyId.isBlank()) { + return false; + } + return loadForgedIds(playerUuid).contains(alloyId.toLowerCase(Locale.ROOT)); + } + + /** The alloy ids this account has forged, sorted; a read-only copy. */ + public Set getForgedIds(UUID playerUuid) { + if (playerUuid == null) { + return Set.of(); + } + return Collections.unmodifiableSet(new TreeSet<>(loadForgedIds(playerUuid))); + } + private Set loadForgedIds(UUID playerUuid) { Set cached = CACHE.get(playerUuid); if (cached != null) { diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java index 52eba4d..2114960 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java @@ -101,12 +101,22 @@ public void run() { if(entry.getValue().tick()) { entry.getKey().sendMessage("§cNaming timed out."); naming.remove(entry.getKey()); + namingTimedOut(entry.getKey(), entry.getValue().getAlloy().getId()); } } } }.runTaskTimer(AdvancedCrafting.plugin, 0, 20L); } + /** The alloy keeps its generated name, but its discoverer has still forged it. */ + static void namingTimedOut(Player p, String alloyId) { + if (p.isOnline()) { + CraftLifecycle.fireAlloyOutcome(p, alloyId); + } else { + CraftLifecycle.recordForge(p.getUniqueId(), alloyId); + } + } + // Keep the existing legacy text representation, formatting, and exact-string comparisons. @SuppressWarnings("deprecation") @EventHandler diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java new file mode 100644 index 0000000..3d84dd2 --- /dev/null +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java @@ -0,0 +1,159 @@ +package net.tfminecraft.advancedcrafting.managers; + +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.Set; + +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; + +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.event.ClickEvent; +import net.kyori.adventure.text.event.HoverEvent; +import net.kyori.adventure.text.format.NamedTextColor; +import net.kyori.adventure.text.serializer.legacy.LegacyComponentSerializer; +import net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle; +import net.tfminecraft.advancedcrafting.lifecycle.PlayerAlloyForgeTracker; +import net.tfminecraft.advancedcrafting.loaders.IngredientLoader; +import net.tfminecraft.advancedcrafting.objects.alloys.Alloy; +import net.tfminecraft.advancedcrafting.objects.data.AlloyRecipe; +import net.tfminecraft.advancedcrafting.objects.ingredients.Ingredient; +import net.tfminecraft.advancedcrafting.utils.AcItemTags; + +/** + * {@code /alloy recipes} and {@code /alloy recipe [name]}: the base and catalysts of the alloys a + * player has forged. Only the ingredients are shown, never hits, stats or forge odds. An alloy the + * player has not forged gets the same reply whether or not it exists, so names can't be probed. + */ +final class AlloyRecipeCommand { + private static final LegacyComponentSerializer LEGACY = LegacyComponentSerializer.legacySection(); + + /** {@code /alloy recipes}: every alloy this account has forged, each name clickable. */ + void list(Player p) { + List known = knownAlloys(p); + if (known.isEmpty()) { + p.sendMessage("§7You haven't forged any alloys yet. Forge one to keep its recipe here."); + return; + } + p.sendMessage("§6Alloys you have forged §7(" + known.size() + ")§6, click one for its recipe:"); + Component line = Component.empty(); + for (int i = 0; i < known.size(); i++) { + Alloy alloy = known.get(i); + if (i > 0) { + line = line.append(Component.text(", ", NamedTextColor.GRAY)); + } + line = line.append(LEGACY.deserialize(alloy.getName()) + .clickEvent(ClickEvent.runCommand("/alloy recipe " + alloy.getId())) + .hoverEvent(HoverEvent.showText(Component.text("Show the recipe", NamedTextColor.GRAY)))); + } + p.sendMessage(line); + } + + /** {@code /alloy recipe [name]}: the named alloy, or the alloy in the main hand. */ + void show(Player p, String[] args) { + if (args.length < 2) { + showHeld(p); + return; + } + String typed = String.join(" ", java.util.Arrays.copyOfRange(args, 1, args.length)); + String id = typed.replace(' ', '_').toLowerCase(Locale.ROOT); + Alloy alloy = AlloyManager.getAlloyById(id); + if (alloy == null || !knows(p, alloy.getId())) { + p.sendMessage("§cYou don't know the recipe for §f" + typed + "§c."); + return; + } + sendRecipe(p, alloy); + } + + private void showHeld(Player p) { + ItemStack held = p.getInventory().getItemInMainHand(); + if (AcItemTags.getKind(held) != AcItemTags.Kind.ALLOY) { + p.sendMessage("§cHold an alloy, or use §f/alloy recipe §c. §f/alloy recipes §clists the ones you know."); + return; + } + Alloy alloy = AlloyManager.getAlloyById(AcItemTags.getId(held)); + if (alloy == null || !knows(p, alloy.getId())) { + String name = held.getItemMeta().hasDisplayName() ? held.getItemMeta().getDisplayName() : "this alloy"; + p.sendMessage("§cYou don't know the recipe for §f" + name + "§c. Forge it yourself to learn it."); + return; + } + sendRecipe(p, alloy); + } + + private void sendRecipe(Player p, Alloy alloy) { + AlloyRecipe recipe = alloy.getData().getRecipe(); + if (recipe == null) { + p.sendMessage("§cThe recipe for §f" + alloy.getName() + " §cwas never written down."); + return; + } + p.sendMessage("§6Recipe for §r" + alloy.getName()); + p.sendMessage("§7Base: §r" + ingredientName(recipe.getBaseId())); + List catalysts = new ArrayList<>(); + for (String id : recipe.getCatalystIds()) { + catalysts.add(ingredientName(id)); + } + p.sendMessage("§7Catalysts: §r" + (catalysts.isEmpty() ? "§fnone" : String.join("§7, §r", catalysts))); + p.sendMessage("§8Add the base to an alloy forge first, then the catalysts in any order."); + } + + /** Tab completion for {@code /alloy recipe}: only alloys this player has forged. */ + List completions(Player p) { + List ids = new ArrayList<>(); + for (Alloy alloy : knownAlloys(p)) { + ids.add(alloy.getId()); + } + return ids; + } + + private List knownAlloys(Player p) { + PlayerAlloyForgeTracker tracker = CraftLifecycle.getTracker(); + if (tracker == null) { + return List.of(); + } + Set ids = tracker.getForgedIds(p.getUniqueId()); + List known = new ArrayList<>(); + for (String id : ids) { + Alloy alloy = AlloyManager.getAlloyById(id); + if (alloy != null) { + known.add(alloy); + } + } + return known; + } + + private boolean knows(Player p, String alloyId) { + PlayerAlloyForgeTracker tracker = CraftLifecycle.getTracker(); + return tracker != null && tracker.hasForged(p.getUniqueId(), alloyId); + } + + /** The ingredient's item name with its colour, or its id in title case if the item can't be built. */ + static String ingredientName(String id) { + Ingredient ingredient = IngredientLoader.getByString(id); + if (ingredient != null) { + try { + ItemStack item = ingredient.build(); + if (item != null && item.hasItemMeta() && item.getItemMeta().hasDisplayName()) { + return item.getItemMeta().getDisplayName(); + } + } catch (RuntimeException ex) { + // The item source (MMOItems, ItemsAdder) may be missing; fall back to the id. + } + } + return "§f" + titleCase(id); + } + + static String titleCase(String id) { + StringBuilder out = new StringBuilder(); + for (String word : id.split("_")) { + if (word.isEmpty()) { + continue; + } + if (out.length() > 0) { + out.append(' '); + } + out.append(Character.toUpperCase(word.charAt(0))).append(word.substring(1)); + } + return out.toString(); + } +} diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java index c29433f..f58a1f4 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java @@ -30,6 +30,7 @@ public class CommandManager implements Listener, CommandExecutor, TabCompleter { private final EquipmentGiveCommand equipmentGive = new EquipmentGiveCommand(); + private final AlloyRecipeCommand alloyRecipes = new AlloyRecipeCommand(); public String cmd1 = "ac"; public String cmd2 = "alloy"; @@ -47,6 +48,18 @@ public boolean onCommand(CommandSender sender, Command cmd, String label, String AdvancedCrafting.getAlloyManager().nameAlloy(p, args[1]); return true; } + if (cmd.getName().equalsIgnoreCase(cmd2) && args.length >= 1 && args[0].equalsIgnoreCase("recipes")) { + alloyRecipes.list(p); + return true; + } + if (cmd.getName().equalsIgnoreCase(cmd2) && args.length >= 1 && args[0].equalsIgnoreCase("recipe")) { + alloyRecipes.show(p, args); + return true; + } + } else if (cmd.getName().equalsIgnoreCase(cmd2) && args.length >= 1 + && (args[0].equalsIgnoreCase("recipe") || args[0].equalsIgnoreCase("recipes"))) { + sender.sendMessage("§cPlayers only."); + return true; } return false; } @@ -165,7 +178,7 @@ public List onTabComplete(CommandSender sender, Command cmd, String labe return onAcTabComplete(sender, args); } if (cmd.getName().equalsIgnoreCase(cmd2)) { - return onAlloyTabComplete(args); + return onAlloyTabComplete(sender, args); } return List.of(); } @@ -229,12 +242,15 @@ private List onAcTabComplete(CommandSender sender, String[] args) { return List.of(); } - private List onAlloyTabComplete(String[] args) { + private List onAlloyTabComplete(CommandSender sender, String[] args) { if (args.length == 0) { - return filterPrefix("", "name"); + return filterPrefix("", "name", "recipe", "recipes"); } if (args.length == 1) { - return filterPrefix(args[0], "name"); + return filterPrefix(args[0], "name", "recipe", "recipes"); + } + if (args.length == 2 && args[0].equalsIgnoreCase("recipe") && sender instanceof Player p) { + return filterPrefix(args[1], alloyRecipes.completions(p)); } return List.of(); } diff --git a/src/main/resources/plugin.yml b/src/main/resources/plugin.yml index 0f7cefa..e8ab1df 100644 --- a/src/main/resources/plugin.yml +++ b/src/main/resources/plugin.yml @@ -12,8 +12,8 @@ commands: usage: / description: AdvancedCrafting Command alloy: - usage: / - description: Alloy Command + usage: "/ name | recipes | recipe [name]" + description: Name a new alloy, or see the recipes of alloys you have forged permissions: advancedcrafting.admin: diff --git a/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java b/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java new file mode 100644 index 0000000..95c6394 --- /dev/null +++ b/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java @@ -0,0 +1,262 @@ +package net.tfminecraft.advancedcrafting; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.util.List; +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.event.ClickEvent; +import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; +import net.tfminecraft.advancedcrafting.lifecycle.AlloyDiscoveredEvent; +import net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle; +import net.tfminecraft.advancedcrafting.lifecycle.PlayerAlloyForgeTracker; +import net.tfminecraft.advancedcrafting.managers.AlloyManager; +import net.tfminecraft.advancedcrafting.managers.CommandManager; +import net.tfminecraft.advancedcrafting.objects.alloys.Alloy; +import net.tfminecraft.advancedcrafting.objects.data.AlloyData; +import net.tfminecraft.advancedcrafting.objects.data.AlloyRecipe; +import net.tfminecraft.advancedcrafting.utils.PDCKeys; +import org.bukkit.Material; +import org.bukkit.command.Command; +import org.bukkit.command.CommandSender; +import org.bukkit.inventory.ItemStack; +import org.bukkit.persistence.PersistentDataType; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockbukkit.mockbukkit.entity.PlayerMock; + +class AlloyRecipeCommandTest extends CoverageSupport { + CommandManager manager; + PlayerAlloyForgeTracker tracker; + + @BeforeEach + void setupTracker() { + manager = new CommandManager(); + tracker = new PlayerAlloyForgeTracker(temp.toFile()); + CraftLifecycle.init(tracker); + } + + @AfterEach + void clearTracker() { + CraftLifecycle.init(null); + } + + Command cmd() { + var c = mock(Command.class); + when(c.getName()).thenReturn("alloy"); + return c; + } + + boolean alloy(CommandSender sender, String... args) { + return manager.onCommand(sender, cmd(), "alloy", args); + } + + Alloy addAlloy(String id, String name, AlloyRecipe recipe) { + var alloy = mock(Alloy.class); + var data = mock(AlloyData.class); + when(alloy.getId()).thenReturn(id); + when(alloy.getName()).thenReturn(name); + when(alloy.getData()).thenReturn(data); + when(data.getRecipe()).thenReturn(recipe); + AlloyManager.addAlloy(alloy); + return alloy; + } + + ItemStack alloyItem(String id, String displayName) { + var item = new ItemStack(Material.IRON_INGOT); + var meta = item.getItemMeta(); + meta.getPersistentDataContainer().set(PDCKeys.alloyId(), PersistentDataType.STRING, id); + if (displayName != null) meta.setDisplayName(displayName); + item.setItemMeta(meta); + return item; + } + + List drain(PlayerMock p) { + var out = new java.util.ArrayList(); + String line; + while ((line = p.nextMessage()) != null) out.add(line); + return out; + } + + @Test + void listShowsOnlyForgedAlloysThatStillExist() { + var p = server.addPlayer("Smith"); + assertTrue(alloy(p, "recipes")); + assertTrue(p.nextMessage().contains("haven't forged any alloys")); + + addAlloy("bronze", "§6Bronze", new AlloyRecipe("copper_ingot", List.of("tin_ingot"))); + addAlloy("darksteel", "§8Darksteel", new AlloyRecipe("steel_ingot", List.of("coal"))); + addAlloy("secret", "§5Secret", new AlloyRecipe("iron_ingot", List.of("coal"))); + tracker.recordForge(p.getUniqueId(), "darksteel"); + tracker.recordForge(p.getUniqueId(), "bronze"); + tracker.recordForge(p.getUniqueId(), "deleted_alloy"); + + assertTrue(alloy(p, "RECIPES")); + assertTrue(p.nextMessage().contains("(2)")); + Component line = p.nextComponentMessage(); + String plain = PlainTextComponentSerializer.plainText().serialize(line); + assertEquals("Bronze, Darksteel", plain); + assertFalse(plain.contains("Secret")); + var clicks = new java.util.ArrayList(); + collectClicks(line, clicks); + assertEquals(List.of("/alloy recipe bronze", "/alloy recipe darksteel"), clicks); + } + + static void collectClicks(Component component, List out) { + ClickEvent click = component.clickEvent(); + if (click != null) out.add(click.value()); + for (Component child : component.children()) collectClicks(child, out); + } + + @Test + void recipeShowsBaseAndCatalystsOnlyToTheForger() throws Exception { + var p = server.addPlayer("Forger"); + var other = server.addPlayer("Stranger"); + ingredient("steel_ingot", ""); + var broken = ingredient("broken_gem", ""); + when(items.getCreator().getItemFromPath("v.iron_ingot")) + .thenAnswer(i -> new ItemStack(Material.IRON_INGOT)) + .thenThrow(new IllegalStateException("no MMOItems")); + assertNotNull(broken); + addAlloy( + "dark_steel", + "§8Dark Steel", + new AlloyRecipe("steel_ingot", List.of("broken_gem", "black_coal"))); + tracker.recordForge(p.getUniqueId(), "dark_steel"); + + assertTrue(alloy(p, "recipe", "Dark", "Steel")); + var lines = drain(p); + assertEquals(4, lines.size(), lines.toString()); + assertTrue(lines.get(0).contains("Recipe for") && lines.get(0).contains("Dark Steel")); + assertTrue(lines.get(1).contains("Base:") && lines.get(1).contains("Iron Ingot"), lines.get(1)); + assertTrue(lines.get(2).contains("Black Coal") && lines.get(2).contains("Broken Gem"), lines.get(2)); + assertTrue(lines.get(3).contains("any order")); + + assertTrue(alloy(other, "recipe", "dark_steel")); + assertEquals(List.of("§cYou don't know the recipe for §fdark_steel§c."), drain(other)); + assertTrue(alloy(other, "recipe", "No", "Such", "Alloy")); + assertEquals(List.of("§cYou don't know the recipe for §fNo Such Alloy§c."), drain(other)); + } + + @Test + void recipeHandlesMissingRecipeAndNoCatalysts() { + var p = server.addPlayer("Legacy"); + addAlloy("old", "§7Old", null); + addAlloy("plain", "§7Plain", new AlloyRecipe("iron_ingot", List.of())); + tracker.recordForge(p.getUniqueId(), "old"); + tracker.recordForge(p.getUniqueId(), "plain"); + assertTrue(alloy(p, "recipe", "old")); + assertTrue(p.nextMessage().contains("never written down")); + assertTrue(alloy(p, "recipe", "plain")); + var lines = drain(p); + assertTrue(lines.get(1).contains("Iron Ingot")); + assertTrue(lines.get(2).endsWith("none")); + } + + @Test + void heldAlloyIsShownOnlyWhenForged() { + var p = server.addPlayer("Holder"); + addAlloy("bronze", "§6Bronze", new AlloyRecipe("copper_ingot", List.of("tin_ingot"))); + + assertTrue(alloy(p, "recipe")); + assertTrue(p.nextMessage().contains("Hold an alloy")); + + p.getInventory().setItemInMainHand(alloyItem("bronze", "§6Bronze")); + assertTrue(alloy(p, "recipe")); + String refusal = p.nextMessage(); + assertTrue(refusal.contains("You don't know the recipe for"), refusal); + assertTrue(refusal.contains("§6Bronze") && refusal.contains("Forge it yourself"), refusal); + + p.getInventory().setItemInMainHand(alloyItem("unloaded", null)); + assertTrue(alloy(p, "recipe")); + assertTrue(p.nextMessage().contains("this alloy")); + + tracker.recordForge(p.getUniqueId(), "bronze"); + p.getInventory().setItemInMainHand(alloyItem("bronze", "§6Bronze")); + assertTrue(alloy(p, "recipe")); + var lines = drain(p); + assertTrue(lines.get(0).contains("Bronze")); + assertTrue(lines.get(1).contains("Copper Ingot")); + assertTrue(lines.get(2).contains("Tin Ingot")); + } + + @Test + void consoleAndMissingTrackerAreHandled() { + var console = mock(CommandSender.class); + assertTrue(alloy(console, "recipes")); + assertTrue(alloy(console, "recipe", "bronze")); + verify(console, times(2)).sendMessage("§cPlayers only."); + assertFalse(alloy(console, "name", "x")); + assertFalse(alloy(console)); + + var p = server.addPlayer("NoTracker"); + addAlloy("bronze", "§6Bronze", new AlloyRecipe("copper_ingot", List.of("tin_ingot"))); + tracker.recordForge(p.getUniqueId(), "bronze"); + CraftLifecycle.init(null); + assertTrue(alloy(p, "recipes")); + assertTrue(p.nextMessage().contains("haven't forged")); + assertTrue(alloy(p, "recipe", "bronze")); + assertTrue(p.nextMessage().contains("don't know")); + assertTrue(manager.onTabComplete(p, cmd(), "alloy", new String[] {"recipe", ""}).isEmpty()); + CraftLifecycle.recordForge(p.getUniqueId(), "ignored"); + } + + @Test + void tabCompletionOffersOnlyOwnAlloys() { + var p = server.addPlayer("Tabber"); + addAlloy("bronze", "§6Bronze", new AlloyRecipe("copper_ingot", List.of("tin_ingot"))); + addAlloy("brass", "§eBrass", new AlloyRecipe("copper_ingot", List.of("zinc"))); + tracker.recordForge(p.getUniqueId(), "bronze"); + assertEquals( + List.of("recipe", "recipes"), + manager.onTabComplete(p, cmd(), "alloy", new String[] {"rec"})); + assertEquals( + List.of("bronze"), + manager.onTabComplete(p, cmd(), "alloy", new String[] {"recipe", "br"})); + assertTrue(manager.onTabComplete(p, cmd(), "alloy", new String[] {"recipes", ""}).isEmpty()); + var console = mock(CommandSender.class); + assertTrue( + manager.onTabComplete(console, cmd(), "alloy", new String[] {"recipe", ""}).isEmpty()); + } + + @Test + void trackerReadsAreSafeAndReadOnly() { + var id = java.util.UUID.randomUUID(); + assertFalse(tracker.hasForged(null, "x")); + assertFalse(tracker.hasForged(id, null)); + assertFalse(tracker.hasForged(id, " ")); + assertTrue(tracker.getForgedIds(null).isEmpty()); + tracker.recordForge(id, "Zinc_Bronze"); + tracker.recordForge(id, "brass"); + assertTrue(tracker.hasForged(id, "ZINC_BRONZE")); + assertEquals(List.of("brass", "zinc_bronze"), List.copyOf(tracker.getForgedIds(id))); + assertThrows(UnsupportedOperationException.class, () -> tracker.getForgedIds(id).add("x")); + } + + @Test + void namingTimeoutStillRecordsTheDiscoverer() throws Exception { + var online = server.addPlayer("Online"); + var offline = server.addPlayer("Offline"); + var events = new java.util.ArrayList(); + server + .getPluginManager() + .registerEvent( + AlloyDiscoveredEvent.class, + new org.bukkit.event.Listener() {}, + org.bukkit.event.EventPriority.NORMAL, + (l, e) -> events.add(((AlloyDiscoveredEvent) e).getAlloyId()), + org.mockbukkit.mockbukkit.MockBukkit.createMockPlugin()); + var method = + AlloyManager.class.getDeclaredMethod( + "namingTimedOut", org.bukkit.entity.Player.class, String.class); + method.setAccessible(true); + method.invoke(null, online, "Generated"); + assertTrue(tracker.hasForged(online.getUniqueId(), "generated")); + assertEquals(List.of("generated"), events); + offline.disconnect(); + method.invoke(null, offline, "other"); + assertTrue(tracker.hasForged(offline.getUniqueId(), "other")); + assertEquals(List.of("generated"), events); + } +} diff --git a/src/test/java/net/tfminecraft/advancedcrafting/CommandCoverageTest.java b/src/test/java/net/tfminecraft/advancedcrafting/CommandCoverageTest.java index 41f9d73..2567485 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/CommandCoverageTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/CommandCoverageTest.java @@ -191,7 +191,9 @@ void tabCompletionFiltersCommandsIdsAndOnlinePlayers() { entry.getValue(), manager.onTabComplete(p, cmd("ac"), "ac", entry.getKey().toArray(String[]::new)), entry.getKey().toString()); - assertEquals(List.of("name"), manager.onTabComplete(p, cmd("alloy"), "alloy", new String[] {})); + assertEquals( + List.of("name", "recipe", "recipes"), + manager.onTabComplete(p, cmd("alloy"), "alloy", new String[] {})); assertEquals( List.of("name"), manager.onTabComplete(p, cmd("alloy"), "alloy", new String[] {"n"})); assertTrue( From a02c60bad9dd5909c5591c75c58c1c8d694148a3 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Tue, 3 Nov 2026 16:29:02 +0100 Subject: [PATCH 2/6] Record new alloys at forge time and move records on rename A second discovery, a restart or a timeout before naming could lose the discoverer's record. Naming now moves every player's record from the generated id to the chosen one. A timed-out discovery is credited to the player as they are now (they may have relogged). A combo that no longer makes the alloy is not shown. Co-Authored-By: Claude Opus 5.5 --- .../lifecycle/CraftLifecycle.java | 29 ++++- .../lifecycle/PlayerAlloyForgeTracker.java | 32 +++++ .../managers/AlloyManager.java | 24 ++-- .../managers/AlloyRecipeCommand.java | 24 ++-- .../managers/CommandManager.java | 32 ++--- .../AlloyManagerCoverageTest.java | 7 ++ .../AlloyRecipeCommandTest.java | 115 ++++++++++++++++-- 7 files changed, 208 insertions(+), 55 deletions(-) diff --git a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java index 575dcab..30fa2fd 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java @@ -40,16 +40,37 @@ public static void fireAlloyOutcome(Player player, String alloyId) { } } - /** - * Records a forge without firing events, for a player who has gone offline - * (a new alloy whose naming prompt timed out after they left). - */ + /** Records a forge without firing events: a new alloy, as soon as it is forged and before it is named. */ public static void recordForge(UUID playerUuid, String alloyId) { if (tracker != null) { tracker.recordForge(playerUuid, alloyId); } } + /** Moves every player's record of a newly named alloy from its generated id to its chosen one. */ + public static void renameAlloy(String oldId, String newId) { + if (tracker != null) { + tracker.renameAlloy(oldId, newId); + } + } + + /** + * Credits a discovery whose naming prompt timed out. The forge was already recorded when the + * alloy was forged, so this fires the crafted and discovered events directly. + */ + public static void fireAlloyDiscovered(Player player, String alloyId) { + if (player == null || alloyId == null || alloyId.isBlank()) { + return; + } + String normalizedId = alloyId.toLowerCase(Locale.ROOT); + UUID playerUuid = player.getUniqueId(); + if (tracker != null) { + tracker.recordForge(playerUuid, normalizedId); + } + Bukkit.getPluginManager().callEvent(new AlloyCraftedEvent(player, playerUuid, normalizedId)); + Bukkit.getPluginManager().callEvent(new AlloyDiscoveredEvent(player, playerUuid, normalizedId)); + } + public static void fireItemCrafted(Player player, String recipeId, String categoryId) { if (player == null || recipeId == null || recipeId.isBlank()) { return; diff --git a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java index 9883d19..71ddc44 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java @@ -64,6 +64,38 @@ public Set getForgedIds(UUID playerUuid) { return Collections.unmodifiableSet(new TreeSet<>(loadForgedIds(playerUuid))); } + /** + * Rewrites every player file that lists {@code oldId} to list {@code newId} instead. Runs once + * per newly named alloy; the files are small and only players who have forged alloys have one. + */ + public void renameAlloy(String oldId, String newId) { + if (oldId == null || newId == null || oldId.isBlank() || newId.isBlank()) { + return; + } + String from = oldId.toLowerCase(Locale.ROOT); + String to = newId.toLowerCase(Locale.ROOT); + if (from.equals(to)) { + return; + } + File[] files = dataFolder.listFiles((dir, name) -> name.endsWith(".json")); + if (files == null) { + return; + } + for (File file : files) { + UUID playerUuid; + try { + playerUuid = UUID.fromString(file.getName().substring(0, file.getName().length() - ".json".length())); + } catch (IllegalArgumentException ex) { + continue; + } + Set forged = loadForgedIds(playerUuid); + if (forged.remove(from)) { + forged.add(to); + saveForgedIds(playerUuid, forged); + } + } + } + private Set loadForgedIds(UUID playerUuid) { Set cached = CACHE.get(playerUuid); if (cached != null) { diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java index 2114960..0a20684 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java @@ -2,7 +2,9 @@ import java.util.HashMap; import java.util.Map; +import java.util.UUID; +import org.bukkit.Bukkit; import org.bukkit.Location; import org.bukkit.Material; import org.bukkit.Sound; @@ -99,22 +101,25 @@ public void tickCycle() { public void run() { for(Map.Entry entry : ((HashMap) naming.clone()).entrySet()) { if(entry.getValue().tick()) { - entry.getKey().sendMessage("§cNaming timed out."); naming.remove(entry.getKey()); - namingTimedOut(entry.getKey(), entry.getValue().getAlloy().getId()); + namingTimedOut(entry.getKey().getUniqueId(), entry.getValue().getAlloy().getId()); } } } }.runTaskTimer(AdvancedCrafting.plugin, 0, 20L); } - /** The alloy keeps its generated name, but its discoverer has still forged it. */ - static void namingTimedOut(Player p, String alloyId) { - if (p.isOnline()) { - CraftLifecycle.fireAlloyOutcome(p, alloyId); - } else { - CraftLifecycle.recordForge(p.getUniqueId(), alloyId); + /** + * The alloy keeps its generated name. It was recorded for its discoverer when forged; this credits + * the discovery as naming would have. The player is looked up again, since they may have relogged. + */ + static void namingTimedOut(UUID playerUuid, String alloyId) { + Player p = Bukkit.getPlayer(playerUuid); + if (p == null) { + return; } + p.sendMessage("§cNaming timed out."); + CraftLifecycle.fireAlloyDiscovered(p, alloyId); } // Keep the existing legacy text representation, formatting, and exact-string comparisons. @@ -207,6 +212,8 @@ public void forgeAlloy(PlayerInteractEvent e) { p.getInventory().getItemInMainHand().setType(Material.BUCKET); removeStation(station); if(alloy != null) { + // Recorded now, so a restart, a second discovery or a timeout before naming can't lose it. + CraftLifecycle.recordForge(p.getUniqueId(), alloy.getAlloy().getId()); p.sendTitle(StringFormatter.formatHex("#d1743fNew Alloy"), StringFormatter.formatHex("#b0a996Use #36e3a4/alloy name #b0a996to name it!"), 10, 80, 10); p.sendMessage("§cThe naming prompt times out in 60 seconds."); naming.put(p, alloy); @@ -254,6 +261,7 @@ public void nameAlloy(Player p, String s) { p.sendMessage("§aNamed the new alloy "+name); addAlloy(alloy.getAlloy()); CraftLifecycle.fireAlloyOutcome(p, alloy.getAlloy().getId()); + CraftLifecycle.renameAlloy(oldId, alloy.getAlloy().getId()); naming.remove(p); } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java index 3d84dd2..5ac874d 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java @@ -5,6 +5,7 @@ import java.util.Locale; import java.util.Set; +import org.apache.commons.lang.WordUtils; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; @@ -13,6 +14,7 @@ import net.kyori.adventure.text.event.HoverEvent; import net.kyori.adventure.text.format.NamedTextColor; import net.kyori.adventure.text.serializer.legacy.LegacyComponentSerializer; +import net.tfminecraft.advancedcrafting.AdvancedCrafting; import net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle; import net.tfminecraft.advancedcrafting.lifecycle.PlayerAlloyForgeTracker; import net.tfminecraft.advancedcrafting.loaders.IngredientLoader; @@ -87,6 +89,12 @@ private void sendRecipe(Player p, Alloy alloy) { p.sendMessage("§cThe recipe for §f" + alloy.getName() + " §cwas never written down."); return; } + // The forge reads this index, so a combo that now makes something else must not be shown. + String result = AdvancedCrafting.getAlloyRecipeStore().getResultByCombo(recipe.comboKey()); + if (result != null && !result.equalsIgnoreCase(alloy.getId())) { + p.sendMessage("§cThat recipe no longer makes §f" + alloy.getName() + "§c."); + return; + } p.sendMessage("§6Recipe for §r" + alloy.getName()); p.sendMessage("§7Base: §r" + ingredientName(recipe.getBaseId())); List catalysts = new ArrayList<>(); @@ -140,20 +148,6 @@ static String ingredientName(String id) { // The item source (MMOItems, ItemsAdder) may be missing; fall back to the id. } } - return "§f" + titleCase(id); - } - - static String titleCase(String id) { - StringBuilder out = new StringBuilder(); - for (String word : id.split("_")) { - if (word.isEmpty()) { - continue; - } - if (out.length() > 0) { - out.append(' '); - } - out.append(Character.toUpperCase(word.charAt(0))).append(word.substring(1)); - } - return out.toString(); + return "§f" + WordUtils.capitalize(id.replace('_', ' ')); } } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java index f58a1f4..c3e671c 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java @@ -39,29 +39,29 @@ public boolean onCommand(CommandSender sender, Command cmd, String label, String if (cmd.getName().equalsIgnoreCase(cmd1)) { return handleAcCommand(sender, args); } - if (sender instanceof Player p) { - if (cmd.getName().equalsIgnoreCase(cmd2) && args.length >= 1 && args[0].equalsIgnoreCase("name")) { + if (!cmd.getName().equalsIgnoreCase(cmd2) || args.length < 1) { + return false; + } + String sub = args[0].toLowerCase(Locale.ROOT); + if (!sub.equals("name") && !sub.equals("recipe") && !sub.equals("recipes")) { + return false; + } + if (!(sender instanceof Player p)) { + sender.sendMessage("§cPlayers only."); + return true; + } + switch (sub) { + case "recipes" -> alloyRecipes.list(p); + case "recipe" -> alloyRecipes.show(p, args); + default -> { if (args.length < 2) { p.sendMessage("§cNo name specified, format: /alloy name newname"); return false; } AdvancedCrafting.getAlloyManager().nameAlloy(p, args[1]); - return true; - } - if (cmd.getName().equalsIgnoreCase(cmd2) && args.length >= 1 && args[0].equalsIgnoreCase("recipes")) { - alloyRecipes.list(p); - return true; - } - if (cmd.getName().equalsIgnoreCase(cmd2) && args.length >= 1 && args[0].equalsIgnoreCase("recipe")) { - alloyRecipes.show(p, args); - return true; } - } else if (cmd.getName().equalsIgnoreCase(cmd2) && args.length >= 1 - && (args[0].equalsIgnoreCase("recipe") || args[0].equalsIgnoreCase("recipes"))) { - sender.sendMessage("§cPlayers only."); - return true; } - return false; + return true; } private boolean handleAcCommand(CommandSender sender, String[] args) { diff --git a/src/test/java/net/tfminecraft/advancedcrafting/AlloyManagerCoverageTest.java b/src/test/java/net/tfminecraft/advancedcrafting/AlloyManagerCoverageTest.java index ebe2aa4..d1affa2 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/AlloyManagerCoverageTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/AlloyManagerCoverageTest.java @@ -160,13 +160,20 @@ void forgeConsumesLavaOnlyForReadyPermittedStationsAndTracksNaming() throws Exce assertFalse(m.hasStation(b.getLocation())); } var alloy = mock(Alloy.class); + when(alloy.getId()).thenReturn("fresh_find"); var named = new NamableAlloy(alloy, new ItemStack(Material.IRON_INGOT)); map(m, "stations").put(b.getLocation(), station); p.getInventory().setItemInMainHand(new ItemStack(Material.LAVA_BUCKET)); + var tracker = + new net.tfminecraft.advancedcrafting.lifecycle.PlayerAlloyForgeTracker(temp.toFile()); + net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle.init(tracker); try (var forgers = mockConstruction(AlloyForger.class, (mock, ctx) -> when(mock.forge(p)).thenReturn(named))) { m.forgeAlloy(event(p, b)); assertSame(named, map(m, "naming").get(p)); + assertTrue(tracker.hasForged(p.getUniqueId(), "fresh_find")); + } finally { + net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle.init(null); } when(plugin.isEnabled()).thenReturn(true); m.start(); diff --git a/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java b/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java index 95c6394..9f23f80 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java @@ -7,6 +7,8 @@ import net.kyori.adventure.text.Component; import net.kyori.adventure.text.event.ClickEvent; import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; +import net.tfminecraft.advancedcrafting.database.AlloyDatabase; +import net.tfminecraft.advancedcrafting.database.AlloyRecipeStore; import net.tfminecraft.advancedcrafting.lifecycle.AlloyDiscoveredEvent; import net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle; import net.tfminecraft.advancedcrafting.lifecycle.PlayerAlloyForgeTracker; @@ -30,11 +32,17 @@ class AlloyRecipeCommandTest extends CoverageSupport { CommandManager manager; PlayerAlloyForgeTracker tracker; + AlloyRecipeStore store; + @BeforeEach - void setupTracker() { + void setupTracker() throws Exception { manager = new CommandManager(); tracker = new PlayerAlloyForgeTracker(temp.toFile()); CraftLifecycle.init(tracker); + store = new AlloyRecipeStore(temp.resolve("data/alloy-recipes").toFile()); + var field = AdvancedCrafting.class.getDeclaredField("alloyRecipeStore"); + field.setAccessible(true); + field.set(plugin, store); } @AfterEach @@ -152,6 +160,16 @@ void recipeHandlesMissingRecipeAndNoCatalysts() { var lines = drain(p); assertTrue(lines.get(1).contains("Iron Ingot")); assertTrue(lines.get(2).endsWith("none")); + + var moved = new AlloyRecipe("iron_ingot", List.of("coal")); + addAlloy("moved", "§7Moved", moved); + tracker.recordForge(p.getUniqueId(), "moved"); + store.upsert(moved, "moved"); + assertTrue(alloy(p, "recipe", "moved")); + assertTrue(drain(p).get(0).contains("Recipe for")); + store.upsert(moved, "scrap"); + assertTrue(alloy(p, "recipe", "moved")); + assertTrue(p.nextMessage().contains("no longer makes")); } @Test @@ -186,8 +204,9 @@ void consoleAndMissingTrackerAreHandled() { var console = mock(CommandSender.class); assertTrue(alloy(console, "recipes")); assertTrue(alloy(console, "recipe", "bronze")); - verify(console, times(2)).sendMessage("§cPlayers only."); - assertFalse(alloy(console, "name", "x")); + assertTrue(alloy(console, "name", "x")); + verify(console, times(3)).sendMessage("§cPlayers only."); + assertFalse(alloy(console, "other")); assertFalse(alloy(console)); var p = server.addPlayer("NoTracker"); @@ -234,10 +253,7 @@ void trackerReadsAreSafeAndReadOnly() { assertThrows(UnsupportedOperationException.class, () -> tracker.getForgedIds(id).add("x")); } - @Test - void namingTimeoutStillRecordsTheDiscoverer() throws Exception { - var online = server.addPlayer("Online"); - var offline = server.addPlayer("Offline"); + java.util.List discoveries() { var events = new java.util.ArrayList(); server .getPluginManager() @@ -247,16 +263,91 @@ void namingTimeoutStillRecordsTheDiscoverer() throws Exception { org.bukkit.event.EventPriority.NORMAL, (l, e) -> events.add(((AlloyDiscoveredEvent) e).getAlloyId()), org.mockbukkit.mockbukkit.MockBukkit.createMockPlugin()); + return events; + } + + @Test + void namingTimeoutCreditsTheDiscovererIfOnline() throws Exception { + var online = server.addPlayer("Online"); + var offline = server.addPlayer("Offline"); + var events = discoveries(); var method = - AlloyManager.class.getDeclaredMethod( - "namingTimedOut", org.bukkit.entity.Player.class, String.class); + AlloyManager.class.getDeclaredMethod("namingTimedOut", java.util.UUID.class, String.class); method.setAccessible(true); - method.invoke(null, online, "Generated"); + tracker.recordForge(online.getUniqueId(), "generated"); + method.invoke(null, online.getUniqueId(), "Generated"); + assertTrue(online.nextMessage().contains("Naming timed out")); assertTrue(tracker.hasForged(online.getUniqueId(), "generated")); assertEquals(List.of("generated"), events); offline.disconnect(); - method.invoke(null, offline, "other"); - assertTrue(tracker.hasForged(offline.getUniqueId(), "other")); + method.invoke(null, offline.getUniqueId(), "other"); + assertFalse(tracker.hasForged(offline.getUniqueId(), "other")); assertEquals(List.of("generated"), events); + CraftLifecycle.fireAlloyDiscovered(null, "x"); + CraftLifecycle.fireAlloyDiscovered(online, null); + CraftLifecycle.fireAlloyDiscovered(online, " "); + CraftLifecycle.init(null); + CraftLifecycle.fireAlloyDiscovered(online, "untracked"); + CraftLifecycle.renameAlloy("a", "b"); + assertEquals(List.of("generated", "untracked"), events); + } + + @Test + void namingMovesEveryRecordToTheChosenId() throws Exception { + var finder = server.addPlayer("Finder"); + var copier = server.addPlayer("Copier"); + var events = discoveries(); + var m = new AlloyManager(); + var base = ingredient("iron", "base: true"); + var alloy = + spy( + new Alloy( + "ferrum_vex", + "Ferrum Vex", + new AlloyData( + base, + new net.tfminecraft.advancedcrafting.objects.data.StatData(), + new java.util.HashMap<>(), + null))); + doReturn(new ItemStack(Material.GOLD_INGOT)).when(alloy).build(); + AlloyManager.addAlloy(alloy); + tracker.recordForge(finder.getUniqueId(), "ferrum_vex"); + tracker.recordForge(copier.getUniqueId(), "ferrum_vex"); + tracker.recordForge(copier.getUniqueId(), "brass"); + var naming = AlloyManager.class.getDeclaredField("naming"); + naming.setAccessible(true); + @SuppressWarnings("unchecked") + var map = (java.util.Map) naming.get(m); + map.put( + finder, + new net.tfminecraft.advancedcrafting.objects.alloys.NamableAlloy( + alloy, new ItemStack(Material.IRON_INGOT))); + try (var db = mockConstruction(AlloyDatabase.class)) { + m.nameAlloy(finder, "Bright_Steel"); + } + assertEquals(List.of("bright_steel"), List.copyOf(tracker.getForgedIds(finder.getUniqueId()))); + assertEquals( + List.of("brass", "bright_steel"), List.copyOf(tracker.getForgedIds(copier.getUniqueId()))); + assertEquals(List.of("bright_steel"), events); + } + + @Test + void renameSkipsBadInputAndForeignFiles() throws Exception { + var id = java.util.UUID.randomUUID(); + tracker.recordForge(id, "old"); + var notes = temp.resolve("forged-alloys/notes.json"); + java.nio.file.Files.writeString(notes, "[\"old\"]"); + java.nio.file.Files.writeString(temp.resolve("forged-alloys/readme.txt"), "x"); + for (String[] pair : + new String[][] {{null, "x"}, {"old", null}, {" ", "x"}, {"old", " "}, {"OLD", "old"}}) + tracker.renameAlloy(pair[0], pair[1]); + assertTrue(tracker.hasForged(id, "old")); + tracker.renameAlloy("Old", "New"); + assertTrue(tracker.hasForged(id, "new")); + assertFalse(tracker.hasForged(id, "old")); + assertEquals("[\"old\"]", java.nio.file.Files.readString(notes)); + var gone = new PlayerAlloyForgeTracker(temp.resolve("elsewhere").toFile()); + java.nio.file.Files.delete(temp.resolve("elsewhere/forged-alloys")); + gone.renameAlloy("a", "b"); } } From 6404f64903f71ec4c8ce84d60d417097f1bfded7 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:23:10 +0100 Subject: [PATCH 3/6] Record known alloy recipes by combo, not by alloy id Alloy ids change when an alloy is named and generated ids can be reused, so keying knowledge by id needed rename scans and changed when discovery events fire. Successful forges now record the station's combo in data/known-alloy-recipes/.json; a player without a file starts from their existing forge history. The naming, timeout and event code is back to what main has. A combo the forge index no longer maps to the alloy is not shown. Co-Authored-By: Claude Opus 5.5 --- .../advancedcrafting/AdvancedCrafting.java | 2 + .../lifecycle/CraftLifecycle.java | 31 -- .../lifecycle/KnownAlloyRecipes.java | 136 ++++++++ .../lifecycle/PlayerAlloyForgeTracker.java | 45 +-- .../managers/AlloyManager.java | 20 +- .../managers/AlloyRecipeCommand.java | 31 +- .../objects/alloys/AlloyForger.java | 2 + .../AlloyManagerCoverageTest.java | 7 - .../AlloyRecipeCommandTest.java | 297 ++++++++---------- .../advancedcrafting/ForgerCoverageTest.java | 37 +++ 10 files changed, 328 insertions(+), 280 deletions(-) create mode 100644 src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java diff --git a/src/main/java/net/tfminecraft/advancedcrafting/AdvancedCrafting.java b/src/main/java/net/tfminecraft/advancedcrafting/AdvancedCrafting.java index cc0de8b..49d04eb 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/AdvancedCrafting.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/AdvancedCrafting.java @@ -29,6 +29,7 @@ import net.tfminecraft.advancedcrafting.objects.crafting.CraftingStation; import net.tfminecraft.advancedcrafting.utils.RevisionTracker; import net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle; +import net.tfminecraft.advancedcrafting.lifecycle.KnownAlloyRecipes; import net.tfminecraft.advancedcrafting.lifecycle.PlayerAlloyForgeTracker; public class AdvancedCrafting extends JavaPlugin{ @@ -68,6 +69,7 @@ public void onEnable() { revisionTracker.load(getDataFolder()); loadConfigs(); CraftLifecycle.init(new PlayerAlloyForgeTracker(new File(getDataFolder(), "data"))); + KnownAlloyRecipes.init(new KnownAlloyRecipes(new File(getDataFolder(), "data"))); registerListeners(); craftRefreshListener.start(this); getCommand(commandManager.cmd1).setExecutor(commandManager); diff --git a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java index 30fa2fd..be0ed25 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/CraftLifecycle.java @@ -40,37 +40,6 @@ public static void fireAlloyOutcome(Player player, String alloyId) { } } - /** Records a forge without firing events: a new alloy, as soon as it is forged and before it is named. */ - public static void recordForge(UUID playerUuid, String alloyId) { - if (tracker != null) { - tracker.recordForge(playerUuid, alloyId); - } - } - - /** Moves every player's record of a newly named alloy from its generated id to its chosen one. */ - public static void renameAlloy(String oldId, String newId) { - if (tracker != null) { - tracker.renameAlloy(oldId, newId); - } - } - - /** - * Credits a discovery whose naming prompt timed out. The forge was already recorded when the - * alloy was forged, so this fires the crafted and discovered events directly. - */ - public static void fireAlloyDiscovered(Player player, String alloyId) { - if (player == null || alloyId == null || alloyId.isBlank()) { - return; - } - String normalizedId = alloyId.toLowerCase(Locale.ROOT); - UUID playerUuid = player.getUniqueId(); - if (tracker != null) { - tracker.recordForge(playerUuid, normalizedId); - } - Bukkit.getPluginManager().callEvent(new AlloyCraftedEvent(player, playerUuid, normalizedId)); - Bukkit.getPluginManager().callEvent(new AlloyDiscoveredEvent(player, playerUuid, normalizedId)); - } - public static void fireItemCrafted(Player player, String recipeId, String categoryId) { if (player == null || recipeId == null || recipeId.isBlank()) { return; diff --git a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java new file mode 100644 index 0000000..bbcc4ed --- /dev/null +++ b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java @@ -0,0 +1,136 @@ +package net.tfminecraft.advancedcrafting.lifecycle; + +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import org.json.simple.JSONArray; +import org.json.simple.parser.JSONParser; + +import net.tfminecraft.advancedcrafting.AdvancedCrafting; +import net.tfminecraft.advancedcrafting.managers.AlloyManager; +import net.tfminecraft.advancedcrafting.objects.alloys.Alloy; +import net.tfminecraft.advancedcrafting.objects.data.AlloyRecipe; + +/** + * The alloy recipes each account has forged, as combo keys ({@code base|catalyst,catalyst}). + * A combo never changes when its alloy is named or renamed and is never reused, unlike alloy ids. + * Files live in {@code data/known-alloy-recipes/.json}. A player without a file yet starts + * from the alloys {@link PlayerAlloyForgeTracker} has on record for them. + */ +public final class KnownAlloyRecipes { + private static KnownAlloyRecipes instance; + + private final File folder; + private final Map> cache = new HashMap<>(); + + public KnownAlloyRecipes(File dataFolder) { + this.folder = new File(dataFolder, "known-alloy-recipes"); + if (!folder.exists()) { + folder.mkdirs(); + } + } + + public static void init(KnownAlloyRecipes known) { + instance = known; + } + + public static KnownAlloyRecipes get() { + return instance; + } + + /** Called for every successful forge (not scrap). No-op before the plugin has set an instance. */ + public static void learn(UUID playerUuid, AlloyRecipe recipe) { + if (instance != null && playerUuid != null && recipe != null) { + instance.add(playerUuid, recipe.comboKey()); + } + } + + public boolean knows(UUID playerUuid, Alloy alloy) { + if (playerUuid == null || alloy == null || alloy.getData() == null) { + return false; + } + AlloyRecipe recipe = alloy.getData().getRecipe(); + return recipe != null && load(playerUuid).contains(recipe.comboKey()); + } + + private void add(UUID playerUuid, String comboKey) { + Set known = load(playerUuid); + if (known.add(comboKey)) { + save(playerUuid, known); + } + } + + private Set load(UUID playerUuid) { + Set known = cache.get(playerUuid); + if (known != null) { + return known; + } + known = new HashSet<>(); + File file = file(playerUuid); + if (file.exists()) { + try (InputStreamReader reader = new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8)) { + Object parsed = new JSONParser().parse(reader); + if (parsed instanceof JSONArray array) { + for (Object entry : array) { + if (entry != null && !entry.toString().isBlank()) { + known.add(entry.toString().trim().toLowerCase()); + } + } + } + } catch (Exception ex) { + // Keep the unreadable file untouched; this session just starts from what was read. + AdvancedCrafting.plugin.getLogger().warning("AC: could not read " + file.getName() + ": " + ex.getMessage()); + cache.put(playerUuid, known); + return known; + } + } else if (seedFromForgeHistory(playerUuid, known)) { + save(playerUuid, known); + } + cache.put(playerUuid, known); + return known; + } + + /** Recipes of the alloys this player forged before recipes were recorded. */ + private boolean seedFromForgeHistory(UUID playerUuid, Set known) { + PlayerAlloyForgeTracker tracker = CraftLifecycle.getTracker(); + if (tracker == null) { + return false; + } + for (String alloyId : tracker.getForgedIds(playerUuid)) { + Alloy alloy = AlloyManager.getAlloyById(alloyId); + if (alloy != null && alloy.getData() != null && alloy.getData().getRecipe() != null) { + known.add(alloy.getData().getRecipe().comboKey()); + } + } + return !known.isEmpty(); + } + + @SuppressWarnings("unchecked") + private void save(UUID playerUuid, Set known) { + List sorted = new ArrayList<>(known); + sorted.sort(null); + JSONArray array = new JSONArray(); + array.addAll(sorted); + try (PrintWriter writer = new PrintWriter(file(playerUuid), StandardCharsets.UTF_8)) { + writer.print(array.toJSONString()); + } catch (IOException ex) { + AdvancedCrafting.plugin.getLogger().warning("AC: could not save known alloy recipes for " + playerUuid + + ": " + ex.getMessage()); + } + } + + private File file(UUID playerUuid) { + return new File(folder, playerUuid + ".json"); + } +} diff --git a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java index 71ddc44..8e602bc 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java @@ -10,7 +10,6 @@ import java.util.Collections; import java.util.Locale; import java.util.Set; -import java.util.TreeSet; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -48,52 +47,12 @@ public boolean recordForge(UUID playerUuid, String alloyId) { return true; } - /** True once this account has forged the alloy (a real alloy, not scrap). */ - public boolean hasForged(UUID playerUuid, String alloyId) { - if (playerUuid == null || alloyId == null || alloyId.isBlank()) { - return false; - } - return loadForgedIds(playerUuid).contains(alloyId.toLowerCase(Locale.ROOT)); - } - - /** The alloy ids this account has forged, sorted; a read-only copy. */ + /** The alloy ids this account has forged; a read-only copy. */ public Set getForgedIds(UUID playerUuid) { if (playerUuid == null) { return Set.of(); } - return Collections.unmodifiableSet(new TreeSet<>(loadForgedIds(playerUuid))); - } - - /** - * Rewrites every player file that lists {@code oldId} to list {@code newId} instead. Runs once - * per newly named alloy; the files are small and only players who have forged alloys have one. - */ - public void renameAlloy(String oldId, String newId) { - if (oldId == null || newId == null || oldId.isBlank() || newId.isBlank()) { - return; - } - String from = oldId.toLowerCase(Locale.ROOT); - String to = newId.toLowerCase(Locale.ROOT); - if (from.equals(to)) { - return; - } - File[] files = dataFolder.listFiles((dir, name) -> name.endsWith(".json")); - if (files == null) { - return; - } - for (File file : files) { - UUID playerUuid; - try { - playerUuid = UUID.fromString(file.getName().substring(0, file.getName().length() - ".json".length())); - } catch (IllegalArgumentException ex) { - continue; - } - Set forged = loadForgedIds(playerUuid); - if (forged.remove(from)) { - forged.add(to); - saveForgedIds(playerUuid, forged); - } - } + return Collections.unmodifiableSet(new HashSet<>(loadForgedIds(playerUuid))); } private Set loadForgedIds(UUID playerUuid) { diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java index 0a20684..52eba4d 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyManager.java @@ -2,9 +2,7 @@ import java.util.HashMap; import java.util.Map; -import java.util.UUID; -import org.bukkit.Bukkit; import org.bukkit.Location; import org.bukkit.Material; import org.bukkit.Sound; @@ -101,27 +99,14 @@ public void tickCycle() { public void run() { for(Map.Entry entry : ((HashMap) naming.clone()).entrySet()) { if(entry.getValue().tick()) { + entry.getKey().sendMessage("§cNaming timed out."); naming.remove(entry.getKey()); - namingTimedOut(entry.getKey().getUniqueId(), entry.getValue().getAlloy().getId()); } } } }.runTaskTimer(AdvancedCrafting.plugin, 0, 20L); } - /** - * The alloy keeps its generated name. It was recorded for its discoverer when forged; this credits - * the discovery as naming would have. The player is looked up again, since they may have relogged. - */ - static void namingTimedOut(UUID playerUuid, String alloyId) { - Player p = Bukkit.getPlayer(playerUuid); - if (p == null) { - return; - } - p.sendMessage("§cNaming timed out."); - CraftLifecycle.fireAlloyDiscovered(p, alloyId); - } - // Keep the existing legacy text representation, formatting, and exact-string comparisons. @SuppressWarnings("deprecation") @EventHandler @@ -212,8 +197,6 @@ public void forgeAlloy(PlayerInteractEvent e) { p.getInventory().getItemInMainHand().setType(Material.BUCKET); removeStation(station); if(alloy != null) { - // Recorded now, so a restart, a second discovery or a timeout before naming can't lose it. - CraftLifecycle.recordForge(p.getUniqueId(), alloy.getAlloy().getId()); p.sendTitle(StringFormatter.formatHex("#d1743fNew Alloy"), StringFormatter.formatHex("#b0a996Use #36e3a4/alloy name #b0a996to name it!"), 10, 80, 10); p.sendMessage("§cThe naming prompt times out in 60 seconds."); naming.put(p, alloy); @@ -261,7 +244,6 @@ public void nameAlloy(Player p, String s) { p.sendMessage("§aNamed the new alloy "+name); addAlloy(alloy.getAlloy()); CraftLifecycle.fireAlloyOutcome(p, alloy.getAlloy().getId()); - CraftLifecycle.renameAlloy(oldId, alloy.getAlloy().getId()); naming.remove(p); } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java index 5ac874d..77ca6f7 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java @@ -3,7 +3,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; -import java.util.Set; +import java.util.TreeSet; import org.apache.commons.lang.WordUtils; import org.bukkit.entity.Player; @@ -15,8 +15,7 @@ import net.kyori.adventure.text.format.NamedTextColor; import net.kyori.adventure.text.serializer.legacy.LegacyComponentSerializer; import net.tfminecraft.advancedcrafting.AdvancedCrafting; -import net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle; -import net.tfminecraft.advancedcrafting.lifecycle.PlayerAlloyForgeTracker; +import net.tfminecraft.advancedcrafting.lifecycle.KnownAlloyRecipes; import net.tfminecraft.advancedcrafting.loaders.IngredientLoader; import net.tfminecraft.advancedcrafting.objects.alloys.Alloy; import net.tfminecraft.advancedcrafting.objects.data.AlloyRecipe; @@ -61,7 +60,7 @@ void show(Player p, String[] args) { String typed = String.join(" ", java.util.Arrays.copyOfRange(args, 1, args.length)); String id = typed.replace(' ', '_').toLowerCase(Locale.ROOT); Alloy alloy = AlloyManager.getAlloyById(id); - if (alloy == null || !knows(p, alloy.getId())) { + if (alloy == null || !knows(p, alloy)) { p.sendMessage("§cYou don't know the recipe for §f" + typed + "§c."); return; } @@ -75,7 +74,7 @@ private void showHeld(Player p) { return; } Alloy alloy = AlloyManager.getAlloyById(AcItemTags.getId(held)); - if (alloy == null || !knows(p, alloy.getId())) { + if (alloy == null || !knows(p, alloy)) { String name = held.getItemMeta().hasDisplayName() ? held.getItemMeta().getDisplayName() : "this alloy"; p.sendMessage("§cYou don't know the recipe for §f" + name + "§c. Forge it yourself to learn it."); return; @@ -84,14 +83,11 @@ private void showHeld(Player p) { } private void sendRecipe(Player p, Alloy alloy) { + // knows() already required a recipe. AlloyRecipe recipe = alloy.getData().getRecipe(); - if (recipe == null) { - p.sendMessage("§cThe recipe for §f" + alloy.getName() + " §cwas never written down."); - return; - } // The forge reads this index, so a combo that now makes something else must not be shown. String result = AdvancedCrafting.getAlloyRecipeStore().getResultByCombo(recipe.comboKey()); - if (result != null && !result.equalsIgnoreCase(alloy.getId())) { + if (!alloy.getId().equalsIgnoreCase(result)) { p.sendMessage("§cThat recipe no longer makes §f" + alloy.getName() + "§c."); return; } @@ -115,24 +111,19 @@ List completions(Player p) { } private List knownAlloys(Player p) { - PlayerAlloyForgeTracker tracker = CraftLifecycle.getTracker(); - if (tracker == null) { - return List.of(); - } - Set ids = tracker.getForgedIds(p.getUniqueId()); List known = new ArrayList<>(); - for (String id : ids) { + for (String id : new TreeSet<>(AlloyManager.getAlloyIds())) { Alloy alloy = AlloyManager.getAlloyById(id); - if (alloy != null) { + if (knows(p, alloy)) { known.add(alloy); } } return known; } - private boolean knows(Player p, String alloyId) { - PlayerAlloyForgeTracker tracker = CraftLifecycle.getTracker(); - return tracker != null && tracker.hasForged(p.getUniqueId(), alloyId); + private boolean knows(Player p, Alloy alloy) { + KnownAlloyRecipes known = KnownAlloyRecipes.get(); + return known != null && known.knows(p.getUniqueId(), alloy); } /** The ingredient's item name with its colour, or its id in title case if the item can't be built. */ diff --git a/src/main/java/net/tfminecraft/advancedcrafting/objects/alloys/AlloyForger.java b/src/main/java/net/tfminecraft/advancedcrafting/objects/alloys/AlloyForger.java index d0aeb29..fd9b962 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/objects/alloys/AlloyForger.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/objects/alloys/AlloyForger.java @@ -23,6 +23,7 @@ import net.tfminecraft.advancedcrafting.objects.ingredients.Ingredient; import net.tfminecraft.advancedcrafting.objects.stats.StatModifier; import net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle; +import net.tfminecraft.advancedcrafting.lifecycle.KnownAlloyRecipes; public class AlloyForger { private static final String GEMSTONE_PATH_PREFIX = "m.gemstones."; @@ -104,6 +105,7 @@ public NamableAlloy forge(Player p, int i) { loc.getWorld().dropItem(loc, template); return null; } + KnownAlloyRecipes.learn(p.getUniqueId(), AlloyRecipe.fromStation(station)); ItemStack item = loc.getWorld().dropItem(loc, a.build()).getItemStack(); if (isNew) { return new NamableAlloy(a, item); diff --git a/src/test/java/net/tfminecraft/advancedcrafting/AlloyManagerCoverageTest.java b/src/test/java/net/tfminecraft/advancedcrafting/AlloyManagerCoverageTest.java index d1affa2..ebe2aa4 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/AlloyManagerCoverageTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/AlloyManagerCoverageTest.java @@ -160,20 +160,13 @@ void forgeConsumesLavaOnlyForReadyPermittedStationsAndTracksNaming() throws Exce assertFalse(m.hasStation(b.getLocation())); } var alloy = mock(Alloy.class); - when(alloy.getId()).thenReturn("fresh_find"); var named = new NamableAlloy(alloy, new ItemStack(Material.IRON_INGOT)); map(m, "stations").put(b.getLocation(), station); p.getInventory().setItemInMainHand(new ItemStack(Material.LAVA_BUCKET)); - var tracker = - new net.tfminecraft.advancedcrafting.lifecycle.PlayerAlloyForgeTracker(temp.toFile()); - net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle.init(tracker); try (var forgers = mockConstruction(AlloyForger.class, (mock, ctx) -> when(mock.forge(p)).thenReturn(named))) { m.forgeAlloy(event(p, b)); assertSame(named, map(m, "naming").get(p)); - assertTrue(tracker.hasForged(p.getUniqueId(), "fresh_find")); - } finally { - net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle.init(null); } when(plugin.isEnabled()).thenReturn(true); m.start(); diff --git a/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java b/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java index 9f23f80..7a16291 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java @@ -3,14 +3,16 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; +import java.nio.file.Files; +import java.util.ArrayList; import java.util.List; +import java.util.UUID; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.event.ClickEvent; import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer; -import net.tfminecraft.advancedcrafting.database.AlloyDatabase; import net.tfminecraft.advancedcrafting.database.AlloyRecipeStore; -import net.tfminecraft.advancedcrafting.lifecycle.AlloyDiscoveredEvent; import net.tfminecraft.advancedcrafting.lifecycle.CraftLifecycle; +import net.tfminecraft.advancedcrafting.lifecycle.KnownAlloyRecipes; import net.tfminecraft.advancedcrafting.lifecycle.PlayerAlloyForgeTracker; import net.tfminecraft.advancedcrafting.managers.AlloyManager; import net.tfminecraft.advancedcrafting.managers.CommandManager; @@ -31,14 +33,16 @@ class AlloyRecipeCommandTest extends CoverageSupport { CommandManager manager; PlayerAlloyForgeTracker tracker; - + KnownAlloyRecipes known; AlloyRecipeStore store; @BeforeEach - void setupTracker() throws Exception { + void setupStores() throws Exception { manager = new CommandManager(); tracker = new PlayerAlloyForgeTracker(temp.toFile()); CraftLifecycle.init(tracker); + known = new KnownAlloyRecipes(temp.toFile()); + KnownAlloyRecipes.init(known); store = new AlloyRecipeStore(temp.resolve("data/alloy-recipes").toFile()); var field = AdvancedCrafting.class.getDeclaredField("alloyRecipeStore"); field.setAccessible(true); @@ -46,8 +50,9 @@ void setupTracker() throws Exception { } @AfterEach - void clearTracker() { + void clearStores() { CraftLifecycle.init(null); + KnownAlloyRecipes.init(null); } Command cmd() { @@ -60,6 +65,7 @@ boolean alloy(CommandSender sender, String... args) { return manager.onCommand(sender, cmd(), "alloy", args); } + /** An alloy whose combo the forge index maps to it, as on a live server. */ Alloy addAlloy(String id, String name, AlloyRecipe recipe) { var alloy = mock(Alloy.class); var data = mock(AlloyData.class); @@ -68,9 +74,14 @@ Alloy addAlloy(String id, String name, AlloyRecipe recipe) { when(alloy.getData()).thenReturn(data); when(data.getRecipe()).thenReturn(recipe); AlloyManager.addAlloy(alloy); + if (recipe != null) store.upsert(recipe, id); return alloy; } + static AlloyRecipe combo(String base, String... catalysts) { + return new AlloyRecipe(base, List.of(catalysts)); + } + ItemStack alloyItem(String id, String displayName) { var item = new ItemStack(Material.IRON_INGOT); var meta = item.getItemMeta(); @@ -81,65 +92,67 @@ ItemStack alloyItem(String id, String displayName) { } List drain(PlayerMock p) { - var out = new java.util.ArrayList(); + var out = new ArrayList(); String line; while ((line = p.nextMessage()) != null) out.add(line); return out; } + static void collectClicks(Component component, List out) { + ClickEvent click = component.clickEvent(); + if (click != null) out.add(click.value()); + for (Component child : component.children()) collectClicks(child, out); + } + @Test - void listShowsOnlyForgedAlloysThatStillExist() { + void listShowsOnlyAlloysWhoseRecipeThePlayerForged() { var p = server.addPlayer("Smith"); assertTrue(alloy(p, "recipes")); assertTrue(p.nextMessage().contains("haven't forged any alloys")); - addAlloy("bronze", "§6Bronze", new AlloyRecipe("copper_ingot", List.of("tin_ingot"))); - addAlloy("darksteel", "§8Darksteel", new AlloyRecipe("steel_ingot", List.of("coal"))); - addAlloy("secret", "§5Secret", new AlloyRecipe("iron_ingot", List.of("coal"))); - tracker.recordForge(p.getUniqueId(), "darksteel"); - tracker.recordForge(p.getUniqueId(), "bronze"); - tracker.recordForge(p.getUniqueId(), "deleted_alloy"); + var bronze = combo("copper_ingot", "tin_ingot"); + var darksteel = combo("steel_ingot", "coal"); + addAlloy("bronze", "§6Bronze", bronze); + addAlloy("darksteel", "§8Darksteel", darksteel); + addAlloy("secret", "§5Secret", combo("iron_ingot", "coal")); + addAlloy("lost", "§7Lost", null); + KnownAlloyRecipes.learn(p.getUniqueId(), darksteel); + KnownAlloyRecipes.learn(p.getUniqueId(), bronze); + KnownAlloyRecipes.learn(p.getUniqueId(), combo("gone_ingot", "coal")); assertTrue(alloy(p, "RECIPES")); assertTrue(p.nextMessage().contains("(2)")); Component line = p.nextComponentMessage(); String plain = PlainTextComponentSerializer.plainText().serialize(line); assertEquals("Bronze, Darksteel", plain); - assertFalse(plain.contains("Secret")); - var clicks = new java.util.ArrayList(); + var clicks = new ArrayList(); collectClicks(line, clicks); assertEquals(List.of("/alloy recipe bronze", "/alloy recipe darksteel"), clicks); } - static void collectClicks(Component component, List out) { - ClickEvent click = component.clickEvent(); - if (click != null) out.add(click.value()); - for (Component child : component.children()) collectClicks(child, out); - } - @Test void recipeShowsBaseAndCatalystsOnlyToTheForger() throws Exception { var p = server.addPlayer("Forger"); var other = server.addPlayer("Stranger"); ingredient("steel_ingot", ""); - var broken = ingredient("broken_gem", ""); + ingredient("broken_gem", ""); when(items.getCreator().getItemFromPath("v.iron_ingot")) .thenAnswer(i -> new ItemStack(Material.IRON_INGOT)) .thenThrow(new IllegalStateException("no MMOItems")); - assertNotNull(broken); - addAlloy( - "dark_steel", - "§8Dark Steel", - new AlloyRecipe("steel_ingot", List.of("broken_gem", "black_coal"))); - tracker.recordForge(p.getUniqueId(), "dark_steel"); + var darkSteel = combo("steel_ingot", "broken_gem", "black_coal"); + addAlloy("dark_steel", "§8Dark Steel", darkSteel); + KnownAlloyRecipes.learn(p.getUniqueId(), darkSteel); assertTrue(alloy(p, "recipe", "Dark", "Steel")); var lines = drain(p); assertEquals(4, lines.size(), lines.toString()); assertTrue(lines.get(0).contains("Recipe for") && lines.get(0).contains("Dark Steel")); assertTrue(lines.get(1).contains("Base:") && lines.get(1).contains("Iron Ingot"), lines.get(1)); - assertTrue(lines.get(2).contains("Black Coal") && lines.get(2).contains("Broken Gem"), lines.get(2)); + assertTrue( + lines.get(2).contains("Black Coal") && lines.get(2).contains("Broken Gem"), lines.get(2)); assertTrue(lines.get(3).contains("any order")); + String shown = String.join("\n", lines); + assertFalse(shown.contains("%") || shown.toLowerCase().contains("hit"), shown); assertTrue(alloy(other, "recipe", "dark_steel")); assertEquals(List.of("§cYou don't know the recipe for §fdark_steel§c."), drain(other)); @@ -148,34 +161,38 @@ void recipeShowsBaseAndCatalystsOnlyToTheForger() throws Exception { } @Test - void recipeHandlesMissingRecipeAndNoCatalysts() { + void recipeHandlesMissingRecipeNoCatalystsAndStaleIndexes() { var p = server.addPlayer("Legacy"); - addAlloy("old", "§7Old", null); - addAlloy("plain", "§7Plain", new AlloyRecipe("iron_ingot", List.of())); - tracker.recordForge(p.getUniqueId(), "old"); - tracker.recordForge(p.getUniqueId(), "plain"); - assertTrue(alloy(p, "recipe", "old")); - assertTrue(p.nextMessage().contains("never written down")); + var plain = combo("iron_ingot"); + addAlloy("plain", "§7Plain", plain); + KnownAlloyRecipes.learn(p.getUniqueId(), plain); assertTrue(alloy(p, "recipe", "plain")); var lines = drain(p); assertTrue(lines.get(1).contains("Iron Ingot")); assertTrue(lines.get(2).endsWith("none")); - var moved = new AlloyRecipe("iron_ingot", List.of("coal")); - addAlloy("moved", "§7Moved", moved); - tracker.recordForge(p.getUniqueId(), "moved"); - store.upsert(moved, "moved"); - assertTrue(alloy(p, "recipe", "moved")); - assertTrue(drain(p).get(0).contains("Recipe for")); - store.upsert(moved, "scrap"); - assertTrue(alloy(p, "recipe", "moved")); + store.upsert(plain, "scrap"); + assertTrue(alloy(p, "recipe", "plain")); assertTrue(p.nextMessage().contains("no longer makes")); + store.deleteByCombo(plain.comboKey()); + assertTrue(alloy(p, "recipe", "plain")); + assertTrue(p.nextMessage().contains("no longer makes")); + + // A recipe recorded for the player whose alloy file later lost its recipe. + var old = addAlloy("old", "§7Old", combo("tin_ingot", "coal")); + KnownAlloyRecipes.learn(p.getUniqueId(), combo("tin_ingot", "coal")); + assertTrue(alloy(p, "recipe", "old")); + assertTrue(drain(p).get(0).contains("Recipe for")); + when(old.getData().getRecipe()).thenReturn(null); + assertTrue(alloy(p, "recipe", "old")); + assertTrue(p.nextMessage().contains("don't know")); } @Test void heldAlloyIsShownOnlyWhenForged() { var p = server.addPlayer("Holder"); - addAlloy("bronze", "§6Bronze", new AlloyRecipe("copper_ingot", List.of("tin_ingot"))); + var bronze = combo("copper_ingot", "tin_ingot"); + addAlloy("bronze", "§6Bronze", bronze); assertTrue(alloy(p, "recipe")); assertTrue(p.nextMessage().contains("Hold an alloy")); @@ -190,7 +207,7 @@ void heldAlloyIsShownOnlyWhenForged() { assertTrue(alloy(p, "recipe")); assertTrue(p.nextMessage().contains("this alloy")); - tracker.recordForge(p.getUniqueId(), "bronze"); + KnownAlloyRecipes.learn(p.getUniqueId(), bronze); p.getInventory().setItemInMainHand(alloyItem("bronze", "§6Bronze")); assertTrue(alloy(p, "recipe")); var lines = drain(p); @@ -200,7 +217,7 @@ void heldAlloyIsShownOnlyWhenForged() { } @Test - void consoleAndMissingTrackerAreHandled() { + void consoleAndMissingStoreAreHandled() { var console = mock(CommandSender.class); assertTrue(alloy(console, "recipes")); assertTrue(alloy(console, "recipe", "bronze")); @@ -209,24 +226,26 @@ void consoleAndMissingTrackerAreHandled() { assertFalse(alloy(console, "other")); assertFalse(alloy(console)); - var p = server.addPlayer("NoTracker"); - addAlloy("bronze", "§6Bronze", new AlloyRecipe("copper_ingot", List.of("tin_ingot"))); - tracker.recordForge(p.getUniqueId(), "bronze"); - CraftLifecycle.init(null); + var p = server.addPlayer("NoStore"); + var bronze = combo("copper_ingot", "tin_ingot"); + addAlloy("bronze", "§6Bronze", bronze); + KnownAlloyRecipes.learn(p.getUniqueId(), bronze); + KnownAlloyRecipes.init(null); + KnownAlloyRecipes.learn(p.getUniqueId(), bronze); assertTrue(alloy(p, "recipes")); assertTrue(p.nextMessage().contains("haven't forged")); assertTrue(alloy(p, "recipe", "bronze")); assertTrue(p.nextMessage().contains("don't know")); assertTrue(manager.onTabComplete(p, cmd(), "alloy", new String[] {"recipe", ""}).isEmpty()); - CraftLifecycle.recordForge(p.getUniqueId(), "ignored"); } @Test void tabCompletionOffersOnlyOwnAlloys() { var p = server.addPlayer("Tabber"); - addAlloy("bronze", "§6Bronze", new AlloyRecipe("copper_ingot", List.of("tin_ingot"))); - addAlloy("brass", "§eBrass", new AlloyRecipe("copper_ingot", List.of("zinc"))); - tracker.recordForge(p.getUniqueId(), "bronze"); + var bronze = combo("copper_ingot", "tin_ingot"); + addAlloy("bronze", "§6Bronze", bronze); + addAlloy("brass", "§eBrass", combo("copper_ingot", "zinc")); + KnownAlloyRecipes.learn(p.getUniqueId(), bronze); assertEquals( List.of("recipe", "recipes"), manager.onTabComplete(p, cmd(), "alloy", new String[] {"rec"})); @@ -240,114 +259,72 @@ void tabCompletionOffersOnlyOwnAlloys() { } @Test - void trackerReadsAreSafeAndReadOnly() { - var id = java.util.UUID.randomUUID(); - assertFalse(tracker.hasForged(null, "x")); - assertFalse(tracker.hasForged(id, null)); - assertFalse(tracker.hasForged(id, " ")); - assertTrue(tracker.getForgedIds(null).isEmpty()); - tracker.recordForge(id, "Zinc_Bronze"); - tracker.recordForge(id, "brass"); - assertTrue(tracker.hasForged(id, "ZINC_BRONZE")); - assertEquals(List.of("brass", "zinc_bronze"), List.copyOf(tracker.getForgedIds(id))); - assertThrows(UnsupportedOperationException.class, () -> tracker.getForgedIds(id).add("x")); - } - - java.util.List discoveries() { - var events = new java.util.ArrayList(); - server - .getPluginManager() - .registerEvent( - AlloyDiscoveredEvent.class, - new org.bukkit.event.Listener() {}, - org.bukkit.event.EventPriority.NORMAL, - (l, e) -> events.add(((AlloyDiscoveredEvent) e).getAlloyId()), - org.mockbukkit.mockbukkit.MockBukkit.createMockPlugin()); - return events; - } - - @Test - void namingTimeoutCreditsTheDiscovererIfOnline() throws Exception { - var online = server.addPlayer("Online"); - var offline = server.addPlayer("Offline"); - var events = discoveries(); - var method = - AlloyManager.class.getDeclaredMethod("namingTimedOut", java.util.UUID.class, String.class); - method.setAccessible(true); - tracker.recordForge(online.getUniqueId(), "generated"); - method.invoke(null, online.getUniqueId(), "Generated"); - assertTrue(online.nextMessage().contains("Naming timed out")); - assertTrue(tracker.hasForged(online.getUniqueId(), "generated")); - assertEquals(List.of("generated"), events); - offline.disconnect(); - method.invoke(null, offline.getUniqueId(), "other"); - assertFalse(tracker.hasForged(offline.getUniqueId(), "other")); - assertEquals(List.of("generated"), events); - CraftLifecycle.fireAlloyDiscovered(null, "x"); - CraftLifecycle.fireAlloyDiscovered(online, null); - CraftLifecycle.fireAlloyDiscovered(online, " "); + void knowledgeSurvivesRenamesAndIsSeededFromForgeHistory() throws Exception { + var smith = UUID.randomUUID(); + var bronze = combo("copper_ingot", "tin_ingot"); + var brass = combo("copper_ingot", "zinc"); + var bronzeAlloy = addAlloy("ferrum_vex", "§6Ferrum Vex", bronze); + addAlloy("brass", "§eBrass", brass); + addAlloy("no_recipe", "§7No Recipe", null); + tracker.recordForge(smith, "brass"); + tracker.recordForge(smith, "no_recipe"); + tracker.recordForge(smith, "deleted"); + KnownAlloyRecipes.learn(smith, bronze); + + // Seeded from the forge history on first use, then the new forge is added. + var file = temp.resolve("known-alloy-recipes/" + smith + ".json"); + assertEquals("[\"copper_ingot|tin_ingot\",\"copper_ingot|zinc\"]", Files.readString(file)); + + // Naming changes the id; the combo still matches. + when(bronzeAlloy.getId()).thenReturn("bright_bronze"); + assertTrue(known.knows(smith, bronzeAlloy)); + + // A fresh instance reads the file back instead of re-seeding. + tracker.recordForge(smith, "other"); + var reread = new KnownAlloyRecipes(temp.toFile()); + assertTrue(reread.knows(smith, AlloyManager.getAlloyById("brass"))); + + // Nothing on record: no file is written. + var newcomer = UUID.randomUUID(); + assertFalse(known.knows(newcomer, bronzeAlloy)); + assertFalse(Files.exists(temp.resolve("known-alloy-recipes/" + newcomer + ".json"))); CraftLifecycle.init(null); - CraftLifecycle.fireAlloyDiscovered(online, "untracked"); - CraftLifecycle.renameAlloy("a", "b"); - assertEquals(List.of("generated", "untracked"), events); - } - - @Test - void namingMovesEveryRecordToTheChosenId() throws Exception { - var finder = server.addPlayer("Finder"); - var copier = server.addPlayer("Copier"); - var events = discoveries(); - var m = new AlloyManager(); - var base = ingredient("iron", "base: true"); - var alloy = - spy( - new Alloy( - "ferrum_vex", - "Ferrum Vex", - new AlloyData( - base, - new net.tfminecraft.advancedcrafting.objects.data.StatData(), - new java.util.HashMap<>(), - null))); - doReturn(new ItemStack(Material.GOLD_INGOT)).when(alloy).build(); - AlloyManager.addAlloy(alloy); - tracker.recordForge(finder.getUniqueId(), "ferrum_vex"); - tracker.recordForge(copier.getUniqueId(), "ferrum_vex"); - tracker.recordForge(copier.getUniqueId(), "brass"); - var naming = AlloyManager.class.getDeclaredField("naming"); - naming.setAccessible(true); - @SuppressWarnings("unchecked") - var map = (java.util.Map) naming.get(m); - map.put( - finder, - new net.tfminecraft.advancedcrafting.objects.alloys.NamableAlloy( - alloy, new ItemStack(Material.IRON_INGOT))); - try (var db = mockConstruction(AlloyDatabase.class)) { - m.nameAlloy(finder, "Bright_Steel"); - } - assertEquals(List.of("bright_steel"), List.copyOf(tracker.getForgedIds(finder.getUniqueId()))); - assertEquals( - List.of("brass", "bright_steel"), List.copyOf(tracker.getForgedIds(copier.getUniqueId()))); - assertEquals(List.of("bright_steel"), events); + assertFalse(new KnownAlloyRecipes(temp.toFile()).knows(newcomer, bronzeAlloy)); } @Test - void renameSkipsBadInputAndForeignFiles() throws Exception { - var id = java.util.UUID.randomUUID(); - tracker.recordForge(id, "old"); - var notes = temp.resolve("forged-alloys/notes.json"); - java.nio.file.Files.writeString(notes, "[\"old\"]"); - java.nio.file.Files.writeString(temp.resolve("forged-alloys/readme.txt"), "x"); - for (String[] pair : - new String[][] {{null, "x"}, {"old", null}, {" ", "x"}, {"old", " "}, {"OLD", "old"}}) - tracker.renameAlloy(pair[0], pair[1]); - assertTrue(tracker.hasForged(id, "old")); - tracker.renameAlloy("Old", "New"); - assertTrue(tracker.hasForged(id, "new")); - assertFalse(tracker.hasForged(id, "old")); - assertEquals("[\"old\"]", java.nio.file.Files.readString(notes)); - var gone = new PlayerAlloyForgeTracker(temp.resolve("elsewhere").toFile()); - java.nio.file.Files.delete(temp.resolve("elsewhere/forged-alloys")); - gone.renameAlloy("a", "b"); + void knowledgeGuardsBadInputUnreadableFilesAndFailedSaves() throws Exception { + var id = UUID.randomUUID(); + var bronze = combo("copper_ingot", "tin_ingot"); + var alloy = addAlloy("bronze", "§6Bronze", bronze); + var noData = mock(Alloy.class); + assertFalse(known.knows(null, alloy)); + assertFalse(known.knows(id, null)); + assertFalse(known.knows(id, noData)); + KnownAlloyRecipes.learn(null, bronze); + KnownAlloyRecipes.learn(id, null); + assertFalse(Files.exists(temp.resolve("known-alloy-recipes/" + id + ".json"))); + + var broken = UUID.randomUUID(); + var brokenFile = temp.resolve("known-alloy-recipes/" + broken + ".json"); + Files.writeString(brokenFile, "{not json"); + assertFalse(known.knows(broken, alloy)); + assertEquals("{not json", Files.readString(brokenFile)); + + var odd = UUID.randomUUID(); + Files.writeString( + temp.resolve("known-alloy-recipes/" + odd + ".json"), + "[\"COPPER_INGOT|TIN_INGOT\", \" \", null]"); + assertTrue(known.knows(odd, alloy)); + var notArray = UUID.randomUUID(); + Files.writeString(temp.resolve("known-alloy-recipes/" + notArray + ".json"), "{}"); + assertFalse(known.knows(notArray, alloy)); + + var gone = new KnownAlloyRecipes(temp.resolve("elsewhere").toFile()); + Files.delete(temp.resolve("elsewhere/known-alloy-recipes")); + KnownAlloyRecipes.init(gone); + KnownAlloyRecipes.learn(id, bronze); + assertTrue(gone.knows(id, alloy)); + assertSame(gone, KnownAlloyRecipes.get()); } } diff --git a/src/test/java/net/tfminecraft/advancedcrafting/ForgerCoverageTest.java b/src/test/java/net/tfminecraft/advancedcrafting/ForgerCoverageTest.java index 3806524..94685bd 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/ForgerCoverageTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/ForgerCoverageTest.java @@ -152,6 +152,43 @@ void forgeReturnsScrapKnownAlloysNewDiscoveriesAndRecoversStaleIndexes() throws assertNull(forger(station, 0).forge(p, 101)); } + @Test + void successfulForgesTeachTheRecipeButScrapDoesNot() throws Exception { + var p = server.addPlayer(); + var loc = new Location(server.addSimpleWorld("world"), 0, 1, 2); + var station = new AlloyStation(loc); + station.addIngredient(ingredient("iron", "base: true")); + station.addIngredient(ingredient("copper", "")); + var knownRecipes = new net.tfminecraft.advancedcrafting.lifecycle.KnownAlloyRecipes(temp.toFile()); + net.tfminecraft.advancedcrafting.lifecycle.KnownAlloyRecipes.init(knownRecipes); + var alloy = mock(Alloy.class, RETURNS_DEEP_STUBS); + when(alloy.getId()).thenReturn("known"); + when(alloy.getData().getRecipe()).thenReturn(AlloyRecipe.fromStation(station)); + when(alloy.build()) + .thenAnswer(i -> new org.bukkit.inventory.ItemStack(org.bukkit.Material.IRON_INGOT)); + AlloyManager.addAlloy(alloy); + try { + try (var dbs = + mockConstruction( + AlloyDatabase.class, (db, ctx) -> when(db.getResult(station)).thenReturn("scrap"))) { + Cache.scrap = "MATERIAL.SCRAP"; + when(net.Indyuce.mmoitems.MMOItems.plugin.getItems().getMMOItem(any(), eq("SCRAP")).newBuilder().build()) + .thenAnswer(i -> new org.bukkit.inventory.ItemStack(org.bukkit.Material.IRON_NUGGET)); + assertNull(forger(station, 0).forge(p)); + } + assertFalse(knownRecipes.knows(p.getUniqueId(), alloy)); + try (var dbs = + mockConstruction( + AlloyDatabase.class, (db, ctx) -> when(db.getResult(station)).thenReturn("known"))) { + assertNull(forger(station, 0).forge(p)); + } + assertTrue(knownRecipes.knows(p.getUniqueId(), alloy)); + } finally { + net.tfminecraft.advancedcrafting.lifecycle.KnownAlloyRecipes.init(null); + AlloyManager.removeAlloy("known"); + } + } + @Test void deterministicMergeCoversGuaranteedInheritanceCapsAndProtectedStats() throws Exception { Cache.maxFactor = 1.5; From 4a2a6ba9996aaee41b7dd8f4f93a20fd1d1ce5fb Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:29:09 +0100 Subject: [PATCH 4/6] Keep unreadable recipe files, write atomically, retry failed saves Co-Authored-By: Claude Opus 5.5 --- .../lifecycle/KnownAlloyRecipes.java | 38 +++++++++++++++---- .../lifecycle/PlayerAlloyForgeTracker.java | 3 +- .../managers/CommandManager.java | 2 +- .../AlloyRecipeCommandTest.java | 19 ++++++++++ .../advancedcrafting/CommandCoverageTest.java | 2 +- 5 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java index bbcc4ed..dcf86da 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java @@ -4,8 +4,11 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStreamReader; -import java.io.PrintWriter; import java.nio.charset.StandardCharsets; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -33,6 +36,8 @@ public final class KnownAlloyRecipes { private final File folder; private final Map> cache = new HashMap<>(); + /** Accounts whose file could not be read; it is never overwritten this session. */ + private final Set unreadable = new HashSet<>(); public KnownAlloyRecipes(File dataFolder) { this.folder = new File(dataFolder, "known-alloy-recipes"); @@ -66,8 +71,9 @@ public boolean knows(UUID playerUuid, Alloy alloy) { private void add(UUID playerUuid, String comboKey) { Set known = load(playerUuid); - if (known.add(comboKey)) { - save(playerUuid, known); + if (known.add(comboKey) && !save(playerUuid, known)) { + // Not on disk, so the next forge of this combo tries again. + known.remove(comboKey); } } @@ -89,8 +95,10 @@ private Set load(UUID playerUuid) { } } } catch (Exception ex) { - // Keep the unreadable file untouched; this session just starts from what was read. - AdvancedCrafting.plugin.getLogger().warning("AC: could not read " + file.getName() + ": " + ex.getMessage()); + // Leave the file for staff to repair; this session works from memory only. + AdvancedCrafting.plugin.getLogger().warning("AC: could not read " + file.getPath() + ", leaving it unchanged: " + + ex.getMessage()); + unreadable.add(playerUuid); cache.put(playerUuid, known); return known; } @@ -116,17 +124,31 @@ private boolean seedFromForgeHistory(UUID playerUuid, Set known) { return !known.isEmpty(); } + /** Writes a temp file and moves it over the old one, so a crash can't leave half a file. */ @SuppressWarnings("unchecked") - private void save(UUID playerUuid, Set known) { + private boolean save(UUID playerUuid, Set known) { + if (unreadable.contains(playerUuid)) { + return true; + } List sorted = new ArrayList<>(known); sorted.sort(null); JSONArray array = new JSONArray(); array.addAll(sorted); - try (PrintWriter writer = new PrintWriter(file(playerUuid), StandardCharsets.UTF_8)) { - writer.print(array.toJSONString()); + Path target = file(playerUuid).toPath(); + Path temp = target.resolveSibling(target.getFileName() + ".tmp"); + try { + Files.createDirectories(folder.toPath()); + Files.writeString(temp, array.toJSONString(), StandardCharsets.UTF_8); + try { + Files.move(temp, target, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); + } catch (AtomicMoveNotSupportedException ex) { + Files.move(temp, target, StandardCopyOption.REPLACE_EXISTING); + } + return true; } catch (IOException ex) { AdvancedCrafting.plugin.getLogger().warning("AC: could not save known alloy recipes for " + playerUuid + ": " + ex.getMessage()); + return false; } } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java index 8e602bc..79191a9 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/PlayerAlloyForgeTracker.java @@ -7,7 +7,6 @@ import java.io.PrintWriter; import java.util.HashSet; import java.util.ArrayList; -import java.util.Collections; import java.util.Locale; import java.util.Set; import java.util.UUID; @@ -52,7 +51,7 @@ public Set getForgedIds(UUID playerUuid) { if (playerUuid == null) { return Set.of(); } - return Collections.unmodifiableSet(new HashSet<>(loadForgedIds(playerUuid))); + return Set.copyOf(loadForgedIds(playerUuid)); } private Set loadForgedIds(UUID playerUuid) { diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java index c3e671c..652492b 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/CommandManager.java @@ -56,7 +56,7 @@ public boolean onCommand(CommandSender sender, Command cmd, String label, String default -> { if (args.length < 2) { p.sendMessage("§cNo name specified, format: /alloy name newname"); - return false; + return true; } AdvancedCrafting.getAlloyManager().nameAlloy(p, args[1]); } diff --git a/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java b/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java index 7a16291..37420a0 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java @@ -309,6 +309,8 @@ void knowledgeGuardsBadInputUnreadableFilesAndFailedSaves() throws Exception { var brokenFile = temp.resolve("known-alloy-recipes/" + broken + ".json"); Files.writeString(brokenFile, "{not json"); assertFalse(known.knows(broken, alloy)); + KnownAlloyRecipes.learn(broken, bronze); + assertTrue(known.knows(broken, alloy)); assertEquals("{not json", Files.readString(brokenFile)); var odd = UUID.randomUUID(); @@ -320,11 +322,28 @@ void knowledgeGuardsBadInputUnreadableFilesAndFailedSaves() throws Exception { Files.writeString(temp.resolve("known-alloy-recipes/" + notArray + ".json"), "{}"); assertFalse(known.knows(notArray, alloy)); + // The folder is recreated if it was removed. var gone = new KnownAlloyRecipes(temp.resolve("elsewhere").toFile()); Files.delete(temp.resolve("elsewhere/known-alloy-recipes")); KnownAlloyRecipes.init(gone); KnownAlloyRecipes.learn(id, bronze); assertTrue(gone.knows(id, alloy)); + assertTrue(Files.exists(temp.resolve("elsewhere/known-alloy-recipes/" + id + ".json"))); assertSame(gone, KnownAlloyRecipes.get()); + + // A save that fails is not kept, so the next forge tries again. + var blocked = UUID.randomUUID(); + var blockedPath = temp.resolve("known-alloy-recipes/" + blocked + ".json"); + var tempPath = temp.resolve("known-alloy-recipes/" + blocked + ".json.tmp"); + Files.createDirectories(tempPath.resolve("inside")); + KnownAlloyRecipes.init(known); + KnownAlloyRecipes.learn(blocked, bronze); + assertFalse(known.knows(blocked, alloy)); + assertFalse(Files.exists(blockedPath)); + Files.delete(tempPath.resolve("inside")); + Files.delete(tempPath); + KnownAlloyRecipes.learn(blocked, bronze); + assertTrue(known.knows(blocked, alloy)); + assertEquals("[\"copper_ingot|tin_ingot\"]", Files.readString(blockedPath)); } } diff --git a/src/test/java/net/tfminecraft/advancedcrafting/CommandCoverageTest.java b/src/test/java/net/tfminecraft/advancedcrafting/CommandCoverageTest.java index 2567485..099861b 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/CommandCoverageTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/CommandCoverageTest.java @@ -77,7 +77,7 @@ void dispatchRequiresAdminAndPlayerWhereApplicable() throws Exception { verify(crafting).setAdminCraftPending(p, 100); assertFalse(manager.onCommand(p, cmd("alloy"), "alloy", new String[] {})); assertFalse(manager.onCommand(p, cmd("alloy"), "alloy", new String[] {"other"})); - assertFalse(manager.onCommand(p, cmd("alloy"), "alloy", new String[] {"name"})); + assertTrue(manager.onCommand(p, cmd("alloy"), "alloy", new String[] {"name"})); assertTrue(manager.onCommand(p, cmd("alloy"), "alloy", new String[] {"name", "steel"})); verify(alloys).nameAlloy(p, "steel"); } From ba7b43a335f7a22c4b32e59f3ee509b75b1ac58b Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:50:15 +0100 Subject: [PATCH 5/6] Cover the remaining branches for the 100% coverage gate Co-Authored-By: Claude Opus 5.5 --- .../managers/AlloyRecipeCommand.java | 6 ++--- .../AlloyRecipeCommandTest.java | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java index 77ca6f7..a15a96a 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/managers/AlloyRecipeCommand.java @@ -131,10 +131,8 @@ static String ingredientName(String id) { Ingredient ingredient = IngredientLoader.getByString(id); if (ingredient != null) { try { - ItemStack item = ingredient.build(); - if (item != null && item.hasItemMeta() && item.getItemMeta().hasDisplayName()) { - return item.getItemMeta().getDisplayName(); - } + // Ingredient.buildTo always gives the item a display name. + return ingredient.build().getItemMeta().getDisplayName(); } catch (RuntimeException ex) { // The item source (MMOItems, ItemsAdder) may be missing; fall back to the id. } diff --git a/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java b/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java index 37420a0..7854b15 100644 --- a/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java +++ b/src/test/java/net/tfminecraft/advancedcrafting/AlloyRecipeCommandTest.java @@ -253,6 +253,8 @@ void tabCompletionOffersOnlyOwnAlloys() { List.of("bronze"), manager.onTabComplete(p, cmd(), "alloy", new String[] {"recipe", "br"})); assertTrue(manager.onTabComplete(p, cmd(), "alloy", new String[] {"recipes", ""}).isEmpty()); + assertTrue( + manager.onTabComplete(p, cmd(), "alloy", new String[] {"recipe", "bronze", ""}).isEmpty()); var console = mock(CommandSender.class); assertTrue( manager.onTabComplete(console, cmd(), "alloy", new String[] {"recipe", ""}).isEmpty()); @@ -266,10 +268,16 @@ void knowledgeSurvivesRenamesAndIsSeededFromForgeHistory() throws Exception { var bronzeAlloy = addAlloy("ferrum_vex", "§6Ferrum Vex", bronze); addAlloy("brass", "§eBrass", brass); addAlloy("no_recipe", "§7No Recipe", null); + var noData = mock(Alloy.class); + when(noData.getId()).thenReturn("no_data"); + AlloyManager.addAlloy(noData); + tracker.recordForge(smith, "no_data"); + assertTrue(tracker.getForgedIds(null).isEmpty()); tracker.recordForge(smith, "brass"); tracker.recordForge(smith, "no_recipe"); tracker.recordForge(smith, "deleted"); KnownAlloyRecipes.learn(smith, bronze); + KnownAlloyRecipes.learn(smith, bronze); // Seeded from the forge history on first use, then the new forge is added. var file = temp.resolve("known-alloy-recipes/" + smith + ".json"); @@ -345,5 +353,23 @@ void knowledgeGuardsBadInputUnreadableFilesAndFailedSaves() throws Exception { KnownAlloyRecipes.learn(blocked, bronze); assertTrue(known.knows(blocked, alloy)); assertEquals("[\"copper_ingot|tin_ingot\"]", Files.readString(blockedPath)); + + // A file system without atomic moves still gets the file, with a plain replace. + var plainMove = UUID.randomUUID(); + try (var files = mockStatic(Files.class, CALLS_REAL_METHODS)) { + files + .when( + () -> + Files.move( + any(java.nio.file.Path.class), + any(java.nio.file.Path.class), + eq(java.nio.file.StandardCopyOption.REPLACE_EXISTING), + eq(java.nio.file.StandardCopyOption.ATOMIC_MOVE))) + .thenThrow(new java.nio.file.AtomicMoveNotSupportedException("a", "b", "test")); + KnownAlloyRecipes.learn(plainMove, bronze); + } + assertEquals( + "[\"copper_ingot|tin_ingot\"]", + Files.readString(temp.resolve("known-alloy-recipes/" + plainMove + ".json"))); } } From 74acb6c4d9946ecfac47752fdf288e7ebe3d097f Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Tue, 3 Nov 2026 20:57:46 +0100 Subject: [PATCH 6/6] Lowercase alloy recipe keys with Locale.ROOT Co-Authored-By: Claude Opus 5.5 --- .../advancedcrafting/lifecycle/KnownAlloyRecipes.java | 3 ++- .../advancedcrafting/objects/data/AlloyRecipe.java | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java index dcf86da..545623f 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/lifecycle/KnownAlloyRecipes.java @@ -13,6 +13,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.UUID; @@ -90,7 +91,7 @@ private Set load(UUID playerUuid) { if (parsed instanceof JSONArray array) { for (Object entry : array) { if (entry != null && !entry.toString().isBlank()) { - known.add(entry.toString().trim().toLowerCase()); + known.add(entry.toString().trim().toLowerCase(Locale.ROOT)); } } } diff --git a/src/main/java/net/tfminecraft/advancedcrafting/objects/data/AlloyRecipe.java b/src/main/java/net/tfminecraft/advancedcrafting/objects/data/AlloyRecipe.java index 28fcc0c..9ab4c23 100644 --- a/src/main/java/net/tfminecraft/advancedcrafting/objects/data/AlloyRecipe.java +++ b/src/main/java/net/tfminecraft/advancedcrafting/objects/data/AlloyRecipe.java @@ -5,6 +5,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.Objects; import net.tfminecraft.advancedcrafting.objects.alloys.AlloyStation; @@ -15,10 +16,10 @@ public class AlloyRecipe { private final List catalystIds; public AlloyRecipe(String baseId, List catalystIds) { - this.baseId = baseId.toLowerCase(); + this.baseId = baseId.toLowerCase(Locale.ROOT); List sorted = new ArrayList<>(catalystIds); for (int i = 0; i < sorted.size(); i++) { - sorted.set(i, sorted.get(i).toLowerCase()); + sorted.set(i, sorted.get(i).toLowerCase(Locale.ROOT)); } Collections.sort(sorted); this.catalystIds = Collections.unmodifiableList(sorted); @@ -69,7 +70,7 @@ public static AlloyRecipe fromComboKey(String comboKey) { if (comboKey == null || comboKey.isBlank()) { return null; } - String normalized = comboKey.toLowerCase(); + String normalized = comboKey.toLowerCase(Locale.ROOT); int pipe = normalized.indexOf('|'); if (pipe < 0) { return new AlloyRecipe(normalized, List.of());