Rewrite language loaders and allow for multiple entrypoints per mod #126
Conversation
This file contains 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
Last commit published: 567632e2ab2c7dd4be91462df1d7b1f11dfc622d. PR PublishingThe artifacts published by this PR:
Repository DeclarationIn order to use the artifacts published by the PR, add the following repository to your buildscript: repositories {
maven {
name 'Maven for PR #126' // https://github.com/neoforged/FancyModLoader/pull/126
url 'https://prmaven.neoforged.net/FancyModLoader/pr126'
content {
includeModule('net.neoforged.fancymodloader', 'earlydisplay')
includeModule('net.neoforged.fancymodloader', 'loader')
}
}
} |
|
@Matyrobbrt, this PR introduces breaking changes. Compatibility checks
|
shartte
reviewed
May 3, 2024
shartte
reviewed
May 3, 2024
Comment on lines
+18
to
+23
| private static final Supplier<Map<String, String>> globals = Suppliers.memoize(() -> { | ||
| final var globals = new HashMap<String, String>(); | ||
| globals.put("mcVersion", FMLLoader.versionInfo() == null ? null : FMLLoader.versionInfo().mcVersion()); | ||
| globals.put("neoForgeVersion", FMLLoader.versionInfo() == null ? null : FMLLoader.versionInfo().neoForgeVersion()); | ||
| return globals; | ||
| }); |
There was a problem hiding this comment.
refactor for later: not make this static
shartte
reviewed
May 3, 2024
| @@ -55,7 +54,8 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si | |||
|
|
|||
| public void buildData(final Set<ModFileScanData.ClassData> classes, final Set<ModFileScanData.AnnotationData> annotations) { | |||
| classes.add(new ModFileScanData.ClassData(this.asmType, this.asmSuperType, this.interfaces)); | |||
| final List<ModFileScanData.AnnotationData> collect = this.annotations.stream().filter(ma -> ModFileScanData.interestingAnnotations().test(ma.getASMType())).map(a -> ModAnnotation.fromModAnnotation(this.asmType, a)).collect(Collectors.toList()); | |||
There was a problem hiding this comment.
so one way (later) to speed up annotation scanning is to reintroduce the "annotation interest" but it's complex, so forget about it for now.
There was a problem hiding this comment.
This implementation has no speed impact regardless, at best a memory impact, as ann data has already been collected by then.
There was a problem hiding this comment.
Yeah I just meant that in general, to make any sense at all it'd need to use constant-pool scanning based on the interest to avoid having to parse the class-file in the first place.
But assuming we can just cache the entire annotation index per-jar based on a file-checksum, the speed advantage probably diminishes.
shartte
approved these changes
May 3, 2024
shartte
approved these changes
May 3, 2024
Matyrobbrt
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 changes language loaders to directly load mods instead of relying on a scan data consumer, and allows a mod file to have different mods loaded with different locators. Right now there's no way to express it in the default toml format, but that can be changed.
It also allows a javafml mod to have multiple
@Modannotations, with an optional dist parameter.