Various command improvements

- Improvements to error messages, handling of invalid arguments, ...
- Changed `/moo nameChangeCheck` so only one name can be checked at once

3 files changed, 82 insertions, 51 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e3b7383..1a5a0b6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,14 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
+## [1.8.9-0.8.0] - unreleased
+### Changed
+- Replaced `/moo nameChangeCheck` with `/moo nameChangeCheck <playerName>`
+  - Instead of triggering a manual check for name changes of *all* best friends, you can now only trigger a manualy check for a single name
+
+### Fixed
+- Various smaller command fixed (including error messages, handling of invalid arguments, ...)
+
 ## [1.8.9-0.7.1] - 05.07.2020
 ### Fixed
 - Fixed Nullpointer on 2nd+ game launch with empty friends file
@@ -87,6 +95,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 *Note:* The 'best friends' list is currently available via <kbd>ESC</kbd> > Mod Options > Cowlection > Config > bestFriends.
 
+[1.8.9-0.8.0]: https://github.com/cow-mc/Cowlection/compare/v1.8.9-0.7.1...master
 [1.8.9-0.7.1]: https://github.com/cow-mc/Cowlection/compare/v1.8.9-0.7.0...v1.8.9-0.7.1
 [1.8.9-0.7.0]: https://github.com/cow-mc/Cowlection/compare/v1.8.9-0.6.0...v1.8.9-0.7.0
 [1.8.9-0.6.0]: https://github.com/cow-mc/Cowlection/compare/v1.8.9-0.5.0...v1.8.9-0.6.0
diff --git a/src/main/java/eu/olli/cowlection/command/MooCommand.java b/src/main/java/eu/olli/cowlection/command/MooCommand.java
index 1798568..bd90564 100644
--- a/src/main/java/eu/olli/cowlection/command/MooCommand.java
+++ b/src/main/java/eu/olli/cowlection/command/MooCommand.java
@@ -143,15 +143,14 @@ public class MooCommand extends CommandBase {
                                 .append(EnumChatFormatting.RED).append("Unknown minion ").append(EnumChatFormatting.YELLOW).append("(new or with minion skin) ").append(tierColor).append(minionTier);
                     });
             main.getChatHelper().sendMessage(EnumChatFormatting.YELLOW, analysisResults.toString());
-        } else if (args.length == 2 && args[0].equalsIgnoreCase("add")) {
-            handleBestFriendAdd(args[1]);
-        } else if (args.length == 2 && args[0].equalsIgnoreCase("remove")) {
-            handleBestFriendRemove(args[1]);
+        } else if (args[0].equalsIgnoreCase("add")) {
+            handleBestFriendAdd(args);
+        } else if (args[0].equalsIgnoreCase("remove")) {
+            handleBestFriendRemove(args);
         } else if (args[0].equalsIgnoreCase("list")) {
             handleListBestFriends();
         } else if (args[0].equalsIgnoreCase("nameChangeCheck")) {
-            main.getChatHelper().sendMessage(EnumChatFormatting.GOLD, "Looking for best friends that have changed their name... This will take a few seconds...");
-            main.getFriendsHandler().updateBestFriends(true);
+            handleNameChangeCheck(args);
         }
         // sub-commands: miscellaneous
         else if (args[0].equalsIgnoreCase("config") || args[0].equalsIgnoreCase("toggle")) {
@@ -163,7 +162,10 @@ public class MooCommand extends CommandBase {
             if (args.length == 1) {
                 main.getChatHelper().sendMessage(EnumChatFormatting.GREEN, "\u279C Current GUI scale: " + EnumChatFormatting.DARK_GREEN + currentGuiScale);
             } else {
-                int scale = Math.min(10, MathHelper.parseIntWithDefault(args[1], 6));
+                int scale = MathHelper.parseIntWithDefault(args[1], -1);
+                if (scale == -1 || scale > 10) {
+                    throw new NumberInvalidException(EnumChatFormatting.DARK_RED + args[1] + EnumChatFormatting.RED + " is an invalid GUI scale value. Valid values are integers below 10");
+                }
                 Minecraft.getMinecraft().gameSettings.guiScale = scale;
                 main.getChatHelper().sendMessage(EnumChatFormatting.GREEN, "\u2714 New GUI scale: " + EnumChatFormatting.DARK_GREEN + scale + EnumChatFormatting.GREEN + " (previous: " + EnumChatFormatting.DARK_GREEN + currentGuiScale + EnumChatFormatting.GREEN + ")");
             }
@@ -414,26 +416,28 @@ public class MooCommand extends CommandBase {
         });
     }
 
