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

Targeting correct instance method call, but AP warns about method from a default interface #438

Closed
gabizou opened this issue Sep 5, 2020 · 1 comment

Comments

@gabizou
Copy link
Member

gabizou commented Sep 5, 2020

So, for the provided method from AbstractSpawner:

    private boolean isActivated() {
        BlockPos lvt_1_1_ = this.getSpawnerPosition();
        return this.getWorld().isPlayerWithin((double)lvt_1_1_.getX() + 0.5D, (double)lvt_1_1_.getY() + 0.5D, (double)lvt_1_1_.getZ() + 0.5D, (double)this.activatingRangeFromPlayer);
    }

with the provided Mixin:

    @Redirect(method = "isActivated()Z",
        at = @At(value = "INVOKE",
            target = "Lnet/minecraft/world/World;isPlayerWithin(DDDD)Z"))
    public boolean onIsPlayerWithin(final World world, final double x, final double y, final double z, final double distance) {
        // some special handling with sponge
        return false;
    }

We get the following error from the AP:

/Volumes/Dev/Tools/Sponge/1.14/SpongeCommon/src/mixins/java/org/spongepowered/common/mixin/core/world/spawner/AbstractSpawnerMixin.java:95: warning: Cannot find method mapping for @At(INVOKE.<target>) 'Lnet/minecraft/world/World;isPlayerWithin(DDDD)Z'

Which of course provides the following refmap:

    "org/spongepowered/common/mixin/core/world/spawner/AbstractSpawnerMixin": {
      "isActivated()Z": "Lnet/minecraft/world/spawner/AbstractSpawner;func_98279_f()Z"
    },

And this is because, the method is defined in the interface net/minecraft/world/IEntityReader, but on changing this to IEntityReader, as it's suggesting (somewhat), we'll have some nice AP obf mapping:

    "org/spongepowered/common/mixin/core/world/spawner/AbstractSpawnerMixin": {
      "Lnet/minecraft/world/IEntityReader;isPlayerWithin(DDDD)Z": "Lnet/minecraft/world/IEntityReader;func_217358_a(DDDD)Z",
      "isActivated()Z": "Lnet/minecraft/world/spawner/AbstractSpawner;func_98279_f()Z"
    },

HOWEVER, mixin application will fail in production, because the matching bytecode (remapped to srg named, but not decompiled and recompiled) looks like this:

     private func_98279_f() { //()Z
         <localVar:index=0 , name=this , desc=Lnet/minecraft/world/spawner/AbstractSpawner;, sig=null, start=L1, end=L2>
         <localVar:index=1 , name=☃ , desc=Lnet/minecraft/util/math/BlockPos;, sig=null, start=L3, end=L2>

         L1 {
             aload0 // reference to self
             invokevirtual net/minecraft/world/spawner/AbstractSpawner.func_177221_b()Lnet/minecraft/util/math/BlockPos;
             astore1
         }
         L3 {
             aload0 // reference to self
             invokevirtual net/minecraft/world/spawner/AbstractSpawner.func_98271_a()Lnet/minecraft/world/World;
             aload1
             invokevirtual net/minecraft/util/math/BlockPos.func_177958_n()I
             i2d
             ldc 0.5 (java.lang.Double)
             dadd
             aload1
             invokevirtual net/minecraft/util/math/BlockPos.func_177956_o()I
             i2d
             ldc 0.5 (java.lang.Double)
             dadd
             aload1
             invokevirtual net/minecraft/util/math/BlockPos.func_177952_p()I
             i2d
             ldc 0.5 (java.lang.Double)
             dadd
             aload0 // reference to self
             getfield net/minecraft/world/spawner/AbstractSpawner.field_98289_l:int
             i2d
             invokevirtual net/minecraft/world/World.func_217358_a(DDDD)Z
             ireturn
         }
         L2 {
         }
     }

Note the last invokevirtual net/minecraft/world/World.func_217358_a(DDDD)Z and it is not IEntityReader.func_217358_a(DDDD)Z, even though in theory this should be fine (relatively speaking, its an invokeinterface, but there's likely some proguard shenanigans at play here with relation to calling default implemented methods).

gabizou added a commit to SpongePowered/Sponge that referenced this issue Sep 5, 2020
@Mumfrey
Copy link
Member

Mumfrey commented Sep 21, 2020

I couldn't figure out why 9438945 didn't auto-close this and then I realised I put the hash in front of Fixed instead of in front of 438. I'm an idiot. This is fixed.

@Mumfrey Mumfrey closed this as completed Sep 21, 2020
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