From 00dcda5a880a53b0c0d959e49f6958218475f957 Mon Sep 17 00:00:00 2001 From: Matyrobbrt Date: Sat, 1 Jun 2024 18:51:44 +0300 Subject: [PATCH] Implement auto bus detection in EBS --- .../main/java/net/neoforged/fml/Bindings.java | 34 --------- .../fml/common/EventBusSubscriber.java | 14 +++- .../neoforged/fml/config/IConfigEvent.java | 4 +- .../javafmlmod/AutomaticEventSubscriber.java | 43 +++++++++-- .../net/neoforged/fml/loading/FMLLoader.java | 19 +++++ loader/src/main/resources/lang/en_us.json | 1 + .../FMLJavaModLanguageProviderTest.java | 71 +++++++++++++++++++ .../neoforged/fml/loading/LauncherTest.java | 14 ++++ .../fml/loading/SimulatedInstallation.java | 10 +++ 9 files changed, 166 insertions(+), 44 deletions(-) delete mode 100644 loader/src/main/java/net/neoforged/fml/Bindings.java diff --git a/loader/src/main/java/net/neoforged/fml/Bindings.java b/loader/src/main/java/net/neoforged/fml/Bindings.java deleted file mode 100644 index 474017947..000000000 --- a/loader/src/main/java/net/neoforged/fml/Bindings.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (c) Forge Development LLC and contributors - * SPDX-License-Identifier: LGPL-2.1-only - */ - -package net.neoforged.fml; - -import java.util.ServiceLoader; -import net.neoforged.bus.api.IEventBus; -import net.neoforged.fml.config.IConfigEvent; -import net.neoforged.fml.loading.FMLLoader; -import org.jetbrains.annotations.ApiStatus; - -@ApiStatus.Internal -public class Bindings { - private static final IBindingsProvider provider; - - static { - var providers = ServiceLoader.load(FMLLoader.getGameLayer(), IBindingsProvider.class) - .stream().toList(); - if (providers.size() != 1) { - throw new IllegalStateException("Could not find bindings provider"); - } - provider = providers.get(0).get(); - } - - public static IEventBus getGameBus() { - return provider.getGameBus(); - } - - public static IConfigEvent.ConfigConfig getConfigConfiguration() { - return provider.getConfigConfiguration(); - } -} diff --git a/loader/src/main/java/net/neoforged/fml/common/EventBusSubscriber.java b/loader/src/main/java/net/neoforged/fml/common/EventBusSubscriber.java index 49499969c..e7f28e7cc 100644 --- a/loader/src/main/java/net/neoforged/fml/common/EventBusSubscriber.java +++ b/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}). * *

Annotated classes will be scanned for static 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. + *

+ * 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}. + *

A class must not mix game and mod bus listeners. + */ + AUTOMATIC } } diff --git a/loader/src/main/java/net/neoforged/fml/config/IConfigEvent.java b/loader/src/main/java/net/neoforged/fml/config/IConfigEvent.java index c5c6ef41f..66b48baab 100644 --- a/loader/src/main/java/net/neoforged/fml/config/IConfigEvent.java +++ b/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 loading, Function reloading, @Nullable Function unloading) {} - ConfigConfig CONFIGCONFIG = Bindings.getConfigConfiguration(); + ConfigConfig CONFIGCONFIG = FMLLoader.getBindings().getConfigConfiguration(); static IConfigEvent reloading(ModConfig modConfig) { return CONFIGCONFIG.reloading().apply(modConfig); diff --git a/loader/src/main/java/net/neoforged/fml/javafmlmod/AutomaticEventSubscriber.java b/loader/src/main/java/net/neoforged/fml/javafmlmod/AutomaticEventSubscriber.java index ee4ed9a83..042c50897 100644 --- a/loader/src/main/java/net/neoforged/fml/javafmlmod/AutomaticEventSubscriber.java +++ b/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 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 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); diff --git a/loader/src/main/java/net/neoforged/fml/loading/FMLLoader.java b/loader/src/main/java/net/neoforged/fml/loading/FMLLoader.java index bff58e9b5..fb45affaf 100644 --- a/loader/src/main/java/net/neoforged/fml/loading/FMLLoader.java +++ b/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; + } } diff --git a/loader/src/main/resources/lang/en_us.json b/loader/src/main/resources/lang/en_us.json index 9e50eaa56..251c4cfde 100644 --- a/loader/src/main/resources/lang/en_us.json +++ b/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}", diff --git a/loader/src/test/java/net/neoforged/fml/javafmlmod/FMLJavaModLanguageProviderTest.java b/loader/src/test/java/net/neoforged/fml/javafmlmod/FMLJavaModLanguageProviderTest.java index a76f96b24..17ce2fddd 100644 --- a/loader/src/test/java/net/neoforged/fml/javafmlmod/FMLJavaModLanguageProviderTest.java +++ b/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"); + } } diff --git a/loader/src/test/java/net/neoforged/fml/loading/LauncherTest.java b/loader/src/test/java/net/neoforged/fml/loading/LauncherTest.java index 0c09120aa..4f260d3a0 100644 --- a/loader/src/test/java/net/neoforged/fml/loading/LauncherTest.java +++ b/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(); diff --git a/loader/src/test/java/net/neoforged/fml/loading/SimulatedInstallation.java b/loader/src/test/java/net/neoforged/fml/loading/SimulatedInstallation.java index 1d3933a2c..2435488a7 100644 --- a/loader/src/test/java/net/neoforged/fml/loading/SimulatedInstallation.java +++ b/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,6 +26,7 @@ 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; @@ -32,6 +34,8 @@ 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 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);