Use mixins instead of wrapping objects

[molenzwiebel]

The way Sponge uses mixins allows them to add new interface implementations to existing classes, instead of wrapping around Forge's classes. For example, net.minecraft's World gets patched with sponge's World interface at runtime, allowing Sponge (and other plugins) to cast the net.minecraft.world.World objects to their Sponge API counterparts.

This sounds like something that would be great for doing in Pore as well. Instead of wrapping everything we can just add the interfaces to already existing code, and just implement the functions. Instead of caching PoreWorld -> (Sponge) World we can just cast a sponge world to a Bukkit world, without any problems.

The backfire of this would be that Pore would need to become a Forge coremod to get access to the ASM libraries available in Forge. This could be avoided, but would require severe overhead (manually doing all the asm and injection).

EDIT: Another backfire would be the excessive casting required. For example: (BukkitWorld) (net.minecraft.World) spongeWorldInstance; because the IDE would not know that SpongeWorld can be cast to BukkitWorld at runtime (it is perfectly legal at runtime though). This won't be too much of a problem as Bukkit (or Sponge for that matter) should never want or need to do that.

[maxov]

Mixins currently do not work even in Sponge. See SpongePowered/Sponge#63. For that reason I would avoid them until they work correctly, especially because the pore class hierarchy is more complex and due to conflict.

And that's the biggest problem with mixins -- we all know that the Java compiler is extremely inflexible, and there are so many conflicts and edge cases with mixins that in my eyes using them with the default compiler is mostly impractical.

In fact, that's the solution Mumfrey came up eventually -- an annotation processor. This means that anyone working on Sponge(and Pore if we adopt mixins) will need to get these annotations working in their IDE. If you've ever used lombok, it's the same kind of effect and makes contributing much harder.

EDIT: The Annotation Processor is to cross the obfuscation boundary and not work at compile-time. This means no problems with IDE support.

Thought dump of additional issues:

• Mixins work at runtime to inject methods manually. This is already pretty dangerous with Sponge doing them(above-mentioned conflicts) but for a two-layer system I predict many conflicts between Sponge and Pore names that will mess the whole thing up.
• I'm not very partial to the idea of Pore being a forge coremod. As you said there is a lot of overhead in being a coremod including having to depend on the asm library. Since pore is more of a wrapper, we shouldn't have to be requiring asm.
• A ton of work has already been done on the wrapper system. It is currently around 13,500 lines of code. Switching to mixins would require a big rewrite and slow the project down tremendously.

[stephan-gh]

@gratimax mentioned most of the things already. We want to implement a Bukkit wrapper on the top of the SpongeAPI, so it could technically also work on a Glowstone implementation of the SpongeAPI or any other one. Using mixins would prevent that. Also, because a lot of methods names are the same with a different return type we would have even more methods to fix than we need to fix for Sponge.

Remember, the Bukkit API was never designed to be implemented directly to Minecraft classes, CraftBukkit was only using wrapper classes. If you try to do that you will notice that you will have a lot of smaller problems because it is not as flexible as the SpongeAPI - they have chosen to prevent a lot of interfaces and used enums or classes instead.

I would rather spend some more time on making the caching of the wrapper classes better than trying to implement mixins now that we have already most things working. The Bukkit API is pretty old, and not always easily implementable directly on top of internal Minecraft classes.

So unless @mproncace really wants to change everything for some reason now we're not going to change it. However, thank you for your suggestion, but right now wrapper classes are easier to implement and maintainable for Pore.