Skip to content

Commit

Permalink
Handle duplicate intermediary names in mapping
Browse files Browse the repository at this point in the history
Fixes #567
  • Loading branch information
Su5eD committed Dec 1, 2023
1 parent 3b64cbd commit 6f6cb04
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 37 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package dev.su5ed.sinytra.connector.transformer;

import dev.su5ed.sinytra.connector.transformer.jar.IntermediateMapping;
import net.fabricmc.accesswidener.AccessWidenerReader;
import net.fabricmc.accesswidener.AccessWidenerVisitor;
import net.fabricmc.loader.impl.MappingResolverImpl;
Expand All @@ -14,12 +15,12 @@ public class AccessWidenerTransformer implements Transformer {

private final String resource;
private final MappingResolverImpl resolver;
private final Map<String, String> flatMapping;
private final IntermediateMapping fastMapping;

public AccessWidenerTransformer(String resource, MappingResolverImpl namedMappingFile, Map<String, String> flatMapping) {
public AccessWidenerTransformer(String resource, MappingResolverImpl namedMappingFile, IntermediateMapping fastMapping) {
this.resource = resource;
this.resolver = namedMappingFile;
this.flatMapping = flatMapping;
this.fastMapping = fastMapping;
}

@Override
Expand Down Expand Up @@ -102,7 +103,7 @@ public void visitMethod(String owner, String name, String descriptor, AccessWide
String mappedName = AccessWidenerTransformer.this.resolver.mapMethodName(this.sourceNamespace, owner, name, descriptor);
// Mods might target inherited methods that are not part of the mapping file, we'll try to remap them using the flat mapping instead
if (name.equals(mappedName)) {
mappedName = AccessWidenerTransformer.this.flatMapping.getOrDefault(name, name);
mappedName = AccessWidenerTransformer.this.fastMapping.mapMethod(name, descriptor);
}
String mappedDescriptor = AccessWidenerTransformer.this.resolver.mapDescriptor(this.sourceNamespace, descriptor);
this.builder.append(modifier).append(" ")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import dev.su5ed.sinytra.adapter.patch.selector.AnnotationHandle;
import dev.su5ed.sinytra.adapter.patch.util.MethodQualifier;
import dev.su5ed.sinytra.connector.transformer.jar.IntermediateMapping;
import net.minecraftforge.fart.api.ClassProvider;
import net.minecraftforge.fart.api.Transformer;
import net.minecraftforge.fart.internal.ClassProviderImpl;
Expand Down Expand Up @@ -42,7 +43,7 @@ public final class OptimizedRenamingTransformer extends RenamingTransformer {
private static final String CLASS_DESC_PATTERN = "^L[a-zA-Z0-9/$_]+;$";
private static final String FQN_CLASS_NAME_PATTERN = "^([a-zA-Z0-9$_]+\\.)*[a-zA-Z0-9$_]+$";

public static Transformer create(ClassProvider classProvider, Consumer<String> log, IMappingFile mappingFile, Map<String, String> flatMappings) {
public static Transformer create(ClassProvider classProvider, Consumer<String> log, IMappingFile mappingFile, IntermediateMapping flatMappings) {
RenamingClassProvider reverseProvider = new RenamingClassProvider(classProvider, mappingFile, mappingFile.reverse(), log);
EnhancedRemapper enhancedRemapper = new RelocatingEnhancedRemapper(reverseProvider, mappingFile, flatMappings, log);
return new OptimizedRenamingTransformer(enhancedRemapper, false);
Expand Down Expand Up @@ -87,10 +88,10 @@ protected void postProcess(ClassNode node) {
}

private static class PostProcessRemapper {
private final Map<String, String> flatMappings;
private final IntermediateMapping flatMappings;
private final Remapper remapper;

public PostProcessRemapper(Map<String, String> flatMappings, Remapper remapper) {
public PostProcessRemapper(IntermediateMapping flatMappings, Remapper remapper) {
this.flatMappings = flatMappings;
this.remapper = remapper;
}
Expand Down Expand Up @@ -119,13 +120,13 @@ else if (obj instanceof List list) {
public Object mapValue(Object value) {
if (value instanceof String str) {
if (str.matches(CLASS_DESC_PATTERN)) {
String mapped = flatMappings.get(str.substring(1, str.length() - 1));
String mapped = flatMappings.map(str.substring(1, str.length() - 1));
if (mapped != null) {
return 'L' + mapped + ';';
}
}
else if (str.matches(FQN_CLASS_NAME_PATTERN)) {
String mapped = flatMappings.get(str.replace('.', '/'));
String mapped = flatMappings.map(str.replace('.', '/'));
if (mapped != null) {
return mapped.replace('/', '.');
}
Expand All @@ -134,12 +135,12 @@ else if (str.matches(FQN_CLASS_NAME_PATTERN)) {
MethodQualifier qualifier = MethodQualifier.create(str).orElse(null);
if (qualifier != null && qualifier.desc() != null) {
String owner = qualifier.owner() != null ? this.remapper.mapDesc(qualifier.owner()) : "";
String name = qualifier.name() != null ? this.flatMappings.getOrDefault(qualifier.name(), qualifier.name()) : "";
String desc = qualifier.desc() != null ? this.remapper.mapMethodDesc(qualifier.desc()) : "";
String name = qualifier.name() != null ? this.flatMappings.mapMethod(qualifier.name(), qualifier.desc()) : "";
String desc = this.remapper.mapMethodDesc(qualifier.desc());
return owner + name + desc;
}

String mapped = this.flatMappings.get(str);
String mapped = this.flatMappings.map(str);
if (mapped != null) {
return mapped;
}
Expand Down Expand Up @@ -194,16 +195,16 @@ public void close() throws IOException {
}

private static class RelocatingEnhancedRemapper extends EnhancedRemapper {
private final Map<String, String> flatMappings;
private final IntermediateMapping flatMappings;

public RelocatingEnhancedRemapper(ClassProvider classProvider, IMappingFile map, Map<String, String> flatMappings, Consumer<String> log) {
public RelocatingEnhancedRemapper(ClassProvider classProvider, IMappingFile map, IntermediateMapping flatMappings, Consumer<String> log) {
super(classProvider, map, log);
this.flatMappings = flatMappings;
}

@Override
public String map(final String key) {
String fastMapped = this.flatMappings.get(key);
String fastMapped = this.flatMappings.map(key);
if (fastMapped != null) {
return fastMapped;
}
Expand All @@ -212,7 +213,7 @@ public String map(final String key) {

@Override
public String mapFieldName(String owner, String name, String descriptor) {
String fastMapped = this.flatMappings.get(name);
String fastMapped = this.flatMappings.mapField(name, descriptor);
if (fastMapped != null) {
return fastMapped;
}
Expand Down Expand Up @@ -240,12 +241,12 @@ public String mapMethodName(String owner, String name, String descriptor) {
int interfacePrefix = name.indexOf("$");
if (interfacePrefix > -1 && name.lastIndexOf("$") == interfacePrefix) {
String actualName = name.substring(interfacePrefix + 1);
String fastMapped = this.flatMappings.get(actualName);
String fastMapped = this.flatMappings.mapMethod(actualName, descriptor);
String mapped = fastMapped != null ? fastMapped : mapMethodName(owner, actualName, descriptor);
return name.substring(0, interfacePrefix + 1) + mapped;
}
}
return this.flatMappings.get(name);
return this.flatMappings.mapMethod(name, descriptor);
})
.orElseGet(() -> super.mapMethodName(owner, name, descriptor));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,62 +1,108 @@
package dev.su5ed.sinytra.connector.transformer.jar;

import com.mojang.datafixers.util.Pair;
import com.mojang.logging.LogUtils;
import net.fabricmc.loader.impl.FabricLoaderImpl;
import net.fabricmc.loader.impl.MappingResolverImpl;
import net.minecraftforge.srgutils.IMappingFile;
import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;

import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;

import static dev.su5ed.sinytra.connector.transformer.jar.JarTransformer.TRANSFORM_MARKER;

public class IntermediateMapping {
private static final Map<String, Map<String, String>> INTERMEDIATE_MAPPINGS_CACHE = new HashMap<>();
private static final Map<String, IntermediateMapping> INTERMEDIATE_MAPPINGS_CACHE = new HashMap<>();
// Filter out non-obfuscated method names used in mapping namespaces as those don't need
// to be remapped and will only cause issues with our barebones find/replace remapper
private static final Map<String, Collection<String>> MAPPING_PREFIXES = Map.of(
"intermediary", Set.of("net/minecraft/class_", "field_", "method_", "comp_")
);
private static final Logger LOGGER = LogUtils.getLogger();

public static Map<String, String> get(String sourceNamespace) {
Map<String, String> map = INTERMEDIATE_MAPPINGS_CACHE.get(sourceNamespace);
if (map == null) {
// Original -> Mapped
private final Map<String, String> mappings;
// Original + Descriptor -> Mapped
private final Map<String, String> extendedMappings;

public static IntermediateMapping get(String sourceNamespace) {
IntermediateMapping existing = INTERMEDIATE_MAPPINGS_CACHE.get(sourceNamespace);
if (existing == null) {
synchronized (JarTransformer.class) {
Map<String, String> existing = INTERMEDIATE_MAPPINGS_CACHE.get(sourceNamespace);
existing = INTERMEDIATE_MAPPINGS_CACHE.get(sourceNamespace);
if (existing != null) {
return existing;
}

LOGGER.debug(TRANSFORM_MARKER, "Creating flat intermediate mapping for namespace {}", sourceNamespace);
// Intermediary sometimes contains duplicate names for different methods (why?). We exclude those.
Set<String> excludedNames = new HashSet<>();
Collection<String> prefixes = MAPPING_PREFIXES.get(sourceNamespace);
MappingResolverImpl resolver = FabricLoaderImpl.INSTANCE.getMappingResolver();
Map<String, String> resolved = new HashMap<>();
Map<String, String> extendedMappings = new HashMap<>();
resolver.getCurrentMap(sourceNamespace).getClasses().stream()
.flatMap(cls -> Stream.concat(Stream.of(cls), Stream.concat(cls.getFields().stream(), cls.getMethods().stream()))
.filter(node -> prefixes.stream().anyMatch(node.getOriginal()::startsWith))
.map(node -> Pair.of(node.getOriginal(), node.getMapped())))
.forEach(pair -> {
String original = pair.getFirst();
if (resolved.containsKey(original)) {
excludedNames.add(original);
.filter(node -> prefixes.stream().anyMatch(node.getOriginal()::startsWith)))
.forEach(node -> {
String original = node.getOriginal();
String mapped = node.getMapped();
String mapping = resolved.get(original);
if (mapping != null && !mapping.equals(mapped)) {
extendedMappings.put(getMappingKey(node), mapped);
resolved.remove(original);
}
if (!excludedNames.contains(original)) {
resolved.put(original, pair.getSecond());
else if (!extendedMappings.containsKey(getMappingKey(node))) {
resolved.put(original, mapped);
}
});
INTERMEDIATE_MAPPINGS_CACHE.put(sourceNamespace, resolved);
return resolved;
IntermediateMapping mapping = new IntermediateMapping(resolved, extendedMappings);
INTERMEDIATE_MAPPINGS_CACHE.put(sourceNamespace, mapping);
return mapping;
}
}
return map;
return existing;
}

private static String getMappingKey(IMappingFile.INode node) {
if (node instanceof IMappingFile.IField field) {
String desc = field.getDescriptor();
return field.getOriginal() + (desc != null ? ":" + desc : "");
}
else if (node instanceof IMappingFile.IMethod method) {
return method.getOriginal() + Objects.requireNonNullElse(method.getDescriptor(), "");
}
return node.getOriginal();
}

public IntermediateMapping(Map<String, String> mappings, Map<String, String> extendedMappings) {
this.mappings = mappings;
this.extendedMappings = extendedMappings;
}

public String map(String name) {
return this.mappings.getOrDefault(name, name);
}

public String mapField(String name, @Nullable String desc) {
String mapped = this.mappings.get(name);
if (mapped == null) {
String qualifier = name + ":" + desc;
mapped = this.extendedMappings.get(qualifier);
}
return mapped != null ? mapped : name;
}

public String mapMethod(String name, String desc) {
String mapped = this.mappings.get(name);
if (mapped == null) {
String qualifier = name + desc;
mapped = this.extendedMappings.get(qualifier);
}
return mapped != null ? mapped : name;
}
}

0 comments on commit 6f6cb04

Please sign in to comment.