Added transformer for onUsingTick in EntityPlayer #414
Conversation
EntityPlayer does not call onUsingTick properly on the server because it uses a == instead of a proper ItemStack.areItemStacksEqual call. Another piece of code constantly copies the itemInUse stack, so this condition is never true. This transformer changes the == to call ItemStack.areItemStacksEqual instead. This isn't a mixin because mixins can't change expressions like this.
|
I've been testing this for a few weeks and it works well. I enabled the transformer on both sides (client & server) even though it's not necessary because the client only works by happenstance. |
what other piece of code ? |
| public byte[] transform(String name, String transformedName, byte[] basicClass) { | ||
| if (basicClass == null) return null; | ||
|
|
||
| if (NameContext.Obf.entityPlayer.equals(name)) { |
There was a problem hiding this comment.
you can check transformedName directly, it won't be obfuscated
There was a problem hiding this comment.
I tested it in the full pack and it looks like this isn't the case. I had to use the obfuscated name.
There was a problem hiding this comment.
you can do "net.minecraft.entity.player.EntityPlayer".equals(transformedName) even when in obfuscated environement
| if (basicClass == null) return null; | ||
|
|
||
| if (NameContext.Obf.entityPlayer.equals(name)) { | ||
| final ClassReader cr = new ClassReader(basicClass); |
There was a problem hiding this comment.
write the class reader code once for both classes
| ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS); | ||
| cn.accept(cw); | ||
|
|
||
| LOGGER.info("Transformed {}", name); |
There was a problem hiding this comment.
you don't need to log that you are transforming a class, however you should use a try catch and if it returns an exception, you should log it and return the untransformed class
| } | ||
|
|
||
| public void transformClassNode(NameContext names, ClassNode cn) { | ||
| LOGGER.info("Found EntityPlayer (class = {}, name context = {})", names.entityPlayer, names.name()); |
|
|
||
| var cursor = method.instructions.getFirst(); | ||
|
|
||
| while (cursor != null) { |
There was a problem hiding this comment.
why don't you use the built in iterator or a for loop over instructions.toArray()
| import org.objectweb.asm.tree.TypeInsnNode; | ||
| import org.objectweb.asm.tree.VarInsnNode; | ||
|
|
||
| public class ASMUtil { |
There was a problem hiding this comment.
is this class even used ? just delete it
There was a problem hiding this comment.
It was useful for me, so I figured I should leave it in for others. Instructions don't have a toString implementation, so you can't see what instructions in an InsnList do without debugging the transformer. Should I still remove it?
There was a problem hiding this comment.
yeah remove it, you can see the instructions in intellij, select the class do View > show bytecode
| final ClassReader cr = new ClassReader(basicClass); | ||
| final ClassNode cn = new ClassNode(); | ||
| cr.accept(cn, 0); | ||
| transformClassNode(NameContext.Deobf, cn); |
There was a problem hiding this comment.
I wouldn't do that to check the obfuscation state, check it from the injectData method in the IFMLLoadingPlugin implementation
|
this introduces a bug where if you have 2 of the same items in your hotbar for example 2 bows, you can draw your first bow and then switch the the second one and it will be fully draw, same with eating the same food We should instead find what code changes the items while they are being used which prevents the vanilla code |
EntityPlayer does not call onUsingTick properly on the server because it uses a == instead of a proper ItemStack.areItemStacksEqual call. Another piece of code constantly copies the itemInUse stack, so this condition is never true. This transformer changes the == to call ItemStack.areItemStacksEqual instead. This isn't a mixin because mixins can't change expressions like this.