LuckPerms

LuckPerms

41.4k Downloads

Need a way to unregister and unsubscribe.

Pangamma opened this issue ยท 9 comments

commented

In the API there are calls to register a context calculator and calls to subscribe to events. I need a way to unsubscribe and unregister as well so my plugin works well with loading and unloading. Otherwise I cannot say that my plugin handles memory management properly.

A lot of people say you should never reload plugins. Personally, I reload my plugins individually 100 times before a server reboot is considered because of good housekeeping practices. To prevent any memory leaks I would like to have some unregister and unsubscribe API calls be added to luckperms. Can we make this happen? If so I would be very grateful. Thank you.

commented

Based on what I have seen so far from the code I believe the easiest way to add this functionality without breaking existing API calls would be to make your system like the bukkit scheduler system. When you add a listener or something of that nature, return an integer key that can be used to remove the item later. Alternatively, allow people to unregister all "things" that have been registered by plugin X.

commented

Unregistering event handlers

Calling EventBus#subscribe returns a EventHandler instance. You can call EventHandler#unregister to unsubscribe.

Alternatively, you can use the subscribe method which accepts a plugin parameter. This will automatically unregister the handler when your plugin disables. (in the same way the Bukkit event system works)

Unregistering context calculators

(this was only added in the commit referenced above)

Use ContextManager#unregisterCalculator.

commented

@lucko have you tested that fix on bungee cord? I see some potential logic issues where you insert a wrapped objects into the list in such a way that the API color does not have a reference to the item that will need to be removed from the list later on. I am reading the code on my mobile phone right now though. It's a possibility that I am missing something or misinterpreting commits

commented

I haven't tested but it should be absolutely fine.

commented

@lucko One of the unregister methods has a blank method body. Is that intentional? I noticed that the register method in that same class is not left blank.

Also I do not use sponge API but what I was browsing the code for that I noticed the register method would wrap the inserted context in a proxied sponge context wrapper or something like that. If that is the case, it would be impossible to remove because the API color has no reference to the wrapped instance of the object. Only a reference to the inner object

commented

Can you send a link to that class?

Also I do not use sponge API but what I was browsing the code for that I noticed the register method would wrap the inserted context in a proxied sponge context wrapper or something like that.

That's correct, but the Sponge API doesn't offer a way to unregister calculators anyway.

commented

Thanks for fixing it man!

commented

Woops - thanks!