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

INVOKE_ASSIGN gets stopped by CHECKCASTs #653

Open
quat1024 opened this issue Dec 20, 2023 · 2 comments
Open

INVOKE_ASSIGN gets stopped by CHECKCASTs #653

quat1024 opened this issue Dec 20, 2023 · 2 comments

Comments

@quat1024
Copy link

quat1024 commented Dec 20, 2023

Here's an assignment from net.minecraft.world.entity.ai.sensing.TemptingSensor#doTick (1.20.1 forge 47.1.3):

List<Player> list = p_148331_.players().stream().filter(EntitySelector.NO_SPECTATORS).filter((p_148342_) -> {
   return TEMPT_TARGETING.test(p_148332_, p_148342_);
}).filter((p_148335_) -> {
   return p_148332_.closerThan(p_148335_, 10.0D);
}).filter(this::playerHoldingTemptation).filter((p_264964_) -> {
   return !p_148332_.hasPassenger(p_264964_);
}).sorted(Comparator.comparingDouble(p_148332_::distanceToSqr)).collect(Collectors.toList());

The instruction stream for this assignment ends like this:

INVOKESTATIC java/util/Comparator.comparingDouble (Ljava/util/function/ToDoubleFunction;)Ljava/util/Comparator; (itf)
INVOKEINTERFACE java/util/stream/Stream.sorted (Ljava/util/Comparator;)Ljava/util/stream/Stream; (itf)
INVOKESTATIC java/util/stream/Collectors.toList ()Ljava/util/stream/Collector;
INVOKEINTERFACE java/util/stream/Stream.collect (Ljava/util/stream/Collector;)Ljava/lang/Object; (itf)
CHECKCAST java/util/List
ASTORE 4

I wanted to inject into doTick after the .collect call and pick up the List<Player>, so I used an INVOKE_ASSIGN injection with local capture. (This felt more appropriate than a modifyvariable, since I also needed to pick up a few method params.)

@Inject(
  method = "doTick(Lnet/minecraft/server/level/ServerLevel;Lnet/minecraft/world/entity/PathfinderMob;)V",
  at = @At(value = "INVOKE_ASSIGN", target = "Ljava/util/stream/Stream;collect(Ljava/util/stream/Collector;)Ljava/lang/Object;", shift = At.Shift.BY, by = 2),
  locals = LocalCapture.PRINT
)

However, the local variable table didn't include list. The injector actually ended up targeting the CHECKCAST instruction before the local was assigned.

/*       Target Class : net.minecraft.world.entity.ai.sensing.TemptingSensor                                     */
/*      Target Method : protected void doTick(ServerLevel p_148331_, PathfinderMob p_148332_)                    */
/*  Target Max LOCALS : 6                                                                                        */
/* Initial Frame Size : 3                                                                                        */
/*      Callback Name : quark$findTroughs                                                                        */
/*        Instruction : InjectionNode CHECKCAST                                                                  */

It looks like the AfterInvoke injector wants to advance over the INVOKE & over any _STORE instructions immediately following it, but the CHECKCAST got in the way.

insn = InjectionPoint.nextNode(insns, insn);
if (insn instanceof VarInsnNode && insn.getOpcode() >= Opcodes.ISTORE) {
insn = InjectionPoint.nextNode(insns, insn);
}

In this case it's workaroundable with shift = At.Shift.BY, by = 2 or targeting a later part of the method.

@LlamaLad7
Copy link
Contributor

This is already tracked I believe, but regardless LocalCapture + INVOKE_ASSIGN would be a very brittle choice here. Consider instead @ModifyVariable and @At("STORE"), or even better https://github.com/LlamaLad7/MixinExtras/wiki/ModifyExpressionValue if you have MixinExtras available.

@quat1024
Copy link
Author

Ahhh i forgot modifyvariable can also include the method parameters x) was confusing it with ModifyArg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants