Fix compatibility with mods using JarJar to include Kotlin without KotlinForForge - #36
Merged
Merged
Conversation
We augment the classpaths constructed by ModLauncher in three ways: 1. We sort them by version, so newer versions reliably get loaded over older ones (see `SortedJarOrPathList`) 2. We merge our Kotlin libraries into KotlinForForge, or add them to the classpath directly if KFF is recent enough (see `KFFMerger`) 3. We rename our bundled KFF module to match the name of any externally istalled ones, since the name of the latter depends on their file name (see `SelfRenamingJarMetadata`) This commit changes the way we accomplish the first two. Previously, we replaced the ModLauncher-internal list which holds the in-progress classpath, with a custom one that does the sorting, and also calls `KFFMerger` on every entry as it is `add`ed. The only reason we originally did `KFFMerger` via `add`, is because `SortedJarOrPathList` existed before `KFFMerger` became a thing, and it simply provided a convenient entrypoint without requiring any substantial changes. With this commit, we still replace the internal list, but instead of acting on addition, we now act only once, namely when ModLauncher tries to turn the finished list into a classpath by calling `.stream` on the list. The motivation for this change is that we'll want to avoid adding our bundled KFF jar altogether when there's no externally installed KFF present. This is necessary for us to be compatible with third-party mods which bundle Kotlin but not KFF. And with the current implementation we cannot know whether that's no external KFF because we only act on addition and there could always be a later addition after the one we're currently evaluating. Note that, barring third-party mods which also replace that internal list, with this commit our code should still behave effectively exactly identical to how it did before this commit. The functional changes to how `KFFMerger` behaves to make us compatible with such mods will be made in a later commit.
Prior to this commit, we were relying on either finding a JarJar-using KFF jar installed by the user (in which case we'd add our Kotlin jars to the classpath directly), or our bundled legacy KFF jar (into which we'd inject our Kotlin jars). This commit removes the requirement for us to bundle a KFF jar, by adding our Kotlin jars to the classpath directly so longe as there isn't a user-installed legacy KFF jar (and if there is, we continue to inject into it). This fixes compatibility with third-party mods which JarJar Kotlin libs themselves. Such mods do not include a KFF jar, and if the user hasn't installed one manually, prior to this commit we'd have then injected our Kotlin into our legacy bundled KFF jar. This would then result in a java.lang.module.ResolutionException because both our KFF jar and the Kotlin libs would contain overlapping packages. With this commit, we now have the option of simply not shipping a bundled KFF jar (we don't need KFF ourselves), so in the same scenario we'd then also just add our Kotlin libs to the classpath directly, and regular highest-version-resolution makes it all work correctly.
This has gone unnoticed because it only affects KFF versions that are ancient enough to where we've always shipped a newer version anyway, so it didn't actually matter. But now that we no longer always ship KFF, we are mis-identifying such old KFF versions as containing only Kotlin (because it looks to us like it doesn't contain kotlinx-coroutines, and we don't check kotlinx-serialization), and then incorrectly treat them as a standalone Kotlin lib. This commit fixes that by returning a non-0 version even without the version file so long as the `Job` class, which has existed since initial stable release, is found.
When there's zero language providers registered, `modFiles` will be `null`. Prior to this commit, we'd simply return in this case, which is wrong, but we never actually ran into this case because we always included KFF. We'll want to no longer do that though (see previous two commits), so we do now run into this condition, and `return`ing results in Kotlin not loading. So this commit instead initializes `modFiles` to an empty list.
Traben-0
self-requested a review
July 6, 2026 05:51
Traben-0
approved these changes
Jul 6, 2026
Traben-0
left a comment
Traben-0
left a comment
Contributor
There was a problem hiding this comment.
lgtm, though as usual with the loader another pair of eyes would be nice
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes Essential crashing when another mod uses (Neo)Forge's JarJar feature to bundle Kotlin libraries (so they do not need users to separately also install KotlinForForge) and KotlinForForge is not installed separately. See e.g. the slack thread linked in the linear issue.
See the description of the two main commit (59abdb0, f28181f) for details.
This PR also includes two relatively simple refactoring commits prior to the main commits and two small related fixes (see the description of those commits for details) that were required to get everything fully working afterwards.
Would highly recommend consuming commit-by-commit.
Linear: EM-3721