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


    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


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


    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:

    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


    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


    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:
    if (block.getState() instanceof Sign) { ... }
    and this is good:
    if (block.getType() == Material.SIGN_POST || block.getType() == Material.WALL_SIGN) { ... }
    Skyost and Ribesg like this.
  9. Offline


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


    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


    I am surprised nobody has already said this: if you are going to cast a CommandSender to a Player like this
    Player player = (Player) sender;
    make sure you check if the CommandSender actually is a Player by doing this
    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


    AmShaegar, chinwe and jacklin213 like this.
  13. Offline


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


    y not == ?
  15. Offline


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


    desht likes this.
  17. Offline


    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


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


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


    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


    Wrong place.
  22. Offline


    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:

    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:
    public class MainClass extends JavaPlugin {
        onEnable() {
            new OtherClass(this);
  23. Offline


    This field should be private.
  24. Offline


    It doesn't matter really.
  25. Offline


  26. Offline


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


    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:
    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:

    Short answer: learn to use StringBuilder.
  28. Offline


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


    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


    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