diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/forge/FieldMigratedMappingsProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/forge/FieldMigratedMappingsProvider.java index 238bac2d..3a32ce49 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/forge/FieldMigratedMappingsProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/forge/FieldMigratedMappingsProvider.java @@ -41,6 +41,7 @@ import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; +import com.google.common.base.Stopwatch; import com.google.common.collect.HashBasedTable; import com.google.common.collect.Table; import com.google.common.reflect.TypeToken; @@ -106,6 +107,7 @@ public class FieldMigratedMappingsProvider extends MappingsProviderImpl { @Override public void manipulateMappings(Path mappingsJar) throws IOException { + Stopwatch stopwatch = Stopwatch.createStarted(); LoomGradleExtension extension = getExtension(); this.rawTinyMappings = tinyMappings; this.rawTinyMappingsWithSrg = tinyMappingsWithSrg; @@ -114,7 +116,7 @@ public class FieldMigratedMappingsProvider extends MappingsProviderImpl { if (getExtension().shouldGenerateSrgTiny()) { if (Files.notExists(rawTinyMappingsWithSrg) || isRefreshDeps()) { // Merge tiny mappings with srg - SrgMerger.mergeSrg(getProject().getLogger(), null, getRawSrgFile(), rawTinyMappings, rawTinyMappingsWithSrg, true); + SrgMerger.mergeSrg(getProject().getLogger(), getExtension().getMappingsProvider()::getMojmapSrgFileIfPossible, getRawSrgFile(), rawTinyMappings, rawTinyMappingsWithSrg, true); } } @@ -126,6 +128,8 @@ public class FieldMigratedMappingsProvider extends MappingsProviderImpl { } catch (IOException e) { throw new UncheckedIOException(e); } + + getProject().getLogger().info(":migrated srg fields in " + stopwatch.stop()); } public void updateFieldMigration() throws IOException { diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/forge/MinecraftPatchedProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/forge/MinecraftPatchedProvider.java index 36924802..e32e741d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/forge/MinecraftPatchedProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/forge/MinecraftPatchedProvider.java @@ -96,7 +96,6 @@ import net.fabricmc.loom.util.TinyRemapperHelper; import net.fabricmc.loom.util.function.FsPathConsumer; import net.fabricmc.loom.util.srg.InnerClassRemapper; import net.fabricmc.loom.util.srg.SpecialSourceExecutor; -import net.fabricmc.loom.util.srg.SrgMerger; import net.fabricmc.mappingio.tree.MemoryMappingTree; public class MinecraftPatchedProvider extends DependencyProvider { @@ -309,13 +308,7 @@ public class MinecraftPatchedProvider extends DependencyProvider { private TinyRemapper buildRemapper(Path input) throws IOException { Path[] libraries = TinyRemapperHelper.getMinecraftDependencies(getProject()); - MemoryMappingTree mappingsWithSrg; - - if (getExtension().isForgeAndOfficial()) { - mappingsWithSrg = SrgMerger.mergeSrg(getProject().getLogger(), getExtension().getMappingsProvider()::getMojmapSrgFileIfPossible, getExtension().getSrgProvider().getMergedMojangTrimmed(), getExtension().getMappingsProvider().tinyMappings, true); - } else { - mappingsWithSrg = getExtension().getMappingsProvider().getMappingsWithSrg(); - } + MemoryMappingTree mappingsWithSrg = getExtension().getMappingsProvider().getMappingsWithSrg(); TinyRemapper remapper = TinyRemapper.newRemapper() .logger(getProject().getLogger()::lifecycle) diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingsProviderImpl.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingsProviderImpl.java index 1a3d5e35..8adfc28b 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingsProviderImpl.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingsProviderImpl.java @@ -156,7 +156,7 @@ public class MappingsProviderImpl extends DependencyProvider implements Mappings if (getExtension().shouldGenerateSrgTiny()) { if (Files.notExists(tinyMappingsWithSrg) || isRefreshDeps()) { // Merge tiny mappings with srg - SrgMerger.mergeSrg(getProject().getLogger(), null, getRawSrgFile(), tinyMappings, tinyMappingsWithSrg, true); + SrgMerger.mergeSrg(getProject().getLogger(), getExtension().getMappingsProvider()::getMojmapSrgFileIfPossible, getRawSrgFile(), tinyMappings, tinyMappingsWithSrg, true); } mappingTreeWithSrg = readMappings(tinyMappingsWithSrg); diff --git a/src/main/java/net/fabricmc/loom/util/srg/SrgMerger.java b/src/main/java/net/fabricmc/loom/util/srg/SrgMerger.java index 9765fa25..ede2dbad 100644 --- a/src/main/java/net/fabricmc/loom/util/srg/SrgMerger.java +++ b/src/main/java/net/fabricmc/loom/util/srg/SrgMerger.java @@ -27,19 +27,23 @@ package net.fabricmc.loom.util.srg; import java.io.BufferedReader; import java.io.IOException; import java.io.StringReader; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; import com.google.common.base.MoreObjects; +import com.google.common.base.Stopwatch; import org.apache.commons.io.IOUtils; import org.gradle.api.logging.Logger; import org.jetbrains.annotations.Nullable; @@ -60,6 +64,49 @@ import net.fabricmc.mappingio.tree.MemoryMappingTree; * @author Juuz */ public final class SrgMerger { + private final Logger logger; + private final Map> addRegardlessSrg = new HashMap<>(); + private final MemoryMappingTree srg; + private final MemoryMappingTree src; + private final MemoryMappingTree output; + private final FlatMappingVisitor flatOutput; + private final List postProcesses = new ArrayList<>(); + private final boolean lenient; + private final Set methodSrgNames = new HashSet<>(); + + public SrgMerger(Logger logger, Path srg, @Nullable Supplier mojmap, Path tiny, boolean lenient) throws IOException { + this.logger = logger; + this.srg = readSrg(srg, mojmap); + this.src = new MemoryMappingTree(); + this.output = new MemoryMappingTree(); + this.flatOutput = new RegularAsFlatMappingVisitor(output); + this.lenient = lenient; + + MappingReader.read(tiny, this.src); + + if (!"official".equals(this.src.getSrcNamespace())) { + throw new MappingException("Mapping file " + tiny + " does not have the 'official' namespace as the default!"); + } + + this.output.visitNamespaces(this.src.getSrcNamespace(), Stream.concat(Stream.of("srg"), this.src.getDstNamespaces().stream()).collect(Collectors.toList())); + } + + public MemoryMappingTree merge() throws IOException { + for (MappingTree.ClassMapping klass : this.srg.getClasses()) { + classToTiny(klass); + } + + try { + for (Runnable process : postProcesses) { + process.run(); + } + } catch (UncheckedIOException e) { + throw e.getCause(); + } + + return output; + } + /** * Merges SRG mappings with a tiny mappings tree through the obf names. * @@ -72,7 +119,9 @@ public final class SrgMerger { * @throws MappingException if the input tiny tree's default namespace is not 'official' * or if an element mentioned in the SRG file does not have tiny mappings */ - public static void mergeSrg(Logger logger, @Nullable Supplier mojmap, Path srg, Path tiny, Path out, boolean lenient) throws IOException, MappingException { + public static void mergeSrg(Logger logger, @Nullable Supplier mojmap, Path srg, Path tiny, Path out, boolean lenient) + throws IOException, MappingException { + Stopwatch stopwatch = Stopwatch.createStarted(); MemoryMappingTree tree = mergeSrg(logger, mojmap, srg, tiny, lenient); try (Tiny2Writer writer = new Tiny2Writer(Files.newBufferedWriter(out), false)) { @@ -80,6 +129,8 @@ public final class SrgMerger { } catch (IOException e) { e.printStackTrace(); } + + logger.info(":merged srg mappings in " + stopwatch.stop()); } /** @@ -94,37 +145,17 @@ public final class SrgMerger { * @throws MappingException if the input tiny tree's default namespace is not 'official' * or if an element mentioned in the SRG file does not have tiny mappings */ - public static MemoryMappingTree mergeSrg(Logger logger, @Nullable Supplier mojmap, Path srg, Path tiny, boolean lenient) throws IOException, MappingException { - Map> addRegardlessSrgs = new HashMap<>(); - MemoryMappingTree arr = readSrg(srg, mojmap, addRegardlessSrgs); - MemoryMappingTree foss = new MemoryMappingTree(); - - try (BufferedReader reader = Files.newBufferedReader(tiny)) { - MappingReader.read(reader, foss); - } - - if (!"official".equals(foss.getSrcNamespace())) { - throw new MappingException("Mapping file " + tiny + " does not have the 'official' namespace as the default!"); - } - - MemoryMappingTree output = new MemoryMappingTree(); - output.visitNamespaces(foss.getSrcNamespace(), Stream.concat(Stream.of("srg"), foss.getDstNamespaces().stream()).collect(Collectors.toList())); - RegularAsFlatMappingVisitor flatMappingVisitor = new RegularAsFlatMappingVisitor(output); - - for (MappingTree.ClassMapping klass : arr.getClasses()) { - classToTiny(logger, addRegardlessSrgs, foss, klass, output, flatMappingVisitor, lenient); - } - - return output; + public static MemoryMappingTree mergeSrg(Logger logger, @Nullable Supplier mojmap, Path srg, Path tiny, boolean lenient) + throws IOException, MappingException { + return new SrgMerger(logger, srg, mojmap, tiny, lenient).merge(); } - private static MemoryMappingTree readSrg(Path srg, @Nullable Supplier mojmap, Map> addRegardlessSrgs) - throws IOException { + private MemoryMappingTree readSrg(Path srg, @Nullable Supplier mojmap) throws IOException { try (BufferedReader reader = Files.newBufferedReader(srg)) { String content = IOUtils.toString(reader); if (content.startsWith("tsrg2") && mojmap != null) { - addRegardlessSrgs(mojmap, addRegardlessSrgs); + addRegardlessSrgs(mojmap); } MemoryMappingTree tsrg = new MemoryMappingTree(); @@ -133,7 +164,7 @@ public final class SrgMerger { } } - private static void addRegardlessSrgs(Supplier mojmap, Map> addRegardlessSrgs) throws IOException { + private void addRegardlessSrgs(Supplier mojmap) throws IOException { MemoryMappingTree mojmapTree = readTsrg2ToTinyTree(mojmap.get()); for (MappingTree.ClassMapping classDef : mojmapTree.getClasses()) { @@ -141,13 +172,13 @@ public final class SrgMerger { String name = methodDef.getSrcName(); if (name.indexOf('<') != 0 && name.equals(methodDef.getDstName(0))) { - addRegardlessSrgs.computeIfAbsent(classDef.getSrcName(), $ -> new ArrayList<>()).add(methodDef); + addRegardlessSrg.computeIfAbsent(classDef.getSrcName(), $ -> new ArrayList<>()).add(methodDef); } } for (MappingTree.FieldMapping fieldDef : classDef.getFields()) { if (fieldDef.getSrcName().equals(fieldDef.getDstName(0))) { - addRegardlessSrgs.computeIfAbsent(classDef.getSrcName(), $ -> new ArrayList<>()).add(fieldDef); + addRegardlessSrg.computeIfAbsent(classDef.getSrcName(), $ -> new ArrayList<>()).add(fieldDef); } } } @@ -163,11 +194,10 @@ public final class SrgMerger { return tree; } - private static void classToTiny(Logger logger, Map> addRegardlessSrgs, MappingTree foss, MappingTree.ClassMapping klass, MappingTree output, FlatMappingVisitor flatOutput, boolean lenient) - throws IOException { + private void classToTiny(MappingTree.ClassMapping klass) throws IOException { String obf = klass.getSrcName(); String srg = klass.getDstName(0); - MappingTree.ClassMapping classDef = foss.getClass(obf); + MappingTree.ClassMapping classDef = this.src.getClass(obf); if (classDef == null) { if (lenient) { @@ -194,46 +224,83 @@ public final class SrgMerger { m -> m.getName("official").equals(method.getSrcName()) && m.getDesc("official").equals(method.getSrcDesc()) ).orElse(null); - if (def == null) { - if (tryMatchRegardlessSrgsMethod(addRegardlessSrgs, obf, output, flatOutput, method)) continue; + String methodSrgName = method.getDstName(0); - if (!lenient) { - throw new MappingException("Missing method: " + method.getSrcName() + " (srg: " + method.getDstName(0) + ")"); + if (def == null) { + if (lenient) { + // TODO Big Hack! + // We are checking if there are methods with the same srg name that are already added, if it is, then we would not fill in these names + // This is especially troublesome with methods annotated with @DontObfuscate (e.g. m_129629_) + // with environments like yarn where methods with the same srg name may not inherit the same names due to parameter mappings and inheritance + // This requires further testing! + postProcesses.add(() -> { + if (!methodSrgNames.contains(methodSrgName)) { + List methodNames = CollectionUtil.map( + output.getDstNamespaces(), + namespace -> "srg".equals(namespace) ? methodSrgName : method.getSrcName() + ); + + try { + flatOutput.visitMethod(obf, method.getSrcName(), method.getSrcDesc(), methodNames.toArray(new String[0])); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + }); + } else { + throw new MappingException("Missing method: " + method.getSrcName() + " (srg: " + methodSrgName + ")"); } continue; } - methodToTiny(obf, method, null, def, output, flatOutput); - } + methodToTiny(obf, method, methodSrgName, def); - // TODO: This second iteration seems a bit wasteful. - // Is it possible to just iterate this and leave SRG out? - for (MappingTree.MethodMapping def : classDef.getMethods()) { - // If obf = some other name: some special name that srg might not contain. - // This includes constructors and overridden JDK methods. - if (!def.getSrcName().equals(def.getDstName(0))) { - continue; - } - - MappingTree.MethodMapping method = CollectionUtil.find( - klass.getMethods(), - m -> m.getSrcName().equals(def.getName("official")) && m.getSrcDesc().equals(def.getDesc("official")) - ).orElse(null); - - if (method == null) { - methodToTiny(obf, null, def.getSrcName(), def, output, flatOutput); + if (methodSrgName.startsWith("func_") || methodSrgName.startsWith("m_")) { + methodSrgNames.add(methodSrgName); } } + postProcesses.add(() -> { + // TODO: This second iteration seems a bit wasteful. + // Is it possible to just iterate this and leave SRG out? + for (MappingTree.MethodMapping def : classDef.getMethods()) { + // If obf = some other name: some special name that srg might not contain. + // This includes constructors and overridden JDK methods. + if (!def.getSrcName().equals(def.getDstName(0))) { + continue; + } + + MappingTree.MethodMapping method = CollectionUtil.find( + klass.getMethods(), + m -> m.getSrcName().equals(def.getName("official")) && m.getSrcDesc().equals(def.getDesc("official")) + ).orElse(null); + + if (method == null) { + try { + methodToTiny(obf, null, def.getSrcName(), def); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + } + }); + for (MappingTree.FieldMapping field : klass.getFields()) { MappingTree.FieldMapping def = CollectionUtil.find( classDef.getFields(), f -> f.getName("official").equals(field.getSrcName()) - ).orElse(nullOrThrow(lenient, () -> new MappingException("Missing field: " + field.getSrcName() + " (srg: " + field.getDstName(0) + ")"))); + ).orElse(nullOrThrow(() -> new MappingException("Missing field: " + field.getSrcName() + " (srg: " + field.getDstName(0) + ")"))); if (def == null) { - if (tryMatchRegardlessSrgsField(addRegardlessSrgs, obf, output, flatOutput, field)) continue; + if (tryMatchRegardlessSrgsField(obf, field)) { + List fieldNames = CollectionUtil.map( + output.getDstNamespaces(), + namespace -> "srg".equals(namespace) ? field.getDstName(0) : field.getSrcName() + ); + + flatOutput.visitField(obf, field.getSrcName(), field.getSrcDesc(), fieldNames.toArray(new String[0])); + } continue; } @@ -251,8 +318,9 @@ public final class SrgMerger { } } - private static void methodToTiny(String obfClassName, @Nullable MappingTree.MethodMapping srgMethod, @Nullable String srgMethodName, MappingTree.MethodMapping actualMethod, MappingTree output, FlatMappingVisitor flatOutput) throws IOException { - if (srgMethod != null) { + private void methodToTiny(String obfClassName, @Nullable MappingTree.MethodMapping srgMethod, @Nullable String srgMethodName, MappingTree.MethodMapping actualMethod) + throws IOException { + if (srgMethod != null && srgMethodName != null) { srgMethodName = srgMethod.getDstName(0); } @@ -299,19 +367,12 @@ public final class SrgMerger { } } - private static boolean tryMatchRegardlessSrgsMethod(Map> addRegardlessSrgs, String obf, - MappingTree output, FlatMappingVisitor flatOutput, MappingTree.MethodMapping method) throws IOException { - List mutableDescriptoredList = addRegardlessSrgs.get(obf); + private boolean tryMatchRegardlessSrgsMethod(String obf, MappingTree.MethodMapping method) { + List mutableDescriptoredList = addRegardlessSrg.get(obf); if (!method.getDstName(0).equals(method.getSrcName())) { for (MappingTreeView.MemberMappingView descriptored : MoreObjects.firstNonNull(mutableDescriptoredList, Collections.emptyList())) { if (descriptored instanceof MappingTree.MethodMapping && descriptored.getSrcName().equals(method.getSrcName()) && descriptored.getSrcDesc().equals(method.getSrcDesc())) { - List methodNames = CollectionUtil.map( - output.getDstNamespaces(), - namespace -> "srg".equals(namespace) ? method.getDstName(0) : method.getSrcName() - ); - - flatOutput.visitMethod(obf, method.getSrcName(), method.getSrcDesc(), methodNames.toArray(new String[0])); return true; } } @@ -320,19 +381,12 @@ public final class SrgMerger { return false; } - private static boolean tryMatchRegardlessSrgsField(Map> addRegardlessSrgs, String obf, - MappingTree output, FlatMappingVisitor flatOutput, MappingTree.FieldMapping field) throws IOException { - List mutableDescriptoredList = addRegardlessSrgs.get(obf); + private boolean tryMatchRegardlessSrgsField(String obf, MappingTree.FieldMapping field) { + List mutableDescriptoredList = addRegardlessSrg.get(obf); if (!field.getDstName(0).equals(field.getSrcName())) { for (MappingTreeView.MemberMappingView descriptored : MoreObjects.firstNonNull(mutableDescriptoredList, Collections.emptyList())) { if (descriptored instanceof MappingTree.FieldMapping && descriptored.getSrcName().equals(field.getSrcName())) { - List fieldNames = CollectionUtil.map( - output.getDstNamespaces(), - namespace -> "srg".equals(namespace) ? field.getDstName(0) : field.getSrcName() - ); - - flatOutput.visitField(obf, field.getSrcName(), field.getSrcDesc(), fieldNames.toArray(new String[0])); return true; } } @@ -342,7 +396,7 @@ public final class SrgMerger { } @Nullable - private static T nullOrThrow(boolean lenient, Supplier exception) throws X { + private T nullOrThrow(Supplier exception) throws X { if (lenient) { return null; } else {