Cleans up salvage code a bit

src/main/java/net/knarcraft/blacksmith/container/SalvageResult.java

@@ -0,0 +1,12 @@
+package net.knarcraft.blacksmith.container;
+
+import net.knarcraft.blacksmith.property.SalvageMethod;
+import net.knarcraft.blacksmith.property.SalvageState;
+import org.bukkit.inventory.ItemStack;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.List;
+
+public record SalvageResult(@NotNull SalvageMethod salvageMethod, @NotNull List<ItemStack> salvage,
+                            @NotNull SalvageState salvageState) {
+}

src/main/java/net/knarcraft/blacksmith/manager/EconomyManager.java

@@ -1,9 +1,9 @@
 package net.knarcraft.blacksmith.manager;
 
 import net.knarcraft.blacksmith.BlacksmithPlugin;
-import net.knarcraft.blacksmith.config.SalvageMethod;
 import net.knarcraft.blacksmith.config.blacksmith.GlobalBlacksmithSettings;
 import net.knarcraft.blacksmith.config.scrapper.GlobalScrapperSettings;
+import net.knarcraft.blacksmith.property.SalvageMethod;
 import net.knarcraft.blacksmith.util.ItemHelper;
 import net.milkbowl.vault.economy.Economy;
 import org.bukkit.Material;

src/main/java/net/knarcraft/blacksmith/property/SalvageMethod.java

@@ -1,4 +1,4 @@
-package net.knarcraft.blacksmith.config;
+package net.knarcraft.blacksmith.property;
 
 /**
  * A representation of the different ways an item can be salvaged

src/main/java/net/knarcraft/blacksmith/property/SalvageState.java

@@ -0,0 +1,23 @@
+package net.knarcraft.blacksmith.property;
+
+/**
+ * The state of trying to find salvage for an item
+ */
+public enum SalvageState {
+
+    /**
+     * Found useful salvage that can be given to a player
+     */
+    FOUND_SALVAGE,
+
+    /**
+     * The item cannot be salvaged using the used method
+     */
+    INCORRECT_METHOD,
+
+    /**
+     * While the method was correct, no useful salvage was created
+     */
+    NO_SALVAGE,
+
+}

src/main/java/net/knarcraft/blacksmith/trait/SalvageSession.java

@@ -2,11 +2,11 @@ package net.knarcraft.blacksmith.trait;
 
 import net.citizensnpcs.api.npc.NPC;
 import net.knarcraft.blacksmith.BlacksmithPlugin;
-import net.knarcraft.blacksmith.config.SalvageMethod;
 import net.knarcraft.blacksmith.config.scrapper.ScrapperNPCSettings;
 import net.knarcraft.blacksmith.event.ScrapperSalvageFailEvent;
 import net.knarcraft.blacksmith.event.ScrapperSalvageSucceedEvent;
 import net.knarcraft.blacksmith.manager.EconomyManager;
+import net.knarcraft.blacksmith.property.SalvageMethod;
 import net.knarcraft.blacksmith.util.ItemHelper;
 import net.knarcraft.blacksmith.util.SalvageHelper;
 import org.bukkit.Sound;

src/main/java/net/knarcraft/blacksmith/trait/ScrapperTrait.java

@@ -2,12 +2,14 @@ package net.knarcraft.blacksmith.trait;
 
 import net.citizensnpcs.api.util.DataKey;
 import net.knarcraft.blacksmith.BlacksmithPlugin;
-import net.knarcraft.blacksmith.config.SalvageMethod;
 import net.knarcraft.blacksmith.config.SmithPreset;
 import net.knarcraft.blacksmith.config.SmithPresetFilter;
 import net.knarcraft.blacksmith.config.scrapper.ScrapperNPCSettings;
 import net.knarcraft.blacksmith.config.scrapper.ScrapperSetting;
+import net.knarcraft.blacksmith.container.SalvageResult;
 import net.knarcraft.blacksmith.manager.EconomyManager;
+import net.knarcraft.blacksmith.property.SalvageMethod;
+import net.knarcraft.blacksmith.property.SalvageState;
 import net.knarcraft.blacksmith.util.ItemHelper;
 import net.knarcraft.blacksmith.util.SalvageHelper;
 import net.knarcraft.knarlib.formatting.StringFormatter;
