Day Command

Discussion in 'Spigot Plugin Development' started by RedstonecraftHD, Aug 17, 2018.

  1. Hi guys!

    I have a problem with my Source Code.

    I don`t know why it is not working when i type /day. It only works when i Type /day <world> .

    Any help is appreciated, thanks in advance!

    Code (Text):
    package de.RedstonecraftHD.Plugin.commands;

    import org.bukkit.Bukkit;
    import org.bukkit.World;
    import org.bukkit.command.Command;
    import org.bukkit.command.CommandExecutor;
    import org.bukkit.command.CommandSender;
    import org.bukkit.entity.Player;

    public class DayCommand implements CommandExecutor {

        @Override
        public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
            Player p =(Player) sender;
            World w = p.getWorld();
            World a = Bukkit.getWorld(args[0]);
       
           
           
            if(sender instanceof Player) {
            if(p.hasPermission("redchat.time.day") ||p.hasPermission("redchat.*") ) {
                if(args.length == 0) {
                    w.setTime(5000);
                    Bukkit.broadcastMessage("§aDie Zeit in §6§l" + w.getName() + "§r§a wurde auf Tag gestellt!");
           
               
               
               
               
               
               
                } else
                    if(p.hasPermission("redchat.time.day.world") ||p.hasPermission("redchat.*") ) {
                        if(args.length == 1) {
                            if(Bukkit.getWorld(args[0]) != null){
                                a.setTime(5000);
                                p.sendMessage("§aTest!");
                               
                            } else
                                p.sendMessage("Welt nicht gefunden");
                           
                        } else
                            p.sendMessage("Meintest du /day <Welt>");
                    } else
                        p.sendMessage("Rechte");
                   
               
                    }  else
                        p.sendMessage("§d[Redchat] §4§lDu hast kein Recht um diesen Befehl auszuführen!");
           
                   
           
           
           
           
           
           
           
            return false;
        }
            return false;
    }
    }
     
  2. Well as Minecraft is multiple worlds, test for the player (sender)'s current world and set their world to that time. @RedstonecraftHD
    You are also testing for the world in the argument args[0], if you looked at your code.

    Edited:

    I would also add to follow Bukkit's basic guidelines for player message colors, from using:
    Code (Text):
    p.sendMessage(ChatColor.translateAlternateColorCodes('&', "message"))
     
  3. Choco

    Moderator

    I was going to suggest some changes, but holy formatting. Could that be fixed please? Unnecessary spacing, strange indentation, missing brackets, etc. It's near impossible to read

    EDIT:
    I pasted this into my IDE and had it format it for me. Here are my notes:
    • You're casting "sender" to Player before checking if instanceof Player. Kind of defeats the purpose of an instanceof check if you're just going to have a ClassCastException anyways. Move that cast after the instanceof check
    • There may very well not be any arguments at all yet you're accessing array index 0. Again, you're assuming there are arguments before checking if there are any. This should be moved down after the check
    • You need not check if "p.hasPermission("redchat.*")" if you already check for a child permission. Just a redundant check.
    • You're nesting a lot of your code. Consider inverting and returning where possible (see example below)
    Code (Java):
    if (!(sender instanceof Player)) {
        sender.sendMessage("Only players are capable of sending this command");
        return;
    }

    Player player = (Player) sender;
    Thread moved to Spigot Plugin Development.
     
    #3 Choco, Aug 17, 2018
    Last edited: Aug 17, 2018
  4. To me it looks like you don't have the World variable in the correct place. Try moving the a variable to your args[1] spot under the check if args == 0.
     
  5. command is messing up because of
    Code (Text):
    World a = Bukkit.getWorld(args[0]);
    at the start
    since this is at the start the command is always looking for
    Code (Text):
    args[0]
    when the command is executed, it won't work if you just use /day since there will be no
    Code (Text):
    args[0]
    just remove that and you should be fine
    also if you're going to check if the sender is a player
    Code (Text):
    if(sender instanceof Player)
    do it before you do
    Code (Text):
    Player p =(Player) sender;
     
    • Agree Agree x 1
  6. Code (Java):
    @Override
    public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) {
        if(!sender instanceof Player) return true;
        // Like the world argument issue, you tried to cast the sender to Player before knowing for sure
        Player p = (Player)sender;
        World w = p.getWorld();
        // One if statement to handle having the right permission for each subcommand
        if(!p.hasPermission("redchat.time.day") || (args.length == 1
                && p.hasPermission("redchat.time.day.world"))) {
            p.sendMessage(ChatColor.PURPLE+"[Redchat] "+ChatColor.DARK_RED.toString()
                    +ChatColor.BOLD+"Du hast kein Recht um diesen Befehl auszuführen!");
            return true;
        }
        if (args.length == 1)
            w = Bukkit.getWorld(args[0]);
        // ^ We changed the previously set world to the specified one if it's there
        // We also allow it to continue if there's multiple words, because they can be totally ignored
        if (w == null) {
            // If it's null, though, we toss out the command and tell them the world wasn't found
            p.sendMessage("Welt nicht gefunden");
            return true;
            // also making use of return statements. if you return false, you get the usage,
            // which you don't always want unless they use the command wrong
        }
        // Now, without room for error anymore and our handy world variable, we can set the time
        w.setTime(5000);
        Bukkit.broadcastMessage(ChatColor.GREEN+"Die Zeit in "+ChatColor.GOLD.toString()
                +ChatColor.BOLD+w.getName()+ChatColor.GREEN+" wurde auf Tag gestellt!");
        return true;
    }
    Compacted the code a lot.
    As mentioned above, the issue was that the world was defined by a variable that might not have been there, which would throw a NullPointerException.
     
    #6 Escad, Aug 17, 2018
    Last edited: Aug 17, 2018
  7. Fixed it, you still had a mistake, which was that p.getWorld() was at the very top, way before Player p is actually introduced into the scope.
     
    • Like Like x 1
    • Agree Agree x 1