TShock
TShock copied to clipboard
Event Handler Normalization
We have a multitude of different events, and a multitude of ways to hook onto them. Some things provide base dotnet events:
event EventHandler E;
E += MyHandler;
Some things use a home-grown hook handler:
HandlerList<EventArgs> E;
E += MyHandler;
//or
E.Register(MyHandler);
Some things use TSAPI hook handling:
HandlerCollection<EventArgs> E;
E.Register(TerrariaPlugin, MyHandler);
Some things use OTAPI hook handling:
OTAPI.Hooks.Net.GetData = MyHandler;
We really need to decide which we like best and normalize on it.
I think there's something to be said for the fact that standard C# events don't support the concept of priorities. Register with a method gives us that (unless someone can figure out a way to make an annotation do that?). However += might be good to have in the event that you don't really care for priority (things like Bukkit's Monitor priority are good for that).
I support @up, we should have everything using .Register to support priorities because they're just required for some types of plugins like rate limiters or anticheats. Perhaps also a global class that has allows registering eventhandler classes? (another nod towards Bukkit)
@QuiCM for what it's worth, I would support this being merged with reasons for handling too, but I can see that being out of scope.
From #1570: Events should be able to do something like Handle("Reason for preventing event propagation");
Since hooking a boolean being changed isn't feasible in my opinion, I'd say that pointing at which plugin "dun did it" should be enough for now. Redoing how event handling works will break every single plugin in existence and that's bad.
The idea works on paper but in practice it'll be a pain to refactor and work with (and let's face it a lot of people are sitting at 4.3.25 (including me) because 4.3.26 is breaking enough, can we please not make it even worse for 4.3.27 or so?)
I think an event refactor is necessary, but I also think adding reasons to everything requires a lot of "re-understanding" the code you'll have to edit to get it working and that's unnecessary effort. Unless reasons are optional, then whatever :P
No longer WIP. As per above, this should probably be reserved for a new major version or similar.