New event handling system

Hello,
I'm working at the moment on a new event handling system. And I want to discuss it first here, because it isn't finished yet, and I want to know, if this handling system has any chance. If so, I will continue the development and implement also the latest events (e.g. the portal event should be missing at the moment).

I was inspired by this “ugly” method and the commentary below: EventExecutor JavaPluginLoader.createExecutor(Event.Type type, Listener listener)

At the moment there are different types of event, and for each type, it will call the correct method in the correct class. Now you have to extend the specific Listener if you want to listen to a event. So you couldn't create classes with to different types of Listener (e.g. PlayerListener and EntityListener).

I now implement a different way of receiving events. The calling is mainly the same (only if the event class name has changed). This will force all plugins which uses listeners (hmmm which does not?) to recompile the plugin. But the changes could be done in 5 minutes (I updated xWarp in less than 5 minutes).

The basic interfaces are org.bukkit.event.Event and org.bukkit.event.Listener. Every event and listener (like before) implements this.

Now the changes in Event:

• Instead of an enum, it has a String of type. So other plugins could create new events/listeners without using the previous enum.
• The execute method, calls the corresponding method in the listener. So for example you have a listener with a method onFoo and a FooEvent, this event would all the listeners method onFoo.
• And the getListener() is only there, to be able to verify is the event handle a specific event. Unfortunately Java doesn't support anything to get the Class<T> via generics.

And the listener is empty as there are no general methods. Instead each type of listener simply defines a single method. For example if you want to listen to “onPlayerMove” you implement in your listener class the PlayerMoveListener. Now if there something fires a PlayerMoveEvent following will happen:

1. SimplePluginManager.callEvent(Event): This will select (depending on the priority) which plugin get first informed.
2. RegisteredListener.callEvent(Event): This simply verifies that the correct listener is called/the event is matching.
3. PlayerMoveEvent.execute(PlayerMoveListener): And there the listener get called.

The rest is in the hand of the plugin. Now the best is to add a new event type:

1. Create a listener interface (like PlayerMoveListener).
2. Create a event class. There are several ways to do this. The minimum requirement is to implement the Event interface. But I also created some helper classes. For example for the PlayerMoveEvent:
   1. PlayerMoveEvent →
   2. AbstractPlayerMoveEvent →
   3. CancellablePlayerEvent →
   4. PlayerEvent →
   5. AbstractEvent →
   6. Event
3. Call somewhere the event.
4. Done.

The second part looks complicated, but I splited the functionality in different classes, so I didn't have to rewrite the Cancellable part for every player event which could be canceled. Also there is also the PlayerTeleportEvent which is a movement event. And I have code duplication (if it isn't reasonable).

These 3 steps are counting for the bukkit developers as for the plugin developers. So in basic there is no difference and CustomEvent anymore. Especially you don't have to modify the enum and ensure, that you create a EventExecutor. In this layout you couldn't accidentally miss anything.

Now has this any chance? The main problem (and because of this there is the plugin support break) is, that the type of Event changed. So I had to move the enum, and even this would break all listener using plugins. Maybe you could introduce this, when you event want to break something (e.g. new Location classes).

If you want to look inside the code I created a new-events branch: org/bukkit of new-events.

Unfortunately java don't support multiple class inheritance, so for each event category is an own CancellableEvent class.

Fabian

 

Why is String better than Enum?

 

Because you don't need no enum anymore, because the information is encoded in the event class itself. And other if you want to add an event, you don't have to modify the enum. I only “need” the type, to print out in which listener type the error happens:

Code:

server.getLogger().log(Level.SEVERE, "Could not pass event " + event.getType() + " to " + registration.getPlugin().getDescription().getName(), ex);

Fabian

 

I see

 

Meh. I don't even see the need for the string. I've always used instanceof to determine the correct event for my custom events. You could just print event.getClass() to the error handler if you want an exact name.

If such large break is going to happen, I'd want to push through some other breaking changes too. Location & Vector should become Interfaces.

Also, rewriting the event priority system would be nice. I mean, nobody uses any priorities except NORMAL and LOWEST, and MONITOR. So ditch the rest.

New Priorities:

• Priority.Test_Permission
  • called before any others, used to test the permission of the event, e.g, if it can happen.
• Priority.Normal
  • name says it all
• Priority.Post_Event
  • called AFTER the event has taken effect, replaces MONITOR, removes the need to use the scheduler to delay 1 tick to examine the effects of things like redstone changes, damage, etc.

 

Because of this great break, I wanted to discuss this first, because (as you already said) other breaks should be done with this one. And because of this I didn't pushed my new Locations, because I want to prevent that the plugin devs have to update the plugin twice.

With your getClass() as type: Nice idea, and as I don't see any other need for the string, as in the listed line above, I will change this.

To your Priority change: Now I think this is independent from my changes, so I think this should be discussed differently, but I see one problem: The Post_Event Priority, is hard to determine, because it is like a different event. At the moment the server/plugin fires an event, and it will go through all priorities. And after this the server will do the rest (if not canceled).

Fabian

 

I like Zise his idea, for the reason listeners in Java are always interfaces. But there is always a but, as you pointed out, in order to prevent what Java does: forcing you to implement ALL functions described in a "PlayerListener", you will have to create seperate listener-interfaces for each event you can think off and additional classes. Java-listeners usually have like 4-5 methods in them, not just one.

My concern with this idea therefore is performance. Bukkit on its own is already quite "heavy", to say the least. Together with custom plugins there is a thin line between stable/unstable servers. I'm already foreseeing plugin-programmers creating a seperate class for each listener. With Java being an interpreted language with a so-so GC, that can never be good.

Have you done performance tests where you compare properly programmed listeners (preferably in one logic class with all required listeners per functionality) to the current Bukkit method? Did it differ? How did Java react if you had like 20 implemented listeners compared to 20 overrides using Bukkits method?

 

No I didn't any performance tests yet.

Fabian