Skip to content

[FG7] Add a way to configure Tools (Mavenizer/Slimelauncher) #971

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

Open
wants to merge 4 commits into
base: FG_7.0
Choose a base branch
from

Conversation

RealMangorage
Copy link
Contributor

@RealMangorage RealMangorage commented Jun 17, 2025

This is mainly a PR to discuss the most ideal way of handling the configurability of these tools that FG7 uses.

The build script would look like this, to override a tool.

dependencies {
    mavenizer "net.minecraftforge:minecraft-mavenizer:0.3.3"
    slimelauncher "net.minecraftforge:slime-launcher:0.1.0"

    implementation(minecraft.dep("net.minecraftforge:forge:1.21.5-55.0.23"))
}

@Jonathing
Copy link
Member

Jonathing commented Jun 17, 2025

There are a couple of things about your implementation I don't like. But with regards to what we want to add, I do like this.

Originally, I was planning something like this:

minecraft {
    mavenizer = 'me.jonathing:mavenizer:0.3.0'
}

But this works better. I do think I need to clean up a bit of my code with the Tools enum, it's not entirely finished.

@RealMangorage
Copy link
Contributor Author

RealMangorage commented Jun 17, 2025

I did it this way, as to not change any method signatures, just add new ones, as I was not entirely sure how you wanted to deal with it.

I can go thru everything (my impl) and clean it up for you, if you would like to.

Copy link
Member

@Jonathing Jonathing left a comment

Choose a reason for hiding this comment

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

I think this whole bit might be better in an extension. I'm still thinking of potentially an alternate implementation? One that looks like this:

dependencies {
    fgTools.mavenizer 'me.jonathing:mavenizer:0.3.0'
}

To do that, I'd need to make a new extension for DependencyHandler that returns configurations. If you'd like to present your own take on this, feel free.

Comment on lines 90 to 93
project.getConfigurations().register(tool.getConfiguration()) {
it.setTransitive(false) // Cant be transitive, maybe allow it to be later?
it.setVisible(true)
}
Copy link
Member

Choose a reason for hiding this comment

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

For the record, this absolutely can be transitive. Instead of having Tools return a Provider<File>, it would instead be better to use the configuration you created in ForgeGradlePlugin, add the default tool to that configuration, and then use that as the classpath in the task.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah great! I will fix that.

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.

2 participants