Removed strategy brainfuck and turned it into a more proper solution.

Long story short: PathingRuleSet nolonger maintains the PathfinderStrategy. It is now passed via the Pathfinder.findPath method. With this change we grant the Pathfinder and -strategies more flexibility
Metaphoriker · Oct 5, 2023 · d6b2d4c · d6b2d4c
1 parent 6dcb31e
commit d6b2d4c
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 33 deletions.
diff --git a/pathetic-api/src/main/java/org/patheloper/api/pathing/Pathfinder.java b/pathetic-api/src/main/java/org/patheloper/api/pathing/Pathfinder.java
@@ -2,6 +2,7 @@
 
 import lombok.NonNull;
 import org.patheloper.api.pathing.result.PathfinderResult;
+import org.patheloper.api.pathing.strategy.PathfinderStrategy;
 import org.patheloper.api.wrapper.PathPosition;
 
 import java.util.concurrent.CompletionStage;
@@ -12,10 +13,24 @@
 public interface Pathfinder {
 
     /**
-     * Tries to find a Path between the two {@link PathPosition}'s provided.
+     * Tries to find a Path between the two {@link PathPosition}'s provided directly.
+     *
+     * @see org.patheloper.api.pathing.strategy.strategies.DirectPathfinderStrategy
      *
      * @return An {@link CompletionStage} that will contain a {@link PathfinderResult}.
+     *
+     * @deprecated Use {@link #findPath(PathPosition, PathPosition, PathfinderStrategy)} instead.
      */
     @NonNull
+    @Deprecated
     CompletionStage<PathfinderResult> findPath(@NonNull PathPosition start, @NonNull PathPosition target);
+
+    /**
+     * Tries to find a Path between the two {@link PathPosition}'s provided with the given strategy.
+     *
+     * @param strategy The {@link PathfinderStrategy} to use
+     * @return An {@link CompletionStage} that will contain a {@link PathfinderResult}.
+     */
+    @NonNull
+    CompletionStage<PathfinderResult> findPath(@NonNull PathPosition start, @NonNull PathPosition target, @NonNull PathfinderStrategy strategy);
 }
diff --git a/pathetic-api/src/main/java/org/patheloper/api/pathing/rules/PathingRuleSet.java b/pathetic-api/src/main/java/org/patheloper/api/pathing/rules/PathingRuleSet.java
@@ -6,16 +6,12 @@
 import lombok.RequiredArgsConstructor;
 import lombok.Value;
 import lombok.With;
-import org.patheloper.api.pathing.strategy.strategies.DirectPathfinderStrategy;
-import org.patheloper.api.pathing.strategy.PathfinderStrategy;
 
 /**
  * Configuration options for pathfinding.
  *
  * This class defines a set of rules that guide the behavior of the pathfinding process.
  *
- * - `strategy`: The class of the strategy to use for pathfinding. Defaults to {@link DirectPathfinderStrategy}.
- *
  * - `maxIterations`: The maximum number of iterations allowed during pathfinding. Set this to prevent infinite loops.
  *
  * - `maxLength`: The maximum length of the path. Avoid setting this too high as it can cause performance issues.
@@ -43,8 +39,6 @@
 @RequiredArgsConstructor(access = AccessLevel.PRIVATE)
 public class PathingRuleSet {
 
-    private static final Class<? extends PathfinderStrategy> DEFAULT_STRATEGY = DirectPathfinderStrategy.class;
-
     /**
      * @return A new {@link PathingRuleSet} with default values but async.
      */
@@ -59,8 +53,6 @@ public static PathingRuleSet createRuleSet() {
         return builder().build();
     }
 
-    @Builder.Default
-    Class<? extends PathfinderStrategy> strategy = DEFAULT_STRATEGY;
     @Builder.Default
     int maxIterations = 5000; // to avoid freewheeling
     int maxLength;

diff --git a/pathetic-example/src/main/java/org/patheloper/example/PatheticPlugin.java b/pathetic-example/src/main/java/org/patheloper/example/PatheticPlugin.java
@@ -17,7 +17,6 @@ public void onEnable() {
 
         // Then you can use the PatheticMapper to get your own Pathfinder instance with your own set rules.
         Pathfinder reusablePathfinder = PatheticMapper.newPathfinder(PathingRuleSet.createAsyncRuleSet()
-                .withStrategy(DirectPathfinderStrategy.class)
                 .withAllowingDiagonal(true)
                 .withAllowingFailFast(true)
                 .withAllowingFallback(true)

diff --git a/pathetic-example/src/main/java/org/patheloper/example/command/PatheticCommand.java b/pathetic-example/src/main/java/org/patheloper/example/command/PatheticCommand.java
@@ -8,6 +8,7 @@
 import org.bukkit.entity.Player;
 import org.patheloper.api.pathing.Pathfinder;
 import org.patheloper.api.pathing.result.PathfinderResult;
+import org.patheloper.api.pathing.strategy.strategies.WalkablePathfinderStrategy;
 import org.patheloper.api.wrapper.PathPosition;
 import org.patheloper.mapping.bukkit.BukkitMapper;
 
@@ -66,7 +67,8 @@ public boolean onCommand(CommandSender sender, Command command, String label, St
                 PathPosition target = BukkitMapper.toPathPosition(playerSession.getPos2());
 
                 player.sendMessage("Starting pathfinding...");
-                CompletionStage<PathfinderResult> pathfindingResult = pathfinder.findPath(start, target); // This is the actual pathfinding.
+                CompletionStage<PathfinderResult> pathfindingResult =
+                        pathfinder.findPath(start, target, new WalkablePathfinderStrategy()); // This is the actual pathfinding.
 
                 // This is just a simple way to display the pathfinding result.
                 pathfindingResult.thenAccept(result -> {

diff --git a/pathetic-model/src/main/java/org/patheloper/model/pathing/pathfinder/AStarPathfinder.java b/pathetic-model/src/main/java/org/patheloper/model/pathing/pathfinder/AStarPathfinder.java
@@ -28,7 +28,7 @@ public AStarPathfinder(PathingRuleSet pathingRuleSet) {
     }
 
     @Override
-    protected PathfinderResult findPath(PathPosition start, PathPosition target, PathfinderStrategy strategy) {
+    protected PathfinderResult resolvePath(PathPosition start, PathPosition target, PathfinderStrategy strategy) {
         Node startNode = new Node(start.floor(), start.floor(), target.floor(), 0);
 
         PriorityQueue<Node> nodeQueue = new PriorityQueue<>(Collections.singleton(startNode));
@@ -114,7 +114,7 @@ private Optional<PathfinderResult> fallback(Node fallbackNode) {
 
     private Optional<PathfinderResult> counterCheck(PathPosition start, PathPosition target, PathfinderStrategy strategy) {
         AStarPathfinder aStarPathfinder = new AStarPathfinder(pathingRuleSet.withCounterCheck(false));
-        PathfinderResult pathfinderResult = aStarPathfinder.findPath(target, start, strategy);
+        PathfinderResult pathfinderResult = aStarPathfinder.resolvePath(target, start, strategy);
 
         if(pathfinderResult.getPathState() == PathState.FOUND)
             return Optional.of(pathfinderResult);

diff --git a/pathetic-model/src/main/java/org/patheloper/model/pathing/pathfinder/AbstractPathfinder.java b/pathetic-model/src/main/java/org/patheloper/model/pathing/pathfinder/AbstractPathfinder.java
@@ -10,6 +10,7 @@
 import org.patheloper.api.pathing.result.PathfinderResult;
 import org.patheloper.api.pathing.rules.PathingRuleSet;
 import org.patheloper.api.pathing.strategy.PathfinderStrategy;
+import org.patheloper.api.pathing.strategy.strategies.DirectPathfinderStrategy;
 import org.patheloper.api.snapshot.SnapshotManager;
 import org.patheloper.api.wrapper.PathBlock;
 import org.patheloper.api.wrapper.PathPosition;
@@ -21,7 +22,6 @@
 import org.patheloper.util.ErrorLogger;
 import org.patheloper.util.NodeUtil;
 
-import java.lang.reflect.InvocationTargetException;
 import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.Set;
@@ -37,6 +37,8 @@ abstract class AbstractPathfinder implements Pathfinder {
     protected static final Set<PathPosition> EMPTY_LINKED_HASHSET =
             Collections.unmodifiableSet(new LinkedHashSet<>(0));
 
+    private static final PathfinderStrategy DEFAULT_STRATEGY = new DirectPathfinderStrategy();
+
     private static final SnapshotManager SIMPLE_SNAPSHOT_MANAGER = new FailingSnapshotManager();
     private static final SnapshotManager LOADING_SNAPSHOT_MANAGER =
             new FailingSnapshotManager.RequestingSnapshotManager();
@@ -56,6 +58,7 @@ abstract class AbstractPathfinder implements Pathfinder {
             );
 
     protected final PathingRuleSet pathingRuleSet;
+
     protected final Offset offset;
     protected final SnapshotManager snapshotManager;
 
@@ -69,17 +72,23 @@ protected AbstractPathfinder(PathingRuleSet pathingRuleSet) {
     @Override
     public @NonNull CompletionStage<PathfinderResult> findPath(@NonNull PathPosition start,
                                                                @NonNull PathPosition target) {
-        PathfinderStrategy strategy = instantiateStrategy();
+        return findPath(start, target, DEFAULT_STRATEGY);
+    }
+
+    @Override
+    public @NonNull CompletionStage<PathfinderResult> findPath(@NonNull PathPosition start,
+                                                               @NonNull PathPosition target,
+                                                               @NonNull PathfinderStrategy strategy) {
         PathingStartFindEvent startEvent = raiseStart(start, target, strategy);
-
+        
         if(initialChecksFailed(start, target, startEvent))
             return CompletableFuture.completedFuture(finishPathing(new PathfinderResultImpl(
                     PathState.FAILED,
                     new PathImpl(start, target, EMPTY_LINKED_HASHSET))));
-
+        
         return producePathing(start, target, strategy);
     }
-
+    
     protected PathfinderResult finishPathing(PathfinderResult pathfinderResult) {
         EventPublisher.raiseEvent(new PathingFinishedEvent(pathfinderResult));
         return pathfinderResult;
@@ -124,17 +133,6 @@ private boolean isBlockUnreachable(PathPosition position) {
         return true;
     }
 
-    private PathfinderStrategy instantiateStrategy() {
-        try {
-            return pathingRuleSet.getStrategy().getDeclaredConstructor().newInstance();
-        } catch (InstantiationException |
-                 IllegalAccessException |
-                 InvocationTargetException |
-                 NoSuchMethodException e) {
-            throw ErrorLogger.logFatalError("Failed to instantiate PathfinderStrategy. Fell back to default");
-        }
-    }
-
     private PathPosition relocateTargetPosition(PathPosition target) {
         if (pathingRuleSet.isAllowingAlternateTarget() && isBlockUnreachable(target))
             return NodeUtil.bubbleSearchAlternative(target, offset, snapshotManager).getPathPosition();
@@ -152,7 +150,7 @@ private CompletionStage<PathfinderResult> producePathing(PathPosition start, Pat
     private CompletionStage<PathfinderResult> produceAsyncPathing(PathPosition start, PathPosition target, PathfinderStrategy strategy) {
         return CompletableFuture.supplyAsync(() -> {
             try {
-                return findPath(start, relocateTargetPosition(target), strategy);
+                return resolvePath(start, relocateTargetPosition(target), strategy);
             } catch (Exception e) {
                 throw ErrorLogger.logFatalError("Failed to find path async");
             }
@@ -161,11 +159,12 @@ private CompletionStage<PathfinderResult> produceAsyncPathing(PathPosition start
 
     private CompletionStage<PathfinderResult> produceSyncPathing(PathPosition start, PathPosition target, PathfinderStrategy strategy) {
         try {
-            return CompletableFuture.completedFuture(findPath(start, relocateTargetPosition(target), strategy));
+            return CompletableFuture.completedFuture(resolvePath(start, relocateTargetPosition(target), strategy));
         } catch (Exception e) {
             throw ErrorLogger.logFatalError("Failed to find path sync");
         }
     }
-
-    protected abstract PathfinderResult findPath(PathPosition start, PathPosition target, PathfinderStrategy strategy);
+
+    // name clash with the interface, therefore "resolve" instead of "find"
+    protected abstract PathfinderResult resolvePath(PathPosition start, PathPosition target, PathfinderStrategy strategy);
 }