@@ -20,6 +22,7 @@ import org.bukkit.inventory.meta.ArmorMeta;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -92,53 +95,22 @@ public class ScrapperTrait extends CustomTrait<ScrapperSetting> {
         }
 
         List<ItemStack> salvage = null;
+        SalvageMethod salvageMethod = null;
 
-        // Deal with armor trim salvage
+        List<SalvageResult> results = List.of(isArmorTrimSalvage(player, itemInHand),
-        SalvageMethod salvageMethod = SalvageMethod.SALVAGE;
+                isNetheriteSalvage(player, itemInHand), isNormalSalvage(player, itemInHand, extended));
-        if (itemInHand.getItemMeta() instanceof ArmorMeta armorMeta && armorMeta.hasTrim()) {
+
-            if (!getSettings().salvageArmorTrims()) {
+        for (SalvageResult result : results) {
-                sendNPCMessage(this.npc, player, getSettings().getCannotSalvageArmorTrimMessage());
+            if (result.salvageState() == SalvageState.NO_SALVAGE) {
+                // No salvage means that the method is correct, but something prevented useful salvage from being returned
                 return;
+            } else if (result.salvageState() == SalvageState.FOUND_SALVAGE) {
+                // Successfully found useful salvage
+                salvage = result.salvage();
+                salvageMethod = result.salvageMethod();
             }
-            salvage = SalvageHelper.getArmorTrimSalvage(itemInHand, armorMeta);
-            if (salvage == null) {
-                sendNPCMessage(this.npc, player, getSettings().getArmorTrimSalvageNotFoundMessage());
-                return;
-            }
-            salvageMethod = SalvageMethod.ARMOR_TRIM;
         }
-
+        if (salvage == null || salvage.isEmpty()) {
-        // Remove the netherite ingot from the item
-        if (salvage == null && SmithPreset.BLACKSMITH.getFilteredMaterials(SmithPresetFilter.NETHERITE).contains(itemInHand.getType())) {
-            if (!getSettings().salvageNetherite()) {
-                sendNPCMessage(this.npc, player, getSettings().getCannotSalvageNetheriteMessage());
-                return;
-            }
-
-            salvage = SalvageHelper.getNetheriteSalvage(itemInHand);
-            salvageMethod = SalvageMethod.NETHERITE;
-        }
-
-        // As there is no recipe for netherite items, the check needs to be after the netherite salvage check
-        if (salvageMethod == SalvageMethod.SALVAGE && !SalvageHelper.isSalvageable(player.getServer(), itemInHand)) {
-            sendNPCMessage(this.npc, player, StringFormatter.replacePlaceholder(getSettings().getInvalidItemMessage(),
-                    "{title}", getSettings().getScrapperTitle()));
-            return;
-        }
-
-        // Check if the item is enchanted, and whether this blacksmith can salvage it
-        if (!itemInHand.getEnchantments().isEmpty() && !getSettings().salvageEnchanted()) {
-            sendNPCMessage(this.npc, player, getSettings().getCannotSalvageEnchantedMessage());
-            return;
-        }
-
-        // Check if any salvage will be produced
-        if (salvage == null) {
-            salvage = getSalvage(itemInHand, extended);
-        }
-        boolean noUsefulSalvage = salvage == null || salvage.isEmpty();
-        if (noUsefulSalvage) {
-            sendNPCMessage(this.npc, player, getSettings().getTooDamagedMessage());
             return;
         }
 
@@ -150,6 +122,93 @@ public class ScrapperTrait extends CustomTrait<ScrapperSetting> {
         printCostMessage(player, itemInHand, EconomyManager.formatSalvageCost(salvageMethod), salvageMethod);
     }
 
+    /**
+     * Gets the result of trying to salvage the item normally
+     *
+     * @param player     <p>The player trying to salvage the item</p>
+     * @param itemInHand <p>The item to be salvaged</p>
+     * @param extended   <p>Whether extended salvage is enabled</p>
+     * @return <p>The result of attempting the salvage</p>
+     */
+    private SalvageResult isNormalSalvage(@NotNull Player player, @NotNull ItemStack itemInHand, boolean extended) {
+
+        // As there is no recipe for netherite items, the check needs to be after the netherite salvage check
+        if (!SalvageHelper.isSalvageable(player.getServer(), itemInHand)) {
+            sendNPCMessage(this.npc, player, StringFormatter.replacePlaceholder(getSettings().getInvalidItemMessage(),
+                    "{title}", getSettings().getScrapperTitle()));
+            return new SalvageResult(SalvageMethod.SALVAGE, new ArrayList<>(), SalvageState.NO_SALVAGE);
+        }
+
+        // Check if the item is enchanted, and whether this blacksmith can salvage it
+        if (!itemInHand.getEnchantments().isEmpty() && !getSettings().salvageEnchanted()) {
+            sendNPCMessage(this.npc, player, getSettings().getCannotSalvageEnchantedMessage());
+            return new SalvageResult(SalvageMethod.SALVAGE, new ArrayList<>(), SalvageState.NO_SALVAGE);
+        }
+
+        // Check if any salvage will be produced
+        List<ItemStack> salvage = getSalvage(itemInHand, extended);
+        boolean noUsefulSalvage = salvage == null || salvage.isEmpty();
+        if (noUsefulSalvage) {
+            sendNPCMessage(this.npc, player, getSettings().getTooDamagedMessage());
+            return new SalvageResult(SalvageMethod.SALVAGE, new ArrayList<>(), SalvageState.NO_SALVAGE);
+        } else {
+            return new SalvageResult(SalvageMethod.SALVAGE, salvage, SalvageState.FOUND_SALVAGE);
+        }
+    }
+
+    /**
+     * Gets the result of trying to salvage the item as a netherite applied item
+     *
+     * @param player     <p>The player trying to salvage the item</p>
+     * @param itemInHand <p>The item to be salvaged</p>
+     * @return <p>The result of attempting the salvage</p>
+     */
+    @NotNull
+    private SalvageResult isNetheriteSalvage(@NotNull Player player, @NotNull ItemStack itemInHand) {
+        if (!SmithPreset.BLACKSMITH.getFilteredMaterials(SmithPresetFilter.NETHERITE).contains(itemInHand.getType())) {
+            return new SalvageResult(SalvageMethod.NETHERITE, new ArrayList<>(), SalvageState.INCORRECT_METHOD);
+        }
+
+        if (!getSettings().salvageNetherite()) {
+            sendNPCMessage(this.npc, player, getSettings().getCannotSalvageNetheriteMessage());
+            return new SalvageResult(SalvageMethod.NETHERITE, new ArrayList<>(), SalvageState.NO_SALVAGE);
+        }
+
+        List<ItemStack> salvage = SalvageHelper.getNetheriteSalvage(itemInHand);
+        if (salvage == null || salvage.isEmpty()) {
+            return new SalvageResult(SalvageMethod.NETHERITE, new ArrayList<>(), SalvageState.NO_SALVAGE);
+        }
+
+        return new SalvageResult(SalvageMethod.NETHERITE, salvage, SalvageState.FOUND_SALVAGE);
+    }
+
+    /**
+     * Gets the result of trying to salvage the item as an armor trim
+     *
+     * @param player     <p>The player trying to salvage the item</p>
+     * @param itemInHand <p>The item to be salvaged</p>
+     * @return <p>The result of attempting the salvage</p>
+     */
+    @NotNull
+    private SalvageResult isArmorTrimSalvage(@NotNull Player player, @NotNull ItemStack itemInHand) {
+        if (!(itemInHand.getItemMeta() instanceof ArmorMeta armorMeta) || !armorMeta.hasTrim()) {
+            return new SalvageResult(SalvageMethod.ARMOR_TRIM, new ArrayList<>(), SalvageState.INCORRECT_METHOD);
+        }
+
+        if (!getSettings().salvageArmorTrims()) {
+            sendNPCMessage(this.npc, player, getSettings().getCannotSalvageArmorTrimMessage());
+            return new SalvageResult(SalvageMethod.ARMOR_TRIM, new ArrayList<>(), SalvageState.NO_SALVAGE);
+        }
+
+        List<ItemStack> salvage = SalvageHelper.getArmorTrimSalvage(itemInHand, armorMeta);
+        if (salvage == null || salvage.isEmpty()) {
+            sendNPCMessage(this.npc, player, getSettings().getArmorTrimSalvageNotFoundMessage());
+            return new SalvageResult(SalvageMethod.ARMOR_TRIM, new ArrayList<>(), SalvageState.NO_SALVAGE);
+        }
+
+        return new SalvageResult(SalvageMethod.ARMOR_TRIM, salvage, SalvageState.FOUND_SALVAGE);
+    }
+
     /**
      * Prints a message to the given player, explaining the cost of salvaging the held item
      *

src/main/java/net/knarcraft/blacksmith/util/SalvageHelper.java

@@ -1,5 +1,6 @@
 package net.knarcraft.blacksmith.util;
 
+import org.bukkit.Bukkit;
 import org.bukkit.Material;
 import org.bukkit.Server;
 import org.bukkit.enchantments.Enchantment;
@@ -172,37 +173,26 @@ public final class SalvageHelper {
         }
 
         for (Recipe recipe : server.getRecipesFor(new ItemStack(salvagedItem.getType(), salvagedItem.getAmount()))) {
-            if (recipe instanceof ShapedRecipe || recipe instanceof ShapelessRecipe) {
+            // Only consider crafting table recipes
-                List<ItemStack> salvage = getRecipeSalvage(recipe, salvagedItem, trashSalvage);
+            if (!(recipe instanceof ShapedRecipe) && !(recipe instanceof ShapelessRecipe)) {
-                if (salvage != null && !salvage.isEmpty()) {
+                continue;
-                    return salvage;
+            }
-                }
+
+            // Make sure the player has enough items
+            if (salvagedItem.getAmount() < getRequiredAmountForSalvage(Bukkit.getServer(), salvagedItem)) {
+                continue;
+            }
+
+            // Get actual salvage, as long as any can be produced
+            List<ItemStack> salvage = getRecipeSalvage(recipe, salvagedItem, trashSalvage);
+            if (salvage != null && !salvage.isEmpty()) {
+                return salvage;
             }
         }
 
         return null;
     }
 
-    /**
-     * Gets the raw salvage of a recipe, assuming full salvage would be possible
-     *
-     * @param recipe <p>The recipe to get salvage for</p>
-     * @return <p>The salvage resulting from the recipe</p>
-     */
-    private static List<ItemStack> getRawRecipeSalvage(@NotNull Recipe recipe) {
-        List<ItemStack> ingredients;
-        if (recipe instanceof ShapedRecipe shapedRecipe) {
-            ingredients = getIngredients(shapedRecipe);
-        } else if (recipe instanceof ShapelessRecipe shapelessRecipe) {
-            ingredients = shapelessRecipe.getIngredientList();
-        } else {
-            //Recipes other than crafting shouldn't be considered for salvaging
-            return null;
-        }
-        //Make things easier by eliminating identical stacks
-        return combineStacks(ingredients);
-    }
-
     /**
      * Gets the salvage resulting from the given recipe and the given item
      *
@@ -252,7 +242,7 @@ public final class SalvageHelper {
         int maxDurability = ItemHelper.getMaxDurability(salvagedItem);
 
         // Prevent divide by zero for items that don't have a set max durability
-        if (maxDurability == 0) {
+        if (maxDurability <= 0) {
             maxDurability = 1;
             durability = 1;
         }
@@ -348,6 +338,26 @@ public final class SalvageHelper {
         return combined;
     }
 
+    /**
+     * Gets the raw salvage of a recipe, assuming full salvage would be possible
+     *
+     * @param recipe <p>The recipe to get salvage for</p>
+     * @return <p>The salvage resulting from the recipe</p>
+     */
+    private static List<ItemStack> getRawRecipeSalvage(@NotNull Recipe recipe) {
+        List<ItemStack> ingredients;
+        if (recipe instanceof ShapedRecipe shapedRecipe) {
+            ingredients = getIngredients(shapedRecipe);
+        } else if (recipe instanceof ShapelessRecipe shapelessRecipe) {
+            ingredients = shapelessRecipe.getIngredientList();
+        } else {
+            //Recipes other than crafting shouldn't be considered for salvaging
+            return null;
+        }
+        //Make things easier by eliminating identical stacks
+        return combineStacks(ingredients);
+    }
+
     /**
      * Gets all ingredients contained in the given shaped recipe
      *