yariplus / minecraft-nodebb-integration

A plugin for Minecraft servers for integration with a NodeBB forum.
Creative Commons Zero v1.0 Universal
9 stars 4 forks source link

Improper naming and failing to mention dependency #3

Closed Terrobility closed 8 years ago

Terrobility commented 8 years ago

You can always make another class to handle the OnTime dependency and change your other classes to use your OnTime task only if it's present. Since you're importing OnTime in a class which will always be run regardless, the plugin refuses to load if it's not present. I've added a temporary fix by listing OnTime as a hard dependency.

To break it down, this code refuses to work because you're importing net.milkbowl.vault and me.edge209.OnTime regardless of whether it's there in https://github.com/yariplus/bukkit-plugin-nodebb-integration/blob/master/src/main/java/com/yaricraft/nodebbintegration/NodeBBIntegration.java

        // Setup OnTime
        try {
            OnTimeAPI.data.values();
            ontime = true;
            log("OnTime found.");
        }catch (Exception e) {
            log("OnTime NOT found.");
        }

        // Setup Vault
        if (setupChat())        { log("Vault chat found.");        }else{ log("Vault chat NOT found."); }
        if (setupPermissions()) { log("Vault permissions found."); }else{ log("Vault permissions NOT found."); }
        if (setupEconomy())     { log("Vault economy found.");     }else{ log("Vault economy NOT found."); }

Bukkit also complained about the naming (it shouldn't have spaces). I've rectified it. When I get time, I will fix it for you.

yariplus commented 8 years ago

Thank you much.

You can go ahead and add Vault as a hard depend. I originally wanted soft depends, but I've changed my mind. No sense in using the plugin if half the features are crippled by a missing soft depend.

Terrobility commented 8 years ago

I may have a version that will start up without Vault and OnTime. I'll have to try it out first before committing.

yariplus commented 8 years ago

Sweet. That would work. Still, I'm fine with Vault being hard for sure, it's pretty universally installed. But not OnTime, I'd really like that to be soft.

Terrobility commented 8 years ago

I'll have to push it in about 12-24 hours, not now. I have made sure that it won't refuse to load without OnTime but I'd like to add the ability to customise messages using the config. It's really late for me and I won't get to finish right now.

yariplus commented 8 years ago

Awesome. No rush. I appreciate the help. :beers: Enjoy the Holiday. :) :christmas_tree:

Terrobility commented 8 years ago

You too 😉

-----Original Message----- From: "Timothy Fike" notifications@github.com Sent: ‎25/‎12/‎2015 04:35 AM To: "yariplus/bukkit-plugin-nodebb-integration" bukkit-plugin-nodebb-integration@noreply.github.com Cc: "InfamousOG" huntergplays@gmail.com Subject: Re: [bukkit-plugin-nodebb-integration] Improper naming and failing tomention dependency (#3)

Awesome. No rush. I appreciate the help. Enjoy the Holiday. :)
— Reply to this email directly or view it on GitHub.

Terrobility commented 8 years ago

There you go. You may want to look over other parts of the code as they may not work. I got to test the commands but I didn't actually link it up with my forum and go through it. The new Vault and OnTime should work, however it would be worthwhile if you tried connecting it to the forum without using OnTime.

yariplus commented 8 years ago

Awesome! Everything seems to work so far. I'll do some more testing before publish though.