--jvm is ignored / crash at launch forge installer

[rutexd]

Seems like jvm argument is ignored. I got installed multiple versions of java. After i tried to launch forge 1.7.10 profile, i got crash on install process. 1.7.10 installer is pretty old and it crashes on new java versions. At least it just stuck from gui and worked after launch from right java version (for example 11)

After i tried specify three different java versions to jvm argument, nothing happened and always was same crash. So i assume there something wrong with it. I tried also to set custom Path and JAVA_HOME variables, but it didnt anything too.

My log.

C:\Users\xxx>portablemc start forge:1.7.10 "--jvm=C:\Program Files\Java\jdk-11.0.15\bin\java.exe"
[  OK  ] Resolved forge 1.7.10-10.13.4.1614, downloading installer and parent version.
[  OK  ] Downloaded 1/1 files, 3.6MB in 7.2s (no error).
[FAILED] Invalid command to start forge installer wrapper (should not happen, contact maintainers, this can also happen if the installer fails).
===================
info: using install runner V1InstallRunner
Exception in thread "main" java.lang.NoSuchMethodError: net.minecraftforge.installer.VersionInfo.getMinecraftFile(Ljava/io/File;)Ljava/io/File;
        at portablemc.wrapper.V1InstallRunner.validate(V1InstallRunner.java:26)
        at portablemc.wrapper.Main.main(Main.java:30)

===================

[rutexd]

Seems like --jvm is ignored only on install process. It launches MC from specific version but not an installer, i guess...

[Ristovski]

For reference: https://github.com/mindstorm38/portablemc/blob/master/src/forge/portablemc_forge/wrapper/src/main/java/portablemc/wrapper/V1InstallRunner.java#L26

[rutexd]

Well, i saw this too but setting any python (and python itself) env for me its a nightmare, so i cant test this. Can you reproduce this, maybe? by putting java 19 to a current path?

[Ristovski]

@mindstorm38 I assume V1InstallRunner is broken for some reason. forge 1.7.10-10.13.4.1614 does not have a getMinecraftFile method (checked by decompiling the installer with cfr).

I also am not sure why the local override of VersionInfo.java returns null, is it just to satisfy the compiler?

[rutexd]

I see the whole problem now. Sorry for confusing with label about java version.

Why do we doing https://github.com/mindstorm38/portablemc/blob/master/src/forge/portablemc_forge/wrapper/src/main/java/portablemc/wrapper/V1InstallRunner.java#L42? isnt forge handles this automatically?

[mindstorm38]

I also am not sure why the local override of VersionInfo.java returns null, is it just to satisfy the compiler?

Almost all classes are placeholders just for giving methods signatures, they are not actually included in the final JAR. This add-on is a java nightmare... I'll try to fix this, thanks for this detailed report.

Why do we doing https://github.com/mindstorm38/portablemc/blob/master/src/forge/portablemc_forge/wrapper/src/main/java/portablemc/wrapper/V1InstallRunner.java#L42? isnt forge handles this automatically?

I don't remember, the comment isn't very helpful here...

[rutexd]

Well, i managed to replace broken getMinecraftFile() https://github.com/rutexd/portablemc/commit/3d0225ea66ffe259cb33acc23f60213bde987653 But its still somehow broken. Maybe this will be usefull, if i understand everything right.

Install:

(pmc) @rutexd ➜ .../src/forge/portablemc_forge/wrapper (fix/ForgeInstaller) $ portablemc start forge:1.12
[  OK  ] Resolved forge 1.12-14.21.1.2387, downloading installer and parent version.
[  OK  ] Downloaded 1/1 files, 5.1MB in 1.1s (no error).                        
[      ] Running installer (can take few minutes)...Starting forge installer wrapper...
Installer file: /home/codespace/.minecraft/versions/forge-1.12-14.21.1.2387/installer.jar
Wrapper jar file: /workspaces/portablemc/src/forge/portablemc_forge/wrapper/target/wrapper.jar
Main dir: /home/codespace/.minecraft
Version id: forge-1.12-14.21.1.2387
JVM exec: /home/codespace/.minecraft/jvm/jre-legacy/bin/java
[  OK  ] Forge installation done.                   
[ INFO ] Consider supporting the forge project through https://www.patreon.com/LexManos/.
[FAILED] start.forge.error.not_installed 

Run

(pmc) @rutexd ➜ .../src/forge/portablemc_forge/wrapper (fix/ForgeInstaller) $ portablemc start forge:1.12
[ INFO ] Consider supporting the forge project through https://www.patreon.com/LexManos/.
[FAILED] start.forge.error.not_installed 

[mindstorm38]

I'm working on this one...

[mindstorm38]

I'm wondering if I can do better by just manually extracting the profile and some required libraires instead of running the installer...

[rutexd]

Why running installer isn't good? I mean... If something going go break again like this was now, you have to rewrite anything again... Otherwise I guess you just have to adapt arguments or something like that...

[mindstorm38]

It seems way either to me because reverse engineering of each installer version is tedious, and for now I've to make some weird reflection and compilation tricks to get the wrapper to work. And even with this, old installers make unwanted UI to appear, and I can't detect this, same for weird download errors.

My idea is to directly use the installer's install_profile.json, it seems so much easier particularly for old versions (prior to 1.13, because no download is required), but can be a bit annoying for modern versions that apply to have a sequence of binaries to generate final jar. The important point is that I only have 2 formats to manage, and I can do this from python (no multi lang project).

[mindstorm38]

