SpongePowered / Mixin

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

ModifyVariable targeting two variables despite specifying index/ordinal #482

Closed The-PPWD closed 3 years ago

The-PPWD commented 3 years ago

With the following annotation:

    @ModifyVariable(
            method = "updateCameraPosition",
            at = @At(
                    value = "LOAD"
            ),
            index = 12 // or ordinal = 2
    )

The mixin is being applied to two seperate variables (index 12 or bl2 and index 22 or bl4 within ThreadedAnvilChunkStorage#updateCameraPosition (Yarn mappings). However, if I specify ordinal = 0 within the @At as such:

    @ModifyVariable(
            method = "updateCameraPosition",
            at = @At(
                    value = "LOAD",
                    ordinal = 0
            ),
            index = 12 // or ordinal = 2
    )

It only targets the index 12 variable. This is correct; however, the issue is that the index/ordinal within @ModifyVariable results in it applying to two different variables. I've confirmed that they are indeed two different variables within the sources as well as within the bytecode (unless there are some optimizations like variable reuse that I'm not seeing).

If there's more information that is needed, please let me know.

Mumfrey commented 3 years ago

I can't replicate the described behaviour with the latest snapshot (currently 0.8.3) and MCP, with your first example which supposedly fails transcribed to MCP mappings and with print enabled to emit the matches:

@Mixin(ChunkManager.class)
class MixinChunkManager {
    @ModifyVariable(
        method = "updatePlayerPosition(Lnet/minecraft/entity/player/ServerPlayerEntity;)V",
        at = @At(
            value = "LOAD"
        ),
        index = 12, // or ordinal = 2,
        print = true
    )
    private boolean modifyBoolean(boolean orginalValue) {
        return orginalValue;
    }
}

I see the expected result:

/*****************************************************************************************************************/
/*         Target Class : net.minecraft.world.server.ChunkManager                                                */
/*        Target Method : updatePlayerPosition                                                                   */
/*        Callback Name : modifyBoolean                                                                          */
/*         Capture Type : boolean                                                                                */
/*          Instruction : [110] VarInsnNode ILOAD                                                                */
/*****************************************************************************************************************/
/*           Match mode : EXPLICIT (match by criteria)                                                           */
/*        Match ordinal : any                                                                                    */
/*          Match index : 12                                                                                     */
/*        Match name(s) : any                                                                                    */
/*            Args only : false                                                                                  */
/*****************************************************************************************************************/
/* INDEX  ORDINAL                            TYPE  NAME                                                CANDIDATE */
/* [  1]    [  0]              ServerPlayerEntity  player                                              -         */
/* [  2]    [  0]                             int  l1                                                  -         */
/* [  3]    [  1]                             int  i2                                                  -         */
/* [  4]    [  0]                      SectionPos  sectionpos                                          -         */
/* [  5]    [  1]                      SectionPos  sectionpos1                                         -         */
/* [  6]    [  0]                            long  i                                                   -         */
/* [  7]                                    <top>                                                                */
/* [  8]    [  1]                            long  j                                                   -         */
/* [  9]                                    <top>                                                                */
/* [ 10]    [  0]                         boolean  flag                                                YES       */
/* [ 11]    [  1]                         boolean  flag1                                               YES       */
/* [ 12]    [  2]                         boolean  flag2                                               YES       */
/* [ 13]                                        -                                                                */
/* [ 14]                                        -                                                                */
/* [ 15]                                        -                                                                */
/* [ 16]                                        -                                                                */
/* [ 17]                                        -                                                                */
/* [ 18]                                        -                                                                */
/* [ 19]                                        -                                                                */
/* [ 20]                                        -                                                                */
/* [ 21]                                        -                                                                */
/* [ 22]                                        -                                                                */
/* [ 23]                                        -                                                                */
/*****************************************************************************************************************/

Please post the results of running your injectors with print enabled.

The-PPWD commented 3 years ago

