Minecraftforge: DecorateBiomeEvent.Pre/Post are fired on the wrong event bus

[Barteks2x]

Documentation says they are fired on TERRAIN_GEN_BUS, but they are currently fired on EVENT_BUS (and it has been fired on EVENT_BUS at least since 1.6.4, I didn't go farther back in history).

I'm not sure if it should be changed to TERRAIN_GEN_BUS (and ignore all mods that may expect it on EVENT_BUS) or if documentation should be changed.

[AlexiyOrlov]

As a user of Post event, I would vote for changing it to TERRAIN bus.

[Barteks2x]

When I looked at your issue, I knew I have seen it somewhere... I completely forgot it was me reporting it.

I can try to submit a PR to fix it, but I'm not sure which way would be better:

• change documentation (and possibly add a TODO for 1.13 to switch the event bus)
• change the event bus (that would be a breaking change, so I think this won't be accepted)
• Fire it on both, and add a comment that it should be changed in next MC update (this could affect performance, but if this is just one event it shouldn't be an issue)

[jabelar]

Well, one time I asked diesieben07 why there were separate buses and he explained it as follows:

Forge then has two more buses, the ORE_GEN_BUS and the TERRAIN_GEN_BUS. Events for terrain and ore generation are fired on a different bus to not put strain on the main bus. The main bus usually has quite a lot of listeners. If the terrain and ore-gen events (which are fired a lot during terrain gen) would fire on the main bus it would have to check all those "normal" event listeners every time, which could be quite the performance hit. This way only the handlers that actually want the terrain gen events are checked for them.

So I guess I assume that the decorate biome event is called a lot during generation and so for performance reasons mentioned above it should be on the terrain bus. On the other hand, since it has been this way for a while I guess it is tolerable as it is.

My point is that the main justification for putting stuff on the terrain bus was performance, so if that applies to this event we should probably follow that same justification.

[Barteks2x]

I really don't see the performance as a justification. Looking at EventBus.post(Event):

    public boolean post(Event event) {
        IEventListener[] listeners = event.getListenerList().getListeners(busID);
        int index = 0;
        try {
            for (; index < listeners.length; index++) {
                listeners[index].invoke(event);
            }
        }
        catch (Throwable throwable) {
            //irrelevant
        }
        return (event.isCancelable() ? event.isCanceled() : false);
    }

So the list of listeners is per-event anyway. And I can't really find a case of the same event being fired on 2 different buses. So why does it even matter? There is no real filtering done. Just plain field and array access. If anything, just one event bus would make the code more efficient by eliminating one array access.

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. Please feel free to reopen it or create a new issue.

[LexManos]

Just to clarify the performance issue is relating to stupid mods listening to Event base class.
Im thinking in 1.13 we merge it all into one bus, but add a @EventRoot annotation to events to prevent it from using it's parents event listeners.
Would accomplish the same thing in a more.. stable.. way.