Problem/Bug Common mistakes thread in plugin developmemt extremely outdated

Discussion in 'Bukkit Help' started by RcExtract, Jun 11, 2018.

  1. Offline


    In the thread it is calling us to code a plugin based on Java 6. However, even Java 7 reaches eol (end of life) and Java 10 is released for awhile already. Someone please update it to recommending programmers to code on Java 8+.

    EDIT: or can i make a new one?
  2. Online

    timtower Ninja on the waves Moderator

    @RcExtract Might need a new one instead.
    And not every server runs java 9, so 8 is probably the highest you can go right now.
  3. Offline


    Post it right in plugin development forum?
  4. Online

    timtower Ninja on the waves Moderator

    Might be better to post it as staff so we have less issues when editing it.
    And maybe post it here first to check if it is correct etc.
  5. Offline


    (Since the old one is archived, a new one must be created.)

    Common Mistakes while coding a Bukkit plugin

    In this article I hope to cover a bunch of mistakes that I see frequently either in the Bukkit Plugin Development forum, the #bukkitdev IRC channel, in plugins that mbaxter help folks write in other places on the net. All of these issues or mistakes can occur on a plugin that compiles fine.

    Building with latest java version - 2
    While technically not a problem for your own computer, you need to keep in mind that plenty of Bukkit servers are running Java 7. However, Java 7 has reached EOL (end of life). There will be no more public updates, which is a very bad idea. Therefore, the server owners will update their Java, and you should also compile your code on Java 8 or higher versions. Note that the information above is probably only applicable to 2018. For anytime measures, you should compile your code with the latest version minus 2 of Java.

    Denying Console from using commands that it could otherwise use
    I see a lot of plugins that, for commands not affecting the sender, deny anyone that isn’t a Player from using them. If your command can operate with console using it, you should allow it for normal situation. Checking for instanceof Player prevents execution from console as well as other situations such as IRC plugins executing commands remotely.

    Not registering events or commands
    You’ve created your amazing event listener full of methods for catching all sorts of events. But none of them are firing! You created an onCommand method or a couple CommandExecutors for your commands but they aren’t triggering! Looks like you forgot to register your events and commands. Event registration is documented here, while command registration is documented here. Also, don't forget to register commands in your plugin.yml.

    Putting other plugins classes (or entire plugins) in your jar
    I’ve seen several developers, in the hope of ensuring their users also get the plugin theirs uses, include plugins or pieces of plugins in their code. First, this is often a license violation. More importantly this can breakthat other plugin and is a bad practice. Only include your own code (or code used with permission under license) in your plugins. If you are using maven, make sure to set the scope of all dependencies to "provided" if you are not sure whether you can add it into ur jar.

    Including CraftBukkit in your plugin
    This is certainly an impressive feat, but I’ve seen it in probably half a dozen submissions. Don’t include CraftBukkit (or Bukkit) in your plugin, the Bukkit server jar already has that.

    Importing individual permissions or chat plugins for chat formatting (prefix/suffix)
    You create your awesome chat plugin that needs to get prefix/suffix data from other plugins. You release it and it works fine for 5 months. In those 5 months a brand new awesome prefix plugin comes out and you have to now edit in support, or one of the plugins you supported completely changes its internals. Your plugin needs constant updates to keep track of all the other plugins you’re checking just for some simple data they all provide. The plugin Vault was created precisely to avoid this scenario. Vault handles keeping track of all these systems for you and is an easy dependency to have your users install. They just drop it in and go. Done! On your end, you now have a single plugin to support whose API isn’t going to change.

    Using Vault to check if a Player has a permission
    On the opposite side of the Vault issue, is people who believe that using Vault to check a permission is the ideal way to go. I’ve seen plugins require Vault just for checking has(Player,permission). Fun fact: Vault’s permission check just calls Bukkit’s methods. There is zero benefit in using Vault to check a permission. Just call player.hasPermission(permission) and your life is far easier!

    Using non-threadsafe methods in Async events (AsyncPlayerPreloginEvent, AsyncPlayerChatEvent) or Async tasks (scheduler)
    Async events are not running as part of the minecraft thread. A good way of judging if you should be calling a method from an async event is asking yourself “does this method change or query something in-game?” If the answer is yes, you probably shouldn’t be doing it. Permission checks are not safe, as are many other methods. Pretty much, if you’re not certain, don’t.
    Examples of valid ways to deal with various async features:
    • AsyncChat: Format the chat with your formatter. Use cached information about the player to format the chat.
    • AsyncPrelogin: Query an external database (async is perfect for this, the delay in reply won’t lag the server) to determine if that username or IP can join.
    • Async task: Update a MySQL database with new data.

    IO operations (writing non-config files, MySQL queries, metrics, update checks, etc) on the main thread
    IO calls can take a long time. Running your MySQL query or update check from the main thread can very quickly slow down the server in a highly noticeable way. Instead, schedule these actions to run in an async task. Need to do something that isn't thread-safe (see above) as a result of an IO operation? Schedule a sync task from that async task when it's time to react!

    Utilizing static variables for information sharing
    There are many reasons this is generally a poor decision. Here is a great description of some reasons:
    Storing constants statically is great. Storing temporary information is less so.

    For main plugin class, you should also keep the fields non-static, and if exists, call the API users to obtain your main plugin class with Bukkit.getPluginManager().getPlugin(String) or provide a static method which returns the main plugin class instance.

    Storing Player objects
    You should store Player objects, but they will cause memory leaks when the player representing leaves. Therefore, make sure to un-store them when they leave. You can handle player leaving with PlayerQuitEvent. Storing UUIDs are not recommended because it only provides coding convenience, but not performance, because Bukkit.getPlayer(UUID) is an expensive method and will slow down the performance. You may also consider using WeakHashMap, which a key along with the corresponding entry will be removed when the key is no longer in ordinary use.

    plugin.yml Dependencies and Soft Dependencies
    For "depend" and "soft-depend" properties in plugin.yml, they are String arrays. Make sure to surround the values with [] and separate values with commas.

    @EventHandler Annootation
    Listener is a marker interface, and Bukkit does not know which method within your Listener is an event handling method. So make sure to add the EventHandler annotation. Furthermore, you can add extra attributes: priority and ignoreCancelled.

    Don't log for plugin enabling or disabling
    The server logs for you, so you don't need to log them for yourself. It does not cause performance issues, but the plugin users may not understand why the plugin enables and disables for "twice", and may cause other confusions.

    Using location.distance() inside a loop
    Commonly seen in code that draws circles, spheres etc. - using getDistance() to check if a block is inside a radius makes a call to Math.sqrt(), which is not cheap, and really inefficient if done inside a loop. Instead, calculate the square of your desired distance outside your loop, and using location.distanceSquared().

    onCommand vs PlayerCommandPreprocessEvent
    onCommand receives command execution from CommandSender, but PlayerCommandPreprocessEvent only receives command execution from Player. Also, PlayerCommandPreprocessEvent is fired before Bukkit checking whether the command really exists. So take good use of them.

    Do not use Thread.sleep()
    Executing Thread.sleep() freezes the entire thread, and you should never do this in the Bukkit server thread.

    Using tabs for indentation in yaml configuration files instead of spaces
    Yaml DOES NOT allow this as part of it's specification, and it will throw errors if you do this.
    Double/quad space indent only.
    This can cause your plugin to not load it's config.yml or worse, not load and spam console because of tabs in plugin.yml

    You are welcome to comment below and even suggest additions.
  6. Online

    timtower Ninja on the waves Moderator

    @RcExtract This one conflicts with the original one though.
  7. Offline


    I thought that u are going to remove the old one lol, but after thinking it seems that the old one cannot be removed due to archived. should I only talk about use Java 8, or Java 8 with additional common mistakes that are not in the old one?
  8. Online

    timtower Ninja on the waves Moderator

    I want to unsticky the old one and sticky a new one. But I don't want them to conflict with each other.
  9. Offline


    So do u mean u don't want me to copy contents from the old one? But they are also common mistakes too. If u unsticky the old one, and I don't copy some from the old one , people may not know about the mistakes.

    我從使用 Tapatalk 的 LG-H815 發送
  10. Online

    timtower Ninja on the waves Moderator

    @RcExtract You are free to copy from it. But right now you have an item about UUID's that goes ahainst mbaxters version.
  11. Offline


    What do u mean?
    Do u mean that I recommended to store player instances instead of UUIDs, which is different from mbaxters version? But his version is old. Also I heard a decent amount of message that storing UUIDs isn't as good as u think.

    我從使用 Tapatalk 的 LG-H815 發送
  12. Online

    timtower Ninja on the waves Moderator

    @RcExtract And I heard that UUID's are better. Love programming.
    Probably deoends from what point of view you look. Memory issues vs speed issues.
  13. Offline


    Does WeakHashMap solve both problem? You dont need to invoke Bukkit.getPlayer everytime, but once there is no more reference of the player object in other places, it will be removed from the map.
  14. Online

    timtower Ninja on the waves Moderator

    @RcExtract Don't know. It depends on what you prefer to look after.
    Speed in Bukkit isn't a big concern in my opinion as the tick rate is limited anyways.

Share This Page