search

SKCraft / Launcher

🚀 Distribute your Minecraft modpacks with a custom launcher
Other
620 stars 433 forks source link

Doesn't warn about java version in some cases #473

Open shinji257 opened 2 years ago

shinji257 commented 2 years ago

The following cases it doesn't seem to trigger the warning (when expected). For verification I tested with the code here and not my own modified code.

If there are no java versions actually installed and it was called via binary directly. In addition, both fallback and instance specified versions would be empty. It just tries to execute it directly (generating an error if you needed Java 16 or 17 because -p option may be missing)

If there isn't a new enough Java version installed for the version of minecraft you are playing. You launch with Java 8 (however you do it) but need Java 17. You may have Java 11 installed. It instead tries to load with the version that you are using to launch the launcher with and doesn't warn at all. It does run fine if Java 17 is installed though.

In both cases the expected behavior is to warn that you don't have a proper version of Java installed for the version of Minecraft that is attempted to be run and to let the user know what version to install. I had done my own testing and found that getRuntime was null when it didn't get a proper version from any other source and comments indicate it should be using the PATH located java but at least on Windows it doesn't. If it did then during my testing (Scenario: Java 8 and 11 installed, running launcher on 8 but pack needs 17 -- expecting prompt indicating 11 found, need 17) it would have tried to run the pack with Java 11 since that was the one available via PATH.

Last but not least this is a side one. When the prompt does come up hitting cancel typically ends up spawning a new instance of the main dialog with the list of packs and leaves the old one around. In many cases you don't even notice unless you moved the original one to a different spot on screen as it comes up in the exact spot as the original one came in.

hedgehog1029 commented 2 years ago

The cancel into new launcher window is a bug, I'll fix that.

Right now the rest is "intended" in the sense that I generally can't detect anything about random Java binaries. The output of java -version isn't very consistent; the auto-detection relies on the release files to detect the java version. I'll look into expanding the scope of the warning further, though.

shinji257 commented 2 years ago

Alright and I understand. It will likely be "good enough" if you can at least check for the existance of a release file in a parent folder from where the binary exists and use that data. In all my testing it does exist. Installed or unzipped.