add a system property to enable fixing package access regardless of target namespace #895
Conversation
SpaceWalkerRS
commented
Jan 23, 2024
SpaceWalkerRS
commented
- independent of game provider
- the launch arg is parsed after the game provider is initialized (so that the mapping configuration is loaded)
|
We usually use system properties for these smaller tweaks. Can you say more about the application/purpose? |
|
Obfuscation can be quite inconsistent in older versions of Minecraft, sometimes obfuscated classes are placed in the same package as the main classes and classes can actually move package between versions. For this reason I'd like to loosen the restrictions on which classes are given intermediary names to make sure all these classes are placed in the intermediary target package. For that to work in production Loader will need to fix illegal access errors this causes. I'm sure it's possible to implement this as a system property too. The reason I went with a launch arg is that it's easy to add that to the launcher profile json. I'm not sure how to do that for system properties though. |
|
System properties are part of the jvm arguments, like -Dname=value, before the jar file or main class name. The vanilla launcher supports them. If you really need the argument, make sure to declare it in the Arguments class and keep the usage consistent with ADD_MODS. I'd however prefer the system property approach. I btw intend to replace the naive "make everything public" with Tiny Remapper's dynamic access fixing at some point - once that's fully working. |
|
Using a launch arg like this, will likely cause the game to crash on startup due an unknown argument, or at best log an error. A system prop the way to go. |
|
alrighty, I'll change it up to a system property 👍 |
SpaceWalkerRS
force-pushed
the
package-access
branch
from
b9d6056 to
6fa0c83
Compare
SpaceWalkerRS
changed the title
| public static byte[] transform(boolean isDevelopment, EnvType envType, String name, byte[] bytes) { | ||
| boolean isMinecraftClass = name.startsWith("net.minecraft.") || name.startsWith("com.mojang.blaze3d.") || name.indexOf('.') < 0; | ||
| boolean transformAccess = isMinecraftClass && FabricLauncherBase.getLauncher().getMappingConfiguration().requiresPackageAccessHack(); | ||
| boolean transformAccess = isMinecraftClass && (FIX_PACKAGE_ACCESS || FabricLauncherBase.getLauncher().getMappingConfiguration().requiresPackageAccessHack()); |
There was a problem hiding this comment.
I would move the system property into MappingConfiguration, and make requiresPackageAccessHack() return true when the system prop is set.
If requiresPackageAccessHack() had multiple places it was being called they would all need changing.
