Cleans comments and code mess in BlacksmithTrait

src/main/java/net/apunch/blacksmith/BlacksmithTrait.java

@@ -26,6 +26,9 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.UUID;
 
+/**
+ * The class representing the Blacksmith NPC trait
+ */
 public class BlacksmithTrait extends Trait {
 
     private final BlacksmithPlugin plugin;
@@ -33,17 +36,30 @@ public class BlacksmithTrait extends Trait {
     private final Map<UUID, Calendar> coolDowns = new HashMap<>();
     private ReforgeSession session;
     private final Settings config;
+    private long _sessionStart = System.currentTimeMillis();
 
+    /**
+     * Instantiates a new blacksmith trait
+     */
     public BlacksmithTrait() {
         super("blacksmith");
         plugin = (BlacksmithPlugin) Bukkit.getServer().getPluginManager().getPlugin("Blacksmith");
         this.config = BlacksmithPlugin.getInstance().getSettings();
     }
 
+    /**
+     * Adds a cool-down for the given player's next blacksmith reforge
+     *
+     * @param playerUUID <p>The ID of the player to add the cool-down for</p>
+     * @param waitUntil  <p>The time when the player can interact again</p>
+     */
     public void addCoolDown(UUID playerUUID, Calendar waitUntil) {
         coolDowns.put(playerUUID, waitUntil);
     }
 
+    /**
+     * Unsets the session of this blacksmith, making it ready for another round
+     */
     public void unsetSession() {
         this.session = null;
     }
@@ -58,18 +74,32 @@ public class BlacksmithTrait extends Trait {
         config.loadVariables(key);
     }
 
+    @Override
+    public void save(DataKey key) {
+        //Save all items the blacksmith knows how to reforge
+        for (int i = 0; i < reforgeAbleItems.size(); i++) {
+            key.getRelative("reforgeable-items").setString(String.valueOf(i),
+                    reforgeAbleItems.get(i).name().toLowerCase().replace('_', '-'));
+        }
+
+        //Save all other config values
+        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().getLocation().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 && BlacksmithPlugin.isArmor(usedItem)) {
             event.setUseItemInHand(Event.Result.DENY);
@@ -77,19 +107,151 @@ public class BlacksmithTrait extends Trait {
         }
     }
 
-    public static <T extends Entity> T getTarget(final Entity entity, final Iterable<T> entities) {
+    @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) {
+        //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());
+        }
+        //Deny if permission is missing
+        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()))) {
+                player.sendMessage(config.getCoolDownUnexpiredMessage());
+                return false;
+            }
+            coolDowns.remove(player.getUniqueId());
+        }
+
+        //If already in a session, but the player has failed to interact, or left the blacksmith, allow a new session
+        if (session != null) {
+            if (System.currentTimeMillis() > _sessionStart + 10 * 1000 ||
+                    this.npc.getEntity().getLocation().distance(session.getPlayer().getLocation()) > 20) {
+                session = null;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Tries to continue the session for the given player
+     *
+     * @param player <p>The player to continue the session for</p>
+     */
+    private void continueSession(Player player) {
+        //Another player is using the blacksmith
+        if (!session.isInSession(player)) {
+            player.sendMessage(config.getBusyWithPlayerMessage());
+            return;
+        }
+
+        //The blacksmith is already reforging for the player
+        if (session.isRunning()) {
+            player.sendMessage(config.getBusyReforgingMessage());
+            return;
+        }
+        if (session.endSession()) {
+            //Quit if the player cannot afford, or has changed their item
+            session = null;
+        } else {
+            //Start reforging for the player
+            reforge(npc, player);
+        }
+    }
+
+    /**
+     * Starts a new session, and prepares to repair the player's item
+     *
+     * @param player <p>The player to start the session for</p>
+     */
+    private void startSession(Player player) {
+        ItemStack hand = player.getInventory().getItemInMainHand();
+        //If not a tool, not armor, and not set in reforgeAbleItems, refuse to repair
+        if ((!plugin.isTool(hand) && !BlacksmithPlugin.isArmor(hand)) ||
+                (!reforgeAbleItems.isEmpty() && !reforgeAbleItems.contains(hand.getType()))) {
+            player.sendMessage(config.getInvalidItemMessage());
+            return;
+        }
+
+        //Start a new reforge session for the player
+        _sessionStart = System.currentTimeMillis();
+        session = new ReforgeSession(this, player, npc, config);
+
+        //Tell the player the cost of repairing the item
+        String cost = plugin.formatCost(player);
+        String itemName = hand.getType().name().toLowerCase().replace('_', ' ');
+        player.sendMessage(config.getCostMessage().replace("<price>", cost).replace("<item>", itemName));
+    }
+
+    /**
+     * Starts reforging the player's item
+     *
+     * @param npc    <p>The NPC performing the reforge</p>
+     * @param player <p>The player that initiated the reforge</p>
+     */
+    private void reforge(NPC npc, Player player) {
+        player.sendMessage(config.getStartReforgeMessage());
+        plugin.withdraw(player);
+        session.beginReforge();
+        ItemStack heldItem = player.getInventory().getItemInMainHand();
+
+        //Display the item in the NPC's hand
+        if (npc.getEntity() instanceof Player) {
+            ((Player) npc.getEntity()).getInventory().setItemInMainHand(heldItem);
+        } else {
+            Objects.requireNonNull(((LivingEntity) npc.getEntity()).getEquipment()).setItemInMainHand(heldItem);
+        }
+        //Remove the item from the player's inventory
+        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()
+            final Vector n = other.getLocation().toVector().subtract(entity.getLocation().toVector());
-                    .subtract(entity.getLocation().toVector());
+            if (entity.getLocation().getDirection().normalize().crossProduct(n).lengthSquared() < threshold &&
-            if (entity.getLocation().getDirection().normalize().crossProduct(n)
+                    n.normalize().dot(entity.getLocation().getDirection().normalize()) >= 0) {
-                    .lengthSquared() < threshold
-                    && n.normalize().dot(
-                    entity.getLocation().getDirection().normalize()) >= 0) {
                 if (target == null ||
                         target.getLocation().distanceSquared(entity.getLocation()) >
                                 other.getLocation().distanceSquared(entity.getLocation())) {
@@ -100,97 +262,4 @@ public class BlacksmithTrait extends Trait {
         return target;
     }
 
-    @EventHandler
-    public void onRightClick(net.citizensnpcs.api.event.NPCRightClickEvent event) {
-        if (this.npc != event.getNPC()) {
-            return;
-        }
-
-        Player player = event.getClicker();
-        if ((config.getDisableCoolDown() & (coolDowns.get(player.getUniqueId()) != (null)))) {
-            coolDowns.remove(player.getUniqueId());
-        }
-        if (!player.hasPermission("blacksmith.reforge")) {
-            return;
-        }
-
-        if (coolDowns.get(player.getUniqueId()) != null) {
-            if (!Calendar.getInstance().after(coolDowns.get(player.getUniqueId()))) {
-                player.sendMessage(config.getCoolDownUnexpiredMessage());
-                return;
-            }
-            coolDowns.remove(player.getUniqueId());
-        }
-
-
-        ItemStack hand = player.getInventory().getItemInMainHand();
-
-        if (session != null) {
-            //timeout
-            if (System.currentTimeMillis() > _sessionStart + 10 * 1000 ||
-                    this.npc.getEntity().getLocation().distance(session.getPlayer().getLocation()) > 20) {
-                session = null;
-            }
-        }
-
-
-        if (session != null) {
-            if (!session.isInSession(player)) {
-                player.sendMessage(config.getBusyWithPlayerMessage());
-                return;
-            }
-
-            if (session.isRunning()) {
-                player.sendMessage(config.getBusyReforgingMessage());
-                return;
-            }
-            if (session.endSession()) {
-                session = null;
-            } else {
-                reforge(npc, player);
-            }
-        } else {
-            if ((!plugin.isTool(hand) && !BlacksmithPlugin.isArmor(hand))
-                    || (!reforgeAbleItems.isEmpty() && !reforgeAbleItems.contains(hand.getType()))) {
-                player.sendMessage(config.getInvalidItemMessage());
-                return;
-            }
-
-            String cost = plugin.formatCost(player);
-
-            _sessionStart = System.currentTimeMillis();
-            session = new ReforgeSession(this, player, npc, config);
-            player.sendMessage(config.getCostMessage().replace("<price>", cost).replace("<item>",
-                    hand.getType().name().toLowerCase().replace('_', ' ')));
-
-        }
-    }
-
-    private long _sessionStart = System.currentTimeMillis();
-
-    @Override
-    public void save(DataKey key) {
-        for (int i = 0; i < reforgeAbleItems.size(); i++) {
-            key.getRelative("reforgeable-items").setString(String.valueOf(i),
-                    reforgeAbleItems.get(i).name().toLowerCase().replace('_', '-'));
-        }
-
-        config.saveVariables(key);
-    }
-
-    private void reforge(NPC npc, Player player) {
-        player.sendMessage(config.getStartReforgeMessage());
-
-        //plugin.deposit(npc, player); // CitiTrader dependency outdated and broken
-
-        plugin.withdraw(player);
-        session.beginReforge();
-        if (npc.getEntity() instanceof Player) {
-            ((Player) npc.getEntity()).getInventory().setItemInMainHand(player.getInventory().getItemInMainHand());
-        } else {
-            Objects.requireNonNull(((LivingEntity) npc.getEntity()).getEquipment()).setItemInMainHand(player.getInventory().getItemInMainHand());
-        }
-        player.getInventory().setItemInMainHand(null);
-    }
-
 }

src/main/java/net/apunch/blacksmith/ReforgeSession.java

@@ -96,7 +96,7 @@ class ReforgeSession implements Runnable {
                     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) + 
+                        itemToReforge.addEnchantment(enchantment, random.nextInt(randomBound) +
                                 enchantment.getStartLevel());
                     }
                 }