Add dev-only crash for when screens are opened in Item.use

(Including useOnBlock and useOnEntity.)

The method often causes threading issues and other crashes,
and a better solution is to follow what Minecraft itself does
and use a packet or a screen handler.

4 files changed, 210 insertions, 0 deletions

diff --git a/src/main/java/io/github/cottonmc/cotton/gui/impl/LibGuiMixinPlugin.java b/src/main/java/io/github/cottonmc/cotton/gui/impl/LibGuiMixinPlugin.java
new file mode 100644
index 0000000..b46cd7f
--- /dev/null
+++ b/src/main/java/io/github/cottonmc/cotton/gui/impl/LibGuiMixinPlugin.java
@@ -0,0 +1,53 @@
+package io.github.cottonmc.cotton.gui.impl;
+
+import net.fabricmc.loader.api.FabricLoader;
+
+import com.google.common.base.Suppliers;
+import org.objectweb.asm.tree.ClassNode;
+import org.spongepowered.asm.mixin.extensibility.IMixinConfigPlugin;
+import org.spongepowered.asm.mixin.extensibility.IMixinInfo;
+
+import java.util.List;
+import java.util.Set;
+import java.util.function.Supplier;
+
+public final class LibGuiMixinPlugin implements IMixinConfigPlugin {
+	private static final Supplier<Boolean> IS_DEVELOPMENT =
+			Suppliers.memoize(() -> FabricLoader.getInstance().isDevelopmentEnvironment());
+
+	@Override
+	public boolean shouldApplyMixin(String targetClassName, String mixinClassName) {
+		return switch (mixinClassName) {
+			case "io.github.cottonmc.impl.mixin.client.MinecraftClientMixin" -> IS_DEVELOPMENT.get();
+			default -> true;
+		};
+	}
+
+	// Boilerplate below
+
+	@Override
+	public void onLoad(String mixinPackage) {
+	}
+
+	@Override
+	public String getRefMapperConfig() {
+		return null;
+	}
+
+	@Override
+	public void acceptTargets(Set<String> myTargets, Set<String> otherTargets) {
+	}
+
+	@Override
+	public List<String> getMixins() {
+		return null;
+	}
+
+	@Override
+	public void preApply(String targetClassName, ClassNode targetClass, String mixinClassName, IMixinInfo mixinInfo) {
+	}
+
+	@Override
+	public void postApply(String targetClassName, ClassNode targetClass, String mixinClassName, IMixinInfo mixinInfo) {
+	}
+}
diff --git a/src/main/java/io/github/cottonmc/cotton/gui/impl/client/ItemUseChecker.java b/src/main/java/io/github/cottonmc/cotton/gui/impl/client/ItemUseChecker.java
new file mode 100644
index 0000000..5a0293c
--- /dev/null
+++ b/src/main/java/io/github/cottonmc/cotton/gui/impl/client/ItemUseChecker.java
@@ -0,0 +1,135 @@
+package io.github.cottonmc.cotton.gui.impl.client;
+
+import net.fabricmc.loader.api.FabricLoader;
+import net.fabricmc.loader.api.MappingResolver;
+import net.minecraft.client.gui.screen.Screen;
+import net.minecraft.item.Item;
+import net.minecraft.util.Pair;
+import net.minecraft.util.Util;
+import net.minecraft.util.crash.CrashException;
+import net.minecraft.util.crash.CrashReport;
+
+import org.jetbrains.annotations.Nullable;
+
+import java.lang.invoke.MethodType;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+/**
+ * Crashes the game if a LibGui screen is opened in {@code Item.use/useOnBlock/useOnEntity}.
+ */
+public final class ItemUseChecker {
+	// Setting this property to "true" disables the check.
+	private static final String ALLOW_ITEM_USE_PROPERTY = "libgui.allowItemUse";
+
+	// Stack walker instance used to check the caller.
+	private static final StackWalker STACK_WALKER =
+			StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);
+
+	// List of banned item use methods.
+	private static final List<Pair<String, MethodType>> ITEM_USE_METHODS = Util.make(new ArrayList<>(), result -> {
+		MappingResolver resolver = FabricLoader.getInstance().getMappingResolver();
+
+		String hand = "class_1268";
+		String actionResult = "class_1269";
+		String typedActionResult = "class_1271";
+		String livingEntity = "class_1309";
+		String playerEntity = "class_1657";
+		String itemStack = "class_1799";
+		String itemUsageContext = "class_1838";
+		String world = "class_1937";
+
+		// use
+		result.add(resolveItemMethod(resolver, "method_7836", typedActionResult, world, playerEntity, hand));
+		// useOnBlock
+		result.add(resolveItemMethod(resolver, "method_7884", actionResult, itemUsageContext));
+		// useOnEntity
+		result.add(resolveItemMethod(resolver, "method_7847", actionResult, itemStack, playerEntity, livingEntity, hand));
+	});
+
+	private static Pair<String, MethodType> resolveItemMethod(MappingResolver resolver, String name, String returnType, String... parameterTypes) {
+		// Build intermediary descriptor for resolving the method in the mappings.
+		StringBuilder desc = new StringBuilder("(");
+		for (String type : parameterTypes) {
+			desc.append("Lnet/minecraft/").append(type).append(';');
+		}
+		desc.append(")Lnet/minecraft/").append(returnType).append(';');
+
+		// Remap the method name.
+		String deobfName = resolver.mapMethodName("intermediary", "net.minecraft.class_1792", name, desc.toString());
+
+		// Remap the descriptor types.
+		Function<String, Class<?>> getIntermediaryClass = className -> {
+			className = resolver.mapClassName("intermediary", "net.minecraft." + className);
+
+			try {
+				return Class.forName(className);
+			} catch (ClassNotFoundException e) {
+				throw new RuntimeException("Could not resolve class net.minecraft." + className, e);
+			}
+		};
+		Class<?>[] paramClasses = Arrays.stream(parameterTypes)
+				.map(getIntermediaryClass)
+				.toArray(Class[]::new);
+		Class<?> returnClass = getIntermediaryClass.apply(returnType);
+
+		// Check that the method actually exists.
+		try {
+			Item.class.getMethod(deobfName, paramClasses);
+		} catch (NoSuchMethodException e) {
+			throw new RuntimeException("Could not find Item method " + deobfName, e);
+		}
+
+		return new Pair<>(deobfName, MethodType.methodType(returnClass, paramClasses));
+	}
+
+	/**
+	 * Checks whether the specified screen is a LibGui screen opened
+	 * from an item usage method.
+	 *
+	 * @throws CrashException if opening the screen is not allowed
+	 */
+	public static void checkSetScreen(Screen screen) {
+		if (!(screen instanceof CottonScreenImpl cs) || Boolean.getBoolean(ALLOW_ITEM_USE_PROPERTY)) return;
+
+		// Check if this is called via Item.use. If so, crash the game.
+
+		// The calling variant of Item.use[OnBlock|OnEntity].
+		// If null, nothing bad happened.
+		@Nullable Pair<? extends Class<?>, String> useMethodCaller = STACK_WALKER.walk(s -> s
+						.skip(3) // checkSetScreen, setScreen injection, setScreen
+						.flatMap(frame -> {
+							if (!Item.class.isAssignableFrom(frame.getDeclaringClass())) return Stream.empty();
+
+							return ITEM_USE_METHODS.stream()
+									.filter(method -> method.getLeft().equals(frame.getMethodName()) &&
+											method.getRight().equals(frame.getMethodType()))
+									.map(method -> new Pair<>(frame.getDeclaringClass(), method.getLeft()));
+						})
+						.findFirst())
+				.orElse(null);
+
+		if (useMethodCaller != null) {
+			String message = """
+						[LibGui] Screens cannot be opened in item use methods. Some alternatives include:
+							- Using a packet together with LightweightGuiDescription
+							- Using an ItemSyncedGuiDescription
+						Setting the screen in item use methods leads to threading issues and
+						other potential crashes on both the client and the server.
+						If you want to disable this check, set the system property %s to "true"."""
+					.formatted(ALLOW_ITEM_USE_PROPERTY);
+			var cause = new UnsupportedOperationException(message);
+			cause.fillInStackTrace();
+			CrashReport report = CrashReport.create(cause, "Opening screen");
+			report.addElement("Screen opening details")
+					.add("Screen class", screen.getClass().getName())
+					.add("GUI description", () -> cs.getDescription().getClass().getName())
+					.add("Item class", () -> useMethodCaller.getLeft().getName())
+					.add("Involved method", useMethodCaller.getRight());
+			throw new CrashException(report);
+		}
+	}
+}
diff --git a/src/main/java/io/github/cottonmc/cotton/gui/impl/mixin/client/MinecraftClientMixin.java b/src/main/java/io/github/cottonmc/cotton/gui/impl/mixin/client/MinecraftClientMixin.java
new file mode 100644
index 0000000..f19aba6
--- /dev/null
+++ b/src/main/java/io/github/cottonmc/cotton/gui/impl/mixin/client/MinecraftClientMixin.java
@@ -0,0 +1,20 @@
+package io.github.cottonmc.cotton.gui.impl.mixin.client;
+
+import net.minecraft.client.MinecraftClient;
+import net.minecraft.client.gui.screen.Screen;
+
+import io.github.cottonmc.cotton.gui.impl.client.ItemUseChecker;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+
+// Prevents LibGui screens from being opened in a dev environment
+// using Item.use/useOnBlock/useOnEntity.
+@Mixin(MinecraftClient.class)
+abstract class MinecraftClientMixin {
+	@Inject(method = "setScreen", at = @At("HEAD"))
+	private void onSetScreen(Screen screen, CallbackInfo info) {
+		ItemUseChecker.checkSetScreen(screen);
+	}
+}
diff --git a/src/main/resources/mixins.libgui.json b/src/main/resources/mixins.libgui.json
index 24cd8ec..6698d08 100644
--- a/src/main/resources/mixins.libgui.json
+++ b/src/main/resources/mixins.libgui.json
@@ -3,8 +3,10 @@
 	"minVersion": "0.8",
 	"compatibilityLevel": "JAVA_17",
 	"package": "io.github.cottonmc.cotton.gui.impl.mixin",
+	"plugin": "io.github.cottonmc.cotton.gui.impl.LibGuiMixinPlugin",
 
 	"client": [
+		"client.MinecraftClientMixin",
 		"client.ScreenAccessor"
 	],