New Mixin Fixer system

src/main/java/me/modmuss50/optifabric/compat/IMixinFixer.java

@@ -0,0 +1,8 @@
+package me.modmuss50.optifabric.compat;
+
+import org.objectweb.asm.tree.ClassNode;
+import org.spongepowered.asm.mixin.extensibility.IMixinInfo;
+
+public interface IMixinFixer {
+	void fix(IMixinInfo mixinInfo, ClassNode mixinNode);
+}

src/main/java/me/modmuss50/optifabric/compat/MixinFixerExtension.java

@@ -0,0 +1,92 @@
+package me.modmuss50.optifabric.compat;
+
+import me.modmuss50.optifabric.mod.OptifabricError;
+import me.modmuss50.optifabric.mod.OptifabricSetup;
+import me.modmuss50.optifabric.util.MixinInternals;
+import org.apache.commons.lang3.tuple.Pair;
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.tree.*;
+import org.spongepowered.asm.mixin.MixinEnvironment;
+import org.spongepowered.asm.mixin.extensibility.IMixinInfo;
+import org.spongepowered.asm.mixin.injection.throwables.InjectionError;
+import org.spongepowered.asm.mixin.transformer.ext.IExtension;
+import org.spongepowered.asm.mixin.transformer.ext.ITargetClassContext;
+
+import java.util.*;
+import java.util.stream.Collectors;
+
+public class MixinFixerExtension implements IExtension {
+	private static final Set<ClassNode> PRE_MIXINS = Collections.newSetFromMap(new WeakHashMap<>());
+	private static final Set<ClassNode> POST_MIXINS = Collections.newSetFromMap(new WeakHashMap<>());
+
+	@Override
+	public boolean checkActive(MixinEnvironment environment) {
+		return true;
+	}
+
+	@Override
+	public void preApply(ITargetClassContext context) {
+		if (OptifabricError.hasError()) return;
+		for (Pair<IMixinInfo, ClassNode> pair : MixinInternals.getMixinsFor(context)) {
+			prepareMixin(pair.getLeft(), pair.getRight());
+		}
+	}
+
+	@Override
+	public void postApply(ITargetClassContext context) {
+		for (Pair<IMixinInfo, ClassNode> pair : MixinInternals.getMixinsFor(context)) {
+			handleErrorInjectors(pair.getLeft(), pair.getRight(), context);
+		}
+	}
+
+	@Override
+	public void export(MixinEnvironment env, String name, boolean force, ClassNode classNode) {
+
+	}
+
+	private static void prepareMixin(IMixinInfo mixinInfo, ClassNode mixinNode) {
+		if (PRE_MIXINS.contains(mixinNode)) {
+			// Don't scan the whole class again.
+			return;
+		}
+		ModMixinFixer.INSTANCE.getFixers(mixinInfo.getClassName()).forEach(transformer -> transformer.fix(mixinInfo, mixinNode));
+		PRE_MIXINS.add(mixinNode);
+	}
+
+	//this could use some refactoring
+	private static void handleErrorInjectors(IMixinInfo mixinInfo, ClassNode mixinNode, ITargetClassContext context) {
+		if (POST_MIXINS.contains(mixinNode)) {
+			return;
+		}
+		ClassNode classNode = context.getClassNode();
+
+		List<String> methods = classNode.methods.stream().map(method -> method.name).collect(Collectors.toList());
+		//check for error methods
+		for (MethodNode method : classNode.methods) {
+			if (method.name.endsWith("$missing") && methods.stream().anyMatch(name -> (name + "$missing").equals(method.name))) {
+				for (AbstractInsnNode insn : method.instructions) {
+					if (insn instanceof LdcInsnNode) {
+						String error = (String) ((LdcInsnNode) insn).cst;
+						if (!OptifabricError.hasError()) {
+							OptifabricError.setError(new InjectionError(error), getError(mixinInfo, method));
+							OptifabricError.modError = true;
+						}
+						break;
+					}
+				}
+				OptifabricSetup.LOGGER.warn("Removed InjectionException from Error Injector method " + method.name);
+				method.instructions.clear();
+				method.instructions.add(new InsnNode(Opcodes.RETURN));
+			}
+		}
+		POST_MIXINS.add(mixinNode);
+	}
+
+	private static String getError(IMixinInfo mixinInfo, MethodNode method) {
+		boolean compat = !ModMixinFixer.INSTANCE.getFixers(mixinInfo.getClassName()).isEmpty();
+		return String.format("Injector method %s in %s couldn't apply due to " +
+				(compat ? "outdated compatibility patch" : "missing compatibility patch!") +
+				" Please report this issue.",
+				method.name, mixinInfo, mixinInfo.getConfig());
+	}
+}

