Repeating code

Discussion in 'Plugin Development' started by Jayyy, Aug 19, 2015.

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

    Jayyy

    [​IMG] So, I was making a kick plugin and I decided to add a reason with the kick, and so I got it no errors but when I kick someome it repeats the code for instance

    Heres the screenshot:

    Heres my code:
    Code:
    package me.jay.factionessentials;
    
    import org.bukkit.Bukkit;
    import org.bukkit.ChatColor;
    import org.bukkit.command.Command;
    import org.bukkit.command.CommandExecutor;
    import org.bukkit.command.CommandSender;
    import org.bukkit.entity.Player;
    
    public class Kick implements CommandExecutor{
     
        Main plugin;
        public Kick(Main passedPlugin)
        {
            this.plugin = passedPlugin;
        }
        @Override
        public boolean onCommand(CommandSender sender, Command cmd, String label,String[] args) {
            if(cmd.getName().equalsIgnoreCase("kick")) {
                if(args.length == 0) {
                    sender.sendMessage(ChatColor.RED + "Usage: /Kick <player> <reason>");
                }
                Player p = (Player)sender;
                @SuppressWarnings({ "deprecation", "unused" })
                Player tp = p.getServer().getPlayer(args[0]);
                if(args.length > 0){
                    String broadcast = "§c";
                    for(String message :args){
                        broadcast = (broadcast + message + " Has been kicked by " + sender.getName() + " for ");
                     
    
                    }
                    tp.kickPlayer(broadcast);
                    return true;
                }
            return false;
        }
            return false;
        }
    }
    
     
    Last edited: Aug 19, 2015
  2. Offline

    finalblade1234

    In one if statement you're saying if (args.length == 1), but then on the next line you've put if(args.length == 0), args.length cannot be both 0 and 1
     
  3. Offline

    Jayyy

    Still not working :l
     
  4. Offline

    finalblade1234

    Could you show me your new code?
    @Jayyy
     
  5. Offline

    AdobeGFX

    Code:
    if (cmd.getName().equalsIgnoreCase("unban")) {
                                Player player = (Player)sender;
                                OfflinePlayer target = player.getServer().getOfflinePlayer(args[0]);
                                OfflinePlayer oplayer = getServer().getOfflinePlayer(args[0]); //You are already getting the same player above..
                             
                                if (sender.hasPermission("jay.unban")){
                                if (cmd.getName().equalsIgnoreCase("unban") && args.length == 1) { //You already checked if the command is unban, so you don't need to do it again
                                    if(args.length == 0) { //Move this..
                                        p.sendMessage(ChatColor.RED + "Usage: /unban <playername>" );
                                        return true;
                                    }
                                    if (sender.hasPermission("jay.unban")){ //You already checked if the player has permission
                                            target.setBanned(false);
                                            player.sendMessage(target.getName() + " is now unbanned!");
                                            oplayer.setBanned(false); //You are unbanning the same player twice..
                                            return true;
                                    }
                                }
                                }else{
                                    player.sendMessage(ChatColor.RED + "You don't have permissions to do that!");
                                    return true;
                                }
    Remove 3 things and move 1 thing.. simpel as that
    Code:
                                if(args.length == 0) { //This goes outside args == 1..
                                 //something
                                    }
                                if (args.length == 1) {
                                           //something
                                }
                                }
     
  6. Offline

    Synapz

    What
    @finalblade1234 is trying to say is use else if not if because it will never be o and 1.

    I notice a ton of mistakes and bad pratices:
    - Dont check cmd.getName again after you have already checked above it
    - Before you cast sender to player check if its an instancof player. If i were to do this I wouldnt even cast because every CommandSender is a Player so you give support to console and players, because you arent using any player specific methods
    - Use else if instead of if
    - Dont check if they have permission more than once, one time is enough

    Also what do you mean it isnt working? Does it show errors or it just doesnt unban?

    Btw @AdobeGFX i love your signature :p
     
  7. Offline

    Jayyy

    Anyone got the Solution?
     
  8. Offline

    mythbusterma

  9. Offline

    Jayyy

    That was for something else, this is an edited post.
     
  10. Offline

    mythbusterma

    @Jayyy

    I'm going to assume your code still has all the same problems he pointed out, so why don't you fix those and then actually think about what that for-loop is doing, then we'll be able to get somewhere.
     
  11. Offline

    AdobeGFX

    You should just create a new thread instead of changing everything..
    it's the same as broadcasting a custom message.. You use StringBuilder for example.

    In your case, you use it like this:
    Code:
    public String reason(String[] args) {
            StringBuilder builder = new StringBuilder();
            for (int i = 1; i < args.length; i++) {
            builder.append(args[i]);
            builder.append(" ");
            }
            return builder.toString().trim();
        }
    Code:
    Player tp = p.getServer().getPlayer(args[0]);
    Remember to check if the target is not null before getting the target player!
     
Thread Status:
Not open for further replies.

Share This Page