Skip to content

Conversation

@LexManos
Copy link
Member

@LexManos LexManos commented Feb 11, 2026

The initial design is to allow easier access to the mapping files for versions of minecraft that need renaming.

This is reliant on finalizing MinecraftForge/MinecraftMavenizer#15
Currently the json is just a Map<String, String> seems to work fine tho.

Changes the obfusication example from:

renamer.classes(jar) {
    mappings('net.minecraft:mappings_official:1.21.10-20251007.101210:map2srg@tsrg.gz')
}

to

renamer.classes(jar) {
    mappings(minecraft.instance().toSrg.get())
}

or

renamer.classes(jar) {
    map.from minecraft.instance().toSrgFile
}

The names of everything is just a quick pass, needs javadocs, etc..
This is a proof of concept so point out anything major you see wrong.

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.

Looks good so far. Losing focus since it's getting late at night so I'd like to wait until I can do a second pass tomorrow morning.

Please fix the typos I pointed out. There aren't many.


@Override
public Provider<File> getToSrgFile() {
return getToSrg().map(this.extension.getProject()::file);
Copy link
Member

Choose a reason for hiding this comment

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

this.extension.getProject()::file makes this illegal to be called during task execution. If it's not used in our tasks, in the future it would be most prudent to turn these into properties, since they can be finalized at the end of configuration time without hassle.

Copy link
Member Author

Choose a reason for hiding this comment

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

As stated on discord, this is intended to be used as inputs to tasks, not in the task body. So in theory this should be perfectly safe and if it isn't then its on the modder for using it wrong.

private Map<String, String> invoke() {
if (this.map == null) {
valueSource.get(); // Execute Mavenizer, probably called before, but just be sure.
this.map = (Map<String, String>) new JsonSlurper().parse(this.jsonFile, "UTF-8");
Copy link
Member

Choose a reason for hiding this comment

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

If we are using Gson in Mavenizer, we should prefer using it in here as well. This is fine for now, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didnt want to add extra deps to FG as we are not using GSON for anything else currently.

Copy link
Member

Choose a reason for hiding this comment

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

FG already bundles Gson as it is a transitive dependency of JSON Data Utils. But not a big deal, we can revisit this as needed.

@LexManos
Copy link
Member Author

LexManos commented Feb 11, 2026

Tempted to make MinecraftExtensionForProject extend MavenizerInstance to remove the need for the instance()

Functionally this works, just need to iterate the API as it exposes some that we need to be happy with.
Notiby:
minecraft.dependency functions get a String name first parameter, that defaults to default.
This name is the counterpart to minecraft.instance(String name) which gives you access to MinecraftInstance which is where all the getters for the output-json.

  • getMappingVersion()
    The finalized version for the mapping, suitable for the dependency attributes, this is to expose the fixed-up parchment version string
  • getDependency
    The dependency object mainly I exposed this incase I wanted to change the api to something like:
    minecraft.register("default", "net.minecraftfroge:forge:123")
    dependencies {
       implementation minecraft.instance().dependency
    }
  • getToSrg() / getToObf()
    The artifact published by Mavenizer for mapping files to convert Mapped -> Srg/Obf.
    Ends up being something like net.minecraft:mappings_{channel}:{minecraft}-{mcp}:map2srg@tsrg.gz
  • getToSrgFile() / getToObfFile()
    The full path to those artifacts, right now its in the cache folder not the output. But honestly it doesn't matter. This just allows for bypassing the gradle dependency resolution chain.

@Jonathing Jonathing changed the title Add support for mavenizer output json, and expose mappings helper methods [7.0] Add support for mavenizer output json, and expose mappings helper methods Feb 11, 2026
@Jonathing
Copy link
Member

I've made a couple of notable changes to this PR. Minor, but substantial.

  • MavenizerInstance now extends ProviderConvertible<ExternalModuleDependency>, which means that it itself can be used as the dependency in the dependencies { } block.
    • All minecraft.dependency methods now return MavenizerInstance instead of Provider<ExternalModuleDependency>.
    • MavenizerInstance#getDependency -> MavenizerInstance#asProvider.
  • MinecraftExtensionForProject#instance -> MinecraftExtensionForProject#getDependency
    • This one is just a personal preference thing. Makes it cleaner and more consistent.

These changes change your proposed API from this:

renamer.classes(jar) {
    mappings(minecraft.instance().toSrg.get())
}

to this:

renamer.classes(jar) {
    mappings(minecraft.dependency.toSrg.get())
}

or this:

def minecraftDep = minecraft.dependency(libs.forge)

dependencies {
    implementation(minecraftDep)
}

renamer.classes(jar) {
    mappings(minecraftDep.toSrg.get())
}

@Jonathing Jonathing self-assigned this Feb 11, 2026
@Jonathing Jonathing added this to the 7.1.0 milestone Feb 11, 2026
@Jonathing Jonathing merged commit ddf1c58 into MinecraftForge:FG_7.0 Feb 11, 2026
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