Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Help using loom in common module #17

Open
Trikzon opened this issue Jun 21, 2022 · 15 comments
Open

Help using loom in common module #17

Trikzon opened this issue Jun 21, 2022 · 15 comments

Comments

@Trikzon
Copy link
Contributor

Trikzon commented Jun 21, 2022

Hello, I have attempted changing my common module to use fabric loom, but I get an error when running the game that I can't solve.

The TLDR of why I want to do this is two reasons. First, the supposed performance improvements. However more importantly, I want to create a new module which my fabric module would depend on that would itself depend on the fabric api. When trying to create this module, I get the same error.

Attempt

You can view my attempt here: dodogang@e4882ea

However, the only changes are in the Common/build.gradle

plugins {
-   id 'org.spongepowered.gradle.vanilla' version '0.2.1-SNAPSHOT'
+   id 'fabric-loom' version '0.12-SNAPSHOT'
}
dependencies {
+   minecraft "com.mojang:minecraft:${minecraft_version}"
+   mappings loom.officialMojangMappings()
+
    compileOnly group:'org.spongepowered', name:'mixin', version:'0.8.5'
    implementation group: 'com.google.code.findbugs', name: 'jsr305', version: '3.0.1'
}

However, this gives the following error when running both Fabric Client and Forge Client

Circular dependency between the following tasks:
:Common:prepareRemapJar
\--- :Fabric:jar
     \--- :Fabric:classes
          \--- :Fabric:compileJava
               \--- :Common:remapJar
                    \--- :Common:prepareRemapJar (*)

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

The full build output is provided here, however it doesn't give much more information than what is above.

I have exhausted my knowledge of gradle today trying to figure out why this error is occurring. I expect there is a loom flag I don't know about that would solve this. Either way, if you have a lead on what can fix this error I'd love to here it :)

@oliviathevampire
Copy link

Have you tried removing loom from the fabric part?

@Trikzon
Copy link
Contributor Author

Trikzon commented Jun 21, 2022

I hadn't, but trying to remove the fabric-loom gradle plugin from the Fabric module (I assume this is what you meant) causes even worse errors.

A short excerpt is below:

A problem occurred evaluating project ':Fabric'.
> Could not find method minecraft() for arguments [com.mojang:minecraft:1.19] on object of type org.gradle.api.internal.artifacts.dsl.dependencies.DefaultDependencyHandler.

I am pretty sure that the Fabric module requires fabric-loom itself too

@jaredlll08
Copy link
Owner

Just spoke with @modmuss50 about this a bit, and this is what I came up with:

diff --git a/Common/build.gradle b/Common/build.gradle
index ea3a28a..9053a53 100644
--- a/Common/build.gradle
+++ b/Common/build.gradle
@@ -1,29 +1,22 @@
 plugins {
     id 'java'
-    id 'org.spongepowered.gradle.vanilla' version '0.2.1-SNAPSHOT'
+    id 'fabric-loom' version '0.12-SNAPSHOT'
     id 'maven-publish'
 }
 
 archivesBaseName = "${mod_name}-common-${minecraft_version}"
 
