Introduces experimental foundation for plugin events#36
Introduces experimental foundation for plugin events#36VideoGameSmash12 wants to merge 2 commits into
Conversation
|
Since we are currently working on a branch where we're improving the entire command system, and your PR is going to affect that, this probably won't be looked at until further notice. |
|
I think that this is a good idea if plugins will be developed targeting TFM, but Bukkit's event system obfuscates the flow of code. That's fine when you're using the event listener to implement side effects of something like unfreezing newly promoted admins, but it seems like unnecessarily convoluted design and perhaps dangerous design to use an event listener for direct side effects like saving the added admin & saving the admin. |
|
Something to keep in mind, listeners for events must never call back into the caller that fired the event. Persistence should only explicitly be handled by the intended management service. So it realistically only allows us to bundle reactions into a central location, instead of having tons of calls bouncing around, but we still have to maintain the actual underlying event target independent of the listener / event when dealing with actual persistent values. |
This is a bit of a proposition, so bear with me. The TotalFreedomMod currently has no event API. While this is fine since we likely don't expect people to hook into this plugin as much, another issue we often have to introduce code spaghetti in management/banning commands to manually call code in bridges or other FreedomServices.
This kind of... sucks, to be honest. So, this pull request does double duty as it allows us to just leverage Bukkit's event system to listen for when certain events occur. In the case of this pull request, it adds events for admins being added and removed. Instead of manually calling code in /saconfig which isn't very flexible, why not just have call a single method that calls the relevant event and then have FreedomServices listen for that event?
This is a barebones pull request as I wanted to see what others thought. Don't merge this until we have something more solid laid out.