src/main/java/me/modmuss50/optifabric/compat/ModMixinFixer.java

@@ -0,0 +1,109 @@
+package me.modmuss50.optifabric.compat;
+
+import com.google.common.collect.Lists;
+import me.modmuss50.optifabric.util.ASMUtils;
+import me.modmuss50.optifabric.util.MixinInternals;
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.Type;
+import org.objectweb.asm.tree.*;
+import org.spongepowered.asm.mixin.extensibility.IMixinInfo;
+import org.spongepowered.asm.mixin.transformer.ClassInfo;
+
+import java.lang.reflect.Modifier;
+import java.util.*;
+import java.util.function.IntSupplier;
+import java.util.stream.Collectors;
+
+public class ModMixinFixer {
+	public static final ModMixinFixer INSTANCE = new ModMixinFixer();
+
+	private final Map<String, List<IMixinFixer>> classFixes = new HashMap<>();
+
+	private ModMixinFixer() {
+	}
+
+	public void addFixer(String mixinClass, IMixinFixer fixer) {
+		classFixes.computeIfAbsent(mixinClass, s -> new ArrayList<>()).add(fixer);
+	}
+
+	public List<IMixinFixer> getFixers(String className) {
+		return classFixes.getOrDefault(className.replace('.', '/'), Collections.emptyList());
+	}
+
+	static IntSupplier getIndexCI(MethodNode method, boolean afterSequence, String... sequence) {
+		return getIndex(method.desc, true, afterSequence, String.join("", sequence));
+	}
+
+	static IntSupplier getIndex(MethodNode method, boolean afterSequence, String... sequence) {
+		return getIndex(method.desc, false, afterSequence, String.join("", sequence));
+	}
+
+	private static IntSupplier getIndex(String methodDesc, boolean afterCallback, boolean afterSequence, String sequence) {
+		return () -> {
+			String desc = methodDesc;
+			int offset = 0;
+			if (afterCallback) {
+				List<Type> params = Lists.newArrayList(Type.getArgumentTypes(desc));
+				for (Type type : params) {
+					offset++;
+					if (type.toString().startsWith("Lorg/spongepowered/asm/mixin/injection/callback/CallbackInfo")) {
+						break;
+					}
+				}
+				desc = params.subList(offset, params.size()).stream().map(Type::toString).collect(Collectors.joining(""));
+			}
+			if (afterSequence) offset++;
+			desc = desc.split(sequence)[afterSequence ? 1 : 0];
+			if (!desc.contains("(")) desc = "(" + desc;
+			if (!desc.contains(")")) desc = desc + ")V";
+			return Type.getArgumentTypes(desc).length + offset;
+		};
+	}
+
+	static void insertParams(MethodNode method, IMixinInfo mixinInfo, IntSupplier index, String... params) {
+		insertParams(method, mixinInfo, index.getAsInt(), params);
+	}
+
+	static void insertParams(MethodNode method, IMixinInfo mixinInfo, int index, String... params) {
+		List<Type> newDesc = Arrays.stream(Type.getArgumentTypes(method.desc)).collect(Collectors.toList());
+		newDesc.addAll(index, Arrays.stream(params).map(Type::getType).collect(Collectors.toList()));
+		int shiftBy = 0;
+		for (String param : params) {
+			shiftBy++;
+			if (ASMUtils.isWideType(param)) shiftBy++;
+		}
+		method.maxLocals += shiftBy;
+
+		for (int i = 0; i < params.length; i++) {
+			method.parameters.add(index + i, new ParameterNode("syn_" + i, Opcodes.ACC_SYNTHETIC));
+		}
+
+		for (int i = index; i > 0; i--) {
+			if (ASMUtils.isWideType(newDesc.get(i))) {
+				index++;
+			}
+		}
+		if (!Modifier.isStatic(method.access)) index++;
+
+		//shift locals (not mandatory)
+		for (LocalVariableNode local : method.localVariables) {
+			if (local.index >= index) {
+				local.index += shiftBy;
+			}
+		}
+		//shift instructions
+		for (AbstractInsnNode insn : method.instructions) {
+			if (insn instanceof VarInsnNode && ((VarInsnNode) insn).var >= index) {
+				((VarInsnNode) insn).var += shiftBy;
+			} else if (insn instanceof IincInsnNode && ((IincInsnNode) insn).var >= index) {
+				((IincInsnNode) insn).var += shiftBy;
+			}
+		}
+
+		ClassInfo info = MixinInternals.getClassInfoFor(mixinInfo);
+		Set<ClassInfo.Method> methods = MixinInternals.getClassInfoMethods(info);
+		methods.removeIf(meth -> method.name.equals(meth.getOriginalName()) && method.desc.equals(meth.getOriginalDesc()));
+		method.desc = Type.getMethodDescriptor(Type.getReturnType(method.desc), newDesc.toArray(new Type[0]));
+		methods.add(info.new Method(method, true));
+	}
+}

