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

Add support for locating mod sources with services from the classpath. #93

Merged
merged 8 commits into from
Mar 25, 2024

Conversation

marchermans
Copy link
Contributor

@marchermans marchermans commented Feb 25, 2024

This allows a SERVICE-layer source set or jar to be loaded from the classpath in userdev if it is part of the MOD_CLASSES.

Copy link
Member

@FiniteReality FiniteReality left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems fine as-is. Maybe we should consider extracting the parsing of MOD_CLASSES to a common location - I believe there's an exact duplicate of this logic in MinecraftLocator as well as one or two other places.

return metadata.providers().stream()
.map(SecureJar.Provider::serviceName)
.anyMatch(TransformerDiscovererConstants.SERVICES::contains);
return TransformerDiscovererConstants.shouldLoadInServiceLayer(path);
Copy link
Member

@FiniteReality FiniteReality Mar 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we're delegating to this method, is shouldLoadInServiceLayer a good name for this method? Would something like isValidServiceLayerJar be a better name?

(I would have put this comment on the declaration of the method a few lines above, but it wasn't changed so I couldn't)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine like this. Find a synonymous name doesn't make the method clearer.

@neoforged-pr-publishing
Copy link

neoforged-pr-publishing bot commented Mar 8, 2024

  • Publish PR to GitHub Packages

Last commit published: 8e92bdc964368f4cf771d9a44099929d071bc910.

PR Publishing

The artifacts published by this PR:

Repository Declaration

In order to use the artifacts published by the PR, add the following repository to your buildscript:

repositories {
    maven {
        name 'Maven for PR #93' // https://github.com/neoforged/FancyModLoader/pull/93
        url 'https://prmaven.neoforged.net/FancyModLoader/pr93'
        content {
            includeModule('net.neoforged.fancymodloader', 'spi')
            includeModule('net.neoforged.fancymodloader', 'earlydisplay')
            includeModule('net.neoforged.fancymodloader', 'loader')
        }
    }
}

@Technici4n Technici4n merged commit 072d112 into main Mar 25, 2024
3 checks passed
@Technici4n Technici4n deleted the feature/service-layer-classpath-discovery branch March 25, 2024 09:37
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.

4 participants