Major refactor to Java launcher library

[TheKodeToad]

solonovamax helped a lot with this. Thank you!

Changes:

• Rename enums to SHOUTING_SNAKE_CASE.
• Add more exit codes.
• Switch back from java.util.Logger to a custom one, this time with better code using an enum. The reason for this is the launcher did not colourise the logs like with the old behaviour. While JUL is better in terms of OOP, it doesn't have all the necessary log levels and in the end would require more code to make the log messages look nice. The API is also not the best. Using logging frameworks would just create bloat for no reason and probably create issues.
• Split the onesix launcher into standard and legacy (Standard, Legacy and AbstractLauncher were introduced).
• Reintroduce MultiMC's printing the chosen launcher, but from the C++ code instead.
• Reformat the code using Eclipse. It has a good formatter, okay.
• Updated license headers.
• Split Utils into different classes.
• Use MethodHandle instead of reflection where possible and practical.
• Fix warnings instead of ignoring them. If you use the correct JDK none will be created (for the source code - there is one unrelated one).
• Removed LauncherFactory again. Sorry icelimetea, but I personally think a factory is not needed for two launcher types.
• Remove constructor in NewLaunch. Why have a field when you can have a local variable? I generally also like to avoid instances of the main class.
• Remove feeding the classpath through stdin to the Java launcher. It's already passed into the java executable with the -cp flag, and NewLaunch completely ignores it.

[flowln]

this creates a few new warnings:

org/prismlauncher/impl/OneSixLauncher.java:63: warning: [unchecked] unchecked method invocation: method allSafe in class Parameters is applied to given types
        mcParams = params.allSafe("param", Collections.EMPTY_LIST);
                                 ^
  required: String,List<String>
  found: String,List
org/prismlauncher/impl/OneSixLauncher.java:63: warning: [unchecked] unchecked conversion
        mcParams = params.allSafe("param", Collections.EMPTY_LIST);
                                                      ^
  required: List<String>
  found:    List
org/prismlauncher/impl/OneSixLauncher.java:63: warning: [unchecked] unchecked conversion
        mcParams = params.allSafe("param", Collections.EMPTY_LIST);
                                 ^
  required: List<String>
  found:    List
org/prismlauncher/impl/OneSixLauncher.java:66: warning: [unchecked] unchecked method invocation: method allSafe in class Parameters is applied to given types
        traits = params.allSafe("traits", Collections.EMPTY_LIST);
                               ^
  required: String,List<String>
  found: String,List
org/prismlauncher/impl/OneSixLauncher.java:66: warning: [unchecked] unchecked conversion
        traits = params.allSafe("traits", Collections.EMPTY_LIST);
                                                     ^
  required: List<String>
  found:    List
org/prismlauncher/impl/OneSixLauncher.java:66: warning: [unchecked] unchecked conversion
        traits = params.allSafe("traits", Collections.EMPTY_LIST);
                               ^
  required: List<String>
  found:    List

they should probably be fixed, even if it incurs in some performance overhead. this isn't exactly a hot path anyways :3

[TheKodeToad]

this creates a few new warnings:

org/prismlauncher/impl/OneSixLauncher.java:63: warning: [unchecked] unchecked method invocation: method allSafe in class Parameters is applied to given types
        mcParams = params.allSafe("param", Collections.EMPTY_LIST);
                                 ^
  required: String,List<String>
  found: String,List
org/prismlauncher/impl/OneSixLauncher.java:63: warning: [unchecked] unchecked conversion
        mcParams = params.allSafe("param", Collections.EMPTY_LIST);
                                                      ^
  required: List<String>
  found:    List
org/prismlauncher/impl/OneSixLauncher.java:63: warning: [unchecked] unchecked conversion
        mcParams = params.allSafe("param", Collections.EMPTY_LIST);
                                 ^
  required: List<String>
  found:    List
org/prismlauncher/impl/OneSixLauncher.java:66: warning: [unchecked] unchecked method invocation: method allSafe in class Parameters is applied to given types
        traits = params.allSafe("traits", Collections.EMPTY_LIST);
                               ^
  required: String,List<String>
  found: String,List