src/main/java/me/modmuss50/optifabric/compat/OptifabricMixinErrorHandler.java

@@ -0,0 +1,60 @@
+package me.modmuss50.optifabric.compat;
+
+import me.modmuss50.optifabric.mod.OptifabricError;
+import me.modmuss50.optifabric.mod.OptifabricSetup;
+import org.apache.logging.log4j.Level;
+import org.spongepowered.asm.mixin.FabricUtil;
+import org.spongepowered.asm.mixin.extensibility.IMixinConfig;
+import org.spongepowered.asm.mixin.extensibility.IMixinErrorHandler;
+import org.spongepowered.asm.mixin.extensibility.IMixinInfo;
+
+public class OptifabricMixinErrorHandler implements IMixinErrorHandler {
+	@Override
+	public ErrorAction onPrepareError(IMixinConfig config, Throwable th, IMixinInfo mixin, ErrorAction action) {
+		return handleError(mixin, action, th, false);
+	}
+
+	@Override
+	public ErrorAction onApplyError(String targetClassName, Throwable th, IMixinInfo mixin, ErrorAction action) {
+		return handleError(mixin, action, th, true);
+	}
+
+	private static ErrorAction handleError(IMixinInfo mixin, ErrorAction action, Throwable th, boolean apply) {
+		boolean compat = !ModMixinFixer.INSTANCE.getFixers(mixin.getClassName()).isEmpty();
+		Level level = action == ErrorAction.ERROR ? Level.ERROR : Level.WARN;
+		IMixinConfig config = mixin.getConfig();
+		String msg = String.format(getMessage(apply, compat), mixin, config.getName(), FabricUtil.getModId(config));
+		OptifabricSetup.LOGGER.log(level, msg);
+		//TODO: make this support more than one error and separate OptiFine errors from mod errors
+		if (!OptifabricError.hasError()) {
+			OptifabricError.setError(th, msg);
+			OptifabricError.modError = true;
+		}
+		if (level == Level.ERROR) {
+			OptifabricSetup.LOGGER.info("The following message should have been an error, but will be logged as a " +
+					"warn instead in order to allow the game to show the crash screen.");
+		}
+		//let the game show the crash screen instead of outright crashing
+		//TODO: there are some cases where doing this will do more bad than good
+		return ErrorAction.WARN;
+	}
+
+	private static String getMessage(boolean apply, boolean compat) {
+		String msg;
+		if (compat) {
+			if (apply) {
+				msg = "Failed to apply compatibility patch for %s! Try downgrading the affected mod.";
+			} else {
+				msg = "Prepare error in patched %s! At least one of the patches has a serious flaw!";
+			}
+		} else {
+			if (apply) {
+				msg = "Mixin %s could not be applied and no compatibility patch was found!";
+			} else {
+				msg = "Prepare error in %s! No compatibility patch was found! This might be an issue with the original mod.";
+			}
+		}
+		msg = String.format(msg, "'%s' in '%s' from mod '%s'") + " Please report this issue.";
+		return msg;
+	}
+}

src/main/java/me/modmuss50/optifabric/compat/OptifabricMixinPlugin.java

@@ -0,0 +1,22 @@
+package me.modmuss50.optifabric.compat;
+
+import me.modmuss50.optifabric.util.MixinInternals;
+import org.objectweb.asm.tree.ClassNode;
+import org.spongepowered.asm.mixin.Mixins;
+import org.spongepowered.asm.mixin.extensibility.IMixinInfo;
+
+public class OptifabricMixinPlugin extends EmptyMixinPlugin {
+	@Override
+	public void onLoad(String mixinPackage) {
+		MixinInternals.registerExtension(new MixinFixerExtension());
+		Mixins.registerErrorHandlerClass(OptifabricMixinErrorHandler.class.getName());
+	}
+
+	@Override
+	public void preApply(String targetClassName, ClassNode targetClass, String mixinClassName, IMixinInfo mixinInfo) {
+	}
+
+	@Override
+	public void postApply(String targetClassName, ClassNode targetClass, String mixinClassName, IMixinInfo mixinInfo) {
+	}
+}

src/main/java/me/modmuss50/optifabric/mixin/MixinTitleScreen.java

@@ -47,6 +47,21 @@ private void init(CallbackInfo info) {
 
 			String actionButtonText, helpButtonText;
 			BooleanConsumer action;
+			//TODO: refactor (almost) everything here
+			if (OptifabricError.modError) {
+				String stack = OptifabricError.getErrorLog();
+				actionButtonText = stack != null ? "Copy stack-trace" : "Open logs folder";
+				helpButtonText = "Open issues";
+				action = help -> {
+					if (help) {
+						Util.getOperatingSystem().open("https://github.com/Chocohead/OptiFabric/issues");
+					} else if (stack != null) {
+						client.keyboard.setClipboard(stack);
+					} else {
+						Util.getOperatingSystem().open(new File(FabricLoader.getInstance().getGameDirectory(), "logs"));
+					}
+				};
+			} else {
 			switch (OptifineVersion.jarType) {
 			case SOMETHING_ELSE: //Valid jar states, we shouldn't be here
 			case OPTIFINE_INSTALLER:
@@ -85,6 +100,7 @@ private void init(CallbackInfo info) {
 				break;
 			}
 			}
+			}
 
 			client.openScreen(new ConfirmScreen(action, Text.literal("There was an error loading OptiFabric!", Formatting.RED),
 					Text.literal(OptifabricError.getError()), Text.literal(helpButtonText, Formatting.GREEN), Text.literal(actionButtonText)));

src/main/java/me/modmuss50/optifabric/mod/OptifabricError.java

@@ -6,6 +6,8 @@
 public class OptifabricError {
 	private static String error;
 	private static String stack;
+	//TODO: handle mod errors separately
+	public static boolean modError;
 
 	public static boolean hasError() {
 		return error != null;

src/main/java/me/modmuss50/optifabric/mod/OptifabricSetup.java

@@ -10,6 +10,8 @@
 
 import org.apache.commons.lang3.tuple.Pair;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.objectweb.asm.ClassWriter;
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.tree.AbstractInsnNode;
@@ -37,6 +39,7 @@
 public class OptifabricSetup implements Runnable {
 	public static File optifineRuntimeJar = null;
 	public static boolean usingScreenAPI;
+	public static final Logger LOGGER = LogManager.getLogger("OptiFabric");
 
 	//This is called early on to allow us to get the transformers in beofore minecraft starts
 	@Override
@@ -56,8 +59,7 @@ public void run() {
 				OptifineVersion.jarType = JarType.INTERNAL_ERROR;
 				OptifabricError.setError(e, "Failed to load OptiFine, please report this!\n\n" + e.getMessage());
 			}
-			System.err.println("Failed to setup optifine:");
-			e.printStackTrace();
+			LOGGER.error("Failed to setup optifine: ", e);
 			return; //Avoid crashing out any other Fabric ASM users
 		}
 
@@ -561,8 +563,7 @@ private static boolean compareVersions(String versionRange, ModMetadata mod) {
 			SemanticVersionImpl version = new SemanticVersionImpl(mod.getVersion().getFriendlyString(), false);
 			return predicate.test(version);
 		} catch (@SuppressWarnings("deprecation") net.fabricmc.loader.util.version.VersionParsingException e) {
-			System.err.println("Error comparing the version for ".concat(MoreObjects.firstNonNull(mod.getName(), mod.getId())));
-			e.printStackTrace();
+			LOGGER.error("Error comparing the version for ".concat(MoreObjects.firstNonNull(mod.getName(), mod.getId())), e);
 			return false; //Let's just gamble on the version not being valid also not being a problem
 		}
 	}

src/main/java/me/modmuss50/optifabric/mod/OptifineInjector.java

@@ -40,7 +40,7 @@ public void setup() {
 		Consumer<ClassNode> transformer = target -> {
 			//Avoid double patching things, not that this should happen
 			if (!patched.add(target.name)) {
-				System.err.println("Already patched " + target.name);
+				OptifabricSetup.LOGGER.error("Already patched " + target.name);
 				return;
 			}