Rewrites a lot of scrapper code to fix actual and potential bugs

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

@@ -0,0 +1,16 @@
+package net.knarcraft.blacksmith.container;
+
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.Recipe;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.List;
+
+/**
+ * The result of a recipe's salvage
+ *
+ * @param recipe  <p>The selected recipe</p>
+ * @param salvage <p>The resulting salvage</p>
+ */
+public record RecipeResult(@NotNull Recipe recipe, @NotNull List<ItemStack> salvage) {
+}

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

@@ -7,6 +7,14 @@ import org.jetbrains.annotations.NotNull;
 
 import java.util.List;
 
+/**
+ * The result of an attempted salvage
+ *
+ * @param salvageMethod  <p>The salvage method used</p>
+ * @param salvage        <p>The produced salvage</p>
+ * @param salvageState   <p>The state of the salvage result</p>
+ * @param requiredAmount <p>The amount of items required for the salvage</p>
+ */
 public record SalvageResult(@NotNull SalvageMethod salvageMethod, @NotNull List<ItemStack> salvage,
-                            @NotNull SalvageState salvageState) {
+                            @NotNull SalvageState salvageState, int requiredAmount) {
 }

src/main/java/net/knarcraft/blacksmith/listener/NPCClickListener.java

@@ -21,7 +21,7 @@ public class NPCClickListener implements Listener {
 
     @EventHandler
     public void onRightClick(@NotNull NPCRightClickEvent event) {
-        //We only care about blacksmiths
+        //We only care about blacksmiths and scrappers
         if (event.getNPC().hasTrait(BlacksmithTrait.class)) {
             handleNPCClick(event, event.getNPC().getTraitNullable(BlacksmithTrait.class));
         } else if (event.getNPC().hasTrait(ScrapperTrait.class)) {

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

@@ -7,7 +7,6 @@ import net.knarcraft.blacksmith.config.Settings;
 import net.knarcraft.blacksmith.config.TraitSettings;
 import net.knarcraft.blacksmith.formatting.TimeFormatter;
 import net.knarcraft.blacksmith.manager.EconomyManager;
-import net.knarcraft.blacksmith.util.SalvageHelper;
 import net.knarcraft.knarlib.formatting.StringFormatter;
 import org.bukkit.entity.LivingEntity;
 import org.bukkit.entity.Player;
@@ -196,9 +195,9 @@ public abstract class CustomTrait<K extends Setting> extends Trait {
             Objects.requireNonNull(((LivingEntity) npc.getEntity()).getEquipment()).setItemInMainHand(heldItem);
         }
         //Remove the item from the player's inventory
-        if (!isBlacksmith) {
+        if (this.session instanceof SalvageSession salvageSession) {
             // For scrappers, just reduce the amounts of items, unless the remaining stack is salvaged
-            int amount = SalvageHelper.getRequiredAmountForSalvage(player.getServer(), heldItem);
+            int amount = salvageSession.getItemsConsumed();
             if (amount != heldItem.getAmount()) {
                 heldItem.setAmount(heldItem.getAmount() - amount);
                 playerInventory.setItemInMainHand(heldItem);

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

@@ -20,6 +20,7 @@ import java.util.Calendar;
 import java.util.List;
 import java.util.Objects;
 import java.util.Random;
+import java.util.logging.Level;
 
 import static net.knarcraft.blacksmith.formatting.BlacksmithStringFormatter.sendNPCMessage;
 
@@ -47,9 +48,9 @@ public class SalvageSession extends Session implements Runnable {
      * @param salvageMethod <p>The salvage method performed in this session</p>
      * @param itemsConsumed <p>The number of items actually consumed, in case a salvaging fails</p>
      */
-    SalvageSession(@NotNull ScrapperTrait scrapperTrait, @NotNull Player player, @NotNull NPC npc,
+    public SalvageSession(@NotNull ScrapperTrait scrapperTrait, @NotNull Player player, @NotNull NPC npc,
-                   @NotNull ScrapperNPCSettings config, @NotNull List<ItemStack> salvage,
+                          @NotNull ScrapperNPCSettings config, @NotNull List<ItemStack> salvage,
-                   @NotNull SalvageMethod salvageMethod, int itemsConsumed) {
+                          @NotNull SalvageMethod salvageMethod, int itemsConsumed) {
         super(player, npc);
         this.scrapperTrait = scrapperTrait;
         this.itemToSalvage = player.getInventory().getItemInMainHand().clone();
@@ -110,6 +111,15 @@ public class SalvageSession extends Session implements Runnable {
         this.scrapperTrait.addCoolDown(this.player.getUniqueId(), wait);
     }
 
+    /**
+     * Gets the number of items consumed in order to perform the salvage
+     *
+     * @return <p>The number of items consumed as part of this salvage</p>
+     */
+    public int getItemsConsumed() {
+        return this.itemsConsumed;
+    }
+
     /**
      * Trues to salvage an item, and gives the return item
      */
@@ -156,6 +166,7 @@ public class SalvageSession extends Session implements Runnable {
         // TODO: Find a better calculation than 1 enchantment level = 1 exp level
         // Gives the player back some of the EXP used on an item
         this.player.giveExpLevels(this.enchantmentLevels);
+        BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Giving salvage " + this.salvage);
         for (ItemStack item : this.salvage) {
             giveResultingItem(this.config.getMaxSalvageDelay() > 0, this.config.getDropItem(), this.npc, item);
         }

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

@@ -6,6 +6,7 @@ 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.RecipeResult;
 import net.knarcraft.blacksmith.container.SalvageResult;
 import net.knarcraft.blacksmith.manager.EconomyManager;
 import net.knarcraft.blacksmith.property.SalvageMethod;
@@ -26,6 +27,7 @@ import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.logging.Level;
 
 import static net.knarcraft.blacksmith.formatting.BlacksmithStringFormatter.sendNPCMessage;
 
@@ -91,35 +93,57 @@ public class ScrapperTrait extends CustomTrait<ScrapperSetting> {
         if (!canBeSalvaged(itemInHand, salvageAbleItems, extended)) {
             sendNPCMessage(this.npc, player, StringFormatter.replacePlaceholder(getSettings().getInvalidItemMessage(),
                     "{title}", getSettings().getScrapperTitle()));
+            BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Cannot salvage provided item: " + itemInHand);
             return;
         }
 
-        List<ItemStack> salvage = null;
+        SalvageResult result = getBestResult(player, itemInHand, extended);
-        SalvageMethod salvageMethod = null;
 
-        List<SalvageResult> results = List.of(isArmorTrimSalvage(player, itemInHand),
+        if (result == null || result.salvage().isEmpty()) {
-                isNetheriteSalvage(player, itemInHand), isNormalSalvage(player, itemInHand, extended));
-
-        for (SalvageResult result : results) {
-            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();
-            }
-        }
-        if (salvage == null || salvage.isEmpty()) {
             return;
         }
 
         //Start a new scrapper session for the player
         currentSessionStartTime = System.currentTimeMillis();
-        int itemsConsumed = SalvageHelper.getRequiredAmountForSalvage(player.getServer(), itemInHand);
+        session = new SalvageSession(this, player, npc, getSettings(), result.salvage(),
-        session = new SalvageSession(this, player, npc, getSettings(), salvage, salvageMethod, itemsConsumed);
+                result.salvageMethod(), result.requiredAmount());
         // Print the cost to the player
-        printCostMessage(player, itemInHand, EconomyManager.formatSalvageCost(salvageMethod), salvageMethod);
+        printCostMessage(player, itemInHand, EconomyManager.formatSalvageCost(result.salvageMethod()),
+                result.salvageMethod());
+    }
+
+    /**
+     * Gets the best available salvage result
+     *
+     * @param player     <p>The player attempting to salvage an item</p>
+     * @param itemInHand <p>The item the player is attempting to salvage</p>
+     * @param extended   <p>Whether extended salvage is enabled</p>
+     * @return <p>The best result, or null if no valid result exists</p>
+     */
+    @Nullable
+    private SalvageResult getBestResult(@NotNull Player player, @NotNull ItemStack itemInHand, boolean extended) {
+        SalvageResult result = isArmorTrimSalvage(player, itemInHand);
+        if (result.salvageState() == SalvageState.NO_SALVAGE) {
+            return null;
+        } else if (result.salvageState() == SalvageState.FOUND_SALVAGE) {
+            return result;
+        }
+
+        result = isNetheriteSalvage(player, itemInHand);
+        if (result.salvageState() == SalvageState.NO_SALVAGE) {
+            return null;
+        } else if (result.salvageState() == SalvageState.FOUND_SALVAGE) {
+            return result;
+        }
+
+        result = isNormalSalvage(player, itemInHand, extended);
+        if (result.salvageState() == SalvageState.NO_SALVAGE) {
+            return null;
+        } else if (result.salvageState() == SalvageState.FOUND_SALVAGE) {
+            return result;
+        }
+
+        return null;
     }
 
     /**
@@ -130,29 +154,32 @@ public class ScrapperTrait extends CustomTrait<ScrapperSetting> {
      * @param extended   <p>Whether extended salvage is enabled</p>
      * @return <p>The result of attempting the salvage</p>
      */
+    @NotNull
     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);
+            BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Provided with non-salvage-able item " + itemInHand);
+            return new SalvageResult(SalvageMethod.SALVAGE, new ArrayList<>(), SalvageState.NO_SALVAGE, 0);
         }
 
         // 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);
+            return new SalvageResult(SalvageMethod.SALVAGE, new ArrayList<>(), SalvageState.NO_SALVAGE, 0);
         }
 
         // Check if any salvage will be produced
-        List<ItemStack> salvage = getSalvage(itemInHand, extended);
+        RecipeResult recipeResult = getSalvage(itemInHand, extended);
-        boolean noUsefulSalvage = salvage == null || salvage.isEmpty();
+        boolean noUsefulSalvage = recipeResult == null || recipeResult.salvage().isEmpty();
         if (noUsefulSalvage) {
             sendNPCMessage(this.npc, player, getSettings().getTooDamagedMessage());
-            return new SalvageResult(SalvageMethod.SALVAGE, new ArrayList<>(), SalvageState.NO_SALVAGE);
+            return new SalvageResult(SalvageMethod.SALVAGE, new ArrayList<>(), SalvageState.NO_SALVAGE, 0);
         } else {
-            return new SalvageResult(SalvageMethod.SALVAGE, salvage, SalvageState.FOUND_SALVAGE);
+            return new SalvageResult(SalvageMethod.SALVAGE, recipeResult.salvage(), SalvageState.FOUND_SALVAGE,
+                    recipeResult.recipe().getResult().getAmount());
         }
     }
 
@@ -166,20 +193,20 @@ public class ScrapperTrait extends CustomTrait<ScrapperSetting> {
     @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);
+            return new SalvageResult(SalvageMethod.NETHERITE, new ArrayList<>(), SalvageState.INCORRECT_METHOD, 0);
         }
 
         if (!getSettings().salvageNetherite()) {
             sendNPCMessage(this.npc, player, getSettings().getCannotSalvageNetheriteMessage());
-            return new SalvageResult(SalvageMethod.NETHERITE, new ArrayList<>(), SalvageState.NO_SALVAGE);
+            return new SalvageResult(SalvageMethod.NETHERITE, new ArrayList<>(), SalvageState.NO_SALVAGE, 0);
         }
 
         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, new ArrayList<>(), SalvageState.NO_SALVAGE, 0);
         }
 
-        return new SalvageResult(SalvageMethod.NETHERITE, salvage, SalvageState.FOUND_SALVAGE);
+        return new SalvageResult(SalvageMethod.NETHERITE, salvage, SalvageState.FOUND_SALVAGE, 1);
     }
 
     /**
@@ -192,21 +219,21 @@ public class ScrapperTrait extends CustomTrait<ScrapperSetting> {
     @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);
+            return new SalvageResult(SalvageMethod.ARMOR_TRIM, new ArrayList<>(), SalvageState.INCORRECT_METHOD, 0);
         }
 
         if (!getSettings().salvageArmorTrims()) {
             sendNPCMessage(this.npc, player, getSettings().getCannotSalvageArmorTrimMessage());
-            return new SalvageResult(SalvageMethod.ARMOR_TRIM, new ArrayList<>(), SalvageState.NO_SALVAGE);
+            return new SalvageResult(SalvageMethod.ARMOR_TRIM, new ArrayList<>(), SalvageState.NO_SALVAGE, 0);
         }
 
         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, new ArrayList<>(), SalvageState.NO_SALVAGE, 0);
         }
 
-        return new SalvageResult(SalvageMethod.ARMOR_TRIM, salvage, SalvageState.FOUND_SALVAGE);
+        return new SalvageResult(SalvageMethod.ARMOR_TRIM, salvage, SalvageState.FOUND_SALVAGE, 1);
     }
 
     /**
@@ -264,7 +291,7 @@ public class ScrapperTrait extends CustomTrait<ScrapperSetting> {
      * @return <p>The possible salvage, or null if not salvage-able</p>
      */
     @Nullable
-    private List<ItemStack> getSalvage(@NotNull ItemStack item, boolean extended) {
+    private RecipeResult getSalvage(@NotNull ItemStack item, boolean extended) {
         // Get the salvage, for the item, but ignore some materials if set, and the item isn't at full durability
         Set<Material> trashSalvage = BlacksmithPlugin.getInstance().getGlobalScrapperSettings().getTrashSalvage(
                 item.getType());

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

@@ -1,6 +1,7 @@
 package net.knarcraft.blacksmith.util;
 
-import org.bukkit.Bukkit;
+import net.knarcraft.blacksmith.BlacksmithPlugin;
+import net.knarcraft.blacksmith.container.RecipeResult;
 import org.bukkit.Material;
 import org.bukkit.Server;
 import org.bukkit.enchantments.Enchantment;
@@ -20,6 +21,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Random;
+import java.util.logging.Level;
 
 /**
  * A helper class for deciding the salvage returned if salvaging an item
@@ -123,23 +125,6 @@ public final class SalvageHelper {
         return false;
     }
 
-    /**
-     * Gets the amount of an item that's required to salvage that item
-     *
-     * @param server <p>The server to get recipes from</p>
-     * @param item   <p>The item to check</p>
-     * @return <p>The number of items required for salvage, or -1 if the recipe was not found</p>
-     */
-    public static int getRequiredAmountForSalvage(@NotNull Server server, @NotNull ItemStack item) {
-        for (Recipe recipe : server.getRecipesFor(new ItemStack(item.getType(), item.getAmount()))) {
-            // Only crafting recipes are allowed.
-            if (recipe instanceof ShapedRecipe || recipe instanceof ShapelessRecipe) {
-                return recipe.getResult().getAmount();
-            }
-        }
-        return 1;
-    }
-
     /**
      * Gets the sum of all enchantment levels for the given item
      *
@@ -165,28 +150,34 @@ public final class SalvageHelper {
      * @param extended     <p>Whether to enable extended salvage, ignoring the repairable restriction</p>
      * @return <p>The items to return to the user, or null if not salvageable</p>
      */
-    public static @Nullable List<ItemStack> getSalvage(@NotNull Server server, @Nullable ItemStack salvagedItem,
+    public static @Nullable RecipeResult getSalvage(@NotNull Server server, @Nullable ItemStack salvagedItem,
-                                                       @NotNull Collection<Material> trashSalvage, boolean extended) {
+                                                    @NotNull Collection<Material> trashSalvage, boolean extended) {
         if (salvagedItem == null || salvagedItem.getAmount() < 1 ||
                 (!extended && !ItemHelper.isRepairable(salvagedItem))) {
             return null;
         }
 
         for (Recipe recipe : server.getRecipesFor(new ItemStack(salvagedItem.getType(), salvagedItem.getAmount()))) {
+            BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Considering recipe: " + recipe.getResult() + " -> " + getRawRecipeSalvage(recipe));
+
             // Only consider crafting table recipes
             if (!(recipe instanceof ShapedRecipe) && !(recipe instanceof ShapelessRecipe)) {
+                BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Recipe had invalid type");
                 continue;
             }
 
             // Make sure the player has enough items
-            if (salvagedItem.getAmount() < getRequiredAmountForSalvage(Bukkit.getServer(), salvagedItem)) {
+            if (salvagedItem.getAmount() < recipe.getResult().getAmount()) {
+                BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Too few items for recipe");
                 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;
+                BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Valid recipe: " + recipe.getResult() + " -> " + getRawRecipeSalvage(recipe));
+                BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Actual salvage: " + salvage);
+                return new RecipeResult(recipe, salvage);
             }
         }
 
@@ -207,7 +198,13 @@ public final class SalvageHelper {
         if (ingredients == null) {
             return null;
         }
-        return combineStacks(getSalvage(copyItems(ingredients), salvagedItem, trashSalvage));
+        List<ItemStack> copy = copyItems(ingredients);
+        BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Copied salvage: " + copy);
+        List<ItemStack> salvage = getSalvage(copy, salvagedItem, trashSalvage);
+        BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Combining salvage: " + salvage);
+        List<ItemStack> combined = combineStacks(salvage);
+        BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Combined : " + combined);
+        return combined;
     }
 
     /**
@@ -247,7 +244,10 @@ public final class SalvageHelper {
             durability = 1;
         }
 
+        BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Durability: " + durability + "/" + maxDurability);
+
         double percentageRemaining = (double) durability / maxDurability;
+        BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "Remaining: " + percentageRemaining);
         return pickRandomSalvage(recipeItems, trashSalvage, percentageRemaining);
     }
 
@@ -268,36 +268,47 @@ public final class SalvageHelper {
             percentageRemaining = 1;
         }
 
-        int totalItems = totalItemAmount(itemsToChooseFrom);
+        // If not damaged, just give everything
-        //Get the amount of recipe items to be returned
+        if (percentageRemaining == 1) {
-        int itemsToReturn = (int) Math.floor(percentageRemaining * totalItems);
+            BlacksmithPlugin.getInstance().getLogger().log(Level.INFO, "100% Remaining. Copying " + itemsToChooseFrom);
-        int bound = itemsToChooseFrom.size();
+            return copyItems(itemsToChooseFrom);
+        }
 
+        // Split into good items and trash items
         List<ItemStack> goodItems = copyItems(itemsToChooseFrom);
         goodItems.removeIf((item) -> trashSalvage.contains(item.getType()));
-        int goodSalvage = totalItemAmount(goodItems);
+        List<ItemStack> trashItems = copyItems(itemsToChooseFrom);
+        trashItems.removeIf((item) -> !trashSalvage.contains(item.getType()));
 
-        List<ItemStack> salvage = new ArrayList<>();
+        int itemsToReturn = (int) Math.floor(totalItemAmount(itemsToChooseFrom) * percentageRemaining);
-        for (int i = 0; i < itemsToReturn; i++) {
+        int goodItemAmount = totalItemAmount(goodItems);
-            // Pick random item
+        int pickedItems = 0;
-            int itemIndex = SalvageHelper.random.nextInt(bound);
+        List<ItemStack> salvage = new ArrayList<>(itemsToReturn);
-            ItemStack itemStack = itemsToChooseFrom.get(itemIndex);
 
-            // The selected item is trash, so skip it
+        while (pickedItems < itemsToReturn && pickedItems < goodItemAmount) {
-            if (trashSalvage.contains(itemStack.getType()) && i < goodSalvage) {
+            int index = SalvageHelper.random.nextInt(goodItems.size());
-                i--;
+            ItemStack itemStack = goodItems.get(index);
+            if (itemStack.getType().isAir() || itemStack.getAmount() == 0) {
                 continue;
             }
 
-            //Make sure to never give more of one item than the amount which exists in the recipe
-            if (itemStack.getAmount() <= 0) {
-                i--;
-                continue;
-            }
-
-            itemStack.setAmount(itemStack.getAmount() - 1);
             salvage.add(new ItemStack(itemStack.getType(), 1));
+            itemStack.setAmount(itemStack.getAmount() - 1);
+            pickedItems++;
         }
+
+        while (pickedItems < itemsToReturn) {
+            int index = SalvageHelper.random.nextInt(trashItems.size());
+            ItemStack itemStack = trashItems.get(index);
+            if (itemStack.getType().isAir() || itemStack.getAmount() == 0) {
+                continue;
+            }
+
+            salvage.add(new ItemStack(itemStack.getType(), 1));
+            itemStack.setAmount(itemStack.getAmount() - 1);
+            pickedItems++;
+        }
+
         return salvage;
     }
 
@@ -328,7 +339,7 @@ public final class SalvageHelper {
         Map<Material, Integer> itemAmounts = new HashMap<>();
         for (ItemStack item : items) {
             Material itemType = item.getType();
-            itemAmounts.put(itemType, itemAmounts.getOrDefault(itemType, 0) + 1);
+            itemAmounts.put(itemType, itemAmounts.getOrDefault(itemType, 0) + item.getAmount());
         }
 
         List<ItemStack> combined = new ArrayList<>();