Truncate player position instead of round for concrete speed boost

[esotericist]

Java rounds the X/Y/Z values of the player position to nearest integer when invoking getBlockState(), causing it to access a block one off when the player is at or past the halfway point within the block.

You may or may not want to exclude the Y element from this, if you want slab on top of concrete to not provide the concrete benefit (although personally, I like not slowing down using slab or chisel-and-bits staircases)

This should probably also be applied to other maintained versions, I was mostly interested in getting this fixed for my private server because it was driving some of us batty.

[ChloeDawn]

.down() can be excluded in favour of player.posY - 1 in the pos constructor parameters, to avoid an additional BlockPos being constructed each time.

[ChloeDawn]

This design will never be truly accurate. The most accurate design I have found can be seen implemented here. This iterates over all bounding boxes that the player is colliding with, and for each box, finds the centre point, and queries the blockstate at that point. Of course, this is considerably more expensive, but accuracy often comes at a cost.

[ChloeDawn]

Alternatively, the vanilla implementation would be as follows, respecting the logic applied to Block#onEntityWalk.

EntityPlayerSP player = (EntityPlayerSP) event.player;
int x = MathHelper.floor(player.posX);
int y = MathHelper.floor(player.posY - 0.2D);
int z = MathHelper.floor(player.posZ);
BlockPos pos = new BlockPos(x, y, z);
IBlockState state = player.world.getBlockState(pos);

if (state.getMaterial() == Material.AIR) {
    BlockPos down = pos.down();
    IBlockState below = player.world.getBlockState(down);
    Block block = below.getBlock();
    if (block instanceof BlockFence || block instanceof BlockWall || block instanceof BlockFenceGate) {
        state = below;
    }
}

Block block = state.getBlock();

if (block != null && speedupBlocks.contains(block)) {
    manualInputCheck.updatePlayerMoveState();
    if ((manualInputCheck.moveForward != 0 || manualInputCheck.moveStrafe != 0) && !player.isInWater()) {
        player.motionX *= Configurations.concreteVelocityMult + 0.05D;
        player.motionZ *= Configurations.concreteVelocityMult + 0.05D;
    }
}

[tterrag1098]

Can you verify that this changes anything at all? From what I can tell your code is 100% equivalent to the current code. Vec3i(double, double, double) already calls floor on all parameters.

[esotericist]

@tterrag1098 I can say for a fact that this code DOES change things. I was having trouble, in-game, with not counting as standing on concrete for the purpose of chiseled speed boost when at or past the halfway point on the concrete.

With the code above, this no longer happens in actual play.

I'm pretty sure that the code I changed never actually has Vec3I(double, double, double) called at any point; BlockPos(Vec3d) appears to just cast the doubles to integers, and java then rounds those.

[tterrag1098]

But where is BlockPos(Vec3d) called? This is what I see:

    public BlockPos getPosition()
    {
        return new BlockPos(this.posX, this.posY + 0.5D, this.posZ);
    }

[esotericist]

I'm not claiming solid understanding of the minecraft codebase. I spend a lot of time flailing around trying to find things, and freely admit I misinterpret what happens how because java is alien to me.

I am claiming that the problem I experienced occurs with the codebase from chisel, and that the change I made has actual genuine impact that fixes the problem I experience.

Can we please focus on that?

[tterrag1098]

No, because I won't merge a change that I don't understand. I believe you that there is an issue. I don't believe that the explanation for this fix is accurate, because it doesn't match my own findings.

[gchpaco]

At least as of the current recommended Forge what actually happens is this:

The original code casts the player to a EntityPlayerSP, not an EntityPlayerMP or a EntityPlayer; as such its implementation will take precedence. EntityPlayerSP::getPosition which the original does not do anything convenient; what it does instead is this

    public BlockPos getPosition()
    {
        return new BlockPos(this.posX + 0.5D, this.posY + 0.5D, this.posZ + 0.5D);
    }

The relevant BlockPos constructor calls the relevant Vec3i constructor, and that calls MathHelper.floor. That methods implementation is as follows:

    public static int floor(double value)
    {
        int i = (int)value;
        return value < (double)i ? i - 1 : i;
    }

That causes a double to int narrowing conversion, then attempts to clean up. Per Section 5.1.3 of the Java Language Specification, a double to int narrowing conversion will first convert NaN to 0 (not relevant here), otherwise it will be truncated using IEEE 754 round-toward-zero mode.

Minecraft blocks have their minimum x and z levels at the south east corner. So if a concrete block is at position 32, 69, 48, I am standing on it if I am anywhere between 32.0, 70, 48.0 or 33.0, 70,
49.0. However, if I am standing at 32.6, 70, 48.6, say, despite standing on the block the cascade of earlier errors will result in the code concluding I am standing on floor(33.1), 70, floor(49.1) which is just 33, 70, 49 which is not on the block.

Due to the bizarre half a block offset included in getPosition, it is probably better just to extract the raw fields and do the floor conversion directly, with the benefit of using a standard library method that can do the floor conversion directly instead of going through int conversion and then correcting.

[tterrag1098]

There's the missing puzzle piece, I didn't realize EntityPlayerSP had a special implementation. I'll be pushing my own fix for this, but thank you @esotericist for bringing it to my attention and @gchpaco for the detailed explanation.

[gchpaco]

tterrag, could I convince you to make the vertical correction you apply in d8895d9 0.4 instead of 0.5? That way, staircases made by placing slabs on top of concrete would still provide the speed buff, while farmland and soul sand wouldn't.