MinecraftForge / ForgeGradle

Minecraft mod development framework used by Forge and FML for the gradle build system
GNU Lesser General Public License v2.1
514 stars 443 forks source link

Set mappings for reobf extension to MCP->SRG to default #615

Closed liach closed 4 years ago

liach commented 5 years ago

https://github.com/MinecraftForge/ForgeGradle/blob/5997fd1e800259fe5f143a077d25ae3e512d86bd/src/userdev/java/net/minecraftforge/gradle/userdev/UserDevPlugin.java#L80-L86

Though the user dev plugin automatically adds reobf/rename tasks for each jar task in a forgegradle project, it only does such task additions in a half-done fashion. It sets up a dependency on the target jar task but fails to specify a dependency on the mcp -> srg mapping generation task and fails to supply the mcp -> srg mapping file to the rename task.

Suggested change:

 project.afterEvaluate(p -> { 
     Task jar = project.getTasks().getByName(jarName); 
     if (!(jar instanceof Jar)) 
         throw new IllegalStateException(jarName + "  is not a jar task. Can only reobf jars!"); 
     task.setInput(((Jar) jar).getArchivePath()); 
     task.dependsOn(jar); 
     Task createMcpToSrg = project.getTasks().getByName("createMcpToSrg");
     task.setMappings(createMcpToSrg.getOutputs().getFiles().getSingleFile());
     task.dependsOn(createMcpToSrg);
 });

Currently, a user must declare as such to make reobf work: (using shadowJar as an example)

reobf {
    shadowJar {
        dependsOn createMcpToSrg
        mappings = createMcpToSrg.outputs.files.singleFile
    }
}

And the dependsOn and mappings must be specified for each of the jar task that needs to be reobfuscated.

With this improvement, it can be shortened to

reobf {
    shadowJar
}

instead.

Given this extension is already named reobf, we can simply assume it is a direct mapping from MCP to Searge names. In other cases, users should just create their own RenameJar tasks.

LexManos commented 5 years ago

We cant assume it's a direct mapping from anything to anything. The point is to allow people to create the reobf tasks with whatever mappings you want. However yes, we could expand this to have a default. We'd just need to add a configure step to the reobf create function...

Also would be worth moving the register function for createMcpToSrg to above the anon class sso we can reference it directly instead of the roundabout getByName.

So you can do both:

reobf {
    srgJar,
    notchJar {
        dependsOn createMcpToNotch
        mappings = createMcpToNotch.output
    }
}

Note, your change wont work as its done in the afterEvaluate. Which wont allow people to override it. It must be done in the configurate state.