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

Separate Forge and NeoForge #128

Closed
wants to merge 1 commit into from

Conversation

trainb0y
Copy link

@trainb0y trainb0y commented Nov 21, 2023

For context, I'm trying to port a mod that uses YACL to NeoForge, but it's incompatible, since NeoForge made breaking changes.

Loosely following this migration guide I tried to add NeoForge to YACL. I'm not even sure if you're interested in supporting NeoForge or want the maintenance burden, but I had some free time so I figured I might as well give it a shot, rather than just opening a feature request issue.

Some Questions/Concerns/Notes/Stuff

  • Moved to Architectury Loom 1.4 beta for NeoForge support. I don't know if it broke anything.
  • I removed the Vineflower gradle plugin, as gradle was complaining that it was already added. (Loom 1.4 added vineflower support). I'm not sure if Vineflower is actually being used now though.
  • There is a lot of duplicated stuff, in both the gradle files and the forge mod toml. The only code changes I made to NeoForge were fixing the imports, since NeoForge repackaged everything. There's probably a good way to share more code, however I'm not qualified to do it lol.
  • The mods.toml files are slightly different, as NeoForge wants mixins to be specified in it, and their FML version seems to have been reset to 1. However, judging by the rest of the project, having hardcoded values (like that 1) in places like that is discouraged. Not really sure what to do about it.
  • Forge and NeoForge were both printing a warning about the mixin config not specifying minVersion so I added it. Was this intentional? Is there a reason not to specify it?
  • Might have broken the access wideners. Not sure. I've never tried to use them before. (edit: works now)
  • Neither test-forge or test-neoforge seem to work. Running the dev environment fails to load (here's a log fragment, the full log is too long to upload due to ModelBakery warnings). I didn't really touch much of the existing Forge stuff, so I'm not sure why it isn't working. Maybe I'm just doing the wrong thing? I'm very new to Architectury and haven't really touched Forge before, so it might just be user error.
  • Running remapJar on both test-forge and test-neoforge seems to output jars that don't include test-common and therefore won't run. Since I couldn't get either of them to run using gradle, I wasn't able to actually get the NeoForge version to load, and I have no idea if it actually, um, works.

So yeah, this is definitely not ready but I figured I should open a PR to see if I'm at least heading in the right direction, before sinking any more time into it.

@trainb0y
Copy link
Author

Since I couldn't get the testmod to load I decided to try with my own mod, and yeah, it crashes, and im fairly sure its an access widener issue. I'll see if I can fix it, but I don't really know what I'm doing when it comes to loom and gradle nonsense, if that wasn't obvious already lol.

https://mclo.gs/ju116Yo

@trainb0y
Copy link
Author

trainb0y commented Nov 21, 2023

Seems to work now

@IMB11
Copy link

IMB11 commented Nov 21, 2023

Would it not be better to drop Forge support and just support NeoForge for 1.20.2+, I don't see why anyone would bother supporting Forge anymore?

@isXander
Copy link
Owner

Would it not be better to drop Forge support and just support NeoForge for 1.20.2+, I don't see why anyone would bother supporting Forge anymore?

For now I think it best to continue to support MinecraftForge, purely because I'm not really privy to the state of the forge community at the moment. If it is clear that old MinecraftForge is dead, it's trivial to remove it.

@isXander
Copy link
Owner

Can't get this to run in a clone. Can't merge until it launches lol.

@trainb0y
Copy link
Author

trainb0y commented Nov 26, 2023

Can't get this to run in a clone. Can't merge until it launches lol.

Ignoring NeoForge for a moment, I tried to run the MinecraftForge testmod, but couldn't get that to launch, even in a fresh clone. This feels like user error, and I don't really know what to do at this point.

Recorded a bunch of logs, maybe you can tell what's going on here?

test-forge on 1.20.x/dev branch, with no changes

No idea why this one doesn't work.

test-forge on 1.20.x/dev branch with loom 1.4

I don't have the beginning of the log for this one because the modelbakery errors are so long, I can go and grab it if its helpful, but I'm lazy.

Loom 1.4 Patch
Subject: [PATCH] Loom 1.4
---
Index: build.gradle.kts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/build.gradle.kts b/build.gradle.kts
--- a/build.gradle.kts	(revision 4cdfa58b27d790786cbb2f17aa8f4635ea9d5a2d)
+++ b/build.gradle.kts	(date 1701015083614)
@@ -1,7 +1,6 @@
 plugins {
     alias(libs.plugins.architectury.plugin)
     alias(libs.plugins.architectury.loom) apply false
-    alias(libs.plugins.loom.vineflower) apply false
 
     alias(libs.plugins.minotaur) apply false
     alias(libs.plugins.cursegradle) apply false
@@ -81,9 +80,6 @@
     }
 }
 
-subprojects {
-    apply(plugin = rootProject.libs.plugins.loom.vineflower.get().pluginId)
-}
 
 githubRelease {
     token(findProperty("GITHUB_TOKEN")?.toString())
Index: gradle/libs.versions.toml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
--- a/gradle/libs.versions.toml	(revision 4cdfa58b27d790786cbb2f17aa8f4635ea9d5a2d)
+++ b/gradle/libs.versions.toml	(date 1701015029352)
@@ -1,6 +1,6 @@
 [versions]
 # Plugins
-architectury_loom = "1.3.+"
+architectury_loom = "1.4.+"
 architectury_plugin = "3.4.+"
 shadow = "7.1.+"
 loom_vineflower = "1.11.+"

test-forge on my pr

Almost identical to the loom 1.4, which makes sense since I didn't touch much of the MinecraftForge stuff.
By these I assume the issue is loom 1.4 and not my pr specifically?

test-neoforge on my pr

Seems to be the same issue, just minus the modelbakery spam?

Conclusion

I really don't know at this point, I'm kind of stumped. Some of this just feels like user error. Like I said, I haven't really touched Forge or Architectury before so maybe this is obvious to someone more experienced. I'm basically out of ideas though.

@isXander
Copy link
Owner

isXander commented Nov 28, 2023

I don't have the beginning of the log for this one because the modelbakery errors are so long, I can go and grab it if its helpful, but I'm lazy.

I found it.
Says something like "failed to find mod metadata for mod ids specified in @Mod entrypoint"

@isXander isXander closed this Feb 12, 2024
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

Successfully merging this pull request may close these issues.

3 participants