From 7ede64fbf2950110305baaa9b9c1681d1ef54b94 Mon Sep 17 00:00:00 2001 From: diesieben07 Date: Thu, 22 May 2014 14:25:23 +0200 Subject: [PATCH 01/11] Make AccessTransformer change INVOKESPECIAL to INVOKEVIRTUAL when making methods visible --- .../asm/transformers/AccessTransformer.java | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/fml/src/main/java/cpw/mods/fml/common/asm/transformers/AccessTransformer.java b/fml/src/main/java/cpw/mods/fml/common/asm/transformers/AccessTransformer.java index efb919ed30..76120c1a48 100644 --- a/fml/src/main/java/cpw/mods/fml/common/asm/transformers/AccessTransformer.java +++ b/fml/src/main/java/cpw/mods/fml/common/asm/transformers/AccessTransformer.java @@ -16,6 +16,8 @@ import static org.objectweb.asm.Opcodes.ACC_FINAL; import static org.objectweb.asm.Opcodes.ACC_PRIVATE; import static org.objectweb.asm.Opcodes.ACC_PROTECTED; import static org.objectweb.asm.Opcodes.ACC_PUBLIC; +import static org.objectweb.asm.Opcodes.INVOKESPECIAL; +import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL; import java.io.BufferedInputStream; import java.io.BufferedOutputStream; @@ -27,6 +29,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.net.URL; import java.util.Collection; +import java.util.Iterator; import java.util.List; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -36,8 +39,10 @@ import net.minecraft.launchwrapper.IClassTransformer; import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.tree.AbstractInsnNode; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.FieldNode; +import org.objectweb.asm.tree.MethodInsnNode; import org.objectweb.asm.tree.MethodNode; import com.google.common.base.Charsets; @@ -206,11 +211,28 @@ public class AccessTransformer implements IClassTransformer } else { + List nowOverridable = Lists.newArrayList(); for (MethodNode n : classNode.methods) { if ((n.name.equals(m.name) && n.desc.equals(m.desc)) || m.name.equals("*")) { n.access = getFixedAccess(n.access, m); + + // constructors always use INVOKESPECIAL + if (!n.name.equals("")) + { + // if we changed from private to something else we need to replace all INVOKESPECIAL calls to this method with INVOKEVIRTUAL + // so that overridden methods will be called. Only need to scan this class, because obviously the method was private. + boolean wasPrivate = (m.oldAccess & ACC_PRIVATE) == ACC_PRIVATE; + boolean isNowPrivate = (m.newAccess & ACC_PRIVATE) == ACC_PRIVATE; + + if (wasPrivate && !isNowPrivate) + { + nowOverridable.add(n); + } + + } + if (DEBUG) { System.out.println(String.format("Method: %s.%s%s %s -> %s", name, n.name, n.desc, toBinary(m.oldAccess), toBinary(m.newAccess))); @@ -222,6 +244,8 @@ public class AccessTransformer implements IClassTransformer } } } + + replaceInvokeSpecial(classNode, nowOverridable); } } @@ -229,6 +253,29 @@ public class AccessTransformer implements IClassTransformer classNode.accept(writer); return writer.toByteArray(); } + + private void replaceInvokeSpecial(ClassNode clazz, List toReplace) + { + for (MethodNode method : clazz.methods) + { + for (Iterator it = method.instructions.iterator(); it.hasNext();) + { + AbstractInsnNode insn = it.next(); + if (insn.getOpcode() == INVOKESPECIAL) + { + MethodInsnNode mInsn = (MethodInsnNode) insn; + for (MethodNode n : toReplace) + { + if (n.name.equals(mInsn.name) && n.desc.equals(mInsn.desc)) + { + mInsn.setOpcode(INVOKEVIRTUAL); + break; + } + } + } + } + } + } private String toBinary(int num) { From 2c5fcc612979ff2ca10249a5dcb1d28fa6382f24 Mon Sep 17 00:00:00 2001 From: Christian Date: Sat, 24 May 2014 16:54:48 -0400 Subject: [PATCH 02/11] Fix the null networkHandler in the MessageContext --- .../common/network/simpleimpl/SimpleChannelHandlerWrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/src/main/java/cpw/mods/fml/common/network/simpleimpl/SimpleChannelHandlerWrapper.java b/fml/src/main/java/cpw/mods/fml/common/network/simpleimpl/SimpleChannelHandlerWrapper.java index 5f6c76c439..dbf089502b 100644 --- a/fml/src/main/java/cpw/mods/fml/common/network/simpleimpl/SimpleChannelHandlerWrapper.java +++ b/fml/src/main/java/cpw/mods/fml/common/network/simpleimpl/SimpleChannelHandlerWrapper.java @@ -28,7 +28,7 @@ public class SimpleChannelHandlerWrapper Date: Sat, 24 May 2014 22:32:24 -0400 Subject: [PATCH 03/11] Add in the objectholder, autopopulated by FML based on simple rules. Allows for reference driven substitution of mod blocks and items based on their server running state. --- .../main/java/cpw/mods/fml/common/Loader.java | 3 + .../mods/fml/common/registry/GameData.java | 4 + .../fml/common/registry/GameRegistry.java | 24 ++++ .../fml/common/registry/ObjectHolderRef.java | 101 ++++++++++++++ .../common/registry/ObjectHolderRegistry.java | 123 ++++++++++++++++++ 5 files changed, 255 insertions(+) create mode 100644 fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRef.java create mode 100644 fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRegistry.java diff --git a/fml/src/main/java/cpw/mods/fml/common/Loader.java b/fml/src/main/java/cpw/mods/fml/common/Loader.java index f59c84117f..375ba0833d 100644 --- a/fml/src/main/java/cpw/mods/fml/common/Loader.java +++ b/fml/src/main/java/cpw/mods/fml/common/Loader.java @@ -62,6 +62,7 @@ import cpw.mods.fml.common.functions.ArtifactVersionNameFunction; import cpw.mods.fml.common.functions.ModIdFunction; import cpw.mods.fml.common.registry.GameData; import cpw.mods.fml.common.registry.GameRegistry.Type; +import cpw.mods.fml.common.registry.ObjectHolderRegistry; import cpw.mods.fml.common.toposort.ModSorter; import cpw.mods.fml.common.toposort.ModSortingException; import cpw.mods.fml.common.toposort.ModSortingException.SortingExceptionData; @@ -502,12 +503,14 @@ public class Loader public void preinitializeMods() { + ObjectHolderRegistry.INSTANCE.findObjectHolders(discoverer.getASMTable()); if (!modController.isInState(LoaderState.PREINITIALIZATION)) { FMLLog.warning("There were errors previously. Not beginning mod initialization phase"); return; } modController.distributeStateMessage(LoaderState.PREINITIALIZATION, discoverer.getASMTable(), canonicalConfigDir); + ObjectHolderRegistry.INSTANCE.applyObjectHolders(); modController.transition(LoaderState.INITIALIZATION, false); } diff --git a/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java b/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java index 79d92d78de..d253fd042a 100644 --- a/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java +++ b/fml/src/main/java/cpw/mods/fml/common/registry/GameData.java @@ -530,6 +530,8 @@ public class GameData { getMain().iBlockRegistry.dump(); getMain().iItemRegistry.dump(); Loader.instance().fireRemapEvent(remaps); + // The id map changed, ensure we apply object holders + ObjectHolderRegistry.INSTANCE.applyObjectHolders(); return ImmutableList.of(); } @@ -664,6 +666,8 @@ public class GameData { getMain().set(frozen); } + // the id mapping has reverted, ensure we sync up the object holders + ObjectHolderRegistry.INSTANCE.applyObjectHolders(); } protected static boolean isFrozen(FMLControlledNamespacedRegistry registry) diff --git a/fml/src/main/java/cpw/mods/fml/common/registry/GameRegistry.java b/fml/src/main/java/cpw/mods/fml/common/registry/GameRegistry.java index da0352db83..dfd90f6252 100644 --- a/fml/src/main/java/cpw/mods/fml/common/registry/GameRegistry.java +++ b/fml/src/main/java/cpw/mods/fml/common/registry/GameRegistry.java @@ -12,6 +12,10 @@ package cpw.mods.fml.common.registry; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; import java.lang.reflect.Constructor; import java.util.ArrayList; import java.util.Collections; @@ -424,4 +428,24 @@ public class GameRegistry return GameData.getUniqueName(item); } + + + /** + * This will cause runtime injection of public static final fields to occur at various points + * where mod blocks and items could be subject to change. This allows for dynamic + * substitution to occur. + * + */ + @Retention(RetentionPolicy.RUNTIME) + @Target({ElementType.TYPE, ElementType.FIELD}) + public @interface ObjectHolder { + /** + * If used on a class, this represents a modid only. + * If used on a field, it represents a name, which can be abbreviated or complete. + * Abbreviated names derive their modid from an enclosing ObjectHolder at the class level. + * + * @return either a modid or a name based on the rules above + */ + String value(); + } } diff --git a/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRef.java b/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRef.java new file mode 100644 index 0000000000..65e2082c71 --- /dev/null +++ b/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRef.java @@ -0,0 +1,101 @@ +package cpw.mods.fml.common.registry; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import org.apache.logging.log4j.Level; +import com.google.common.base.Throwables; +import cpw.mods.fml.common.FMLLog; +import cpw.mods.fml.common.registry.GameRegistry.ObjectHolder; +import net.minecraft.block.Block; +import net.minecraft.item.Item; + + +/** + * Internal class used in tracking {@link ObjectHolder} references + * + * @author cpw + * + */ +class ObjectHolderRef { + private Field field; + private String injectedObject; + private boolean isBlock; + private boolean isItem; + + + ObjectHolderRef(Field field, String injectedObject, boolean extractFromExistingValues) + { + this.field = field; + this.isBlock = Block.class.isAssignableFrom(field.getType()); + this.isItem = Item.class.isAssignableFrom(field.getType()); + if (extractFromExistingValues) + { + try + { + Object existing = field.get(null); + this.injectedObject = isBlock ? GameData.getBlockRegistry().getNameForObject(existing) : + isItem ? GameData.getItemRegistry().getNameForObject(existing) : null; + } catch (Exception e) + { + throw Throwables.propagate(e); + } + } + else + { + this.injectedObject = injectedObject; + } + + if (this.injectedObject == null || !isValid()) + { + throw new IllegalStateException("The ObjectHolder annotation cannot apply to a field that is not an Item or Block"); + } + makeWritable(field); + } + + private static Field modifiersField; + private static Object reflectionFactory; + private static Method newFieldAccessor; + private static Method fieldAccessorSet; + private static void makeWritable(Field f) + { + try + { + if (modifiersField == null) + { + Method getReflectionFactory = Class.forName("sun.reflect.ReflectionFactory").getDeclaredMethod("getReflectionFactory"); + reflectionFactory = getReflectionFactory.invoke(null); + newFieldAccessor = Class.forName("sun.reflect.ReflectionFactory").getDeclaredMethod("newFieldAccessor", Field.class, boolean.class); + fieldAccessorSet = Class.forName("sun.reflect.FieldAccessor").getDeclaredMethod("set", Object.class, Object.class); + modifiersField = Field.class.getDeclaredField("modifiers"); + modifiersField.setAccessible(true); + } + modifiersField.setInt(f, f.getModifiers() & ~Modifier.FINAL); + } catch (Exception e) + { + throw Throwables.propagate(e); + } + } + + public boolean isValid() + { + return isBlock || isItem; + } + public void apply() + { + Object thing; + if (isBlock) + thing = GameData.getBlockRegistry().getObject(injectedObject); + else + thing = GameData.getItemRegistry().getObject(injectedObject); + try + { + Object fieldAccessor = newFieldAccessor.invoke(reflectionFactory, field, false); + fieldAccessorSet.invoke(fieldAccessor, null, thing); + } + catch (Exception e) + { + FMLLog.log(Level.WARN, e, "Unable to set %s with value %s (%s)", this.field, thing, this.injectedObject); + } + } +} diff --git a/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRegistry.java b/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRegistry.java new file mode 100644 index 0000000000..d4dbb0c7f6 --- /dev/null +++ b/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRegistry.java @@ -0,0 +1,123 @@ +package cpw.mods.fml.common.registry; + +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.List; +import java.util.Map; +import java.util.Set; +import com.google.common.base.Throwables; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import cpw.mods.fml.common.FMLLog; +import cpw.mods.fml.common.discovery.ASMDataTable; +import cpw.mods.fml.common.discovery.ASMDataTable.ASMData; +import cpw.mods.fml.common.registry.GameRegistry.ObjectHolder; + +/** + * Internal registry for tracking {@link ObjectHolder} references + * @author cpw + * + */ +public enum ObjectHolderRegistry { + INSTANCE; + private List objectHolders = Lists.newArrayList(); + + public void findObjectHolders(ASMDataTable table) + { + FMLLog.info("Processing ObjectHolder annotations"); + Set allObjectHolders = table.getAll(GameRegistry.ObjectHolder.class.getName()); + Map classModIds = Maps.newHashMap(); + Map> classCache = Maps.newHashMap(); + for (ASMData data : allObjectHolders) + { + String className = data.getClassName(); + String annotationTarget = data.getObjectName(); + String value = (String) data.getAnnotationInfo().get("value"); + boolean isClass = className.equals(annotationTarget); + scanTarget(classModIds, classCache, className, annotationTarget, value, isClass, false); + } + scanTarget(classModIds, classCache, "net.minecraft.init.Blocks", null, "minecraft", true, true); + scanTarget(classModIds, classCache, "net.minecraft.init.Items", null, "minecraft", true, true); + FMLLog.info("Found %d ObjectHolder annotations", objectHolders.size()); + } + + private void scanTarget(Map classModIds, Map> classCache, String className, String annotationTarget, String value, boolean isClass, boolean extractFromValue) + { + Class clazz; + if (classCache.containsKey(className)) + { + clazz = classCache.get(className); + } + else + { + try + { + clazz = Class.forName(className, true, getClass().getClassLoader()); + classCache.put(className, clazz); + } + catch (Exception ex) + { + // unpossible? + throw Throwables.propagate(ex); + } + } + if (isClass) + { + scanClassForFields(classModIds, className, value, clazz, extractFromValue); + } + else + { + if (value.indexOf(':') == -1) + { + String prefix = classModIds.get(className); + if (prefix == null) + { + FMLLog.warning("Found an unqualified ObjectHolder annotation (%s) without a modid context at %s.%s, ignoring", value, className, annotationTarget); + throw new IllegalStateException("Unqualified reference to ObjectHolder"); + } + value = prefix + ":" + value; + } + try + { + Field f = clazz.getField(annotationTarget); + addHolderReference(new ObjectHolderRef(f, value, extractFromValue)); + } + catch (Exception ex) + { + // unpossible? + throw Throwables.propagate(ex); + } + } + } + + private void scanClassForFields(Map classModIds, String className, String value, Class clazz, boolean extractFromExistingValues) + { + classModIds.put(className, value); + for (Field f : clazz.getFields()) + { + int mods = f.getModifiers(); + boolean isMatch = Modifier.isPublic(mods) && Modifier.isStatic(mods) && Modifier.isFinal(mods); + if (!isMatch || f.isAnnotationPresent(ObjectHolder.class)) + { + continue; + } + addHolderReference(new ObjectHolderRef(f, value + ":"+ f.getName(), extractFromExistingValues)); + } + } + + private void addHolderReference(ObjectHolderRef ref) + { + objectHolders.add(ref); + } + + public void applyObjectHolders() + { + FMLLog.info("Applying holder lookups"); + for (ObjectHolderRef ohr : objectHolders) + { + ohr.apply(); + } + FMLLog.info("Holder lookups applied"); + } + +} From e69fd72d4277aae54ae74d785802e7a6604837af Mon Sep 17 00:00:00 2001 From: Lex Manos Date: Sat, 24 May 2014 20:44:25 -0700 Subject: [PATCH 04/11] Fixed deobfusication of nested inner classes. And removed legacy ModLoader remaps. --- .../deobf/FMLDeobfuscatingRemapper.java | 45 +++++-------------- 1 file changed, 11 insertions(+), 34 deletions(-) diff --git a/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java b/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java index f8788b077f..f0eb578dee 100644 --- a/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java +++ b/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java @@ -53,7 +53,6 @@ public class FMLDeobfuscatingRemapper extends Remapper { public static final FMLDeobfuscatingRemapper INSTANCE = new FMLDeobfuscatingRemapper(); private BiMap classNameBiMap; - private BiMap mcpNameBiMap; private Map> rawFieldMaps; private Map> rawMethodMaps; @@ -71,7 +70,6 @@ public class FMLDeobfuscatingRemapper extends Remapper { private FMLDeobfuscatingRemapper() { classNameBiMap=ImmutableBiMap.of(); - mcpNameBiMap=ImmutableBiMap.of(); } public void setupLoadOnly(String deobfFileName, boolean loadAll) @@ -85,7 +83,6 @@ public class FMLDeobfuscatingRemapper extends Remapper { rawMethodMaps = Maps.newHashMap(); rawFieldMaps = Maps.newHashMap(); Builder builder = ImmutableBiMap.builder(); - Builder mcpBuilder = ImmutableBiMap.builder(); Splitter splitter = Splitter.on(CharMatcher.anyOf(": ")).omitEmptyStrings().trimResults(); for (String line : srgList) { @@ -94,7 +91,6 @@ public class FMLDeobfuscatingRemapper extends Remapper { if ("CL".equals(typ)) { parseClass(builder, parts); - parseMCPClass(mcpBuilder,parts); } else if ("MD".equals(typ) && loadAll) { @@ -106,7 +102,6 @@ public class FMLDeobfuscatingRemapper extends Remapper { } } classNameBiMap = builder.build(); - mcpNameBiMap = mcpBuilder.build(); } catch (IOException ioe) { @@ -128,7 +123,6 @@ public class FMLDeobfuscatingRemapper extends Remapper { rawMethodMaps = Maps.newHashMap(); rawFieldMaps = Maps.newHashMap(); Builder builder = ImmutableBiMap.builder(); - Builder mcpBuilder = ImmutableBiMap.builder(); Splitter splitter = Splitter.on(CharMatcher.anyOf(": ")).omitEmptyStrings().trimResults(); for (String line : srgList) { @@ -137,7 +131,6 @@ public class FMLDeobfuscatingRemapper extends Remapper { if ("CL".equals(typ)) { parseClass(builder, parts); - parseMCPClass(mcpBuilder,parts); } else if ("MD".equals(typ)) { @@ -149,13 +142,6 @@ public class FMLDeobfuscatingRemapper extends Remapper { } } classNameBiMap = builder.build(); - // Special case some mappings for modloader mods - mcpBuilder.put("BaseMod","net/minecraft/src/BaseMod"); - mcpBuilder.put("ModLoader","net/minecraft/src/ModLoader"); - mcpBuilder.put("EntityRendererProxy","net/minecraft/src/EntityRendererProxy"); - mcpBuilder.put("MLProp","net/minecraft/src/MLProp"); - mcpBuilder.put("TradeEntry","net/minecraft/src/TradeEntry"); - mcpNameBiMap = mcpBuilder.build(); } catch (IOException ioe) { @@ -235,12 +221,6 @@ public class FMLDeobfuscatingRemapper extends Remapper { builder.put(parts[1],parts[2]); } - private void parseMCPClass(Builder builder, String[] parts) - { - int clIdx = parts[2].lastIndexOf('/'); - builder.put("net/minecraft/src/"+parts[2].substring(clIdx+1),parts[2]); - } - private void parseMethod(String[] parts) { String oldSrg = parts[1]; @@ -280,13 +260,12 @@ public class FMLDeobfuscatingRemapper extends Remapper { { return classNameBiMap.get(typeName); } - int dollarIdx = typeName.indexOf('$'); - String realType = dollarIdx > -1 ? typeName.substring(0, dollarIdx) : typeName; - String subType = dollarIdx > -1 ? typeName.substring(dollarIdx+1) : ""; - - String result = classNameBiMap.containsKey(realType) ? classNameBiMap.get(realType) : mcpNameBiMap.containsKey(realType) ? mcpNameBiMap.get(realType) : realType; - result = dollarIdx > -1 ? result+"$"+subType : result; - return result; + int dollarIdx = typeName.lastIndexOf('$'); + if (dollarIdx > -1) + { + return map(typeName.substring(0, dollarIdx)) + "$" + typeName.substring(dollarIdx + 1); + } + return typeName; } public String unmap(String typeName) @@ -301,13 +280,11 @@ public class FMLDeobfuscatingRemapper extends Remapper { return classNameBiMap.inverse().get(typeName); } int dollarIdx = typeName.indexOf('$'); - String realType = dollarIdx > -1 ? typeName.substring(0, dollarIdx) : typeName; - String subType = dollarIdx > -1 ? typeName.substring(dollarIdx+1) : ""; - - - String result = classNameBiMap.containsValue(realType) ? classNameBiMap.inverse().get(realType) : mcpNameBiMap.containsValue(realType) ? mcpNameBiMap.inverse().get(realType) : realType; - result = dollarIdx > -1 ? result+"$"+subType : result; - return result; + if (dollarIdx > -1) + { + return unmap(typeName.substring(0, dollarIdx)) + "$" + typeName.substring(dollarIdx + 1); + } + return typeName; } From cad049caa12a727e4546a570e3e3c2a8e8b2f331 Mon Sep 17 00:00:00 2001 From: Lex Manos Date: Sat, 24 May 2014 21:11:08 -0700 Subject: [PATCH 05/11] -.- Save File First. My Bad. --- .../asm/transformers/deobf/FMLDeobfuscatingRemapper.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java b/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java index f0eb578dee..c25c0790eb 100644 --- a/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java +++ b/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java @@ -153,8 +153,7 @@ public class FMLDeobfuscatingRemapper extends Remapper { public boolean isRemappedClass(String className) { - className = className.replace('.', '/'); - return classNameBiMap.containsKey(className) || mcpNameBiMap.containsKey(className) || (!classNameBiMap.isEmpty() && className.indexOf('/') == -1); + return !map(className).equals(className); } private void parseField(String[] parts) From bcec606b127776b6a26462b4eb757b2a903d50da Mon Sep 17 00:00:00 2001 From: Lex Manos Date: Sat, 24 May 2014 21:48:55 -0700 Subject: [PATCH 06/11] lastIndexOf, Note Don't code while sick. --- .../common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java b/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java index c25c0790eb..2d6d451ae8 100644 --- a/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java +++ b/fml/src/main/java/cpw/mods/fml/common/asm/transformers/deobf/FMLDeobfuscatingRemapper.java @@ -278,7 +278,7 @@ public class FMLDeobfuscatingRemapper extends Remapper { { return classNameBiMap.inverse().get(typeName); } - int dollarIdx = typeName.indexOf('$'); + int dollarIdx = typeName.lastIndexOf('$'); if (dollarIdx > -1) { return unmap(typeName.substring(0, dollarIdx)) + "$" + typeName.substring(dollarIdx + 1); From d932a237518aaf5240c5e7efbe80210217899b52 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 26 May 2014 10:58:13 -0400 Subject: [PATCH 07/11] Warn when the objectholder finds nothing in the registry. Helps debug mismatched names. Also, actually make the scoping thing work with objectholder Also handle Blocks.air as a special case. It should never be a valid block in the block registry. Nothing can ever replace the default block registry block. It is disabled for replacement. Air is properly skipped --- .../FMLControlledNamespacedRegistry.java | 7 +++- .../fml/common/registry/ObjectHolderRef.java | 37 +++++++++++++++++-- .../common/registry/ObjectHolderRegistry.java | 22 ++++++++++- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/fml/src/main/java/cpw/mods/fml/common/registry/FMLControlledNamespacedRegistry.java b/fml/src/main/java/cpw/mods/fml/common/registry/FMLControlledNamespacedRegistry.java index a53d9250ff..6375e64d13 100644 --- a/fml/src/main/java/cpw/mods/fml/common/registry/FMLControlledNamespacedRegistry.java +++ b/fml/src/main/java/cpw/mods/fml/common/registry/FMLControlledNamespacedRegistry.java @@ -330,7 +330,7 @@ public class FMLControlledNamespacedRegistry extends RegistryNamespaced { if (name.isEmpty()) throw new IllegalArgumentException(String.format("Can't use an empty name for the registry, object %s.", thing)); if (name.indexOf(':') == -1) throw new IllegalArgumentException(String.format("Can't add the name (%s) without a prefix, object %s", name, thing)); if (thing == null) throw new NullPointerException(String.format("Can't add null-object to the registry, name %s.", name)); - if (name.equals(optionalDefaultName)) + if (name.equals(optionalDefaultName) && this.optionalDefaultObject == null) { this.optionalDefaultObject = thing; } @@ -420,4 +420,9 @@ public class FMLControlledNamespacedRegistry extends RegistryNamespaced { underlyingIntegerMap.func_148746_a(thing, id); // obj <-> id super.putObject(name, thing); // name <-> obj } + + public I getDefaultValue() + { + return optionalDefaultObject; + } } diff --git a/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRef.java b/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRef.java index 65e2082c71..c072d3a525 100644 --- a/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRef.java +++ b/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRef.java @@ -8,6 +8,7 @@ import com.google.common.base.Throwables; import cpw.mods.fml.common.FMLLog; import cpw.mods.fml.common.registry.GameRegistry.ObjectHolder; import net.minecraft.block.Block; +import net.minecraft.init.Blocks; import net.minecraft.item.Item; @@ -34,8 +35,20 @@ class ObjectHolderRef { try { Object existing = field.get(null); - this.injectedObject = isBlock ? GameData.getBlockRegistry().getNameForObject(existing) : - isItem ? GameData.getItemRegistry().getNameForObject(existing) : null; + // nothing is ever allowed to replace AIR + if (existing == null || existing == GameData.getBlockRegistry().getDefaultValue()) + { + this.injectedObject = null; + this.field = null; + this.isBlock = false; + this.isItem = false; + return; + } + else + { + this.injectedObject = isBlock ? GameData.getBlockRegistry().getNameForObject(existing) : + isItem ? GameData.getItemRegistry().getNameForObject(existing) : null; + } } catch (Exception e) { throw Throwables.propagate(e); @@ -85,9 +98,27 @@ class ObjectHolderRef { { Object thing; if (isBlock) + { thing = GameData.getBlockRegistry().getObject(injectedObject); - else + if (thing == Blocks.air) + { + thing = null; + } + } + else if (isItem) + { thing = GameData.getItemRegistry().getObject(injectedObject); + } + else + { + thing = null; + } + + if (thing == null) + { + FMLLog.warning("Unable to lookup %s for %s. Is there something wrong with the registry?", injectedObject, field); + return; + } try { Object fieldAccessor = newFieldAccessor.invoke(reflectionFactory, field, false); diff --git a/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRegistry.java b/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRegistry.java index d4dbb0c7f6..b1173e25e4 100644 --- a/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRegistry.java +++ b/fml/src/main/java/cpw/mods/fml/common/registry/ObjectHolderRegistry.java @@ -34,7 +34,22 @@ public enum ObjectHolderRegistry { String annotationTarget = data.getObjectName(); String value = (String) data.getAnnotationInfo().get("value"); boolean isClass = className.equals(annotationTarget); - scanTarget(classModIds, classCache, className, annotationTarget, value, isClass, false); + if (isClass) + { + scanTarget(classModIds, classCache, className, annotationTarget, value, isClass, false); + } + } + // double pass - get all the class level annotations first, then the field level annotations + for (ASMData data : allObjectHolders) + { + String className = data.getClassName(); + String annotationTarget = data.getObjectName(); + String value = (String) data.getAnnotationInfo().get("value"); + boolean isClass = className.equals(annotationTarget); + if (!isClass) + { + scanTarget(classModIds, classCache, className, annotationTarget, value, isClass, false); + } } scanTarget(classModIds, classCache, "net.minecraft.init.Blocks", null, "minecraft", true, true); scanTarget(classModIds, classCache, "net.minecraft.init.Items", null, "minecraft", true, true); @@ -107,7 +122,10 @@ public enum ObjectHolderRegistry { private void addHolderReference(ObjectHolderRef ref) { - objectHolders.add(ref); + if (ref.isValid()) + { + objectHolders.add(ref); + } } public void applyObjectHolders() From c56fc3b490dcee91f3ccb85dd27745dfdabf8522 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 26 May 2014 14:57:14 -0400 Subject: [PATCH 08/11] Fix the side for the compatibility check. Should result in green ticks finally! --- fml/src/main/java/cpw/mods/fml/client/FMLClientHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/src/main/java/cpw/mods/fml/client/FMLClientHandler.java b/fml/src/main/java/cpw/mods/fml/client/FMLClientHandler.java index 46475e8c80..558a425f49 100644 --- a/fml/src/main/java/cpw/mods/fml/client/FMLClientHandler.java +++ b/fml/src/main/java/cpw/mods/fml/client/FMLClientHandler.java @@ -716,7 +716,7 @@ public class FMLClientHandler implements IFMLSidedHandler } Map modListMap = modListBldr.build(); - serverDataTag.put(data, new ExtendedServerListData(type, FMLNetworkHandler.checkModList(modListMap, Side.CLIENT) == null, modListMap, !moddedClientAllowed)); + serverDataTag.put(data, new ExtendedServerListData(type, FMLNetworkHandler.checkModList(modListMap, Side.SERVER) == null, modListMap, !moddedClientAllowed)); } else { From 72fbc45a19eab04184e1e3e97ad7be6818d97742 Mon Sep 17 00:00:00 2001 From: Christian Date: Tue, 27 May 2014 17:59:53 -0400 Subject: [PATCH 09/11] If a packet is not handled by the channel, log it, and clear it. Could be spammy, but this is a serious bug and could cause a memory leak otherwise. --- .../mods/fml/common/network/internal/FMLProxyPacket.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fml/src/main/java/cpw/mods/fml/common/network/internal/FMLProxyPacket.java b/fml/src/main/java/cpw/mods/fml/common/network/internal/FMLProxyPacket.java index a732518ff0..82d42d9ee2 100644 --- a/fml/src/main/java/cpw/mods/fml/common/network/internal/FMLProxyPacket.java +++ b/fml/src/main/java/cpw/mods/fml/common/network/internal/FMLProxyPacket.java @@ -12,6 +12,7 @@ import net.minecraft.network.play.client.C17PacketCustomPayload; import net.minecraft.network.play.server.S3FPacketCustomPayload; import org.apache.logging.log4j.Level; import cpw.mods.fml.common.FMLLog; +import cpw.mods.fml.common.network.ByteBufUtils; import cpw.mods.fml.common.network.FMLNetworkException; import cpw.mods.fml.common.network.NetworkRegistry; import cpw.mods.fml.common.network.handshake.NetworkDispatcher; @@ -68,7 +69,11 @@ public class FMLProxyPacket extends Packet { internalChannel.attr(NetworkRegistry.NET_HANDLER).set(this.netHandler); try { - internalChannel.writeInbound(this); + if (internalChannel.writeInbound(this)) + { + FMLLog.severe("A packet for channel %s for side %s was not processed by the embedded channel - it has been dropped. Payload\n%s", this.channel, this.target, ByteBufUtils.getContentDump(this.payload)); + } + internalChannel.inboundMessages().clear(); } catch (FMLNetworkException ne) { From 014acf6f1dd5aea31bf5ac59d51088e149de86de Mon Sep 17 00:00:00 2001 From: Christian Date: Tue, 27 May 2014 18:56:02 -0400 Subject: [PATCH 10/11] Log the queue as well --- .../cpw/mods/fml/common/network/internal/FMLProxyPacket.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/src/main/java/cpw/mods/fml/common/network/internal/FMLProxyPacket.java b/fml/src/main/java/cpw/mods/fml/common/network/internal/FMLProxyPacket.java index 82d42d9ee2..7726a615db 100644 --- a/fml/src/main/java/cpw/mods/fml/common/network/internal/FMLProxyPacket.java +++ b/fml/src/main/java/cpw/mods/fml/common/network/internal/FMLProxyPacket.java @@ -71,7 +71,7 @@ public class FMLProxyPacket extends Packet { { if (internalChannel.writeInbound(this)) { - FMLLog.severe("A packet for channel %s for side %s was not processed by the embedded channel - it has been dropped. Payload\n%s", this.channel, this.target, ByteBufUtils.getContentDump(this.payload)); + FMLLog.severe("Messages for channel %s for side %s were not processed by the embedded channel %s.\n They have been dropped.\n%s", this.channel, this.target, internalChannel.inboundMessages(), ByteBufUtils.getContentDump(this.payload)); } internalChannel.inboundMessages().clear(); } From 8883803260e6cf5cf6922925c2c71e07bb79aba5 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 2 Jun 2014 22:12:13 -0400 Subject: [PATCH 11/11] Nullcheck the dispatchers on players. Should stop some crashes when spamming connectivity. --- .../cpw/mods/fml/common/network/FMLOutboundHandler.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fml/src/main/java/cpw/mods/fml/common/network/FMLOutboundHandler.java b/fml/src/main/java/cpw/mods/fml/common/network/FMLOutboundHandler.java index 21f01ed89f..9e97ede6be 100644 --- a/fml/src/main/java/cpw/mods/fml/common/network/FMLOutboundHandler.java +++ b/fml/src/main/java/cpw/mods/fml/common/network/FMLOutboundHandler.java @@ -106,8 +106,8 @@ public class FMLOutboundHandler extends ChannelOutboundHandlerAdapter { public List selectNetworks(Object args, ChannelHandlerContext context, FMLProxyPacket packet) { EntityPlayerMP player = (EntityPlayerMP) args; - NetworkDispatcher dispatcher = player.playerNetServerHandler.netManager.channel().attr(NetworkDispatcher.FML_DISPATCHER).get(); - return ImmutableList.of(dispatcher); + NetworkDispatcher dispatcher = player == null ? null : player.playerNetServerHandler.netManager.channel().attr(NetworkDispatcher.FML_DISPATCHER).get(); + return dispatcher == null ? ImmutableList.of() : ImmutableList.of(dispatcher); } }, /** @@ -223,7 +223,7 @@ public class FMLOutboundHandler extends ChannelOutboundHandlerAdapter { public List selectNetworks(Object args, ChannelHandlerContext context, FMLProxyPacket packet) { NetworkManager clientConnection = FMLCommonHandler.instance().getClientToServerNetworkManager(); - return clientConnection == null ? ImmutableList.of() : ImmutableList.of(clientConnection.channel().attr(NetworkDispatcher.FML_DISPATCHER).get()); + return clientConnection == null || clientConnection.channel().attr(NetworkDispatcher.FML_DISPATCHER).get() == null ? ImmutableList.of() : ImmutableList.of(clientConnection.channel().attr(NetworkDispatcher.FML_DISPATCHER).get()); } };