Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hierarchy validation/transformation does not consider superinterfaces #541

Open
zml2008 opened this issue Dec 5, 2021 · 1 comment
Open

Comments

@zml2008
Copy link
Member

zml2008 commented Dec 5, 2021

Given a Mixin class with the signature:

@Mixin(Level.class)
public abstract class LevelMixin_Tracker implements LevelBridge, LevelAccessorMixin_Tracker {}

where LevelBridge is an ordinary interface, and LevelAccessorMixin_Tracker is an interface mixin targeting LevelAccessor (an interface which Level implements), I would expect Mixin to handle the implementation of LevelAccessorMixin_Tracker in the same way that it would handle a superclass that is a Mixin claass.

However, currently Mixin directly applies both interfaces to the target class, resulting in a raw reference to a Mixin class appearing in a real class. This causes classloading to fail with a cryptic message:

[23:24:10] [main/INFO] [mixin]: Mixing minecraft.world.level.LevelMixin_API from mixins.sponge.api.json into net.minecraft.world.level.Level
[23:24:10] [main/INFO] [mixin]: Mixing world.level.LevelMixin from mixins.sponge.core.json into net.minecraft.world.level.Level
[23:24:10] [main/INFO] [mixin]: Mixing world.level.LevelMixin_Timings from mixins.sponge.core.json into net.minecraft.world.level.Level
[23:24:10] [main/INFO] [mixin]: Mixing world.level.LevelMixin_Tracker from mixins.sponge.tracker.json into net.minecraft.world.level.Level
[23:24:10] [main/WARN] [mixin]: Write access to @Mutable @Final field dimensionType:Lnet/minecraft/world/level/dimension/DimensionType; in mixins.sponge.core.json:world.level.LevelMixin::bridge$adjustDimensionLogic
Exception in thread "main" java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
	at cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:39)
	at cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:54)
	at cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:72)
	at cpw.mods.modlauncher.Launcher.run(Launcher.java:82)
	at cpw.mods.modlauncher.Launcher.main(Launcher.java:66)
	at org.spongepowered.vanilla.applaunch.Main.run(Main.java:95)
	at org.spongepowered.vanilla.applaunch.Main.main(Main.java:65)
	at org.spongepowered.vanilla.applaunch.Main.main(Main.java:59)
Caused by: java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.spongepowered.vanilla.applaunch.handler.dev.ClientDevLaunchHandler.launchService0(ClientDevLaunchHandler.java:45)
	at org.spongepowered.vanilla.applaunch.handler.AbstractVanillaLaunchHandler.lambda$2(AbstractVanillaLaunchHandler.java:197)
	at cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:37)
	... 7 more
Caused by: java.lang.NoClassDefFoundError: org.spongepowered.common.mixin.tracker.world.level.LevelAccessorMixin_Tracker is invalid
	at org.spongepowered.asm.service.modlauncher.ModLauncherClassTracker.handlesClass(ModLauncherClassTracker.java:91)
	at org.spongepowered.asm.launch.MixinLaunchPluginLegacy.handlesClass(MixinLaunchPluginLegacy.java:109)
	at cpw.mods.modlauncher.LaunchPluginHandler.computeLaunchPluginTransformerSet(LaunchPluginHandler.java:66)
	at cpw.mods.modlauncher.ClassTransformer.transform(ClassTransformer.java:64)
	at cpw.mods.modlauncher.TransformingClassLoader$DelegatedClassLoader.findClass(TransformingClassLoader.java:265)
	at cpw.mods.modlauncher.TransformingClassLoader.loadClass(TransformingClassLoader.java:136)
	at cpw.mods.modlauncher.TransformingClassLoader.loadClass(TransformingClassLoader.java:98)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1012)
	at cpw.mods.modlauncher.TransformingClassLoader.loadClass(TransformingClassLoader.java:138)
	at cpw.mods.modlauncher.TransformingClassLoader.loadClass(TransformingClassLoader.java:98)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
	at net.minecraft.client.main.Main.main(Main.java:136)
	at org.spongepowered.vanilla.launch.ClientLaunch.lambda$0(ClientLaunch.java:51)
	at org.spongepowered.vanilla.launch.VanillaBootstrap.perform(VanillaBootstrap.java:65)
	at org.spongepowered.vanilla.launch.ClientLaunch.performBootstrap(ClientLaunch.java:51)
	at org.spongepowered.vanilla.launch.VanillaLaunch.launchPlatform(VanillaLaunch.java:119)
	at org.spongepowered.vanilla.launch.ClientLaunch.launch(ClientLaunch.java:41)
	... 14 more

Ideally superinterfaces that are Mixins would have the same handling as superclasses that are Mixins, but an acceptable alternative would be providing a more clear & specific error message indicating that this inheritance layout is not supported.

zml2008 added a commit to SpongePowered/Sponge that referenced this issue Dec 5, 2021
@Mumfrey
Copy link
Member

Mumfrey commented Dec 6, 2021

Ideally superinterfaces that are Mixins would have the same handling as superclasses that are Mixins, but an acceptable alternative would be providing a more clear & specific error message indicating that this inheritance layout is not supported.

There are so many cases like this because of the way interfaces were originally designed to be handled not taking into account the Java 8 changes, it's why I'm looking at reimplementing the whole thing before allowing stuff like injectors in interfaces, the way mixin handles interfaces is just too outdated compared with how the use-cases available want to employ them.

This isn't an isolated issue and this is basically one small issue which goes away after tackling the larger problem which is "interfaces are second-class citizens in mixin". It's not even its own class of problem, it's related to several others.

Hence I'm not even really classing this as a bug because this is just a case that mixin was never designed to handle in the first place, and while it could admittedly handle this illegal state more gracefully, the real solution is all of the interface handling needs to be improved. Hence I'm labelling this as enhancement because it's really part of a feature which is "support interface mixins properly".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants