API for altering how the characteristics of EventBus / EventListener are determined

[ZZZank]

The API of EventBus7 is quite unique, making it hard to migrate from previous version, and basically impossible if backward compatibility is required.

The main burden of migration is the change in how some key data are determined, for example:

• event priority is determined by byte instead of enum EventPriority
• whether an event is cancellable is determined by implement Cancellable instead of @CancellableEvent
• whether an event is cancelled is determined by predicate.test(...) instead of event.isCancelled()

So backward compatibility should be much easier if these process can be customized. This can be done by adding an API for determining the characteristic of EventBus / EventListener. There's a rough example:

// null -> fall through

public interface BusCharacteristicProvider {
    @Nullable Boolean isCancellable(Class<? extends Event> eventType);
    @Nullable Boolean isMonitorAware(Class<? extends Event> eventType);
    @Nullable Boolean isSelfPosting(Class<? extends Event> eventType);
    @Nullable Boolean isSelfDestructing(Class<? extends Event> eventType)
}

interface ListenerCharacteristicProvider {
    @Nullable Byte getPriority(Method method, Class<?> parent);
    @Nullable BiPredicate<EventListener, Event> getCancellationReader(EventListener listener, Method method);
}

then implementations can provide compatibility with old code by providing customized XXXCharacteristicProvider, for example using Event::isCancelled as the return value of getCancellationReader(...) can bring backward compatibility with old event cancelling.

Note that the API here is quite rough and not performance-wise, there's probably be a better option

[Jonathing]

I'll leave this one up to Lex or Paint. Although I would personally be very wary of adding backwards compatibility directly into EB7.

EventBus 7 was intentionally re-designed to make it simpler to work with on its own. So yes, upgradeability wasn't really a factor when discussing the spec and implementations. I've talked to Lex about how it might be possible to create some sort of wrapper for EB6 that translates its API to EB7 calls.

It's difficult to add arbitrary data to listeners because their characteristics are effectively generated as part of their consumption when registered onto an event bus. See EventListenerFactory#register.

This is one of those situations where I prefer that the project was re-designed and rewritten because since all the internals of EB7 are now actually internal, we don't need to worry about implementation details which have historically plagued EB6 and older. Then again, API and spec modifications are still fair game since EB7 is still a beta.

[LexManos]

To be clear, once I am back from my vacation, evaluating the backwards compatibility work that is feasible with EB7 is on my list.

I can't garentee complete backwards compatibility. But I have ideas on how to maintain the intentionally exposed api compatibility. Obviously with associated compile errors/warnings to nudge people in the correct direction.

No promises, but one of my major goals in these rewrites is backwards compatibility. So don't worry it's in mind. It's just unsure how much we can do.

[ZZZank]

Update: BusCharacteristicProvider seems to be not necessary, because @Cancellable (Cancellable is annotation) -> implement Cancellable (Cancellable is interface) can be done by transforming event classes.

My current hypothesis on handling EB6 -> EB7 compatibility:

• @Cancellable: inject a Cancellable interface to the class
• event.setCancelled(...): customized ListenerCharacteristicProvider#getCancellationReader
• enum EventPriority: customized ListenerCharacteristicProvider#getPriority
• IGenericEvent: this will probably require a EventListener transform(EventListener listener, ... otherInfos) method

As for other APIs, I dont see anyone (any modders) using them, so it should be safe to simply remove.

[PaintNinja]

EventBus 7 is a full re-write and redesign that aims to solve the root design flaws of EventBus 6 that led to problems such as needing a ModLauncher environment and runtime transforms for performance, event subclasses being accidentally cancellable and people registering event listeners to the wrong bus and wondering why their listener wasn't called, among many other things. Attempting to restore backwards-compatibility risks reintroducing those issues. EB6 will continue to be maintained separately from EB7.

Backwards-compatibility with EB6 is not a goal at all, I'm not sure why Lex said that as we already agreed it's intentionally a major breaking change deserving of a major version bump. Sure I remember him suggesting some helper methods in Forge, but not built-into EventBus itself as they would rely on hacks that are likely to break in the next couple of Java releases which contradicts one of EB7's design goals of being built with modern and future Java in mind, unlike Forge where you're expected to use the same Java version as MC and breaking changes are more expected when MC updates. Presumably I'm misunderstanding what he's intending?

---

I'll provide some pointers to help you migrate and explain why things have changed:

event priority is determined by byte instead of enum EventPriority

Use net.minecraftforge.eventbus.api.listener.Priority for common priority values. This mirrors the old EventPriority enum. The introduction of byte is to offer more fine-grained priorities in-between the common ones, instead of being restricted to only 6 in total.

Rationale
As Jonathing already mentioned, EB7 introduces clear separation of API and internals, allowing for drastic changes to the internal implementation without always having to break the API again. This involved moving the package structure, so the EventPriority enum wouldn't work out of the box anyway without changing your code to import it from the new package. With that in mind, I also renamed it to just "Priority" because it dictates the ordering of listeners, not events, and the package it's in already makes it clear it's for listeners (hence no "ListenerPriority").

enum EventPriority: customized ListenerCharacteristicProvider#getPriority
ListenerCharacteristicProvider

When adding a listener, you're given a net.minecraftforge.eventbus.api.listener.EventListener object (or for bulk registration, a collection of them). You can call the priority() method on there to get that listener's priority.

BusCharacteristicProvider

To check if an event implements a given characteristic, simply do instanceof`isAssignableFrom()` checks. For example:

// instance
boolean isCancellable = myEvent instanceof Cancellable;

// class
boolean isCancellable = Cancellable.class.isAssignableFrom(MyEvent.class);

For the buses themselves, CancellableEventBus.create(clazz) throws a compile error if you attempt to give it an event class that isn't Cancellable. You can look at the static final BUS' field type to see if the EventBus is a cancellable one or not, or use your IDE to see if the event implements Cancellable.

whether an event is cancellable is determined by implement Cancellable instead of @CancellableEvent

This isn't hard to migrate, it's just tedious. As you later suggest, you can write compile-time transforms to allow an annotation usage to be turned into an interface usage if you really want, but I don't think it's worth the effort over a find & replace in your IDE.

Rationale
In the old system, checking for the annotation across the inheritance chain involved a decent amount of complexity compared to a single instanceof check. In NoLoader mode, this involves multiple Map lookups. In ModLauncher mode, this involves transforming classes at runtime to skip most of those lookups. By using an interface instead of an annotation, we better conform with standard Java practices and simplify the checks for this.

whether an event is cancelled is determined by predicate.test(...) instead of event.isCancelled()
event.setCancelled(...)

This is a long one, bare with me here...

This was a hard requirement I was given while working on this and I eventually took advantage of it to further improve performance and API design.

Performance
By having listeners cancel using a standard Java Predicate while still keeping Consumer, the EventBus is able to statically determine ahead-of-time if an event might be cancelled by the listener. In the case that all listeners registered to an event never cancel, the cancellation checks are removed entirely.

Likewise, if it's known that a listener always cancels the event, all the lower priority listeners after it aren't built into the array which reduces memory access times. For example, if you have a highest priority listener that always cancels and a hundred listeners after it, the performance is as fast as a single listener because all the ones after are skipped ahead-of-time until that always cancelling listener is removed.

API design
Separating cancellation state from the event object itself allows for cancellable RecordEvents, which makes it possible for many events to be declared in a much more concise manner than before - see here for a minimal example of this. The use of a Predicate over calling a specific method inside of a Consumer is also easier for new users to grasp as it better follows standard Java practices.

Preventing events from implicitly cancelling themselves when a listener calls an unrelated method also helps avoid surprises.

Drawbacks and possible solutions
I'm aware it's quite useful being able to do EventBus.fire().isCancelled() in EB6, as you can pass around a single fired object and also check the cancellation state on it. In EB7 you have to keep the original object handy, then call EventBus.post() to grab the cancellation state.

Wrapping fire() in an object to bundle cancellation state with event state again wouldn't be great as it involves allocating on the heap on the posting hotpath, at least until Valhalla arrives. But you could do that for some of your own events if they aren't performance-sensitive or are rarely fired (especially if they're SelfDestructing).

I could add a new EventCharacteristic called CancellableAware, which works similarly to MonitorAware. This would restore the isCancelled() method with the caveat that the event would need to be a MutableEvent - records and interface events wouldn't be supported, same restriction as MonitorAware. Would this suffice for your needs?

IGenericEvent

Generic events are not supported in EB7. Their performance in EB6 is very poor because it filters the listeners during post time using TypeTools to reflectively check and compare the generic type of each listener with that of the posted event.

It is recommended to use inheritance instead, for example:

sealed interface MyGenericEvent<T extends Entity> {
    T entity();

    record Player(Player entity) implements MyGenericEvent<Player> {
        public static final EventBus<Player> BUS = EventBus.create(Player.class);
    }

    record Ghast(Ghast entity) implements MyGenericEvent<Ghast> {
        public static final EventBus<Ghast> BUS = EventBus.create(Ghast.class);
    }

    // etc...
}

You would then add a listener to the subclass instead of the generic interface. If you need to support the same listener for multiple subclasses, make MyGenericEvent<T> extends InheritableEvent and add an EventBus field to it. Listeners can then either register to the exact subclass they care about, or do a pattern-matching switch on the MyGenericEvent.

---

I hope this helps, if you have further questions feel free to ask. Let me know if the CancellableAware characteristic would be useful for you. :)

[LexManos]

Yes EB7 was meant to be a complete break. Paint is free to do as he wishes. There is one issue with the design I'll have to speak with him about when I get home. And that being separation of mutable parts of an event into a separate object so that we can reuse event objects. But. That's not a hard requirement.

To better explain my intent when it comes to backwards compatibility.

1. ease of consumer migration. This is basically making sure Bus.registrr(object) works and the like. I'd say 95% of our users that's basically all the api they use. (Consuming events)
2. if possible get binary compatibility in..most likely as a side project maybe with some transformers i don't know. But I have some ideas. Ideally we wouldn't have to maintain the full EB6 branch but instead a "compatibility library". But if that doesn't work out EB6 will still be maintained. Heck it will need to for older java targets.

My goal at the end of the day is to support all old versions of forge modding. In the simplest way possible for us on the maintains side. Without having our hands tied by legacy code moving forward. We will see once I start what is feasible.

Also IGenericEvent can die in a fire. All it was was a object.getClass == class. Paint is a little mistaken there. It is not the cause for TypeUitils. The fucking lambda register methods are. But he solved that by making rhe lambda on the event themselves. So they don't have to figure out what event they belong to.