Finishes the ActionCost implementation

pom.xml

@@ -65,7 +65,7 @@
         <dependency>
             <groupId>org.spigotmc</groupId>
             <artifactId>spigot-api</artifactId>
-            <version>1.20.6-R0.1-SNAPSHOT</version>
+            <version>1.21.3-R0.1-SNAPSHOT</version>
             <scope>provided</scope>
         </dependency>
         <dependency>

src/main/java/net/knarcraft/blacksmith/command/CostCommand.java

@@ -0,0 +1,65 @@
+package net.knarcraft.blacksmith.command;
+
+import net.knarcraft.blacksmith.BlacksmithPlugin;
+import org.bukkit.command.Command;
+import org.bukkit.command.CommandExecutor;
+import org.bukkit.command.CommandSender;
+import org.bukkit.command.TabExecutor;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.List;
+
+public class CostCommand implements TabExecutor {
+
+    @Override
+    public boolean onCommand(@NotNull CommandSender commandSender, @NotNull Command command, @NotNull String s, @NotNull String[] arguments) {
+        //TODO: Whenever a cost is specified (not for the per-material and per-enchantment costs), allow either a simple
+        // cost or an ActionCost. When loading costs, first try and parse a double. If not, parse an ActionCost object.
+        // TODO: The command should look like "blacksmithEdit <option> <simple|advanced> <double|economy|item|permission|exp>
+        //  <double|blank/null|comma-separated-string|integer>"
+
+        //TODO: Check arguments size
+        if (arguments.length < 2) {
+            return false;
+        }
+        switch (arguments[0]) {
+            case "simple":
+                // TODO: Expect the next argument to be the cost
+                try {
+                    Double cost = Double.parseDouble(arguments[1]);
+
+                } catch (NumberFormatException exception) {
+                    // TODO: Make this translatable?
+                    BlacksmithPlugin.getStringFormatter().displayErrorMessage(commandSender, "You must supply a numeric (double) cost");
+                    return true;
+                }
+                break;
+            case "advanced":
+                switch (arguments[1]) {
+                    case "economy":
+                        // TODO: Expect the next argument to be the cost
+                        break;
+                    case "item":
+                        // TODO: The next argument would either be "null" to clear the value, or "mainHand" to use the item in the player's main hand
+                        break;
+                    case "permission":
+                        // TODO: The next argument will be a comma-separated list of permissions
+                        break;
+                    case "exp":
+                        // TODO: Expect the next argument to be an integer
+                        break;
+                }
+                break;
+            default:
+                return false;
+        }
+        return false;
+    }
+
+    @Override
+    public @Nullable List<String> onTabComplete(@NotNull CommandSender commandSender, @NotNull Command command, @NotNull String s, @NotNull String[] strings) {
+        return List.of();
+    }
+
+}

src/main/java/net/knarcraft/blacksmith/config/SettingValueType.java

@@ -55,4 +55,9 @@ public enum SettingValueType {
      */
     ENCHANTMENT_LIST,
 
+    /**
+     * Advanced cost, that supports either a simple double, or specifying money cost, permission requirement, exp cost and an item cost
+     */
+    ADVANCED_COST,
+
 }

src/main/java/net/knarcraft/blacksmith/config/scrapper/GlobalScrapperSettings.java

@@ -222,15 +222,6 @@ public class GlobalScrapperSettings implements Settings<ScrapperSetting> {
         return asDouble(ScrapperSetting.ENCHANTED_BOOK_SALVAGE_BASE_COST);
     }
 
-    /**
-     * Gets the item cost of salvaging an enchanted book
-     * 
-     * @return <p>The enchanted book salvage cost</p>
-     */
-    public ItemStack getEnchantedBookItemCost() {
-        return ScrapperSetting.ENCHANTED_BOOK_SALVAGE_ITEM_COST;
-    }
-
     /**
      * Whether to multiply the cost of salvaging enchanted books based on the number of enchantments
      *

src/main/java/net/knarcraft/blacksmith/config/scrapper/ScrapperSetting.java

@@ -307,8 +307,8 @@ public enum ScrapperSetting implements Setting {
     /**
      * The setting for the enchanted book item cost
      */