-    private void handleBestFriendAdd(String username) throws CommandException {
-        if (!Utils.isValidMcName(username)) {
-            throw new InvalidPlayerNameException(username);
-        }
-
-        // TODO Add check if 'best friend' is on normal friend list
-        if (main.getFriendsHandler().isBestFriend(username, true)) {
-            throw new MooCommandException(EnumChatFormatting.DARK_RED + username + EnumChatFormatting.RED + " is a best friend already.");
+    private void handleBestFriendAdd(String[] args) throws CommandException {
+        if (args.length != 2) {
+            throw new WrongUsageException("/" + getCommandName() + " add <playerName>");
+        } else if (!Utils.isValidMcName(args[1])) {
+            throw new InvalidPlayerNameException(args[1]);
+        } else if (main.getFriendsHandler().isBestFriend(args[1], true)) {
+            throw new MooCommandException(EnumChatFormatting.DARK_RED + args[1] + EnumChatFormatting.RED + " is a best friend already.");
         } else {
-            main.getChatHelper().sendMessage(EnumChatFormatting.GOLD, "Fetching " + EnumChatFormatting.YELLOW + username + EnumChatFormatting.GOLD + "'s unique user id. This may take a few seconds...");
+            // TODO Add check if 'best friend' is on normal friend list
+            main.getChatHelper().sendMessage(EnumChatFormatting.GOLD, "Fetching " + EnumChatFormatting.YELLOW + args[1] + EnumChatFormatting.GOLD + "'s unique user id. This may take a few seconds...");
             // add friend async
-            main.getFriendsHandler().addBestFriend(username);
+            main.getFriendsHandler().addBestFriend(args[1]);
         }
     }
 
-    private void handleBestFriendRemove(String username) throws CommandException {
-        if (!Utils.isValidMcName(username)) {
-            throw new InvalidPlayerNameException(username);
+    private void handleBestFriendRemove(String[] args) throws CommandException {
+        if (args.length != 2) {
+            throw new WrongUsageException("/" + getCommandName() + " remove <playerName>");
+        } else if (!Utils.isValidMcName(args[1])) {
+            throw new InvalidPlayerNameException(args[1]);
         }
-
+        String username = args[1];
         boolean removed = main.getFriendsHandler().removeBestFriend(username);
         if (removed) {
             main.getChatHelper().sendMessage(EnumChatFormatting.GREEN, "Removed " + EnumChatFormatting.DARK_GREEN + username + EnumChatFormatting.GREEN + " from best friends list.");
@@ -446,7 +450,25 @@ public class MooCommand extends CommandBase {
         Set<String> bestFriends = main.getFriendsHandler().getBestFriends();
 
         // TODO show fancy gui with list of best friends; maybe with buttons to delete them
-        main.getChatHelper().sendMessage(EnumChatFormatting.GREEN, "\u279C Best friends: " + EnumChatFormatting.DARK_GREEN + String.join(EnumChatFormatting.GREEN + ", " + EnumChatFormatting.DARK_GREEN, bestFriends));
+        main.getChatHelper().sendMessage(EnumChatFormatting.GREEN, "\u279C Best friends: " + ((bestFriends.isEmpty())
+                ? EnumChatFormatting.ITALIC + "none :c"
+                : EnumChatFormatting.DARK_GREEN + String.join(EnumChatFormatting.GREEN + ", " + EnumChatFormatting.DARK_GREEN, bestFriends)));
+    }
+
+    private void handleNameChangeCheck(String[] args) throws CommandException {
+        if (args.length != 2) {
+            throw new WrongUsageException("/" + getCommandName() + " nameChangeCheck <playerName>");
+        } else if (!Utils.isValidMcName(args[1])) {
+            throw new InvalidPlayerNameException(args[1]);
+        }
+        Friend bestFriend = main.getFriendsHandler().getBestFriend(args[1]);
+        if (bestFriend.equals(Friend.FRIEND_NOT_FOUND)) {
+            throw new MooCommandException(EnumChatFormatting.DARK_RED + args[1] + EnumChatFormatting.RED + " isn't a best friend.");
+        } else {
+            main.getChatHelper().sendMessage(EnumChatFormatting.GOLD, "Checking if " + bestFriend.getName() + " changed their name... This will take a few seconds...");
+            // check for name change async
+            main.getFriendsHandler().updateBestFriend(bestFriend, true);
+        }
     }
 
     @Override
@@ -468,7 +490,7 @@ public class MooCommand extends CommandBase {
                 .appendSibling(createCmdHelpEntry("add", "Add best friends"))
                 .appendSibling(createCmdHelpEntry("remove", "Remove best friends"))
                 .appendSibling(createCmdHelpEntry("list", "View list of best friends"))
-                .appendSibling(createCmdHelpEntry("nameChangeCheck", "Force a scan for changed names of best friends"))
+                .appendSibling(createCmdHelpEntry("nameChangeCheck", "Force a scan for a changed name of a best friend"))
                 .appendSibling(createCmdHelpEntry("toggle", "Toggle join/leave notifications"))
                 .appendSibling(createCmdHelpSection(2, "Miscellaneous"))
                 .appendSibling(createCmdHelpEntry("config", "Open mod's configuration"))
diff --git a/src/main/java/eu/olli/cowlection/handler/FriendsHandler.java b/src/main/java/eu/olli/cowlection/handler/FriendsHandler.java
index 27e4a3a..39a49e2 100644
--- a/src/main/java/eu/olli/cowlection/handler/FriendsHandler.java
+++ b/src/main/java/eu/olli/cowlection/handler/FriendsHandler.java
@@ -4,10 +4,10 @@ import com.google.gson.JsonParseException;
 import com.google.gson.reflect.TypeToken;
 import eu.olli.cowlection.Cowlection;
 import eu.olli.cowlection.command.exception.ApiContactException;
+import eu.olli.cowlection.command.exception.MooCommandException;
 import eu.olli.cowlection.data.Friend;
 import eu.olli.cowlection.util.ApiUtils;
 import eu.olli.cowlection.util.GsonUtils;
-import eu.olli.cowlection.util.TickDelay;
 import io.netty.util.internal.ConcurrentSet;
 import net.minecraft.command.PlayerNotFoundException;
 import net.minecraft.event.ClickEvent;
@@ -25,22 +25,21 @@ import java.util.Set;
 import java.util.TreeSet;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
 public class FriendsHandler {
-    private static final long UPDATE_FREQUENCY_DEFAULT = TimeUnit.HOURS.toMillis(15);
-    private static final long UPDATE_FREQUENCY_MINIMUM = TimeUnit.MINUTES.toMillis(5);
+    private static final long UPDATE_FREQUENCY_DEFAULT = TimeUnit.HOURS.toMillis(10);
     private final Cowlection main;
-    private Set<Friend> bestFriends = new ConcurrentSet<>();
-    private File bestFriendsFile;
-    private UpdateStatus updateStatus;
+    private final Set<Friend> bestFriends = new ConcurrentSet<>();
+    private final File bestFriendsFile;
+    private final AtomicInteger bestFriendQueue = new AtomicInteger();
 
     public FriendsHandler(Cowlection main, File friendsFile) {
         this.main = main;
         this.bestFriendsFile = friendsFile;
-        this.updateStatus = UpdateStatus.IDLE;
         loadBestFriends();
-        updateBestFriends(false);
+        updateBestFriends();
     }
 
     public boolean isBestFriend(String playerName, boolean ignoreCase) {
@@ -91,24 +90,21 @@ public class FriendsHandler {
         return bestFriends.stream().filter(friend -> friend.getUuid().equals(uuid)).findFirst().orElse(Friend.FRIEND_NOT_FOUND);
     }
 
-    public void updateBestFriends(boolean isCommandTriggered) {
-        bestFriends.stream().filter(friend -> System.currentTimeMillis() - friend.getLastChecked() > (isCommandTriggered ? UPDATE_FREQUENCY_MINIMUM : UPDATE_FREQUENCY_DEFAULT)).forEach(this::updateBestFriend);
-
-        new TickDelay(() -> {
-            if (this.updateStatus != UpdateStatus.IDLE) {
-                if (isCommandTriggered && updateStatus == UpdateStatus.NO_NAME_CHANGES) {
-                    main.getChatHelper().sendMessage(EnumChatFormatting.GOLD, "No name changes detected.");
-                }
-                saveBestFriends();
-                this.updateStatus = UpdateStatus.IDLE;
-            }
-        }, 10 * 20);
+    public void updateBestFriends() {
+        bestFriends.stream().filter(friend -> System.currentTimeMillis() - friend.getLastChecked() > UPDATE_FREQUENCY_DEFAULT)
+                .forEach(friend1 -> {
+                    bestFriendQueue.incrementAndGet();
+                    updateBestFriend(friend1, false);
+                });
     }
 
-    private void updateBestFriend(Friend friend) {
+    public void updateBestFriend(Friend friend, boolean isCommandTriggered) {
         ApiUtils.fetchCurrentName(friend, newName -> {
             if (newName == null) {
                 // skipping friend, something went wrong with API request
+                if (isCommandTriggered) {
+                    throw new ApiContactException("Mojang", "couldn't check " + EnumChatFormatting.DARK_RED + friend.getName() + EnumChatFormatting.RED + " (possible) new player name");
+                }
             } else if (newName.equals(ApiUtils.UUID_NOT_FOUND)) {
                 throw new PlayerNotFoundException("How did you manage to get a unique id on your best friends list that has no name attached to it?");
             } else if (newName.equals(friend.getName())) {
@@ -116,8 +112,8 @@ public class FriendsHandler {
                 Friend bestFriend = getBestFriend(friend.getUuid());
                 if (!bestFriend.equals(Friend.FRIEND_NOT_FOUND)) {
                     bestFriend.setLastChecked(System.currentTimeMillis());
-                    if (this.updateStatus == UpdateStatus.IDLE) {
-                        this.updateStatus = UpdateStatus.NO_NAME_CHANGES;
+                    if (isCommandTriggered) {
+                        throw new MooCommandException(friend.getName() + " hasn't changed his name");
                     }
                 }
             } else {
@@ -131,7 +127,15 @@ public class FriendsHandler {
                 if (!bestFriend.equals(Friend.FRIEND_NOT_FOUND)) {
                     bestFriend.setName(newName);
                     bestFriend.setLastChecked(System.currentTimeMillis());
-                    this.updateStatus = UpdateStatus.NAME_CHANGED;
+                }
+            }
+            if (isCommandTriggered) {
+                saveBestFriends();
+            } else {
+                int remainingFriendsToCheck = bestFriendQueue.decrementAndGet();
+                if (remainingFriendsToCheck == 0) {
+                    // we're done with checking for name changes, save updates to file!
+                    saveBestFriends();
                 }
             }
         });
@@ -169,8 +173,4 @@ public class FriendsHandler {
         }.getType();
         return GsonUtils.fromJson(bestFriendsData, collectionType);
     }
-
-    private enum UpdateStatus {
-        IDLE, NAME_CHANGED, NO_NAME_CHANGES
-    }
 }