Deal with obfuscation properly (#645)

* make one method to dump the classes instead of copy pasting 6 times

* deal with obfuscation properly in ASM transformers instead of brute forcing both obfuscated and clear names

* remove sorting index on AngelicaTweaker to avoid confusion

* use static final variables for the class names with "."

src/main/java/com/gtnewhorizons/angelica/loading/AngelicaTweaker.java

@@ -1,6 +1,7 @@
 package com.gtnewhorizons.angelica.loading;
 
 import com.google.common.collect.ImmutableMap;
+import com.gtnewhorizon.gtnhlib.asm.ASMUtil;
 import com.gtnewhorizon.gtnhlib.config.ConfigException;
 import com.gtnewhorizon.gtnhlib.config.ConfigurationManager;
 import com.gtnewhorizon.gtnhmixins.IEarlyMixinLoader;
@@ -21,17 +22,24 @@ import org.spongepowered.asm.launch.GlobalProperties;
 import org.spongepowered.asm.service.mojang.MixinServiceLaunchWrapper;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+// ================== Important ==================
+// Due to a bug caused by this class both implementing
+// IFMLLoadingPlugin and IEarlyMixinLoader,
+// the IClassTransformer registered in this class
+// will not respect the sorting index defined.
+// They will instead use default index 0 which means they will see
+// obfuscated mappings and not SRG mappings when running outside of dev env.
+// ===============================================
+//@IFMLLoadingPlugin.SortingIndex(Integer.MAX_VALUE - 5)
 @IFMLLoadingPlugin.MCVersion("1.7.10")
 @IFMLLoadingPlugin.TransformerExclusions({
         "com.gtnewhorizons.angelica.transform.RedirectorTransformer",
         "com.gtnewhorizons.angelica.glsm.GLStateManager"})
-@IFMLLoadingPlugin.SortingIndex(Integer.MAX_VALUE - 5)
 public class AngelicaTweaker implements IFMLLoadingPlugin, IEarlyMixinLoader {
 
     private static final boolean DUMP_CLASSES = Boolean.parseBoolean(System.getProperty("angelica.dumpClass", "false"));
@@ -67,13 +75,6 @@ public class AngelicaTweaker implements IFMLLoadingPlugin, IEarlyMixinLoader {
         if (mixinTweakClasses != null) {
             mixinTweakClasses.add(MixinCompatHackTweaker.class.getName());
         }
-
-        // ================== Important ==================
-        // Due to a bug with mixins, the IClassTransformer registered here
-        // will not respect the sorting index defined in @IFMLLoadingPlugin.SortingIndex
-        // They will instead use default index 0 which means they will deal with
-        // obfuscated mappings and not SRG mappings when running outside of dev env
-        // ===============================================
         if (transformerClasses == null) {
             final List<String> transformers = new ArrayList<>(CompatASMTransformers.getTransformers());
             final List<String> notFineTransformers = AsmTransformers.getTransformers();
@@ -125,6 +126,13 @@ public class AngelicaTweaker implements IFMLLoadingPlugin, IEarlyMixinLoader {
         return OBF_ENV;
     }
 
+    public static void dumpClass(String className, byte[] originalBytes, byte[] transformedBytes, Object transformer) {
+        if (AngelicaTweaker.DUMP_CLASSES()) {
+            ASMUtil.saveAsRawClassFile(originalBytes, className + "_PRE", transformer);
+            ASMUtil.saveAsRawClassFile(transformedBytes, className + "_POST", transformer);
+        }
+    }
+
     private static final ImmutableMap<String, TargetedMod> MODS_BY_CLASS = ImmutableMap.<String, TargetedMod>builder()
         .put("optifine.OptiFineForgeTweaker", TargetedMod.OPTIFINE)
         .put("fastcraft.Tweaker", TargetedMod.FASTCRAFT)

src/main/java/com/gtnewhorizons/angelica/transform/BlockTransformer.java

@@ -1,10 +1,8 @@
 package com.gtnewhorizons.angelica.transform;
 
 import com.google.common.collect.ImmutableList;
-import com.gtnewhorizon.gtnhlib.asm.ASMUtil;
 import com.gtnewhorizons.angelica.loading.AngelicaTweaker;
 import net.minecraft.launchwrapper.IClassTransformer;
-import org.apache.commons.lang3.tuple.Pair;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassWriter;
 import org.objectweb.asm.tree.ClassNode;
@@ -14,14 +12,24 @@ import java.util.List;
 public class BlockTransformer implements IClassTransformer {
 
     private static final String BlockClassFriendly = "net.minecraft.block.Block";
-    public static final List<Pair<String, String>> BlockBoundsFields = ImmutableList.of(
+    private static final List<String> mcpMapping = ImmutableList.of(
-        Pair.of("minX", "field_149759_B"),
+        "minX",
-        Pair.of("minY", "field_149760_C"),
+        "minY",
-        Pair.of("minZ", "field_149754_D"),
+        "minZ",
-        Pair.of("maxX", "field_149755_E"),
+        "maxX",
-        Pair.of("maxY", "field_149756_F"),
+        "maxY",
-        Pair.of("maxZ", "field_149757_G")
+        "maxZ");
-    );
+    private static final List<String> srgMapping = ImmutableList.of(
+        "field_149759_B",
+        "field_149760_C",
+        "field_149754_D",
+        "field_149755_E",
+        "field_149756_F",
+        "field_149757_G");
+
+    public static List<String> getBlockBoundsFields() {
+        return AngelicaTweaker.isObfEnv() ? srgMapping : mcpMapping;
+    }
 
     /**
      * Delete the global vanilla bounding box fields off the Block object. {@link RedirectorTransformer}
@@ -33,16 +41,14 @@ public class BlockTransformer implements IClassTransformer {
             final ClassReader cr = new ClassReader(basicClass);
             final ClassNode cn = new ClassNode();
             cr.accept(cn, 0);
-            cn.fields.removeIf(field -> BlockBoundsFields.stream().anyMatch(pair -> field.name.equals(pair.getLeft()) || field.name.equals(pair.getRight())));
+            cn.fields.removeIf(field -> getBlockBoundsFields().contains(field.name));
-            ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS);
+            ClassWriter cw = new ClassWriter(0);
             cn.accept(cw);
             final byte[] bytes = cw.toByteArray();
-            if (AngelicaTweaker.DUMP_CLASSES()) {
+            AngelicaTweaker.dumpClass(transformedName, basicClass, bytes, this);
-                ASMUtil.saveAsRawClassFile(basicClass, transformedName + "_PRE", this);
-                ASMUtil.saveAsRawClassFile(bytes, transformedName + "_POST", this);
-            }
             return bytes;
         }
         return basicClass;
     }
+
 }

src/main/java/com/gtnewhorizons/angelica/transform/HUDCachingTransformer.java

@@ -2,7 +2,6 @@ package com.gtnewhorizons.angelica.transform;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.gtnewhorizon.gtnhlib.asm.ASMUtil;
 import com.gtnewhorizons.angelica.loading.AngelicaTweaker;
 import net.minecraft.launchwrapper.IClassTransformer;
 import org.spongepowered.asm.lib.ClassReader;
@@ -64,10 +63,7 @@ public class HUDCachingTransformer implements IClassTransformer, Opcodes {
             MixinClassWriter cw = new MixinClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES);
             cn.accept(cw);
             final byte[] bytes = cw.toByteArray();
-            if (AngelicaTweaker.DUMP_CLASSES()) {
+            AngelicaTweaker.dumpClass(transformedName, basicClass, bytes, this);
-                ASMUtil.saveAsRawClassFile(basicClass, transformedName + "_PRE", this);
-                ASMUtil.saveAsRawClassFile(bytes, transformedName + "_POST", this);
-            }
             return bytes;
         }
 

src/main/java/com/gtnewhorizons/angelica/transform/RedirectorTransformer.java

@@ -1,19 +1,17 @@
 package com.gtnewhorizons.angelica.transform;
 
-import com.google.common.collect.ImmutableSet;
-import com.gtnewhorizon.gtnhlib.asm.ASMUtil;
 import com.gtnewhorizon.gtnhlib.asm.ClassConstantPoolParser;
 import com.gtnewhorizons.angelica.loading.AngelicaTweaker;
 import net.coderbot.iris.IrisLogging;
 import net.minecraft.launchwrapper.IClassTransformer;
 import net.minecraft.launchwrapper.Launch;
-import org.apache.commons.lang3.tuple.Pair;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassWriter;
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.tree.AbstractInsnNode;
 import org.objectweb.asm.tree.ClassNode;
 import org.objectweb.asm.tree.FieldInsnNode;
+import org.objectweb.asm.tree.FieldNode;
 import org.objectweb.asm.tree.InsnList;
 import org.objectweb.asm.tree.InsnNode;
 import org.objectweb.asm.tree.IntInsnNode;
@@ -30,7 +28,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.stream.Collectors;
 
 /**
  * This transformer redirects many GL calls to our custom GLStateManager
@@ -52,13 +49,10 @@ public class RedirectorTransformer implements IClassTransformer {
     private static final String OpenGlHelper = "net/minecraft/client/renderer/OpenGlHelper";
     private static final String EXTBlendFunc = "org/lwjgl/opengl/EXTBlendFuncSeparate";
     private static final String ARBMultiTexture = "org/lwjgl/opengl/ARBMultitexture";
-    private static final String MinecraftClient = "net.minecraft.client";
-    private static final String SplashProgress = "cpw.mods.fml.client.SplashProgress";
     private static final String ThreadedBlockData = "com/gtnewhorizons/angelica/glsm/ThreadedBlockData";
-    private static final Set<String> ExcludedMinecraftMainThreadChecks = ImmutableSet.of(
+    private static final String MinecraftClient_dot = "net.minecraft.client.Minecraft";
-        "startGame", "func_71384_a",
+    private static final String SplashProgress_dot = "cpw.mods.fml.client.SplashProgress";
-        "initializeTextures", "func_77474_a"
+    private static final String OpenGlHelper_dot = "net.minecraft.client.renderer.OpenGlHelper";
-    );
     /** All classes in <tt>net.minecraft.block.*</tt> are the block subclasses save for these. */
     private static final List<String> VanillaBlockExclusions = Arrays.asList(
         "net/minecraft/block/IGrowable",
@@ -273,10 +267,7 @@ public class RedirectorTransformer implements IClassTransformer {
             final ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS);
             cn.accept(cw);
             final byte[] bytes = cw.toByteArray();
-            if (AngelicaTweaker.DUMP_CLASSES()) {
+            AngelicaTweaker.dumpClass(transformedName, basicClass, bytes, this);
-                ASMUtil.saveAsRawClassFile(basicClass, transformedName + "_PRE", this);
-                ASMUtil.saveAsRawClassFile(bytes, transformedName + "_POST", this);
-            }
             return bytes;
         }
         return basicClass;
@@ -295,18 +286,25 @@ public class RedirectorTransformer implements IClassTransformer {
         }
 
         // Check if this class shadows any fields of the parent class
-        if(moddedBlockSubclasses.contains(cn.name)) {
+        if (moddedBlockSubclasses.contains(cn.name)) {
-            // If a superclass shadows, then so do we, because JVM will resolve a reference on our class to that
+            // If a superclass shadows, then so do we, because JVM
-            // superclass
+            // will resolve a reference on our class to that superclass
-            boolean doWeShadow;
+            final boolean doWeShadow;
-            if(blockOwnerExclusions.contains(cn.superName)) {
+            if (blockOwnerExclusions.contains(cn.superName)) {
                 doWeShadow = true;
             } else {
                 // Check if we declare any known field names
-                Set<String> fieldsDeclaredByClass = cn.fields.stream().map(f -> f.name).collect(Collectors.toSet());
+                final List<String> blockBoundsNames = BlockTransformer.getBlockBoundsFields();
-                doWeShadow = BlockTransformer.BlockBoundsFields.stream().anyMatch(pair -> fieldsDeclaredByClass.contains(pair.getLeft()) || fieldsDeclaredByClass.contains(pair.getRight()));
+                boolean b = false;
+                for (FieldNode field : cn.fields) {
+                    if (blockBoundsNames.contains(field.name)) {
+                        b = true;
+                        break;
+                    }
+                }
+                doWeShadow = b;
             }
-            if(doWeShadow) {
+            if (doWeShadow) {
                 AngelicaTweaker.LOGGER.info("Class '{}' shadows one or more block bounds fields, these accesses won't be redirected!", cn.name);
                 blockOwnerExclusions.add(cn.name);
             }
@@ -314,7 +312,7 @@ public class RedirectorTransformer implements IClassTransformer {
 
         boolean changed = false;
         for (MethodNode mn : cn.methods) {
-            if (transformedName.equals("net.minecraft.client.renderer.OpenGlHelper") && (mn.name.equals("glBlendFunc") || mn.name.equals("func_148821_a"))) {
+            if (transformedName.equals(OpenGlHelper_dot) && (mn.name.equals("glBlendFunc") || mn.name.equals("func_148821_a"))) {
                 continue;
             }
             boolean redirectInMethod = false;
@@ -376,26 +374,26 @@ public class RedirectorTransformer implements IClassTransformer {
                     }
                 }
                 else if ((node.getOpcode() == Opcodes.GETFIELD || node.getOpcode() == Opcodes.PUTFIELD) && node instanceof FieldInsnNode fNode) {
-                    if(!blockOwnerExclusions.contains(fNode.owner) && isBlockSubclass(fNode.owner) && isSodiumEnabled()) {
+                    if (!blockOwnerExclusions.contains(fNode.owner) && isBlockSubclass(fNode.owner) && isSodiumEnabled()) {
-                        Pair<String, String> fieldToRedirect = null;
+                        String fieldToRedirect = null;
-                        for(Pair<String, String> blockPairs : BlockTransformer.BlockBoundsFields) {
+                        for (String name : BlockTransformer.getBlockBoundsFields()) {
-                            if(fNode.name.equals(blockPairs.getLeft()) || fNode.name.equals(blockPairs.getRight())) {
+                            if (fNode.name.equals(name)) {
-                                fieldToRedirect = blockPairs;
+                                fieldToRedirect = name;
                                 break;
                             }
                         }
-                        if(fieldToRedirect != null) {
+                        if (fieldToRedirect != null) {
                             if (IrisLogging.ENABLE_SPAM) {
                                 AngelicaTweaker.LOGGER.info("Redirecting Block.{} in {} to thread-safe wrapper", fNode.name, transformedName);
                             }
                             // Perform the redirect
-                            fNode.name = fieldToRedirect.getLeft(); // use unobfuscated name
+                            fNode.name = fieldToRedirect;
                             fNode.owner = ThreadedBlockData;
                             // Inject getter before the field access, to turn Block -> ThreadedBlockData
                             final MethodInsnNode getter = new MethodInsnNode(Opcodes.INVOKESTATIC, ThreadedBlockData, "get", "(L" + BlockClass + ";)L" + ThreadedBlockData + ";", false);
-                            if(node.getOpcode() == Opcodes.GETFIELD) {
+                            if (node.getOpcode() == Opcodes.GETFIELD) {
                                 mn.instructions.insertBefore(fNode, getter);
-                            } else if(node.getOpcode() == Opcodes.PUTFIELD) {
+                            } else if (node.getOpcode() == Opcodes.PUTFIELD) {
                                 // FIXME: this code assumes doubles
                                 // Stack: Block, double
                                 final InsnList beforePut = new InsnList();
@@ -413,11 +411,10 @@ public class RedirectorTransformer implements IClassTransformer {
                             }
                             changed = true;
                         }
-
                     }
                 }
             }
-            if (ASSERT_MAIN_THREAD && redirectInMethod && !transformedName.startsWith(SplashProgress) && !(transformedName.startsWith(MinecraftClient) && ExcludedMinecraftMainThreadChecks.contains(mn.name))) {
+            if (ASSERT_MAIN_THREAD && redirectInMethod && !isExcludeFromMainThreadAssert(transformedName, mn.name)) {
                 mn.instructions.insert(new MethodInsnNode(Opcodes.INVOKESTATIC, GLStateManager, "assertMainThread", "()V", false));
             }
         }
@@ -425,6 +422,16 @@ public class RedirectorTransformer implements IClassTransformer {
         return changed;
     }
 
+    private static boolean isExcludeFromMainThreadAssert(String className, String methodName) {
+        if (className.startsWith(SplashProgress_dot)) {
+            return true;
+        }
+        if (MinecraftClient_dot.equals(className) && methodName.equals(AngelicaTweaker.isObfEnv() ? "func_71384_a" :"startGame")) {
+            return true;
+        }
+        return OpenGlHelper_dot.equals(className) && methodName.equals(AngelicaTweaker.isObfEnv() ? "func_77474_a" : "initializeTextures");
+    }
+
     private static class RedirectMap<K> extends HashMap<K, K> {
         public static RedirectMap<String> newMap() {
             return new RedirectMap<>();

src/main/java/com/gtnewhorizons/angelica/transform/compat/ExtraUtilsTransformer.java

@@ -1,6 +1,5 @@
 package com.gtnewhorizons.angelica.transform.compat;
 
-import com.gtnewhorizon.gtnhlib.asm.ASMUtil;
 import com.gtnewhorizons.angelica.loading.AngelicaTweaker;
 import net.minecraft.launchwrapper.IClassTransformer;
 import org.objectweb.asm.ClassReader;
@@ -53,10 +52,7 @@ public class ExtraUtilsTransformer implements IClassTransformer {
         ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS);
         cn.accept(cw);
         final byte[] bytes = cw.toByteArray();
-        if (AngelicaTweaker.DUMP_CLASSES()) {
+        AngelicaTweaker.dumpClass(transformedName, basicClass, bytes, this);
-            ASMUtil.saveAsRawClassFile(basicClass, transformedName + "_PRE", this);
-            ASMUtil.saveAsRawClassFile(bytes, transformedName + "_POST", this);
-        }
         return bytes;
     }
 

src/main/java/jss/notfine/asm/RenderBlocksTransformer.java

@@ -2,7 +2,6 @@ package jss.notfine.asm;
 
 import static jss.notfine.asm.ASMUtils.matchesNodeSequence;
 
-import com.gtnewhorizon.gtnhlib.asm.ASMUtil;
 import com.gtnewhorizons.angelica.loading.AngelicaTweaker;
 import net.minecraft.launchwrapper.IClassTransformer;
 
@@ -35,10 +34,7 @@ public class RenderBlocksTransformer implements IClassTransformer {
     public byte[] transform(String name, String transformedName, byte[] basicClass) {
         if ("net.minecraft.client.renderer.RenderBlocks".equals(transformedName)) {
             final byte[] bytes = patchRenderBlocks(basicClass);
-            if (AngelicaTweaker.DUMP_CLASSES()) {
+            AngelicaTweaker.dumpClass(transformedName, basicClass, bytes, this);
-                ASMUtil.saveAsRawClassFile(basicClass, transformedName + "_PRE", this);
-                ASMUtil.saveAsRawClassFile(bytes, transformedName + "_POST", this);
-            }
             return bytes;
         }
         return basicClass;

src/main/java/jss/notfine/asm/WorldRendererTransformer.java

@@ -2,7 +2,6 @@ package jss.notfine.asm;
 
 import static jss.notfine.asm.ASMUtils.matchesNodeSequence;
 
-import com.gtnewhorizon.gtnhlib.asm.ASMUtil;
 import com.gtnewhorizons.angelica.loading.AngelicaTweaker;
 import net.minecraft.launchwrapper.IClassTransformer;
 
@@ -29,10 +28,7 @@ public class WorldRendererTransformer implements IClassTransformer {
     public byte[] transform(String name, String transformedName, byte[] basicClass) {
         if ("net.minecraft.client.renderer.WorldRenderer".equals(transformedName)) {
             final byte[] bytes = patchWorldRenderer(basicClass);
-            if (AngelicaTweaker.DUMP_CLASSES()) {
+            AngelicaTweaker.dumpClass(transformedName, basicClass, bytes, this);
-                ASMUtil.saveAsRawClassFile(basicClass, transformedName + "_PRE", this);
-                ASMUtil.saveAsRawClassFile(bytes, transformedName + "_POST", this);
-            }
             return bytes;
         }
         return basicClass;