Command arguments aren't working

Discussion in 'Spigot Plugin Development' started by CameronVezina, Jan 9, 2020.

  1. This is my command...
    Code (Java):
        @Override
        public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
            if (label.equalsIgnoreCase("crates")) {
                if (args.length == 0) {
                    if (sender instanceof Player) {
                        Player p = (Player) sender;
                        p.openInventory(cratesGUI);
                    }
                } else {
                    if (args.length < 1) {
                        if (sender.hasPermission("crates.edit")) {
                            if (args[0].equalsIgnoreCase("give")) {
                                if (args[1].length() >= 3) {
                                    if (args[2].equalsIgnoreCase("seed")) {
                                        if (isInt(args[3])) {
                                            sender.sendMessage("§e§l[!] §e" + args[1] + " has been given " + args[3] + "x "
                                                    + args[2].toUpperCase() + " keys.");
                                        } else {
                                            sender.sendMessage("§c§l[!] §cYou must specify how many keys to give.");
                                        }
                                    } else if (args[2].equalsIgnoreCase("lucky")) {
                                        //
                                    } else if (args[2].equalsIgnoreCase("event")) {
                                        //
                                    } else if (args[2].equalsIgnoreCase("vote")) {
                                        //
                                    } else {
                                        sender.sendMessage("§c§l[!] §cYou must specify a key to give.");
                                    }
                                } else {
                                    sender.sendMessage("§c§l[!] §cYou must specify a player.");
                                }
                            } else if (args[0].equalsIgnoreCase("take")) {
                                sender.sendMessage("take debug");
                            } else {
                                sender.sendMessage("§c§l[!] §cMissing arguments, try");
                                sender.sendMessage("§c§l§m->§c /crates give <username> <key> <amount>");
                                sender.sendMessage("§c§l§m->§c /crates take <username> <key> <amount>");
                            }
                        } else {
                            sender.sendMessage("§e§l[!] §eYou don't have permission to use this command.");
                        }
                    }

                }
            }
            return true;
        }
    Whenever I type "/crates give", "/crates give aient", "/crates give aient seed", or "/crates give aient seed 1", nothing shows up in chat or in console...
     
  2. Code (Text):
    if (args.length < 1)
    I'm sure you mean >= 1 here. If it was < 1 then all the code within it is redundant.
     
  3. args.length < 1 is the same as args.length == 0, you need if args.length == 1

    Edit: typed too slow again

    edit2:
    actually you can just remove the if(args.length < 1) completely, if the code makes it to this point we know its not 0 because of the previous if statement
     
  4. Thanks, that seemed to fix the "/crates give aient seed 1" issue, but now "/crates give", "/crates give aient", "/crates give aient seed" all spit out a null error, https://pastebin.com/2wSntmsm
     
  5. I have some advice. I suggest you do not nest that many if statements.
    What I mean by this, is that you can make an if statement that will return true if the condition is true. This is helpful in making your code look cleaner and much more readable. :)
     
    • Agree Agree x 2
  6. Could you provide the method containing this line? CratesCMD.java:113
     
  7. if (args[1].length() >= 3) {
    (Specifying a player)

    Not really sure how to do that
     
  8. I suggest looking through some source code of plugins and seeing how developers use return statements instead of nesting if statements.
    Using return; is very important in Java to make your code quite effective as well.
     
  9. Code (Java):
     if (args[0].equalsIgnoreCase("give")) {
                                if (args[1].length() >= 3) {
                                    if (args[2].equalsIgnoreCase("seed")) {
    the first line has no errors, because you checked that args[] cointains at least one argument
    Code (Java):
    if (args.length == 0) {
       ...
    }
    else {
       ...
    }
    In the second line you're taking the second element of the args array, but what if the array contains only one element? You're not checking that, so it may result in an exception. From what I understood, you're checking if the command contains at least 4 words (the command and 3 arguments), so you should check if args.length is at least 3 to ensure you have the minimum number of arguments. I'm not sure what do you want to do with
    Code (Java):
    if (args[1].length() >= 3) {
    I guess you were trying to check this thing I just wrote
     
  10. Code (Text):
    if (args[1].length() >= 3)
    is checking if the 2nd argument is a player's name (at least 3 characters)
     
  11. I suppose you want to give the crate to an online player. Then you can just get your player with
    Code (Java):
    Player player = Bukkit.getPlayer(args[1]);
    and check if player is null, to determine if the player is online or not (if player is null, then the player is not online and you should display some error message instead of trying to give items to a null player)
     
  12. Did you register your command into plugin.yml ?

    Your plugin.yml should be like this:

    Code (YAML):
    name: PluginName
    author
    : TheAuthor
    main
    : YourMainClassName

    Commands
    :
         crates
    :
             aliases
    : [crates]
             description
    : Optional/Command Description
    Edit:

    Also, did you register commands class into main class ?
    Main class should be like this:
    Code (Java):
    getServer().getPluginManager().registerEvents(this, this);
    getServer().getPluginManager().registerEvents(this, new YourCommandsClass());
    or
    Code (Java):
    getServer().getPluginManager().registerEvents(this, this);
    getCommand("crates").setExecutor(new YourCommandsClass()) ;
     
    #12 Vlad1101, Jan 10, 2020
    Last edited: Jan 10, 2020
  13. No I don't just want to give crates to online players, I want to give the crate to an online player. Besides the point, thats not my problem.


    Yes I've registered it into the plugin.yml and added it into my main class, that's not the issue, the issue is the arguments.
     
  14. I've gone ahead and reformatted your command.
    Code (Java):
        @Override
        public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
            if (!(label.equalsIgnoreCase("crates")))
                return false; // If not our command, dont handle, this is a negative check
            if (args.length == 0) {
                if (sender instanceof Player) {
                    Player p = (Player) sender;
                    p.openInventory(cratesGUI);
                    return true;
                }
                // and if they arent a player? Make sure that EVERY possiblity is covered.
            }

            // Note that the arg check should logically be after the perm check.
            if (args.length < 1) {
                // send some sort of message
                return false;
            }

            // This is an example of a negative check. Check for false, then return so we can save indention and readability
            if (!sender.hasPermission("crates.edit")) {
                sender.sendMessage("§e§l[!] §eYou don't have permission to use this command.");
                return false;
            }

            if (args[0].equalsIgnoreCase("give")) {
                if (!(args[1].length() >= 3)) {
                    // This is a bad way to check this, use Bukkit#getPlayer, and check for null
                    sender.sendMessage("§c§l[!] §cYou must specify a player.");
                    return false;
                }
                if (args[2].equalsIgnoreCase("seed")) {
                    // im 100% sure your isInt it not better then Integer#parseInt#
                    if (isInt(args[3])) {
                        sender.sendMessage("§e§l[!] §e" + args[1] + " has been given " + args[3] + "x "
                                + args[2].toUpperCase() + " keys.");
                        return true;
                    } else {
                        sender.sendMessage("§c§l[!] §cYou must specify how many keys to give.");
                        return false;
                    }
                }
                if (args[2].equalsIgnoreCase("lucky")) {
                    return true;
                }
                if (args[2].equalsIgnoreCase("event")) {
                    return true;
                }
                if (args[2].equalsIgnoreCase("vote")) {
                    return true;
                }
                sender.sendMessage("§c§l[!] §cYou must specify a key to give.");
                return false;
            }
            if (args[0].equalsIgnoreCase("take")) {
                sender.sendMessage("take debug");
                return true;
            }
            sender.sendMessage("§c§l[!] §cMissing arguments, try");
            sender.sendMessage("§c§l§m->§c /crates give <username> <key> <amount>");
            sender.sendMessage("§c§l§m->§c /crates take <username> <key> <amount>");
            return true;
        }
    I have added comments explaining various things. Let me know if you need help with anything I have written.

    EDIT: DO NOT use copy and paste, read and learn
     
  15. Strahan

    Benefactor

    That's still a little messy. I'd use switch instead of a boatload of IFs. Also checking label is not recommended, you should get the name of the command and if you keep your classes clean you really shouldn't need to do that either as it's best to register each command to its own class. Also don't embed the color character. Either use the ChatColor enum to embed the desired codes (ChatColor.BOLD + ChatColor.RED for example) or the translateAlternateColorCodes method to convert & to color codes. As that's a bitch to type over and over, I would make a little function to do it then call that. Though me, personally, I'd use a messaging function to leverage a message config so the end user can change the messages easily if they wish rather than hardcoding it at all.

    As to structure, I'd do something akin to this. I wasn't entirely clear on the command usage parameters so that part may be off, but it's a general idea:
    Code (Text):
    @Override
    public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
      if (args.length == 0) {
        if (sender instanceof Player) {
          ((Player) sender).openInventory(cratesGUI);
          return true;
        }
     
        // Send help message to console user
        return true;
      }

      if (!sender.hasPermission("crates.edit")) {
        sender.sendMessage(cc("&e&l[!] &eYou don't have permission to use this command."));
        return true;
      }

      switch (args[0].toLowerCase()) {
      case "give":
        if (args.length < 3) {
          // show help
          break;
        }

        Player target = getServer().getPlayer(args[1]);
        if (target == null) {
          sender.sendMessage(cc("&c&l[!] &cYou must specify a player that is online."));
          break;
        }

        switch (args[2].toLowerCase()) {
        case "seed":
          try {
            int qty = Integer.parseInt(args[3]);
          } catch (NumberFormatException e) {
            // send an error
            break;
          }

          sender.sendMessage(cc("&e&l[!] &e" + args[1] + " has been given " + args[3] + "x " + args[2].toUpperCase() + " keys."));
         break;

        case "lucky":
          break;

        case "event":
          break;

        case "vote":
          break;

        default:
          send invalid option message
          break;
        }
        break;

      case "take":
        // do stuff
        break;

      default:
        // send invalid option msg
        break;
      }

      return true;
    }

    private String cc(String msg) {
      return ChatColor.translateAlternateColorCodes('&', msg);
    }
     
    #15 Strahan, Jan 11, 2020
    Last edited: Jan 11, 2020
  16. In any case, not the best way for sub-commands any how :)
     
    #16 xXXIMMAMTTHEWXXx, Jan 11, 2020
    Last edited: Jan 11, 2020
  17. Strahan

    Benefactor

    I think it is fine. As long as checks are done logically and you switch on args while properly handling invalid input, it's simple and efficient. I usually don't put the "meat" of the subcommand in the command switch, I call functions for the processing of each, but the general struct is pretty clean IMO. I'm curious, if you don't think that's the way to go, how do you usually handle it?
     
  18. Never said it was no the absolute only way to go, everyone makes everything differently. Most times, commands are manually mapped into the command map manually, and each command object points a sub-command to it its own executer. Although it being that most commands dont require many sub commands anyhow.
     
  19. Strahan

    Benefactor

    I know, it wasn't a "well what the hell do YOU do then?" type of reply, I just always am curious to learn other possibly better ways to do things ;)