-minecraft {
-    version(minecraft_version)
-    runs {   
-        if (project.hasProperty('common_runs_enabled') ? project.findProperty('common_runs_enabled').toBoolean() : true) {
-        
-            server(project.hasProperty('common_server_run_name') ? project.findProperty('common_server_run_name') : 'vanilla_server') {
-                workingDirectory(this.file("run"))
-            }
-            client(project.hasProperty('common_client_run_name') ? project.findProperty('common_client_run_name') : 'vanilla_client') {
-                workingDirectory(this.file("run"))
-            }
-        }
-    }
+dependencies {
+    minecraft "com.mojang:minecraft:${minecraft_version}"
+    mappings loom.officialMojangMappings()
 }
 
-dependencies {
-     compileOnly group:'org.spongepowered', name:'mixin', version:'0.8.5'
-     implementation group: 'com.google.code.findbugs', name: 'jsr305', version: '3.0.1'
+loom {
+    shareCaches()
+    remapArchives = false
+    mixin {
+        useLegacyMixinAp = false
+    }
 }
 
 processResources {
diff --git a/Fabric/build.gradle b/Fabric/build.gradle
index 337a4a2..56712f1 100644
--- a/Fabric/build.gradle
+++ b/Fabric/build.gradle
@@ -12,10 +12,11 @@ dependencies {
     modImplementation "net.fabricmc:fabric-loader:${fabric_loader_version}"
     modImplementation "net.fabricmc.fabric-api:fabric-api:${fabric_version}"
     implementation group: 'com.google.code.findbugs', name: 'jsr305', version: '3.0.1'
-    implementation project(":Common")
+    implementation project(path: ":Common", configuration: "namedElements")
 }
 
 loom {
+    shareCaches()
     runs {
         client {
             client()

I believe the line that solves the original issue (circular tasks) is:

implementation project(path: ":Common", configuration: "namedElements")
loom {
    mixin {
        useLegacyMixinAp = false
    }
}

Is required to fix:
FabricMC/fabric-loom#676

Unfortunately, applying this diff and building will not give you a complete Common jar file (Which you should be publishing to maven for other MultiLoader projects to pull in and use in their Common projects), the jar file it generates for Common won't have any of your code, just your resources.

If you look at Common/build/devlibs/ though, you will find a jar file with the correct files, I'm not sure if you are able to publish that jar file to maven though, but it could be a solution.

This should be resolved by:

loom {
    remapArchives = false
}

however there is currently a bug in loom where that line is read to late and ends up doing nothing (FabricMC/fabric-loom#677).

Once that loom bug is fixed, I can re look at what needs doing and hopefully it all works (tm) and I can add it as an example project.

Also just so this is noted somewhere, the improvements you get from using Loom for Common are (Thank you modmuss for the info):

  • newer loom automatically re-uses resources between projects where possible, so should be a decent amount faster
  • Your fabric project will have project specific transformations applied to it from mods. (Such as access wideners from fabric api) thus it needs its own jar
  • You can use parchment on the common project, I dont believe VG supports that.
  • You can also make use of the new split sourcesets if wanted as well.
    The split sourceset stuff will prob need more fiddling to get working, but shouldn't be too bad

This was referenced Jun 25, 2022
@lukebemish
Copy link
Contributor

In case anyone wishes to compare, here is how I solved this myself a while back:
https://github.com/lukebemish/MultiLoader-Template/tree/1.18%2Bquilt_beta/Common
Though I haven't updated my fork, the same approach works in 1.19.

@Trikzon
Copy link
Contributor Author

Trikzon commented Jun 29, 2022

Thank you @jaredlll08 for spending time on my issue. I got loom working in the common module thanks to it. I'm not concerned with the maven issue because I don't have a maven set up and don't plan to. As far as I understand it—while it's not a great solution—a mod that depends on my mod could just put the fabric jar into the common module using cursemaven and then only use common classes in the common module. Either way, I don't expect anyone to depend on my mod and they could always contact me if needed.

I also made progress on my original plan of having a shared submodule library between mods, however I ran into some issues. I was able to have the shared library's classes relocate using shadow (to avoid class conflicts), however I ran into an issue with the mixin refmaps not respecting the package relocation.

I have decided that it's too hacky of a solution for now and will not continue with it. I may try again in the future, but I think it's more likely that I'll simply make a separate library mod instead.

I'm satisfied with closing this issue, but I'll leave it up to you whether you want to keep it open until the mentioned loom bug is fixed.

Thanks again :D

@jaredlll08
Copy link
Owner

@lukebemish

In case anyone wishes to compare, here is how I solved this myself a while back: https://github.com/lukebemish/MultiLoader-Template/tree/1.18%2Bquilt_beta/Common Though I haven't updated my fork, the same approach works in 1.19.

I believe your setup is basically the same as the setup I described here #17 (comment), including all the issues that comes with it.

@Trikzon

I'm satisfied with closing this issue, but I'll leave it up to you whether you want to keep it open until the mentioned loom bug is fixed.

I'm going to keep the issue open until that loom bug is fixed and until there is a version using loom for common with proper built jars

@lukebemish
Copy link
Contributor

I have not noticed most of the issues you describe with my setup. I am using the non-legacy mixin AP, and I have not had issues publishing common jars

@jaredlll08
Copy link
Owner

Where do you get the common jar from?

When I pulled your repo and did a build, I didn't see a usable jar (only javadoc was there) in Common/build/libs, I did see one in Common/build/devlibs though, which from what I understand turning remapArchives off will get rid of (if remapArchives was working)

@lukebemish
Copy link
Contributor

lukebemish commented Jun 29, 2022

Mine's just in libs. Seems to be in Mojmaps and all

@lukebemish
Copy link
Contributor

Not sure what I did to get that to work though

@lukebemish
Copy link
Contributor

lukebemish commented Jun 29, 2022

If you look at https://lukebemish/excavated_variants, that has this setup in 1.19 and it works fine, with the exception of the sources jar, which ends up in devlibs. However, I reference that one on its own when I publish to get around this.

@lukebemish
Copy link
Contributor

lukebemish commented Aug 21, 2022

I've come up with a rather clever way to force usable jars to be generated with loom in common that I figured I should share here in case this is still relevant. Add the following somewhere after the dependency block in the common build.gradle:

import net.fabricmc.loom.task.AbstractRemapJarTask
tasks.withType(AbstractRemapJarTask).each {
    it.targetNamespace = "named"
}

Instead of disabling remapping entirely, I just force loom to map from named to named.

@KirboSoftware
Copy link

@jaredlll08 Do you know how to do this with fabric-loom 1.0+??

As I get this when I apply id 'fabric-loom' version '1.0-SNAPSHOT' to the common's build.gradle

plugins {
    id 'java'
    id 'fabric-loom' version '1.0-SNAPSHOT'
    id 'maven-publish'
}

archivesBaseName = 'blocksterhunter-platform-common'

loom {
    shareCaches()
    remapArchives = false
    mixin {
        useLegacyMixinAp = false
    }
}

dependencies {
    minecraft "com.mojang:minecraft:${minecraft_version}"
    mappings loom.layered(){
        officialMojangMappings()
        parchment("org.parchmentmc.data:parchment-${minecraft_version}:${parchment_version}@zip")
    }

//    compileOnly group:'org.spongepowered', name:'mixin', version:'0.8.5'
//    implementation group: 'com.google.code.findbugs', name: 'jsr305', version: '3.0.1'
}
Build file '/home/joost/dev/mc-mods/BlocksterHunter/blocksterhunter-platform-common/build.gradle' line: 9

A problem occurred evaluating project ':blocksterhunter-platform-common'.
> No signature of method: build_bk9u5ancg09and6pt8avbcy7f.loom() is applicable for argument types: (build_bk9u5ancg09and6pt8avbcy7f$_run_closure1) values: [build_bk9u5ancg09and6pt8avbcy7f$_run_closure1@393a92f8]
  Possible solutions: copy(groovy.lang.Closure), copy(org.gradle.api.Action), wait(), run(), run(), find()

@MerchantPug
Copy link
Contributor

MerchantPug commented May 23, 2023

I'm going to keep the issue open until that loom bug is fixed and until there is a version using loom for common with proper built jars

The Loom bug is now fixed fyi, but the second point is something I'm unsure on.

@MerchantPug
Copy link
Contributor

For future reference.
Witixin made a Loom in Common Gist, you can find it here.

https://gist.github.com/Witixin1512/bbb8484523ea6a0e8440f17621f89f95

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants