Fix #2501 with a special check for repeated nodes. Bonus documentation of ReplaceNode...
authorSteve Bennett <stevagewp@gmail.com>
Sat, 5 May 2012 16:49:10 +0000 (02:49 +1000)
committerSteve Bennett <stevagewp@gmail.com>
Sat, 5 May 2012 16:49:10 +0000 (02:49 +1000)
net/systemeD/halcyon/connection/Node.as
net/systemeD/halcyon/connection/Way.as
net/systemeD/halcyon/connection/actions/ReplaceNodeWithNewAction.as
net/systemeD/potlatch2/controller/SelectedWayNode.as

index 16c6a63..29aa126 100644 (file)
@@ -138,8 +138,10 @@ package net.systemeD.halcyon.connection {
                }
                
         /** Merge another node into this one, removing the other one. */
-        public function mergeWith(node:Node, performAction:Function):void {
-            performAction(new MergeNodesAction(this, node));
+        public function mergeWith(node:Node, performAction:Function):MergeNodesAction {
+            var mna:MergeNodesAction = new MergeNodesAction(this, node);
+            performAction(mna);
+            return mna; // Access to the action is useful for stacking more actions onto it.
         }
                
     }
index 408a831..0cca312 100644 (file)
@@ -162,6 +162,16 @@ package net.systemeD.halcyon.connection {
         public function reverseNodes(performAction:Function):void {
             performAction(new ReverseNodesAction(this, nodes));
         }
+        
+        /** Check for, and remove, consecutive series of the same node */ 
+        public function removeRepeatedNodes(performAction:Function):void {
+               var n: Node = nodes[0];
+               for (var i:int = 1; i < nodes.length; i++) {
+                       if (nodes[i] == nodes[i-1]) {
+                               removeNodeByIndex(i, performAction);
+                       }
+               }
+        } 
 
                
                /** Is a point within this way?
index 86c8e32..a330903 100644 (file)
@@ -2,7 +2,7 @@ package net.systemeD.halcyon.connection.actions {
 
     import net.systemeD.halcyon.connection.*;
 
-    /** Action that substitutes one node instead of another, in all the ways and relations that that node is part of. */
+    /** Action that creates a new node, then replaces an existing one with the new one in all the ways and relations that that node was part of. */
     public class ReplaceNodeWithNewAction extends ReplaceNodeAction {
 
         private var connection:Connection;
@@ -12,7 +12,7 @@ package net.systemeD.halcyon.connection.actions {
 
         /**
         * @param node The node we're getting rid of
-        * @param replacement The node we want to end up with
+        * @param connection, lat, lon, tags: Properties to define the new node.
         */
         public function ReplaceNodeWithNewAction(node:Node, connection:Connection, lat:Number, lon:Number, tags:Object) {
                        super(node,null);
@@ -22,14 +22,11 @@ package net.systemeD.halcyon.connection.actions {
             this.tags = tags;
         }
 
+        /** Create new node, then as for ReplaceNodeAction.doAction() */
         public override function doAction():uint {
             replacement = connection.createNode(tags,lat,lon,push);
                        return super.doAction();
         }
-
-        public override function undoAction():uint {
-            return super.undoAction();
-        }
     }
 }
 
index 8d234b2..0fe9122 100644 (file)
@@ -235,7 +235,14 @@ package net.systemeD.potlatch2.controller {
         }
         
         private function doMergeNodes(n:Node): ControllerState {
-               n.mergeWith(Node(firstSelected), MainUndoStack.getGlobalStack().addAction);
+               var nways:Array = n.parentWays.concat(Node(firstSelected).parentWays);
+               var mna:MergeNodesAction = n.mergeWith(Node(firstSelected), MainUndoStack.getGlobalStack().addAction);
+            /* Duplicated consecutive nodes happen if the two merged nodes are consecutive nodes of a (different) way */
+            for each (var w:Way in nways) {
+               // If there's a node to remove, jam that action into the existing MergeNodesAction. 
+               w.removeRepeatedNodes(function (a:UndoableAction):void { a.doAction(); mna.push(a); } );
+            }
+               
             // only merge one node at a time - too confusing otherwise?
             var msg:String = "Nodes merged"
             if (MergeNodesAction.lastTagsMerged) msg += ": check conflicting tags";