LiteLDev/LiteLoader.NET

Effective event system

S3v3Nice opened this issue ยท 11 comments

I would like to be able to create custom events and handle them along with the standard ones in the EventListener class - not with Subscribe(), but with method attributes ([EventHandler]). Like in Bukkit, Nukkit, PocketMine... And for this, it would be useful to have a base abstract class on which the standard and custom events are based. Is it possible to implement this?

Pd233 commented

okay...But how to implement it? Could you give some suggestions?

I suggest replacing the template EventTemplate with an abstract class EventBase.
But if LLNET is based on the LiteLoader SDK, I don't know if this is even possible. However, if it is possible, it would be great and more correct from an OOP point of view in C#, I think.

Registering the event listener class would be done through the EventManager class (EventManager.RegisterListener<IListener>). Listener's methods with [EventHandler] attribute would be checked there, and if a method takes a single argument of type EventBase, then put it in the list of handlers of this event type (in EventManager or in a separate HandlerListManager class).
Calling event can also be implemented in EventManager class (EventManager.CallEvent(EventBase e)).

And it would be great if you could cancel the event (event.Cancell() if event class implements ICancellable interface) :D

I see developments on the new event system, looks cool, just what I wanted. But I have a couple of comments:

  1. EventAttribute is not really needed. You can use it to set a custom id for an event, but what for? And user can by mistake specify already existing id, and an exception will be thrown.

  2. If the basis for an event class is an interface (IEvent), then you will have to constantly implement IsCancelled and Cancell() yourself. Wouldn't it be better to make an abstract class with IsCancelled, Cancell() (or without it, there is IsCancelled), and Call() (which will refer to EventManager.CallEvent())? And it is better to add the interface ICancellable, then IsCancelled property of the abstract class will check whether the current class implements the ICancellable interface (if not - exception).

Pd233 commented

I see developments on the new event system, looks cool, just what I wanted. But I have a couple of comments:

  1. EventAttribute is not really needed. You can use it to set a custom id for an event, but what for? And user can by mistake specify already existing id, and an exception will be thrown.
  2. If the basis for an event class is an interface (IEvent), then you will have to constantly implement IsCancelled and Cancell() yourself. Wouldn't it be better to make an abstract class with IsCancelled, Cancell() (or without it, there is IsCancelled), and Call() (which will refer to EventManager.CallEvent())? And it is better to add the interface ICancellable, then IsCancelled property of the abstract class will check whether the current class implements the ICancellable interface (if not - exception).

EventId provides a way to call this event faster. If you provide EventId when calling an event, EventManager will skip checking event type. This feature will be added after finish this system.

EventId(0~128) were reserved by EventManager, it's provided for native events. If your costom event's id is zero, EventManager will allocate a unique EventId for it.

And Inheritance is a valuable resource in .NET I think, so I used an interface to replace it. And you can also implement more functions by using this interface. :D

EventId provides a way to call this event faster. If you provide EventId when calling an event, EventManager will skip checking event type.

But you can just take an event class type and take its id from eventIds, no need to check if the type is correct, as all the events added to eventIds have been checked. I don't think it takes much time for this process :)
And to get the id of the event class, you need to get the type of that class anyway. Or do you mean it would be possible to use EventManager.CallEvent(int eventId)? But is that convenient? Then you would need to remember the id of your events :/

And Inheritance is a valuable resource in .NET I think, so I used an interface to replace it. And you can also implement more functions by using this interface. :D

What's wrong with an abstract class? I've googled about this and found no information that the abstract class is bad at performance. Moreover: "The performance of an abstract class is fast. The performance of interface is slow because it requires time to search actual method in the corresponding class."

Pd233 commented

What's wrong with an abstract class? I've googled about this and found no information that the abstract class is bad at performance. Moreover: "The performance of an abstract class is fast. The performance of interface is slow because it requires time to search actual method in the corresponding class."

.NET class can only have one base class. If I occupy it, my conscience will be disturbed.
D0F25D1DF1246EDED36DC56C5C0C18D6
.

.NET class can only have one base class. If I occupy it, my conscience will be disturbed.

Oh, there's no problem with that. You can inherit an abstract class that inherits an abstract EventBase class and complements it.
For example, there are two events PlayerPlaceBlockEvent and PlayerBreakBlockEvent. They inherit the abstract class PlayerEvent, which inherits the EventBase class.
This is normal practice in languages such as Java and C#.
You can check how this is implemented in Bukkit or Nukkit, for example. Exactly like that)

Pd233 commented

okay...I will redesign it. :D
Call event by EventId is almost designed for native event, for highest efficiency.

Call event by EventId is almost designed for native event, for highest efficiency.

How about making EventAttribute optional then? And maybe rename it to EventIdAttribute :D