Implement auto bus detection in EBS #152
Conversation
|
| } | ||
| } | ||
|
|
||
| if (hasGame && hasMod) { |
There was a problem hiding this comment.
Although I understand the idea, but why do we limit this to one bus per listener?
There was a problem hiding this comment.
This is a "limitation" imposed by event bus' class, where all listeners must be of the correct marker interface. And given that game and mod events are fired during different parts of the runtime, I think the limitation makes sense
There was a problem hiding this comment.
Oke perfect, was just curious. Thanks for the explanation. Nice work!
There was a problem hiding this comment.
This approach does not provide meaningful separation (there is no reason that these must be separated at the
classlevel, aside from the current EBS implementation), and unifying these events in the same class can readily be achieved viaIEventBus#addListener.
Despite the explained "limitation", leaving a class level separation here does not fix #140's main expectations.
Moving further to change the EBS implementation would be better than an incomplete solution imo.
There was a problem hiding this comment.
There is no raw variant of addListener that would support that use case, and reflection based consumers on the neo side is not acceptable, both because of performance concerns and because it will make unregistering those listeners impossible. In any case, I've decided to not proceed with this PR further because more magic will increase confusion.
Instead, we should make the bus parameter required and the mod id default to first in the file (instead of all).
| .withModsToml(builder -> { | ||
| builder.unlicensedJavaMod().addMod("testmod", "1.0"); | ||
| }) |
There was a problem hiding this comment.
There's a shorthand for the testmod mods toml .withTestmodModsToml()
| // Simulates NeoForge's game event bus | ||
| private final Supplier<IEventBus> gameBus = Suppliers.memoize(() -> BusBuilder.builder().build()); |
There was a problem hiding this comment.
Hm, you are putting this here because it's usually in the NeoForge jar?
There was a problem hiding this comment.
I don't like this. It doesn't make life easier for beginners because they still need to understand the difference between the two kinds of busses. And it doesn't really make life easier for more experienced developers because typing the kind of bus you want is already trivial. This just add another way of doing the same thing, which is generally a sign of incoherent design.
Closes #140