TheFantasyCraft / Forge-Permittor

Recoded Tekkit Permittor that automatically find blocks that need to be protected.
GNU General Public License v2.0
5 stars 8 forks source link

Use event system rather than custom hooks. #20

Open boy0001 opened 9 years ago

boy0001 commented 9 years ago

Instead of using the standard event system, it is reliant on custom hooks for every protection plugin: https://github.com/TheFantasyCraft/Forge-Permittor/blob/master/src/main/java/com/fantasycraft/forgepermittor/protection/ProtectionManager.java

The simple fix would be not to develop a hook for each plugin and instead using the event system. This can be done by calling and checking the result of BlockPlaceEvent, or PlayerInteractEvent.

Bukkit.getServer().getPluginManager().callEvent(event);
if (!event.isCancelled()) {
    // Do something
}
thomas15v commented 9 years ago

Hmmm indeed that would be much simpler. But it still would require a hook sadly enough. Like for example core-protect would log things twice than, and thats kinda bad.

I think that if I can grab the plugins listener and call the listener instead. I could get some better results.

boy0001 commented 9 years ago

To exempt specific plugins from an event, you could try something similar to the code below. Then you could put a section in the config for plugins to ignore.

// Preprocessing to check which listeners need to be unregistered //
// This only needs to be done once, and you store the results of toRemove //
Set<String> exempt = new HashSet<>(Arrays.asList("CoreProtect"));
ArrayList<RegisteredListener> toRemove = new ArrayList<>();
HandlerList handlers = PlayerInteractEvent.getHandlerList();
for (RegisteredListener listener : handlers.getRegisteredListeners()) {
    exempt.contains(listener.getPlugin().getName());
    toRemove.add(listener);
}
// end //

// The following would be inside your check
PlayerInteractEvent event = new PlayerInteractEvent( ... );
HandlerList handler = PlayerInteractEvent.getHandlerList();
for (RegisteredListener listener : toRemove) {
    handler.unregister(listener);
}
Bukkit.getPluginManager().callEvent(event);
if (!event.isCancelled()) {
    // do something
}
for (RegisteredListener listener : toRemove) {
    handler.register(listener);
}

Another solution which is not foolproof, and probably a bit hackier would be to have your own listener and cancel it during EventPriority.HIGHEST so that it is not used by anything listening to the result on EventPriority.MONITOR The only issue with this is if some protection plugin uses EventPriority.HIGHEST in which case there is a 50% chance it wont work for that protection plugin.

thomas15v commented 9 years ago

https://github.com/TheFantasyCraft/Forge-Permittor/blob/8c059e186031f2b156cfa50ac6e31a25645a33a5/src/main/java/com/fantasycraft/forgepermittor/protection/ProtectionManager.java#L25-L41

I did some test with that way and its looking good :smile: . I will just make it configurable. Something like:

plugins: ["towny", "plotsquared"]

I don't like the guessing thing. I only want to give the event for plugins that need it. And I am sure a server owner would think the same of it.

boy0001 commented 9 years ago

That looks great! A whitelist is fine way to go, I was just giving alternatives. :)

boy0001 commented 9 years ago

I'll close this since it was added in: fb8091db3c2242b7af0f283e0eece93fdfcff9cc

thomas15v commented 9 years ago

Its far from done tho :/. But indeed the basics are their. Just need to figure out something for damage events....

thomas15v commented 9 years ago

After some testing. This build of the plugin totally doesn't work like intended. I reopen this issue so I don't forget to work on this.

Examples of problems: