Improves un-setting of NPC values #10

README.md

@@ -12,6 +12,8 @@ fee. Costs are highly customizable.
 - EnchantmentTarget is used instead of a hard-coded list of repairable items
 - All settings (except default reforge-able-items), both global and for each blacksmith, can be changed using commands,
   and support tab-completion.
+- This plugin is not directly compatible with the original. If you are using the old one, you will need to set it up
+  again!
 
 ### Dependencies
 
@@ -53,6 +55,10 @@ In addition to just being able to repair items, blacksmiths have some random fea
 
 For /blacksmith and /blacksmithconfig, if a new value isn't specified, the current value is displayed instead.
 
+For /blacksmith, using "" or null as the value will clear a custom value, making the NPC use the default value set in
+the config instead. Additionally, every value overridden for an NPC will display a marker (default \[NPC]) when
+displaying the current value.
+
 Note: basePrice, pricePerDurabilityPoint and enchantmentCost can be set like: `/blacksmithconfig option 4` or
 like `/blacksmithconfig option material/enchantment 4` depending on whether you are setting the default or an override
 for a specific material/enchantment.

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

@@ -7,6 +7,7 @@ import net.knarcraft.blacksmith.config.NPCSetting;
 import net.knarcraft.blacksmith.config.SettingValueType;
 import net.knarcraft.blacksmith.formatting.StringFormatter;
 import net.knarcraft.blacksmith.formatting.TranslatableMessage;
+import net.knarcraft.blacksmith.formatting.Translator;
 import net.knarcraft.blacksmith.trait.BlacksmithTrait;
 import net.knarcraft.blacksmith.util.TypeValidationHelper;
 import net.md_5.bungee.api.ChatColor;
@@ -18,6 +19,8 @@ import org.jetbrains.annotations.NotNull;
 import java.util.Arrays;
 
 import static net.knarcraft.blacksmith.formatting.StringFormatter.displaySuccessMessage;
+import static net.knarcraft.blacksmith.formatting.TranslatableMessage.getCurrentValueMessage;
+import static net.knarcraft.blacksmith.formatting.TranslatableMessage.getValueChangedMessage;
 
 /**
  * The main command used for blacksmith editing
@@ -66,31 +69,49 @@ public class BlackSmithEditCommand implements CommandExecutor {
                                               CommandSender sender) {
         if (newValue == null) {
             //Display the current value of the setting
-            String rawValue = String.valueOf(blacksmithTrait.getSettings().getRawValue(npcSetting));
+            displayNPCSetting(blacksmithTrait, npcSetting, sender);
-            //Displays the default value, if no custom value has been specified
-            if (rawValue == null || rawValue.equalsIgnoreCase("null")) {
-                rawValue = String.valueOf(BlacksmithPlugin.getInstance().getSettings().getRawValue(npcSetting));
-            }
-            displaySuccessMessage(sender, TranslatableMessage.getCurrentValueMessage(npcSetting.getCommandName(), rawValue));
-            if (npcSetting.getPath().startsWith("defaults.messages")) {
-                sender.sendMessage(TranslatableMessage.getRawValueMessage(
-                        rawValue.replace(ChatColor.COLOR_CHAR, '&')));
-            }
-            return true;
         } else {
+            //If an empty value or null, clear the value instead of changing it
+            if (newValue.equalsIgnoreCase("null") || newValue.equals("\"\"") || newValue.trim().isEmpty()) {
+                newValue = null;
+            } else {
+                //Abort if an invalid value is given
                 boolean isValidType = TypeValidationHelper.isValid(npcSetting.getValueType(), newValue, sender);
-            if (isValidType) {
+                if (!isValidType) {
-                //Change the setting
-                Object nullCheckedValue = newValue.equalsIgnoreCase("null") ? null :
-                        ChatColor.translateAlternateColorCodes('&', newValue);
-                blacksmithTrait.getSettings().changeSetting(npcSetting, nullCheckedValue);
-                displaySuccessMessage(sender, TranslatableMessage.getValueChangedMessage(npcSetting.getNodeName(), newValue));
-                //Save the changes immediately to prevent data loss on server crash
-                CitizensAPI.getNPCRegistry().saveToStore();
-                return true;
-            } else {
                     return false;
                 }
+                newValue = ChatColor.translateAlternateColorCodes('&', newValue);
+            }
+
+            //Change the setting
+            blacksmithTrait.getSettings().changeSetting(npcSetting, newValue);
+            displaySuccessMessage(sender, getValueChangedMessage(npcSetting.getNodeName(), String.valueOf(newValue)));
+            //Save the changes immediately to prevent data loss on server crash
+            CitizensAPI.getNPCRegistry().saveToStore();
+        }
+        return true;
+    }
+
+    /**
+     * Displays the current value of the given NPC setting
+     *
+     * @param blacksmithTrait <p>The blacksmith trait of the NPC to get the value from</p>
+     * @param npcSetting      <p>The NPC setting to see the value of</p>
+     * @param sender          <p>The command sender to display the value to</p>
+     */
+    private void displayNPCSetting(BlacksmithTrait blacksmithTrait, NPCSetting npcSetting, CommandSender sender) {
+        String rawValue = String.valueOf(blacksmithTrait.getSettings().getRawValue(npcSetting));
+        if (rawValue == null || rawValue.trim().isEmpty() || rawValue.equalsIgnoreCase("null")) {
+            //Display the default value, if no custom value has been specified
+            rawValue = String.valueOf(BlacksmithPlugin.getInstance().getSettings().getRawValue(npcSetting));
+            displaySuccessMessage(sender, getCurrentValueMessage(npcSetting.getCommandName(), rawValue));
+        } else {
+            //Add a marker if the value has been customized
+            String marker = Translator.getTranslatedMessage(TranslatableMessage.SETTING_OVERRIDDEN_MARKER);
+            displaySuccessMessage(sender, getCurrentValueMessage(npcSetting.getCommandName(), rawValue) + marker);
+        }
+        if (npcSetting.getPath().startsWith("defaults.messages")) {
+            sender.sendMessage(TranslatableMessage.getRawValueMessage(rawValue.replace(ChatColor.COLOR_CHAR, '&')));
         }
     }
 

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

@@ -361,7 +361,7 @@ public class NPCSettings {
         newReforgeAbleItems = (String) replaceReforgeAblePresets(newReforgeAbleItems);
 
         for (String item : newReforgeAbleItems.split(",")) {
-            if (item == null || item.equalsIgnoreCase("null")) {
+            if (item == null || item.trim().isEmpty() || item.equalsIgnoreCase("null")) {
                 continue;
             }
             Material material = Material.matchMaterial(item.replace('-', '_'));

src/main/java/net/knarcraft/blacksmith/formatting/TranslatableMessage.java

@@ -158,7 +158,12 @@ public enum TranslatableMessage {
     /**
      * The text to display when describing more than 5 minutes remaining
      */
-    INTERVAL_MORE_THAN_5_MINUTES;
+    INTERVAL_MORE_THAN_5_MINUTES,
+
+    /**
+     * The marker used for displaying that a given setting has been overridden for the selected NPC
+     */
+    SETTING_OVERRIDDEN_MARKER;
 
     /**
      * Gets the message to display when displaying the raw value of messages

src/main/resources/strings.yml

@@ -29,3 +29,4 @@ en:
   INTERVAL_LESS_THAN_1_MINUTE: "in a while"
   INTERVAL_LESS_THAN_5_MINUTES: "after some time"
   INTERVAL_MORE_THAN_5_MINUTES: "in quite a while"
+  SETTING_OVERRIDDEN_MARKER: " [NPC]"