Implement auto bus detection in EBS

loader/src/main/java/net/neoforged/fml/common/EventBusSubscriber.java

@@ -12,10 +12,12 @@
 import net.neoforged.api.distmarker.Dist;
 import net.neoforged.bus.api.SubscribeEvent;
 import net.neoforged.fml.ModContainer;
+import net.neoforged.fml.event.IModBusEvent;
 
 // @formatter:off - spotless doesn't like @
 /**
- * Annotate a class which will be subscribed to an Event Bus at mod construction time. Defaults to subscribing the current modid to the {@code NeoForge#EVENT_BUS} on both sides.
+ * Annotate a class which will be subscribed to an Event Bus at mod construction time. Defaults to selecting the event bus
+ * based on the listeners you have in the class ({@link Bus#MOD} if all listeners listen to an event of type {@link IModBusEvent}).
  *
  * <p>Annotated classes will be scanned for <b>static</b> methods that have the {@link SubscribeEvent} annotation.
  * For example:
@@ -60,7 +62,7 @@
      *
      * @return the bus you wish to listen to
      */
-    Bus bus() default Bus.GAME;
+    Bus bus() default Bus.AUTOMATIC;
 
     enum Bus {
         /**
@@ -75,5 +77,13 @@ enum Bus {
          * @see ModContainer#getEventBus()
          */
         MOD,
+        /**
+         * Detect the bus to use automatically based on the listeners in the class.
+         * <p>
+         * If all listeners listen to an event of type {@link IModBusEvent}, the bus will be the {@linkplain #MOD mod bus},
+         * otherwise it will be the {@linkplain #GAME game bus}.
+         * <p><strong>A class must not mix game and mod bus listeners</strong>.
+         */
+        AUTOMATIC
     }
 }

loader/src/main/java/net/neoforged/fml/config/IConfigEvent.java

@@ -8,13 +8,13 @@
 import java.util.function.Function;
 import net.neoforged.bus.api.Event;
 import net.neoforged.bus.api.IEventBus;
