MC-56361Discrepancy dismounting behaviour between quadrants
When dismounting from a "vehicle", the game isn't checking the right place if any coordinate is negative.
Way to witness the bug
- Create a new single-player superflat game.
- Head to South-East quadrant ( /tp 10 10 10 )
- Lay a rail
- Put a minecart on it
- Enter the minecart and then shift-exit it
- Destroy the block you were put on (optional - fill it with lava)
- repeat the enter-exit cycle
- Notice you have been ejected to another (safe) block
- Head to North-West quadrant ( /tp -10 10 -10 )
- Lay a rail
- Put a minecart on it
- Enter the minecart and then shift-exit it
- Destroy the block you were put on (optional - fill it with lava)
- repeat the enter-exit cycle
- Notice you have been ejected to the same place (optional - you are now swimming into lava)
Second part can be also done on NE or SW quadrants, to the same result.
Created Issue:
Discrepancy dismounting behaviour between quadrants
When dismounting from a "vehicle", the game isn't checking the right place if any coordinate is negative.
Way to witness the bug
- Create a new single-player superflat game.
- Head to South-East quadrant ( /tp 10 10 10 )
- Lay a rail
- Put a minecart on it
- Enter the minecart and then shift-exit it
- Destroy the block you were put on (optional - fill it with lava)
- repeat the enter-exit cycle
- Notice you have been ejected to another (safe) block
- Head to North-West quadrant ( /tp -10 10 -10 )
- Lay a rail
- Put a minecart on it
- Enter the minecart and then shift-exit it
- Destroy the block you were put on (optional - fill it with lava)
- repeat the enter-exit cycle
- Notice you have been ejected to the same place (optional - you are now swimming into lava)
Second part can be also done on NE or SW quadrants, to the same result.
![[Mod] Pokechu22](../../../data/avatars/users/92931.png){kind=small}
When exiting a minecart (or any other vehicle), and the NW, W, N, NE, and E directions are blocked, the player will exit in the middle of the minecart, even if there is free space in the S, SE, or SW directions.
It results in the player spawning above/centered from the minecart, even when there is a block above it.
In the screenshot, the red and green color show the exit pattern.
Red = Player will never exit here
Green = Player exits on these blocks if there is room
Additionally, when attempting to dismount a minecart in a quadrant other than +, +, the dismount location is offset by 1 block. (MC-56361).
To reproduce this, put a block in the air with a rail on it in the +, + quadrant, put a minecart on it, and dismount. You will dismount onto the block. Attempt to repeat it in any other quadrant. You will be dismounted in the air next to the block.
The attached world DismountTestWorld.zip
can be used to see this bug in action.
Why this matters
Inaccurate dismount locations make minecarts inherently dangerous. You can build a rail across lava that looks safe, but instead ends up dropping you into lava and causing you to lose your stuff.
Programmatic cause
Note: MCP names for Minecraft 1.8 (mcp910) are used here. Obfuscated names as of 15w38b appear at the bottom. Older obfuscation is archived here.
The entirety of this bug is in the entity dismount code (net.minecraft.entity.EntityLivingBase.dismountEntity(Entity)).
/**
* Moves the entity to a position out of the way of its mount.
*/
public void dismountEntity(Entity p_110145_1_)
{
double var3 = p_110145_1_.posX;
double var5 = p_110145_1_.getEntityBoundingBox().minY + (double)p_110145_1_.height;
double var7 = p_110145_1_.posZ;
byte var9 = 1;
for (int var10 = -var9; var10 <= var9; ++var10)
{
for (int var11 = -var9; var11 < var9; ++var11)
{
if (var10 != 0 || var11 != 0)
{
int var12 = (int)(this.posX + (double)var10);
int var13 = (int)(this.posZ + (double)var11);
AxisAlignedBB var2 = this.getEntityBoundingBox().offset((double)var10, 1.0D, (double)var11);
if (this.worldObj.func_147461_a(var2).isEmpty())
{
if (World.doesBlockHaveSolidTopSurface(this.worldObj, new BlockPos(var12, (int)this.posY, var13)))
{
this.setPositionAndUpdate(this.posX + (double)var10, this.posY + 1.0D, this.posZ + (double)var11);
return;
}
if (World.doesBlockHaveSolidTopSurface(this.worldObj, new BlockPos(var12, (int)this.posY - 1, var13)) || this.worldObj.getBlockState(new BlockPos(var12, (int)this.posY - 1, var13)).getBlock().getMaterial() == Material.water)
{
var3 = this.posX + (double)var10;
var5 = this.posY + 1.0D;
var7 = this.posZ + (double)var11;
}
}
}
}
}
this.setPositionAndUpdate(var3, var5, var7);
}
Now, that code is obfuscated pretty badly, so here's a version with renamed variables:
/**
* Moves the entity to a position out of the way of its mount.
*/
public void dismountEntity(Entity dismountedFrom)
{
double dismountedX = dismountedFrom.posX;
double dismountedY = dismountedFrom.getEntityBoundingBox().minY + (double)dismountedFrom.height;
double dismountedZ = dismountedFrom.posZ;
byte testRange = 1;
for (int offsetX = -testRange; offsetX <= testRange; ++offsetX)
{
for (int offsetZ = -testRange; offsetZ < testRange; ++offsetZ)
{
if (offsetX != 0 || offsetZ != 0)
{
int x = (int)(this.posX + (double)offsetX);
int z = (int)(this.posZ + (double)offsetZ);
AxisAlignedBB aabb = this.getEntityBoundingBox().offset((double)offsetX, 1.0D, (double)offsetZ);
if (this.worldObj.func_147461_a(aabb).isEmpty()) //Gets colision boxes of all blocks intersecting with the aabb.
{
if (World.doesBlockHaveSolidTopSurface(this.worldObj, new BlockPos(x, (int)this.posY, z)))
{
this.setPositionAndUpdate(this.posX + (double)offsetX, this.posY + 1.0D, this.posZ + (double)offsetZ);
return;
}
if (World.doesBlockHaveSolidTopSurface(this.worldObj, new BlockPos(x, (int)this.posY - 1, z)) || this.worldObj.getBlockState(new BlockPos(x, (int)this.posY - 1, z)).getBlock().getMaterial() == Material.water)
{
dismountedX = this.posX + (double)offsetX;
dismountedY = this.posY + 1.0D;
dismountedZ = this.posZ + (double)offsetZ;
}
}
}
}
}
this.setPositionAndUpdate(dismountedX, dismountedY, dismountedZ);
}
It should be fairly obvious why dismounting doesn't work to the south: the loop for offsetZ uses only <, meaning that it only checks offsets of -1 and 0!
The fix is just to change < to <=. Then, it's possible to dismount to the south.
The second half (offset dismounting on negative quadrants) is more subtle. It's related to the way that the block position checks occur. Specifically, note that x and z are {{int}}s.
Well, posX and posZ are usually offset by .5. And, (int)(-1.5 - 1) is -2, rather than -3. That causes the tested block to be offset by one, meaning that the teleported location is off by one. Here's an ideone demo.
This is also an easy fix, though. BlockPos has a constructor that takes double}}s rather than {{int}}s, and it automatically gets the correct value (it floors it). So, all that needs to be done make {{x and z doubles, and remove the other int casts on posY in the BlockPos constructor calls.
Here's code with both of these issues corrected:
/**
* Moves the entity to a position out of the way of its mount.
*/
public void dismountEntity(Entity dismountedFrom)
{
double dismountedX = dismountedFrom.posX;
double dismountedY = dismountedFrom.getEntityBoundingBox().minY + (double)dismountedFrom.height;
double dismountedZ = dismountedFrom.posZ;
byte testRange = 1;
for (int offsetX = -testRange; offsetX <= testRange; ++offsetX)
{
for (int offsetZ = -testRange; offsetZ <= testRange; ++offsetZ) //Changed < to <=
{
if (offsetX != 0 || offsetZ != 0)
{
double x = (this.posX + offsetX); //Changed int to double and removed cast
double z = (this.posZ + offsetZ); //Changed int to double and removed cast
AxisAlignedBB aabb = this.getEntityBoundingBox().offset((double)offsetX, 1.0D, (double)offsetZ);
if (this.worldObj.func_147461_a(aabb).isEmpty())
{
if (World.doesBlockHaveSolidTopSurface(this.worldObj, new BlockPos(x, this.posY, z))) //Removed posY cast
{
this.setPositionAndUpdate(this.posX + (double)offsetX, this.posY + 1.0D, this.posZ + (double)offsetZ);
return;
}
if (World.doesBlockHaveSolidTopSurface(this.worldObj, new BlockPos(x, this.posY - 1, z)) || this.worldObj.getBlockState(new BlockPos(x, this.posY - 1, z)).getBlock().getMaterial() == Material.water) //Removed posY casts
{
dismountedX = this.posX + (double)offsetX;
dismountedY = this.posY + 1.0D;
dismountedZ = this.posZ + (double)offsetZ;
}
}
}
}
}
this.setPositionAndUpdate(dismountedX, dismountedY, dismountedZ);
}
Here are the code snippets from above with 15w38b's obfuscation (jd is used to decompile):
Original EntityLivingBase.dismountEntity:
public void q(rc ☃) { double ☃ = ☃.q; double ☃ = ☃.aZ().b + ☃.I; double ☃ = ☃.s; int ☃ = 1; for (int ☃ = -☃; ☃ <= ☃; ☃++) { for (int ☃ = -☃; ☃ < ☃; ☃++) { if ((☃ != 0) || (☃ != 0)) { int ☃ = (int)(this.q + ☃); int ☃ = (int)(this.s + ☃); ayz ☃ = aZ().c(☃, 1.0D, ☃); if (this.m.a(☃).isEmpty()) { if (agr.a(this.m, new cj(☃, (int)this.r, ☃))) { a(this.q + ☃, this.r + 1.0D, this.s + ☃); return; } if ((agr.a(this.m, new cj(☃, (int)this.r - 1, ☃))) || (this.m.p(new cj(☃, (int)this.r - 1, ☃)).a() == awe.h)) { ☃ = this.q + ☃; ☃ = this.r + 1.0D; ☃ = this.s + ☃; } } } } } a(☃, ☃, ☃); }
Fixed version:
public void q(rc ☃) { double ☃ = ☃.q; double ☃ = ☃.aZ().b + ☃.I; double ☃ = ☃.s; int ☃ = 1; for (int ☃ = -☃; ☃ <= ☃; ☃++) { for (int ☃ = -☃; ☃ <= ☃; ☃++) { //Changed < to <= if ((☃ != 0) || (☃ != 0)) { dobule ☃ = (this.q + ☃); //Changed to double and removed cast dobule ☃ = (this.s + ☃); //Changed to double and removed cast ayz ☃ = aZ().c(☃, 1.0D, ☃); if (this.m.a(☃).isEmpty()) { if (agr.a(this.m, new cj(☃, this.r, ☃))) { //Removed int cast on this.r a(this.q + ☃, this.r + 1.0D, this.s + ☃); return; } if ((agr.a(this.m, new cj(☃, (int)this.r - 1, ☃))) || (this.m.p(new cj(☃, this.r - 1, ☃)).a() == awe.h)) { //Removed int cast on this.r ☃ = this.q + ☃; ☃ = this.r + 1.0D; ☃ = this.s + ☃; } } } } } a(☃, ☃, ☃); }
I found the cause.
The main cause of this bug is that the loop in EntityLivingBase.dismountEntity(Entity) (obf: xm.q(wv)) just doesn't test a z of +1. This is the main reason why this bug exists. To fix it, all that would need to be done is replacing the < with <=.
Here's the original code, with proper variable names for readability.
public void dismountEntity(Entity entity) { double newX = entity.posX; double newY = entity.getEntityBoundingBox().minY + entity.height; double newZ = entity.posZ; byte range = 1; for (int offsetX = -range; offsetX <= range; ++offsetX) { for (int offsetZ = -range; offsetZ < range; ++offsetZ) { if (offsetX != 0 || offsetZ != 0) { int testX = (int)(this.posX + offsetX); int testZ = (int)(this.posZ + offsetZ); AxisAlignedBB aabb = this.getEntityBoundingBox().offset( offsetX, 1.0D, offsetZ); if (this.worldObj.func_147461_a(aabb).isEmpty()) { if (World.doesBlockHaveSolidTopSurface(this.worldObj, new BlockPos(testX, (int) this.posY, testZ))) { this.setPositionAndUpdate(this.posX + offsetX, this.posY + 1.0D, this.posZ + offsetZ); return; } if (World .doesBlockHaveSolidTopSurface(this.worldObj, new BlockPos(testX, (int) this.posY - 1, testZ)) || this.worldObj .getBlockState( new BlockPos(testX, (int) this.posY - 1, testZ)).getBlock() .getMaterial() == Material.water) { newX = this.posX + offsetX; newY = this.posY + 1.0D; newZ = this.posZ + offsetZ; } } } } } this.setPositionAndUpdate(newX, newY, newZ); }
The first part of the bug is in the loop: offsetZ only goes through the range of -1 to 0, rather than -1 to +1. To fix that, all that would need to be done is changing < to <=. Changing that fixes part of the bug – dismounts now work perfectly in positive x and z areas.
However, there is a second bug in that method (which is actually MC-56361), causing dismounts to still not work everywhere else. This happens when x or z is negative. It's caused by the int casts (for testX and testZ). To fix it, all that is needed is to remove the casts and change the type of testX and testZ to double – BlockPos (obf: dt) already has a constructor taking doubles, which handles the flooring automatically. With both of these changes, the bug is fixed in its entirety.
Here's the code for that method with both of these changes:
public void dismountEntity(Entity entity) { double newX = entity.posX; double newY = entity.getEntityBoundingBox().minY + entity.height; double newZ = entity.posZ; byte range = 1; for (int offsetX = -range; offsetX <= range; ++offsetX) { for (int offsetZ = -range; offsetZ <= range; ++offsetZ) { //Changed if (offsetX != 0 || offsetZ != 0) { double testX = (this.posX + offsetX); //Changed double testZ = (this.posZ + offsetZ); //Changed AxisAlignedBB aabb = this.getEntityBoundingBox().offset( offsetX, 1.0D, offsetZ); if (this.worldObj.func_147461_a(aabb).isEmpty()) { if (World.doesBlockHaveSolidTopSurface(this.worldObj, new BlockPos(testX, this.posY, testZ))) { this.setPositionAndUpdate(this.posX + offsetX, this.posY + 1.0D, this.posZ + offsetZ); return; } if (World.doesBlockHaveSolidTopSurface(this.worldObj, new BlockPos(testX, this.posY - 1, testZ)) || this.worldObj .getBlockState( new BlockPos(testX, this.posY - 1, testZ)) .getBlock().getMaterial() == Material.water) { newX = this.posX + offsetX; newY = this.posY + 1.0D; newZ = this.posZ + offsetZ; } } } } } this.setPositionAndUpdate(newX, newY, newZ); }
Hopefully this is helpful and usable. I've also attached a test world for experimenting with the bug – it has examples of what currently happens and should happen, and also a place to test the different behaviors. It uses floating saddled pigs instead of minecarts so that they don't get shifted out of place, but it's the same behavior.
Note: Code is given using MCP910 names and line numbers, with 1.8's obfuscated names and numbers in parentheses.
Duplicate of
MC-51987/MC-52274