Skip to content

Move builtin transform selection to game provider #1056

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 2, 2025
Merged

Move builtin transform selection to game provider #1056

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7b7da6
Move builtin transform selection to game provider
Player3324 Jun 22, 2025
e801471
Add trailing separator to classic and pre-classic package tests
Player3324 Jun 22, 2025
27166db
Add documentation, rename to WIDEN_ALL_PACKAGE_ACCESS
Player3324 Jun 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions minecraft/src/main/java/net/fabricmc/loader/impl/game/minecraft/MinecraftGameProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -159,6 +160,29 @@ public boolean requiresUrlClassLoader() {
return hasModLoader;
}

@Override
public Set<BuiltinTransform> getBuiltinTransforms(String className) {
boolean isMinecraftClass = className.startsWith("net.minecraft.") // unobf classes in indev and later
|| className.startsWith("com.mojang.minecraft") // unobf classes in classic
|| className.startsWith("com.mojang.rubydung") // unobf classes in pre-classic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these both end with . to be consistent with the lines above and below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should, thanks for spotting

|| className.startsWith("com.mojang.blaze3d.") // unobf blaze3d classes
|| className.indexOf('.') < 0; // obf classes

if (isMinecraftClass) {
if (FabricLoaderImpl.INSTANCE.isDevelopmentEnvironment()) { // combined client+server jar, strip back down to production equivalent
return TRANSFORM_WIDENALL_STRIPENV_CLASSTWEAKS;
} else { // environment specific jar, inherently env stripped
return TRANSFORM_WIDENALL_CLASSTWEAKS;
}
} else { // mod class TODO: exclude game libs
return TRANSFORM_STRIPENV;
}
}

private static final Set<BuiltinTransform> TRANSFORM_WIDENALL_STRIPENV_CLASSTWEAKS = EnumSet.of(BuiltinTransform.WIDEN_ALL_ACCESS, BuiltinTransform.STRIP_ENVIRONMENT, BuiltinTransform.CLASS_TWEAKS);
private static final Set<BuiltinTransform> TRANSFORM_WIDENALL_CLASSTWEAKS = EnumSet.of(BuiltinTransform.WIDEN_ALL_ACCESS, BuiltinTransform.CLASS_TWEAKS);
private static final Set<BuiltinTransform> TRANSFORM_STRIPENV = EnumSet.of(BuiltinTransform.STRIP_ENVIRONMENT);

@Override
public boolean isEnabled() {
return System.getProperty(SystemProperties.SKIP_MC_PROVIDER) == null;
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/net/fabricmc/loader/impl/game/GameProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.Set;

import net.fabricmc.loader.api.metadata.ModMetadata;
import net.fabricmc.loader.impl.game.patch.GameTransformer;
Expand All @@ -38,6 +39,13 @@ public interface GameProvider { // name directly referenced in net.fabricmc.load
Path getLaunchDirectory();
boolean isObfuscated();
boolean requiresUrlClassLoader();
Set<BuiltinTransform> getBuiltinTransforms(String className);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this a default method for backwards compat? Other games couldn't use transforms previously so an empty set would make sense.

Doesn't really matter too much though.

Copy link
Contributor Author

@Player3324 Player3324 Jun 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't usually do this for internal interfaces like this, a custom GameProvider is probably better off with thinking about what the game actually wants.

We don't know what the right package for an arbitrary game would be and I haven't implemented tracking what a class is yet (game, game lib, mod, maybe more types).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 makes sense.


enum BuiltinTransform {
Copy link
Contributor

@Gamebuster19901 Gamebuster19901 Jun 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unfamiliar with these terms. Specifically STRIP_ENVIRONMENT and CLASS_TWEAKS. I looked in FabricTransformer but the usage isn't immediately clear to me. Perhaps we could add some documentation?

EDIT: I put this note in the wrong spot. It's for the enum values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I adjusted it, class tweakers is a bit of a future term for this branch, it's the new and eventually more powerful replacement for access wideners.

STRIP_ENVIRONMENT,
WIDEN_ALL_ACCESS,
CLASS_TWEAKS,
}

boolean isEnabled();
boolean locateGame(FabricLauncher launcher, String[] args);
Expand Down
11 changes: 7 additions & 4 deletions src/main/java/net/fabricmc/loader/impl/transformer/FabricTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,24 @@

package net.fabricmc.loader.impl.transformer;

import java.util.Set;

import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter;

import net.fabricmc.accesswidener.AccessWidenerClassVisitor;
import net.fabricmc.api.EnvType;
import net.fabricmc.loader.impl.FabricLoaderImpl;
import net.fabricmc.loader.impl.game.GameProvider.BuiltinTransform;
import net.fabricmc.loader.impl.launch.FabricLauncherBase;

public final class FabricTransformer {
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 environmentStrip = !isMinecraftClass || isDevelopment;
boolean applyAccessWidener = isMinecraftClass && FabricLoaderImpl.INSTANCE.getAccessWidener().getTargets().contains(name);
Set<BuiltinTransform> transforms = FabricLoaderImpl.INSTANCE.getGameProvider().getBuiltinTransforms(name);
boolean transformAccess = transforms.contains(BuiltinTransform.WIDEN_ALL_ACCESS) && FabricLauncherBase.getLauncher().getMappingConfiguration().requiresPackageAccessHack();
boolean environmentStrip = transforms.contains(BuiltinTransform.STRIP_ENVIRONMENT);
boolean applyAccessWidener = transforms.contains(BuiltinTransform.CLASS_TWEAKS) && FabricLoaderImpl.INSTANCE.getAccessWidener().getTargets().contains(name);

if (!transformAccess && !environmentStrip && !applyAccessWidener) {
return bytes;
Expand Down