Improves code structure, and performs some necessary work for commands

src/main/java/net/knarcraft/blacksmith/BlacksmithPlugin.java

@@ -2,8 +2,14 @@ package net.knarcraft.blacksmith;
 
 import net.citizensnpcs.api.CitizensAPI;
 import net.knarcraft.blacksmith.command.BlackSmithCommand;
+import net.knarcraft.blacksmith.command.BlackSmithTabCompleter;
 import net.knarcraft.blacksmith.config.GlobalSettings;
+import net.knarcraft.blacksmith.listener.NPCClickListener;
+import net.knarcraft.blacksmith.listener.PlayerListener;
+import net.knarcraft.blacksmith.manager.EconomyManager;
+import net.knarcraft.blacksmith.trait.BlacksmithTrait;
 import org.bukkit.command.PluginCommand;
+import org.bukkit.plugin.PluginManager;
 import org.bukkit.plugin.java.JavaPlugin;
 
 import java.util.logging.Level;
@@ -69,8 +75,13 @@ public class BlacksmithPlugin extends JavaPlugin {
         PluginCommand blacksmithCommand = this.getCommand("blacksmith");
         if (blacksmithCommand != null) {
             blacksmithCommand.setExecutor(new BlackSmithCommand());
+            blacksmithCommand.setTabCompleter(new BlackSmithTabCompleter());
         }
 
+        PluginManager pluginManager = getServer().getPluginManager();
+        pluginManager.registerEvents(new PlayerListener(), this);
+        pluginManager.registerEvents(new NPCClickListener(), this);
+
         getLogger().log(Level.INFO, " v" + getDescription().getVersion() + " enabled.");
     }
 

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

@@ -1,5 +1,6 @@
 package net.knarcraft.blacksmith.command;
 
+import net.md_5.bungee.api.ChatColor;
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandExecutor;
 import org.bukkit.command.CommandSender;
