SpongePowered / Mixin

Mixin is a trait/mixin and bytecode weaving framework for Java using ASM
MIT License
1.42k stars 193 forks source link

Unable to use Accessor Mixin in Mixin merged method between dependencies #329

Closed gabizou closed 5 years ago

gabizou commented 5 years ago

In doing an overall cleanup of SpongeCommon's mixins, I've started moving away from using AT's for certain fields and using Accessor Mixins. While they work well in consumer code, I've found a corner case where the application of a SpongeForge mixin fails.

The setup: Accessor Mixin that exists in SpongeCommon:

package org.spongepowered.common.mixin.core.entity;
@Mixin(net.minecraft.entity.Entity.class)
public interface AccessorEntity {
  @Accessor("rand") Random accessor$getEntityRandom();
}

The Consumer code in non-Mixin class

    @Override
    protected boolean set(EntityFireworkRocket firework, Integer modifier) {
        ItemStack item = FireworkUtils.getItem(firework);
        NBTTagCompound fireworks = item.getOrCreateSubCompound("Fireworks");
        fireworks.setByte("Flight", modifier.byteValue());
        ((AccessorEntityFireworkRocket) firework).spongeImpl$setLifeTime(10 * modifier.byteValue() + ((AccessorEntity) firework).accessor$getRandom().nextInt(6) + ((AccessorEntity) firework).accessor$getRandom().nextInt(7));
        return true;
    }

And consumer code in a SpongeForge Mixin class:

package org.spongepowered.mod.mixin.core.item;

@Mixin(value = ItemShears.class)
public abstract class MixinItemShears_Forge extends Item {

    /**
     * @author gabizou - June 21st, 2016
     * @reason Rewrites the forge handling of this to properly handle
     * when sheared drops are captured by whatever current phase the
     * {@link PhaseTracker} is in.
     *
     * Returns true if the item can be used on the given entity, e.g. shears on sheep.
     */
    @SuppressWarnings({"unchecked", "rawtypes"})
    @Overwrite(remap = false)
    @Override // Overrides the forge added method from net.minecraft.item.Item
    public boolean itemInteractionForEntity(ItemStack itemstack, EntityPlayer player, EntityLivingBase entity, EnumHand hand) {

        if (entity.world.isRemote) {
            return false;
        }
        if (entity instanceof IShearable) {
            IShearable target = (IShearable) entity;
            BlockPos pos = new BlockPos(entity.posX, entity.posY, entity.posZ);
            if (target.isShearable(itemstack, entity.world, pos)) {
                List<ItemStack> drops = target.onSheared(itemstack, entity.world, pos, EnchantmentHelper.getEnchantmentLevel(Enchantments.FORTUNE, itemstack));
                // Sponge Start - Handle drops according to the current phase

                final Random random = ((AccessorEntity) entity).accessor$getRandom(); // This is where the application fails

            }
            return true;
        }
        return false;
    }

}

The error that comes up isn't about AccessorEntity not being found in the hierarchy of net.minecraft.entity.Entity, but about AccessorEntity not being found in the hierarchy of net.minecraft.item.ItemShears:

