Fix instructions for some packets #42
Conversation
| operations.append(Operation(instruction.pos, "if", | ||
| condition=field.value + ".isLeft()")) | ||
| operations += _PIT._lambda_operations( | ||
| classloader, classes, instruction.pos, verbose, | ||
| left_consumer, [instance, field.value + ".left()"] | ||
| ) | ||
| operations.append(Operation(instruction.pos + 1 - SUB_INS_EPSILON, "else")) | ||
| operations += _PIT._lambda_operations( | ||
| classloader, classes, instruction.pos + 1, verbose, | ||
| right_consumer, [instance, field.value + ".right()"] | ||
| ) | ||
| operations.append(Operation(instruction.pos + 2 - SUB_INS_EPSILON, "endif")) |
There was a problem hiding this comment.
I'm not 100% sure this will work correctly. There's a different implementation for java/util/Map (which I'm also not sure is correct, but it does guarantee that instruction.pos <= pos and pos < instruction.pos + 1). Though, I also don't 100% remember what pos corresponds to - if it's a byte offset, this might be better as call instructions are multiple bytes.
There was a problem hiding this comment.
When I didn't add to the instruction pos the else was empty and the instructions were getting mixed with the if instructions, so I think the implementation for Map might actually be wrong. I also just checked by doing print(instruction.pos) at the beginning of a loop where a method is disassembled, and it does look like they're byte offsets since the positions usually increase by more than 1 at a time.
Ok also you don't need to answer this, but why do operations even need to store their position? Can't that just be figured out from the order in the operations list?
There was a problem hiding this comment.
In that case, it's probably fine.
Ok also you don't need to answer this, but why do operations even need to store their position? Can't that just be figured out from the order in the operations list?
I'm pretty sure that specific part of the code predates me. I do know that they aren't initially sorted (which is what ordered_operations does). I assume that it's related to endif, which I think gets put into the list as soon as a jump is seen. Not that the if handling is great; see #2 and #3. I think it also relates to converting from a list of instructions into a nested tree structure (see format).
|
I haven't reviewed this in detail, but it seems to work fine (including on 1.18 and on 22w43a), both of which had broken packets, so I'll merge it. Thanks! |
I forgot to update burger on my pi, so it was building with the old version.
This PR fixes instruction extraction erroring for several packets. I'm not sure how many of these were broken specifically by 22w42a but I fixed them all regardless.
Changes:
lengthfield to it, and I also added that length field to String because I might as well)putfieldinstruction (Advancement.Builder's writer set a field when writing a packet for some reason)(Note: I haven't looked into the new snapshot that released today yet so I'm hoping they didn't add more weird types)