Open kwvanderlinde opened 10 months ago
kwvanderlinde
commented
10 months ago Something else that may be related is that we draw waypoints wherever a token is dragged back over its path, e.g., by dragging it right, setting a waypoint, then dragging back left:
The waypoint is only visible if the token has some transparency.
Edit: Actually this just looks like a pure rendering issue due to start and end points internally being treated as waypoints.
kwvanderlinde
commented
2 weeks ago I'm looking at addressing this via a refactoring of the zone walkers. It's hard to verify the correctness of the existing ZoneWalker in the presence of multithreading, because of how much it does and the differing behaviour in its various mutators.
What I'm hoping to do is reduce the ZoneWalker interface to the bare minimum needed to provide different pathfinding behaviour. It would explicitly not be safe to use concurrently, but could still be used with different threads. It would look something like:
public interface ZoneWalker {
/** Updates topology, terrain modifiers, and flags. */
void update( /* some state */ );
/** Calculates the partial path from start to end */
SomeResultType calculatePath(CellPoint start, CellPoint end);The update() method would always be called on the EDT, which is important since it sources data from the Zone. Meanwhile, calculatePath() can be run on a background thread if desired, provided it never runs concurrently with update() for the same walker.
The above interface deliberately doesn't understand waypoints or anything about how to manage them. That will fall to Selectionset and RenderPathWorker to figure out. RenderPathWorker will certainly have a list of waypoints it needs to get through, and will iteratively call ZoneWalker#update() until it is canceled or a complete path has been calculated.
Describe the Bug
If waypoints are being set quickly during a token drag, it is possible for the path to have multiple branches, for waypoints to not match the path, or for CMEs to the thrown and shown to the user. Any of these symptoms seem to be achievable without the others or in conjunction with the others.
Waypoint issues include missing waypoints and repeated waypoints when they should be unique.
To Reproduce
Expected Behaviour
Pathfinding should produce true paths (no branches) and should include all waypoints necessary to produce the path. Waypoints should not be duplicated within a path. Errors should not be produced.
Screenshots
Here is an example problem path found in this campaign:
There's a few weird things going on with this case. The path distances show a branch in the path when stepping out of cell (17, 8) (the step from distance=185 to distance=190). The actual list of cells steps normally all the way to the left at (-2, 8), but then jumps back to to (18, 8) (where distance = 180) to continue down the second branch. A waypoint was set at (-2, 8), but that waypoint was either lost or toggled back off (I'm not sure). Another oddity is that waypoint (14, 6) is repeated at two different points in the path.
MapTool Info
1.13.2, develop
Desktop
Linux 21.2
Additional Context
Here is an example CME as generated on latest develop (efc31d943):
java.util.ConcurrentModificationException: null at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1095) ~[?:?] at java.base/java.util.ArrayList$Itr.next(ArrayList.java:1049) ~[?:?] at net.rptools.maptool.client.walker.AbstractZoneWalker.removeWaypoint(AbstractZoneWalker.java:182) ~[main/:?] at net.rptools.maptool.client.walker.AbstractZoneWalker.toggleWaypoint(AbstractZoneWalker.java:203) ~[main/:?] at net.rptools.maptool.client.ui.zone.renderer.SelectionSet.toggleWaypoint(SelectionSet.java:169) ~[main/:?] at net.rptools.maptool.client.ui.zone.renderer.ZoneRenderer.toggleMoveSelectionSetWaypoint(ZoneRenderer.java:358) ~[main/:?] at net.rptools.maptool.client.tool.PointerTool.setWaypoint(PointerTool.java:1454) ~[main/:?] at net.rptools.maptool.client.tool.PointerTool$PointerActionListener.actionPerformed(PointerTool.java:1475) ~[main/:?] at java.desktop/javax.swing.SwingUtilities.notifyAction(SwingUtilities.java:1810) ~[?:?] at java.desktop/javax.swing.JComponent.processKeyBinding(JComponent.java:2956) ~[?:?] at java.desktop/javax.swing.JComponent.processKeyBindings(JComponent.java:3018) ~[?:?] at java.desktop/javax.swing.JComponent.processKeyEvent(JComponent.java:2918) ~[?:?] at java.desktop/java.awt.Component.processEvent(Component.java:6398) ~[?:?] at java.desktop/java.awt.Container.processEvent(Container.java:2266) ~[?:?] at java.desktop/java.awt.Component.dispatchEventImpl(Component.java:4996) ~[?:?] at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2324) ~[?:?] at java.desktop/java.awt.Component.dispatchEvent(Component.java:4828) ~[?:?] at java.desktop/java.awt.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1952) ~[?:?] at java.desktop/java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(DefaultKeyboardFocusManager.java:883) ~[?:?] at java.desktop/java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:1146) ~[?:?] at java.desktop/java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:1020) ~[?:?] at java.desktop/java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:848) ~[?:?] at java.desktop/java.awt.Component.dispatchEventImpl(Component.java:4877) ~[?:?] at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2324) ~[?:?] at java.desktop/java.awt.Window.dispatchEventImpl(Window.java:2780) ~[?:?] at java.desktop/java.awt.Component.dispatchEvent(Component.java:4828) ~[?:?] at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:775) ~[?:?] at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:720) ~[?:?] at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:714) ~[?:?] at java.base/java.security.AccessController.doPrivileged(AccessController.java:400) ~[?:?] at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:87) ~[?:?] at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:98) ~[?:?] at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:747) ~[?:?] at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:745) ~[?:?] at java.base/java.security.AccessController.doPrivileged(AccessController.java:400) ~[?:?] at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:87) ~[?:?] at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:744) ~[?:?] at net.rptools.maptool.client.swing.MapToolEventQueue.dispatchEvent(MapToolEventQueue.java:54) [main/:?] at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203) [?:?] at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124) [?:?] at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113) [?:?] at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109) [?:?] at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101) [?:?] at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90) [?:?]