-    ENCHANTED_BOOK_SALVAGE_ITEM_COST("enchantedBookSalvageItemCost", SettingValueType.ITEM_STACK, null,
+    ENCHANTED_BOOK_SALVAGE_ITEM_COST("enchantedBookSalvageCost", SettingValueType.ADVANCED_COST, null,
-            "The item that needs to be provided in order to salvage an enchanted book", false, false),
+            "The cost to pay in order to salvage an enchanted book", false, false),
 
     /**
      * The setting for whether to multiply enchanted book cost

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

@@ -1,6 +1,9 @@
 package net.knarcraft.blacksmith.container;
 
+import net.knarcraft.blacksmith.BlacksmithPlugin;
 import net.knarcraft.blacksmith.manager.EconomyManager;
+import org.bukkit.configuration.ConfigurationSection;
+import org.bukkit.configuration.serialization.ConfigurationSerializable;
 import org.bukkit.entity.Player;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
@@ -8,11 +11,7 @@ import org.bukkit.inventory.meta.ItemMeta;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.HashMap;
+import java.util.*;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
 
 /**
  * The cost of performing an action
@@ -23,7 +22,24 @@ import java.util.Set;
  * @param requiredPermissions <p>The permission required for the action</p>
  */
 public record ActionCost(double monetaryCost, int expCost, @Nullable ItemStack itemCost,
-                         @NotNull Set<String> requiredPermissions) {
+                         @NotNull Set<String> requiredPermissions) implements ConfigurationSerializable {
+
+    @NotNull
+    public String displayCost() {
+        StringBuilder builder = new StringBuilder();
+        if (monetaryCost > 0) {
+            builder.append(EconomyManager.format(monetaryCost)).append(", ");
+        }
+        if (expCost > 0) {
+            builder.append(expCost).append("exp, ");
+        }
+        if (itemCost != null) {
+            // TODO: Present name, amount and name + lore
+            builder.append(itemCost);
+        }
+        // TODO: Display required permissions if the player doesn't have them?
+        return builder.toString();
+    }
 
     /**
      * Checks whether the given player is able to pay this action cost
@@ -32,68 +48,161 @@ public record ActionCost(double monetaryCost, int expCost, @Nullable ItemStack i
      * @return <p>True if the player is able to pay</p>
      */
     public boolean canPay(@NotNull Player player) {
-        for (String permission : requiredPermissions) {
+        for (String permission : this.requiredPermissions) {
             if (!player.hasPermission(permission)) {
                 return false;
             }
         }
         
-        if (player.getExp() < expCost || !EconomyManager.hasEnough(player, monetaryCost)) {
+        if (player.getExp() < this.expCost || !EconomyManager.hasEnough(player, this.monetaryCost)) {
             return false;
         }
         
-        return itemCost == null || player.getInventory().contains(itemCost);
+        return hasEnoughValidItemsInInventory(player);
-    }
-    
-    public void takePayment(@NotNull Player player) {
-        player.giveExp(-expCost);
-        EconomyManager.withdraw(monetaryCost);
-        
     }
 
     /**
-     * Takes payment for printing a number of books by withdrawing the correct item
+     * Takes exp, money and items from the player, according to this cost
      *
-     * @param player   <p>The player which needs to pay</p>
+     * @param player <p>The player to take the payment from</p>
-     * @param item <p>The item to pay</p>
      */
-    private static void payForBookPrintingItem(Player player, ItemStack item) {
+    public void takePayment(@NotNull Player player) {
+        player.giveExp(-expCost);
+        EconomyManager.withdraw(player, monetaryCost);
+        takeItemCost(player);
+    }
+
+    /**
+     * Checks whether the given player has enough items specified as the item cost
+     *
+     * @param player <p>The player to check</p>
+     * @return <p>True if the player has enough items in their inventory</p>
+     */
+    private boolean hasEnoughValidItemsInInventory(@NotNull Player player) {
+        if (this.itemCost == null) {
+            return true;
+        }
+
+        int amountInInventory = 0;
+        for (Map.Entry<Integer, Integer> entry : getValidItems(player).entrySet()) {
+            amountInInventory += entry.getValue();
+        }
+        return this.itemCost.getAmount() >= amountInInventory;
+    }
+
+    /**
+     * Gets all items in a player's inventory equal to the specified item
+     *
+     * @param player <p>The player to get valid items for</p>
+     * @return <p>All valid items in the format: Inventory id -> Amount</p>
+     */
+    @NotNull
+    private Map<Integer, Integer> getValidItems(@NotNull Player player) {
+        if (this.itemCost == null) {
+            return new HashMap<>();
+        }
+
         PlayerInventory playerInventory = player.getInventory();
-        ItemMeta targetMeta = item.getItemMeta();
+        ItemMeta targetMeta = this.itemCost.getItemMeta();
         String displayName = null;
         List<String> lore = null;
         if (targetMeta != null) {
             displayName = targetMeta.hasDisplayName() ? targetMeta.getDisplayName() : null;
             lore = targetMeta.hasLore() ? targetMeta.getLore() : null;
         }
-        
+
-        HashMap<Integer, ? extends ItemStack> itemsOfType = playerInventory.all(item.getType());
+        Map<Integer, Integer> output = new HashMap<>();
+        HashMap<Integer, ? extends ItemStack> itemsOfType = playerInventory.all(this.itemCost.getType());
+
+        for (Map.Entry<Integer, ? extends ItemStack> entry : itemsOfType.entrySet()) {
+            if (targetMeta != null) {
+                // Only consider item if the name and lore is the same
+                ItemMeta meta = entry.getValue().getItemMeta();
+                if (meta == null || (displayName != null && (!meta.hasDisplayName() ||
+                        !meta.getDisplayName().equals(displayName)) || lore != null && (!meta.hasLore() ||
+                        meta.getLore() == null || !meta.getLore().equals(lore)))) {
+                    continue;
+                }
+            }
+            output.put(entry.getKey(), (Integer) entry.getValue().getAmount());
+        }
+
+        return output;
+    }
+
+    /**
+     * Takes the amount of items specified as the cost for this action
+     *
+     * @param player <p>The player to take the items from</p>
+     */
+    private void takeItemCost(@NotNull Player player) {
+        if (this.itemCost == null) {
+            return;
+        }
 
         int clearedAmount = 0;
-        for (Map.Entry<Integer, ? extends ItemStack> entry : itemsOfType.entrySet()) {
+        for (Map.Entry<Integer, Integer> entry : getValidItems(player).entrySet()) {
-            if (targetMeta == null) {
+            int inventory = entry.getKey();
-                if (Objects.requireNonNull(entry.getValue()).getAmount() <= item.getAmount() - clearedAmount) {
+            int amount = entry.getValue();
-                    clearedAmount += entry.getValue().getAmount();
+            if (amount <= this.itemCost.getAmount() - clearedAmount) {
-                    player.getInventory().clear(entry.getKey());
+                clearedAmount += amount;
-                } else {
+                player.getInventory().clear(entry.getKey());
-                    clearedAmount = item.getAmount();
-                    entry.getValue().setAmount(entry.getValue().getAmount() - (clearedAmount));
-                }
             } else {
-                // TODO: Only consider item if the name and lore is the same
+                clearedAmount = this.itemCost.getAmount();
-                ItemMeta meta = entry.getValue().getItemMeta();
+                ItemStack item = player.getInventory().getItem(inventory);
-                if (meta == null) {
+                if (item != null) {
-                    break;
+                    item.setAmount(amount - clearedAmount);
-                }
+                } else {
-                if (displayName != null && (!meta.hasDisplayName() || !meta.getDisplayName().equals(displayName))) {
+                    BlacksmithPlugin.error("An item changed after calculating item cost. Was unable to take " +
-                    break;
+                            amount + " items");
                 }
             }
 
-            if (clearedAmount <= item.getAmount()) {
+            if (clearedAmount >= this.itemCost.getAmount()) {
                 break;
             }
         }
     }
-    
+
+    /**
+     * Loads an action cost from a configuration section
+     *
+     * @param configurationSection <p>The configuration section to load from</p>
+     * @param key <p>The key of the cost to load</p>
+     * @return <p>The loaded cost</p>
+     */
+    private static ActionCost loadActionCost(@NotNull ConfigurationSection configurationSection, @NotNull String key) {
+        double cost = configurationSection.getDouble(key, Double.MIN_VALUE);
+        if (cost != Double.MIN_VALUE) {
+            return new ActionCost(cost, 0, null, Set.of());
+        } else {
+            return (ActionCost) configurationSection.get(key);
+        }
+    }
+
+    /**
+     * Deserializes an action cost
+     *
+     * @param serialized <p>The serialized action cost</p>
+     * @return <p>The deserialized action cost</p>
+     */
+    @SuppressWarnings({"unused", "unchecked"})
+    public static ActionCost deserialize(Map<String, Object> serialized) {
+        double monetaryCost = (double) serialized.get("monetaryCost");
+        int expCost = (int) serialized.get("expCost");
+        ItemStack itemCost = (ItemStack) serialized.get("itemCost");
+        Set<String> requiredPermissions = (Set<String>) serialized.get("requiredPermissions");
+        return new ActionCost(monetaryCost, expCost, itemCost, requiredPermissions);
+    }
+
+    @Override
+    public @NotNull Map<String, Object> serialize() {
+        Map<String, Object> serialized = new HashMap<>();
+        serialized.put("monetaryCost", Optional.of(this.monetaryCost));
+        serialized.put("expCost", Optional.of(this.expCost));
+        serialized.put("itemCost", itemCost);
+        serialized.put("requiredPermissions", this.requiredPermissions);
+        return serialized;
+    }
+
 }

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

@@ -151,6 +151,20 @@ public class EconomyManager {
         }
     }
 
+    /**
+     * Withdraws money from a player
+     *
+     * @param player <p>The player to withdraw from</p>
+     * @param monetaryCost <p>The cost to withdraw</p>
+     * @throws IllegalArgumentException <p>If a negative cost is given</p>
+     */
+    public static void withdraw(@NotNull Player player, double monetaryCost) {
+        if (monetaryCost < 0) {
+            throw new IllegalArgumentException("Cannot withdraw a negative amount");
+        }
+        economy.withdrawPlayer(player, monetaryCost);
+    }
+
     /**
      * Gets the cost of the item in the given player's main hand
      *

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

@@ -328,13 +328,14 @@ public class ScrapperTrait extends CustomTrait<ScrapperSetting> {
 
         GlobalScrapperSettings scrapperSettings = BlacksmithPlugin.getInstance().getGlobalScrapperSettings();
         String moneyCost = EconomyManager.format(scrapperSettings.getEnchantedBookSalvageCost());
-        ItemStack itemCost = scrapperSettings.
+        //ItemStack itemCost = scrapperSettings.
         if (scrapperSettings.requireMoneyAndItemForEnchantedBookSalvage()) {
             // TODO: Print both with a + between them (if item has a special name, use that instead of the material name)
             
         } else {
             // TODO: If the item is not null, print it, otherwise print the monetary cost
         }
+        return "";
     }
 
     @Override

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

@@ -38,6 +38,8 @@ public final class TabCompleteValuesHelper {
             case MATERIAL -> getAllReforgeAbleMaterialNames();
             case ENCHANTMENT, ENCHANTMENT_LIST -> getAllEnchantments();
             case STRING_LIST -> List.of("*_SHOVEL,*_PICKAXE,*_AXE,*_HOE,*_SWORD:STICK,SMITHING_TABLE:*_PLANKS");
+            // TODO: Change this to something that makes sense
+            case ADVANCED_COST -> List.of();
         };
     }
 

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

@@ -36,7 +36,8 @@ public final class TypeValidationHelper {
                 case STRING -> isNonEmptyString(value, sender);
                 case POSITIVE_INTEGER -> isPositiveInteger(value, sender);
                 case PERCENTAGE -> isPercentage(value, sender);
-                case BOOLEAN -> true;
+                // TODO: Implement proper advanced cost checking
+                case BOOLEAN, ADVANCED_COST -> true;
                 case REFORGE_ABLE_ITEMS, STRING_LIST -> isStringList(value, sender);
                 case MATERIAL -> isMaterial(value, sender);
                 case ENCHANTMENT -> isEnchantment(value, sender);