Support Forge 1.20.6+ and NeoForge 1.20.4+ #23
Conversation
Former is Forge specific, latter is ModLauncher specific, and while they tend to change at the same time, that'll no longer be true once we also consider NeoForge.
Newer Forge versions will put inner jars (such as Kotlin inside KFF) on a layer which depends on the outer jar (KFF), which may be different than what one would expect from just looking at the Kotlin jar. As such we can no longer determine the layer ahead of time and will instead assume that layers are built in order, and just assume whichever layer is the most recent one. Note that we cannot easily just check which layer our SecureJar is in because it may be further wrapped by e.g. KFFMerger and as such may not directly appear in any of the lists at all.
Will be required for KFF5 which uses JarJar, so we want to emit all our Kotlin jars separately instead of merging them into the outer jar.
DJtheRedstoner
left a comment
DJtheRedstoner
left a comment
There was a problem hiding this comment.
Looks good, pending appropriate testing
| // ModLauncher somehow manages to report the version of some jars (really unsure which ones, best guess | ||
| // right now is all those without a `FMLModType` manifest attribute? seemingly completely irregardless of | ||
| // whether they have an `Implementation-Version` attribute!) | ||
| // as "Optional.empty" (yes, that's a String, somewhere someone must have blindly `toString`ed). |
There was a problem hiding this comment.
This was fixed here, but not in forge's fork (not exactly sure what causes it, but definitely only affects modular jars, i.e. those with module-info.class). For present values Optional#toString does wrap the inner toString in Optional[], does this affect anything?
There was a problem hiding this comment.
Hm, while I haven't ever observed a non-empty one (looking at the Java spec, there doesn't even appear to be a way to set your module's version, you have to manually tell Gradle to add it, so predicatively most people just don't have one set; only 270 search results in all of GitHub), we should definitely go unwrap those in case a future version of any of the libs we care about ends up doing it.
Now that I'm looking at the source of that bug, and the fact that it's been fixed in NeoForge, I realize that we should probably also remove the early if (version == null) return FALLBACK_VERSION; given there's a non-negligible chance that it might show up if they change something in their mod loading such that that class actually gets used for one of the libs we care about.
|
(will re-order the release commits to be after this new commit before merge) |
There are really two independent issues, the `toString` and the fact that it returns no version, which may happen independently. As such, this commit also unwraps the `toString` result of non-empty `Optional`s, and runs the fallback version reading code when the raw version reported is already `null`.
Johni0702
force-pushed
the
feature/neoforge
branch
from
8dae5d4 to
2b35cce
Compare
See individual commits.
ML11 is required for NeoForge 1.21 (LexForge stayed on its fork of ML10).
KFF5 is required for 1.20.6+ (and is the only thing required for Forge 1.20.6+).