From 55348575fb8b3d134e6b8953b22fbe4f9177fd48 Mon Sep 17 00:00:00 2001 From: Steve Bennett Date: Sat, 3 Mar 2012 20:01:00 +1100 Subject: [PATCH 1/3] One attempt. Undo sort of works; redo doesn't. Defines BeginWayAction as including node creation and way creation. When the second last node gets undone, the BWA gets split apart and the node creation is preserved, but repackaged as a CreatePOIAction. Ugh. --- .../connection/CompositeUndoableAction.as | 4 ++ .../halcyon/connection/MainUndoStack.as | 9 +++- .../connection/actions/AddNodeToWayAction.as | 17 +++++++- .../connection/actions/BeginWayAction.as | 41 ++++++++++++++++++- .../connection/actions/CreatePOIAction.as | 7 +++- net/systemeD/potlatch2/controller/DrawWay.as | 1 + .../potlatch2/controller/NoSelection.as | 13 ++++-- 7 files changed, 82 insertions(+), 10 deletions(-) diff --git a/net/systemeD/halcyon/connection/CompositeUndoableAction.as b/net/systemeD/halcyon/connection/CompositeUndoableAction.as index ffec7dc0..4979dbf5 100644 --- a/net/systemeD/halcyon/connection/CompositeUndoableAction.as +++ b/net/systemeD/halcyon/connection/CompositeUndoableAction.as @@ -105,6 +105,10 @@ package net.systemeD.halcyon.connection { var str:String = " {" + actions.join(",") + "}"; return name + str; } + + protected function getActions():Array { + return actions; + } } } diff --git a/net/systemeD/halcyon/connection/MainUndoStack.as b/net/systemeD/halcyon/connection/MainUndoStack.as index 38f87ff0..dd4c299e 100644 --- a/net/systemeD/halcyon/connection/MainUndoStack.as +++ b/net/systemeD/halcyon/connection/MainUndoStack.as @@ -104,10 +104,15 @@ package net.systemeD.halcyon.connection { } } - public function removeLastIfAction(action:Class):void { + /** + * Remove (without undoing) the most recent action, but only if it's a particular class + * @param action The class of the previous action. + */ + public function removeLastIfAction(action:Class):UndoableAction { if (undoActions.length && undoActions[undoActions.length-1] is action) { - undoActions.pop(); + return undoActions.pop() as UndoableAction; } + return null; } [Bindable(event="new_undo_item")] diff --git a/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as b/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as index 266af855..85bc8c29 100644 --- a/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as +++ b/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as @@ -63,8 +63,21 @@ package net.systemeD.halcyon.connection.actions { way.dispatchEvent(new EntityEvent(Connection.WAY_DELETED, way)); firstNode=way.getNode(0); firstNode.removeParent(way); - if (!firstNode.hasParentWays) firstNode.connection.registerPOI(firstNode); - MainUndoStack.getGlobalStack().removeLastIfAction(BeginWayAction); + var bwa: BeginWayAction; + bwa = MainUndoStack.getGlobalStack().removeLastIfAction(BeginWayAction) as BeginWayAction; + if (!firstNode.hasParentWays) { + var cpa: CreatePOIAction = new CreatePOIAction( + way.connection, + firstNode.getTagsCopy()/*getTagsHash?*/, + firstNode.lat, + firstNode.lon, + firstNode, + bwa.getNodeCreation() + ); + //firstNode.connection.registerPOI(firstNode); + MainUndoStack.getGlobalStack().addAction(cpa); + + } } return SUCCESS; } diff --git a/net/systemeD/halcyon/connection/actions/BeginWayAction.as b/net/systemeD/halcyon/connection/actions/BeginWayAction.as index 293e7443..79657514 100644 --- a/net/systemeD/halcyon/connection/actions/BeginWayAction.as +++ b/net/systemeD/halcyon/connection/actions/BeginWayAction.as @@ -5,9 +5,48 @@ package net.systemeD.halcyon.connection.actions { /* This is needed so that the specific type of CUA can be detected when CreatePOIAction is called */ public class BeginWayAction extends CompositeUndoableAction { - public function BeginWayAction(){ + private var newNode:Node; + private var newWay:Way; + private var lat:Number; + private var lon:Number; + private var connection:Connection; + + public function BeginWayAction(connection:Connection, lat:Number, lon:Number){ super("Begin Way Action"); + this.connection = connection; + this.lat = lat; + this.lon = lon; + } + + public override function doAction():uint { + if (newNode == null) { + newNode = connection.createNode({}, lat, lon, push); + newWay = connection.createWay({}, [newNode], push); + } + super.doAction(); + + return SUCCESS; + } + + public override function undoAction():uint { + super.undoAction(); + // this is needed because AddNodeToWayAction turns the first node into a POI when undoing. Hrm... + connection.unregisterPOI(newNode); + + return SUCCESS; + } + + public function getNode():Node { + return newNode; } + public function getWay():Way { + return newWay; + } + + public function getNodeCreation():CreateEntityAction { + return this.getActions()[0]; + } + } } \ No newline at end of file diff --git a/net/systemeD/halcyon/connection/actions/CreatePOIAction.as b/net/systemeD/halcyon/connection/actions/CreatePOIAction.as index 42a35fbf..b3630332 100644 --- a/net/systemeD/halcyon/connection/actions/CreatePOIAction.as +++ b/net/systemeD/halcyon/connection/actions/CreatePOIAction.as @@ -12,13 +12,18 @@ package net.systemeD.halcyon.connection.actions { private var lon:Number; private var connection:Connection; - public function CreatePOIAction(connection:Connection, tags:Object, lat:Number, lon:Number) { + public function CreatePOIAction(connection:Connection, tags:Object, lat:Number, lon:Number, node: Node=null, nodeCreation: CreateEntityAction=null) { super("Create POI"); this.connection = connection; this.tags = tags; this.lat = lat; this.lon = lon; + if (node) { + this.newNode = node; + push(nodeCreation); + } } + public override function doAction():uint { if (newNode == null) { diff --git a/net/systemeD/potlatch2/controller/DrawWay.as b/net/systemeD/potlatch2/controller/DrawWay.as index 0101db37..dea2959b 100644 --- a/net/systemeD/potlatch2/controller/DrawWay.as +++ b/net/systemeD/potlatch2/controller/DrawWay.as @@ -200,6 +200,7 @@ package net.systemeD.potlatch2.controller { protected function keyExitDrawing():ControllerState { var cs:ControllerState=stopDrawing(); + // If they've only drawn one node, discard it and the attached way with no remorse. if (selectedWay.length==1) { if (MainUndoStack.getGlobalStack().undoIfAction(BeginWayAction)) { return new NoSelection(); diff --git a/net/systemeD/potlatch2/controller/NoSelection.as b/net/systemeD/potlatch2/controller/NoSelection.as index 873ddb8a..59de43ca 100644 --- a/net/systemeD/potlatch2/controller/NoSelection.as +++ b/net/systemeD/potlatch2/controller/NoSelection.as @@ -26,15 +26,20 @@ package net.systemeD.potlatch2.controller { if (event.type==MouseEvent.MOUSE_UP && (focus==null || (paint && paint.isBackground)) && map.dragstate!=map.DRAGGING && map.dragstate!=map.SWALLOW_MOUSEUP) { map.dragstate=map.NOT_DRAGGING; // ** FIXME: BeginWayAction ought to be a discrete class - var undo:CompositeUndoableAction = new BeginWayAction(); + /*var undo:CompositeUndoableAction = new BeginWayAction(); var conn:Connection = layer.connection; var startNode:Node = conn.createNode( {}, controller.map.coord2lat(event.localY), controller.map.coord2lon(event.localX), undo.push); - var way:Way = conn.createWay({}, [startNode], undo.push); - MainUndoStack.getGlobalStack().addAction(undo); - return new DrawWay(way, true, false); + var way:Way = conn.createWay({}, [startNode], undo.push);*/ + var undo:BeginWayAction = new BeginWayAction( + layer.connection, + controller.map.coord2lat(event.localY), + controller.map.coord2lon(event.localX)); + MainUndoStack.getGlobalStack().addAction(undo); + + return new DrawWay(undo.getWay(), true, false); } return this; } From 2373a60b0323753c92d3a1f89d5e000880da1419 Mon Sep 17 00:00:00 2001 From: Steve Bennett Date: Sun, 4 Mar 2012 00:46:28 +1100 Subject: [PATCH 2/3] Working fix for #3860 - undoing first node of ways. Turned out to be pretty hard to come up with a clean solution because the way is created before the user has chosen where the second node goes etc. Anyway, it works. --- .../halcyon/connection/MainUndoStack.as | 23 ++++++++++ .../connection/actions/AddNodeToWayAction.as | 30 ++++-------- .../connection/actions/BeginWayAction.as | 46 ++++++++++--------- .../potlatch2/controller/NoSelection.as | 26 ++++++----- 4 files changed, 73 insertions(+), 52 deletions(-) diff --git a/net/systemeD/halcyon/connection/MainUndoStack.as b/net/systemeD/halcyon/connection/MainUndoStack.as index dd4c299e..65ae6d79 100644 --- a/net/systemeD/halcyon/connection/MainUndoStack.as +++ b/net/systemeD/halcyon/connection/MainUndoStack.as @@ -18,6 +18,8 @@ package net.systemeD.halcyon.connection { private var undoActions:Array = []; private var redoActions:Array = []; + private var undorequests:int = 0; + private var redorequests:int = 0; /** * Performs the action, then puts it on the undo stack. @@ -88,6 +90,10 @@ package net.systemeD.halcyon.connection { redoActions.push(action); dispatchEvent(new Event("new_undo_item")); dispatchEvent(new Event("new_redo_item")); + if (undorequests > 0) { + undorequests --; + undo(); + } } /** @@ -139,6 +145,23 @@ package net.systemeD.halcyon.connection { undoActions.push(action); dispatchEvent(new Event("new_undo_item")); dispatchEvent(new Event("new_redo_item")); + if (redorequests > 0) { + redorequests --; + redo(); + } + + } + + /** The cleanest solution to an ugly problem. Say an undo event X wants to call + * another undo event Y: if it calls it directly, they end up swapped + * in the undo history. This way, they can get called in the normal order. It's + * a way to loosely chain two events together. */ + public function requestUndo():void { + undorequests = undorequests + 1; + } + + public function requestRedo():void { + redorequests = redorequests + 1; } } diff --git a/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as b/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as index 85bc8c29..16495997 100644 --- a/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as +++ b/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as @@ -34,6 +34,13 @@ package net.systemeD.halcyon.connection.actions { nodeList.splice(index, 0, node); markDirty(); way.expandBbox(node); + + // sadly this gets ignored + /* if (way.length == 1) { + // way had no nodes before, so this event was probably ignored by WayUI etc. + way.dispatchEvent(new EntityEvent(Connection.NEW_WAY,way)); + }*/ + way.dispatchEvent(new WayNodeEvent(Connection.WAY_NODE_ADDED, node, way, index)); return SUCCESS; @@ -57,27 +64,10 @@ package net.systemeD.halcyon.connection.actions { markClean(); way.dispatchEvent(new WayNodeEvent(Connection.WAY_NODE_REMOVED, removed[0], way, index)); - // delete way if it's now 1-length, and convert the one remaining node to a POI + // If it's now 1-length, we want to delete the way and convert the one remaining node to a POI. + // We can't do this directly, so request the MainUndoStack to do it. if (autoDelete && way.length==1) { - way.setDeletedState(true); - way.dispatchEvent(new EntityEvent(Connection.WAY_DELETED, way)); - firstNode=way.getNode(0); - firstNode.removeParent(way); - var bwa: BeginWayAction; - bwa = MainUndoStack.getGlobalStack().removeLastIfAction(BeginWayAction) as BeginWayAction; - if (!firstNode.hasParentWays) { - var cpa: CreatePOIAction = new CreatePOIAction( - way.connection, - firstNode.getTagsCopy()/*getTagsHash?*/, - firstNode.lat, - firstNode.lon, - firstNode, - bwa.getNodeCreation() - ); - //firstNode.connection.registerPOI(firstNode); - MainUndoStack.getGlobalStack().addAction(cpa); - - } + MainUndoStack.getGlobalStack().requestUndo(); } return SUCCESS; } diff --git a/net/systemeD/halcyon/connection/actions/BeginWayAction.as b/net/systemeD/halcyon/connection/actions/BeginWayAction.as index 79657514..d0d89ac3 100644 --- a/net/systemeD/halcyon/connection/actions/BeginWayAction.as +++ b/net/systemeD/halcyon/connection/actions/BeginWayAction.as @@ -2,51 +2,55 @@ package net.systemeD.halcyon.connection.actions { import net.systemeD.halcyon.connection.*; - /* This is needed so that the specific type of CUA can be detected when CreatePOIAction is called */ + /** The action of starting drawing a way. It's a messy action to define because the + * user deals with nodes rather than ways, and we don't want an undo step that is + * invisible. Going forwards (redo), the redo of the following node is triggered + * automatically. Going backwards (undo), the conversion of the last node into a + * POI happens automatically. In some ideal universe, both this step and the + * creation of either the preceding or following node would be bundled together + * in one CUA, but it turns out to be very hard to implement that way. */ public class BeginWayAction extends CompositeUndoableAction { - private var newNode:Node; + private var firstNode:Node; private var newWay:Way; - private var lat:Number; - private var lon:Number; private var connection:Connection; - public function BeginWayAction(connection:Connection, lat:Number, lon:Number){ + public function BeginWayAction(connection:Connection, firstNode: Node){ super("Begin Way Action"); this.connection = connection; - this.lat = lat; - this.lon = lon; + this.firstNode = firstNode; } public override function doAction():uint { - if (newNode == null) { - newNode = connection.createNode({}, lat, lon, push); - newWay = connection.createWay({}, [newNode], push); + if (newWay == null) { + //newWay = connection.createWay({}, [firstNode], push); + // we create the way, then add the node to it. doing it in one step + // seemed to cause the undo process to delete the node with the way. + // A bug in DeleteWayAction.doAction()? + newWay = connection.createWay({}, [], push); + newWay.appendNode(firstNode,push); + } else { + // This is a redo. Way creation is an invisible step, so we request + // that the next step (the next node) gets redone as well. + MainUndoStack.getGlobalStack().requestRedo(); } + super.doAction(); - + connection.sendEvent(new EntityEvent(Connection.NEW_WAY, newWay), false); + connection.unregisterPOI(firstNode); return SUCCESS; } public override function undoAction():uint { super.undoAction(); - // this is needed because AddNodeToWayAction turns the first node into a POI when undoing. Hrm... - connection.unregisterPOI(newNode); - + connection.registerPOI(firstNode); return SUCCESS; } - public function getNode():Node { - return newNode; - } public function getWay():Way { return newWay; } - public function getNodeCreation():CreateEntityAction { - return this.getActions()[0]; - } - } } \ No newline at end of file diff --git a/net/systemeD/potlatch2/controller/NoSelection.as b/net/systemeD/potlatch2/controller/NoSelection.as index 59de43ca..00356a58 100644 --- a/net/systemeD/potlatch2/controller/NoSelection.as +++ b/net/systemeD/potlatch2/controller/NoSelection.as @@ -25,21 +25,25 @@ package net.systemeD.potlatch2.controller { if (event.type==MouseEvent.MOUSE_UP && (focus==null || (paint && paint.isBackground)) && map.dragstate!=map.DRAGGING && map.dragstate!=map.SWALLOW_MOUSEUP) { map.dragstate=map.NOT_DRAGGING; - // ** FIXME: BeginWayAction ought to be a discrete class - /*var undo:CompositeUndoableAction = new BeginWayAction(); var conn:Connection = layer.connection; - var startNode:Node = conn.createNode( - {}, - controller.map.coord2lat(event.localY), - controller.map.coord2lon(event.localX), undo.push); - var way:Way = conn.createWay({}, [startNode], undo.push);*/ - var undo:BeginWayAction = new BeginWayAction( - layer.connection, + + // User just created a node... + var nodeAction:CreatePOIAction = new CreatePOIAction( + conn, + {}, controller.map.coord2lat(event.localY), controller.map.coord2lon(event.localX)); - MainUndoStack.getGlobalStack().addAction(undo); - return new DrawWay(undo.getWay(), true, false); + MainUndoStack.getGlobalStack().addAction(nodeAction); + + // And a way. See BeginWayAction doco for why we keep these separate. + var wayAction:BeginWayAction = new BeginWayAction( + layer.connection, + nodeAction.getNode()); + + MainUndoStack.getGlobalStack().addAction(wayAction); + + return new DrawWay(wayAction.getWay(), true, false); } return this; } From 017c1e0515eb9ba4c21f33e89d17b2c5fb2aa17a Mon Sep 17 00:00:00 2001 From: Steve Bennett Date: Sun, 4 Mar 2012 01:08:32 +1100 Subject: [PATCH 3/3] Cleanup fix for #3860 --- net/systemeD/halcyon/connection/CompositeUndoableAction.as | 5 +---- .../halcyon/connection/actions/AddNodeToWayAction.as | 6 ------ 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/net/systemeD/halcyon/connection/CompositeUndoableAction.as b/net/systemeD/halcyon/connection/CompositeUndoableAction.as index 4979dbf5..5968205e 100644 --- a/net/systemeD/halcyon/connection/CompositeUndoableAction.as +++ b/net/systemeD/halcyon/connection/CompositeUndoableAction.as @@ -105,10 +105,7 @@ package net.systemeD.halcyon.connection { var str:String = " {" + actions.join(",") + "}"; return name + str; } - - protected function getActions():Array { - return actions; - } + } } diff --git a/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as b/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as index 16495997..575667ba 100644 --- a/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as +++ b/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as @@ -35,12 +35,6 @@ package net.systemeD.halcyon.connection.actions { markDirty(); way.expandBbox(node); - // sadly this gets ignored - /* if (way.length == 1) { - // way had no nodes before, so this event was probably ignored by WayUI etc. - way.dispatchEvent(new EntityEvent(Connection.NEW_WAY,way)); - }*/ - way.dispatchEvent(new WayNodeEvent(Connection.WAY_NODE_ADDED, node, way, index)); return SUCCESS;