diff --git a/fmlearlydisplay/src/main/java/net/minecraftforge/fml/earlydisplay/DisplayWindow.java b/fmlearlydisplay/src/main/java/net/minecraftforge/fml/earlydisplay/DisplayWindow.java index 81a2673d87..c570fd1357 100644 --- a/fmlearlydisplay/src/main/java/net/minecraftforge/fml/earlydisplay/DisplayWindow.java +++ b/fmlearlydisplay/src/main/java/net/minecraftforge/fml/earlydisplay/DisplayWindow.java @@ -604,10 +604,25 @@ public class DisplayWindow implements ImmediateWindowProvider { @Override public void updateModuleReads(final ModuleLayer layer) { - var fm = layer.findModule("forge").orElseThrow(); - getClass().getModule().addReads(fm); - var clz = FMLLoader.getGameLayer().findModule("forge").map(l->Class.forName(l, "net.minecraftforge.client.loading.ForgeLoadingOverlay")).orElseThrow(); - loadingOverlay = Arrays.stream(clz.getDeclaredMethods()).filter(m-> Modifier.isStatic(m.getModifiers()) && m.getName().equals("newInstance")).findFirst().orElseThrow(); + final var FORGE_MODULE = "net.minecraftforge.forge"; + var forge_module = layer.findModule(FORGE_MODULE).orElse(null); + if (forge_module == null) + throw new IllegalStateException("Could not find " + FORGE_MODULE + " in " + layer); + + getClass().getModule().addReads(forge_module); + + + var clz = Class.forName(forge_module, "net.minecraftforge.client.loading.ForgeLoadingOverlay"); + + for (var mtd : clz.getDeclaredMethods()) { + if (Modifier.isStatic(mtd.getModifiers()) && "newInstance".equals(mtd.getName())) { + this.loadingOverlay = mtd; + break; + } + } + + if (loadingOverlay == null) + throw new IllegalStateException("Could not find static newInstace method in " + clz.getName()); } public int getFramebufferTextureId() { diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/ImmediateWindowHandler.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/ImmediateWindowHandler.java index 267b27d004..15cf3a3617 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/ImmediateWindowHandler.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/ImmediateWindowHandler.java @@ -140,7 +140,7 @@ public class ImmediateWindowHandler { @Override public void updateModuleReads(final ModuleLayer layer) { - var fm = layer.findModule("forge"); + var fm = layer.findModule("net.minecraftforge.forge"); if (fm.isPresent()) { getClass().getModule().addReads(fm.get()); var clz = fm.map(l -> Class.forName(l, "net.minecraftforge.client.loading.NoVizFallback")).orElseThrow(); diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/UniqueModListBuilder.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/UniqueModListBuilder.java index 467cc6dc56..b2486c1bee 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/UniqueModListBuilder.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/UniqueModListBuilder.java @@ -135,7 +135,7 @@ public class UniqueModListBuilder return modFile.getSecureJar().name(); } - return modFile.getModFileInfo().moduleName(); + return modFile.getModFileInfo().getMods().get(0).getModId(); } public record UniqueModListData(List modFiles, Map> modFilesByFirstId) {} diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/AbstractModProvider.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/AbstractModProvider.java index c768c5ce12..fd63fdfca1 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/AbstractModProvider.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/AbstractModProvider.java @@ -6,6 +6,7 @@ package net.minecraftforge.fml.loading.moddiscovery; import com.mojang.logging.LogUtils; + import cpw.mods.jarhandling.JarMetadata; import cpw.mods.jarhandling.SecureJar; import net.minecraftforge.fml.loading.LogMarkers; @@ -22,19 +23,25 @@ import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import java.io.IOException; +import java.lang.module.InvalidModuleDescriptorException; +import java.lang.module.ModuleDescriptor; +import java.lang.module.ModuleDescriptor.Version; import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.Consumer; import java.util.jar.JarFile; +import java.util.stream.Collectors; @ApiStatus.Internal public abstract class AbstractModProvider implements IModProvider { - private static final Logger LOGGER = LogUtils.getLogger(); - protected static final String MODS_TOML = "META-INF/mods.toml"; + private static final Logger LOGGER = LogUtils.getLogger(); + protected static final String MODS_TOML = "META-INF/mods.toml"; + protected static final String MODULE_INFO = "module-info.class"; protected IModLocator.ModFileOrException createMod(Path path) { return createMod(path, false); @@ -48,10 +55,12 @@ public abstract class AbstractModProvider implements IModProvider { @Nullable protected IModLocator.ModFileOrException createMod(Path path, boolean ignoreUnknown, String defaultType) { var mjm = new ModJarMetadata(); - var sj = SecureJar.from( - jar -> jar.moduleDataProvider().findFile(MODS_TOML).isPresent() ? mjm : JarMetadata.from(jar, path), - path - ); + SecureJar sj = null; + try { + sj = SecureJar.from(jar -> loadMetaFromJar(jar, mjm), path); + } catch (Throwable t) { + return new IModLocator.ModFileOrException(null, new ModFileLoadingException("Failed to create secure jar for \"" + path + "\" - " + t.getMessage())); + } IModFile mod; var type = sj.moduleDataProvider().getManifest().getMainAttributes().getValue(ModFile.TYPE); @@ -61,6 +70,8 @@ public abstract class AbstractModProvider implements IModProvider { if (sj.moduleDataProvider().findFile(MODS_TOML).isPresent()) { LOGGER.debug(LogMarkers.SCAN, "Found {} mod of type {}: {}", MODS_TOML, type, path); mod = new ModFile(sj, this, ModFileParser::modsTomlParser); + // ModJarMetadata is only used when loading mods.toml mods as ModJarMetadata + mjm.setModFile(mod); if (mod.getModFileInfo().getFileProperties().containsKey(ModFileInfo.NOT_A_FORGE_MOD_PROP)) { LOGGER.error(LogMarkers.SCAN, "Unable to load file \"{}\" because its mods.toml is requesting an invalid javafml loaderVersion (use \"*\" if you want to allow all versions) and is missing a forge modId dependency declaration (see the sample mods.toml in the MDK).", path); return new IModLocator.ModFileOrException(null, new ModFileLoadingException("File \"%s\" is not a Forge mod and cannot be loaded. Look for a Forge version of this mod or consider alternative mods.".formatted(mod.getFileName()))); @@ -73,10 +84,54 @@ public abstract class AbstractModProvider implements IModProvider { } else return new IModLocator.ModFileOrException(null, new ModFileLoadingException("Invalid mod file found " + path)); - mjm.setModFile(mod); return new IModLocator.ModFileOrException(mod, null); } + private static JarMetadata loadMetaFromJar(SecureJar jar, ModJarMetadata mjm) { + var info = jar.moduleDataProvider().open(MODULE_INFO).orElse(null); + if (info != null) { + try { + var desc = ModuleDescriptor.read(info, jar::getPackages); + var all = new HashSet<>(jar.getPackages()); + all.removeAll(desc.packages()); + if (!all.isEmpty()) { + var missing = all.stream().sorted().collect(Collectors.joining(", ")); + LOGGER.error("Invalid module-info, missing packages " + missing); + throw new ModFileLoadingException("Invalid module-info, missing packages " + missing); + } else { + return new JarMetadata() { + @Override + public String name() { + return desc.name(); + } + + @Override + public String version() { + return desc.version().map(Version::toString).or(desc::rawVersion).orElse(null); + } + + @Override + public ModuleDescriptor descriptor() { + return desc; + } + }; + } + } catch (InvalidModuleDescriptorException | IOException e) { + LOGGER.error("Failed to parse " + jar.getPrimaryPath() + " module-info", e); + throw new ModFileLoadingException("Invalid module-info: " + e.getMessage()); + } finally { + try { + info.close(); + } catch (IOException e) {} + } + } + + if (jar.moduleDataProvider().findFile(MODS_TOML).isEmpty()) + return JarMetadata.from(jar, jar.getPrimaryPath()); + + return mjm; + } + protected IModFileInfo manifestParser(final IModFile mod) { var mf = mod.getSecureJar().moduleDataProvider().getManifest().getMainAttributes(); var license = mf.getValue("LICENSE"); diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/MinecraftLocator.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/MinecraftLocator.java index 42b725bb36..430b3f4849 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/MinecraftLocator.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/MinecraftLocator.java @@ -22,13 +22,11 @@ public final class MinecraftLocator extends AbstractModProvider implements IModL @Override public List scanMods() { var minecraft = FMLLoader.getLaunchHandler().getMinecraftPaths(); + var paths = minecraft.toArray(Path[]::new); // Minecraft itself. var meta = new ModJarMetadata(); - var mcjar = SecureJar.from( - jar -> meta, - minecraft.toArray(Path[]::new) - ); + var mcjar = SecureJar.from(jar -> meta, paths); var mc = ModFileFactory.FACTORY.build(mcjar, this, MinecraftLocator::buildMinecraftTOML); meta.setModFile(mc); diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModFileInfo.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModFileInfo.java index 4316c5f40b..520032925d 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModFileInfo.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModFileInfo.java @@ -259,7 +259,7 @@ public class ModFileInfo implements IModFileInfo, IConfigurable { @Override public String moduleName() { - return getMods().get(0).getModId(); + return this.modFile.getSecureJar().name(); } @Override diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModJarMetadata.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModJarMetadata.java index 5fb58fb69b..85a2e448ed 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModJarMetadata.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModJarMetadata.java @@ -9,49 +9,64 @@ import cpw.mods.jarhandling.JarMetadata; import net.minecraftforge.forgespi.locating.IModFile; import java.lang.module.ModuleDescriptor; import java.util.Objects; - import org.jetbrains.annotations.ApiStatus; @ApiStatus.Internal public final class ModJarMetadata implements JarMetadata { + private static final String AUTOMATIC_MODULE_NAME = "Automatic-Module-Name"; private IModFile modFile; + private String name; + private String version; private ModuleDescriptor descriptor; - ModJarMetadata() { - } + ModJarMetadata() { } public void setModFile(IModFile file) { this.modFile = file; + var mods = this.modFile.getModFileInfo().getMods(); + + if (!mods.isEmpty()) { + var main = mods.get(0); + this.name = main.getModId(); + this.version = main.getVersion().toString(); + } + + var jar = file.getSecureJar(); + var aname = jar.moduleDataProvider().getManifest().getMainAttributes().getValue(AUTOMATIC_MODULE_NAME); + if (aname != null) + this.name = aname; } @Override public String name() { - return modFile.getModFileInfo().moduleName(); + return this.name; } @Override public String version() { - return modFile.getModFileInfo().versionString(); + return this.version; } @Override public ModuleDescriptor descriptor() { - if (descriptor != null) return descriptor; + if (descriptor != null) + return descriptor; + var bld = ModuleDescriptor.newAutomaticModule(name()) - .version(version()) - .packages(modFile.getSecureJar().getPackages()); - modFile.getSecureJar().getProviders().stream() - .filter(p -> !p.providers().isEmpty()) - .forEach(p -> bld.provides(p.serviceName(), p.providers())); - modFile.getModFileInfo().usesServices().forEach(bld::uses); + .version(version()) + .packages(modFile.getSecureJar().getPackages()); + + for (var provider : modFile.getSecureJar().getProviders()) { + if (provider.providers().isEmpty()) + continue; + + bld.provides(provider.serviceName(), provider.providers()); + } + descriptor = bld.build(); return descriptor; } - public IModFile modFile() { - return modFile; - } - @Override public boolean equals(Object obj) { if (obj == this) return true; diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/targets/CommonLaunchHandler.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/targets/CommonLaunchHandler.java index abcfe7c3eb..6ed1a52e7f 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/targets/CommonLaunchHandler.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/targets/CommonLaunchHandler.java @@ -73,7 +73,7 @@ public abstract class CommonLaunchHandler implements ILaunchHandlerService { protected static final LaunchType CLIENT = new LaunchType("client", "minecraft", "net.minecraft.client.main.Main", Dist.CLIENT, false); protected static final LaunchType DATA = new LaunchType("data", "minecraft", "net.minecraft.data.Main", Dist.CLIENT, true); protected static final LaunchType SERVER = new LaunchType("server", "minecraft", "net.minecraft.server.Main", Dist.DEDICATED_SERVER, false); - protected static final LaunchType SERVER_GAMETEST = new LaunchType("server_gametest", "forge", "net.minecraftforge.gametest.GameTestMain", Dist.DEDICATED_SERVER, false); + protected static final LaunchType SERVER_GAMETEST = new LaunchType("server_gametest", "net.minecraftforge.forge", "net.minecraftforge.gametest.GameTestMain", Dist.DEDICATED_SERVER, false); protected void runTarget(String module, String target, final String[] arguments, final ModuleLayer layer) throws Throwable { var mod = layer.findModule(module).orElse(null); diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/targets/ForgeDevLocator.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/targets/ForgeDevLocator.java index 19edc71adc..290cacb1c0 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/targets/ForgeDevLocator.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/targets/ForgeDevLocator.java @@ -91,23 +91,24 @@ public final class ForgeDevLocator extends AbstractModProvider implements IModLo // First lets find all class files, that have the @Mod annotation and map packages to modids. var packages = findTestModPackages(path); for (var entry : packages.entrySet()) { - var pkg = entry.getKey(); + var pkg = entry.getKey() + '/'; var modids = entry.getValue(); // Find resource directories for every mod in this group - var resourcePaths = new LinkedHashSet(); + var paths = new LinkedHashSet(); for (var modid : modids) { var rsc = path.resolve(modid); if (Files.exists(rsc)) - resourcePaths.add(rsc); + paths.add(rsc); } var root = memory.getPath(modids.iterator().next()); // use the first modid as our root - buildModsToml(resourcePaths, modids, root); - buildPackMeta(resourcePaths, root); + buildModsToml(paths, modids, root); + buildPackMeta(paths, root); + moveModuleInfo(paths, root); if (Files.exists(root)) - resourcePaths.add(root); + paths.add(root); // We want just the class files from the root of the input paths. So make a new union with a filter. var classes = UnionHelper.newFileSystem( @@ -121,14 +122,10 @@ public final class ForgeDevLocator extends AbstractModProvider implements IModLo }, new Path[] { path } ); - // And we want the resources, and none of the classes, just in case things get kinda fucky and somehow a package overlaps a sub package {shading?} - var resources = UnionHelper.newFileSystem((name, base) -> !name.endsWith(".class"), resourcePaths.toArray(Path[]::new)); + paths.addFirst(classes.getRootDirectories().iterator().next()); // Union of unions, Yay! - var union = UnionHelper.newFileSystem(null, new Path[] { - classes.getRootDirectories().iterator().next(), - resources.getRootDirectories().iterator().next() - }); + var union = UnionHelper.newFileSystem(null, paths.stream().toArray(Path[]::new)); mod.add(union.getRootDirectories().iterator().next()); } @@ -274,6 +271,25 @@ public final class ForgeDevLocator extends AbstractModProvider implements IModLo } } + private static void moveModuleInfo(Set paths, Path root) { + var existing = paths.stream() + .map(p -> p.resolve("module-info.dat")) + .filter(Files::exists) + .findFirst() + .orElse(null); + + if (existing == null) + return; + + var target = root.resolve("module-info.class"); + try { + Files.createDirectories(target.getParent()); + Files.copy(existing, target); + } catch (IOException e) { + throw new IllegalStateException("Failed to copy module-info.dat to memory: " + target, e); + } + } + @SuppressWarnings("unchecked") private static R sneak(Throwable e) throws E { throw (E)e; diff --git a/javafmllanguage/build.gradle b/javafmllanguage/build.gradle index eaa4f3e9b2..da904da2e0 100644 --- a/javafmllanguage/build.gradle +++ b/javafmllanguage/build.gradle @@ -13,6 +13,8 @@ dependencies { compileOnly(libs.nulls) implementation(project(':fmlloader')) implementation(project(':fmlcore')) + implementation(libs.unsafe) + implementation(libs.securemodules) } java { @@ -57,7 +59,7 @@ publishing { license PomUtils.Licenses.LGPLv2_1 } } - + repositories { maven gradleutils.publishingForgeMaven } diff --git a/javafmllanguage/src/main/java/net/minecraftforge/fml/javafmlmod/FMLModContainer.java b/javafmllanguage/src/main/java/net/minecraftforge/fml/javafmlmod/FMLModContainer.java index 4deaaaf4cd..a83bdb531b 100644 --- a/javafmllanguage/src/main/java/net/minecraftforge/fml/javafmlmod/FMLModContainer.java +++ b/javafmllanguage/src/main/java/net/minecraftforge/fml/javafmlmod/FMLModContainer.java @@ -16,15 +16,21 @@ import net.minecraftforge.fml.ModLoadingStage; import net.minecraftforge.fml.event.IModBusEvent; import net.minecraftforge.forgespi.language.IModInfo; import net.minecraftforge.forgespi.language.ModFileScanData; +import net.minecraftforge.unsafe.UnsafeHacks; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.Marker; import org.apache.logging.log4j.MarkerManager; +import cpw.mods.jarhandling.SecureJar; + import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.Objects; import java.util.Optional; +import java.util.jar.Attributes; public class FMLModContainer extends ModContainer { private static final Logger LOGGER = LogManager.getLogger(); @@ -47,14 +53,82 @@ public class FMLModContainer extends ModContainer { var moduleName = info.getOwningFile().moduleName(); var module = gameLayer.findModule(moduleName) .orElseThrow(() -> new IllegalStateException("Failed to find " + moduleName + " in " + gameLayer)); + + openModules(gameLayer, module, info.getOwningFile().getFile().getSecureJar()); + modClass = Class.forName(module, className); - LOGGER.trace(LOADING,"Loaded modclass {} with {}", modClass.getName(), modClass.getClassLoader()); + LOGGER.debug(LOADING,"Loaded modclass {}/{} with {}", modClass.getModule().getName(), modClass.getName(), modClass.getClassLoader()); } catch (Throwable e) { LOGGER.error(LOADING, "Failed to load class {}", className, e); throw new ModLoadingException(info, ModLoadingStage.CONSTRUCT, "fml.modloading.failedtoloadmodclass", e); } } + /** + *
+     * Reads the Add-Exports and Add-Opens attributes (see note) from a mod file and
+     * attempts to apply them. This differers from the JEP in two significant ways:
+     *   1) Instead of opening things to ALL-UNNAMED it instead only opens to the
+     *      mod's module itself. Because I don't want other mods accidently relying
+     *      on transitive behavior
+     *   2) It is read from all mods not just the executable jars.
+     *
+     * From JEP 261: Module System
+     *
+     * Two new JDK-specific JAR-file manifest attributes are defined to correspond
+     * to the --add-exports and --add-opens command-line options:
+     *     Add-Exports: <module>/<package>( <module>/<package>)*
+     *     Add-Opens: <module>/<package>( <module>/<package>)*
+     *
+     * The value of each attribute is a space-separated list of slash-separated
+     * module-name/package-name pairs. A >module</>package< pair in
+     * the value of an Add-Exports attribute has the same meaning as the
+     * command-line option --add-exports >module</>package<=ALL-UNNAMED.
+     *
+     * A >module</>package< pair in the value of an Add-Opens attribute has the
+     * same meaning as the command-line option --add-opens >module</>package<=ALL-UNNAMED.
+     *
+     * Each attribute can occur at most once, in the main section of a MANIFEST.MF file.
+     * A particular pair can be listed more than once. If a specified module was not
+     * resolved, or if a specified package does not exist, then the corresponding pair
+     * is ignored.
+     */
+    private static void openModules(ModuleLayer layer, Module self, SecureJar jar) throws NoSuchMethodException, SecurityException, IllegalAccessException, InvocationTargetException {
+        var manifest = jar.moduleDataProvider().getManifest().getMainAttributes();
+        addOpenOrExports(layer, self, true, manifest);
+        addOpenOrExports(layer, self, false, manifest);
+    }
+
+    private static void addOpenOrExports(ModuleLayer layer, Module self, boolean open, Attributes attrs) throws NoSuchMethodException, SecurityException, IllegalAccessException, InvocationTargetException {
+        var key = open ? "Add-Opens" : "Add-Exports";
+        var entry = attrs.getValue(key);
+        if (entry == null)
+            return;
+
+        for (var pair : entry.split(" ")) {
+            var pts = pair.trim().split("/");
+            if (pts.length == 2) {
+                var target = layer.findModule(pts[0]).orElse(null);
+                if (target == null || !target.getDescriptor().packages().contains(pts[1]))
+                    continue;
+                addOpenOrExport(target, pts[1], self, open);
+            } else {
+                LOGGER.warn(LOADING, "Invalid {} entry in {}: {}", key, self.getName(), pair);
+            }
+        }
+    }
+
+    private static Method implAddExportsOrOpens;
+    private static void addOpenOrExport(Module target, String pkg, Module reader, boolean open) throws NoSuchMethodException, SecurityException, IllegalAccessException, InvocationTargetException {
+        if (implAddExportsOrOpens == null) {
+            implAddExportsOrOpens = Module.class.getDeclaredMethod("implAddExportsOrOpens", String.class, Module.class, boolean.class, boolean.class);
+            UnsafeHacks.setAccessible(implAddExportsOrOpens);
+        }
+
+        LOGGER.info(LOADING, "{} {}/{} to {}", open ? "Opening" : "Exporting", target.getName(), pkg, reader.getName());
+        implAddExportsOrOpens.invoke(target, pkg, reader, open, /*syncVM*/true);
+    }
+
     private static void onEventFailed(IEventBus iEventBus, Event event, IEventListener[] iEventListeners, int i, Throwable throwable) {
         LOGGER.error(new EventBusErrorMessage(event, i, iEventListeners, throwable));
     }
diff --git a/src/generated_test/resources/criterion_test/data/criterion_test/advancements/break_glass_with_fish.json b/src/generated_test/resources/criterion_test/data/criterion_test/advancements/break_glass_with_fish.json
deleted file mode 100644
index f06d1537c7..0000000000
--- a/src/generated_test/resources/criterion_test/data/criterion_test/advancements/break_glass_with_fish.json
+++ /dev/null
@@ -1,37 +0,0 @@
-{
-  "parent": "minecraft:story/root",
-  "criteria": {
-    "break_glass_with_fish": {
-      "conditions": {
-        "breakingBlock": {
-          "tag": "forge:glass"
-        },
-        "holdingItem": {
-          "tag": "criterion_test:fish"
-        }
-      },
-      "trigger": "criterion_test:criterion"
-    }
-  },
-  "display": {
-    "announce_to_chat": true,
-    "description": {
-      "text": "Fish wins!"
-    },
-    "frame": "task",
-    "hidden": false,
-    "icon": {
-      "item": "minecraft:cod"
-    },
-    "show_toast": true,
-    "title": {
-      "text": "Fish vs Glass"
-    }
-  },
-  "requirements": [
-    [
-      "break_glass_with_fish"
-    ]
-  ],
-  "sends_telemetry_event": false
-}
\ No newline at end of file
diff --git a/src/generated_test/resources/criterion_test/data/criterion_test/tags/items/fish.json b/src/generated_test/resources/criterion_test/data/criterion_test/tags/items/fish.json
deleted file mode 100644
index 0177ea1d70..0000000000
--- a/src/generated_test/resources/criterion_test/data/criterion_test/tags/items/fish.json
+++ /dev/null
@@ -1,10 +0,0 @@
-{
-  "values": [
-    "minecraft:cod",
-    "minecraft:salmon",
-    "minecraft:tropical_fish",
-    "minecraft:pufferfish",
-    "minecraft:cooked_cod",
-    "minecraft:cooked_salmon"
-  ]
-}
\ No newline at end of file
diff --git a/src/main/java/net/minecraftforge/gametest/ForgeGameTestHooks.java b/src/main/java/net/minecraftforge/gametest/ForgeGameTestHooks.java
index 84c35327d8..1c088378da 100644
--- a/src/main/java/net/minecraftforge/gametest/ForgeGameTestHooks.java
+++ b/src/main/java/net/minecraftforge/gametest/ForgeGameTestHooks.java
@@ -182,10 +182,10 @@ public class ForgeGameTestHooks {
         var batch = func.batchName();
         classes.add(batch);
 
-        int idx = batch.indexOf('.');
+        int idx = batch.lastIndexOf('.');
         while (idx != -1) {
             batch = batch.substring(0, idx);
-            idx = batch.indexOf('.');
+            idx = batch.lastIndexOf('.');
             classes.add(batch);
         }
     }
diff --git a/src/test/generated/closed_module/module-info.dat b/src/test/generated/closed_module/module-info.dat
new file mode 100644
index 0000000000..50ef464ee2
Binary files /dev/null and b/src/test/generated/closed_module/module-info.dat differ
diff --git a/src/test/generated/closed_module_test/META-INF/MANIFEST.MF b/src/test/generated/closed_module_test/META-INF/MANIFEST.MF
new file mode 100644
index 0000000000..bbe254a20c
--- /dev/null
+++ b/src/test/generated/closed_module_test/META-INF/MANIFEST.MF
@@ -0,0 +1,4 @@
+Manifest-Version: 1.0
+Add-Opens: net.minecraftforge.debug.modules.closed/net.minecraftforge.de
+ bug.modules.closed.internala
+
diff --git a/src/test/generated/closed_module_test/module-info.dat b/src/test/generated/closed_module_test/module-info.dat
new file mode 100644
index 0000000000..f75679cea6
Binary files /dev/null and b/src/test/generated/closed_module_test/module-info.dat differ
diff --git a/src/test/java/net/minecraftforge/debug/client/ObjModelTest.java b/src/test/java/net/minecraftforge/debug/client/ObjModelTest.java
index 8b53e98d6f..be4c7618a4 100644
--- a/src/test/java/net/minecraftforge/debug/client/ObjModelTest.java
+++ b/src/test/java/net/minecraftforge/debug/client/ObjModelTest.java
@@ -2,6 +2,7 @@
  * Copyright (c) Forge Development LLC and contributors
  * SPDX-License-Identifier: LGPL-2.1-only
  */
+
 /*
 package net.minecraftforge.debug.client;
 
diff --git a/src/test/java/net/minecraftforge/debug/gameplay/crafting/TagsTest.java b/src/test/java/net/minecraftforge/debug/gameplay/crafting/TagsTest.java
index 7d4a089052..c6e487ef7c 100644
--- a/src/test/java/net/minecraftforge/debug/gameplay/crafting/TagsTest.java
+++ b/src/test/java/net/minecraftforge/debug/gameplay/crafting/TagsTest.java
@@ -1,3 +1,8 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
 package net.minecraftforge.debug.gameplay.crafting;
 
 import net.minecraft.gametest.framework.GameTest;
diff --git a/src/test/java/net/minecraftforge/debug/modules/automatic/AutomaticModuleMod.java b/src/test/java/net/minecraftforge/debug/modules/automatic/AutomaticModuleMod.java
new file mode 100644
index 0000000000..a996baa498
--- /dev/null
+++ b/src/test/java/net/minecraftforge/debug/modules/automatic/AutomaticModuleMod.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.automatic;
+
+import net.minecraft.gametest.framework.GameTest;
+import net.minecraft.gametest.framework.GameTestHelper;
+import net.minecraftforge.fml.common.Mod;
+import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
+import net.minecraftforge.gametest.GameTestHolder;
+import net.minecraftforge.test.BaseTestMod;
+
+@Mod(AutomaticModuleMod.MODID)
+@GameTestHolder("forge.module.automatic")
+public class AutomaticModuleMod extends BaseTestMod {
+    public static final String MODID = "automatic_module";
+
+    public AutomaticModuleMod(FMLJavaModLoadingContext context) {
+        super(context);
+    }
+
+    @GameTest(template = "forge:empty3x3x3")
+    public static void correct_name(GameTestHelper helper) throws ReflectiveOperationException {
+        var mod = AutomaticModuleMod.class.getModule();
+        if ("net.minecraftforge.debug.modules.automatic".equals(mod.getName()))
+            helper.succeed();
+        else
+            helper.fail("Invalid module name: " + mod.getName());
+    }
+}
diff --git a/src/test/java/net/minecraftforge/debug/modules/closed/ClosedMod.java b/src/test/java/net/minecraftforge/debug/modules/closed/ClosedMod.java
new file mode 100644
index 0000000000..9f7e91c385
--- /dev/null
+++ b/src/test/java/net/minecraftforge/debug/modules/closed/ClosedMod.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.closed;
+
+import java.lang.module.ModuleDescriptor;
+import java.util.Set;
+import net.minecraftforge.data.event.GatherDataEvent;
+import net.minecraftforge.debug.modules.closed.api.PublicUtils;
+import net.minecraftforge.debug.modules.closed.internala.InternalA;
+import net.minecraftforge.debug.modules.closed.internalb.InternalB;
+import net.minecraftforge.eventbus.api.SubscribeEvent;
+import net.minecraftforge.fml.common.Mod;
+import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
+import net.minecraftforge.test.BaseTestMod;
+import net.minecraftforge.test.ModuleProvider;
+
+@Mod(ClosedMod.MODID)
+public class ClosedMod extends BaseTestMod {
+    public static final String MODID = "closed_module";
+
+    public ClosedMod(FMLJavaModLoadingContext context) {
+        super(context);
+    }
+
+    @SubscribeEvent
+    public void runData(GatherDataEvent event) {
+        var out = event.getGenerator().getPackOutput();
+        event.getGenerator().addProvider(true, new ModuleProvider(out, module()));
+    }
+
+    private ModuleDescriptor module() {
+        var self = ClosedMod.class.getPackageName();
+        var api = PublicUtils.class.getPackageName();
+        var internalA = InternalA.class.getPackageName();
+        var internalB = InternalB.class.getPackageName();
+        var forge = Set.of(
+            "net.minecraftforge.javafmlmod",
+            "net.minecraftforge.eventbus",
+            "net.minecraftforge.fmlcore",
+            "net.minecraftforge.forge"
+        );
+        var bldr = ModuleDescriptor.newModule(self)
+            .packages(Set.of(self, api, internalA, internalB))
+            .opens(api)
+            .exports(api)
+            .opens(self, forge);
+        forge.forEach(bldr::requires);
+        return bldr.build();
+    }
+}
diff --git a/src/test/java/net/minecraftforge/debug/modules/closed/api/PublicUtils.java b/src/test/java/net/minecraftforge/debug/modules/closed/api/PublicUtils.java
new file mode 100644
index 0000000000..15b9b6849f
--- /dev/null
+++ b/src/test/java/net/minecraftforge/debug/modules/closed/api/PublicUtils.java
@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.closed.api;
+
+public class PublicUtils {
+    public static void publicMethod() {
+        System.out.println("Public method called");
+    }
+}
diff --git a/src/test/java/net/minecraftforge/debug/modules/closed/internala/InternalA.java b/src/test/java/net/minecraftforge/debug/modules/closed/internala/InternalA.java
new file mode 100644
index 0000000000..bb400f1acd
--- /dev/null
+++ b/src/test/java/net/minecraftforge/debug/modules/closed/internala/InternalA.java
@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.closed.internala;
+
+public class InternalA {
+    public static void internalMethod() {
+        System.out.println("Internal A method called");
+    }
+}
diff --git a/src/test/java/net/minecraftforge/debug/modules/closed/internalb/InternalB.java b/src/test/java/net/minecraftforge/debug/modules/closed/internalb/InternalB.java
new file mode 100644
index 0000000000..88216b364b
--- /dev/null
+++ b/src/test/java/net/minecraftforge/debug/modules/closed/internalb/InternalB.java
@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.closed.internalb;
+
+public class InternalB {
+    public static void internalMethod() {
+        System.out.println("Internal B method called");
+    }
+}
diff --git a/src/test/java/net/minecraftforge/debug/modules/closedtest/ClosedTestsMod.java b/src/test/java/net/minecraftforge/debug/modules/closedtest/ClosedTestsMod.java
new file mode 100644
index 0000000000..2da347ff3c
--- /dev/null
+++ b/src/test/java/net/minecraftforge/debug/modules/closedtest/ClosedTestsMod.java
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug.modules.closedtest;
+
+import java.lang.module.ModuleDescriptor;
+import java.util.jar.Manifest;
+
+import net.minecraftforge.data.event.GatherDataEvent;
+import net.minecraftforge.debug.modules.closed.api.PublicUtils;
+import net.minecraftforge.eventbus.api.SubscribeEvent;
+import net.minecraftforge.fml.common.Mod;
+import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
+import net.minecraftforge.gametest.GameTestHolder;
+import net.minecraftforge.test.BaseTestMod;
+import net.minecraftforge.test.ManifestProvider;
+import net.minecraftforge.test.ModuleProvider;
+import net.minecraft.gametest.framework.GameTest;
+import net.minecraft.gametest.framework.GameTestHelper;
+
+@Mod(ClosedTestsMod.MODID)
+@GameTestHolder("forge.module.closed")
+public class ClosedTestsMod extends BaseTestMod {
+    public static final String MODID = "closed_module_test";
+
+    public ClosedTestsMod(FMLJavaModLoadingContext context) {
+        super(context);
+    }
+
+    @SubscribeEvent
+    public void runData(GatherDataEvent event) {
+        var out = event.getGenerator().getPackOutput();
+        event.getGenerator().addProvider(true, new ModuleProvider(out, module()));
+        event.getGenerator().addProvider(true, new ManifestProvider(out, MODID, manifest()));
+    }
+
+    private ModuleDescriptor module() {
+        return ModuleDescriptor.newOpenModule(getClass().getPackageName())
+            .requires("net.minecraftforge.eventbus")
+            .requires("net.minecraftforge.fmlcore")
+            .requires("net.minecraftforge.forge")
+            .requires("net.minecraftforge.javafmlmod")
+            .requires("net.minecraftforge.debug.modules.closed")
+            .build();
+    }
+
+    private Manifest manifest() {
+        var ret = new Manifest();
+        // Add-Opens is respected by FMLModContainer, it should give us access to closed packages
+        ret.getMainAttributes().putValue("Add-Opens", "net.minecraftforge.debug.modules.closed/net.minecraftforge.debug.modules.closed.internala" );
+        return ret;
+    }
+
+    /*
+     * This attempted to access a class and method that IS exported by ClosedMod
+     * Should succeed with no issues
+     */
+    @GameTest(template = "forge:empty3x3x3")
+    public static void can_reflect_exported(GameTestHelper helper) throws ReflectiveOperationException {
+        var method = PublicUtils.class.getDeclaredMethod("publicMethod");
+        method.invoke(null);
+        helper.succeed();
+    }
+
+    /*
+     * This is opened by us having an Add-Opens entry in ClosedTest's manifest
+     * It is NOT exported/opened by ClosedMod
+     * Should succeed with no exceptions
+     */
+    @GameTest(template = "forge:empty3x3x3")
+    public static void can_reflect_opened_internal(GameTestHelper helper) throws ReflectiveOperationException {
+        try {
+            var cls = Class.forName("net.minecraftforge.debug.modules.closed.internala.InternalA");
+            var method = cls.getDeclaredMethod("internalMethod");
+            method.invoke(null);
+            helper.succeed();
+        } catch (IllegalAccessException e) {
+            helper.fail("Failed to invoke internal method: " + e.getMessage());
+        }
+    }
+
+    /*
+     * This is NOT opened by us, and is NOT exported by ClosedTest
+     * This should error with IllegalAccessException because java is enforcing access control.
+     */
+    @GameTest(template = "forge:empty3x3x3")
+    public static void cant_reflect_internal(GameTestHelper helper) throws ReflectiveOperationException {
+        try {
+            var cls = Class.forName("net.minecraftforge.debug.modules.closed.internalb.InternalB");
+            var method = cls.getDeclaredMethod("internalMethod");
+            method.invoke(null);
+            helper.fail("Invoked internal method without error");
+        } catch (IllegalAccessException e) {
+            helper.succeed();
+        }
+    }
+}
diff --git a/src/test/java/net/minecraftforge/test/ManifestProvider.java b/src/test/java/net/minecraftforge/test/ManifestProvider.java
new file mode 100644
index 0000000000..321a0bbbc1
--- /dev/null
+++ b/src/test/java/net/minecraftforge/test/ManifestProvider.java
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import java.util.jar.JarFile;
+import java.util.jar.Manifest;
+import java.util.jar.Attributes;
+import com.google.common.hash.Hashing;
+import net.minecraft.Util;
+import net.minecraft.data.CachedOutput;
+import net.minecraft.data.DataProvider;
+import net.minecraft.data.PackOutput;
+
+public class ManifestProvider implements DataProvider {
+    private final PackOutput output;
+    private final String name;
+    private final Manifest manifest;
+
+    public ManifestProvider(PackOutput output, String name, Manifest manifest) {
+        this.output = output;
+        this.name = name;
+        this.manifest = manifest;
+    }
+
+    @Override
+    public String getName() {
+        return "ManifestProvider[" + this.name + ']';
+    }
+
+    @Override
+    public CompletableFuture run(CachedOutput cache) {
+        return CompletableFuture.runAsync(() -> {
+            var target = this.output.getOutputFolder().resolve(JarFile.MANIFEST_NAME);
+            try {
+                if (this.manifest.getMainAttributes().getValue(Attributes.Name.MANIFEST_VERSION) == null)
+                    this.manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
+
+                var bos = new ByteArrayOutputStream();
+                this.manifest.write(bos);
+                var data = bos.toByteArray();
+                @SuppressWarnings("deprecation")
+                var hash = Hashing.sha1().hashBytes(data);
+                cache.writeIfNeeded(target, data, hash);
+            } catch (IOException ioexception) {
+                LOGGER.error("Failed to save file to {}", target, ioexception);
+            }
+        }, Util.backgroundExecutor());
+    }
+}
diff --git a/src/test/java/net/minecraftforge/test/ModuleProvider.java b/src/test/java/net/minecraftforge/test/ModuleProvider.java
new file mode 100644
index 0000000000..137f13afc9
--- /dev/null
+++ b/src/test/java/net/minecraftforge/test/ModuleProvider.java
@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.test;
+
+import java.io.IOException;
+import java.lang.module.ModuleDescriptor;
+import java.lang.module.ModuleDescriptor.Exports;
+import java.lang.module.ModuleDescriptor.Opens;
+import java.lang.module.ModuleDescriptor.Provides;
+import java.lang.module.ModuleDescriptor.Requires;
+import java.lang.module.ModuleDescriptor.Version;
+import java.lang.reflect.AccessFlag;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
+
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Opcodes;
+
+import com.google.common.hash.Hashing;
+import net.minecraft.Util;
+import net.minecraft.data.CachedOutput;
+import net.minecraft.data.DataProvider;
+import net.minecraft.data.PackOutput;
+
+public class ModuleProvider implements DataProvider {
+    private final PackOutput output;
+    private final ModuleDescriptor desc;
+
+    public ModuleProvider(PackOutput output, ModuleDescriptor desc) {
+        this.output = output;
+        this.desc = desc;
+    }
+
+    @Override
+    public String getName() {
+        return "ModuleProvider[" + this.desc.name() + ']';
+    }
+
+    @Override
+    public CompletableFuture run(CachedOutput cache) {
+        return CompletableFuture.runAsync(() -> {
+            var target = this.output.getOutputFolder().resolve("module-info.dat");
+            try {
+                var data = writeModuleInfo();
+                @SuppressWarnings("deprecation")
+                var hash = Hashing.sha1().hashBytes(data);
+                cache.writeIfNeeded(target, data, hash);
+            } catch (IOException ioexception) {
+                LOGGER.error("Failed to save file to {}", target, ioexception);
+            }
+        }, Util.backgroundExecutor());
+    }
+
+    private byte[] writeModuleInfo() {
+        var writer = new ClassWriter(0);
+
+        writer.visit(Opcodes.V9, Opcodes.ACC_MODULE, "module-info", null, null, null);
+
+        var module = writer.visitModule(desc.name(), flags(desc.accessFlags()), version(desc.version(), desc.rawVersion()));
+
+        desc.mainClass().ifPresent(module::visitMainClass);
+
+        for (var pkg : sorted(desc.packages(), Function.identity()))
+            module.visitPackage(binary(pkg));
+
+        for (var req : sorted(desc.requires(), Requires::name))
+            module.visitRequire(req.name(), flags(req.accessFlags()), version(req.compiledVersion(), req.rawCompiledVersion()));
+
+        for (var exp : sorted(desc.exports(), Exports::source))
+            module.visitExport(binary(exp.source()), flags(exp.accessFlags()), array(exp.targets()));
+
+        for (var open : sorted(desc.opens(), Opens::source))
+            module.visitOpen(binary(open.source()), flags(open.accessFlags()), array(open.targets()));
+
+        for (var uses : sorted(desc.uses(), Function.identity()))
+            module.visitUse(binary(uses));
+
+        for (var provide : sorted(desc.provides(), Provides::service)) {
+            var providers = new ArrayList();
+            for (var provider : provide.providers())
+                providers.add(binary(provider));
+
+            module.visitProvide(provide.service(), array(providers));
+        }
+
+        module.visitEnd();
+
+        return writer.toByteArray();
+    }
+
+    private static int flags(Set flags) {
+        int access = 0;
+        for (var flag : flags)
+            access |= flag.mask();
+        return access;
+    }
+
+    private static String[] array(Collection lst) {
+        return lst.stream().toArray(String[]::new);
+    }
+
+    private static String binary(String cls) {
+        return cls.replace('.', '/');
+    }
+
+    private static String version(Optional ver, Optional str) {
+        var version = ver.map(Version::toString).orElse(null);
+        if (version == null)
+            return str.orElse(null);
+        return version;
+    }
+
+    private static  List sorted(Collection data, Function toString) {
+        var ret = new ArrayList();
+        ret.addAll(data);
+        Collections.sort(ret, (a, b) -> toString.apply(a).compareTo(toString.apply(b)));
+        return ret;
+    }
+
+}
diff --git a/src/test/resources/automatic_module/META-INF/MANIFEST.MF b/src/test/resources/automatic_module/META-INF/MANIFEST.MF
new file mode 100644
index 0000000000..bee789c70f
--- /dev/null
+++ b/src/test/resources/automatic_module/META-INF/MANIFEST.MF
@@ -0,0 +1 @@
+Automatic-Module-Name: net.minecraftforge.debug.modules.automatic