org/prismlauncher/impl/OneSixLauncher.java:66: warning: [unchecked] unchecked conversion
        traits = params.allSafe("traits", Collections.EMPTY_LIST);
                                                     ^
  required: List<String>
  found:    List
org/prismlauncher/impl/OneSixLauncher.java:66: warning: [unchecked] unchecked conversion
        traits = params.allSafe("traits", Collections.EMPTY_LIST);
                               ^
  required: List<String>
  found:    List

they should probably be fixed, even if it incurs in some performance overhead. this isn't exactly a hot path anyways :3

I have fixed this now. Even if the performance impact is nothing to lose sleep about (well, it is for me), it's still generally preferred to use emptyList when you want an immutable empty list. Fortunately I managed to keep using this method in a safer way.

Anyway, does anyone have any idea of what I should change with the license headers, if anything?

[txtsd]

java.applet.Applet is deprecated in newer Java versions. Is this required for older modloaders? And can it be replaced with whatever the modern alternative for applets is?

[TheKodeToad]

java.applet.Applet is deprecated in newer Java versions. Is this required for older modloaders? And can it be replaced with whatever the modern alternative for applets is?

Old versions of Minecraft used Java applets so they could be run in web browsers. I think there is a standalone main class though. However, since there are other compatibility issues with newer versions of Java, I can't see many reasons to replace it.

[solonovamax]

Perhaps it might be better to split the applet/legacy launcher and the default launcher into separate classes?

There already exists support for different launcher types, so why not use it?

That way you can @Suppress all instances of deprecated for the entire class, while only affecting the applet code.

[solonovamax]

Also, in the line of some improvements to the java code, why not use the ParseException class here instead of IllegalStateException?

PrismLauncher/libraries/launcher/org/prismlauncher/impl/OneSixLauncher.java

Line 93 in deb42d1

throw new IllegalArgumentException("Unexpected window size parameter value: " + windowParams);

[TheKodeToad]

Okay, I will try to improve the code more - maybe more of a refactor. I was trying to avoid it because other projects (it was mainly one though) didn't like it when I kept adding to my pull requests.

Time to fire up an IDE that isn't totally terrible ;)

[txtsd]

Okay, I will try to improve the code more - maybe more of a refactor. I was trying to avoid it because other projects (it was mainly one though) didn't like it when I kept adding to my pull requests.

Time to fire up an IDE that isn't totally terrible ;)

Take your time and make your changes. We won't merge it until it's deemed ready.

[solonovamax]

Okay, I will try to improve the code more - maybe more of a refactor. I was trying to avoid it because other projects (it was mainly one though) didn't like it when I kept adding to my pull requests.

I was gonna make my own pr with a bunch of code cleanup changes later as well, bc I noticed quite a few things that could be changed

[TheKodeToad]

I think I've done enough for this pull request. I've implemented #291 (comment), as well as replaced some string splitting with substringing (just personal preference, but I think it's best to avoid regex when searching for single characters - and it can have side-effects if the character appears more times than you want). Maybe I'll open one aimed at improving the legacy launcher.

Also, somebody's been here before.

I reverted the change only for one of the fields which is changed.

No idea why re-requesting a review removed txtsd's.

[solonovamax]

few things I just noticed:

