Uploaded image for project: 'Minecraft: Java Edition'
  1. Minecraft: Java Edition
  2. MC-52274

Dismounting does not work to the South, South east, and South west, and is offset in negative quadrants

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Minecraft 15w39a
    • Minecraft 1.7.5, Minecraft 14w11b, Minecraft 1.7.10, Minecraft 14w28b, Minecraft 14w33c, Minecraft 1.8, Minecraft 1.8.3, Minecraft 1.8.4, Minecraft 1.8.6, Minecraft 1.8.7, Minecraft 1.8.8, Minecraft 15w31a, Minecraft 15w31b, Minecraft 15w31c, Minecraft 15w32a, Minecraft 15w32b, Minecraft 15w32c, Minecraft 15w33c, Minecraft 15w34a, Minecraft 15w34b, Minecraft 15w34c, Minecraft 15w34d, Minecraft 15w35b, Minecraft 15w35d, Minecraft 15w35e, Minecraft 15w36b, Minecraft 15w36c, Minecraft 15w36d, Minecraft 15w37a, Minecraft 15w38a, Minecraft 15w38b
    • Confirmed

      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)).

      net.minecraft.entity.EntityLivingBase, lines 1501 - 1541
          /**
           * 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:

      EntityLivingBase.dismountEntity, variables renamed
          /**
           * 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:

      EntityLivingBase corrected, lines 1501-1541
          /**
           * 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:

      rl, lines 1184 to 1284
        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:

      rl, lines 1184 to 1214
        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(☃, ☃, ☃);
        }
      

            ProfMobius ProfMobius (Thomas Guimbretiere)
            pokechu22 [Mod] Pokechu22
            Votes:
            242 Vote for this issue
            Watchers:
            19 Start watching this issue

              Created:
              Updated:
              Resolved:
              CHK: