FAForever / downlords-faf-client

Official client for Forged Alliance Forever
https://faforever.com
MIT License
196 stars 121 forks source link

Support Java 16+ #2298

Closed geekingfrog closed 2 years ago

geekingfrog commented 3 years ago

Is your feature request related to a problem? Please describe. Currently, when running the client on linux, it requires java version 15 exactly.

Describe the solution you'd like Arch (my distro) recently upgraded to java 16. Knowing the commitment to backward compatibility of java, I have no reason to suspect java 16 cannot run the client. So perhaps instead of checking for version 15, check for version 15 or higher?

Client version: 1.4.6, gotten from the published released zip file on github

geekingfrog commented 3 years ago

Note, changing the 15 with a 16 in the bash script seems to work.

  if [ "$ver_major" -gt "16" ]; then  # was 15 before
    return;
  fi
germanicianus commented 3 years ago

Which script do you mean? Running the client is quite simple and should be possible with a JRE >= 15. Have a look at this post: https://steamcommunity.com/app/9420/discussions/0/3044978436774077526/?ctp=2#c3044978436774345617

geekingfrog commented 3 years ago

When you download the zip file from the release 1.4.7 and extract, there is a script called downlords-faf-client. I couldn't find the source in this repo. There's a function starting line 99: test_jvm which runs a bunch of check to see if the location of the jre is valid. And towards the end of the function there is this block:

  if [ "$ver_major" = "" ]; then
    return;
  fi
  if [ "$ver_major" -lt "15" ]; then
    return;
  fi

  if [ "$ver_major" = "" ]; then
    return;
  fi
  if [ "$ver_major" -gt "15" ]; then
    return;
  fi

  app_java_home=$test_dir

The two checks -lt "15" and -gt "15" basically force the major version to 15. I tweaked the second condition to allow newer jdk and everything works indeed.

It would be nice to have the script patched so that the next releases work out of the box with newer java versions.

micheljung commented 3 years ago

We need to change (or remove) javaMaxVersion here: https://github.com/FAForever/downlords-faf-client/blob/960c35199420dc3b6401624b5dac9dbeff8bcc08/downlords-faf-client.install4j#L4

I wouln't do it by hand but use the Install4J IDE.

The only issue is that we can't guarantee that newer JDKs will work. E. g. our Java 8 build was not compatible with Java 9. But such changes are very rare so I'd say the gain overweights the risks.

Even better, however, would be to bundle a JRE with the client, like we do for Windows. However, IIRC, this is too much work because of how many distros there are to be supported (and Linux isn't officially supported).

germanicianus commented 3 years ago

geekingfrog, sorry that I didn't notice your answer. micheljung already said everything.

Just some minor addition for easier understanding. install4j is used to bundle the packages to release. It creates the script you modified, so it isn't contained in this repo. The modification micheljung mentioned applies to the configuration file of install4j which tells it on how to create the package.

Theoretically you could run the client without using this script, but then you would need to apply the necessary Java commandline parameters by yourself.

geekingfrog commented 3 years ago

If you're unsure you want to change the maximum supported java version I would understand. In this case, adding a note on the wiki page I followed to get it working is probably sufficient.

Angular-Angel commented 2 years ago

For anyone who finds this after me, I couldn't get the 'line 99: test_jvm' fix to work properly, but when I uncommented the 'INSTALL4J_JAVA_HOME_OVERRIDE=' line up at the top and slammed in my jvm, it worked immediately. I haven't tested it by playing an actual game yet, but I can navigate the client just fine.