The latest version of the forge add-on 2.0.2 should partially fix the issues. It's just some fixes with the wrapper. It's not yet what I've talked about in my previous message. This will probably come with portablemc v4

[rutexd]

Sounds great, then i will wait for v4 👍 🥇

[necauqua]

So I actually had the issue of "forge installer does not use --jvm and unconditionally downloads the mojang jvm", which does not work at all on NixOS, and I fixed it by just changing this line to self.jvm_exec = context.ns.jvm

Ideally, the forge plugin could also add a --forge-installer-jvm parameter and then try to use them in order --forge-installer-jvm -> --jvm -> mojang jvm or something like that.

[mindstorm38]

In the future, the installer will no longer run the JAR file! So the JVM will be irrelevant to the installation

[mindstorm38]

So the JVM will be irrelevant to the installation

I was completely wrong because many post processor JAR executables must be run. But this seems to work just fine with the Mojang's JVM associated to each version. So as long as you have the correct JVM for each Minecraft's version, it should work.

[mindstorm38]

I released the first pre-release: https://github.com/mindstorm38/portablemc/issues/146#issuecomment-1656874939 Can you tell me if this fixes your problem?

[rutexd]

Just tested v.4rc2. Vanilla works excelent. Forge looks broken for me. Maybe i have to clear cache or something?

1.7.10 still isnt working 1.18.2 wasnt working first time. I run start command some times (see screenshoots) and for third time it was working.

1.7.10

1.18.2

[mindstorm38]

For 1.7.10, you'll need to wait for rc3 since I found some issues with how the metadata is installed, that wasn't causing problem before because I was wrong about libraries resolution. For 1.18.2, have you tried removing the version and then installing it again?

Forge is definitely a mess, I would like to automatically fix versions if they are not working as in your case, but it's complicated to validate forge versions...

Connection errors may be from your side, but idk...

The charmap unicode error is still a big problem, because I don't want to prevent the game from running even if it's not outputting correct unicode!

[mindstorm38]

I fixed the charmap encoding problem.

[mindstorm38]

I'm afraid of these connection errors, I get these randomly in the test workflow...

[mindstorm38]

I added better error logging, and the error causing all this mess is CannotSendRequest... I'll try to understand why

Edit (interesting):

[rutexd]

I'm afraid of these connection errors, I get these randomly in the test workflow...

I guess this is fine, can happen sometimes. I guess good solution to add retry in case of failed download

[mindstorm38]

It's already the case, it's why I was a bit worried ahah... But I just implemented a good solution consisting of, in addition to retrying, re-opening a new HTTP connection before retrying. I want to release rc3 this evening.

[mindstorm38]

Here is pre-release 3, https://github.com/mindstorm38/portablemc/issues/146#issuecomment-1657250298

After manually removing directories for those problematic versions (in versions dir), could you test this new version @rutexd?

[rutexd]

Installed RC3, deleted whole .minecraft dir for sure.

Downloading looks better now, at least almost no significant problems or errors. Maybe its a nice idea to add line like "downloading X", where X = current file or url (or even together), since i noticed small freeze on downloading last files. If this wont break any design or jump inside whole console.

Vanilla 1.7.10, forge 1.7.10, vanilla 1.18.2, forge 1.18.2, fabric 1.18.2 looks like working fine.

In my shell doesnt working CTRL+C. Idk, if its related to my shell or not.

Also, if you kill running process of portablemc while mc is running, mc will continue to run. Maybe its good idea to kill mc together with launcher? or with adding extra option...

And one last thing, maybe its cool to add a "latest" alias? like "mc start latest:forge"? since "mc start forge" or fabric producing [FAILED] Version forge not found...

Rest looks very good for me and you did amazing job :)

--------- UP ---------

looks like CTRL+C doesnt work when game is running and on some certain launcher stages, but hard to debug on my side.

[mindstorm38]

Also, if you kill running process of portablemc while mc is running, mc will continue to run. Maybe its good idea to kill mc together with launcher? or with adding extra option...

It's actually really, really complicated to do with Python, because we can't catch process' kill that easily. For the Ctrl-C issue I need to test this on Windows, since I'm developing on Linux.

And one last thing, maybe its cool to add a "latest" alias? like "mc start latest:forge"? since "mc start forge" or fabric producing [FAILED] Version forge not found...

You can already do that! By using ... start forge: or ... start fabric:.

[rutexd]

Doesn't python have any exit events or something? Under Linux it's can be hard little bit because of hard kill but for kill command?

Well... Sad. But it's working so it's already good. These are just small improvements

[mindstorm38]

I added a bunch of examples in README: https://github.com/mindstorm38/portablemc/tree/v4-rc#start-minecraft

[mindstorm38]

Ok I can reproduce your issue on Windows with Ctrl-C, I'll try to fix it.

[rutexd]

btw,

https://docs.python.org/2.7/library/multiprocessing.html#multiprocessing.Process.daemon

this looks like this is what we need to kill subprocess

[mindstorm38]

Sadly, this is no longer available in Python versions the launcher support, and I'm not sure if this would fix the problem. But I found a fix, I tested it on Windows, and it works great, and it continues to work on Linux the same way as before.

I'll wait tomorrow to see if no more issues are discovered, and I'll publish the final v4.0.0 version.

[mindstorm38]

Released! Changelog: https://github.com/mindstorm38/portablemc/releases/tag/v4.0.0 Release page: https://pypi.org/project/portablemc/4.0.0/ Install with pip install portablemc==4.0.0