1. The license header

   /*
    *  PolyMC - Minecraft Launcher
    *  Copyright (C) 2022 Sefa Eyeoglu <contact@scrumplex.net>
    *  Copyright (C) 2022 Jamie Mansfield <jmansfield@cadixdev.org>
    *  Copyright (C) 2022 TheKodeToad <TheKodeToad@proton.me>
   

   should be changed for

   /*
    *  Prism Launcher - Minecraft Launcher
    *  Copyright (C) 2022 Sefa Eyeoglu <contact@scrumplex.net>
    *  Copyright (C) 2022 Jamie Mansfield <jmansfield@cadixdev.org>
    *  Copyright (C) 2022 TheKodeToad <TheKodeToad@proton.me>
   

2. You removed OneSixLauncher.java from the launcher code, however, I'm pretty sure that the "onesix launcher" is referenced in many other places in the codebase. May want to change that to just reference the "launcher". (Search for "onesix" or "one six" in the codebase)

Also, I'd very much like to contribute some of my own changes to this, but I want to commit them myself, so they're under my name (As they're slightly more significant than the previous changes I suggested). Would that be possible? If so, how do you want me to do that? I could make a fork of your fork, then PR to it, so that way it can be merged into this branch.

[TheKodeToad]

few things I just noticed:

1. The license header
   ```
   /*
    *  PolyMC - Minecraft Launcher
    *  Copyright (C) 2022 Sefa Eyeoglu <contact@scrumplex.net>
    *  Copyright (C) 2022 Jamie Mansfield <jmansfield@cadixdev.org>
    *  Copyright (C) 2022 TheKodeToad <TheKodeToad@proton.me>
   ```
   
   
       
         
       
   
         
       
   
       
     
   should be changed for
   ```
   /*
    *  Prism Launcher - Minecraft Launcher
    *  Copyright (C) 2022 Sefa Eyeoglu <contact@scrumplex.net>
    *  Copyright (C) 2022 Jamie Mansfield <jmansfield@cadixdev.org>
    *  Copyright (C) 2022 TheKodeToad <TheKodeToad@proton.me>
   ```

2. You removed `OneSixLauncher.java` from the launcher code, however, I'm pretty sure that the "onesix launcher" is referenced in many other places in the codebase. May want to change that to just reference the "launcher". (Search for "onesix" or "one six" in the codebase)

Also, I'd very much like to contribute some of my own changes to this, but I want to commit them myself, so they're under my name (As they're slightly more significant than the previous changes I suggested). Would that be possible? If so, how do you want me to do that? I could make a fork of your fork, then PR to it, so that way it can be merged into this branch.

Oh dear, this is getting confusing. I'll give you push access. I'm not sure if people normally do that, but since you're not part of the Prism Launcher organisation it would be that or a fork fork (and this repo is already a fork fork).

[solonovamax]

👍 alright

I pinky promise not to git push --force

[Scrumplex]

emptyList when you want an immutable empty list

We used to use emptyList() for everything but some lists can be modified conditionally so it should actually all be mutable.

EDIT: nvm you already saw my commit :D

[solonovamax]

@TheKodeToad I was going to keep LegacyUtils, because I wanted to move a bit of the messy logic happening in the LegacyLauncher into that, to clean it up.

You alright if I revert that commit later?

[solonovamax]

Also, avoid force pushes if possible, because it forces me to have to rebase/re-clone my entire branch

[TheKodeToad]

@TheKodeToad I was going to keep LegacyUtils, because I wanted to move a bit of the messy logic happening in the LegacyLauncher into that, to clean it up.

You alright if I revert that commit later?

It seems weird to couple a class with a utils one. I feel like it would be more logical to move some of the messy logic into other methods.

Also, avoid force pushes if possible, because it forces me to have to rebase/re-clone my entire branch

Oops, sorry. I had to because I forgot signing off. I think I've got the hang of it now.

Edit: OMG I keep pushing and then realising my mistake. Still, it shouldn't make a difference unless you already have the commits on your local copy - rebasing is normally required even without force pushing.

[solonovamax]

Edit: OMG I keep pushing and then realising my mistake. Still, it shouldn't make a difference unless you already have the commits on your local copy - rebasing is normally required even without force pushing.

If you want to make sure to sign every single commit, add the following to your .gitconfig:

[user]
    name = [YOUR NAME HERE]
    email = [YOUR EMAIL HERE]
    signingkey = [YOUR SIGNING KEY HERE]
[commit]
    gpgsign = true

Then, it will sign every single one of your commits by default (so you don't need to specify -S on the git commit command.)

[TheKodeToad]

I have managed to squash and drop a lot of commits. It should be ready for review now. The code is at a stage where to see something wrong with it you'd have to be not me.

[Scrumplex]

Thanks a lot!