From 909863a2f70ab2fab8f098e824ea025630cc4716 Mon Sep 17 00:00:00 2001
From: EpicKnarvik97 <kristian.knarvik@knett.no>
Date: Wed, 19 Oct 2022 12:58:12 +0200
Subject: [PATCH] Improves blacksmith description, and adds some missing
 comments

---
 .../net/knarcraft/dynmapcitizens/Icon.java    | 23 ++++++++++++++++
 .../trait/AbstractTraitHandler.java           |  3 +++
 .../trait/BlacksmithHandler.java              | 26 +++++++++++++++----
 .../trait/CitizensTraitHandler.java           |  3 +++
 .../dynmapcitizens/trait/QuestsHandler.java   |  6 ++++-
 .../dynmapcitizens/trait/SentinelHandler.java |  3 +++
 6 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/src/main/java/net/knarcraft/dynmapcitizens/Icon.java b/src/main/java/net/knarcraft/dynmapcitizens/Icon.java
index d1ea93d..20e79a7 100644
--- a/src/main/java/net/knarcraft/dynmapcitizens/Icon.java
+++ b/src/main/java/net/knarcraft/dynmapcitizens/Icon.java
@@ -5,11 +5,34 @@ package net.knarcraft.dynmapcitizens;
  */
 public enum Icon {
 
+    /**
+     * An icon representing a quest giver
+     */
     QUEST_GIVER,
+
+    /**
+     * An icon representing an interactable NPC part of some quest
+     */
     QUEST_INTERACT,
+
+    /**
+     * An icon representing an NPC to be killed as part of a quest
+     */
     QUEST_KILL,
+
+    /**
+     * An icon representing an NPC taking deliveries as part of a quest
+     */
     QUEST_DELIVER,
+
+    /**
+     * An icon representing a sentinel NPC
+     */
     SENTINEL,
+
+    /**
+     * An icon representing a blacksmith NPC
+     */
     BLACKSMITH
 
 }
diff --git a/src/main/java/net/knarcraft/dynmapcitizens/trait/AbstractTraitHandler.java b/src/main/java/net/knarcraft/dynmapcitizens/trait/AbstractTraitHandler.java
index 67457ee..76cad64 100644
--- a/src/main/java/net/knarcraft/dynmapcitizens/trait/AbstractTraitHandler.java
+++ b/src/main/java/net/knarcraft/dynmapcitizens/trait/AbstractTraitHandler.java
@@ -13,6 +13,9 @@ import org.dynmap.markers.MarkerSet;
 import java.util.UUID;
 import java.util.logging.Level;
 
