Fabric API

Fabric API

106M Downloads

Event#unregister(T)

Johni0702 opened this issue · 4 comments

commented

There should probably be a way to unregister(T) a previously register(T)ed listener from an Event<T> if one is no longer interested in it.

Such a method should be fairly straight forward to implement except that it's a backwards-incompatible change to the Event<T> type and I'm not sure how fabric deals with those.

commented

Alright, I was under the impression the events were intended to be more like those in Forge/Sponge and less like an abstraction on top of Mixins.
Putting your own distribution layer on top would probably require quite a bit of code duplication (more or less the current invoker code for each event) but works for me because I'm only using like two or three of the events provided by fabric and am mostly injecting my own, application-specific ones, so I'm going to do that.

commented

I purposefully did not add unregister(T), fearing that mods would abuse it to do something silly like register and unregister themselves once a tick - register operations are assumed to be "slow" (not seconds slow, just "slow" as in not instantenous or not as cheap as just adding to an ArrayList).

commented

While I can understand your fear, I don't think that outweighs having to sprinkle what will effectively have to be singletons (the listeners) all over ones code.
I can think of several use cases for being able to unregister a listener, most of mine amount to having a listener registered for a certain amount of time (e.g. while a specific GUI is opened, while in a specific world or while a specific player entity exists) but then unregistering it once the GUI/world/entity has been disposed of.

The performance argument can also be made the other way around:
I inject lots of events into various rendering-related parts of MC which are only active a small fraction of the total play time (think: 360° screenshots) and I'd much rather take the small overhead from registering and the unregistering them once every time the user takes a screenshot over having the hooks active all the time and having to do a check whether I'm currently taking a screenshot X times every frame.
And that is me being aware that it's getting called X times per frame. Someone who's not aware of that might perform who knows how complex tasks X times per frame which can basically be just as bad as abusing an unregister method.

commented

The event system is designed for hooks, i.e. injecting calls that effectively embed subscriber-supplied code at the call site, a bit like dependency injection. They are not meant for notifications or what you'd expect from other kinds of event systems. If you want to the the latter, a distribution layer on top may be advisable.

In the future the overhead from changing the subscribers may grow significantly, I have some ideas around generating handler invocation code at runtime that allows the JVM to inline through everything.