Common Mistakes

Discussion in 'Plugin Development' started by mbaxter, Sep 12, 2012.

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

    mbaxter ʇıʞʞnq ɐ sɐɥ ı Retired Staff

    No, if your plugin's job is changing/supplementing permissions plugin functionality, then using Vault can be helpful because it lets you hook into multiple plugins. That section is talking about using Vault to get if a player has a permission, which only ends up calling player.hasPermission anyway.
     
  2. Offline

    Digi

    I guess another common mistake, a subtle one, using Bukkit.getPlayer() when you're retrieving stored player names instead of Bukkit.getPlayerExact().

     
    AmShaegar likes this.
  3. Offline

    chasechocolate

    Yeah. Some of my old plugins use Bukkit.getPlayer()... *starts editing my old plugins*
     
    XlegitXcrazymanX likes this.
  4. Offline

    afistofirony

    One thing I think should be added:

    If you use sdr.getName(); you can crash a server by trying to get the name of a ConsoleCommandSender. Therefore it is always recommended to use a method like the following if names of anyone besides players are allowed:

    Code:
    public static String getName(CommandSender sdr){ // Player can be cast to this, don't worry.
        if(sdr instanceof ConsoleCommandSender) return "Console";
        else if(sdr instanceof BlockCommandSender) return "Command block";
        else if(!(sdr instanceof Player)) return "???";
        return sdr.getName();
    }
     
  5. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı Retired Staff

    Under what scenario would getName() crash the server?
     
    xxyy98 likes this.
  6. Offline

    afistofirony

    I'm not sure exactly why, but whenever I tried to use a plugin I made, the server would crash if I tried to send the command as console. I would use getServer().broadcastMessage(ChatColor.AQUA + sdr.getName() + " facepalmed.");

    Maybe I was doing something incorrectly. But it seems to throw an error whenever I try to do it.
     
  7. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı Retired Staff

    Post a new topic with the error ;)
     
  8. Offline

    desht

    Using block.getState() just to check the type of a block.

    I've seen this too often (and seen it recommended too often); it's really inefficient compared to checking block.getType() or block.getTypeId(). block.getState() constructs a new BlockState object via reflection, and depending on the block type does extra processing (e.g. getting the block state for a sign involves constructing a copy of the sign's text too). If you just want to check a block's type, this is a gross waste of CPU resources.

    This is bad:
    PHP:
    if (block.getState() instanceof Sign) { ... }
    and this is good:
    PHP:
    if (block.getType() == Material.SIGN_POST || block.getType() == Material.WALL_SIGN) { ... }
     
    Skyost and Ribesg like this.
  9. Offline

    maschill92

    So is the best way to test permissions to use the default permissions handling that Bukkit comes with?
     
  10. Offline

    desht

    If you just need to check if a user has a given permission node, then yes; use player.hasPermission().

    If you need to check for permission group membership etc., then Vault is probably the way to go.
     
  11. Offline

    SnipsRevival

    I am surprised nobody has already said this: if you are going to cast a CommandSender to a Player like this
    Code:
    Player player = (Player) sender;
    make sure you check if the CommandSender actually is a Player by doing this
    Code:
    if(sender instance of Player) {
            Player player = (Player) sender;
            //sender is a Player
    }
    else {
            //sender is not a Player
            //such as when a command is issued from the console
    }
     
  12. Offline

    desht

    AmShaegar, chinwe and jacklin213 like this.
  13. Offline

    Chinwe

    More of a basic java error, but I see a lot people trying to compare strings with == instead of .equals() :oops:
     
  14. Offline

    jacklin213

    y not == ?
     
  15. Offline

    desht

    Jaker232, Skyost, jacklin213 and 2 others like this.
  16. Offline

    jacklin213

    desht likes this.
  17. Offline

    stuntguy3000


    Let me put a few tabs in the config, and reboot the server on a 10 sec timer. I think you see my point l0l.
     
  18. Offline

    Chinwe


    Combine stack traces of the same type/from the same server, and only email at specific intervals? :rolleyes:
     
  19. Offline

    Me4502

    People setting the blockstate of the sign in SignChangeEvent, instead of event.getLine and event.setLine
     
  20. Offline

    jacklin213

    Dono if anyone else is getting the same error as me but if using config in another class the plugin variable needs to be static or you need to create a separate get method or plugin.getConfig() wont work
     
  21. Offline

    stuntguy3000

    Wrong place.
     
  22. Offline

    SnipsRevival

    This is the wrong place to ask a question, but that brings up a good point.

    If you want to use getConfig() in another class other than your main class, you must create an instance of your plugin in the other class and initialize it in your other class's constructor like this:

    Code:
    public class OtherClass {
     
        MainClass plugin;
     
        public OtherClass(MainClass plugin) {
            this.plugin = plugin;
        }
    }
    
    and then you will need to call your other class's constructor inside your onEnable() method in your MainClass like this:
    Code:
    public class MainClass extends JavaPlugin {
     
        @Override
        onEnable() {
            new OtherClass(this);
        }
    }
     
  23. Offline

    Ribesg

    This field should be private.
     
  24. Offline

    stuntguy3000

    It doesn't matter really.
     
  25. Offline

    desht

  26. Offline

    FreeStyle

    Thanks man! I got a big errormessage because of accessing stored Player object... helped a lot!
     
  27. Offline

    desht

    Using String.concat() (aka s += "string") in a loop

    Not strictly a Bukkit problem, but commonly seen on these forums, and a lot of people are unaware of how shockingly inefficient code like this is:
    PHP:
    String s "";
    for (
    Player p Bukkit.getOnlinePlayers()) {
      
    += p.getName() + ",  ";
    }
    You won't notice it for one or two players (a typical development/testing server), but push that to a busy server with 50 or 100 or more players, and you'll be wasting a ton of CPU cycles copying data and giving the garbage collector unnecessary work to do.

    Because String is an immutable class, you're constructing a new String every time you do the += operation, and copying the old String's contents into it, and throwing the old String away. As the String gets longer and longer, there's more & more data to shuffle around. Here's a graph (and an in-depth explanation) of just how inefficient it is: http://kaioa.com/node/59

    Short answer: learn to use StringBuilder.
     
  28. Offline

    iKeirNez


    Hmm, maybe have it send to a global server and then everyday it would send you one email with all the issues?
     
  29. Offline

    Saposhiente

    Not picking a priority for your EventHandlers
    I see this on 9/10 code samples in this subforum that feature an event handler, usually they want EventPriority.MONITOR and ignoreCancelled=true.

    It'd be nice if, whenever someone tries to create a new thread in this subforum, it automatically searched the forum (or at least this thread) for keywords from their thread, and shows them the results.
     
  30. Offline

    stuntguy3000

    Actually, i wouldn't consider that a mistake... Most basic plugins don't need to give a damn about priorities.
     
Thread Status:
Not open for further replies.

Share This Page