/*********************************************************************************************************************************************************************************************************************************************/
/*                                                                                                               Invalid Mixin                                                                                                               */
/*-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------*/
/*     Action : APPLY                                                                                                                                                                                                                        */
/*      Mixin : org.spongepowered.mod.mixin.core.item.MixinItemShears_Forge                                                                                                                                                                  */
/*     Config : mixins.forge.core.json                                                                                                                                                                                                       */
/*      Phase : DEFAULT                                                                                                                                                                                                                      */
/*-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------*/
/*     org.spongepowered.asm.mixin.transformer.throwables.InvalidMixinException                                                                                                                                                              */
/*-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------*/
/*     Resolution error: unable to find corresponding type for                                                                                                                                                                               */
/*     org/spongepowered/common/mixin/core/entity/AccessorEntity in hierarchy of                                                                                                                                                             */
/*     net/minecraft/item/ItemShears                                                                                                                                                                                                         */
/*-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------*/
/* org.spongepowered.asm.mixin.transformer.throwables.InvalidMixinException: Resolution error: unable to find corresponding type for org/spongepowered/common/mixin/core/entity/AccessorEntity in hierarchy of net/minecraft/item/ItemShears */
/*         org.spongepowered.asm.mixin.transformer.MixinTargetContext.findRealType(MixinTargetContext.java:438)                                                                                                                              */
/*         org.spongepowered.asm.mixin.transformer.MixinTargetContext.transformSingleDescriptor(MixinTargetContext.java:913)                                                                                                                 */
/*         org.spongepowered.asm.mixin.transformer.MixinTargetContext.transformDescriptor(MixinTargetContext.java:865)                                                                                                                       */
/*         org.spongepowered.asm.mixin.transformer.MixinTargetContext.transformTypeNode(MixinTargetContext.java:654)                                                                                                                         */
/*         org.spongepowered.asm.mixin.transformer.MixinTargetContext.transformMethod(MixinTargetContext.java:471)                                                                                                                           */
/*         org.spongepowered.asm.mixin.transformer.MixinApplicatorStandard.applyNormalMethod(MixinApplicatorStandard.java:445)                                                                                                               */
/*         org.spongepowered.asm.mixin.transformer.MixinApplicatorStandard.applyMethods(MixinApplicatorStandard.java:432)                                                                                                                    */
/*         org.spongepowered.asm.mixin.transformer.MixinApplicatorStandard.applyMixin(MixinApplicatorStandard.java:312)                                                                                                                      */
/*         org.spongepowered.asm.mixin.transformer.MixinApplicatorStandard.apply(MixinApplicatorStandard.java:280)                                                                                                                           */
/*         org.spongepowered.asm.mixin.transformer.TargetClassContext.applyMixins(TargetClassContext.java:353)                                                                                                                               */
/*         org.spongepowered.asm.mixin.transformer.MixinTransformer.apply(MixinTransformer.java:724)                                                                                                                                         */
/*         org.spongepowered.asm.mixin.transformer.MixinTransformer.applyMixins(MixinTransformer.java:703)                                                                                                                                   */
/*         org.spongepowered.asm.mixin.transformer.MixinTransformer.transformClassBytes(MixinTransformer.java:509)                                                                                                                           */
/*         org.spongepowered.asm.mixin.transformer.Proxy.transform(Proxy.java:72)                                                                                                                                                            */
/*         net.minecraft.launchwrapper.LaunchClassLoader.runTransformers(LaunchClassLoader.java:279)                                                                                                                                         */
/*         net.minecraft.launchwrapper.LaunchClassLoader.findClass(LaunchClassLoader.java:176)                                                                                                                                               */
/*         java.lang.ClassLoader.loadClass(ClassLoader.java:424)                                                                                                                                                                             */
/*         java.lang.ClassLoader.loadClass(ClassLoader.java:357)                                                                                                                                                                             */
/*         net.minecraft.item.Item.registerItems(Item.java:1779)                                                                                                                                                                             */
/*         net.minecraft.init.Bootstrap.register(Bootstrap.java:575)                                                                                                                                                                         */
/*         net.minecraft.server.MinecraftServer.main(MinecraftServer.java:1663)                                                                                                                                                              */
/*         sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)                                                                                                                                                                       */
/*         sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)                                                                                                                                                     */
/*         sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)                                                                                                                                             */
/*         java.lang.reflect.Method.invoke(Method.java:498)                                                                                                                                                                                  */
/*         net.minecraft.launchwrapper.Launch.launch(Launch.java:135)                                                                                                                                                                        */
/*         net.minecraft.launchwrapper.Launch.main(Launch.java:28)                                                                                                                                                                           */
/*         sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)                                                                                                                                                                       */
/*         sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)                                                                                                                                                     */
/*         sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)                                                                                                                                             */
/*         java.lang.reflect.Method.invoke(Method.java:498)                                                                                                                                                                                  */
/*         net.minecraftforge.gradle.GradleStartCommon.launch(GradleStartCommon.java:97)                                                                                                                                                     */
/*         GradleStartServer.main(GradleStartServer.java:10)                                                                                                                                                                                 */
/*********************************************************************************************************************************************************************************************************************************************/

Now, given that I understand accessor mixins are quasi mixins and should be allowed, I feel the bug in this case lies here, in finding the MixinTargetContext#transformSingleDescriptor(). I've not been able to test whether changing that if statement to be as follows:

        if (!typeInfo.isMixin() || typeInfo.isAccessor()) { // Would need to add this method to ClassInfo
            return desc;
        }

I've been able to work around the issue by using the native method EntityLivingBase#getRNG(), but I feel that using accessor mixins in other mixin code should still be a valid practice.

gabizou commented 5 years ago

Duplicate of #305 and #189