overhaul and fixes to cost calculation

src/main/java/baritone/pathing/movement/CalculationContext.java

@@ -53,6 +53,8 @@ public class CalculationContext {
     private final boolean canSprint;
     private final double placeBlockCost;
     private final boolean allowBreak;
+    private final boolean allowParkour;
+    private final boolean allowParkourPlace;
     private final int maxFallHeightNoWater;
     private final int maxFallHeightBucket;
     private final double waterWalkSpeed;
@@ -76,6 +78,8 @@ public class CalculationContext {
         this.canSprint = Baritone.settings().allowSprint.get() && player.getFoodStats().getFoodLevel() > 6;
         this.placeBlockCost = Baritone.settings().blockPlacementPenalty.get();
         this.allowBreak = Baritone.settings().allowBreak.get();
+        this.allowParkour = Baritone.settings().allowParkour.get();
+        this.allowParkourPlace = Baritone.settings().allowParkourPlace.get();
         this.maxFallHeightNoWater = Baritone.settings().maxFallHeightNoWater.get();
         this.maxFallHeightBucket = Baritone.settings().maxFallHeightBucket.get();
         int depth = EnchantmentHelper.getDepthStriderModifier(player);
@@ -173,6 +177,14 @@ public class CalculationContext {
         return allowBreak;
     }
 
+    public boolean allowParkour() {
+        return allowParkour;
+    }
+
+    public boolean allowParkourPlace() {
+        return allowParkourPlace;
+    }
+
     public int maxFallHeightNoWater() {
         return maxFallHeightNoWater;
     }

src/main/java/baritone/pathing/movement/MovementHelper.java

@@ -127,9 +127,9 @@ public interface MovementHelper extends ActionCosts, Helper {
      * not including water, and not including ladders or vines or cobwebs (they slow us down)
      *
      * @param context Calculation context to provide block state lookup
-     * @param x The block's x position
+     * @param x       The block's x position
-     * @param y The block's y position
+     * @param y       The block's y position
-     * @param z The block's z position
+     * @param z       The block's z position
      * @return Whether or not the block at the specified position
      */
     static boolean fullyPassable(CalculationContext context, int x, int y, int z) {
@@ -247,12 +247,11 @@ public interface MovementHelper extends ActionCosts, Helper {
      * through? Includes water because we know that we automatically jump on
      * water
      *
-     * @param bsi Block state provider
+     * @param bsi   Block state provider
-     * @param x The block's x position
+     * @param x     The block's x position
-     * @param y The block's y position
+     * @param y     The block's y position
-     * @param z The block's z position
+     * @param z     The block's z position
      * @param state The state of the block at the specified location
-     *
      * @return Whether or not the specified block can be walked on
      */
     static boolean canWalkOn(BlockStateInterface bsi, int x, int y, int z, IBlockState state) {
@@ -378,7 +377,7 @@ public interface MovementHelper extends ActionCosts, Helper {
      * AutoTool for a specific block
      *
      * @param ctx The player context
-     * @param b the blockstate to mine
+     * @param b   the blockstate to mine
      */
     static void switchToBestToolFor(IPlayerContext ctx, IBlockState b) {
         switchToBestToolFor(ctx, b, new ToolSet(ctx.player()));
@@ -388,8 +387,8 @@ public interface MovementHelper extends ActionCosts, Helper {
      * AutoTool for a specific block with precomputed ToolSet data
      *
      * @param ctx The player context
-     * @param b  the blockstate to mine
+     * @param b   the blockstate to mine
-     * @param ts previously calculated ToolSet
+     * @param ts  previously calculated ToolSet
      */
     static void switchToBestToolFor(IPlayerContext ctx, IBlockState b, ToolSet ts) {
         ctx.player().inventory.currentItem = ts.getBestSlot(b.getBlock());

src/main/java/baritone/pathing/movement/movements/MovementAscend.java

@@ -57,18 +57,7 @@ public class MovementAscend extends Movement {
     }
 
     public static double cost(CalculationContext context, int x, int y, int z, int destX, int destZ) {
-        IBlockState srcDown = context.get(x, y - 1, z);
-        if (srcDown.getBlock() == Blocks.LADDER || srcDown.getBlock() == Blocks.VINE) {
-            return COST_INF;
-        }
-        // we can jump from soul sand, but not from a bottom slab
-        boolean jumpingFromBottomSlab = MovementHelper.isBottomSlab(srcDown);
         IBlockState toPlace = context.get(destX, y, destZ);
-        boolean jumpingToBottomSlab = MovementHelper.isBottomSlab(toPlace);
-
-        if (jumpingFromBottomSlab && !jumpingToBottomSlab) {
-            return COST_INF;// the only thing we can ascend onto from a bottom slab is another bottom slab
-        }
         boolean hasToPlace = false;
         if (!MovementHelper.canWalkOn(context.bsi(), destX, y, destZ, toPlace)) {
             if (!context.canPlaceThrowawayAt(destX, y, destZ)) {
@@ -95,8 +84,8 @@ public class MovementAscend extends Movement {
                 return COST_INF;
             }
         }
-        IBlockState srcUp2 = null;
+        IBlockState srcUp2 = context.get(x, y + 2, z); // used lower down anyway
-        if (context.get(x, y + 3, z).getBlock() instanceof BlockFalling && (MovementHelper.canWalkThrough(context.bsi(), x, y + 1, z) || !((srcUp2 = context.get(x, y + 2, z)).getBlock() instanceof BlockFalling))) {//it would fall on us and possibly suffocate us
+        if (context.get(x, y + 3, z).getBlock() instanceof BlockFalling && (MovementHelper.canWalkThrough(context.bsi(), x, y + 1, z) || !(srcUp2.getBlock() instanceof BlockFalling))) {//it would fall on us and possibly suffocate us
             // HOWEVER, we assume that we're standing in the start position
             // that means that src and src.up(1) are both air
             // maybe they aren't now, but they will be by the time this starts
@@ -114,6 +103,16 @@ public class MovementAscend extends Movement {
             // it's possible srcUp is AIR from the start, and srcUp2 is falling
             // and in that scenario, when we arrive and break srcUp2, that lets srcUp3 fall on us and suffocate us
         }
+        IBlockState srcDown = context.get(x, y - 1, z);
+        if (srcDown.getBlock() == Blocks.LADDER || srcDown.getBlock() == Blocks.VINE) {
+            return COST_INF;
+        }
+        // we can jump from soul sand, but not from a bottom slab
+        boolean jumpingFromBottomSlab = MovementHelper.isBottomSlab(srcDown);
+        boolean jumpingToBottomSlab = MovementHelper.isBottomSlab(toPlace);
+        if (jumpingFromBottomSlab && !jumpingToBottomSlab) {
+            return COST_INF;// the only thing we can ascend onto from a bottom slab is another bottom slab
+        }
         double walk;
         if (jumpingToBottomSlab) {
             if (jumpingFromBottomSlab) {
@@ -136,10 +135,9 @@ public class MovementAscend extends Movement {
         if (hasToPlace) {
             totalCost += context.placeBlockCost();
         }
-        if (srcUp2 == null) {
+        // start with srcUp2 since we already have its state
-            srcUp2 = context.get(x, y + 2, z);
+        // includeFalling isn't needed because of the falling check above -- if srcUp3 is falling we will have already exited with COST_INF if we'd actually have to break it
-        }
+        totalCost += MovementHelper.getMiningDurationTicks(context, x, y + 2, z, srcUp2, false);
-        totalCost += MovementHelper.getMiningDurationTicks(context, x, y + 2, z, srcUp2, false); // TODO MAKE ABSOLUTELY SURE we don't need includeFalling here, from the falling check above
         if (totalCost >= COST_INF) {
             return COST_INF;
         }

src/main/java/baritone/pathing/movement/movements/MovementDescend.java

@@ -63,11 +63,6 @@ public class MovementDescend extends Movement {
     }
 
     public static void cost(CalculationContext context, int x, int y, int z, int destX, int destZ, MutableMoveResult res) {
-        Block fromDown = context.get(x, y - 1, z).getBlock();
-        if (fromDown == Blocks.LADDER || fromDown == Blocks.VINE) {
-            return;
-        }
-
         double totalCost = 0;
         IBlockState destDown = context.get(destX, y - 1, destZ);
         totalCost += MovementHelper.getMiningDurationTicks(context, destX, y - 1, destZ, destDown, false);
@@ -83,6 +78,11 @@ public class MovementDescend extends Movement {
             return;
         }
 
+        Block fromDown = context.get(x, y - 1, z).getBlock();
+        if (fromDown == Blocks.LADDER || fromDown == Blocks.VINE) {
+            return;
+        }
+
         // A
         //SA
         // A
@@ -107,7 +107,7 @@ public class MovementDescend extends Movement {
         double walk = WALK_OFF_BLOCK_COST;
         if (fromDown == Blocks.SOUL_SAND) {
             // use this ratio to apply the soul sand speed penalty to our 0.8 block distance
-            walk = WALK_ONE_OVER_SOUL_SAND_COST;
+            walk *= WALK_ONE_OVER_SOUL_SAND_COST / WALK_ONE_BLOCK_COST;
         }
         totalCost += walk + Math.max(FALL_N_BLOCKS_COST[1], CENTER_AFTER_FALL_COST);
         res.x = destX;

src/main/java/baritone/pathing/movement/movements/MovementDiagonal.java

@@ -58,10 +58,6 @@ public class MovementDiagonal extends Movement {
     }
 
     public static double cost(CalculationContext context, int x, int y, int z, int destX, int destZ) {
-        Block fromDown = context.get(x, y - 1, z).getBlock();
-        if (fromDown == Blocks.LADDER || fromDown == Blocks.VINE) {
-            return COST_INF;
-        }
         IBlockState destInto = context.get(destX, y, destZ);
         if (!MovementHelper.canWalkThrough(context.bsi(), destX, y, destZ, destInto) || !MovementHelper.canWalkThrough(context.bsi(), destX, y + 1, destZ)) {
             return COST_INF;
@@ -75,6 +71,10 @@ public class MovementDiagonal extends Movement {
         if (destWalkOn.getBlock() == Blocks.SOUL_SAND) {
             multiplier += (WALK_ONE_OVER_SOUL_SAND_COST - WALK_ONE_BLOCK_COST) / 2;
         }
+        Block fromDown = context.get(x, y - 1, z).getBlock();
+        if (fromDown == Blocks.LADDER || fromDown == Blocks.VINE) {
+            return COST_INF;
+        }
         if (fromDown == Blocks.SOUL_SAND) {
             multiplier += (WALK_ONE_OVER_SOUL_SAND_COST - WALK_ONE_BLOCK_COST) / 2;
         }

src/main/java/baritone/pathing/movement/movements/MovementDownward.java

@@ -51,14 +51,13 @@ public class MovementDownward extends Movement {
         if (!MovementHelper.canWalkOn(context.bsi(), x, y - 2, z)) {
             return COST_INF;
         }
-        IBlockState d = context.get(x, y - 1, z);
+        IBlockState down = context.get(x, y - 1, z);
-        Block td = d.getBlock();
+        Block downBlock = down.getBlock();
-        boolean ladder = td == Blocks.LADDER || td == Blocks.VINE;
+        if (downBlock == Blocks.LADDER || downBlock == Blocks.VINE) {
-        if (ladder) {
             return LADDER_DOWN_ONE_COST;
         } else {
             // we're standing on it, while it might be block falling, it'll be air by the time we get here in the movement
-            return FALL_N_BLOCKS_COST[1] + MovementHelper.getMiningDurationTicks(context, x, y - 1, z, d, false);
+            return FALL_N_BLOCKS_COST[1] + MovementHelper.getMiningDurationTicks(context, x, y - 1, z, down, false);
         }
     }
 

src/main/java/baritone/pathing/movement/movements/MovementParkour.java

@@ -17,7 +17,6 @@
 
 package baritone.pathing.movement.movements;
 
-import baritone.Baritone;
 import baritone.api.IBaritone;
 import baritone.api.pathing.movement.MovementStatus;
 import baritone.api.utils.BetterBlockPos;
@@ -30,7 +29,6 @@ import baritone.pathing.movement.Movement;
 import baritone.pathing.movement.MovementHelper;
 import baritone.pathing.movement.MovementState;
 import baritone.utils.BlockStateInterface;
-import baritone.utils.Helper;
 import baritone.utils.pathing.MutableMoveResult;
 import net.minecraft.block.Block;
 import net.minecraft.block.BlockStairs;
@@ -65,24 +63,22 @@ public class MovementParkour extends Movement {
     }
 
     public static void cost(CalculationContext context, int x, int y, int z, EnumFacing dir, MutableMoveResult res) {
-        if (!Baritone.settings().allowParkour.get()) {
+        if (!context.allowParkour()) {
-            return;
-        }
-        IBlockState standingOn = context.get(x, y - 1, z);
-        if (standingOn.getBlock() == Blocks.VINE || standingOn.getBlock() == Blocks.LADDER || standingOn.getBlock() instanceof BlockStairs || MovementHelper.isBottomSlab(standingOn)) {
-            return;
-        }
-        int xDiff = dir.getXOffset();
-        int zDiff = dir.getZOffset();
-        IBlockState adj = context.get(x + xDiff, y - 1, z + zDiff);
-        if (MovementHelper.avoidWalkingInto(adj.getBlock()) && adj.getBlock() != Blocks.WATER && adj.getBlock() != Blocks.FLOWING_WATER) { // magma sucks
-            return;
-        }
-        if (MovementHelper.canWalkOn(context.bsi(), x + xDiff, y - 1, z + zDiff, adj)) { // don't parkour if we could just traverse (for now)
             return;
         }
 
+        int xDiff = dir.getXOffset();
+        int zDiff = dir.getZOffset();
         if (!MovementHelper.fullyPassable(context, x + xDiff, y, z + zDiff)) {
+            // most common case at the top -- the adjacent block isn't air
+            return;
+        }
+        IBlockState adj = context.get(x + xDiff, y - 1, z + zDiff);
+        if (MovementHelper.canWalkOn(context.bsi(), x + xDiff, y - 1, z + zDiff, adj)) { // don't parkour if we could just traverse (for now)
+            // second most common case -- we could just traverse not parkour
+            return;
+        }
+        if (MovementHelper.avoidWalkingInto(adj.getBlock()) && adj.getBlock() != Blocks.WATER && adj.getBlock() != Blocks.FLOWING_WATER) { // magma sucks
             return;
         }
         if (!MovementHelper.fullyPassable(context, x + xDiff, y + 1, z + zDiff)) {
@@ -94,6 +90,10 @@ public class MovementParkour extends Movement {
         if (!MovementHelper.fullyPassable(context, x, y + 2, z)) {
             return;
         }
+        IBlockState standingOn = context.get(x, y - 1, z);
+        if (standingOn.getBlock() == Blocks.VINE || standingOn.getBlock() == Blocks.LADDER || standingOn.getBlock() instanceof BlockStairs || MovementHelper.isBottomSlab(standingOn)) {
+            return;
+        }
         int maxJump;
         if (standingOn.getBlock() == Blocks.SOUL_SAND) {
             maxJump = 2; // 1 block gap
@@ -122,20 +122,16 @@ public class MovementParkour extends Movement {
         if (maxJump != 4) {
             return;
         }
-        if (!Baritone.settings().allowParkourPlace.get()) {
+        if (!context.allowParkourPlace()) {
-            return;
-        }
-        if (!Baritone.settings().allowPlace.get()) {
-            Helper.HELPER.logDirect("allowParkourPlace enabled but allowPlace disabled?");
             return;
         }
         int destX = x + 4 * xDiff;
         int destZ = z + 4 * zDiff;
-        IBlockState toPlace = context.get(destX, y - 1, destZ);
         if (!context.canPlaceThrowawayAt(destX, y - 1, destZ)) {
             return;
         }
-        if (toPlace.getBlock() != Blocks.AIR && !MovementHelper.isWater(toPlace.getBlock()) && !MovementHelper.isReplacable(destX, y - 1, destZ, toPlace, context.world())) {
+        IBlockState toReplace = context.get(destX, y - 1, destZ);
+        if (toReplace.getBlock() != Blocks.AIR && !MovementHelper.isWater(toReplace.getBlock()) && !MovementHelper.isReplacable(destX, y - 1, destZ, toReplace, context.world())) {
             return;
         }
         for (int i = 0; i < 5; i++) {

src/main/java/baritone/pathing/movement/movements/MovementPillar.java

@@ -47,33 +47,36 @@ public class MovementPillar extends Movement {
     }
 
     public static double cost(CalculationContext context, int x, int y, int z) {
-        Block fromDown = context.get(x, y, z).getBlock();
+        Block from = context.get(x, y, z).getBlock();
-        boolean ladder = fromDown instanceof BlockLadder || fromDown instanceof BlockVine;
+        boolean ladder = from == Blocks.LADDER || from == Blocks.VINE;
-        IBlockState fromDownDown = context.get(x, y - 1, z);
+        IBlockState fromDown = context.get(x, y - 1, z);
         if (!ladder) {
-            if (fromDownDown.getBlock() instanceof BlockLadder || fromDownDown.getBlock() instanceof BlockVine) {
+            if (fromDown.getBlock() == Blocks.LADDER || fromDown.getBlock() == Blocks.VINE) {
-                return COST_INF;
+                return COST_INF; // can't pillar from a ladder or vine onto something that isn't also climbable
             }
-            if (fromDownDown.getBlock() instanceof BlockSlab && !((BlockSlab) fromDownDown.getBlock()).isDouble() && fromDownDown.getValue(BlockSlab.HALF) == BlockSlab.EnumBlockHalf.BOTTOM) {
+            if (fromDown.getBlock() instanceof BlockSlab && !((BlockSlab) fromDown.getBlock()).isDouble() && fromDown.getValue(BlockSlab.HALF) == BlockSlab.EnumBlockHalf.BOTTOM) {
                 return COST_INF; // can't pillar up from a bottom slab onto a non ladder
             }
         }
-        if (fromDown instanceof BlockVine && !hasAgainst(context, x, y, z)) {
+        if (from instanceof BlockVine && !hasAgainst(context, x, y, z)) { // TODO this vine can't be climbed, but we could place a pillar still since vines are replacable, no? perhaps the pillar jump would be impossible because of the slowdown actually.
             return COST_INF;
         }
         IBlockState toBreak = context.get(x, y + 2, z);
         Block toBreakBlock = toBreak.getBlock();
-        if (toBreakBlock instanceof BlockFenceGate) {
+        if (toBreakBlock instanceof BlockFenceGate) { // see issue #172
             return COST_INF;
         }
         Block srcUp = null;
-        if (MovementHelper.isWater(toBreakBlock) && MovementHelper.isWater(fromDown)) {
+        if (MovementHelper.isWater(toBreakBlock) && MovementHelper.isWater(from)) { // TODO should this also be allowed if toBreakBlock is air?
             srcUp = context.get(x, y + 1, z).getBlock();
             if (MovementHelper.isWater(srcUp)) {
-                return LADDER_UP_ONE_COST;
+                return LADDER_UP_ONE_COST; // allow ascending pillars of water, but only if we're already in one
             }
         }
-        if (!ladder && !context.canPlaceThrowawayAt(x, y, z)) {
+        if (!ladder && !context.canPlaceThrowawayAt(x, y, z)) { // we need to place a block where we started to jump on it
+            return COST_INF;
+        }
+        if (from instanceof BlockLiquid || fromDown.getBlock() instanceof BlockLiquid) {//can't pillar on water or in water
             return COST_INF;
         }
         double hardness = MovementHelper.getMiningDurationTicks(context, x, y + 2, z, toBreak, true);
@@ -81,10 +84,10 @@ public class MovementPillar extends Movement {
             return COST_INF;
         }
         if (hardness != 0) {
-            if (toBreakBlock instanceof BlockLadder || toBreakBlock instanceof BlockVine) {
+            if (toBreakBlock == Blocks.LADDER || toBreakBlock == Blocks.VINE) {
                 hardness = 0; // we won't actually need to break the ladder / vine because we're going to use it
             } else {
-                IBlockState check = context.get(x, y + 3, z);
+                IBlockState check = context.get(x, y + 3, z); // the block on top of the one we're going to break, could it fall on us?
                 if (check.getBlock() instanceof BlockFalling) {
                     // see MovementAscend's identical check for breaking a falling block above our head
                     if (srcUp == null) {
@@ -103,9 +106,6 @@ public class MovementPillar extends Movement {
                 //}
             }
         }
-        if (fromDown instanceof BlockLiquid || fromDownDown.getBlock() instanceof BlockLiquid) {//can't pillar on water or in water
-            return COST_INF;
-        }
         if (ladder) {
             return LADDER_UP_ONE_COST + hardness * 5;
         } else {

src/main/java/baritone/pathing/movement/movements/MovementTraverse.java

@@ -80,11 +80,11 @@ public class MovementTraverse extends Movement {
                     WC += (WALK_ONE_OVER_SOUL_SAND_COST - WALK_ONE_BLOCK_COST) / 2;
                 }
             }
-            double hardness1 = MovementHelper.getMiningDurationTicks(context, destX, y + 1, destZ, pb0, true);
+            double hardness1 = MovementHelper.getMiningDurationTicks(context, destX, y, destZ, pb1, false);
             if (hardness1 >= COST_INF) {
                 return COST_INF;
             }
-            double hardness2 = MovementHelper.getMiningDurationTicks(context, destX, y, destZ, pb1, false);
+            double hardness2 = MovementHelper.getMiningDurationTicks(context, destX, y + 1, destZ, pb0, true); // only include falling on the upper block to break
             if (hardness1 == 0 && hardness2 == 0) {
                 if (!water && context.canSprint()) {
                     // If there's nothing in the way, and this isn't water, and we aren't sneak placing
@@ -106,39 +106,39 @@ public class MovementTraverse extends Movement {
             if (destOn.getBlock().equals(Blocks.AIR) || MovementHelper.isReplacable(destX, y - 1, destZ, destOn, context.world())) {
                 boolean throughWater = MovementHelper.isWater(pb0.getBlock()) || MovementHelper.isWater(pb1.getBlock());
                 if (MovementHelper.isWater(destOn.getBlock()) && throughWater) {
+                    // this happens when assume walk on water is true and this is a traverse in water, which isn't allowed
                     return COST_INF;
                 }
                 if (!context.canPlaceThrowawayAt(destX, y - 1, destZ)) {
                     return COST_INF;
                 }
-                double hardness1 = MovementHelper.getMiningDurationTicks(context, destX, y, destZ, pb0, false);
+                double hardness1 = MovementHelper.getMiningDurationTicks(context, destX, y, destZ, pb1, false);
                 if (hardness1 >= COST_INF) {
                     return COST_INF;
                 }
-                double hardness2 = MovementHelper.getMiningDurationTicks(context, destX, y + 1, destZ, pb1, true);
+                double hardness2 = MovementHelper.getMiningDurationTicks(context, destX, y + 1, destZ, pb0, true); // only include falling on the upper block to break
-
                 double WC = throughWater ? context.waterWalkSpeed() : WALK_ONE_BLOCK_COST;
                 for (int i = 0; i < 4; i++) {
                     int againstX = destX + HORIZONTALS[i].getXOffset();
                     int againstZ = destZ + HORIZONTALS[i].getZOffset();
-                    if (againstX == x && againstZ == z) {
+                    if (againstX == x && againstZ == z) { // this would be a backplace
                         continue;
                     }
-                    if (MovementHelper.canPlaceAgainst(context.bsi(), againstX, y - 1, againstZ)) {
+                    if (MovementHelper.canPlaceAgainst(context.bsi(), againstX, y - 1, againstZ)) { // found a side place option
                         return WC + context.placeBlockCost() + hardness1 + hardness2;
                     }
                 }
+                // now that we've checked all possible directions to side place, we actually need to backplace
                 if (srcDown == Blocks.SOUL_SAND || (srcDown instanceof BlockSlab && !((BlockSlab) srcDown).isDouble())) {
                     return COST_INF; // can't sneak and backplace against soul sand or half slabs =/
                 }
                 if (srcDown == Blocks.FLOWING_WATER || srcDown == Blocks.WATER) {
                     return COST_INF; // this is obviously impossible
                 }
-                WC = WC * SNEAK_ONE_BLOCK_COST / WALK_ONE_BLOCK_COST;//since we are placing, we are sneaking
+                WC = WC * SNEAK_ONE_BLOCK_COST / WALK_ONE_BLOCK_COST;//since we are sneak backplacing, we are sneaking lol
                 return WC + context.placeBlockCost() + hardness1 + hardness2;
             }
             return COST_INF;
-            // Out.log("Can't walk on " + Baritone.get(positionsToPlace[0]).getBlock());
         }
     }