Annoying problem.

Discussion in 'Spigot Plugin Development' started by Sirenum, May 29, 2018.

  1. So I have a custom core that uses it's own permission systems, not bukkit. My issue is related to having the console use the same commands as a player online the server. Obviously, you cannot have an unsafe cast, but for the console, I don't really need to check for permission. The issue is when I need to check for the player but I'm having to rewrite the command for CommandSender sender, for it to be usable. How can I go about this without having to rewrite the command for the console and player.
     
  2. Just restructure your checks. Only check for permission if the sender is a player, and then if the permission needs aren't met then return.

    Code (Text):
    if(sender instanceof Player){
    //Permission check pseudocode
    if(!permission) return false;
    }
     
    • Agree Agree x 1
  3. But even with that, I will still need to use CommandSender to send feedback to the console, and declare a player variable for someone in game correct? My problem is that I have to rewrite all of the checks for each one. Is there no way around this?
     
  4. What do you mean? If you need to have the sender be a player to do certain things, why can the console use the command at all?

    You should use CommandSender#sendMessage(); to send feedback to both the console and the player, not a casted "Player player = (Player) sender" variable. Just use the onCommand sender.

    Ninja edit: It would be a lot easier to see what you mean if you pasted what you've got- the command in question.
     
    • Agree Agree x 1
  5. But if you use sender for both, won't that produce a nullpointer exception. And I need to get the Players UUID. CommandSender is only by name. I also want the console to be able to use /rank etc. Both the console and an ingame player will need to use it.

    Code (Java):

    /**
    * Created by Signifies on 4/22/2018 - 18:21.
    */

    public class RankCommand extends ValkyrieCommand implements CommandExecutor
    {

        public RankCommand()
        {
            super("rank","&a/rank <user> <rank>","",Rank.MOD, true);
        }


        public boolean onCommand(CommandSender sender, Command cmd, String commandLabel, String args[])
        {
            if(cmd.getName().equalsIgnoreCase(getCommand()))
            {
                if(!(sender instanceof Player))
                {
                    if(args.length == 0)
                    {
                        sender.sendMessage(getUsage());
                    }else if(args.length >0)
                    {
                        Player t = Bukkit.getPlayer(args[0]);
                        String result = msg("&7{1}'s Rank is currently, &b{2}&7.",sender.getName(),t.getName(), User.getRank(t).toString());
                        sender.sendMessage(result);
                        if(args.length >1)
                        {
                            Player target = Bukkit.getPlayer(args[0]);
                            instance.getUtils().setRank(sender,target,Rank.valueOf(args[1]));
                        }
                    }
                }else
                {
                    Player admin = (Player)sender;
                    if(User.isPermissible(admin,getReqiredRank()))
                    {
                        if(args.length < 1)
                        {
                            admin.sendMessage(getUsage());
                        }else if (args.length >0)
                        {
                            Player target = Bukkit.getPlayer(args[0]);
                            String result = msg("&7{1}'s Rank is currently, &b{2}&7.",admin.getName(),target.getName(), User.getRank(target).toString());
                            admin.sendMessage(result);
                            if(args.length >1)
                            {
                                instance.getUtils().setRank(target,admin,Rank.valueOf(args[1]));
                                //instance.getUtils().setRank(target,admin,Rank.valueOf(args[1]));
                            }
                        }
                    }else
                    {
                        admin.sendMessage(getPermission(true));
                    }
                }
            }
            return true;
        }
    }
     
     
  6. Code (Java):
    UUID uuid;
    try {
        uuid = ((Player)sender).getUniqueId();
    } catch (ClassCastException e) {
        //We leave this empty if you want nothing to happen
    }
    if (uuid != null) {
        //Player will do this
    }
    //Whatever both senders can do
    Something like this?
     
  7. Interesting suggestion. I feel however that is somewhat of a "hacky" method. I'm trying to implement this system as bug free as possible.

    The main issue with my above code is that there are two #setRank() commands, one for Player (inGame) one for Sender console. I want to do it where there is one method that will account for each option while keeping the custom permission system in mind without having to do separate methods for each option.
     
  8. Strahan

    Benefactor

    No. CommandSender can send messages to both a Player and the console without having to cast anything.

    That aside, in that code you posted earlier, you are duplicating your command functionality based on whether or not sender is a Player.. that's definitely not what you want. If you find yourself copying the same code over and over, it means there is a design flaw. I'm not familiar with that permission check you are using, so I can't really suggest anything there except to say break the command code out to a function then just call the function after doing your check so you don't have duplication.

    PS - it's "required" not "reqired" btw :)
     
    • Agree Agree x 1
  9. Custom Ranks stored as an Enum. The #isPermissible method needs a uuid from a player. I'm aware that you can sendmessages without casting but won't I get a Nullpointer if the console uses the command?
     
  10. Thanks for finding the spelling error.

    Here's a better example of my dilemma.

    Code (Java):
    /**
    * Created by Signifies on 4/22/2018 - 18:28.
    */

    public class Broadcast extends ValkyrieCommand implements CommandExecutor {
        public Broadcast(){
            super("broadcast","/broadcast <message> || <time> ", "No permission to use this command.", Rank.SMOD, true);
        }
        public boolean onCommand(CommandSender sender, Command cmd, String commandLabel, String args[]){

            /*
            Here's the problem. I want the command broadcast to be able to be executed from console and ingame,
            but there's no way that I can see to prevent a null pointer exception from the console
             */

            if(sender instanceof Player)
            {
                Player player = (Player)sender;
                if(!User.isPermissible(player,getReqiredRank()))
                {
                    sender.sendMessage(getPermission());
                    return false;
                }
                //In order for the below code to work for a player, I'd need to again, copy it inside the above statement.
                //However, If I run this command without the instanceOf check I get a null pointer.
                //How do I go about doing this so I can avoid that completely?
            }else
            {
                if(args.length <1)
                {
                    Bukkit.getServer().broadcastMessage(msg("&bTesting a message with new command setup"));
                }else
                {
                    sender.sendMessage("You should try more arguments....");
                }
            }
            return false;
        }
    }
     
    [​IMG]
     
  11. Strahan

    Benefactor

    I dunno, maybe cuz it's about 2 AM my brain isn't working but I'm not understanding the problem. You said for the below code to work as a player, you need to copy it. I don't know what the "msg" is doing, but broadcasting doesn't matter what the sender is.

    I don't see you doing anything save the permission check that could cause any issue, and by doing the instanceof check you handle that. Maybe I'm just too sleep deprived, heh. Is this custom permission handling core available anywhere? I'd be curious to load it in my system and see if I can replicate the problem.
     
  12. The point being, is that I keep having to duplicate my code and functions because if I try to put them together, I will trigger a null pointer. There seems to be no way around having two methods. It's just a test at this point the above example, but the point being, is that if I run the command from the console it will work, but if I try to run it from a player ingame it does not work, unless I duplicate my command check again for the player.
     
  13. Tbh, I can't even find any documentation on the #isPermissible method. I've always just used sender.hasPermission("permission");
    What's the javadoc?
    You could also try this:
    Code (Java):
    if (sender instanceof Player) {
        if (!User.isPermissible((Player)sender, getReqiredRank())) {
            sender.sendMessage(getPermission());
            return false;
        }
    }
    //your code
    That way, if the sender is a player and has no permission, it'll return false, but will not if it's the console. You're seriously overcomplicating it.
     
  14. Strahan

    Benefactor

    Well.. I assume you neutered the example you posted last because there is nothing there that would care about sender being console or player. What is the actual code that requires the dup, the one you posted earlier?

    So this is to set a player's rank? I'm a bit confused because you are passing arguments to the setRank that don't make sense based on the sender. If sender is the console, you pass sender, player whose rank is being adjusted, then the desired rank. If sender is player, you are passing the player whose rank is being adjusted, the player running the command, then the desired rank. You've flip flopped the first two arguments and that seems weird to me, I guess because I don't know what "setRank" is expecting and what it does, same with msg. Do you have multiple method declarations for that or something? Without seeing the method it's hard to say how to fix how it handles the types coming in. I assume those are the ones you fear throwing NPE right?

    Ditto. I just use .hasPermission and it works fine on the ConsoleSender, regardless of being a Player or not.
     
    • Agree Agree x 1
  15. Well you could just set a boolean that looks like this:
    boolean hasPermission = sender instanceof Player? isPermissible(...) : true;
    And then you could just use this boolean to continue and as long as your command does not require a player you can go on without distinguishing between player and console.
     
    • Informative Informative x 1
  16. You took what I posted and implemented it completely incorrect. The reason it doesn't work if a player is using it is because you have all of the actual meat of the command inside of an else statement for "sender instanceof player" Of course a player won't be able to do a anything. Get rid of the "else" after "if(sender instanceof Player)". I said to use that as a permission check, the return is used because it stops further execution. Basically- if the sender is a player, then check if the sender has permission. If the sender isn't a Player, permissions don't matter so skip over it. Either way, the command will only execute if the sender either has permission or is a console.

    Edit: As an addition to that, I'm stunned that you didn't see that yourself. When trying to figure something out you should ALWAYS rubber-duck-debug. Say exactly what the code is doing. It would've been obvious why a player couldn't use the command if you'd said aloud "if the sender is a player, then check if the sender has permission, if they don't then broadcast and return. if the sender isn't a player, then- oh."

    Secondary edit: On my phone so it was slow typing this up but here's a better explained example.

    Code (Text):
    public boolean onCommand(CommandSender sender, Command cmd, String commandLabel, String args[]){
    //If the sender is a player...
      if(sender instanceof Player) {
          Player player = (Player) sender;
          //If the player doesnt have permission
          if(!player.permissible){
              sender.sendMessage("You do not have permission to use that command!");
              //We return here because returning stops further execution. Nothing below this is ran.
              return false;
           }
       }

       //No else statement here because an else statement would be
       //"if the sender ISNT a player" and we don't care at this point. Anything down here will only execute
       //if either the sender isn't a player, or the return wasn't called- thus the player has permission.
       Bukkit.broadcastMessage("Console or a player with permissions sent this");
    }
     
    #16 Velariyel, May 30, 2018
    Last edited: May 30, 2018
  17. This is a custom permission system, It does not use bukkits default system, there is no documentation other than the authors. The method #isPermissible() takes a UUID and checks their set rank from a permission file. CommandSender sender has no UUID, and as such, if I want the console and an ingame player to execute both commands, I have to define two functions because combining both will force a null pointer if used by the console, and using only commandSender will prevent me from getting a users UUID.
     
    • Optimistic Optimistic x 1
  18. Why do you have to use two functions? You're only checking if the sender has permissions if the sender is a player. Read what I posted. It seems to me you don't understand the way a method executes. The way I posted at the very top of the thread, and just again now explaining it even more fully- there is no need to have a second function. There aren't any permission checks at all if the sender is a console.
     
    • Like Like x 1
  19. Yeah, to be honest, I was up very late writing this. I probably should have waited till morning. Thanks for your elaberation, I believe that is more what I'm looking for,however, I will do more testing.
     
  20. Summary: I'm an idiot. Thanks to all that contributed your help. Sorry that I didn't explain it better, its an older project that I'm going through and trying to update it, so there's a lot of errors and bugs to fix. I also shouldn't program so late.

    @Rabbitual @Strahan @Jalau
     
    • Funny Funny x 1
    • Friendly Friendly x 1