Skip to content

Block Position encoding appears to be broken #107

@codingcat2468

Description

@codingcat2468

Hi, there appears to be an issue with how BlockPositions are being encoded in DataTypeIO:

public static void writeBlockPosition(DataOutputStream out, BlockPosition position) throws IOException {
out.writeLong(((position.getX() & 0x3FFFFFF) << 38) | ((position.getZ() & 0x3FFFFFF) << 12) | (position.getY() & 0xFFF));
}

Since the the getX(), getY()and getZ() methods all return integers and writeLong() obviously expects a long, the shifted integers get implicitly casted into a long here. This is not the desired behavior, since applying high those high bitshifts on an integer instead of a long results in some of the bits overflowing, which corrupts the position data.

We can see this issue by logging the packets the client receives, and checking the decoded block position values. For example, checking the values of a received PacketPlayOutSpawnPosition (where this method is used) results in the following data on the client side:

net.minecraft.network.protocol.game.ClientboundSetDefaultSpawnPositionPacket
comp_4904: RespawnData[globalPos=ResourceKey[minecraft:dimension / minecraft:world] BlockPos{x=0, y=979, z=18}, yaw=0.0, pitch=0.0]

While the expected position (as specified in server.properties) is completely different:

world-spawn=world;15;19;18;0;0

This issued could be fixed by just casting all block positions to a long before applying the shifting.
I assume this issue went unnoticed for a while due to writeBlockPosition not being used in many places where the received values actually matter much, e.g. the default spawn position.
(I didn't want to open a PR for this, since it's just a very simple one-line fix!)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions