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

    The_Wrecker

    What you are posting as a hack here would be the way I would do some of the the re-coding right now. And yes it is dirty in my opinion(I actually was tinkering with that 'hack' before you posted it). Not something I would advise to another developer unless you really need to.
     
  2. Offline

    EvilSeph

    While this isn't a "plugin break" per se, this thread wasn't really just meant as a warning against plugin breaks. It's a thread that gives developers a heads up on changes coming that will most likely affect them.

    That being said, as per http://forums.bukkit.org/threads/introducing-recommended-builds.5137/, we'll be expecting plugin developers to keep up with Recommended builds at the very least. In the near future, we may even make it a plugin submissions/releases requirement.

    To give you an idea of where the builds are at:
    With Jenkins, I believe http://ci.bukkit.org/job/dev-CraftBukkit/45/ is the first build to implement the TSLPC (That Stupidly Long Plugin Constructor) check.

    While the latest build http://ci.bukkit.org/job/dev-CraftBukkit/403/ corresponding to this change in CraftBukkit:
    https://github.com/Bukkit/CraftBukkit/commit/c18627d99ef1d19db663fae77cc495e0677a073d

    And this change in Bukkit:
    https://github.com/Bukkit/Bukkit/commit/112a04a08cb187f2befb3ad422215a0a7a728ca4
     
  3. Offline

    Tahg

    Many of the methods of Block and BlockState are being pulled out into a super interface. If you currently use Block.getData(), you will need to now use getRawData(). getData() returns a block specific MaterialData to be consistent with BlockState and the javadocs.
     
  4. Offline

    xZise

    Yep it is not a break, but every server administrator now gets this naging message (and even if they support the newer version). So (in my opinion) a note/message here (or in the non discussion thread) would be nice.

    And to avoid this message you only have to swap four lines of code:
    Code:
                try {
                     Constructor<? extends JavaPlugin> constructor = plugin.getConstructor();
                     result = constructor.newInstance();
                } catch (NoSuchMethodException ex) {
                     Constructor<? extends JavaPlugin> constructor =  plugin.getConstructor(PluginLoader.class, Server.class,  PluginDescriptionFile.class, File.class, File.class, ClassLoader.class);
                     result = constructor.newInstance(this, server, description, dataFolder, file, loader);
                }
    Fabian

    PS: At the moment ci.bukkit.org seems to be down, so I couldn't check your links.

    Fabian
     
  5. Offline

    EvilSeph

    On the topic of TSLPC (That Stupidly Long Plugin Constructor) check:
    We now include a check for people using the plugin constructor:
    Code:java
    1. public JavaPlugin(PluginLoader pluginLoader, Server instance, PluginDescriptionFile desc, File folder, File plugin, ClassLoader cLoader) {


    You can usually solve this issue by removing your plugin constructor and moving whatever was in it into onEnable.

    For example:
    Code:java
    1. public Fillr(PluginLoader pluginLoader, Server instance, PluginDescriptionFile desc, File folder, File plugin, ClassLoader cLoader) {
    2. super(pluginLoader, instance, desc, folder, plugin, cLoader);
    3. }

    Just remove it.

    If you get an error like:
    Make sure to update your Bukkit reference.

    For further information, see this thread:
    http://forums.bukkit.org/threads/too-long-constructor.5032/
     
  6. Offline

    xTom

    You used my acronym suggestion :')
     
  7. Offline

    nossr50

    Just updated all my plugins to support the RB
    /flex
     
  8. Offline

    matejdro

    About recommended builds - is there a way to create RSS feed? I want to be notified when new recommended build arrive.
     
  9. Offline

    xZise

    And break every build pre this change (I don't understand the new build numbering system). Instead it is easier (and not less bad) if you switch the calls. I mean you have try to implement a fall back system. But actually it is ... not really fall back as it always fall back only if he can not fall back.

    There are following cases:
    1. Plugin only support TSLPC: Your system will warn, My system will warn. Will run on pre and post builds.
    2. Plugin support both: Your system will warn (y?). My system won't warn. Will run on pre and post builds.
    3. Plugin support default constructor: Your system won't warn. My system won't warn. Will run only on post builds.
    So now explain me please, why you have to print a warn message in the second case? I mean in my system it will always call the default constructor (if available) and this is your preferred way and the author hasn't to change anything. Sometime when (s)he decides to drop support for pre builds the developer could remove it. Also it is only a change of 4 lines. I'm disappointed and will still support both constructors as there are some ones who uses older builds.

    Fabian

    PS: Same as case 3 is to calling the default constructor inside the TSLPC as the TSLPC is then only an alias for the default constructor.
     
  10. Offline

    Dinnerbone Bukkit Team Member

    ConsoleCommandSender no longer has a default constructor, use ConsoleCommandSender(server)
    --- merged: Feb 24, 2011 12:22 PM ---
    Support for the previous constructor and disabled event registration will be removedtomorrow. Any plugins which still use it will be broken.

    (Basically: If you receive a nag in your console, it means update it or it'll break this time tomorrow.)
     
  11. Offline

    The_Wrecker

    @Dinnerbone

    is there going to be a method I can override which gets executed before onEnable and with all the variables filled in?. I still can't move all my code to onEnable.
    I agree the constructor had to go, but such a method could also be elegant in my opinion.

    The method could look something like this:
    @Override
    public void Initialize()
    {
    /*
    Initialization and memory declarations code here
    Making the plugin ready to be enabled in onEnable().
    */

    }

    I'm afraid that I have to resolve to some real hacky/dirty programming.

    I wonder what happens when Bukkit is getting support to enable and disable plugins on the fly.The onEnable() would be executed multiple times. With ALL the initialization code in there...

    If the method already exists (I didn't check the latest build, but I also did not see anyone mentioning its existstance on the forums) could you say so here? Or make some sort of announcement?
     
  12. Offline

    Dinnerbone Bukkit Team Member

    What do you need that you can't put in onEnable?
     
  13. Offline

    The_Wrecker

    No I can't put it all in onEnable.
    It has to be done before another plugin get's enabled. And I don't have all the variables in my contructor yet (they are null).

    As said before I'm afraid I'll have to implement the thing in a dirty way. If I can't do some initialization before the onEnable().

    If you search the forums, you can find postst of other users who are having similar issues with the latest change. (It was in The long constructor topic as I recall it).



    I know that I should start my threads, processes and register listeners in the onEnable (and I do that, don't worry), but it would also be nice to have those objects set up and ready to go before calling onEnable();

    It's like filling the car up with gas and the onEnable() is like turning the key to start it.
     
  14. Offline

    Dinnerbone Bukkit Team Member

    For what reasons?
     
  15. Offline

    The_Wrecker

    Let my ask you, why you won't add it? Or do not like it or wan't it?

    or why you are so reluctant. What are your reasons?
     
  16. Offline

    Dinnerbone Bukkit Team Member

    What you're saying sounds like dirty hacky code. I can think of absolutely no legit reasons you'd want to do what you're trying to do, so I need you to tell me otherwise.
     
  17. Offline

    The_Wrecker

    So you are implying that adding such a method would inevitably result in hacky/dirty code? I think it isn't necessarily the case with that.

    Anyway let me explain / put it this way.

    I need to instantiate (among other things) an object with a path to its data folder (getDataFolder()) :
    PluginControlPlugin.permission = new PermissionNiji(folder);

    This object is accessed by another plugin at any given time(except when that plugin is disabled).

    What is my guarantee that that object is not null? Can I enable that one plugin from within another plugin when it is not yet enabled?

    I think I could, but I don't know if I should...


    In other words I have plugin(s) (I was planning on making more) that access resources from within another plugin. That other plugin acts like a code-base. A basic set of extendable/usable features. Any feature built in the'code-base' would automatically also be available in the other plugins I make.
     
  18. Offline

    DjDCH

    Why nobody have tell that BlockDamageLevel.BROKEN wasn't working and has been replaced by onBlockBreak ? I have a chance to have found this relatively quickly: http://bugs.bukkit.org/issues/476
     
  19. Offline

    Afforess


    I believe the Bukkit team plays a game called "Break the Plugins!". Points are awarded for each plugin broken after each git commit. Points are doubled each day the plugins remain unfixed. A bonus of 500 points is awarded if the plugin is still not fixed after 2 weeks. The only rule is that commits must still add features to Bukkit/CraftBukkit, so the pretense of working is maintained to users.

    IIRC, Dinner is winning.
     
  20. Offline

    Dinnerbone Bukkit Team Member

    Winning? I used to hold the prize trophy but I dropped it :(
     
  21. Offline

    cjc343

    You actually want to do pretty much the same as permissions initialization. From the plugin that uses your code-base:

    Code:
    //modified setupPermissions method courtesy of Nijikokun and Acru and now redone to be 'applicable' by me.
    private void setupBackend() { //was: Permissions
        Plugin p = getServer().getPluginManager().getPlugin("BackendName");
        if (p == null) {
            //log error
        } else {
            getServer().getPluginManager().enablePlugin(p);
        }
    }
    Should work.
     
  22. Offline

    The_Wrecker

    @cjc343

    Accessing the plugin itself is not the problem. I already use that bit of code. It's the object inside the plugin that needs to exist.
     
  23. Offline

    EvilSeph

    There are always unanticipated issues whenever we need to update our code to work with the latest Minecraft update. This is one of them...
     
  24. Offline

    cppchriscpp

    A bunch of older plugins have broken as of builds 449 and 450; they're throwing invalid plugin exceptions. Anyone know why?
     
  25. Offline

    Afforess

    Bukkit got sealed, I expect those plugins are using com.bukkit.something, which is not accepted.
     
  26. Offline

    cppchriscpp

    Ah, thanks for the quick reply. If the need arises and source is available I could fix those myself.
     
  27. Offline

    Archelaus

    Also, plugins now using the constructor throw an exception instead of the warning.
     
  28. Offline

    The_Wrecker

    I think I got it fixed using only onEnable(). I would still like an initialize method of some sort though. onEnable() is getting cluttered (even though I put stuff in other classes objects and methods.

    So I would be totally in favor of such a method. Just so you guys know.
     
  29. Offline

    Dinnerbone Bukkit Team Member

     
  30. Offline

    matejdro

    Then how can we prevent commands like /kill?
     
Thread Status:
Not open for further replies.

Share This Page