Here are both my prints (as it's modifying two variables). Mixin version is 0.8.2.

/*****************************************************************************************************************/
/*         Target Class : net.minecraft.server.world.ThreadedAnvilChunkStorage                                   */
/*        Target Method : updateCameraPosition                                                                   */
/*        Callback Name : updateCameraPosition_LOAD                                                              */
/*         Capture Type : boolean                                                                                */
/*          Instruction : [118] VarInsnNode ILOAD                                                                */
/*****************************************************************************************************************/
/*           Match mode : EXPLICIT (match by criteria)                                                           */
/*        Match ordinal : 2                                                                                      */
/*          Match index : any                                                                                    */
/*        Match name(s) : any                                                                                    */
/*            Args only : false                                                                                  */
/*****************************************************************************************************************/
/* INDEX  ORDINAL                            TYPE  NAME                                                CANDIDATE */
/* [  1]    [  0]              ServerPlayerEntity  player                                              -         */
/* [  2]    [  0]                             int  i                                                   -         */
/* [  3]    [  1]                             int  j                                                   -         */
/* [  4]    [  0]                 ChunkSectionPos  chunkSectionPos                                     -         */
/* [  5]    [  1]                 ChunkSectionPos  chunkSectionPos2                                    -         */
/* [  6]    [  0]                            long  l                                                   -         */
/* [  7]                                    <top>                                                                */
/* [  8]    [  1]                            long  m                                                   -         */
/* [  9]                                    <top>                                                                */
/* [ 10]    [  0]                         boolean  bl                                                  YES       */
/* [ 11]    [  1]                         boolean  bl2                                                 YES       */
/* [ 12]    [  2]                         boolean  bl3                                                 YES       */
/* [ 13]                                        -                                                                */
/* [ 14]                                        -                                                                */
/* [ 15]                                        -                                                                */
/* [ 16]                                        -                                                                */
/* [ 17]                                        -                                                                */
/* [ 18]                                        -                                                                */
/* [ 19]                                        -                                                                */
/* [ 20]                                        -                                                                */
/* [ 21]                                        -                                                                */
/* [ 22]                                        -                                                                */
/* [ 23]                                        -                                                                */
/* [ 24]                                        -                                                                */
/*****************************************************************************************************************/
/*****************************************************************************************************************/
/*         Target Class : net.minecraft.server.world.ThreadedAnvilChunkStorage                                   */
/*        Target Method : updateCameraPosition                                                                   */
/*        Callback Name : updateCameraPosition_LOAD                                                              */
/*         Capture Type : boolean                                                                                */
/*          Instruction : [332] VarInsnNode ILOAD                                                                */
/*****************************************************************************************************************/
/*           Match mode : EXPLICIT (match by criteria)                                                           */
/*        Match ordinal : 2                                                                                      */
/*          Match index : any                                                                                    */
/*        Match name(s) : any                                                                                    */
/*            Args only : false                                                                                  */
/*****************************************************************************************************************/
/* INDEX  ORDINAL                            TYPE  NAME                                                CANDIDATE */
/* [  1]    [  0]              ServerPlayerEntity  player                                              -         */
/* [  2]    [  0]                             int  i                                                   -         */
/* [  3]    [  1]                             int  j                                                   -         */
/* [  4]    [  0]                 ChunkSectionPos  chunkSectionPos                                     -         */
/* [  5]    [  1]                 ChunkSectionPos  chunkSectionPos2                                    -         */
/* [  6]    [  0]                            long  l                                                   -         */
/* [  7]                                    <top>                                                                */
/* [  8]    [  1]                            long  m                                                   -         */
/* [  9]                                    <top>                                                                */
/* [ 10]    [  0]                         boolean  bl                                                  YES       */
/* [ 11]    [  1]                         boolean  bl2                                                 YES       */
/* [ 12]                                        -                                                                */
/* [ 13]    [  2]                             int  k                                                   -         */
/* [ 14]    [  3]                             int  n                                                   -         */
/* [ 15]    [  4]                             int  o                                                   -         */
/* [ 16]    [  5]                             int  p                                                   -         */
/* [ 17]    [  6]                             int  q                                                   -         */
/* [ 18]    [  7]                             int  r                                                   -         */
/* [ 19]    [  8]                             int  s                                                   -         */
/* [ 20]    [  9]                             int  t                                                   -         */
/* [ 21]    [  0]                        ChunkPos  chunkPos                                            -         */
/* [ 22]    [  2]                         boolean  bl4                                                 YES       */
/* [ 23]    [  3]                         boolean  bl5                                                 YES       */
/* [ 24]                                        -                                                                */
/*****************************************************************************************************************/
Mumfrey commented 3 years ago

Closing as duplicate of #481 as the root cause of these issues is the same.