@@ -10,11 +11,20 @@ public class BlackSmithCommand implements CommandExecutor {
     @Override
     public boolean onCommand(@NotNull CommandSender sender, @NotNull Command command, @NotNull String label,
                              @NotNull String[] args) {
+        if (!sender.hasPermission("blacksmith.admin")) {
+            sender.sendMessage(ChatColor.RED + "You don't have the necessary permission for using this command.");
+            return true;
+        }
+
         //TODO: This command should have one config sub-command which changes the default config values, and all 
         // setting names which can be changed for each NPC.
         if (args.length > 0) {
             if (args[0].equalsIgnoreCase("reload")) {
                 return new ReloadCommand().onCommand(sender, command, label, args);
+            } else if (args[0].equalsIgnoreCase("config")) {
+                //TODO: Allow changing any global/default setting + reloading here
+            } else {
+                return new NPCSettingCommand().onCommand(sender, command, label, args);
             }
         }
         return false;

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

@@ -0,0 +1,31 @@
+package net.knarcraft.blacksmith.command;
+
+import net.knarcraft.blacksmith.config.NPCSetting;
+import org.bukkit.command.Command;
+import org.bukkit.command.CommandSender;
+import org.bukkit.command.TabCompleter;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class BlackSmithTabCompleter implements TabCompleter {
+
+    @Nullable
+    @Override
+    public List<String> onTabComplete(@NotNull CommandSender sender, @NotNull Command command, @NotNull String label,
+                                      @NotNull String[] args) {
+        if (!sender.hasPermission("blacksmith.admin")) {
+            return new ArrayList<>();
+        }
+
+        List<String> availableCommands = new ArrayList<>();
+        for (NPCSetting setting : NPCSetting.values()) {
+            availableCommands.add(setting.getCommandName());
+        }
+        availableCommands.add("reload");
+        return availableCommands;
+    }
+
+}

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

@@ -0,0 +1,38 @@
+package net.knarcraft.blacksmith.command;
+
+import net.citizensnpcs.api.CitizensAPI;
+import net.citizensnpcs.api.npc.NPC;
+import net.knarcraft.blacksmith.config.NPCSetting;
+import net.knarcraft.blacksmith.trait.BlacksmithTrait;
+import org.bukkit.command.Command;
+import org.bukkit.command.CommandExecutor;
+import org.bukkit.command.CommandSender;
+import org.jetbrains.annotations.NotNull;
+
+public class NPCSettingCommand implements CommandExecutor {
+
+    @Override
+    public boolean onCommand(@NotNull CommandSender sender, @NotNull Command command, @NotNull String label, @NotNull String[] args) {
+        NPC npc = CitizensAPI.getDefaultNPCSelector().getSelected(sender);
+        if (npc == null || !npc.hasTrait(BlacksmithTrait.class)) {
+            sender.sendMessage("You must select an NPC before running this command");
+            return true;
+        }
+        BlacksmithTrait blacksmithTrait = npc.getTrait(BlacksmithTrait.class);
+
+        for (NPCSetting npcSetting : NPCSetting.values()) {
+            String commandName = npcSetting.getCommandName();
+            if (commandName.equalsIgnoreCase(args[0])) {
+                if (args.length < 2) {
+                    sender.sendMessage("Current value of " + commandName + ": " +
+                            blacksmithTrait.getSettings().getRawValue(npcSetting));
+                } else {
+                    blacksmithTrait.getSettings().changeSetting(npcSetting, args[1]);
+                }
+                return true;
+            }
+        }
+        return false;
+    }
+
+}

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

@@ -7,54 +7,60 @@ import java.util.Arrays;
  */
 public enum NPCSetting {
 
-    DROP_ITEM("defaults.drop-item", true),
+    DROP_ITEM("defaults.drop-item", true, "dropItem"),
-    DISABLE_COOL_DOWN("defaults.disable-cool-down", false),
+    DISABLE_COOL_DOWN("defaults.disable-cool-down", false, "disableCoolDown"),
-    DISABLE_DELAY("defaults.disable-delay", false),
+    DISABLE_DELAY("defaults.disable-delay", false, "disableDelay"),
-    FAIL_CHANCE("defaults.percent-chance-to-fail-reforge", 10),
+    FAIL_CHANCE("defaults.percent-chance-to-fail-reforge", 10, "failReforgeChance"),
-    EXTRA_ENCHANTMENT_CHANCE("defaults.percent-chance-for-extra-enchantment", 5),
+    EXTRA_ENCHANTMENT_CHANCE("defaults.percent-chance-for-extra-enchantment", 5,
-    MAX_ENCHANTMENTS("defaults.maximum-enchantments", 3),
+            "extraEnchantmentChance"),
-    MAX_REFORGE_DELAY("defaults.delays-in-seconds.maximum", 30),
+    MAX_ENCHANTMENTS("defaults.maximum-enchantments", 3, "maxEnchantments"),
-    MIN_REFORGE_DELAY("defaults.delays-in-seconds.minimum", 5),
+    MAX_REFORGE_DELAY("defaults.delays-in-seconds.maximum", 30, "maxReforgeDelay"),
-    REFORGE_COOL_DOWN("defaults.delays-in-seconds.reforge-cool-down", 60),
+    MIN_REFORGE_DELAY("defaults.delays-in-seconds.minimum", 5, "minReforgeDelay"),
-    REFORGE_ABLE_ITEMS("defaults.reforge-able-items", new String[]{}),
+    REFORGE_COOL_DOWN("defaults.delays-in-seconds.reforge-cool-down", 60, "reforgeCoolDown"),
+    REFORGE_ABLE_ITEMS("defaults.reforge-able-items", new String[]{}, "reforgeAbleItems"),
 
     /*-----------
      | Messages |
      -----------*/
-    BUSY_WITH_PLAYER_MESSAGE("defaults.messages.busy-with-player", "§cI'm busy at the moment. Come back later!"),
+    BUSY_WITH_PLAYER_MESSAGE("defaults.messages.busy-with-player",
-    BUSY_WITH_REFORGE_MESSAGE("defaults.messages.busy-with-reforge", "§cI'm working on it. Be patient!"),
+            "§cI'm busy at the moment. Come back later!", "busyPlayerMessage"),
-    COOL_DOWN_UNEXPIRED_MESSAGE(
+    BUSY_WITH_REFORGE_MESSAGE("defaults.messages.busy-with-reforge", "§cI'm working on it. Be patient!",
-            "defaults.messages.cool-down-not-expired",
+            "busyReforgeMessage"),
-            "§cYou've already had your chance! Give me a break!"),
+    COOL_DOWN_UNEXPIRED_MESSAGE("defaults.messages.cool-down-not-expired",
+            "§cYou've already had your chance! Give me a break!", "coolDownUnexpiredMessage"),
     COST_MESSAGE(
             "defaults.messages.cost",
-            "§eIt will cost §a<price> §eto reforge that §a<item>§e! Click again to reforge!"),
+            "§eIt will cost §a<price> §eto reforge that §a<item>§e! Click again to reforge!", "costMessage"),
-    FAIL_MESSAGE("defaults.messages.fail-reforge", "§cWhoops! Didn't mean to do that! Maybe next time?"),
+    FAIL_MESSAGE("defaults.messages.fail-reforge", "§cWhoops! Didn't mean to do that! Maybe next time?",
-    INSUFFICIENT_FUNDS_MESSAGE(
+            "failReforgeMessage"),
-            "defaults.messages.insufficient-funds",
+    INSUFFICIENT_FUNDS_MESSAGE("defaults.messages.insufficient-funds",
-            "§cYou don't have enough money to reforge that item!"),
+            "§cYou don't have enough money to reforge that item!", "insufficientFundsMessage"),
-    INVALID_ITEM_MESSAGE("defaults.messages.invalid-item", "§cI'm sorry, but I don't know how to reforge that!"),
+    INVALID_ITEM_MESSAGE("defaults.messages.invalid-item", "§cI'm sorry, but I don't know how to reforge that!",
-    ITEM_UNEXPECTEDLY_CHANGED_MESSAGE(
+            "invalidItemMessage"),
-            "defaults.messages.item-changed-during-reforge",
+    ITEM_UNEXPECTEDLY_CHANGED_MESSAGE("defaults.messages.item-changed-during-reforge",
-            "§cThat's not the item you wanted to reforge before!"),
+            "§cThat's not the item you wanted to reforge before!", "itemChangedMessage"),
-    START_REFORGE_MESSAGE("defaults.messages.start-reforge", "§eOk, let's see what I can do..."),
+    START_REFORGE_MESSAGE("defaults.messages.start-reforge", "§eOk, let's see what I can do...",
-    SUCCESS_MESSAGE("defaults.messages.successful-reforge", "There you go! All better!");
+            "startReforgeMessage"),
+    SUCCESS_MESSAGE("defaults.messages.successful-reforge", "There you go! All better!", "successMessage");
 
     private final String path;
     private final String childPath;
     private final Object value;
+    private final String commandName;
 
     /**
      * Instantiates a new setting
      *
      * @param path        <p>The full config path for this setting</p>
      * @param value       <p>The default value of this setting</p>
+     * @param commandName <p>The name of the command used to change this setting</p>
      */
-    NPCSetting(String path, Object value) {
+    NPCSetting(String path, Object value, String commandName) {
         this.path = path;
         this.value = value;
         String[] pathParts = path.split("\\.");
         this.childPath = String.join(".", Arrays.copyOfRange(pathParts, 1, pathParts.length));
+        this.commandName = commandName;
     }
 
     /**
@@ -80,8 +86,17 @@ public enum NPCSetting {
      *
      * @return <p>The value of this setting</p>
      */
-    Object getDefaultValue() {
+    public Object getDefaultValue() {
         return value;
     }
 
+    /**
+     * The name of the command used to change this setting
+     *
+     * @return <p>The name of this setting's command</p>
+     */
+    public String getCommandName() {
+        return commandName;
+    }
+
 }

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

@@ -50,6 +50,26 @@ public class NPCSettings {
         }
     }
 
+    /**
+     * Changes one setting to the given value
+     *
+     * @param setting  <p>The setting to change</p>
+     * @param newValue <p>The new value of the setting</p>
+     */
+    public void changeSetting(NPCSetting setting, Object newValue) {
+        currentValues.put(setting, newValue);
+    }
+
+    /**
+     * Gets the raw current value of a setting
+     *
+     * @param setting <p>The setting to get the value of</p>
+     * @return <p>The current value of the setting</p>
+     */
+    public Object getRawValue(NPCSetting setting) {
+        return currentValues.get(setting);
+    }
+
     /**
      * Gets the message to display when the blacksmith is busy with another player
      *

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

@@ -0,0 +1,33 @@
+package net.knarcraft.blacksmith.listener;
+
+import net.knarcraft.blacksmith.trait.BlacksmithTrait;
+import org.bukkit.entity.Player;
+import org.bukkit.event.EventHandler;
+import org.bukkit.event.Listener;
+
+public class NPCClickListener implements Listener {
+
+    @EventHandler
+    public void onRightClick(net.citizensnpcs.api.event.NPCRightClickEvent event) {
+        //We only care about blacksmiths
+        if (!event.getNPC().hasTrait(BlacksmithTrait.class)) {
+            return;
+        }
+        BlacksmithTrait blacksmithTrait = event.getNPC().getTrait(BlacksmithTrait.class);
+
+        //Perform any necessary pre-session work
+        Player player = event.getClicker();
+        if (!blacksmithTrait.prepareForSession(player)) {
+            return;
+        }
+
+        if (blacksmithTrait.hasSession()) {
+            //Continue the existing session
+            blacksmithTrait.continueSession(player);
+        } else {
+            //Start a new session
+            blacksmithTrait.startSession(player);
+        }
+    }
+
+}

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

@@ -0,0 +1,91 @@
+package net.knarcraft.blacksmith.listener;
+
+import net.citizensnpcs.api.CitizensAPI;
+import net.knarcraft.blacksmith.trait.BlacksmithTrait;
+import org.bukkit.enchantments.EnchantmentTarget;
+import org.bukkit.entity.Entity;
+import org.bukkit.event.Event;
+import org.bukkit.event.EventHandler;
+import org.bukkit.event.EventPriority;
+import org.bukkit.event.Listener;
+import org.bukkit.event.block.Action;
+import org.bukkit.event.player.PlayerInteractEvent;
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.util.Vector;
+
+public class PlayerListener implements Listener {
+
+    /**
+     * Blocks armor equipment if right-clicking a blacksmith
+     *
+     * @param event <p>The triggered event</p>
+     */
+    @EventHandler(priority = EventPriority.HIGHEST)
+    public void onClick(PlayerInteractEvent event) {
+        if (event.getHand() == null || (event.getAction() != Action.RIGHT_CLICK_AIR && event.getAction() !=
+                Action.RIGHT_CLICK_BLOCK)) {
+            return;
+        }
+        //If the player is not looking at a blacksmith, there is no need to do anything
+        Entity target = getTarget(event.getPlayer(), event.getPlayer().getNearbyEntities(15, 10, 15));
+        if (!CitizensAPI.getNPCRegistry().isNPC(target) ||
+                !CitizensAPI.getNPCRegistry().getNPC(target).hasTrait(BlacksmithTrait.class)) {
+            return;
+        }
+
+        //Block armor equip if interacting with a blacksmith
+        ItemStack usedItem = event.getPlayer().getInventory().getItem(event.getHand());
+        if (usedItem != null && isArmor(usedItem)) {
+            event.setUseItemInHand(Event.Result.DENY);
+            event.getPlayer().updateInventory();
+        }
+    }
+
+    /**
+     * Gets whether the given item is a type of armor
+     *
+     * @param item <p>The item to check if is armor or not</p>
+     * @return <p>True if the given item is a type of armor</p>
+     */
+    public static boolean isArmor(ItemStack item) {
+        return EnchantmentTarget.WEARABLE.includes(item);
+        //TODO: Remove this commented-out code if the above line works
+        /*return switch (item.getType()) {
+            case LEATHER_HELMET, LEATHER_CHESTPLATE, LEATHER_LEGGINGS, LEATHER_BOOTS, CHAINMAIL_HELMET,
+                    CHAINMAIL_CHESTPLATE, CHAINMAIL_LEGGINGS, CHAINMAIL_BOOTS, GOLDEN_HELMET, GOLDEN_CHESTPLATE,
+                    GOLDEN_LEGGINGS, GOLDEN_BOOTS, IRON_HELMET, IRON_CHESTPLATE, IRON_LEGGINGS, IRON_BOOTS,
+                    DIAMOND_HELMET, DIAMOND_CHESTPLATE, DIAMOND_LEGGINGS, DIAMOND_BOOTS, TURTLE_HELMET, ELYTRA,
+                    NETHERITE_HELMET, NETHERITE_CHESTPLATE, NETHERITE_LEGGINGS, NETHERITE_BOOTS -> true;
+            default -> false;
+        };*/
+    }
+
+    /**
+     * Gets the target-entity the entity is looking at
+     *
+     * @param entity   <p>The entity looking at something</p>
+     * @param entities <p>Entities near the player</p>
+     * @param <T>      <p>The type of entity the player is looking at</p>
+     * @return <p>The entity the player is looking at, or null if no such entity exists</p>
+     */
+    private static <T extends Entity> T getTarget(final Entity entity, final Iterable<T> entities) {
+        if (entity == null) {
+            return null;
+        }
+        T target = null;
+        final double threshold = 1;
+        for (final T other : entities) {
+            final Vector n = other.getLocation().toVector().subtract(entity.getLocation().toVector());
+            if (entity.getLocation().getDirection().normalize().crossProduct(n).lengthSquared() < threshold &&
+                    n.normalize().dot(entity.getLocation().getDirection().normalize()) >= 0) {
+                if (target == null ||
+                        target.getLocation().distanceSquared(entity.getLocation()) >
+                                other.getLocation().distanceSquared(entity.getLocation())) {
+                    target = other;
+                }
+            }
+        }
+        return target;
+    }
+
+}

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

@@ -1,5 +1,6 @@
-package net.knarcraft.blacksmith;
+package net.knarcraft.blacksmith.manager;
 
+import net.knarcraft.blacksmith.BlacksmithPlugin;
 import net.knarcraft.blacksmith.config.GlobalSettings;
 import net.milkbowl.vault.economy.Economy;
 import org.bukkit.Material;

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

@@ -1,24 +1,17 @@
-package net.knarcraft.blacksmith;
+package net.knarcraft.blacksmith.trait;
 
-import net.citizensnpcs.api.CitizensAPI;
 import net.citizensnpcs.api.npc.NPC;
 import net.citizensnpcs.api.trait.Trait;
 import net.citizensnpcs.api.util.DataKey;
+import net.knarcraft.blacksmith.BlacksmithPlugin;
 import net.knarcraft.blacksmith.config.NPCSettings;
+import net.knarcraft.blacksmith.manager.EconomyManager;
 import org.bukkit.Bukkit;
 import org.bukkit.Material;
-import org.bukkit.enchantments.EnchantmentTarget;
-import org.bukkit.entity.Entity;
 import org.bukkit.entity.LivingEntity;
 import org.bukkit.entity.Player;
-import org.bukkit.event.Event;
-import org.bukkit.event.EventHandler;
-import org.bukkit.event.EventPriority;
-import org.bukkit.event.block.Action;
-import org.bukkit.event.player.PlayerInteractEvent;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.meta.Damageable;
-import org.bukkit.util.Vector;
 
 import java.util.Calendar;
 import java.util.HashMap;
@@ -47,6 +40,24 @@ public class BlacksmithTrait extends Trait {
         this.config = new NPCSettings(BlacksmithPlugin.getInstance().getSettings());
     }
 
+    /**
+     * Gets the current settings for this NPC
+     *
+     * @return <p>The current settings for this NPC</p>
+     */
+    public NPCSettings getSettings() {
+        return config;
+    }
+
+    /**
+     * Gets whether this blacksmith is already in a re-forge session
+     *
+     * @return <p>Whether already in a re-forge session</p>
+     */
+    public boolean hasSession() {
+        return this.session != null;
+    }
+
     /**
      * Adds a cool-down for the given player's next blacksmith reforge
      *
@@ -84,55 +95,13 @@ public class BlacksmithTrait extends Trait {
         config.saveVariables(key);
     }
 
-    @EventHandler(priority = EventPriority.HIGHEST)
-    public void onClick(PlayerInteractEvent event) {
-        if (event.getHand() == null || (event.getAction() != Action.RIGHT_CLICK_AIR && event.getAction() !=
-                Action.RIGHT_CLICK_BLOCK)) {
-            return;
-        }
-        //If the player is not looking at a blacksmith, there is no need to do anything
-        Entity target = getTarget(event.getPlayer(), event.getPlayer().getNearbyEntities(15, 10, 15));
-        if (!CitizensAPI.getNPCRegistry().isNPC(target) ||
-                !CitizensAPI.getNPCRegistry().getNPC(target).hasTrait(BlacksmithTrait.class)) {
-            return;
-        }
-
-        //Block armor equip if interacting with a blacksmith
-        ItemStack usedItem = event.getPlayer().getInventory().getItem(event.getHand());
-        if (usedItem != null && isArmor(usedItem)) {
-            event.setUseItemInHand(Event.Result.DENY);
-            event.getPlayer().updateInventory();
-        }
-    }
-
-    @EventHandler
-    public void onRightClick(net.citizensnpcs.api.event.NPCRightClickEvent event) {
-        if (this.npc != event.getNPC()) {
-            return;
-        }
-
-        //Perform any necessary pre-session work
-        Player player = event.getClicker();
-        if (!prepareForSession(player)) {
-            return;
-        }
-
-        if (session != null) {
-            //Continue the existing session
-            continueSession(player);
-        } else {
-            //Start a new session
-            startSession(player);
-        }
-    }
-
     /**
      * Performs necessary work before the session is started or continued
      *
      * @param player <p>The player to prepare the session for</p>
      * @return <p>True if preparations were successful. False if a session shouldn't be started</p>
      */
-    private boolean prepareForSession(Player player) {
+    public boolean prepareForSession(Player player) {
         //If cool-down has been disabled after it was set for this player, remove the cool-down
         if (config.getDisableCoolDown() && coolDowns.get(player.getUniqueId()) != null) {
             coolDowns.remove(player.getUniqueId());
@@ -141,6 +110,7 @@ public class BlacksmithTrait extends Trait {
         if (!player.hasPermission("blacksmith.reforge")) {
             return false;
         }
+
         //Deny if on cool-down, or remove cool-down if expired
         if (coolDowns.get(player.getUniqueId()) != null) {
             if (!Calendar.getInstance().after(coolDowns.get(player.getUniqueId()))) {
@@ -165,7 +135,7 @@ public class BlacksmithTrait extends Trait {
      *
      * @param player <p>The player to continue the session for</p>
      */
-    private void continueSession(Player player) {
+    public void continueSession(Player player) {
         //Another player is using the blacksmith
         if (!session.isInSession(player)) {
             player.sendMessage(config.getBusyWithPlayerMessage());
@@ -191,7 +161,7 @@ public class BlacksmithTrait extends Trait {
      *
      * @param player <p>The player to start the session for</p>
      */
-    private void startSession(Player player) {
+    public void startSession(Player player) {
         ItemStack hand = player.getInventory().getItemInMainHand();
         //Refuse if not repairable, or if reforge-able items is set, but doesn't include the held item
         List<Material> reforgeAbleItems = config.getReforgeAbleItems();
@@ -232,34 +202,6 @@ public class BlacksmithTrait extends Trait {
         player.getInventory().setItemInMainHand(null);
     }
 
-    /**
-     * Gets the target-entity the entity is looking at
-     *
-     * @param entity   <p>The entity looking at something</p>
-     * @param entities <p>Entities near the player</p>
-     * @param <T>      <p>The type of entity the player is looking at</p>
-     * @return <p>The entity the player is looking at, or null if no such entity exists</p>
-     */
-    private static <T extends Entity> T getTarget(final Entity entity, final Iterable<T> entities) {
-        if (entity == null) {
-            return null;
-        }
-        T target = null;
-        final double threshold = 1;
-        for (final T other : entities) {
-            final Vector n = other.getLocation().toVector().subtract(entity.getLocation().toVector());
-            if (entity.getLocation().getDirection().normalize().crossProduct(n).lengthSquared() < threshold &&
-                    n.normalize().dot(entity.getLocation().getDirection().normalize()) >= 0) {
-                if (target == null ||
-                        target.getLocation().distanceSquared(entity.getLocation()) >
-                                other.getLocation().distanceSquared(entity.getLocation())) {
-                    target = other;
-                }
-            }
-        }
-        return target;
-    }
-
     /**
      * Gets whether the given item is repairable
      *
@@ -270,23 +212,4 @@ public class BlacksmithTrait extends Trait {
         return item.getItemMeta() instanceof Damageable;
     }
 
-    /**
-     * Gets whether the given item is a type of armor
-     *
-     * @param item <p>The item to check if is armor or not</p>
-     * @return <p>True if the given item is a type of armor</p>
-     */
-    public static boolean isArmor(ItemStack item) {
-        return EnchantmentTarget.WEARABLE.includes(item);
-        //TODO: Remove this commented-out code if the above line works
-        /*return switch (item.getType()) {
-            case LEATHER_HELMET, LEATHER_CHESTPLATE, LEATHER_LEGGINGS, LEATHER_BOOTS, CHAINMAIL_HELMET,
-                    CHAINMAIL_CHESTPLATE, CHAINMAIL_LEGGINGS, CHAINMAIL_BOOTS, GOLDEN_HELMET, GOLDEN_CHESTPLATE,
-                    GOLDEN_LEGGINGS, GOLDEN_BOOTS, IRON_HELMET, IRON_CHESTPLATE, IRON_LEGGINGS, IRON_BOOTS,
-                    DIAMOND_HELMET, DIAMOND_CHESTPLATE, DIAMOND_LEGGINGS, DIAMOND_BOOTS, TURTLE_HELMET, ELYTRA,
-                    NETHERITE_HELMET, NETHERITE_CHESTPLATE, NETHERITE_LEGGINGS, NETHERITE_BOOTS -> true;
-            default -> false;
-        };*/
-    }
-
 }

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

@@ -1,7 +1,9 @@
-package net.knarcraft.blacksmith;
+package net.knarcraft.blacksmith.trait;
 
 import net.citizensnpcs.api.npc.NPC;
+import net.knarcraft.blacksmith.BlacksmithPlugin;
 import net.knarcraft.blacksmith.config.NPCSettings;
+import net.knarcraft.blacksmith.manager.EconomyManager;
 import org.bukkit.NamespacedKey;
 import org.bukkit.enchantments.Enchantment;
 import org.bukkit.entity.LivingEntity;
@@ -27,6 +29,7 @@ public class ReforgeSession implements Runnable {
     private int taskId;
     private final NPCSettings config;
     private static final String[] enchantments = new String[Enchantment.values().length];
+    private static final Random random = new Random();
 
     /**
      * Instantiates a new session
@@ -49,16 +52,24 @@ public class ReforgeSession implements Runnable {
         }
     }
 
+    /**
+     * Runs the actual re-forge which fixes the item and gives it back to the player
+     */
     @Override
     public void run() {
-        player.sendMessage(reforgeItemInHand() ? config.getSuccessMessage() : config.getFailMessage());
+        player.sendMessage(reforgeItem() ? config.getSuccessMessage() : config.getFailMessage());
+
+        //Stop the re-forged item from displaying in the blacksmith's hand
         if (npc.getEntity() instanceof Player) {
             ((Player) npc.getEntity()).getInventory().setItemInMainHand(null);
         } else {
             Objects.requireNonNull(((LivingEntity) npc.getEntity()).getEquipment()).setItemInMainHand(null);
         }
+
+        //Give the item back to the player
         if (!config.getDisableDelay()) {
-            if (config.getDropItem()) {
+            //If the player isn't online, drop the item to prevent it from disappearing
+            if (config.getDropItem() || !player.isOnline()) {
                 player.getWorld().dropItemNaturally(npc.getEntity().getLocation(), itemToReforge);
             } else {
                 player.getInventory().addItem(itemToReforge);
@@ -66,18 +77,69 @@ public class ReforgeSession implements Runnable {
         } else {
             player.getInventory().setItemInMainHand(itemToReforge);
         }
+
+        //Mark this blacksmith as available
         blacksmithTrait.unsetSession();
-        // Start cool down
+
+        // Start cool-down
         Calendar wait = Calendar.getInstance();
         wait.add(Calendar.SECOND, config.getReforgeCoolDown());
         blacksmithTrait.addCoolDown(player.getUniqueId(), wait);
     }
 
-    private boolean reforgeItemInHand() {
+    /**
-        Random random = new Random();
+     * Performs the actual re-forge where the item's damage is reduced
+     *
+     * @return <p>Whether the re-forge was successful. False if the blacksmith failed to fully repair the item.</p>
+     */
+    private boolean reforgeItem() {
         if (random.nextInt(100) < config.getFailChance()) {
+            failReforge();
+            return false;
+        } else {
+            succeedReforge();
+            return true;
+        }
+    }
+
+    /**
+     * The method to run when a blacksmith successfully re-forges an item
+     */
+    private void succeedReforge() {
+        // Remove any damage done to the item
+        updateDamage(itemToReforge, 0);
+
+        // Add random enchantments
+        int roll = random.nextInt(100);
+        if (!(roll < config.getExtraEnchantmentChance() &&
+                itemToReforge.getEnchantments().keySet().size() < config.getMaxEnchantments())) {
+            // Abort if randomness isn't on our side, or if max enchantments has been reached
+            return;
+        }
+        // Choose a random enchantment
+        Enchantment enchantment;
+        int maxRetries = 100;
+        int retries = 0;
+        do {
+            // Try to find a working enchantment for the re-forged item up to maxRetries times
+            enchantment = Enchantment.getByKey(NamespacedKey.fromString(enchantments[random.nextInt(enchantments.length)]));
+        } while ((enchantment == null || !enchantment.canEnchantItem(itemToReforge)) && (retries++ < maxRetries));
+
+        if (enchantment != null && enchantment.canEnchantItem(itemToReforge)) {
+            int randomBound = enchantment.getMaxLevel() - enchantment.getStartLevel();
+            //A workaround for the random method's bound sometimes being negative
+            if (randomBound >= 0) {
+                itemToReforge.addEnchantment(enchantment, random.nextInt(randomBound) + enchantment.getStartLevel());
+            }
+        }
+    }
+
+    /**
+     * The method to run when a blacksmith fails re-forging an item
+     */
+    private void failReforge() {
+        // Remove or downgrade existing enchantments
         for (Enchantment enchantment : itemToReforge.getEnchantments().keySet()) {
-                // Remove or downgrade enchantments
             if (random.nextBoolean()) {
                 itemToReforge.removeEnchantment(enchantment);
             } else {
@@ -87,36 +149,17 @@ public class ReforgeSession implements Runnable {
                 }
             }
         }
+
         // Damage the item
-            short reforgeDurability = EconomyManager.getDurability(itemToReforge);
+        short currentItemDurability = EconomyManager.getDurability(itemToReforge);
-            short durability = (short) (reforgeDurability + reforgeDurability * random.nextInt(8));
+        short newDurability = (short) (currentItemDurability + (currentItemDurability * random.nextInt(8)));
         short maxDurability = itemToReforge.getType().getMaxDurability();
-            if (durability <= 0) {
+        if (newDurability <= 0) {
-                durability = (short) (maxDurability / 3);
+            newDurability = (short) (maxDurability / 3);
-            } else if (reforgeDurability + durability > maxDurability) {
+        } else if (currentItemDurability + newDurability > maxDurability) {
-                durability = (short) (maxDurability - random.nextInt(maxDurability - 25));
+            newDurability = (short) (maxDurability - random.nextInt(maxDurability - 25));
-            }
-            updateDamage(itemToReforge, maxDurability - durability);
-            return false;
-        } else {
-            updateDamage(itemToReforge, 0);
-
-            // Add random enchantments
-            int roll = random.nextInt(100);
-            if (roll < config.getExtraEnchantmentChance() && itemToReforge.getEnchantments().keySet().size() < config.getMaxEnchantments()) {
-                Enchantment enchantment = Enchantment.getByKey(NamespacedKey.fromString(enchantments[random.nextInt(enchantments.length)]));
-                if (Objects.requireNonNull(enchantment).canEnchantItem(itemToReforge)) {
-                    int randomBound = enchantment.getMaxLevel() - enchantment.getStartLevel();
-                    //A workaround for the random method's bound sometimes being negative
-                    if (randomBound >= 0) {
-                        itemToReforge.addEnchantment(enchantment, random.nextInt(randomBound) +
-                                enchantment.getStartLevel());
-                    }
-                }
-
-            }
-            return true;
         }
+        updateDamage(itemToReforge, maxDurability - newDurability);
     }
 
     /**

src/main/resources/plugin.yml

@@ -10,7 +10,7 @@ api-version: 1.18
 commands:
   blacksmith:
     permission: blacksmith.admin
-    description: reloads the config file for Blacksmith
+    description: Used for configuring a blacksmith or the plugin
 permissions:
   blacksmith.admin:
     description: Allows blacksmith configuration