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

    EvilSeph

    This is the discussion thread for "Oops, I broke your plugins!" so that we can keep the actual notices thread clean.
     
  2. Offline

    Nineza

    Yeah, now you should probably update the wiki.

    It has a link to SamplePlugin.java in the google code bukkit page, the link is found on the Programming_A_Plugin page.
    On line 22, it has the old version.
    Code:
    public <pluginname>(PluginLoader pluginLoader, Server instance, PluginDescriptionFile desc, File plugin, ClassLoader cLoader)
    
    Might wanna change that to the new version...
    I had to change that when I just made my first plugin. Some people might not realise about it...
     
  3. Offline

    MysticX

    Also the imports are outdated (like discussed here)
    Took me a while to get started, I was wondering what's going wrong first ;)
     
  4. Offline

    Jinux

  5. Offline

    Plague

    Not that it is a big problem or anything, but don't you think UNIX has a reason for including the command name to args (I don't mean argv, but also the exec(command, args) fucntion)?
     
  6. Offline

    Tahg

    This is a heads up to any plugin devs working directly with Materials or the inventory. API changes are likely to these classes. No changes have been set in stone yet, but I will keep you informed.
     
  7. Offline

    DjDCH

    Sooo ... Inventory event hooks are for soon ?
     
  8. Offline

    Steve Member

    Those of you registering and handling commands will need to update you code to work with a push coming very soon which changes Commands to be called by a CommandSender instead of a Player.

    Effected
    org.bukkit.command.Command
    - public abstract boolean execute(Player player, String currentAlias, String[] args);
    + public abstract boolean execute(CommandSender sender, String currentAlias, String[] args);
    org.bukkit.command.CommandMap
    - public boolean dispatch(Player sender, String cmdLine);
    + public boolean dispatch(CommandSender sender, String cmdLine);
    org.bukkit.command.PluginCommand
    - public boolean execute(Player player, String commandLabel, String[] args) {
    + public boolean execute(CommandSender sender, String commandLabel, String[] args) {
    org.bukkit.command.SimpleCommandMap
    - public boolean dispatch(Player sender, String commandLine) {
    + public boolean dispatch(CommandSender sender, String commandLine) {
    - public boolean execute(Player player, String currentAlias, String[] args) {
    + public boolean execute(CommandSender sender, String currentAlias, String[] args) {
    org.bukkit.plugin.Plugin.java
    - public boolean onCommand(Player player, Command command, String commandLabel, String[] args);
    + public boolean onCommand(CommandSender sender, Command command, String commandLabel, String[] args);

    Sorry for the breakage but changes are required so we can cleanly implement external command sources such as rcon.
     
  9. Offline

    Plague

    So this only affects the command interface based plugins and not player based ones? Also is the command interface ready then?
     
  10. Offline

    WMisiedjan

  11. Offline

    Dinnerbone Bukkit Team Member

    Server.getWorlds() now returns a List<World>
     
  12. Offline

    feverdream

    Same here. Its really annoying and is confusing some users, who think its error output. Got me the first time as well.
     
  13. Offline

    WMisiedjan

    The red line with your command showing up only appears if onCommand returns false,
     
  14. Offline

    Dinnerbone Bukkit Team Member

    REDSTONE_CHANGE now takes a BlockRedstoneEvent, which no longer extends BlockFromToEvent. A simple recompile should fix any brekages.
     
  15. Offline

    Cogito

  16. Offline

    Dinnerbone Bukkit Team Member

     
  17. Offline

    Plague

    Grrr, so creating the centralized function for all those typed of damage was for nothing!
    But seriously this is more clean way in an OO project, so thanks.
     
  18. Offline

    matejdro

    Alright, this is my code:

    Code:
    if (!(event instanceof EntityDamageByEntityEvent)) return;
    And i get this:

    EDIT: Oops, i forgot to import EntityDamageByEntity. It works now :)
     
  19. Offline

    xZise

    Newly I get following message:
    Code:
    2011-02-20 16:44:18 [WARNING] Using the stupidly long constructor me.taylorkelly.mywarp.MyWarp(PluginLoader, Server, PluginDescriptionFile, File, File, ClassLoader) is no longer recommended. Go nag the plugin author of xWarp to remove it! (Nothing is broken, we just like to keep code clean.)
    What can I do to fix this? Simply removing everything out of this constructor. But when I could access to the description for example? If I see this correct only after initialize() (if the default constructor is called) and this after calling the constructor. So then you couldn't do some initialize things (like initialize a logger). Only onEnable is the first method call where everything is initialized.

    And even with also support a short constructor I get this message, because you first search for the complex method. But if I delete the big constructor it won't run on older builds (which I want to prevent at the moment, as the commit is only 15 hours ago).

    Fabian
     
  20. Offline

    Dinnerbone Bukkit Team Member

    Remove the constructor.

    Initializing a logger can just be where you declare the logger. Same for all other objects.
     
  21. Offline

    xZise

    And what if I need my plugin name? And if I remove this, which build won't understand my plugin?

    Fabian
    --- merged: Feb 20, 2011 5:41 PM ---
    Okay build 323 is too old :(
     
  22. Offline

    The_Wrecker

    Alrighty,

    I understand the change of the constructor. Is the original constructor still going to be called when available?

    If not I'd opt for a pre-Enable method in the plugin. Which can optionally be implemented by the author.
    In this method all the getServer(), getDescription() stuff returns the needed items (and not null).

    onEnable is used to enable the plugin right? Not to load the plugin...


    Some plugins need to do some work before getting enabled... Simply putting everything in onEnabled is not always desirable (but for simple plugins I guess it would suffice).
     
  23. Offline

    tommytony

    @The_Wrecker
    I don't want my plugins doing anything before onEnabled. What use-case do you have for work that should be done in the plugin constructor?
     
  24. Offline

    Afforess

    For those of you who need some hand holding:
    [​IMG]
     
  25. Offline

    xZise

    Yep I would also like something like this. For example a method “onLoad()” directly after initialize it.

    Fabian
    --- merged: Feb 20, 2011 7:50 PM ---
    Only that you don't need this means that nobody needs this. And in the plugin constructor are the variables like name etc. not initalized.

    Fabian
     
  26. Offline

    tommytony

    @xZise
    Fabian, please let me know: what your use-case is for doing something in the constructor instead of in onEnable?

    Just because you think you need it doesn't mean you should do it. ;)
     
  27. Offline

    The_Wrecker

    I do some file finding and initialization to detect if something exists relative to my plugin file. And I do that only once. Not everytime my plugin gets enabled and/or disabled. A reload is different from an enable or disable in my opinion.

    Besides that, another plugin depends on that type of initialization/detection. That's the reason I can't easily move it to the onEnable.

    As I said in my earlier post: you don't always need it and I also said it would be an optional thing to implement for a plugin dev. It's just a single call after the constructor.

    I also think that registering event hooks:
    "this.getServer().getPluginManager().registerEvent("
    in the onEnable is a good idea, but i also want to unregister those when a onDisable occurs (seems logical right?). I haven't seen that method yet (maybe on the roadmap somewhere?). But when you call onEnable again you could just do the onEnable instead of reloading the entire plugin.

    Btw, pre/post methods are very common in a lot of frameworks and they are there for numerous reasons. I agree you won't always need them, but in case you do need them they are there.

    When I think of it a post method might be useful to unload your plugin. Although I won't need it right now (I actually use onDisable to accomplish that).

    I think I would call it onLoad / onUnload. Just an idea though.
     
  28. Offline

    xZise

    Jep you are right, most of this could also be done in onEnable(). But what me disturbs is that this could be called multiple times. And e.g. the name only will set one time.

    But the order of calling the constructors should be switched. Because the big constructor is fall back or not? So newer plugins don't brings up a error message and also works with CB build pre 354.

    Fabian
    --- merged: Feb 20, 2011 10:01 PM ---
    @ The_Wrecker: There is already a pull request for.

    Fabian
     
  29. Offline

    The_Wrecker

    A thanks for the pointer to the pull. Unregistering event listeners is on the todo I see.

    And yes I agree. You don't always want to execute code which is in the onEnable. I have code in onEnable which start up certain threads. Needless to say those threads are stopped in the ondisable call. So that's ok and works flawlessly (since hmod actually).

    But setting up variables and some other pre check things I do those before the plugin gets enabled. I don't wan't to do that everytime it gets enabled.

    Right now I have code that needs to be initialized before another plugin accesses it. I Can't guarantee that now since I don't know which get's enabled first. But that is probably an edge case scenario, but still.
    --- merged: Feb 20, 2011 10:23 PM ---
    I think it works with reflection. Basically reading your classfile if your plugin has that constructor. But I didn't look at the source to back that up. The warning logging about the constructor would be in the super class your extending (that's my guess anyway). Could be totally wrong about this.
     
  30. Offline

    xZise

    At the moment bukkit first searches the complex constructor and if found executes this. If not found it calls the default constructor. A hack could be to call the default constructor in the complex constructor. But this won't work in builds prior 354. Because there the JavaPlugin class only has the complex constructor which you than never calls.

    The relevant code (block) you could find here.

    Fabian
     
Thread Status:
Not open for further replies.

Share This Page