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

[FG3.0] [Mac] GLFW windows may only be created on the main thread #540

Closed GoryMoon closed 5 years ago

GoryMoon commented 5 years ago

I encountered an issue with mac and trying to run 1.13. When trying to start the client everything runs until the game tries to create the window and then the following crash happens: https://haste.gorymoon.se/haqusubuvo.mccrash

The fix for this is to include -XstartOnFirstThread in the JVM args when starting the client. I tested this by adding the JVM args to the task in FG3.0 in UserDevPlugin.java and it works as intended.

task.setJvmArgs(Collections.singleton("-XstartOnFirstThread"));

Because I don't know if you want to add JVM args like you do normal args and environment variables or just add a special case I didn't make a pull for it. Tested by loading FG as a Composite Build into a mdk workspace.

The code in LWJGL that needs this is in EventLoop.java

Versions

Forge: 24.0.55-1.13-pre Mappings: 20181008-1.13 FG: Latest from FG https://github.com/MinecraftForge/ForgeGradle/commit/56559d371b402463854bf667d7f05d501cf94a38

Pokechu22 commented 5 years ago

This argument is specified in the 1.13.2 version manifest as applying to mac only, with some other arguments only applying on other platforms:

{
  "arguments": {
    "game": ["-snip-"],
    "jvm": [
      {
        "rules": [
          {
            "action": "allow",
            "os": {
              "name": "osx"
            }
          }
        ],
        "value": [
          "-XstartOnFirstThread"
        ]
      },
      {
        "rules": [
          {
            "action": "allow",
            "os": {
              "name": "windows"
            }
          }
        ],
        "value": "-XX:HeapDumpPath=MojangTricksIntelDriversForPerformance_javaw.exe_minecraft.exe.heapdump"
      },
      {
        "rules": [
          {
            "action": "allow",
            "os": {
              "name": "windows",
              "version": "^10\\."
            }
          }
        ],
        "value": [
          "-Dos.name=Windows 10",
          "-Dos.version=10.0"
        ]
      },
      {
        "rules": [
          {
            "action": "allow",
            "os": {
              "arch": "x86"
            }
          }
        ],
        "value": "-Xss1M"
      },
      "-Djava.library.path=${natives_directory}",
      "-Dminecraft.launcher.brand=${launcher_name}",
      "-Dminecraft.launcher.version=${launcher_version}",
      "-cp",
      "${classpath}"
    ]
  }
}

Ideally forgegradle should use those values directly. The other ones are less important (other than the stack size one for 32-bit OS's) but probably should also be respected.

GoryMoon commented 5 years ago

Noticed this was still open, this has been fixed for a while now.

@JDLogic Also with the new launcher the -Xss1M in the manifest is correctly matched on the system launch.diff, it was recognized as an issue in the launcher MCL-10408