+/**
+ * An abstract class with useful code for all citizens-trait-handlers
+ */
 public abstract class AbstractTraitHandler implements CitizensTraitHandler {
 
     protected boolean isEnabled = false;
diff --git a/src/main/java/net/knarcraft/dynmapcitizens/trait/BlacksmithHandler.java b/src/main/java/net/knarcraft/dynmapcitizens/trait/BlacksmithHandler.java
index 0d72bf9..b40f307 100644
--- a/src/main/java/net/knarcraft/dynmapcitizens/trait/BlacksmithHandler.java
+++ b/src/main/java/net/knarcraft/dynmapcitizens/trait/BlacksmithHandler.java
@@ -53,12 +53,8 @@ public class BlacksmithHandler extends AbstractTraitHandler {
                 if (trait == null) {
                     DynmapCitizens.getInstance().getLogger().log(Level.WARNING, "Unable to get blacksmith trait");
                 } else {
-                    NPCSettings npcSettings = trait.getSettings();
                     //TODO: Make a setting which allows disabling extra information (except title and reforge-able items?)
-                    description = "<b>Blacksmith name:</b> " + npc.getName() + "<br><b>Blacksmith title:</b> " +
-                            npcSettings.getBlacksmithTitle() + "<br><b>Fail chance:</b> " + npcSettings.getFailChance() +
-                            "<br><b>Enchantment chance:</b> " + npcSettings.getExtraEnchantmentChance() +
-                            "<br><b>Reforge-able items:</b> " + getReforgeAbleItemsString(npcSettings.getReforgeAbleItems());
+                    description = getDetailedBlacksmithInfo(npc, trait.getSettings());
                 }
                 addNPCMarker(npc.getUniqueId(), "Blacksmith NPC: ", description,
                         DynmapCitizens.getInstance().getMarkerIcons().get(Icon.BLACKSMITH), blacksmithSet);
@@ -66,6 +62,26 @@ public class BlacksmithHandler extends AbstractTraitHandler {
         }
     }
 
+    /**
+     * Gets detailed information about a blacksmith NPC
+     *
+     * @param npc         <p>The NPC the settings belong to</p>
+     * @param npcSettings <p>The settings to search for information</p>
+     * @return <p>A string describing the blacksmith</p>
+     */
+    private String getDetailedBlacksmithInfo(NPC npc, NPCSettings npcSettings) {
+        String info = "<b>Blacksmith name:</b> " + npc.getName() + "<br><b>Blacksmith title:</b> " +
+                npcSettings.getBlacksmithTitle() + "<br><b>Fail chance:</b> " + npcSettings.getFailChance() +
+                "<br><b>Enchantment chance:</b> " + npcSettings.getExtraEnchantmentChance() + "<br><b>Delay:</b> " +
+                npcSettings.getMinReforgeDelay() + " to " + npcSettings.getMaxReforgeDelay() +
+                " seconds<br><b>Cool-down:</b> " + npcSettings.getReforgeCoolDown() + " seconds<br><b>Drop item:</b> " +
+                npcSettings.getDropItem();
+        if (!npcSettings.getReforgeAbleItems().isEmpty()) {
+            info += "<br><b>Reforge-able items:</b> " + getReforgeAbleItemsString(npcSettings.getReforgeAbleItems());
+        }
+        return info;
+    }
+
     /**
      * Gets reforge-able items as a string
      *
diff --git a/src/main/java/net/knarcraft/dynmapcitizens/trait/CitizensTraitHandler.java b/src/main/java/net/knarcraft/dynmapcitizens/trait/CitizensTraitHandler.java
index 281db57..3cdb03e 100644
--- a/src/main/java/net/knarcraft/dynmapcitizens/trait/CitizensTraitHandler.java
+++ b/src/main/java/net/knarcraft/dynmapcitizens/trait/CitizensTraitHandler.java
@@ -1,5 +1,8 @@
 package net.knarcraft.dynmapcitizens.trait;
 
+/**
+ * A handler which takes care of everything for one citizen trait
+ */
 public interface CitizensTraitHandler {
 
     /**
diff --git a/src/main/java/net/knarcraft/dynmapcitizens/trait/QuestsHandler.java b/src/main/java/net/knarcraft/dynmapcitizens/trait/QuestsHandler.java
index 4773beb..88e246d 100644
--- a/src/main/java/net/knarcraft/dynmapcitizens/trait/QuestsHandler.java
+++ b/src/main/java/net/knarcraft/dynmapcitizens/trait/QuestsHandler.java
@@ -19,6 +19,9 @@ import java.util.Map;
 import java.util.UUID;
 import java.util.logging.Level;
 
+/**
+ * A handler class for the quests trait
+ */
 public class QuestsHandler extends AbstractTraitHandler {
 
     private QuestsAPI questsAPI;
@@ -38,9 +41,10 @@ public class QuestsHandler extends AbstractTraitHandler {
                 questMarkerSet.setHideByDefault(false);
                 questAreaMarkerSet.setHideByDefault(true);
                 isEnabled = true;
-                //TODO: Initialize the dynmap drawer with the marker sets
+                return;
             }
         }
+        isEnabled = false;
     }
 
     @Override
diff --git a/src/main/java/net/knarcraft/dynmapcitizens/trait/SentinelHandler.java b/src/main/java/net/knarcraft/dynmapcitizens/trait/SentinelHandler.java
index f26f49e..295e74a 100644
--- a/src/main/java/net/knarcraft/dynmapcitizens/trait/SentinelHandler.java
+++ b/src/main/java/net/knarcraft/dynmapcitizens/trait/SentinelHandler.java
@@ -1,5 +1,8 @@
 package net.knarcraft.dynmapcitizens.trait;
 
+/**
+ * A handler class for the sentinel trait
+ */
 public class SentinelHandler extends AbstractTraitHandler {
 
     @Override