Damageable interface clashes with required extend EntityPlayer for Player Mixin

Both Bukkit and Sponge have chosen to implement damage as a double instead of the default implementation of an float. What this means is that when attempting to create a Mixin for the Player or Entity interfaces, which requires both implementing the Sponge interface and extending the Forge/Minecraft class, that it won’t be possible to implement anything using the Damageable interface.

I may be mistaken and it might be a simple workaround that I’m missing, but I can’t seem to figure it out.

Minecraft it self uses double though I thought

Minecraft has

float getHealth()

Sponge has

double getHealth()

sorry. So Minecraft uses a float, but we use a double.

Can’t you just cast it ?

Short answer: No.

Long answer:
If I mixin I have to extend the superclass of the class I’m mixing into. But that superclass has methods that conflict with interfaces I also have to mixin.

Code might help

CraftBukkit also had this issue, it solved it by changing vanilla base classes for getHealth to
_INVALID_getHealth and then running a maven plugin that would map them back after they were compiled but before they went into the jar.

Ok the issue is super simple to solve to hack in event handlers and for a more proper fix I’ve already got that figured out too. Not too complex either of them.

You should look to the prefix option in @shadow. That normally fixes issues with conflicting methods.

This is an issue the @shadow annotation can’t solve, because the parent class I have to extend defines the interfaces. @Zidane and I spoke very briefly about this, I would like to hear what he thinks we should do. I tried to implement a few additional Mixin annotations that would inject interfaces and allow methods to be unprefixed with asm. While they seem to work perfectly, using them to solve this problem makes the server not start.

Hmmm, I am not sure, what class you are using in your gist. Seeing the constructor your trying to implement a base class. And so far I know you don’t need to add constructors.

Could you include the imports as well?

Sure, it’s a compiler issue, hence the comment. We don’t have a special Mixin compiler that only requires what we will mixin, the compiler sees the extension of a class and requires the constructor.

import com.mojang.authlib.GameProfile;
import net.minecraft.entity.player.EntityPlayer;
import net.minecraft.entity.player.EntityPlayerMP;
import net.minecraft.world.World;
import org.spongepowered.api.entity.Player;
import org.spongepowered.mod.mixin.Mixin;
import org.spongepowered.mod.mixin.Shadow;

Here comes your problem. Your mixing class is extending a base class. So far I know that is not how they work. This is how I think the health method should be implemented.

If you look at the implementation code for MixinTransformer you see that you must use the same super class, which means you have to extend it, otherwise it won’t inject your class. If you bypass this, any super calls will be wrong because it will refer to object instead of the correct super class.

Ohhhh I see, the naming of the classes has to be equal. Yep this is an isue. It could be easily fixed by removing the namecheck in verifyclasses method. Oh dang event that doesn’t work. I think I understand the problems:

  • You can’t call methods from EntityPlayer in EntityPlayerMP.
  • Name’s have to match.

Not really, that name check is important, I’ve found a solution, I added additional transformer annotations @Prefixed(prefix = "shaded$") which is simply stripped, does half of what @Shadow does. Then @MixinInterface(Player.class) which injects the interface during transformation. The combination of those two fixes the issue. The only other thing is that for event mixins to work, the MixinTransformer has to happen last, after the EventTransformer has added all the Sponge event stuff.

Maybe we don’t need to mix EntityPlayerMP. If we mix EntityLivingBase and implement damageable than we can access the getHealth method.

The @rename (basicly does the same what @prefix would do) thing I already tried. I send a PR but he wasn’t really happy. So I don’t think that is what he wants.

That doesn’t work either, because you still have to compile the mixin for player to add the player specific stuff. Which means that while you don’t have to implement a getHealth, setHealth or damage method, the compiler has to have two definitions when it compiles the mixin. That’s why you have to rename at runtime.

If @Mumfrey is working on this then they need to explain they are doing to solve the problem. Because it’s crucial to people who want to mess with the implementation. Even if someone is working on it, 17 days is a long time without any updates or discussion. We can’t depend on the promise of code when we want to make ourselves a basic implementation to test things out.

Also your implementation duplicated the @Shadow(prefix = "shadow$") feature, the reason @Shadow doesn’t work form me is because it’s made for renaming classes that exist in your mixin class but conflict with your new definition. The methods I need to rename are ones that need to be renamed but aren’t in the compiler.

I thought the rename at runtime was needed because the java compiler doesn’t understand the same name with parameters but with a different return value. Java understands that but their is no syntax to code in that.

And If the EntityPlayerMP class extends EntityLivingBase, with EntityLivingBase implementing Damageable. Does EntityPlayerMP not get that method?

Code

Not until runtime when ASM transforms the classes, during the compile things are more strict.

That’s what I’m doing in my modified version with the new annotations, but I still have to transform player to add getName().