There have been discussions of the event system used in NeoForge, but no plan has been decided yet. This document exists to propose a plan for 1.20.2 by collecting all the issues that people have with the current system and proposing fixes.
:heavy_check_mark: = cpw approval, not done yet
:white_check_mark: = done
### Mod busses
At the moment, FML and registration events go on the mod bus. I think this is a fine choice. There are some usability issues, but these can be addressed:
#### Proposed changes:
1. Prevent registering mod bus listeners to the Forge event bus. See https://github.com/neoforged/Bus/pull/11. :white_check_mark:
2. Make event phases work as expected for events posted via `ModLoader` (i.e. all Forge events dispatched on the mod busses). See https://github.com/neoforged/Bus/pull/12 and https://github.com/neoforged/FancyModLoader/pull/8. :heavy_check_mark:
### Event inheritance
At the moment, listeners can subscribe to a parent class, and receive all the subclass events. This can be error prone, for example if a modder subscribes to `PlayerEvent` instead of one of the subclasses.
I propose that we remove this feature.
#### Proposed changes:
3. Remove inheritance for event listeners. Listeners will only receive the exact event class that they subscribed to. This also applies to the `@Cancelable` and `@Event.HasResult` annotations. :heavy_check_mark:
4. To reduce user mistakes, throw an exception if a listener is registered for an abstract event class. Also make `Event` itself and the various holder/parent event classes (`PlayerEvent`, `TickEvent`, ...) abstract. :heavy_check_mark:
### Performance
EventBus already has good performance, but for some cases we might want to squeeze out every nanosecond. I identified a few areas where performance can be improved.
#### Proposed changes:
5. Remove `IEventListener` and replace it by `Consumer<Event>` to reduce indirection. (Because lambdas already implement `Consumer`.)
6. Do not wrap non-cancelable events behind a check for `event.isCanceled()`. (The check is useless and the wrapper adds indirection.) :heavy_check_mark:
7. ~~Add a `protected reset()` method to `Event` to allow reusing an event object multiple times. With this, we will go from 1 object allocation per dispatch to 0 allocations per dispatch.~~ (Not needed anymore with change 12.)
### Generic events
Generic events have poor performance and are not particularly friendly. The only remaining use of them in Forge is `AttachCapabilitiesEvent`, which is expected to disappear with the capability rework. As such, I propose deprecating the feature for removal.
#### Proposed changes:
8. Deprecate `GenericEvent` and related methods for removal. :heavy_check_mark:
### TypeTools
Some people dislike the use of typetools for `addListener` type resolution. But for now it's still working and reasonably convenient. We should also make it convenient to not use typetools.
Note that with proposed changes 3 and 4, Typetools type resolution failures will lead to an exception, thus reducing the probability of mods shipping broken code to production.
#### Proposed changes:
9. Add more `addListener` overloads to make it more convenient to not rely on typetools. See https://github.com/neoforged/Bus/pull/8. :white_check_mark:
### ASM transformations
While ASM transformations work at the moment, they are not necessary, and can be removed to make the code easier to follow and likely easier to maintain in the future. One can show that removing ASM transformations does not reduce performance.
#### Proposed changes:
10. Remove ASM transformations. See https://github.com/neoforged/Bus/pull/2. :heavy_check_mark:
### Cleanup
#### Proposed changes:
11. Remove `IEventBusInvokeDispatcher` entirely. I do not see the use case for it, and could not find a use through GitHub code search either. :heavy_check_mark: (assuming neither Forge nor FML is still using it)
12. Remove `Event#getPhase` and `Event#setPhase`. I do not see the use case, and it prevents posting the same event multiple times. :white_check_mark:
### Wishlist
Items for which the solution hasn't been figured out yet:
#### Some way of printing the listener list for debugging puroposes
> One very low-priority wish I'd have is an easier way to ask the bus for a purely informational listener list for a given event as it would allow better debugging in certain cases without having to cause a crash in a listener to have the list printed in the error
>
> make sure, if such a method is implemented, to explicitly specify the contents being a implementation-specific human-readable blurb, akin to iirc the spec for Object#toString()
#### Possibly removing the annotations
> hmm no mention of removing the Cancellable & HasResult annotations and replacing them with equivalent interfaces?
#### Possibly removing the Event.Result enum
> also can we remove the [...] result enum. it's way too generic and causes a lot of confusion, each event that uses one should provide its own enum with descriptive names and docs