Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reworking the event listeners #65

Open
erik1988 opened this issue Jan 5, 2017 · 2 comments
Open

Reworking the event listeners #65

erik1988 opened this issue Jan 5, 2017 · 2 comments

Comments

@erik1988
Copy link

erik1988 commented Jan 5, 2017

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) }

@RoboMWM
Copy link
Member

RoboMWM commented Jan 5, 2017

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).

@RoboMWM
Copy link
Member

RoboMWM commented Nov 18, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants