movement and goal cleanup, more pure with CalculationContext

src/main/java/baritone/bot/Settings.java

@@ -31,6 +31,11 @@ import java.util.*;
 public class Settings {
     public Setting<Boolean> allowBreak = new Setting<>(true);
     public Setting<Boolean> allowPlaceThrowaway = new Setting<>(true);
+    /**
+     * It doesn't actually take twenty ticks to place a block, this cost is so high
+     * because we want to generally conserve blocks which might be limited
+     */
+    public Setting<Double> blockPlacementPenalty = new Setting<>(20D);
     public Setting<Boolean> allowSprint = new Setting<>(true);
     public Setting<Double> costHeuristic = new <Double>Setting<Double>(4D);
     public Setting<Boolean> chuckCaching = new Setting<>(false);
@@ -44,7 +49,7 @@ public class Settings {
     public Setting<Boolean> slowPath = new Setting<>(false);
     public Setting<Number> slowPathTimeDelayMS = new Setting<>(100L);
     public Setting<Number> slowPathTimeoutMS = new Setting<>(40000L);
-    public Setting<List<Item>> acceptableThrowAwayItems = new Setting<>(Arrays.asList(
+    public Setting<List<Item>> acceptableThrowawayItems = new Setting<>(Arrays.asList(
             Item.getItemFromBlock(Blocks.DIRT),
             Item.getItemFromBlock(Blocks.COBBLESTONE),
             Item.getItemFromBlock(Blocks.NETHERRACK)

src/main/java/baritone/bot/pathing/goals/GoalBlock.java

@@ -68,9 +68,9 @@ public class GoalBlock implements Goal {
 
     @Override
     public double heuristic(BlockPos pos) {
-        double xDiff = pos.getX() - this.x;
+        int xDiff = pos.getX() - this.x;
-        double yDiff = pos.getY() - this.y;
+        int yDiff = pos.getY() - this.y;
-        double zDiff = pos.getZ() - this.z;
+        int zDiff = pos.getZ() - this.z;
         return calculate(xDiff, yDiff, zDiff);
     }
 
@@ -86,25 +86,26 @@ public class GoalBlock implements Goal {
         return new BlockPos(x, y, z);
     }
 
-    public static double calculate(double xDiff, double yDiff, double zDiff) {
+    public static double calculate(double xDiff, int yDiff, double zDiff) {
         double pythaDist = Math.sqrt(xDiff * xDiff + zDiff * zDiff);
         double heuristic = 0;
-        double baseline = (PLACE_ONE_BLOCK_COST + FALL_N_BLOCKS_COST[1]) * 32;
         if (pythaDist < MAX) {//if we are more than MAX away, ignore the Y coordinate. It really doesn't matter how far away your Y coordinate is if you X coordinate is 1000 blocks away.
             //as we get closer, slowly reintroduce the Y coordinate as a heuristic cost
-            double multiplier = pythaDist < MIN ? 1 : 1 - (pythaDist - MIN) / (MAX - MIN);
+            double multiplier;
-            if (yDiff < 0) {//pos.getY()-this.y<0 therefore pos.getY()<this.y, so target is above current
+            if (pythaDist < MIN) {
-                heuristic -= yDiff * (PLACE_ONE_BLOCK_COST * 0.7 + JUMP_ONE_BLOCK_COST);//target above current
+                multiplier = 1;
             } else {
-                heuristic += yDiff * (10 + FALL_N_BLOCKS_COST[1]);//target below current
+                multiplier = 1 - (pythaDist - MIN) / (MAX - MIN);
             }
+
+            // if yDiff is 1 that means that pos.getY()-this.y==1 which means that we're 1 block below where we should be
+            // therefore going from 0,0,0 to a GoalYLevel of pos.getY()-this.y is accurate
+            heuristic += new GoalYLevel(yDiff).heuristic(new BlockPos(0, 0, 0));
+
             heuristic *= multiplier;
-            heuristic += (1 - multiplier) * baseline;
-        } else {
-            heuristic += baseline;
         }
         //use the pythagorean and manhattan mixture from GoalXZ
-        heuristic += GoalXZ.calculate(xDiff, zDiff, pythaDist);
+        heuristic += GoalXZ.calculate(xDiff, zDiff);
         return heuristic;
     }
 }

src/main/java/baritone/bot/pathing/goals/GoalTwoBlocks.java

@@ -60,7 +60,7 @@ public class GoalTwoBlocks implements Goal {
     @Override
     public double heuristic(BlockPos pos) {
         double xDiff = pos.getX() - this.x;
-        double yDiff = pos.getY() - this.y;
+        int yDiff = pos.getY() - this.y;
         if (yDiff < 0) {
             yDiff++;
         }

src/main/java/baritone/bot/pathing/goals/GoalXZ.java

@@ -64,24 +64,6 @@ public class GoalXZ implements Goal {
     }
 
     public static double calculate(double xDiff, double zDiff) {
-        return calculate(xDiff, zDiff, 0);
-    }
-
-    /*
-     public static double calculate(double xDiff, double zDiff) {
-     double pythaDist = Math.sqrt(xDiff * xDiff + zDiff * zDiff);
-     return calculate(xDiff, zDiff, pythaDist);
-     }
-     public static double calculateOld(double xDiff, double zDiff, double pythaDist) {
-     double heuristic = 0;
-     heuristic += Math.abs(xDiff) * Movement.WALK_ONE_BLOCK_COST * 1.1;//overestimate
-     heuristic += Math.abs(zDiff) * Movement.WALK_ONE_BLOCK_COST * 1.1;
-     heuristic += pythaDist / 10 * Movement.WALK_ONE_BLOCK_COST;
-     return heuristic;
-     }
-     */
-
-    public static double calculate(double xDiff, double zDiff, double pythaDist) {
         //This is a combination of pythagorean and manhattan distance
         //It takes into account the fact that pathing can either walk diagonally or forwards
 

src/main/java/baritone/bot/pathing/goals/GoalYLevel.java

@@ -42,9 +42,15 @@ public class GoalYLevel implements Goal {
 
     @Override
     public double heuristic(BlockPos pos) {
-        // The number 20 was chosen somewhat randomly.
+        if (pos.getY() > level) {
-        // TODO fix that ^
+            // need to descend
-        return 20 * Math.abs(pos.getY() - level);
+            return FALL_N_BLOCKS_COST[pos.getY() - level];
+        }
+        if (pos.getY() < level) {
+            // need to ascend
+            return (level - pos.getY()) * JUMP_ONE_BLOCK_COST;
+        }
+        return 0;
     }
 
     @Override

src/main/java/baritone/bot/pathing/movement/ActionCosts.java

@@ -38,12 +38,6 @@ public interface ActionCosts extends ActionCostsButOnlyTheOnesThatMakeMickeyDieI
      */
     double CENTER_AFTER_FALL_COST = WALK_ONE_BLOCK_COST - WALK_OFF_BLOCK_COST;
 
-    /**
-     * It doesn't actually take ten ticks to place a block, this cost is so high
-     * because we want to generally conserve blocks which might be limited
-     */
-    double PLACE_ONE_BLOCK_COST = 20;
-
     /**
      * don't make this Double.MAX_VALUE because it's added to other things, maybe other COST_INFs,
      * and that would make it overflow to negative

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

@@ -36,6 +36,8 @@ public class CalculationContext implements Helper {
     private final boolean hasWaterBucket;
     private final boolean hasThrowaway;
     private final boolean canSprint;
+    private final double placeBlockCost;
+    private final boolean allowBreak;
 
     public CalculationContext() {
         this(new ToolSet());
@@ -44,9 +46,14 @@ public class CalculationContext implements Helper {
     public CalculationContext(ToolSet toolSet) {
         player().setSprinting(true);
         this.toolSet = toolSet;
-        this.hasWaterBucket = Baritone.settings().allowWaterBucketFall.get() && InventoryPlayer.isHotbar(player().inventory.getSlotFor(STACK_BUCKET_WATER)) && !world().provider.isNether();
         this.hasThrowaway = Baritone.settings().allowPlaceThrowaway.get() && MovementHelper.throwaway(false);
+        this.hasWaterBucket = Baritone.settings().allowWaterBucketFall.get() && InventoryPlayer.isHotbar(player().inventory.getSlotFor(STACK_BUCKET_WATER)) && !world().provider.isNether();
         this.canSprint = Baritone.settings().allowSprint.get() && player().getFoodStats().getFoodLevel() > 6;
+        this.placeBlockCost = Baritone.settings().blockPlacementPenalty.get();
+        this.allowBreak = Baritone.settings().allowBreak.get();
+        // why cache these things here, why not let the movements just get directly from settings?
+        // because if some movements are calculated one way and others are calculated another way,
+        // then you get a wildly inconsistent path that isn't optimal for either scenario.
     }
 
     public ToolSet getToolSet() {
@@ -64,4 +71,12 @@ public class CalculationContext implements Helper {
     public boolean canSprint() {
         return canSprint;
     }
+
+    public double placeBlockCost() {
+        return placeBlockCost;
+    }
+
+    public boolean allowBreak() {
+        return allowBreak;
+    }
 }

src/main/java/baritone/bot/pathing/movement/Movement.java

@@ -178,7 +178,7 @@ public abstract class Movement implements Helper, MovementHelper {
     }
 
 
-    public double getTotalHardnessOfBlocksToBreak(ToolSet ts) {
+    public double getTotalHardnessOfBlocksToBreak(CalculationContext ctx) {
         /*
         double sum = 0;
         HashSet<BlockPos> toBreak = new HashSet();
@@ -209,7 +209,7 @@ public abstract class Movement implements Helper, MovementHelper {
         //^ the above implementation properly deals with falling blocks, TODO integrate
         double sum = 0;
         for (BlockPos pos : positionsToBreak) {
-            sum += MovementHelper.getMiningDurationTicks(ts, pos);
+            sum += MovementHelper.getMiningDurationTicks(ctx, pos);
             if (sum >= COST_INF) {
                 return COST_INF;
             }

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

@@ -161,22 +161,22 @@ public interface MovementHelper extends ActionCosts, Helper {
         return BlockStateInterface.get(pos).getBlock() instanceof BlockFalling;
     }
 
-    static double getMiningDurationTicks(ToolSet ts, BlockPos position) {
+    static double getMiningDurationTicks(CalculationContext context, BlockPos position) {
         IBlockState state = BlockStateInterface.get(position);
-        return getMiningDurationTicks(ts, position, state);
+        return getMiningDurationTicks(context, position, state);
     }
 
-    static double getMiningDurationTicks(ToolSet ts, BlockPos position, IBlockState state) {
+    static double getMiningDurationTicks(CalculationContext context, BlockPos position, IBlockState state) {
         Block block = state.getBlock();
-        if (!block.equals(Blocks.AIR) && !canWalkThrough(position, state)) {
+        if (!block.equals(Blocks.AIR) && !canWalkThrough(position, state)) { // TODO is the air check really necessary? Isn't air canWalkThrough?
-            if (!Baritone.settings().allowBreak.get()) {
+            if (!context.allowBreak()) {
                 return COST_INF;
             }
             if (avoidBreaking(position, state)) {
                 return COST_INF;
             }
-            double m = Blocks.CRAFTING_TABLE.equals(block) ? 10 : 1;
+            double m = Blocks.CRAFTING_TABLE.equals(block) ? 10 : 1; // TODO see if this is still necessary. it's from MineBot when we wanted to penalize breaking its crafting table
-            return m / ts.getStrVsBlock(state, position);
+            return m / context.getToolSet().getStrVsBlock(state, position);
         }
         return 0;
     }
@@ -230,7 +230,12 @@ public interface MovementHelper extends ActionCosts, Helper {
         NonNullList<ItemStack> inv = p.inventory.mainInventory;
         for (byte i = 0; i < 9; i++) {
             ItemStack item = inv.get(i);
-            if (Baritone.settings().acceptableThrowAwayItems.get().contains(item.getItem())) {
+            // this usage of settings() is okay because it's only called once during pathing
+            // (while creating the CalculationContext at the very beginning)
+            // and then it's called during execution
+            // since this function is never called during cost calculation, we don't need to migrate
+            // acceptableThrowawayItems to the CalculationContext
+            if (Baritone.settings().acceptableThrowawayItems.get().contains(item.getItem())) {
                 if (select) {
                     p.inventory.currentItem = i;
                 }

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

@@ -77,7 +77,7 @@ public class MovementAscend extends Movement {
             }
             for (BlockPos against1 : against) {
                 if (BlockStateInterface.get(against1).isBlockNormalCube()) {
-                    return JUMP_ONE_BLOCK_COST + WALK_ONE_BLOCK_COST + PLACE_ONE_BLOCK_COST + getTotalHardnessOfBlocksToBreak(context.getToolSet());
+                    return JUMP_ONE_BLOCK_COST + WALK_ONE_BLOCK_COST + context.placeBlockCost() + getTotalHardnessOfBlocksToBreak(context);
                 }
             }
             return COST_INF;
@@ -109,7 +109,7 @@ public class MovementAscend extends Movement {
             walk *= WALK_ONE_OVER_SOUL_SAND_COST / WALK_ONE_BLOCK_COST;
         }
         // we hit space immediately on entering this action
-        return Math.max(JUMP_ONE_BLOCK_COST, walk) + getTotalHardnessOfBlocksToBreak(context.getToolSet());
+        return Math.max(JUMP_ONE_BLOCK_COST, walk) + getTotalHardnessOfBlocksToBreak(context);
     }
 
     @Override

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

@@ -51,7 +51,7 @@ public class MovementDescend extends Movement {
             // use this ratio to apply the soul sand speed penalty to our 0.8 block distance
             walk *= WALK_ONE_OVER_SOUL_SAND_COST / WALK_ONE_BLOCK_COST;
         }
-        return walk + Math.max(FALL_N_BLOCKS_COST[1], CENTER_AFTER_FALL_COST) + getTotalHardnessOfBlocksToBreak(context.getToolSet());
+        return walk + Math.max(FALL_N_BLOCKS_COST[1], CENTER_AFTER_FALL_COST) + getTotalHardnessOfBlocksToBreak(context);
     }
 
     int numTicks = 0;

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

@@ -93,8 +93,8 @@ public class MovementDiagonal extends Movement {
         if (BlockStateInterface.get(positionsToBreak[4].down()).getBlock() instanceof BlockMagma) {
             return COST_INF;
         }
-        double optionA = MovementHelper.getMiningDurationTicks(context.getToolSet(), positionsToBreak[0]) + MovementHelper.getMiningDurationTicks(context.getToolSet(), positionsToBreak[1]);
+        double optionA = MovementHelper.getMiningDurationTicks(context, positionsToBreak[0]) + MovementHelper.getMiningDurationTicks(context, positionsToBreak[1]);
-        double optionB = MovementHelper.getMiningDurationTicks(context.getToolSet(), positionsToBreak[2]) + MovementHelper.getMiningDurationTicks(context.getToolSet(), positionsToBreak[3]);
+        double optionB = MovementHelper.getMiningDurationTicks(context, positionsToBreak[2]) + MovementHelper.getMiningDurationTicks(context, positionsToBreak[3]);
         if (optionA != 0 && optionB != 0) {
             return COST_INF;
         }

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

@@ -47,7 +47,7 @@ public class MovementDownward extends Movement {
         if (ladder) {
             return LADDER_DOWN_ONE_COST;
         } else {
-            return FALL_N_BLOCKS_COST[1] + MovementHelper.getMiningDurationTicks(context.getToolSet(), dest, d);
+            return FALL_N_BLOCKS_COST[1] + MovementHelper.getMiningDurationTicks(context, dest, d);
         }
     }
 

src/main/java/baritone/bot/pathing/movement/movements/MovementFall.java

@@ -18,7 +18,10 @@
 package baritone.bot.pathing.movement.movements;
 
 import baritone.bot.behavior.impl.LookBehaviorUtils;
-import baritone.bot.pathing.movement.*;
+import baritone.bot.pathing.movement.CalculationContext;
+import baritone.bot.pathing.movement.Movement;
+import baritone.bot.pathing.movement.MovementHelper;
+import baritone.bot.pathing.movement.MovementState;
 import baritone.bot.pathing.movement.MovementState.MovementStatus;
 import baritone.bot.pathing.movement.MovementState.MovementTarget;
 import baritone.bot.utils.BlockStateInterface;
@@ -52,9 +55,9 @@ public class MovementFall extends Movement {
             if (!context.hasWaterBucket()) {
                 return COST_INF;
             }
-            placeBucketCost = ActionCosts.PLACE_ONE_BLOCK_COST;
+            placeBucketCost = context.placeBlockCost();
         }
-        double frontTwo = MovementHelper.getMiningDurationTicks(context.getToolSet(), positionsToBreak[0]) + MovementHelper.getMiningDurationTicks(context.getToolSet(), positionsToBreak[1]);
+        double frontTwo = MovementHelper.getMiningDurationTicks(context, positionsToBreak[0]) + MovementHelper.getMiningDurationTicks(context, positionsToBreak[1]);
         if (frontTwo >= COST_INF) {
             return COST_INF;
         }
@@ -64,7 +67,7 @@ public class MovementFall extends Movement {
             // lilypads (i think?) are 0 ticks to mine, but they definitely cause fall damage
             // same thing for falling through water... we can't actually do that
             // and falling through signs is possible, but they do have a mining duration, right?
-            if (MovementHelper.getMiningDurationTicks(context.getToolSet(), positionsToBreak[i]) > 0) {
+            if (MovementHelper.getMiningDurationTicks(context, positionsToBreak[i]) > 0) {
                 //can't break while falling
                 return COST_INF;
             }

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

@@ -56,7 +56,7 @@ public class MovementPillar extends Movement {
                 return COST_INF;
             }
         }
-        double hardness = getTotalHardnessOfBlocksToBreak(context.getToolSet());
+        double hardness = getTotalHardnessOfBlocksToBreak(context);
         if (hardness != 0) {
             Block tmp = BlockStateInterface.get(src.up(2)).getBlock();
             if (tmp instanceof BlockLadder || tmp instanceof BlockVine) {
@@ -78,7 +78,7 @@ public class MovementPillar extends Movement {
         if (ladder) {
             return LADDER_UP_ONE_COST + hardness;
         } else {
-            return JUMP_ONE_BLOCK_COST + PLACE_ONE_BLOCK_COST + hardness;
+            return JUMP_ONE_BLOCK_COST + context.placeBlockCost() + hardness;
         }
     }
 

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

@@ -94,7 +94,7 @@ public class MovementTraverse extends Movement {
             //double hardness1 = blocksToBreak[0].getBlockHardness(Minecraft.getMinecraft().world, positionsToBreak[0]);
             //double hardness2 = blocksToBreak[1].getBlockHardness(Minecraft.getMinecraft().world, positionsToBreak[1]);
             //Out.log("Can't walk through " + blocksToBreak[0] + " (hardness" + hardness1 + ") or " + blocksToBreak[1] + " (hardness " + hardness2 + ")");
-            return WC + getTotalHardnessOfBlocksToBreak(context.getToolSet());
+            return WC + getTotalHardnessOfBlocksToBreak(context);
         } else {//this is a bridge, so we need to place a block
             Block srcDown = BlockStateInterface.get(src.down()).getBlock();
             if (srcDown instanceof BlockLadder || srcDown instanceof BlockVine) {
@@ -108,14 +108,14 @@ public class MovementTraverse extends Movement {
                 double WC = BlockStateInterface.isWater(pb0.getBlock()) || BlockStateInterface.isWater(pb1.getBlock()) ? WALK_ONE_IN_WATER_COST : WALK_ONE_BLOCK_COST;
                 for (BlockPos against1 : against) {
                     if (BlockStateInterface.get(against1).isBlockNormalCube()) {
-                        return WC + PLACE_ONE_BLOCK_COST + getTotalHardnessOfBlocksToBreak(context.getToolSet());
+                        return WC + context.placeBlockCost() + getTotalHardnessOfBlocksToBreak(context);
                     }
                 }
                 if (BlockStateInterface.get(src).getBlock().equals(Blocks.SOUL_SAND)) {
                     return COST_INF; // can't sneak and backplace against soul sand =/
                 }
                 WC = WC * SNEAK_ONE_BLOCK_COST / WALK_ONE_BLOCK_COST;//since we are placing, we are sneaking
-                return WC + PLACE_ONE_BLOCK_COST + getTotalHardnessOfBlocksToBreak(context.getToolSet());
+                return WC + context.placeBlockCost() + getTotalHardnessOfBlocksToBreak(context);
             }
             return COST_INF;
             //Out.log("Can't walk on " + Baritone.get(positionsToPlace[0]).getBlock());