Legacy plugin for 1.17 to 1.20.1 #118

shartte · 2024-07-29T23:33:12Z

Adds a separate net.neoforged.moddev.legacy plugin that lives in a different sourceset and can be used to develop for Minecraft and Forge 1.17 to 1.20.1.

neoforged-pr-publishing · 2024-07-29T23:33:25Z

Publish PR to GitHub Packages

Last commit published: 39bb3eb0a6295626809482051fe08631d45c216b.

PR Publishing

The artifacts published by this PR:

Repository Declaration

In order to use the artifacts published by the PR, add the following repository to your buildscript:

repositories {
    maven {
        name 'Maven for PR #118' // https://github.com/neoforged/ModDevGradle/pull/118
        url 'https://prmaven.neoforged.net/ModDevGradle/pr118'
        content {
            includeModule('net.neoforged.moddev.legacy', 'net.neoforged.moddev.legacy.gradle.plugin')
            includeModule('net.neoforged.moddev', 'net.neoforged.moddev.gradle.plugin')
            includeModule('net.neoforged', 'moddev-gradle')
            includeModule('net.neoforged.moddev.repositories', 'net.neoforged.moddev.repositories.gradle.plugin')
        }
    }
}

neoforged-automation · 2024-08-13T20:22:59Z

@shartte, this pull request has conflicts, please resolve them for this PR to move forward.

Technici4n

Not too bad. However, it is clear that this can only work as long as keep the toolchain essentially the same. When we will remove the legacy CP, this legacy plugin will go out of the window. This is also why I don't see this ever supporting 1.16. This raises the question of whether the plugin should exist in the same branch or not.

Technici4n · 2024-09-11T13:20:08Z

gradle/wrapper/gradle-wrapper.properties

@@ -1,6 +1,6 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.9-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.10-bin.zip


Suggested change

distributionUrl=https\://services.gradle.org/distributions/gradle-8.10-bin.zip

distributionUrl=https\://services.gradle.org/distributions/gradle-8.10-all.zip

We are writing a plugin, I want the sources.

Technici4n · 2024-09-11T13:20:46Z

legacytest/build.gradle

+repositories {
+    mavenLocal()
+    maven {
+        name 'cursemaven'
+        url 'https://cursemaven.com'
+        content {
+            includeGroup "curse.maven"
+        }
+    }
+}


Remove unused mavens. (Before merging)

Technici4n · 2024-09-11T13:22:04Z

legacytest/forge/build.gradle

