From 3074ac37e1d05541add0cf777d1183581754e112 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Fri, 6 Dec 2024 14:10:50 -0800 Subject: [PATCH 1/2] Simplify SelectionSet case work The presence of the walker is used to indicated whether we are currently using pathfinding and snap-to-grid behaviour. We no longer check that at every opportunity. Also, the task requires a non-null walker, which was not checked. This enables us to easily check against a null walker when it matters. --- .../client/ui/zone/RenderPathWorker.java | 4 +- .../client/ui/zone/renderer/SelectionSet.java | 49 ++++++++++--------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/RenderPathWorker.java b/src/main/java/net/rptools/maptool/client/ui/zone/RenderPathWorker.java index 9c90c84e1c..d6156e9a8a 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/RenderPathWorker.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/RenderPathWorker.java @@ -27,14 +27,14 @@ public class RenderPathWorker extends SwingWorker { // private static final Logger log = LogManager.getLogger(RenderPathWorker.class); ZoneRenderer zoneRenderer; - ZoneWalker walker; + @Nonnull ZoneWalker walker; CellPoint startPoint, endPoint; private final boolean restrictMovement; private final Set terrainModifiersIgnored; private final @Nonnull Token keyToken; public RenderPathWorker( - ZoneWalker walker, + @Nonnull ZoneWalker walker, CellPoint endPoint, boolean restrictMovement, Set terrainModifiersIgnored, diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/renderer/SelectionSet.java b/src/main/java/net/rptools/maptool/client/ui/zone/renderer/SelectionSet.java index a55c57dfda..e50a115b4b 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/renderer/SelectionSet.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/renderer/SelectionSet.java @@ -20,6 +20,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import net.rptools.maptool.client.MapTool; import net.rptools.maptool.client.ui.zone.RenderPathWorker; import net.rptools.maptool.client.walker.ZoneWalker; @@ -36,7 +37,7 @@ public class SelectionSet { private final Set selectionSet = new HashSet(); private final GUID keyToken; private final String playerId; - private ZoneWalker walker; + private @Nullable ZoneWalker walker; private final Token token; private Path gridlessPath; @@ -114,29 +115,32 @@ public boolean contains(Token token) { /** Aborts the movement for this selection. */ public void cancel() { - walker.close(); + if (walker != null) { + walker.close(); - renderPathTask.cancel(true); + if (renderPathTask != null) { + renderPathTask.cancel(true); + } + } } // This is called when movement is committed/done. It'll let the last thread either finish or // timeout public void renderFinalPath() { - walker.close(); - - if (renderer.zone.getGrid().getCapabilities().isPathingSupported() - && token.isSnapToGrid() - && renderPathTask != null) { - - log.trace("Waiting on Path Rendering... "); - while (true) { - try { - renderPathTask.get(); - break; - } catch (InterruptedException e) { - } catch (ExecutionException e) { - log.error("Error while waiting for task to finish", e); - break; + if (walker != null) { + walker.close(); + + if (renderPathTask != null) { + log.trace("Waiting on Path Rendering... "); + while (true) { + try { + renderPathTask.get(); + break; + } catch (InterruptedException e) { + } catch (ExecutionException e) { + log.error("Error while waiting for task to finish", e); + break; + } } } } @@ -146,7 +150,7 @@ public void update(ZonePoint newAnchorPosition) { currentPoint.x = newAnchorPosition.x; currentPoint.y = newAnchorPosition.y; - if (renderer.zone.getGrid().getCapabilities().isPathingSupported() && token.isSnapToGrid()) { + if (walker != null) { CellPoint point = renderer.zone.getGrid().convert(currentPoint); // walker.replaceLastWaypoint(point, restrictMovement); // OLD WAY @@ -174,7 +178,7 @@ public void update(ZonePoint newAnchorPosition) { * @param location The point where the waypoint is toggled. */ public void toggleWaypoint(ZonePoint location) { - if (walker != null && token.isSnapToGrid() && renderer.getZone().getGrid() != null) { + if (walker != null) { walker.toggleWaypoint(renderer.getZone().getGrid().convert(location)); } else { gridlessPath.appendWaypoint(location); @@ -189,14 +193,11 @@ public void toggleWaypoint(ZonePoint location) { */ public ZonePoint getLastWaypoint() { ZonePoint zp; - if (walker != null && token.isSnapToGrid() && renderer.getZone().getGrid() != null) { + if (walker != null) { CellPoint cp = walker.getLastPoint(); if (cp == null) { - // log.info("cellpoint is null! FIXME! You have Walker class updating outside of - // thread..."); // Why not save last waypoint to this class? cp = renderer.zone.getGrid().convert(token.getDragAnchor(renderer.zone)); - // log.info("So I set it to: " + cp); } zp = renderer.getZone().getGrid().convert(cp); From fdc7b5634254096dbaf0cbd528891db2c32f0ee9 Mon Sep 17 00:00:00 2001 From: Kenneth VanderLinde Date: Sun, 8 Dec 2024 11:05:53 -0800 Subject: [PATCH 2/2] Deliberate be less reactive to grid changes --- .../client/ui/zone/renderer/SelectionSet.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/net/rptools/maptool/client/ui/zone/renderer/SelectionSet.java b/src/main/java/net/rptools/maptool/client/ui/zone/renderer/SelectionSet.java index e50a115b4b..234dfcf984 100644 --- a/src/main/java/net/rptools/maptool/client/ui/zone/renderer/SelectionSet.java +++ b/src/main/java/net/rptools/maptool/client/ui/zone/renderer/SelectionSet.java @@ -32,6 +32,7 @@ public class SelectionSet { private final ZoneRenderer renderer; + private final Grid grid; private final Logger log = LogManager.getLogger(SelectionSet.class); private final Set selectionSet = new HashSet(); @@ -70,12 +71,13 @@ public SelectionSet( startPoint = new ZonePoint(anchorPoint); currentPoint = new ZonePoint(anchorPoint); - if (token.isSnapToGrid() && renderer.zone.getGrid().getCapabilities().isSnapToGridSupported()) { - if (renderer.zone.getGrid().getCapabilities().isPathingSupported()) { - CellPoint tokenPoint = renderer.zone.getGrid().convert(currentPoint); + grid = renderer.getZone().getGrid(); + if (token.isSnapToGrid() && grid.getCapabilities().isSnapToGridSupported()) { + if (grid.getCapabilities().isPathingSupported()) { + CellPoint tokenPoint = grid.convert(currentPoint); - walker = renderer.zone.getGrid().createZoneWalker(); - walker.setFootprint(token.getFootprint(renderer.zone.getGrid())); + walker = grid.createZoneWalker(); + walker.setFootprint(token.getFootprint(grid)); walker.setWaypoints(tokenPoint, tokenPoint); } } else { @@ -151,7 +153,7 @@ public void update(ZonePoint newAnchorPosition) { currentPoint.y = newAnchorPosition.y; if (walker != null) { - CellPoint point = renderer.zone.getGrid().convert(currentPoint); + CellPoint point = grid.convert(currentPoint); // walker.replaceLastWaypoint(point, restrictMovement); // OLD WAY // New way threaded, off the swing UI thread... @@ -179,7 +181,7 @@ public void update(ZonePoint newAnchorPosition) { */ public void toggleWaypoint(ZonePoint location) { if (walker != null) { - walker.toggleWaypoint(renderer.getZone().getGrid().convert(location)); + walker.toggleWaypoint(grid.convert(location)); } else { gridlessPath.appendWaypoint(location); } @@ -197,10 +199,10 @@ public ZonePoint getLastWaypoint() { CellPoint cp = walker.getLastPoint(); if (cp == null) { - cp = renderer.zone.getGrid().convert(token.getDragAnchor(renderer.zone)); + cp = grid.convert(token.getDragAnchor(renderer.zone)); } - zp = renderer.getZone().getGrid().convert(cp); + zp = grid.convert(cp); } else { // Gridless path will never be empty if set. zp = gridlessPath.getWayPointList().getLast();