Skip to content
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.

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

Fix support for Forge 50+ #227

Merged
merged 12 commits into from
Aug 12, 2024
2 changes: 0 additions & 2 deletions gradle/runtime.libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ access-transformers-new = "8.0.5"
access-transformers-neo = "10.0.2"
unprotect = "1.2.0"
asm = "9.7"
union-relauncher = "1.1.1"
access-transformers-log4j = "2.17.1"

[libraries]
Expand All @@ -45,5 +44,4 @@ access-transformers-new = { module = "net.minecraftforge:accesstransformers", ve
access-transformers-neo = { module = "net.neoforged.accesstransformers:at-cli", version.ref = "access-transformers-neo" }
unprotect = { module = "io.github.juuxel:unprotect", version.ref = "unprotect" }
asm = { module = "org.ow2.asm:asm", version.ref = "asm" }
union-relauncher = { module = "io.github.juuxel:union-relauncher", version.ref = "union-relauncher" }
access-transformers-log4j-bom = { module = "org.apache.logging.log4j:log4j-bom", version.ref = "access-transformers-log4j" }
6 changes: 5 additions & 1 deletion src/main/java/net/fabricmc/loom/LoomGradleExtension.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.Collection;
import java.util.List;

import org.gradle.api.GradleException;
import org.gradle.api.Project;
import org.gradle.api.file.ConfigurableFileCollection;
import org.gradle.api.file.FileCollection;
Expand Down Expand Up @@ -114,7 +115,10 @@ default List<Path> getMinecraftJars(MappingsNamespace mappingsNamespace) {
yield getSrgMinecraftProvider().getMinecraftJarPaths();
}
case MOJANG -> {
ModPlatform.assertPlatform(this, ModPlatform.NEOFORGE, () -> "Mojang-mapped jars are only available on NeoForge.");
if (!this.isForgeLike() || !this.getForgeProvider().usesMojangAtRuntime()) {
throw new GradleException("Mojang-mapped jars are only available on NeoForge / Forge 50+.");
}

yield getMojangMappedMinecraftProvider().getMinecraftJarPaths();
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public static MappingsNamespace runtimeIntermediaryNamespace(Project project) {
/**
* Potentially replaces the remapping target namespace for mixin refmaps.
*
* <p>All {@linkplain #intermediary(Project) intermediary-like namespaces} are replaced
* <p>All {@linkplain #runtimeIntermediary(Project) intermediary-like namespaces} are replaced
* by {@code intermediary} since fabric-mixin-compile-extensions only supports intermediary.
* We transform the namespaces in the input mappings, e.g. {@code intermediary} -> {@code yraidemretni} and
* {@code srg} -> {@code intermediary}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,9 @@ private synchronized void setupMinecraft(ConfigContext configContext) throws Exc
final SrgMinecraftProvider<?> srgMinecraftProvider = jarConfiguration.createSrgMinecraftProvider(project);
extension.setSrgMinecraftProvider(srgMinecraftProvider);
srgMinecraftProvider.provide(provideContext);
} else if (extension.isNeoForge()) {
}

if (extension.isForgeLike() && extension.getForgeProvider().usesMojangAtRuntime()) {
final MojangMappedMinecraftProvider<?> mojangMappedMinecraftProvider = jarConfiguration.createMojangMappedMinecraftProvider(project);
extension.setMojangMappedMinecraftProvider(mojangMappedMinecraftProvider);
mojangMappedMinecraftProvider.provide(provideContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ public record ArtifactMetadata(boolean isFabricMod, RemapRequirements remapRequi
private static final String QUILT_INSTALLER_PATH = "quilt_installer.json";

public static ArtifactMetadata create(ArtifactRef artifact, String currentLoomVersion) throws IOException {
return create(artifact, currentLoomVersion, ModPlatform.FABRIC);
return create(artifact, currentLoomVersion, ModPlatform.FABRIC, null);
}

public static ArtifactMetadata create(ArtifactRef artifact, String currentLoomVersion, ModPlatform platform) throws IOException {
public static ArtifactMetadata create(ArtifactRef artifact, String currentLoomVersion, ModPlatform platform, @Nullable Boolean forcesStaticMixinRemap) throws IOException {
boolean isFabricMod;
RemapRequirements remapRequirements = RemapRequirements.DEFAULT;
InstallerData installerData = null;
Expand Down Expand Up @@ -93,11 +93,10 @@ public static ArtifactMetadata create(ArtifactRef artifact, String currentLoomVe
} catch (IllegalArgumentException e) {
throw new IllegalStateException("Unknown mixin remap type: " + mixinRemapType);
}
} else if (platform == ModPlatform.FORGE) {
// Use certain refmap remap types by the current platform
refmapRemapType = MixinRemapType.MIXIN;
} else if (platform == ModPlatform.NEOFORGE) {
refmapRemapType = MixinRemapType.STATIC;
} else if (forcesStaticMixinRemap != null) {
// The mixin remap type is not specified in the manifest, but we have a forced value
// This is forced to be static on NeoForge or Forge 50+.
refmapRemapType = forcesStaticMixinRemap ? MixinRemapType.STATIC : MixinRemapType.MIXIN;
}

if (loomVersion != null && refmapRemapType != MixinRemapType.STATIC) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ public static void supplyModConfigurations(Project project, SharedServiceManager

artifactMetadata = metaCache.computeIfAbsent(artifact, a -> {
try {
return ArtifactMetadata.create(a, LoomGradlePlugin.LOOM_VERSION, extension.getPlatform().get());
return ArtifactMetadata.create(a, LoomGradlePlugin.LOOM_VERSION, extension.getPlatform().get(),
extension.isForgeLike() && extension.getForgeProvider().usesMojangAtRuntime() ? true : null);
} catch (IOException e) {
throw ExceptionUtil.createDescriptiveWrapper(UncheckedIOException::new, "Failed to read metadata from " + a.path(), e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ private void remapJars(List<ModDependency> remapList) throws IOException {

final TinyRemapper remapper = builder.build();

remapper.readClassPath(extension.getMinecraftJars(IntermediaryNamespaces.intermediaryNamespace(project)).toArray(Path[]::new));
remapper.readClassPath(extension.getMinecraftJars(IntermediaryNamespaces.runtimeIntermediaryNamespace(project)).toArray(Path[]::new));

final Map<ModDependency, InputTag> tagMap = new HashMap<>();
final Map<ModDependency, OutputConsumerPath> outputConsumerMap = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ public static void provide(MappingConfiguration mappingConfiguration, Project pr
}
}

if (lib.startsWith("net.minecraftforge:bootstrap:")) {
if (extension.isForge() && extension.getForgeProvider().getVersion().getMajorVersion() >= Constants.Forge.MIN_BOOTSTRAP_DEV_VERSION) {
String version = lib.substring(lib.lastIndexOf(":"));
dependencies.add(project.getDependencies().create("net.minecraftforge:bootstrap-dev" + version));
}
}

if (dep == null) {
dep = lib;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import net.fabricmc.loom.LoomGradleExtension;
import net.fabricmc.loom.configuration.DependencyInfo;
import net.fabricmc.loom.util.Constants;
import net.fabricmc.loom.util.LoomVersions;
import net.fabricmc.loom.util.ModPlatform;

public class ForgeProvider extends DependencyProvider {
Expand All @@ -50,10 +49,6 @@ public void provide(DependencyInfo dependency) throws Exception {
version = new ForgeVersion(dependency.getResolvedVersion());
addDependency(dependency.getDepString() + ":userdev", Constants.Configurations.FORGE_USERDEV);
addDependency(dependency.getDepString() + ":installer", Constants.Configurations.FORGE_INSTALLER);

if (getExtension().isForge() && version.getMajorVersion() >= Constants.Forge.MIN_UNION_RELAUNCHER_VERSION) {
addDependency(LoomVersions.UNION_RELAUNCHER.mavenNotation(), Constants.Configurations.FORGE_EXTRA);
}
}

public ForgeVersion getVersion() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,6 @@ public void applyTo(RunConfigSettings settings, ConfigValue.Resolver configValue

// Add MOD_CLASSES, this is something that ForgeGradle does
settings.getEnvironmentVariables().computeIfAbsent("MOD_CLASSES", $ -> ConfigValue.of("{source_roots}").resolve(configValueResolver));

final ForgeProvider forgeProvider = settings.getExtension().getForgeProvider();

if (settings.getExtension().isForge() && forgeProvider.getVersion().getMajorVersion() >= Constants.Forge.MIN_UNION_RELAUNCHER_VERSION) {
settings.defaultMainClass(Constants.Forge.UNION_RELAUNCHER_MAIN_CLASS);
settings.property(Constants.Forge.UNION_RELAUNCHER_MAIN_CLASS_PROPERTY, main);
}
}

public Resolved resolve(ConfigValue.Resolver configValueResolver) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ private String resolve(@Nullable RunConfigSettings runConfig, ConfigValue.Variab
// Use a set-valued multimap for deduplicating paths.
Multimap<String, String> modClasses = MultimapBuilder.hashKeys().linkedHashSetValues().build();
NamedDomainObjectContainer<ModSettings> mods = extension.getMods();
// Forge 49+ bootstrap-dev uses ; as a separator, instead of File.pathSeparator
String separator = extension.getForgeProvider().getVersion().getMajorVersion() >= Constants.Forge.MIN_BOOTSTRAP_DEV_VERSION ? ";" : File.pathSeparator;
shedaniel marked this conversation as resolved.
Show resolved Hide resolved

if (runConfig != null && !runConfig.getMods().isEmpty()) {
mods = runConfig.getMods();
Expand All @@ -147,7 +149,7 @@ private String resolve(@Nullable RunConfigSettings runConfig, ConfigValue.Variab

string = modClasses.entries().stream()
.map(entry -> entry.getKey() + "%%" + entry.getValue())
.collect(Collectors.joining(File.pathSeparator));
.collect(Collectors.joining(separator));
} else if (key.equals("mcp_mappings")) {
string = "loom.stub";
} else if (json.has(key)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,14 @@
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.EnumMap;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import com.google.common.base.Stopwatch;
import com.google.common.base.Supplier;
import com.google.gson.JsonObject;
import dev.architectury.loom.util.MappingOption;
import org.apache.tools.ant.util.StringUtils;
Expand Down Expand Up @@ -94,6 +96,7 @@ public class MappingConfiguration {
public Path tinyMappingsWithSrg;
public final Map<String, Path> mixinTinyMappings; // The mixin mappings have other names in intermediary.
public final Path srgToNamedSrg; // FORGE: srg to named in srg file format
private final Map<MappingOption, Supplier<Path>> mappingOptions;
private final Path unpickDefinitions;

private boolean hasUnpickDefinitions;
Expand All @@ -112,6 +115,8 @@ protected MappingConfiguration(String mappingsIdentifier, Path mappingsWorkingDi
this.tinyMappingsWithMojang = mappingsWorkingDir.resolve("mappings-mojang.tiny");
this.mixinTinyMappings = new HashMap<>();
this.srgToNamedSrg = mappingsWorkingDir.resolve("mappings-srg-named.srg");
this.mappingOptions = new EnumMap<>(MappingOption.class);
this.mappingOptions.put(MappingOption.DEFAULT, () -> this.tinyMappings);
}

public static MappingConfiguration create(Project project, SharedServiceManager serviceManager, DependencyInfo dependency, MinecraftProvider minecraftProvider) {
Expand Down Expand Up @@ -164,25 +169,15 @@ public TinyMappingsService getMappingsService(SharedServiceManager serviceManage
}

public TinyMappingsService getMappingsService(SharedServiceManager serviceManager, MappingOption mappingOption) {
final Path tinyMappings = switch (mappingOption) {
case WITH_SRG -> {
if (Files.notExists(this.tinyMappingsWithSrg)) {
throw new UnsupportedOperationException("Cannot get mappings service with SRG mappings without SRG enabled!");
}
Supplier<Path> mappingsSupplier = this.mappingOptions.get(mappingOption);

yield this.tinyMappingsWithSrg;
if (mappingsSupplier == null) {
throw new UnsupportedOperationException("Unsupported mapping option: " + mappingOption + ", it is possible that this option is not supported by this project / platform!");
} else if (Files.notExists(mappingsSupplier.get())) {
throw new UnsupportedOperationException("Mapping option " + mappingOption + " found but file does not exist!");
}
case WITH_MOJANG -> {
if (Files.notExists(this.tinyMappingsWithMojang)) {
throw new UnsupportedOperationException("Cannot get mappings service with Mojang mappings without Mojang merging enabled!");
}

yield this.tinyMappingsWithMojang;
}
default -> this.tinyMappings;
};

return TinyMappingsService.create(serviceManager, Objects.requireNonNull(tinyMappings));
return TinyMappingsService.create(serviceManager, Objects.requireNonNull(mappingsSupplier.get()));
}

protected void setup(Project project, SharedServiceManager serviceManager, MinecraftProvider minecraftProvider, Path inputJar) throws IOException {
Expand All @@ -208,6 +203,8 @@ public void setupPost(Project project) throws IOException {
LoomGradleExtension extension = LoomGradleExtension.get(project);

if (extension.isNeoForge()) {
this.mappingOptions.put(MappingOption.WITH_MOJANG, () -> this.tinyMappingsWithMojang);

// Generate the Mojmap-merged mappings if needed.
// Note that this needs to happen before manipulateMappings for FieldMigratedMappingConfiguration.
if (Files.notExists(tinyMappingsWithMojang) || extension.refreshDeps()) {
Expand All @@ -216,6 +213,12 @@ public void setupPost(Project project) throws IOException {
}

if (extension.shouldGenerateSrgTiny()) {
this.mappingOptions.put(MappingOption.WITH_SRG, () -> this.tinyMappingsWithSrg);

if (extension.isForge() && extension.getForgeProvider().usesMojangAtRuntime()) {
this.mappingOptions.put(MappingOption.WITH_MOJANG, () -> this.tinyMappingsWithSrg);
}

if (Files.notExists(tinyMappingsWithSrg) || extension.refreshDeps()) {
if (extension.isForge() && extension.getForgeProvider().usesMojangAtRuntime()) {
Path tmp = Files.createTempFile("mappings", ".tiny");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ private void remapJar(RemappedJars remappedJars, ConfigContext configContext) th
className = "net.minecraftforge.registries.ObjectHolderRegistry";
}

final String sourceNamespace = IntermediaryNamespaces.intermediary(project);
final String sourceNamespace = IntermediaryNamespaces.runtimeIntermediary(project);
final MemoryMappingTree mappings = mappingsService.getMappingTree();
RemapObjectHolderVisitor.remapObjectHolder(remappedJars.outputJar().getPath(), className, mappings, sourceNamespace, "named");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public abstract class MixinExtensionApiImpl implements MixinExtensionAPI {
public MixinExtensionApiImpl(Project project) {
this.project = Objects.requireNonNull(project);
this.useMixinAp = project.getObjects().property(Boolean.class)
.convention(project.provider(() -> !LoomGradleExtension.get(project).isNeoForge()));
.convention(project.provider(() -> !LoomGradleExtension.get(project).isNeoForge() && (!LoomGradleExtension.get(project).isForge() || !LoomGradleExtension.get(project).getForgeProvider().usesMojangAtRuntime())));
Copy link
Member

Choose a reason for hiding this comment

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

Does the PR fix the crash bypassed by 8c2b72a?

Copy link
Member Author

Choose a reason for hiding this comment

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

I never experienced any crash?
To get a crash here you would need to ref the property in your own buildscripts early, so imo just don't do it?

Copy link
Member

Choose a reason for hiding this comment

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

I never experienced any crash? To get a crash here you would need to ref the property in your own buildscripts early, so imo just don't do it?

I think this crash was Architectury Plugin's fault, if so, the Arch Plugin fix should fix that problem if that was the cause.

Copy link
Member Author

Choose a reason for hiding this comment

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

So the crash in that commit is caused by architectury plugin, this should just require a change in arch plugin (which is in a separate branch there)
I think that is a better solution than making this false for forge.


this.refmapTargetNamespace = project.getObjects().property(String.class)
.convention(project.provider(() -> IntermediaryNamespaces.runtimeIntermediary(project)));
Expand Down
6 changes: 2 additions & 4 deletions src/main/java/net/fabricmc/loom/util/Constants.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,9 @@ public static final class Forge {
public static final String MIXIN_CONFIGS_MANIFEST_KEY = "MixinConfigs";

/**
* The minimum Forge version that needs Union Relauncher to use {@code MOD_CLASSES}.
* The minimum Forge version that needs bootstrap-dev to use {@code MOD_CLASSES}.
*/
public static final int MIN_UNION_RELAUNCHER_VERSION = 49;
public static final String UNION_RELAUNCHER_MAIN_CLASS = "juuxel.unionrelauncher.UnionRelauncher";
public static final String UNION_RELAUNCHER_MAIN_CLASS_PROPERTY = "unionRelauncher.mainClass";
public static final int MIN_BOOTSTRAP_DEV_VERSION = 49;

/**
* The minimum version of Forge that uses "mojang" as the namespace in production.
Expand Down
12 changes: 4 additions & 8 deletions src/main/java/net/fabricmc/loom/util/SourceRemapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void remapAll() {
return;
}

project.getLogger().lifecycle(":remapping sources");
project.getLogger().lifecycle(":remapping sources (Mercury, {} -> {})", from, to);

ProgressLoggerFactory progressLoggerFactory = ((ProjectInternal) project).getServices().get(ProgressLoggerFactory.class);
ProgressLogger progressLogger = progressLoggerFactory.newOperation(SourceRemapper.class.getName());
Expand Down Expand Up @@ -199,13 +199,9 @@ private Mercury getMercuryInstance() {
mercury.getClassPath().add(intermediaryJar);
}

if (extension.isForge()) {
for (Path srgJar : extension.getMinecraftJars(MappingsNamespace.SRG)) {
mercury.getClassPath().add(srgJar);
}
} else if (extension.isNeoForge()) {
for (Path mojangJar : extension.getMinecraftJars(MappingsNamespace.MOJANG)) {
mercury.getClassPath().add(mojangJar);
if (extension.isForgeLike()) {
for (Path jar : extension.getMinecraftJars(IntermediaryNamespaces.runtimeIntermediaryNamespace(project))) {
mercury.getClassPath().add(jar);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import static org.gradle.testkit.runner.TaskOutcome.SUCCESS

class Forge1206Test extends Specification implements GradleProjectTestTrait {
@Unroll
def "build #mcVersion #neoforgeVersion #mappings #patches"() {
def "build #mcVersion #forgeVersion #mappings #patches"() {
if (Integer.valueOf(System.getProperty("java.version").split("\\.")[0]) < 21) {
println("This test requires Java 21. Currently you have Java ${System.getProperty("java.version")}.")
return
Expand All @@ -43,7 +43,7 @@ class Forge1206Test extends Specification implements GradleProjectTestTrait {
setup:
def gradle = gradleProject(project: "forge/1206", version: DEFAULT_GRADLE)
gradle.buildGradle.text = gradle.buildGradle.text.replace('@MCVERSION@', mcVersion)
.replace('@FORGEVERSION@', neoforgeVersion)
.replace('@FORGEVERSION@', forgeVersion)
.replace('MAPPINGS', mappings) // Spotless doesn't like the @'s
.replace('PATCHES', patches)

Expand All @@ -54,7 +54,7 @@ class Forge1206Test extends Specification implements GradleProjectTestTrait {
result.task(":build").outcome == SUCCESS

where:
mcVersion | neoforgeVersion | mappings | patches
mcVersion | forgeVersion | mappings | patches
'1.20.6' | '1.20.6-50.1.3' | 'loom.officialMojangMappings()' | ''
}
}
11 changes: 11 additions & 0 deletions src/test/resources/projects/forge/1206/build.gradle
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a known forge mod to the test to see if it gets remapped correctly?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can add it, but the tests won't be able to pick it up if it doesn't get remapped properly anyways, so I am not sure if it makes sense. I usually just have an external project.

Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ group = project.maven_group
def mcVersion = "@MCVERSION@"
def forgeVersion = "@FORGEVERSION@"


loom {
forge {
mixinConfig "examplemod.mixins.json"
}

afterEvaluate {
assert mixin.useLegacyMixinAp.get() == false
shedaniel marked this conversation as resolved.
Show resolved Hide resolved
}
}

repositories {
// Add repositories to retrieve artifacts from in here.
// You should only use this when depending on other mods because
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.example.examplemod.mixin;

import net.minecraft.client.Minecraft;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;

@Mixin(Minecraft.class)
public class ExampleModMixin {
@Inject(method = "<clinit>", at = @At("HEAD"))
private static void initInject(CallbackInfo info) {
System.out.println("Hello from the example mod mixin!");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"required": true,
"minVersion": "0.8",
"package": "com.example.examplemod.mixin",
"compatibilityLevel": "JAVA_17",
"mixins": [
],
"client": [
"ExampleModMixin"
],
"injectors": {
"defaultRequire": 1
}
}