[7.0] Minecraft 1.20.2 Forge 48.1 fails to runClient

[Jonathing]

I'm able to reproduce this using a clean, minimal FG7 MDK using 1.20.2 Forge 48.1.

Here's the MDK I used to test this: fg7.0.7-mc1.20.2-mdk-test.zip
And here's the runClient log: https://gist.github.com/Jonathing/1774fa157daed7dac1b9f6d7d1c378b8

[Jonathing]

From @LexManos on Discord:

bsl: file '/home/**/.gradle/caches/modules-2/files-2.1/org.ow2.asm/asm-analysis/9.6/9ce6c7b174bd997fc2552dff47964546bd7a5ec3/asm-analysis-9.6.jar' ignored because filename starts with 'asm-'

Love that old hacky shit

[LexManos]

Okay so looking more into it, that file is supposed to be ignored because the ignore list specifically species asm-analysis.
I think I understand the issue.
Basically, UserDevConfigV2 introduced support for launching the process in module mode (out favorite thing yay!)
Doing this by adding the modules field.
This is not taken into account anywhere in Mavenizer or SlimeLauncher.

Here is an example the relevant parts of the UserdevConfig:

{
  "modules": [
    "cpw.mods:bootstraplauncher:1.1.2",
    "cpw.mods:securejarhandler:2.1.10",
    "org.ow2.asm:asm-commons:9.7.1",
    "org.ow2.asm:asm-util:9.7.1",
    "org.ow2.asm:asm-analysis:9.7.1",
    "org.ow2.asm:asm-tree:9.7.1",
    "org.ow2.asm:asm:9.7.1",
    "net.minecraftforge:JarJarFileSystems:0.3.19"
  ],
  "runs": {
    "server": {
      "jvmArgs": [
        "-Djava.net.preferIPv6Addresses\u003dsystem",
        "-p",
        "{modules}",
        "--add-modules",
        "ALL-MODULE-PATH",
        "--add-opens",
        "java.base/java.util.jar\u003dcpw.mods.securejarhandler",
        "--add-opens",
        "java.base/java.lang.invoke\u003dcpw.mods.securejarhandler",
        "--add-exports",
        "java.base/sun.security.util\u003dcpw.mods.securejarhandler",
        "--add-exports",
        "jdk.naming.dns/com.sun.jndi.dns\u003djava.naming"
      ]
    }
  }
}

And a runClient test:

-Dforge.enableGameTest=true
-DignoreList=JarJarFileSystems-,asm-,asm-analysis-,asm-commons-,asm-tree-,asm-util-,bootstrap-,client-extra-,failureaccess-,fmlcore-,forge-,guava-,javafmllanguage-,lowcodelanguage-,mclanguage-,securemodules-,unsafe-
-Djava.net.preferIPv6Addresses=system
-DlegacyClassPath.file={minecraft_classpath_file}
-DmergeModules=jna-5.10.0.jar,jna-platform-5.10.0.jar
-p {modules}
--add-modules ALL-MODULE-PATH
--add-exports jdk.naming.dns/com.sun.jndi.dns=java.naming
-Dfile.encoding=UTF-8
-Duser.country=US
-Duser.language=en
-Duser.variant
-cp <SNIPPED> net.minecraftforge.launcher.Main
--main net.minecraftforge.bootstrap.BootstrapLauncher
--cache C:\Users\Lex\.gradle\caches\minecraftforge\forgegradle\slime-launcher\cache\net\minecraftforge\forge\1.20.2-48.1.0
--metadata Z:\test\asdf\.gradle\mavenizer\repo\net\minecraftforge\forge\1.20.2-48.1.0\forge-1.20.2-48.1.0-metadata.zip
--
--launchTarget forge_userdev_client
--version MOD_DEV
--assetIndex {asset_index}
--assetsDir {assets_root}
--gameDir .
--fml.forgeVersion 48.1.0
--fml.mcVersion 1.20.2
--fml.forgeGroup 

Note the lack of filling in {modules}, and {minecraft_classpath_file}

For completeness sake a looked into where FG6 configures the tokens. And found here and here and here and here

So i think we need to support:

key	value
source_roots	The desitnation directories of each sourceset added to this run config. This is the old hacky thing for people who dont merge their sourcesets. Or use older versions that manually builds them. May not actually need this.
runtime_classpath	A string version of the entire classpath
minecraft_classpath	A string version of just the Minecraft Dependency (Forge, and its dependencies)
runtime_classpath_file	File version of runtime_classpath because of command length constraints
minecraft_classpath_file	File version of minecraft_classpath because of command length constraints
asset_index	Same as Minecraft Launcher value - The index id number for game assets
assets_root	Same as Minecraft Launcher value - The root 'assets' folder
natives	Same as Minecraft Launcher value - The extracted natives folder for old versions which don't self extract
mc_version	The simple vanilla Minecraft Version - 1.21.11
mcp_version	The full MCP Config version - 1.21.11-000000000.000000
mcp_mappings	I think this {channel}_{version}
mcp_to_srg	Despite the name this is set to createSrgToMcp.getOutput().get().getAsFile().getAbsolutePath() so.. Srg -> MCP .srg mapping file.
modules	The classpath of a detatched configuration containing the dependencies from UserDevV2.modules

It also manually forces the environment variable MOD_CLASSES to {source_roots} which then gets filled by these tokens
It looks like you support asset_index and assets_root in SlimeLauncher for arguments. But I didn't see any environment/jvm token replacements.

[Jonathing]

Talk about annoying. Let me think on this for a bit. In theory it shouldn't be too hard but it depends on how much Slime Launcher actually needs to transform itself.

[LexManos]

Ya, SlimeLauncher should be happy in module or non-module land. So that side should be fine just being a shunt as before.