+
+dependencies {
+    modCompileOnly('mezz.jei:jei-1.20.1-forge:15.17.0.76') {
+        transitive = false


Technici4n · 2024-09-11T13:22:17Z

legacytest/forge/build.gradle

+dependencies {
+    modCompileOnly('mezz.jei:jei-1.20.1-forge:15.17.0.76') {
+        transitive = false
+    }
+    modRuntimeOnly('curse.maven:mekanism-268560:5662583')
+    modImplementation('curse.maven:applied-energistics-2-223794:5641282')
+}


Do we really want to have these deps in repo? 😅

For a test project of an old MC version, why not
My only concern is these mavens going down tbh
The artifacts aren't going to change so we will never update these again

Technici4n · 2024-09-11T13:27:24Z

src/legacy/java/net/neoforged/moddevgradle/legacy/MixinExtension.java

+import javax.inject.Inject;
+import java.util.List;
+
+public abstract class MixinExtension {


We should move all extensions to a root extension. For example neoForgeLegacy.

Technici4n · 2024-09-11T14:24:38Z

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

@@ -244,7 +257,7 @@ public static ModFoldersProvider getIdeaModFoldersProvider(Project project,
            folders = getModFoldersForGradle(project, modsProvider, includeUnitTests);
        }

-        var modFoldersProvider = project.getObjects().newInstance(ModFoldersProvider.class);
+        var modFoldersProvider = project.getObjects().newInstance(ModFoldersProvider.class, project);


Unnecessary.

Technici4n · 2024-09-11T14:25:25Z

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

@@ -395,24 +408,29 @@ record AssetProperties(String assetIndex, String assetsRoot) {

 abstract class ModFoldersProvider implements CommandLineArgumentProvider {
    @Inject
-    public ModFoldersProvider() {
+    public ModFoldersProvider(Project project) {
+        getClassesArgument().set(project.provider(() -> {


You don't need the ugly provider call. You can use getModFolders().map(...).

Technici4n · 2024-09-11T14:26:04Z

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


 public record UserDevConfig(String mcp, String sources, String universal, List<String> libraries, List<String> modules,
                            Map<String, UserDevRunType> runs) implements Serializable {
    public static UserDevConfig from(File userDevFile) {
+        // For backwards compatibility reasons we also support loading this from the userdev jar


I don't like this at all, it's such a hack.

Technici4n · 2024-09-11T14:26:28Z

testproject/oldneo/build.gradle

@@ -0,0 +1,8 @@
+


not working (yet)?

Technici4n · 2024-09-11T14:27:22Z

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

                        RunUtils.escapeJvmArg(RunUtils.getEclipseModFoldersProvider(project, run.getMods(), false).getArgument())
                )
                .args(RunUtils.escapeJvmArg(RunUtils.getArgFileParameter(prepareTask.getProgramArgsFile().get())))
-                .envVar(run.getEnvironment().get())
+                .envVar(RunUtils.replaceModClassesEnv(run, () -> RunUtils.getEclipseModFoldersProvider(project, run.getMods(), false)))


Not to mention that this sets both MOD_CLASSES and --fml.modFolders.

neoforged-automation · 2024-10-03T21:02:24Z

@shartte, this pull request has conflicts, please resolve them for this PR to move forward.

neoforged-automation · 2024-10-13T10:03:15Z

@shartte, this pull request has conflicts, please resolve them for this PR to move forward.

Co-authored-by: Matyrobbrt <[email protected]>

shartte · 2024-10-13T13:57:04Z

legacytest/forge/src/main/java/mymod/JeiCompat.java

+
+    @Override
+    public void registerItemSubtypes(ISubtypeRegistration registration) {
+        registration.registerSubtypeInterpreter(Items.ALLIUM, (ingredient, context) -> "allium");


We might want to comment here that calling this method tests the remapping of the JEI artifact since this method call would otherwise fail to compile because of the Item argument

shartte · 2024-10-13T13:58:26Z

src/legacy/java/net/neoforged/moddevgradle/legacy/LegacyForgeMetadataTransform.java

+
+import javax.inject.Inject;
+
+// @CacheableTransform


Re enable before merge

neoforged-automation · 2024-10-13T21:48:00Z

@shartte, this pull request has conflicts, please resolve them for this PR to move forward.

pietro-lopes · 2024-10-26T01:13:38Z

I have been using this for a while and the only issue I found was with this setup when using a version of GregTech when it was built when they were still using ArchLoom.

https://github.com/pietro-lopes/mdg-legacy-pr118

Clone repo and run each of the 3 available runs: 1 failing, 2 working.

I noticed that their metadata.json lacks the info about if the jarjar is obfuscated or not, but I don't know if that alone is enough.

Edit: Can confirm that editing metadata.json takes no effect.

neoforged-automation bot added the needs rebase This Pull Request needs to be rebased before being merged label Aug 13, 2024

neoforged-automation bot removed the needs rebase This Pull Request needs to be rebased before being merged label Sep 6, 2024

Matyrobbrt marked this pull request as ready for review September 10, 2024 09:41

Matyrobbrt changed the title ~~Legacy~~ Legacy plugin for 1.17 to 1.20.1 Sep 10, 2024

Technici4n requested changes Sep 11, 2024

View reviewed changes

neoforged-automation bot added the needs rebase This Pull Request needs to be rebased before being merged label Oct 3, 2024

shartte force-pushed the legacy branch 2 times, most recently from 55c7be9 to 102b7bf Compare October 12, 2024 23:55

neoforged-automation bot removed the needs rebase This Pull Request needs to be rebased before being merged label Oct 12, 2024

shartte force-pushed the legacy branch from e8d53ee to b69df63 Compare October 13, 2024 09:27

neoforged-automation bot added the needs rebase This Pull Request needs to be rebased before being merged label Oct 13, 2024

Add legacy support for Forge 1.17 to 1.20.1

39bb3eb

Co-authored-by: Matyrobbrt <[email protected]>

shartte force-pushed the legacy branch from b69df63 to 39bb3eb Compare October 13, 2024 10:28

neoforged-automation bot removed the needs rebase This Pull Request needs to be rebased before being merged label Oct 13, 2024

shartte commented Oct 13, 2024

View reviewed changes

neoforged-automation bot added the needs rebase This Pull Request needs to be rebased before being merged label Oct 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Legacy plugin for 1.17 to 1.20.1 #118

Legacy plugin for 1.17 to 1.20.1 #118

shartte commented Jul 29, 2024 •

edited by Matyrobbrt

Loading

neoforged-pr-publishing bot commented Jul 29, 2024 •

edited

Loading

The artifacts published by this PR:

Repository Declaration

neoforged-automation bot commented Aug 13, 2024

Technici4n left a comment

Technici4n Sep 11, 2024

Technici4n Sep 11, 2024

Technici4n Sep 11, 2024

Technici4n Sep 11, 2024

shartte Oct 13, 2024

Technici4n Sep 11, 2024

Technici4n Sep 11, 2024

Technici4n Sep 11, 2024

Technici4n Sep 11, 2024

Technici4n Sep 11, 2024

Technici4n Sep 11, 2024

neoforged-automation bot commented Oct 3, 2024

neoforged-automation bot commented Oct 13, 2024

shartte Oct 13, 2024

shartte Oct 13, 2024

neoforged-automation bot commented Oct 13, 2024

pietro-lopes commented Oct 26, 2024 •

edited

Loading

	distributionUrl=https\://services.gradle.org/distributions/gradle-8.10-bin.zip
	distributionUrl=https\://services.gradle.org/distributions/gradle-8.10-all.zip

Legacy plugin for 1.17 to 1.20.1 #118

Are you sure you want to change the base?

Legacy plugin for 1.17 to 1.20.1 #118

Conversation

shartte commented Jul 29, 2024 • edited by Matyrobbrt Loading

neoforged-pr-publishing bot commented Jul 29, 2024 • edited Loading

The artifacts published by this PR:

Repository Declaration

neoforged-automation bot commented Aug 13, 2024

Technici4n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neoforged-automation bot commented Oct 3, 2024

neoforged-automation bot commented Oct 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neoforged-automation bot commented Oct 13, 2024

pietro-lopes commented Oct 26, 2024 • edited Loading

shartte commented Jul 29, 2024 •

edited by Matyrobbrt

Loading

neoforged-pr-publishing bot commented Jul 29, 2024 •

edited

Loading

pietro-lopes commented Oct 26, 2024 •

edited

Loading