Discussion: Oops, I broke your plugins!

Discussion in 'Plugin Development' started by EvilSeph, Jan 17, 2011.

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

    Dinnerbone Bukkit Team Member

    I would suggest registering a command called "thisisntreallyused" and has "kill" as an alias, allowing other plugins to handle it if they register it but overriding the default behavior.
     
  2. Offline

    Plague

    Could you change the comment in artifactory (or how do you callit)? It says removed onCommand, whereas you removed onPlayerCommand and everybody has to actually use onCommand!
     
  3. Offline

    Dinnerbone Bukkit Team Member

    It's too late now. We can't rewrite history. (Well, we could, but then we screw so many things up)
     
  4. Offline

    Plague

    Oh, I'm just afraid that there will be unneeded threads asked :D
     
  5. Offline

    DjDCH

    Oh, seriously ... Why for the constructor ? I need to re-design all my plugin to get it work. I can remove the super(...) part, but why removing completely the constructor ? Now, I cannot do something like this this.logger = new Logger(desc.getName()); because it need to be initialized at the instantiation time (because other plugin will try to get this instance when the plugin is initialized).

    And why for the package ? Why my package com.djdch.bukkit.stackable get rejected ? (So, why I need to remove the bukkit part to get it work ?) This is my domain name, so why bukkit is rejecting this ?
     
  6. Offline

    Dinnerbone Bukkit Team Member

    On the logger thing:

    1 - Why are you making a logger with the name of a description? Not... a name?
    2 - Why not use a fixed name of it?
    3 - Why not use the one on Server? :p
    4 - If you need your own, why not make it at the same time you declare it?
    5 - Failing that, it has to be dynamic, use onEnable
     
  7. Offline

    DjDCH

    It's what I've done, in a different way ... See the code bellow for my logger class, you will understand its purpose:

    And before I forget it, what's happen for the package name ?

    Code:java
    1. package com.djdch.stackable;
    2.  
    3. /**
    4.  * Class who wrap the Minecraft Logger instance.
    5.  * <p>
    6.  * The purpose of the wrapper is to prefix each log message with the plugin name.
    7.  *
    8.  * @author DjDCH
    9.  */
    10. public class Logger {
    11. /**
    12.   * Contains the Minecraft Logger instance.
    13.   */
    14. protected java.util.logging.Logger logger;
    15.  
    16. /**
    17.   * Contains the prefix who will be added to each log message.
    18.   */
    19. protected String prefix;
    20.  
    21. /**
    22.   * Contains the name of the Minecraft Logger instance.
    23.   */
    24. public static final String MINECRAFT_LOGGER = "Minecraft";
    25.  
    26. /**
    27.   * Constructor for the initialization of the Logger.
    28.   *
    29.   * @param prefix Contains the plugin name.
    30.   */
    31. public Logger(String prefix) {
    32. this.logger = java.util.logging.Logger.getLogger(MINECRAFT_LOGGER);
    33. this.prefix = prefix;
    34. }
    35.  
    36. /**
    37.   * Constructor for the initialization of the Logger.
    38.   */
    39. public Logger() {
    40. this("");
    41. }
    42.  
    43. /**
    44.   * Method who log a config message prefix with the plugin name.
    45.   *
    46.   * @param msg Contains the log message.
    47.   */
    48. public void config(String msg) {
    49. logger.config(prefix+": "+msg);
    50. }
    51.  
    52. /**
    53.   * Method who log a fine message prefix with the plugin name.
    54.   *
    55.   * @param msg Contains the log message.
    56.   */
    57. public void fine(String msg) {
    58. logger.fine(prefix+": "+msg);
    59. }
    60.  
    61. /**
    62.   * Method who log a finer message prefix with the plugin name.
    63.   *
    64.   * @param msg Contains the log message.
    65.   */
    66. public void finer(String msg) {
    67. logger.finer(prefix+": "+msg);
    68. }
    69.  
    70. /**
    71.   * Method who log a finest message prefix with the plugin name.
    72.   *
    73.   * @param msg Contains the log message.
    74.   */
    75. public void finest(String msg) {
    76. logger.finest(prefix+": "+msg);
    77. }
    78.  
    79. /**
    80.   * Method who log a info message prefix with the plugin name.
    81.   *
    82.   * @param msg Contains the log message.
    83.   */
    84. public void info(String msg) {
    85. logger.info(prefix+": "+msg);
    86. }
    87.  
    88. /**
    89.   * Method who log a severe message prefix with the plugin name.
    90.   *
    91.   * @param msg Contains the log message.
    92.   */
    93. public void severe(String msg) {
    94. logger.severe(prefix+": "+msg);
    95. }
    96.  
    97. /**
    98.   * Method who log a warning message prefix with the plugin name.
    99.   *
    100.   * @param msg Contains the log message.
    101.   */
    102. public void warning(String msg) {
    103. logger.warning(prefix+": "+msg);
    104. }
    105.  
    106. /**
    107.   * Accessor who return the current used prefix.
    108.   *
    109.   * @return Contains the prefix string.
    110.   */
    111. public String getPrefix()
    112. {
    113. return prefix;
    114. }
    115.  
    116. /**
    117.   * Mutator who set the used prefix.
    118.   *
    119.   * @param prefix Contains the prefix string.
    120.   */
    121. public void setPrefix(String prefix)
    122. {
    123. this.prefix = prefix;
    124. }
    125. }
    126.  
     
  8. Offline

    The_Wrecker

    Why deprecate it for a month and remove it at the same time?:confused:

    I'm confused. Are you putting it back in after a month? Should we developers move to onCommand permanently?

    Btw.

    Javadocs mentioned onPlayerCommandPreprocess(). I'm not (planning to) using that to be clear, but that is going to disappear as well I presume?

    I was just using onPlayerCommand for in-game commands only. I don't have any console commands yet. I guess I'll move to onCommand or something.
     
  9. Offline

    Dinnerbone Bukkit Team Member

    1 month ago (give or take), it was marked deprecated and we told everyone to use the better alternative, onCommand.
    It is now a month later, and we have removed it.

    You should move to onCommand for *every* form of command. onPlayerCommandPreprocess is extremely situational and certainly not to be used for actual command handling.
     
  10. Offline

    Plague

    Actually you said it will most surely not go away because of some thing that cannot be done otherwise ;)
    Not that I really mind, onCommand looks better, but I would have switched earlier.
     
  11. Offline

    The_Wrecker

    Hmm, I noticed the new command methods before, yes.

    My eclipse didn't see the deprecation though. Maybe because of missing javadoc. Not sure. I read your change as if you made it deprecated now(and removed it). Instead you meant you removed it (while being deprecated). Makes sense now.
     
  12. Offline

    nossr50

    Where is this onCommand? I don't see it in the Player Listener or Server Listener.
     
  13. Offline

    xZise

    Doesn't seems so. What if a plugin implements both constructors? A neg message before, but it should run after this change. It simply ignore the old constructor.


    Okay and I posted a suggestion 12 days ago should be a month – or not ... 17. Feb is 10 days not ~30 days ago?! 12 days ago. Because I need the complete line not (stupid) parsed. At the moment the only possibility is to switch to PLAYER_COMMAND_PREPROCESS. Jep I could concat the arguments but why separating first? My solution is also backwards compatible and won't break the plugins.

    Fabian
     
  14. Offline

    ndoto

    I'd love if someone could answer this as well - I'm a novice plugin developer and can't quite figure out where it is.
     
  15. Offline

    The_Wrecker

    @ndoto and @nossr50
    It is in the Plugin. Not in a Listener. Takes some figuring out if you don't know.

    class bla extends JavaPlugin
    {
    @Override
    public boolean onCommand(CommandSender sender, Command cmd, String commandLabel, String[] args)
    {}
    }
     
  16. Offline

    ndoto

    @The_Wrecker
    Thanks very much for the quick response! Possibly a silly question, but given that it's no longer in a Listener, how will that affect the registerEvent?
     
  17. Offline

    The_Wrecker

    The Player_Command event does not exist anymore. The rest should still work for PlayerListener I guess.
     
  18. Offline

    Dinnerbone Bukkit Team Member

    You don't register it.

    Actual deprecation in code was delayed because deprecating methods to override doesn't really get noticed much. We gave ample notice in the form of threads, tweets and an announcement.

    All command processing should go through there. If you need to get a player instance from it, you check if it's a player and return if it isn't. But I highly suggest you give http://forums.bukkit.org/threads/co...ouldnt-just-return-if-instanceof-player.3186/ a read though
     
  19. Offline

    xZise

    Eh? My plugins works nice with the command layout you build, but I allowed previous to place spaces within a warpname. For example somebody could write:
    The result would be only one parameter (loch haus). But your parser forces me to split at the spaces so I get two values ("loch, haus"). I started a thread (22nd January!) which nobody answered and because of this I write my solution to this.
    There are two ways to manage this: Either add a parameter with the complete command line (either “/warp "loch haus"” or “"loch haus"”) so I could parse it on my own, or a possibility that everybody could use my parser which allows more detailed information (as spaces etc.).
    I want to use “onCommand()” but it doesn't make sense to use it at the moment. So if there is a problem with my pull request tell it me, but without no response I have no idea what I made wrong?

    So I want to know what I didn't register?

    Fabian

    PS: Okay I don't use Twitter and missed the thread so I pinned down the “deprecated” to the commit (and yep it wasn't visible in override methods).
     
  20. Offline

    NathanWolf

    Oooh- missed this! Good to know.
    --- merged: Feb 27, 2011 7:24 PM ---
    A good solution to this is, if (presumably) other plugins need to access your objects, is to instantiate them lazily, in the getter of your plugins' API.

    So, your plugin would have a PermissionNiji object that is null until the first time it is requested- this could be by you, in your onEnable- or by any other plugin accessing your API.

    When someone asks for it, you notice it's null, instantiate, and return.

    Nobody should be able to access your plugin before it's at least constructed- meaning you should always have access to the data you need.

    Not sure if that addresses your concerns or not, but this sounds like I use case I've handled so I thought I'd chime in.
     
  21. Offline

    The_Wrecker

    Thanks Nathan,
    I haven't thought about it that way. Solved it by calling onEnable from the other plugin for the moment.
     
  22. Offline

    WMisiedjan

    Dit the onEntityDamage break in build 444jkns?
    and bukkit build: 423.

    Or are they just not compatible with each-other?
     
  23. Offline

    VanillaPie

    Hey,
    how does the PlayerCommand now work?
    "Event.Type.PLAYER_CHAT" and "org.bukkit.event.player.PlayerChatEvent" as Event doesn't work for me.
     
  24. Offline

    Plague

    PlayerCommand has been removed, use onCommand instead.
     
  25. Offline

    VanillaPie

    I can't find it :( Can you say me where I find it exactly?
     
  26. Offline

    cjc343

    Right here.
     
  27. Offline

    EvilSeph

    As you can probably see above, the next Recommended Build will break your plugins if you haven't already prepared to address the issues in this thread.

    The following changes (and possibly more, if I've missed any) will break any outdated plugins:
    • Using the The Stupidly Long Plugin Constructor will result in an exception being thrown.
    • onPlayerCommand and PLAYER_COMMAND have been removed.
    • Commands HAVE to be handled in onCommand which HAS to be in the plugin class or you can set an executor.
    • The *.bukkit namespace has been sealed. This includes namespaces like com.bukkit.org, not com.thevoxelbox.bukkit.
    • Mob -> Creature (See this post for more information).
    • Damage events have been condensed into one, all encompassing event.

    Please make sure you're prepared for the next recommended build which will happen sometime this week!
     
  28. Offline

    xZise

    I want to but I couldn't do, because I need the complete command line. I don't know why you ignore this part everytime, but …

    So tell me what I could do to provide all features in my plugin with your “onCommand()”?

    Fabian
     
  29. Offline

    DjDCH

    Ok. So, why I cannot use com.djdch.bukkit.stackable ? Craftbukkit is rejecting my plugin in cause of that. This is what I use from the beginning, and from what I see, that was right thing to do. But, now ... ?!?
     
  30. Offline

    EvilSeph

    Updated the breakage list that will occur with the next Recommended Build as we missed a few items.
     
Thread Status:
Not open for further replies.

Share This Page