-import net.neoforged.fml.Bindings;
+import net.neoforged.fml.loading.FMLLoader;
 import org.jetbrains.annotations.Nullable;
 
 public interface IConfigEvent {
     record ConfigConfig(Function<ModConfig, IConfigEvent> loading, Function<ModConfig, IConfigEvent> reloading, @Nullable Function<ModConfig, IConfigEvent> unloading) {}
 
-    ConfigConfig CONFIGCONFIG = Bindings.getConfigConfiguration();
+    ConfigConfig CONFIGCONFIG = FMLLoader.getBindings().getConfigConfiguration();
 
     static IConfigEvent reloading(ModConfig modConfig) {
         return CONFIGCONFIG.reloading().apply(modConfig);

loader/src/main/java/net/neoforged/fml/javafmlmod/AutomaticEventSubscriber.java

@@ -7,18 +7,24 @@
 
 import static net.neoforged.fml.Logging.LOADING;
 
+import java.lang.reflect.Modifier;
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.stream.Collectors;
 import net.neoforged.api.distmarker.Dist;
+import net.neoforged.bus.api.Event;
 import net.neoforged.bus.api.IEventBus;
-import net.neoforged.fml.Bindings;
+import net.neoforged.bus.api.SubscribeEvent;
 import net.neoforged.fml.ModContainer;
+import net.neoforged.fml.ModLoader;
+import net.neoforged.fml.ModLoadingIssue;
 import net.neoforged.fml.common.EventBusSubscriber;
 import net.neoforged.fml.common.Mod;
+import net.neoforged.fml.event.IModBusEvent;
 import net.neoforged.fml.loading.FMLEnvironment;
+import net.neoforged.fml.loading.FMLLoader;
 import net.neoforged.fml.loading.modscan.ModAnnotation;
 import net.neoforged.neoforgespi.language.ModFileScanData;
 import org.apache.logging.log4j.LogManager;
@@ -42,22 +48,47 @@ public static void inject(final ModContainer mod, final ModFileScanData scanData
         Map<String, String> modids = scanData.getAnnotations().stream().filter(annotationData -> MOD_TYPE.equals(annotationData.annotationType())).collect(Collectors.toMap(a -> a.clazz().getClassName(), a -> (String) a.annotationData().get("value")));
 
         ebsTargets.forEach(ad -> {
-            @SuppressWarnings("unchecked")
             final EnumSet<Dist> sides = getSides(ad.annotationData().get("value"));
             final String modId = (String) ad.annotationData().getOrDefault("modid", modids.getOrDefault(ad.clazz().getClassName(), mod.getModId()));
-            final ModAnnotation.EnumHolder busTargetHolder = (ModAnnotation.EnumHolder) ad.annotationData().getOrDefault("bus", new ModAnnotation.EnumHolder(null, EventBusSubscriber.Bus.GAME.name()));
+            final ModAnnotation.EnumHolder busTargetHolder = (ModAnnotation.EnumHolder) ad.annotationData().getOrDefault("bus", new ModAnnotation.EnumHolder(null, EventBusSubscriber.Bus.AUTOMATIC.name()));
             final EventBusSubscriber.Bus busTarget = EventBusSubscriber.Bus.valueOf(busTargetHolder.value());
             if (Objects.equals(mod.getModId(), modId) && sides.contains(FMLEnvironment.dist)) {
                 try {
+                    var clazz = Class.forName(ad.clazz().getClassName(), true, layer.getClassLoader());
+
                     IEventBus bus = switch (busTarget) {
-                        case GAME -> Bindings.getGameBus();
+                        case GAME -> FMLLoader.getBindings().getGameBus();
                         case MOD -> mod.getEventBus();
+                        case AUTOMATIC -> {
+                            boolean hasGame = false, hasMod = false;
+                            for (var method : clazz.getDeclaredMethods()) {
+                                if (!Modifier.isStatic(method.getModifiers()) || !method.isAnnotationPresent(SubscribeEvent.class) || method.getParameterTypes().length != 1) {
+                                    continue;
+                                }
+
+                                var type = method.getParameterTypes()[0];
+                                if (IModBusEvent.class.isAssignableFrom(type)) {
+                                    hasMod = true;
+                                } else if (Event.class.isAssignableFrom(type)) {
+                                    hasGame = true;
+                                }
+                            }
+
+                            if (hasGame && hasMod) {
+                                ModLoader.addLoadingIssue(ModLoadingIssue.error("fml.modloading.javafml.ebs.mixed_buses", clazz.getName()).withAffectedMod(mod.getModInfo()));
+                                yield null;
+                            }
+
+                            // We'll let the bus do the rest of the checking (instance methods, or inheritance) to catch other errors
+                            // if we haven't found any methods
+                            yield hasMod ? mod.getEventBus() : FMLLoader.getBindings().getGameBus();
+                        }
                     };
 
                     if (bus != null) {
-                        LOGGER.debug(LOADING, "Auto-subscribing {} to {}", ad.clazz().getClassName(), busTarget);
+                        LOGGER.debug(LOADING, "Auto-subscribing {} to the {} bus", ad.clazz().getClassName(), bus == mod.getEventBus() ? "mod" : "game");
 
-                        bus.register(Class.forName(ad.clazz().getClassName(), true, layer.getClassLoader()));
+                        bus.register(clazz);
                     }
                 } catch (ClassNotFoundException e) {
                     LOGGER.fatal(LOADING, "Failed to load mod class {} for @EventBusSubscriber annotation", ad.clazz(), e);

loader/src/main/java/net/neoforged/fml/loading/FMLLoader.java

@@ -19,10 +19,12 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.ServiceLoader;
 import net.neoforged.accesstransformer.api.AccessTransformerEngine;
 import net.neoforged.accesstransformer.ml.AccessTransformerService;
 import net.neoforged.api.distmarker.Dist;
 import net.neoforged.coremod.CoreModScriptingEngine;
+import net.neoforged.fml.IBindingsProvider;
 import net.neoforged.fml.common.asm.RuntimeDistCleaner;
 import net.neoforged.fml.loading.mixin.DeferredMixinConfigRegistration;
 import net.neoforged.fml.loading.moddiscovery.ModDiscoverer;
@@ -33,6 +35,7 @@
 import net.neoforged.neoforgespi.ILaunchContext;
 import net.neoforged.neoforgespi.locating.IModFileCandidateLocator;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.VisibleForTesting;
 import org.slf4j.Logger;
 
 public class FMLLoader {
@@ -53,6 +56,8 @@ public class FMLLoader {
     private static boolean production;
     @Nullable
     private static ModuleLayer gameLayer;
+    @VisibleForTesting
+    static IBindingsProvider bindings;
 
     static void onInitialLoad(IEnvironment environment) throws IncompatibleEnvironmentException {
         final String version = LauncherVersion.getVersion();
@@ -93,6 +98,8 @@ static void onInitialLoad(IEnvironment environment) throws IncompatibleEnvironme
         coreModEngine = new CoreModScriptingEngine();
         LOGGER.debug(LogMarkers.CORE, "FML found CoreMods version : {}", coreModEngine.getClass().getPackage().getImplementationVersion());
 
+        bindings = null;
+
         try {
             Class.forName("com.electronwill.nightconfig.core.Config", false, environment.getClass().getClassLoader());
             Class.forName("com.electronwill.nightconfig.toml.TomlFormat", false, environment.getClass().getClassLoader());
@@ -209,4 +216,16 @@ public static ModuleLayer getGameLayer() {
     public static VersionInfo versionInfo() {
         return versionInfo;
     }
+
+    public static IBindingsProvider getBindings() {
+        if (bindings == null) {
+            var providers = ServiceLoader.load(getGameLayer(), IBindingsProvider.class)
+                    .stream().toList();
+            if (providers.size() != 1) {
+                throw new IllegalStateException("Could not find bindings provider");
+            }
+            bindings = providers.get(0).get();
+        }
+        return bindings;
+    }
 }

loader/src/main/resources/lang/en_us.json

@@ -12,6 +12,7 @@
   "fml.modloading.failedtoloadforge": "Failed to load NeoForge",
 
   "fml.modloading.javafml.dangling_entrypoint": "File {5} contains mod entrypoint class \u00a76{4}\u00a7r for mod with id \u00a7e{3}\u00a7r, which does not exist or is not in the same file.\nDid you forget to update the mod id in the entrypoint?",
+  "fml.modloading.javafml.ebs.mixed_buses": "Class {3} annotated with @EventBusSubscriber has listeners for both mod and game bus events, which cannot be mixed",
 
   "fml.modloading.missingdependency": "Mod \u00a7e{4}\u00a7r requires \u00a76{3}\u00a7r \u00a7o{5,vr}\u00a7r\n\u00a77Currently, \u00a76{3}\u00a7r\u00a77 is \u00a7o{6,i18n,fml.messages.artifactversion.ornotinstalled}§r\n{7,optional,§7Reason for the dependency: §r}",
   "fml.modloading.missingdependency.optional": "Mod \u00a7e{4}\u00a7r only supports \u00a73{3}\u00a7r \u00a7o{5,vr}\u00a7r\n\u00a77Currently, \u00a73{3}\u00a7r\u00a77 is \u00a7o{6}",

loader/src/test/java/net/neoforged/fml/javafmlmod/FMLJavaModLanguageProviderTest.java

@@ -5,6 +5,7 @@
 
 package net.neoforged.fml.javafmlmod;
 
+import static org.assertj.core.api.Assertions.as;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.util.ArrayList;
@@ -170,4 +171,74 @@ public EntryPoint(net.neoforged.bus.api.IEventBus modEventBus) {
         assertThat(getTranslatedIssues(e.getIssues())).containsOnly("ERROR: testmod (testmod) encountered an error while dispatching the net.neoforged.fml.event.lifecycle.FMLClientSetupEvent event"
                 + "\njava.lang.RuntimeException: null");
     }
+
+    @Test
+    void testEventBusSubscriberAutomaticDetection() throws Exception {
+        installation.setupProductionClient();
+        installation.buildModJar("test.jar")
+                .withModsToml(builder -> {
+                    builder.unlicensedJavaMod().addMod("testmod", "1.0");
+                })
+                .addClass("testmod.TestEvent", """
+                        public class TestEvent extends net.neoforged.bus.api.Event {
+                        }""")
+                .addClass("testmod.EBSListener", """
+                        @net.neoforged.fml.common.EventBusSubscriber(bus = net.neoforged.fml.common.EventBusSubscriber.Bus.AUTOMATIC)
+                        public class EBSListener {
+                            @net.neoforged.bus.api.SubscribeEvent
+                            public static void onClientSetup(net.neoforged.fml.event.lifecycle.FMLClientSetupEvent event) {
+                                net.neoforged.fml.javafmlmod.FMLJavaModLanguageProviderTest.EVENTS.add(event);
+                                net.neoforged.fml.loading.FMLLoader.getBindings().getGameBus().post(new TestEvent());
+                            }
+                        }
+                        """)
+                .addClass("testmod.EBSListenerGame", """
+                        @net.neoforged.fml.common.EventBusSubscriber(bus = net.neoforged.fml.common.EventBusSubscriber.Bus.AUTOMATIC)
+                        public class EBSListenerGame {
+                            @net.neoforged.bus.api.SubscribeEvent
+                            public static void onTest(TestEvent event) {
+                                net.neoforged.fml.javafmlmod.FMLJavaModLanguageProviderTest.MESSAGES.add("game listener invoked");
+                            }
+                        }
+                        """)
+                .build();
+
+        var result = launch("forgeclient");
+        loadMods(result);
+
+        ModLoader.dispatchParallelEvent("test", Runnable::run, Runnable::run, () -> {}, FMLClientSetupEvent::new);
+
+        assertThat(EVENTS).hasSize(1);
+        assertThat(MESSAGES).containsExactly("game listener invoked");
+    }
+
+    @Test
+    void testEventBusSubscriberMixedBuses() throws Exception {
+        installation.setupProductionClient();
+        installation.buildModJar("test.jar")
+                .withModsToml(builder -> {
+                    builder.unlicensedJavaMod().addMod("testmod", "1.0");
+                })
+                .addClass("testmod.TestEvent", """
+                        public class TestEvent extends net.neoforged.bus.api.Event {
+                        }""")
+                .addClass("testmod.EBSListener", """
+                        @net.neoforged.fml.common.EventBusSubscriber(bus = net.neoforged.fml.common.EventBusSubscriber.Bus.AUTOMATIC)
+                        public class EBSListener {
+                            @net.neoforged.bus.api.SubscribeEvent
+                            public static void onClientSetup(net.neoforged.fml.event.lifecycle.FMLClientSetupEvent event) {
+                            }
+
+                            @net.neoforged.bus.api.SubscribeEvent
+                            public static void onGame(TestEvent event) {
+                            }
+                        }
+                        """)
+                .build();
+
+        var result = launch("forgeclient");
+        loadMods(result);
+
+        assertThat(getTranslatedIssues(ModLoader.getLoadingIssues())).containsExactly("ERROR: Class testmod.EBSListener annotated with @EventBusSubscriber has listeners for both mod and game bus events, which cannot be mixed");
+    }
 }

loader/src/test/java/net/neoforged/fml/loading/LauncherTest.java

@@ -28,8 +28,11 @@
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
+import net.neoforged.bus.api.IEventBus;
+import net.neoforged.fml.IBindingsProvider;
 import net.neoforged.fml.ModLoader;
 import net.neoforged.fml.ModLoadingIssue;
+import net.neoforged.fml.config.IConfigEvent;
 import net.neoforged.fml.i18n.FMLTranslations;
 import net.neoforged.neoforgespi.Environment;
 import org.junit.jupiter.api.AfterEach;
@@ -96,6 +99,17 @@ protected LaunchResult launch(String launchTarget) throws Exception {
 
         FMLPaths.loadAbsolutePaths(installation.getGameDir());
 
+        FMLLoader.bindings = new IBindingsProvider() {
+            @Override
+            public IEventBus getGameBus() {
+                return installation.getGameBus();
+            }
+
+            @Override
+            public IConfigEvent.ConfigConfig getConfigConfiguration() {
+                return null;
+            }
+        };
         FMLLoader.onInitialLoad(environment);
         FMLPaths.setup(environment);
         FMLConfig.load();

loader/src/test/java/net/neoforged/fml/loading/SimulatedInstallation.java

@@ -8,6 +8,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
+import com.google.common.base.Suppliers;
 import com.google.common.io.MoreFiles;
 import com.google.common.io.RecursiveDeleteOption;
 import cpw.mods.jarhandling.SecureJar;
@@ -25,13 +26,16 @@
 import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.jar.Attributes;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.jar.JarOutputStream;
 import java.util.jar.Manifest;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import net.neoforged.bus.api.BusBuilder;
+import net.neoforged.bus.api.IEventBus;
 import net.neoforged.jarjar.metadata.ContainedJarMetadata;
 import net.neoforged.jarjar.metadata.Metadata;
 import net.neoforged.jarjar.metadata.MetadataIOHandler;
@@ -81,6 +85,8 @@ public class SimulatedInstallation implements AutoCloseable {
     private final Path librariesDir;
     // Used for testing running out of a Gradle project. Is the simulated Gradle project root directory.
     private final Path projectRoot;
+    // Simulates NeoForge's game event bus
+    private final Supplier<IEventBus> gameBus = Suppliers.memoize(() -> BusBuilder.builder().build());
 
     private static final IdentifiableContent[] SERVER_EXTRA_JAR_CONTENT = { SHARED_ASSETS };
     private static final IdentifiableContent[] CLIENT_EXTRA_JAR_CONTENT = { CLIENT_ASSETS, SHARED_ASSETS };
@@ -96,6 +102,10 @@ public SimulatedInstallation() throws IOException {
         projectRoot = Files.createTempDirectory("projectRoot");
     }
 
+    public IEventBus getGameBus() {
+        return gameBus.get();
+    }
+
     public Path getModsFolder() throws IOException {
         var modsFolder = gameDir.resolve("mods");
         Files.createDirectories(modsFolder);