modrinth / code

The Modrinth monorepo containing all code which powers Modrinth!
https://modrinth.com
Other
1.01k stars 189 forks source link

Some Mods don't work and Vanilla Game features are broken as theseus ships JRE instead of JDK #1192

Open Dev0Louis opened 6 months ago

Dev0Louis commented 6 months ago

Describe the bug

As the Modrinth launcher uses a JRE by default instrumentation isn't available, but some mods do rely on it to work. Examples are: https://modrinth.com/mod/tameable-axolotls and https://modrinth.com/mod/impirium.

Steps to reproduce

  1. Create new profile with the default java from theseus
  2. Add Tamable Axolotls or Impirium
  3. Launch the Game
  4. Watch it crash

Expected behavior

Theseus should ship a JDK by default as that is what most users have and is required for some extreme modifications of Minecraft.

System information

Linux, but that should be irrelevant

Additional context

No response

brawaru commented 6 months ago

Because Minecraft runs with just JRE, this should be the target for Minecraft mods. If you want to use mods that rely on APIs meant for developers, you are welcome to install a JDK yourself. Most users don't need that.

Gaming32 commented 6 months ago

This is true for 1.16.x and older. Since 1.17, the game uses a full JDK. While it doesn't use any JDK-specific features yet, they officially run on a full JDK. If the Modrinth launcher doesn't do that (like the official launcher does), it's just setting itself up for incompatibilities like these.

Gaming32 commented 6 months ago

Additional comment: the Vanilla game does actually use the JDK. The absence of it might make users confused why the /jfr command throws an error on the Modrinth launcher (and only on the Modrinth launcher).

Dev0Louis commented 6 months ago

Because Minecraft runs with just JRE, this should be the target for Minecraft mods. If you want to use mods that rely on APIs meant for developers, you are welcome to install a JDK yourself. Most users don't need that.

Saying most users don't need it isn't really helping. There are mods that need the JDK and it is used by the minecraft launcher. Mod devs go off what the Minecraft Launcher uses and so should theseus. Making it needlessly break mods that work on the minecraft launcher is not something that should be wanted.

And yeah I can install a jdk, but people that don't even know what JDK or JRE means still pay modded minecraft. And the mods should just work and not need tinkering.

And as @Gaming32 pointed out, it seems to also break vanilla behavior.

Felix14-v2 commented 6 months ago

Some mods are made only for development environment, but we shouldn't turn the launcher into a DE because of these specific mods. However, breaking the vanilla functionality is unacceptable for the launcher, so I support the suggestion.

blryface commented 6 months ago

I don't think the mods linked in the issue are meant to only run on a dev env, and I don't see how or why a mod would be specifically made for a dev env? It may have features that help you develop but it wouldn't make that much sense to make them "only for" a dev env

Felix14-v2 commented 6 months ago

Here's an example from my bookmarks: https://github.com/EZForever/ThatOrThis/pull/27#issuecomment-1609111562 Also iirc CaffeineMC used a mod which allows them to freeze some parts of the rendering engine for debug purposes, but I can't find it

Dev0Louis commented 6 months ago

Some mods are made only for development environment, but we shouldn't turn the launcher into a DE because of these specific mods. However, breaking the vanilla functionality is unacceptable for the launcher, so I support the suggestion.

None of the mods are for "dev env" its just that there are some features in the JDK (Instruments) for runtime class redefinition which are not present in the JRE. These features need to be present for some mods to work.

brawaru commented 6 months ago

I wasn't aware that the MoJREs are JDKs, so yeah, for compatibility reasons it would be better to map MoJREs to Azul's JDK packages, but that's because it seems that Modrinth App is breaking some of the vanilla functionality like /jfr command^1, not because of the mods in question that override superclasses in a seemingly cursed way^2 with dependence on development APIs.

Updated to acknowledge that all MoJREs are JDKs.

Dev0Louis commented 6 months ago

I wasn't aware that the latest MoJRE is a JDK, so yeah, for compatibility reasons it would be better to map MoJRE 21 to Azul's JDK package, but that's because it seems that Modrinth App is breaking some of the vanilla functionality like /jfr command1, not because of the mods in question that override superclasses in a seemingly cursed way2 with dependence on development APIs.

Footnotes

1. https://discord.com/channels/734077874708938864/1061855024252207167/1240516520057110620 [↩](#user-content-fnref-1-2a1cd9a7d048778b424d20d349c8955c)

2. https://github.com/ekulxam/TameableAxolotls/blob/84d1813ebe163e0cf43946f0cc1f506211bcea85/src/main/java/survivalblock/tameable_axolotls/TameableAxolotlsAgent.java#L21 [↩](#user-content-fnref-2-2a1cd9a7d048778b424d20d349c8955c)

Well but there is not other way that I know of to change the inheritance structure of Classes. And do what has been done Another example of a mod using this is impirium as I pointed out above.

That behavior is supported by all other launchers, including the vanilla Default launcher, so it should be too on Modrinth launcher without any tinkering.

And why only 21? 17 is just as affected by this.

Sl3epy commented 3 months ago

Has there been any update to this?