Common Mistakes

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

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

    makskay

    For some reason this has been going on a lot lately. I've even seen people recommending it over traditional command registration >.>
     
  2. Offline

    deltahat

    I bet they think fewer classes equates to better code. Either that or they think it will give their plugin an edge when commands conflict. Either way this is a terrible idea and will completely break the help system!
     
  3. broadcasting a message to al players whit a loop
    instead of this:
    Code:java
    1. for(Player p:Bukkit.getOnlinePlayer()){p.sendMessage();}

    do
    Code:java
    1. Bukkit.boradcastMessage();

    using the for loop stops some irc plugins getting the things announced, that it not works whit pex it the error of pex breaking the system, mbaxter has writen a page full of why not to use pex
     
    WarmakerT likes this.
  4. Offline

    chaseoes

    Using thread.sleep.
    I see this all the time when people attempt to make countdowns/delays.
     
    ipodtouch0218, Plo124, Skyost and 2 others like this.
  5. Offline

    Comphenix

    I've been thinking the same thing too. Maybe a WeakHashMap (or Guava's MapMaker) is a better fit?

    I haven't really seen a good reason NOT too use weak references. Retrieving a player by name is, as you've noted, not optimized at all. That might make a difference, especially if you use loops in events that are frequently invoked.
     
  6. Offline

    desht

    Yeah... an alternative might be to add a little optimisation to CraftBukkit itself - store a Map<String,Player> cache internal to CraftPlayer which is updated when players connect & disconnect. I suppose there's a risk of invalid cached data if connect/disconnect events are missed, though.
     
  7. Offline

    CorrieKay

    how often can events be missed..? It doesnt really seem like something that should happen at all.
     
  8. Offline

    desht

    Player join events should never be missed - the server knows for sure when a new player connects. Player quit events almost certainly shouldn't be missed - it really depends on how well the server detects dead clients (crashed PC, lost network connection...).
     
  9. Offline

    Sleaker

    desht - the issue is that player objects go stale, for multiple different reasons, or can flip-flop the actual valid object. So storing the player object is in no way safe ever. I can give multiple examples of this occuring with Heroes (due to other plugins or Bukkit), and player object entity IDs flip-flopping between calls. If the EID is flip-flopping, then the player object is definitely going to have bad data somewhere.

    Using it as a key is very very bad. CraftPlayer.hashCode does not match CraftPlayer.equals:

    https://github.com/Bukkit/CraftBukk...kkit/craftbukkit/entity/CraftPlayer.java#L193

    the equals method checks the entityID while the hashcode does not. This issue means that if the player object has problems with it's entity ID then it will have massive problems checking equality. I suggest never ever using player.equals(otherPlayer) in any circumstance as it will return mixed results for the very reasons detailed above (entityID is not consistent for a player object). In addition, this means attempting to use the player object as a key will fail in various circumstances (especially when used in conjunction with plugins like CombatTag, or ones that create a 'copy' of the player).
     
  10. Offline

    CorrieKay

    what if a plugin holds a map of the player objects with the player name as the key, and the player object as the value.

    Sure you dont get to store the direct player object in your specific map, however, it would be a lot faster than getPlayer, or getPlayerExact, maybe..?
     
  11. Offline

    Sleaker

    sure it might save a bit of processing, but you also have to manage it, and if it breaks, or has problems then you can potentially cause problems in every other plugin not just your own (stale player data being stored still).
     
  12. Offline

    CorrieKay

    Okay, what if it had a running task. Once every couple of seconds, have a runnable check if(player.isOnline())

    If theyre not online, remove them from the map. Would that be decent :)?
     
  13. Offline

    Sleaker

    well you should be able to manage that with just the join/quit events. no need for a scheduled task.
     
  14. Offline

    Sabersamus


    How is that a plugin coding problem?
     
  15. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı

    It's not.
     
  16. Offline

    Kodfod

    Fixed then =)
     
  17. Offline

    HollowCube

    Shouldn't we be encouraging server owners to update to Java 7? Read this:

    Where can I get the latest version of Java 6?

    We highly recommend downloading and installing Java 7. The latest release for Java contains many new features, performance enhancements, and bug fixes to improve the running of Java Applets or applications.

    Java SE 6 End of Public Updates

    After February 2013, Oracle will no longer post updates of Java SE 6 to its public download sites. Existing Java SE 6 downloads already posted as of February 2013 will remain accessible in the Java Archive on Oracle Technology Network. Developers and end-users are encouraged to update to more recent Java SE versions that remain available for public download.

    Copied and pasted from java.com:
    http://www.java.com/en/download/faq/java_6.xml
     
    kroltan likes this.
  18. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı

    HollowCube Encouraging to use Java 7 isn't the same as forcing. Many server admins are in environments where they can't control the java version they're running.
     
  19. Offline

    HollowCube

    Thanks for pointing that out. I guess I'll use Java 6 to compile my plugins for the time being then.
     
  20. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı

    Extra points if you compile on java 5, as CraftBukkit is java 5 compatible.
     
    ferrybig and HollowCube like this.
  21. Offline

    HollowCube

    Looking into it now... who knows, I might actually decide on using it.
     
  22. Offline

    CorrieKay

    oh of course, thats just a backup/safeguard to prevent memory leaks.
     
  23. 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
     
  24. Offline

    CorrieKay

    I hate that notepad++ defaults to tabs, yet can tell you when its wrong, and doesnt correct it :p
     
  25. Offline

    jacklin213

    you can change tabs to spaces in the settings in Notepad++ so every time you press tab it will do 4 spaces
     
    kroltan likes this.
  26. Offline

    CorrieKay

    mmh, oh i know that. what i meant was that notepad++ will bug you about it being incorrect, yet wont do anything automatically. Though, maybe you can default to spaces/tabs for different languages/filetypes...
     
  27. Offline

    Hoolean

    Mistake found:
    No space between silently and break

    Hope I've helped :D
     
  28. Didn't know the one about Vault permission checks, thanks!
     
  29. Offline

    KollegahDerBoss

    What's wrong with that?
     
  30. Offline

    SirTyler

    Since the server IS the main thread, if you call thread.sleep you are making the server stall.
     
Thread Status:
Not open for further replies.

Share This Page