Fix 1.20.4 compatibility (#212)

legacytest/forge/src/main/resources/pack.mcmeta

@@ -0,0 +1,8 @@
+{
+  "pack": {
+    "description": {
+      "text": "My Mod resources"
+    },
+    "pack_format": 18
+  }
+}

src/legacy/java/net/neoforged/moddevgradle/legacyforge/internal/LegacyForgeModDevPlugin.java

@@ -6,11 +6,9 @@
 import net.neoforged.moddevgradle.internal.DataFileCollections;
 import net.neoforged.moddevgradle.internal.ModdingDependencies;
 import net.neoforged.moddevgradle.internal.jarjar.JarJarPlugin;
-import net.neoforged.moddevgradle.internal.LegacyForgeFacade;
 import net.neoforged.moddevgradle.internal.ModDevArtifactsWorkflow;
 import net.neoforged.moddevgradle.internal.ModDevRunWorkflow;
 import net.neoforged.moddevgradle.internal.RepositoriesPlugin;
-import net.neoforged.moddevgradle.internal.WorkflowArtifact;
 import net.neoforged.moddevgradle.internal.utils.ExtensionUtils;
 import net.neoforged.moddevgradle.internal.utils.VersionCapabilitiesInternal;
 import net.neoforged.moddevgradle.legacyforge.dsl.LegacyForgeExtension;
@@ -33,7 +31,6 @@
 import org.slf4j.LoggerFactory;
 
 import java.net.URI;
-import java.util.Map;
 import java.util.stream.Stream;
 
 @ApiStatus.Internal
@@ -125,26 +122,16 @@ public void enable(Project project, LegacyForgeModdingSettings settings, LegacyF
                 throw new InvalidUserCodeException("Specifying a Forge version is mutually exclusive with NeoForge or MCP");
             }
 
-            var artifactPrefix = "forge-" + forgeVersion;
-            // We have to ensure that client resources are named "client-extra" and *do not* contain forge-<version>
-            // otherwise FML might pick up the client resources as the main Minecraft jar.
-            artifactNamingStrategy = (artifact) -> {
-                if (artifact == WorkflowArtifact.CLIENT_RESOURCES) {
-                    return "client-extra-" + forgeVersion + ".jar";
-                } else {
-                    return artifactPrefix + artifact.defaultSuffix + ".jar";
-                }
-            };
-
             versionCapabilities = VersionCapabilitiesInternal.ofForgeVersion(forgeVersion);
+            artifactNamingStrategy = ArtifactNamingStrategy.createNeoForge(versionCapabilities, "forge", forgeVersion);
 
             String groupId = forgeVersion != null ? "net.minecraftforge" : "net.neoforged";
             var neoForge = depFactory.create(groupId + ":forge:" + forgeVersion);
             var neoForgeNotation = groupId + ":forge:" + forgeVersion + ":userdev";
             dependencies = ModdingDependencies.create(neoForge, neoForgeNotation, null, null, versionCapabilities);
         } else if (mcpVersion != null) {
-            artifactNamingStrategy = ArtifactNamingStrategy.createDefault("vanilla-" + mcpVersion);
             versionCapabilities = VersionCapabilitiesInternal.ofMinecraftVersion(mcpVersion);
+            artifactNamingStrategy = ArtifactNamingStrategy.createVanilla(mcpVersion);
 
             var neoForm = depFactory.create("de.oceanlabs.mcp:mcp_config:" + mcpVersion);
             var neoFormNotation = "de.oceanlabs.mcp:mcp_config:" + mcpVersion + "@zip";
@@ -186,7 +173,8 @@ public void enable(Project project, LegacyForgeModdingSettings settings, LegacyF
         obf.getSrgToNamedMappings().set(mappingsCsv);
 
         extension.getRuns().configureEach(run -> {
-            LegacyForgeFacade.configureRun(project, run);
+            // Old BSL versions before 2022 (i.e. on 1.18.2) did not export any packages, causing DevLaunch to be unable to access the main method
+            run.getJvmArguments().addAll("--add-exports", "cpw.mods.bootstraplauncher/cpw.mods.bootstraplauncher=ALL-UNNAMED");
 
             // Mixin needs the intermediate (SRG) -> named (Mojang, MCP) mapping file in SRG (TSRG is not supported) to be able to ignore the refmaps of dependencies
             run.getSystemProperties().put("mixin.env.remapRefMap", "true");
@@ -206,13 +194,6 @@ public void enable(Project project, LegacyForgeModdingSettings settings, LegacyF
         artifacts.runtimeDependencies()
                 .getDependencies().add(project.getDependencyFactory().create(project.files(mappingsCsv)));
 
-        // Forge expects to find the Forge and client-extra jar on the legacy classpath
-        // Newer FML versions also search for it on the java.class.path.
-        // MDG already adds cilent-extra, but the forge jar is missing.
-        runs.getAdditionalClasspath()
-                .extendsFrom(artifacts.runtimeDependencies())
-                .exclude(Map.of("group", "net.neoforged", "module", "DevLaunch"));
-
         var remapDeps = project.getConfigurations().create("remappingDependencies", spec -> {
             spec.setDescription("An internal configuration that contains the Minecraft dependencies, used for remapping mods");
             spec.setCanBeConsumed(false);

src/main/java/net/neoforged/moddevgradle/internal/ArtifactNamingStrategy.java

@@ -1,14 +1,26 @@
 package net.neoforged.moddevgradle.internal;
 
+import net.neoforged.moddevgradle.internal.utils.VersionCapabilitiesInternal;
 import org.jetbrains.annotations.ApiStatus;
 
 @FunctionalInterface
 @ApiStatus.Internal
 public interface ArtifactNamingStrategy {
-    static ArtifactNamingStrategy createDefault(String artifactFilenamePrefix) {
+    static ArtifactNamingStrategy createVanilla(String version) {
         return (artifact) -> {
-            // It's helpful to be able to differentiate the Vanilla jar and the NeoForge jar in classic multiloader setups.
-            return artifactFilenamePrefix + artifact.defaultSuffix + ".jar";
+            return "vanilla-%s%s.jar".formatted(version, artifact.defaultSuffix);
+        };
+    }
+
+    static ArtifactNamingStrategy createNeoForge(VersionCapabilitiesInternal versionCapabilities, String loader, String version) {
+        return (artifact) -> {
+            if (artifact != WorkflowArtifact.CLIENT_RESOURCES || versionCapabilities.modLocatorRework()) {
+                return "%s-%s%s.jar".formatted(loader, version, artifact.defaultSuffix);
+            } else {
+                // We have to ensure that client resources are named "client-extra" and *do not* contain forge-<version>
+                // otherwise FML might pick up the client resources as the main Minecraft jar.
+                return "client-extra-" + version + ".jar";
+            }
         };
     }
 

src/main/java/net/neoforged/moddevgradle/internal/ModDevArtifactsWorkflow.java

@@ -11,13 +11,13 @@
 import org.gradle.api.Named;
 import org.gradle.api.Project;
 import org.gradle.api.artifacts.Configuration;
+import org.gradle.api.artifacts.Dependency;
 import org.gradle.api.artifacts.ModuleDependency;
 import org.gradle.api.attributes.Attribute;
 import org.gradle.api.attributes.AttributeContainer;
 import org.gradle.api.attributes.Category;
 import org.gradle.api.attributes.DocsType;
 import org.gradle.api.attributes.Usage;
-import org.gradle.api.file.ConfigurableFileCollection;
 import org.gradle.api.file.Directory;
 import org.gradle.api.file.RegularFile;
 import org.gradle.api.plugins.JavaPluginExtension;
@@ -44,6 +44,7 @@ public record ModDevArtifactsWorkflow(
         ModdingDependencies dependencies,
         VersionCapabilitiesInternal versionCapabilities,
         TaskProvider<CreateMinecraftArtifacts> createArtifacts,
+        Provider<? extends Dependency> minecraftClassesDependency,
         TaskProvider<DownloadAssets> downloadAssets,
         Configuration runtimeDependencies,
         Configuration compileDependencies,
@@ -164,11 +165,11 @@ public static ModDevArtifactsWorkflow create(Project project,
 
         // For IntelliJ, we attach a combined sources+classes artifact which enables an "Attach Sources..." link for IJ users
         // Otherwise, attaching sources is a pain for IJ users.
-        Provider<ConfigurableFileCollection> minecraftClassesArtifact;
+        Provider<? extends Dependency> minecraftClassesDependency;
         if (ideIntegration.shouldUseCombinedSourcesAndClassesArtifact()) {
-            minecraftClassesArtifact = createArtifacts.map(task -> project.files(task.getCompiledWithSourcesArtifact()));
+            minecraftClassesDependency = createArtifacts.map(task -> project.files(task.getCompiledWithSourcesArtifact())).map(dependencyFactory::create);
         } else {
-            minecraftClassesArtifact = createArtifacts.map(task -> project.files(task.getCompiledArtifact()));
+            minecraftClassesDependency = createArtifacts.map(task -> project.files(task.getCompiledArtifact())).map(dependencyFactory::create);
         }
 
         // Name of the configuration in which we place the required dependencies to develop mods for use in the runtime-classpath.
@@ -178,7 +179,7 @@ public static ModDevArtifactsWorkflow create(Project project,
             config.setCanBeResolved(false);
             config.setCanBeConsumed(false);
 
-            config.getDependencies().addLater(minecraftClassesArtifact.map(dependencyFactory::create));
+            config.getDependencies().addLater(minecraftClassesDependency);
             config.getDependencies().addLater(createArtifacts.map(task -> project.files(task.getResourcesArtifact())).map(dependencyFactory::create));
             // Technically, the Minecraft dependencies do not strictly need to be on the classpath because they are pulled from the legacy class path.
             // However, we do it anyway because this matches production environments, and allows launch proxies such as DevLogin to use Minecraft's libraries.
@@ -191,7 +192,7 @@ public static ModDevArtifactsWorkflow create(Project project,
             config.setDescription("The compile-time dependencies to develop a mod, including Minecraft and modding platform classes.");
             config.setCanBeResolved(false);
             config.setCanBeConsumed(false);
-            config.getDependencies().addLater(minecraftClassesArtifact.map(dependencyFactory::create));
+            config.getDependencies().addLater(minecraftClassesDependency);
             config.getDependencies().add(moddingDependencies.gameLibrariesDependency());
         });
 
@@ -210,6 +211,7 @@ public static ModDevArtifactsWorkflow create(Project project,
                 moddingDependencies,
                 versionCapabilities,
                 createArtifacts,
+                minecraftClassesDependency,
                 downloadAssets,
                 runtimeDependencies,
                 compileDependencies,

src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java

@@ -72,19 +72,19 @@ public void enable(
             neoFormNotation = "net.neoforged:neoform:" + neoFormVersion + "@zip";
         }
 
-        // When a NeoForge version is specified, we use the dependencies published by that, and otherwise
-        // we fall back to a potentially specified NeoForm version, which allows us to run in "Vanilla" mode.
+        var versionCapabilities = neoForgeVersion != null ? VersionCapabilitiesInternal.ofNeoForgeVersion(neoForgeVersion)
+                : VersionCapabilitiesInternal.ofNeoFormVersion(neoFormVersion);
+
         ArtifactNamingStrategy artifactNamingStrategy;
+        // It's helpful to be able to differentiate the Vanilla jar and the NeoForge jar in classic multiloader setups.
         if (neoForge != null) {
-            artifactNamingStrategy = ArtifactNamingStrategy.createDefault("neoforge-" + neoForgeVersion);
+            artifactNamingStrategy = ArtifactNamingStrategy.createNeoForge(versionCapabilities, "neoforge", neoForgeVersion);
         } else {
-            artifactNamingStrategy = ArtifactNamingStrategy.createDefault("vanilla-" + neoFormVersion);
+            artifactNamingStrategy = ArtifactNamingStrategy.createVanilla(neoFormVersion);
         }
 
         var configurations = project.getConfigurations();
 
-        var versionCapabilities = neoForgeVersion != null ? VersionCapabilitiesInternal.ofNeoForgeVersion(neoForgeVersion)
-                : VersionCapabilitiesInternal.ofNeoFormVersion(neoFormVersion);
 
         var dependencies = neoForge != null ? ModdingDependencies.create(neoForge, neoForgeNotation, neoForm, neoFormNotation, versionCapabilities)
                 : ModdingDependencies.createVanillaOnly(neoForm, neoFormNotation);

src/main/java/net/neoforged/moddevgradle/internal/ModDevRunWorkflow.java

@@ -105,6 +105,11 @@ private ModDevRunWorkflow(Project project,
 
             spec.getDependencies().add(gameLibrariesDependency);
             addClientResources(project, spec, artifactsWorkflow.createArtifacts());
+            if (!versionCapabilities.modLocatorRework()) {
+                // Forge expects to find the Forge and client-extra jar on the legacy classpath
+                // Newer FML versions also search for it on the java.class.path.
+                spec.getDependencies().addLater(artifactsWorkflow.minecraftClassesDependency());
+            }
         });
 
         setupRuns(
@@ -242,6 +247,11 @@ public static void setupRuns(
 
         Map<RunModel, TaskProvider<PrepareRun>> prepareRunTasks = new IdentityHashMap<>();
         runs.all(run -> {
+            if (!versionCapabilities.modLocatorRework()) {
+                // TODO: do this properly now that we have a flag in the version capabilities
+                // This will explicitly be replaced in RunUtils to make this work for IDEs
+                run.getEnvironment().put("MOD_CLASSES", RunUtils.getGradleModFoldersProvider(project, run.getLoadedMods(), null).getClassesArgument());
+            }
             var prepareRunTask = setupRunInGradle(
                     project,
                     branding,

src/main/java/net/neoforged/moddevgradle/internal/utils/VersionCapabilitiesInternal.java

@@ -17,16 +17,17 @@
  * @param testFixtures     If the NeoForge version for this Minecraft version supports test fixtures.
  */
 public record VersionCapabilitiesInternal(String minecraftVersion, int javaVersion, boolean splitDataRuns,
-                                          boolean testFixtures) implements VersionCapabilities, Serializable {
+                                          boolean testFixtures, boolean modLocatorRework) implements VersionCapabilities, Serializable {
     private static final Logger LOG = LoggerFactory.getLogger(VersionCapabilitiesInternal.class);
 
-    private static final VersionCapabilitiesInternal LATEST = new VersionCapabilitiesInternal(MinecraftVersionList.VERSIONS.get(0), 21, true, true);
+    private static final VersionCapabilitiesInternal LATEST = ofVersionIndex(0);
 
     private static final Pattern NEOFORGE_PATTERN = Pattern.compile("^(\\d+\\.\\d+)\\.\\d+(|-.*)$");
     // Strips NeoForm timestamp suffixes OR dynamic version markers
     private static final Pattern NEOFORM_PATTERN = Pattern.compile("^(.*)-(?:\\+|\\d{8}\\.\\d{6})$");
 
     private static final int MC_24W45A_INDEX = getReferenceVersionIndex("24w45a");
+    private static final int MC_1_20_5_INDEX = getReferenceVersionIndex("1.20.5");
     private static final int MC_24W14A_INDEX = getReferenceVersionIndex("24w14a");
     private static final int MC_1_20_4_INDEX = getReferenceVersionIndex("1.20.4");
     private static final int MC_1_18_PRE2_INDEX = getReferenceVersionIndex("1.18-pre2");
@@ -55,8 +56,9 @@ public static VersionCapabilitiesInternal ofVersionIndex(int versionIndex, Strin
         var javaVersion = getJavaVersion(versionIndex);
         var splitData = hasSplitDataEntrypoints(versionIndex);
         var testFixtures = hasTestFixtures(versionIndex);
+        var modLocatorRework = hasModLocatorRework(versionIndex);
 
-        return new VersionCapabilitiesInternal(minecraftVersion, javaVersion, splitData, testFixtures);
+        return new VersionCapabilitiesInternal(minecraftVersion, javaVersion, splitData, testFixtures, modLocatorRework);
     }
 
     static int getJavaVersion(int versionIndex) {
@@ -79,6 +81,10 @@ static boolean hasTestFixtures(int versionIndex) {
         return versionIndex <= MC_1_20_4_INDEX;
     }
 
+    static boolean hasModLocatorRework(int versionIndex) {
+        return versionIndex <= MC_1_20_5_INDEX;
+    }
+
     static int indexOfNeoForgeVersion(String version) {
         // NeoForge omits the "1." at the start of the Minecraft version and just adds an incrementing last digit
         var matcher = NEOFORGE_PATTERN.matcher(version);
@@ -162,7 +168,8 @@ public VersionCapabilitiesInternal withMinecraftVersion(String minecraftVersion)
                 minecraftVersion,
                 javaVersion,
                 splitDataRuns,
-                testFixtures
+                testFixtures,
+                modLocatorRework
         );
     }
 }

src/test/java/net/neoforged/moddevgradle/internal/ModDevPluginTest.java

@@ -49,7 +49,7 @@ void testModdingCannotBeEnabledTwice() {
     @Test
     void testEnableForTestSourceSetOnly() {
         extension.enable(settings -> {
-            settings.setVersion("2.3.0");
+            settings.setVersion("100.3.0"); // Needs to be at least 20.5 to use paths for newer FML
             settings.setEnabledSourceSets(Set.of(testSourceSet));
         });
 
@@ -58,25 +58,25 @@ void testEnableForTestSourceSetOnly() {
         assertThatDependencies(mainSourceSet.getRuntimeClasspathConfigurationName()).isEmpty();
 
         // While the test classpath should have modding dependencies
-        assertContainsModdingCompileDependencies("2.3.0", testSourceSet.getCompileClasspathConfigurationName());
-        assertContainsModdingRuntimeDependencies("2.3.0", testSourceSet.getRuntimeClasspathConfigurationName());
+        assertContainsModdingCompileDependencies("100.3.0", testSourceSet.getCompileClasspathConfigurationName());
+        assertContainsModdingRuntimeDependencies("100.3.0", testSourceSet.getRuntimeClasspathConfigurationName());
     }
 
     @Test
     void testAddModdingDependenciesTo() {
-        extension.setVersion("2.3.0");
+        extension.setVersion("100.3.0"); // Needs to be at least 20.5 to use paths for newer FML
 
         // Initially, only the main source set should have the dependencies
-        assertContainsModdingCompileDependencies("2.3.0", mainSourceSet.getCompileClasspathConfigurationName());
-        assertContainsModdingRuntimeDependencies("2.3.0", mainSourceSet.getRuntimeClasspathConfigurationName());
+        assertContainsModdingCompileDependencies("100.3.0", mainSourceSet.getCompileClasspathConfigurationName());
+        assertContainsModdingRuntimeDependencies("100.3.0", mainSourceSet.getRuntimeClasspathConfigurationName());
         assertThatDependencies(testSourceSet.getCompileClasspathConfigurationName()).isEmpty();
         assertThatDependencies(testSourceSet.getRuntimeClasspathConfigurationName()).isEmpty();
 
         // Now add it to the test source set too
         extension.addModdingDependenciesTo(testSourceSet);
 
-        assertContainsModdingCompileDependencies("2.3.0", testSourceSet.getCompileClasspathConfigurationName());
-        assertContainsModdingRuntimeDependencies("2.3.0", testSourceSet.getRuntimeClasspathConfigurationName());
+        assertContainsModdingCompileDependencies("100.3.0", testSourceSet.getCompileClasspathConfigurationName());
+        assertContainsModdingRuntimeDependencies("100.3.0", testSourceSet.getRuntimeClasspathConfigurationName());
     }
 
     @Test