Mixin icon indicating copy to clipboard operation
Mixin copied to clipboard

Issues with constructor accessors with one or more inaccessible classes

Open Sollace opened this issue 4 years ago • 4 comments

Perhaps someone can help, as I'm having trouble getting the constructor invokers to work correctly. I have a setup where I need to work with two inaccessible type (ModelPart.Quad) and (ModelPart.Vertex) in the 1.15 snapshots.

Quad requires an array of Vertex to be constructed, so I have a case where inaccessible types are present in the parameter descriptors for the constructors I'm trying to invoke.

Previously I've used interfaces to reference the types in my code (added via mixins) and method handles to get a handle on the constructors.

When switching to use Mixin0.8 I thought I could achieve the same effect using these mixin classes:

@Mixin(targets = {"net.minecraft.client.model.ModelPart$Vertex"})
interface MixinVertex extends Vert {
    @Coerce
    @Invoker("<init>(FFFFF)V")
    static Vert create(float x, float y, float z, float u, float v) {
        return null;
    }
}
@Mixin(targets = {"net.minecraft.client.model.ModelPart$Quad"})
interface MixinQuad extends Rect {
    @Coerce
    @Invoker("<init>([Lnet/minecraft/client/model/ModelPart$Vertex;FFFFFFKLnet/minecraft/util/math/Direction;)V")
    static Rect create(
            @Coerce
            Vert[] vertices,
            float u1, float v1,
            float u2, float v2,
            float squishU, float squishV,
            boolean mirror,
            Direction direction) {
        return null;
    }
}

However the game crashes with an exception saying "No candidates were found matching..."

I tried replacing the Vert and Rect references in my methods above with the respective MixinVertex and MixinQuad but the same error remains with the new descriptor.

[14:32:07] [main/FATAL]: Mixin apply failed mson.mixin.json:MixinQuad -> net.minecraft.client.model.ModelPart$Quad: org.spongepowered.asm.mixin.gen.throwables.InvalidAccessorException No candidates were found matching <init>([Lcom/minelittlepony/mson/impl/mixin/MixinVertex;FFFFFFZLnet/minecraft/util/math/Direction;)Lcom/minelittlepony/mson/impl/mixin/MixinQuad; in net/minecraft/client/model/ModelPart$Quad for mson.mixin.json:MixinQuad->@Invoker[METHOD_PROXY]::create([Lcom/minelittlepony/mson/impl/mixin/MixinVertex;FFFFFFZLnet/minecraft/util/math/Direction;)Lcom/minelittlepony/mson/impl/mixin/MixinQuad; [INJECT Applicator Phase -> mson.mixin.json:MixinQuad -> Apply Accessors ->  -> Locate -> mson.mixin.json:MixinQuad->@Invoker[METHOD_PROXY]::create([Lcom/minelittlepony/mson/impl/mixin/MixinVertex;FFFFFFZLnet/minecraft/util/math/Direction;)Lcom/minelittlepony/mson/impl/mixin/MixinQuad;]
org.spongepowered.asm.mixin.gen.throwables.InvalidAccessorException: No candidates were found matching <init>([Lcom/minelittlepony/mson/impl/mixin/MixinVertex;FFFFFFZLnet/minecraft/util/math/Direction;)Lcom/minelittlepony/mson/impl/mixin/MixinQuad; in net/minecraft/client/model/ModelPart$Quad for mson.mixin.json:MixinQuad->@Invoker[METHOD_PROXY]::create([Lcom/minelittlepony/mson/impl/mixin/MixinVertex;FFFFFFZLnet/minecraft/util/math/Direction;)Lcom/minelittlepony/mson/impl/mixin/MixinQuad; [INJECT Applicator Phase -> mson.mixin.json:MixinQuad -> Apply Accessors ->  -> Locate -> mson.mixin.json:MixinQuad->@Invoker[METHOD_PROXY]::create([Lcom/minelittlepony/mson/impl/mixin/MixinVertex;FFFFFFZLnet/minecraft/util/math/Direction;)Lcom/minelittlepony/mson/impl/mixin/MixinQuad;]

This can be written off as either me being stupid, or there being a bug, or something that could possibly be added to Mixins.

Sollace avatar Nov 20 '19 12:11 Sollace

At the moment that's not possible. Ctor invokers are a step towards removing access transformers but still aren't all the way there, Shadow Classes will address that and are roadmapped for 3 releases ahead (there are two features ahead of them in the queue of pending features).

Also note:

  • Per the javadoc, @Invoke does not take a signature as its argument, it takes either the name of the method or fq class name
  • Coerce is used for injector arguments and while its use has been expanded in 0.8, it is not supported on accessors or invokers

I'll leave this open since it will be closed by shadow classes, but bear in mind that's likely to be close to dec/jan before they are released and there's currently no way to do this without using access transformers.

Mumfrey avatar Nov 20 '19 13:11 Mumfrey

@Mumfrey Ah, okay. Looks like I'll be sticking to method handles a little longer, then.

Thanks for the speedy responds, Mumfry!

Coerce is used for injector arguments and while its use has been expanded in 0.8, it is not supported on accessors or invokers

Could you perhaps put that down as a feature request, please? I used coerce there because I'd thought it was failing because the of the mixin interfaces in the method signatures, and that coerce would allow it to figure out the correct match.

Sollace avatar Nov 20 '19 16:11 Sollace

The main issue is that accessors and invokers use the method signature as part of the query, they don't support it in the annotation. Where an injector has its targets selected by the selector and then the arguments just need to "fit" to the target, doing that with an invoker would be quite complicated without changing invoker logic to require the signature in the annotation, meaning the signature could then be different and could therefore be coerced.

Basically it's a significant change which would require some careful planning to make sure it ended up backward compatible. And shadow classes fix the issue anyway and are better than coerce, so I'm definitely not going to make it a high priority. There are other things which are more useful to be working on in the short term, and it ceases to be a problem in the long term. So no, I'm not going to accept it as a feature request because it doesn't actually fix the problem, it just creates more.

Mumfrey avatar Nov 20 '19 17:11 Mumfrey

Alrighty. Thanks Mumfry.

Sollace avatar Nov 20 '19 18:11 Sollace