Reworking the event listeners
erik1988 opened this issue ยท 2 comments
The way it works not is that every listener is activated regardless if they are on/off in config (e.g. weight in water). Then when an event is activated (e.g player move) it will check every time if its activated in the config (thats allot of checking).
However these settings would only change during restart or reload so its unnecessary to check it every time. This will create unnecessary calculations for the server regardless if you have this feature activated or not.
what I propose instead is to change the way this work and instead only register the listeners if they are activated in config.
for instance:
registerModule(Water.class, new Water(this));
is changed to this:
if(wateractive){ registerModule(Water.class, new Water(this)); }
And then the check is removed from the listener.
This will then only be checked on statup/reload and never again.
Alternatively the check chan be moved before the listeners:
if(wateractive){ @EventHandler(priority = EventPriority.NORMAL) void onPlayerMove(PlayerMoveEvent event) }
Though lag from using unnecessary listeners is probably minimal at worst, I'm open to a PR that only registers the necessary listeners.
However, if such a PR is made, consideration for #24 should also be kept in mind. I think the code, as it is(?) allows for such a change, since it is calling some abstracted method for checking if the feature is enabled for that world (which, afaik right now, is simply returning whether EHM is enabled for that world, and if so, if it is enabled in the global config).
There are only a few cases where registering listeners causes lag because of how the events are constructed - InventoryMoveItemEvent is one of these few cases. Otherwise, the hit is negligible at worst. Many plugins like NCP do all sorts of things with PlayerMoveEvent but because the operations are fast they have very little impact.