Events

[Rongmario]

Summary

Maintain compatibility with current event system that Forge provides, and to enhance upon it in both performance and cleanliness.

Goals

• Discontinue usages of multiple EventBus (thus merging current TERRAIN_GEN/ORE_GEN event buses).
• Discontinue usages of both Forge and Guava event systems.
• Allow asynchronously firing events, but properly denote the capability of doing so.
• Allow programmatic registration of event listeners, rather than it being purely annotation-based.
• Low memory footprint & low overhead in every part of the event system.
• Clear up the confusion when listeners aren't being properly registered.
• Ensure events receive correct contexts & know which mod they are registered from.

Non Goals

• Not to remove IGenericEvent, as they are abundant in functionality and uses.

Motivation

Events have always been a bikeshedding topic, even more so now. Thus, it is perfect to take all the good talking points from modern Fabric and Forge and attempt to establish a clean approach for Cleanroom's events.

As well as that, Forge's event system have always suffered from annotation magic and the unclear nature of how it was being registered. Developers would see no errors when registering their event classes but finding out the listeners were inactive. It is time to clear the confusion and to modernize the system while keeping overheads low.

Description

Events right now work as such, and we can see that there are two different ways of registering events and how they differ vastly.

1. Guava Event System: @Subscribe for FMLEvents
2. Forge Event System which uses @SubscribeEvent for Events

This should be united, there is no reason of separating these. As well as the different EventBus types:

1. Main: MinecraftForge.EVENT_BUS
2. Ore Gen: MinecraftForge.ORE_GEN_BUS
3. Terrain Gen: MinecraftForge.TERRAIN_GEN_BUS

... should also be united. The configuration can differ from one Event to another Event instead.

Registration of events should be revamped as well, away from being annotation-based. With using modern lambda syntax such as:

public class CustomEvent extends Event {
    // ...
}

// Having to explicitly state the class here maybe necessary instead of using the diamond operator, due to erasure issues
Event.register(CustomEvent.class, customEvent -> { });

With using lambdas like this, using ASM to create event listener classes internally would also no longer be required, as the event should call this lambda like its a callback (it essentially is). However, what if we have a lot of listeners to register, registering it one-by-one could be troublesome also. Hence we still use the old method of EventBus#register(Class), and we essentially re-route each method to the Event#register method, but instead of as a lambda, we could utilize the invocation API's MethodHandle, with near if not native call speeds with some penalty in bootstrapping.

[Unfinished Post]

Dependencies

No response

References

No response

Guidelines

• I have and will continue to adhere to the guidelines

[Desoroxxx]

Sounds and looks great so far

[Ecdcaeb]

I expects

MinecraftForge.EVENT_BUS.addListener(  (Consumer<RegistryEvent.Register<Item>>) (evt -> {...} ) )

[Rongmario]

That wouldn't work, unless you mean

MinecraftForge.EVENT_BUS.<RegistryEvent.Register<Item>>addListener(evt -> { });

Which wouldn't work for generics, hence why modern Forge has a separate class passed in for which generic to target.

[Ametsuchiru]

The syntax seems good enough.

Events.register(CustomEvent.class, customEvent -> { });

For generics, the equivalent would be:

Events.register(CustomGenericEvent.class, GenericType.class, customGenericEvent -> { });

I think we should still allow an annotation-based way of registering, similar to Forge's, but perhaps with MethodHandles backing it. I had written a few tests and the MethodHandles way of invocations pretty much match direct invocation speeds. Perhaps it would even have less overhead to Forge's ASM class generation way as that has a pretty hefty wrapper around it.

I do believe there is still overhead to be optimized when actually publishing (firing) the events. Such as the listener should not be checking if itself is a generic event listener, etc.

[Rongmario]

@Ametsuchiru waiting on your benchmarks, I need some numbers, preferably with comparison to Forge's current ASM system.

[vfyjxf]

In neoforge,they have a checker for event listener
https://neoforged.net/news/20.2eventbus-changes/

[vfyjxf]

The annotated approach I think is better because I don't need to call a registered function somewhere obvious, just an annotation is enough.

[vfyjxf]

Also take a look at Arch Event and KubeJs, they have some nice designs as well.

[Ecdcaeb]

As I mentioned before in Discord, I hope ModBus (@EventHandler) supports static methods. Or, by extension, let Mod Bus and Event bus share the same underlying implementation.

IBus -> CommonEventBus -> SignaturedEventBus (set the active mod container) -> ModBus & EventBus

To maintain compatibility, ModBus extends the original EventBus (google), implements IBus, and delegates to a SignaturedEventBus.

Now, mod bus uses reflection driver and constructs a bus for each mod to implement activeModContainer. For compatibility considerations, we have almost no possibility of parallel loading like high versions, so one bus based on MethodHandler is enough.

I wonder if we should start now and make some related attempts or stopgap measures.

[Ecdcaeb]

Registration of events should be revamped as well, away from being annotation-based. With using modern lambda syntax such as:

I'm curious if this API is worth it. Most mods don't use lambda directly, but build a method and then reference it (Foo::foo). This just replaces @SubscribeEvent with explicit registration, which I don't see any advantages, but makes things more troublesome:

1. We need to get the type of Consumer, as you can see, Generic will be broken and handled separately.
2. Bug handling becomes more difficult: novice modders often forget to register their listeners, and explicit registration with method references makes remote control more difficult.

In summary, supporting lambda has no visible benefits and will bring visible disadvantages. We can't support lambda just for the sake of supporting it - as a Java feature. If we support it just for the sake of supporting it, we can also try to support the new DataFixUpper (with Codec) and Brigadier.

[Ecdcaeb]

That wouldn't work, unless you mean

MinecraftForge.EVENT_BUS.<RegistryEvent.Register>addListener(evt -> { });

Which wouldn't work for generics, hence why modern Forge has a separate class passed in for which generic to target

We can try introducing ListenerToken to implement generics and unregister.
use the ListenerToken to wrap a Consumer.

var token = BUS.addListener(new ListenerToken<AttachCapabilityEvent<ItemStack>>(event ->...){});
token.unregister();

Works like TypeToken.