Multiple Arguments In One Command Help

Discussion in 'Plugin Development' started by devdiamond, Apr 22, 2016.

Thread Status:
Not open for further replies.
  1. Offline

    devdiamond

    Ok So I hhave been working on a plugin i hope to release soon as my first plugin on bukkit, but my plugin wont work as i need it to. my issue is that the command i try to use is a base command like this



    Code:
    public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args){
      
            if (cmd.getName().equalsIgnoreCase("derp") && sender instanceof Player)
            {
      
                 Player p = (Player) sender;
          
                p.sendMessage(ChatColor.GOLD+"plugin Info");
      
                if(args.length == 2){
          
                    if(args[0].equalsIgnoreCase("derp")){
                    Player target = Bukkit.getServer().getPlayer(args[1]);
                    }
                }
        
    
                Bukkit.broadcastMessage(ChatColor.GOLD+"i derped out"+args[1]+);
          
          
            }
    
    
    say someone says "derp" as a base command and it pops up with things about the plugin. Then Someone types "derp derp (player)" to kick the player and it still sends the plugin info to the player as well as kicking the player.
    in better detail my plugin keeps running the base command with the other command arguments such as say "/derp"
    and "derp fly" or something it would still run derp and derp fly
     
    Last edited: Apr 22, 2016
  2. Offline

    567legodude

    @devdiamond My guess is you are returning false at the end of onCommand, whenever you return false to a command you are telling Bukkit that something went wrong, and that it should send the help info to the player.
     
  3. Offline

    devdiamond

    when i change it to true nothing happens at all the commands dont say "/help to see a list of commands" flat out nothing happens
     
  4. Offline

    Zombie_Striker

    @devdiamond
    You never explained your problem, you just said what is currently happening without saying why it's wrong/ what is should do.
     
  5. Offline

    devdiamond

    in better detail my plugin keeps running the base command with the other command arguments such as say "/derp"
    and "derp fly" or something it would still run derp and derp fly
     
  6. Offline

    Zombie_Striker

    @devdiamond
    Add more checks and returns. If one commend is selected, return (which stops the command from readi1ng any other commands), or use else statements for each argument, that way only one sub-command can run.
     
  7. Offline

    elian1203

    You're casting the sender as a player before checking. That can cause errors. You're not doing anything but getting the target player if the arguments length is two, however I'm not sure what you're trying to do. Send a message to the command sender containing the target's name?
     
  8. Offline

    Davesan

    You should code that like this:
    Code:
    public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
        if (cmd.getName().equalsIgnoreCase("derp") && sender instanceof Player) {
            Player p = (Player) sender;
            if (args.length == 0) {
               p.sendMessage(ChatColor.GOLD + "plugin Info");
            } else if (args.length == 2) {
               if(args[0].equalsIgnoreCase("derp")){
                  Player target = Bukkit.getServer().getPlayer(args[1]);
                  if (target != null) {
                      //kick player or whatever
                      Bukkit.broadcastMessage(ChatColor.WHITE + p.getDisplayName() + ChatColor.GOLD + " depred out " + ChatColor.WHITE + target.getDisplayName() + ChatColor.GOLD + '!');
                  } else
                      p.sendMessage(ChatColor.RED + "Player not found: " + ChatColor.YELLOW + args[1]);
               } else
                  p.sendMessage(ChatColor.GRAY + "Invalid derp command, /derp for help.");
            } else
               p.sendMessage(ChatColor.GRAY + "Invalid derp command, /derp for help.");
    
            return true; //true means not to show the default usage message
        }
        return false; //false means to show the usage described in the plugin.yml for the command
    }
    [EDITED]
     
    Last edited: Apr 27, 2016
  9. Offline

    I Al Istannen

    @Davesan
    • This if nesting is just incredible. It makes the whole thing extremly hard to read.
    • Returning false doesn't mean you "don't care about the command at all", it means that the usage message defined in the plugin.yml will be send.
    • Similarily returning true means to not send the usage message.
    • You could use Bukkit.getPlayer(String) instead. Both methods should return null if the player is not online.
    • The Player not found message should be the Targeted player not online, as the getPlayer() command will return null if the player is not online.
    • If the player is online he has played before. No need for checking that.
    • No comments. Why do you do what you do?

    Apart from that you could read this thread, and the first paragraph. Spoonfeeding isn't that good, unless you don't make any mistakes, your code is extremly well structured, has no bad practices AND the other person actually understands what you did and is able to apply it to different situations. It is explained in more detail in the liked thread. In short, you shouldn't do it.

    I don't want to offend you or anything, and I think you had good intentions, but spoonfeeding is probably not the best way to help :)
     
  10. Offline

    Davesan

    @I Al Istannen It seems that I worked too many with OfflinePlayer objects the last time ><" There is .isOnline() to get if it is online, and .hasPlayed() is a valuable stuff, because OfflinePlayer object is always returned, even if there is no player ever played on the server on that name. Anyways, thanks for the mentions, I change my code above according to it. The "already defined the usage" was unknown to me, since I always use command "roots" so there is not only one correct usage of the command, but like a hundred. So, for me, false always showed the same x)

    The nesting is required, I think. There are multiple parameters, what you should check. Sure, you can use validate() stuff, and a "break-if-not" structured code, which I use sometimes.. but don't think if some if's are hard to read either.
     
  11. Offline

    I Al Istannen

    @Davesan
    args.length() is no method. It should be args.length, as it is a variable.
    Yes, the if nesting is partly needed. The logic is, but not the structure. And, it may be prefernce, but I would take this
    (Code not 1:1 applicable to question, just showing a different way of structuring a method. You would need to at least check for args.length == 0 to show the menu to make what the op wanted.)
    Code:
        public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
            // Depends on how you register it, may be unneeded
            if(!cmd.getName().equalsIgnoreCase("derp") || !(sender instanceof Player)) {
                return true; // or false, I would move this method in a dedicated CommandExecutor, so the first check for the command name would be entirely useless
            }
        
            // name the variables descriptive. It does really help, although it may be tedious. That is what CTRL+ALT+SPACE is for.
            Player player = (Player) sender;
        
            // not enough args, show usage information
            if(args.length < 2) {
                return false;
            }
        
            // if it is not a valid derp send a message
            if(!args[0].equalsIgnoreCase("derp")) {
                player.sendMessage("Invalid derp");
                return true;
            }
        
            Player targetPlayer = Bukkit.getPlayer(args[1]);
        
            // player not online
            if(targetPlayer == null) {
                player.sendMessage("Player not online");
                return true;
            }
        
            // do what you want. Kick himn, send a message, idk.
            player.sendMessage("Action will be executed for " + targetPlayer.getDisplayName());
        
            return false;
        }
    over this

    Code:
        public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
            if (cmd.getName().equalsIgnoreCase("derp") && sender instanceof Player) {
                Player p = (Player) sender;
                if (args.length == 0) {
                   p.sendMessage(ChatColor.GOLD + "plugin Info");
                } else if (args.length == 2) {
                   if(args[0].equalsIgnoreCase("derp")){
                      Player target = Bukkit.getServer().getPlayer(args[1]);
                      if (target != null) {
                          //kick player or whatever
                          Bukkit.broadcastMessage(ChatColor.WHITE + p.getDisplayName() + ChatColor.GOLD + " depred out " + ChatColor.WHITE + target.getDisplayName() + ChatColor.GOLD + '!');
                      } else
                          p.sendMessage(ChatColor.RED + "Player not found: " + ChatColor.YELLOW + args[1]);
                   } else
                      p.sendMessage(ChatColor.GRAY + "Invalid derp command, /derp for help.");
                } else
                   p.sendMessage(ChatColor.GRAY + "Invalid derp command, /derp for help.");
     
                return true; //true means not to show the default usage message
            }
            return false; //false means to show the usage described in the plugin.yml for the command
        }
    any day.

    BUT to make it back to this thread. My main intention was to stop you from spoonfeeding. There are some flaws in your code and no comments. I don't know how it will help the off poster.

    I am honestly sorry if this is too offtopic to be kept in this thread, don't want to cause the mods trouble, but want to discuss and maybe correct parts of his code at the same time.
     
    Last edited: Apr 27, 2016
Thread Status:
Not open for further replies.

Share This Page