From 392b92a055caaaab1bd64ca621ba8d75b09f1ece Mon Sep 17 00:00:00 2001 From: james-strauss-uwa Date: Wed, 3 Jul 2024 14:36:28 +0800 Subject: [PATCH 1/3] Fixed bug where 'adding nodes to subgraph' could not be undone/redone multiple times --- src/Eagle.ts | 1 + src/LogicalGraph.ts | 34 ++++++++++++++++++++-------------- src/Undo.ts | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/src/Eagle.ts b/src/Eagle.ts index 3477713ee..a1ad054e5 100644 --- a/src/Eagle.ts +++ b/src/Eagle.ts @@ -1024,6 +1024,7 @@ export class Eagle { // update selection node.setParentKey(parentNode.getKey()); + console.log("set parent of node", node.getKey(), node.getName(), "to", parentNode.getKey(), parentNode.getName()); } // shrink/expand subgraph node to fit children diff --git a/src/LogicalGraph.ts b/src/LogicalGraph.ts index 3b2d52979..43aa77afe 100644 --- a/src/LogicalGraph.ts +++ b/src/LogicalGraph.ts @@ -55,11 +55,11 @@ export class LogicalGraph { result.modelData = FileInfo.toOJSJson(graph.fileInfo()); result.modelData.schemaVersion = Daliuge.SchemaVersion.OJS; - result.modelData.numLGNodes = graph.getNodes().length; + result.modelData.numLGNodes = graph.nodes().length; // add nodes result.nodeDataArray = []; - for (const node of graph.getNodes()){ + for (const node of graph.nodes()){ const nodeData : any = Node.toOJSGraphJson(node); result.nodeDataArray.push(nodeData); } @@ -267,7 +267,7 @@ export class LogicalGraph { getCommentNodes = () : Node[] => { const commentNodes: Node[] = []; - for (const node of this.getNodes()){ + for (const node of this.nodes()){ if (node.isComment()){ commentNodes.push(node); } @@ -419,14 +419,13 @@ export class LogicalGraph { } findNodeGraphIdByNodeName = (name:string) :string =>{ - const eagle: Eagle = Eagle.getInstance(); - let graphNodeId:string - eagle.logicalGraph().getNodes().forEach(function(node){ - if(node.getName() === name){ - graphNodeId = node.getId() + for (const node of this.nodes()){ + if (node.getName() === name){ + return node.getId(); } - }) - return graphNodeId + } + + return null; } removeNode = (node: Node) : void => { @@ -547,12 +546,14 @@ export class LogicalGraph { // TODO: shrinkNode and normaliseNodes seem to share some common code, maybe factor out or combine? shrinkNode = (node : Node) : void => { + console.log("shrinkNode()", node.getKey()); + // abort shrink of non-group node if (!node.isGroup()){ + console.log("not group"); return; } - const nodes : Node[] = this.getNodes(); let minX : number = Number.MAX_SAFE_INTEGER; let minY : number = Number.MAX_SAFE_INTEGER; let maxX : number = Number.MIN_SAFE_INTEGER; @@ -560,7 +561,9 @@ export class LogicalGraph { let numChildren : number = 0; // loop through all nodes, finding all children and determining minimum bounding box to contain all children - for (const n of nodes){ + for (const n of this.nodes()){ + console.log("n", n.getKey(), n.getParentKey()); + if (n.getParentKey() === node.getKey()){ numChildren += 1; @@ -581,6 +584,7 @@ export class LogicalGraph { // if no children were found, set to default size if (numChildren === 0){ + console.log("no children"); node.setRadius(GraphConfig.MINIMUM_CONSTRUCT_RADIUS); return; } @@ -595,6 +599,8 @@ export class LogicalGraph { node.setPosition(minX, minY); const maxDimension = Math.max(maxX - minX, maxY - minY); node.setRadius(maxDimension); + + console.log("Shrink node", node.getName(), node.getKey(), "to", minX, minY, maxDimension); } findMultiplicity = (node : Node) : number => { @@ -729,7 +735,7 @@ export class LogicalGraph { // populate index plus depths for (let i = 0 ; i < this.nodes().length ; i++){ - const node = this.getNodes()[i]; + const node = this.nodes()[i]; const depth = this.findDepthByKey(node.getKey()); @@ -743,7 +749,7 @@ export class LogicalGraph { // write nodes to result in sorted order for (const indexPlusDepth of indexPlusDepths){ - result.push(this.getNodes()[indexPlusDepth.index]); + result.push(this.nodes()[indexPlusDepth.index]); } return result; diff --git a/src/Undo.ts b/src/Undo.ts index c2ca96bf8..e01a90d78 100644 --- a/src/Undo.ts +++ b/src/Undo.ts @@ -80,6 +80,7 @@ export class Undo { return; } + console.log("Undo: write to memory at", this.current()); this.memory()[this.current()] = new Snapshot(description, newContent); this.memory.valueHasMutated(); this.front((this.current() + 1) % Config.UNDO_MEMORY_SIZE); @@ -123,6 +124,8 @@ export class Undo { } eagle.checkGraph(); + + this._updateSelection(); } nextSnapshot = (eagle: Eagle) : void => { @@ -140,6 +143,8 @@ export class Undo { } eagle.checkGraph(); + + this._updateSelection(); } toString = () : string => { @@ -165,6 +170,7 @@ export class Undo { } _loadFromIndex = (index: number, eagle: Eagle) : void => { + console.log("_loadFromIndex()", index); const snapshot : Snapshot = this.memory()[index]; if (snapshot === null){ @@ -173,8 +179,34 @@ export class Undo { } const dataObject: LogicalGraph = snapshot.data(); + console.log("dataObject", "nodes", dataObject.getNodes().length, "edges", dataObject.getEdges().length); + + eagle.logicalGraph(dataObject.clone()); + } + + // if we undo, or redo, then the objects in selectedObject are from the graph prior to the new snapshot + // so the references will be to non-existent objects + // in this function, we use the ids of the old selectedObjects, and attempt to add the matching objects in the new snapshot to the selectedObjects list + _updateSelection = () : void => { + const eagle: Eagle = Eagle.getInstance(); + const objectIds: string[] = []; - eagle.logicalGraph(dataObject); + // build a list of the ids of the selected objects + for (const object of eagle.selectedObjects()){ + objectIds.push(object.getId()); + } + + // clear selection + eagle.setSelection(Eagle.RightWindowMode.Hierarchy, null, Eagle.FileType.Graph); + + // find the objects in the ids list, and add them to the selection + for (const id of objectIds){ + const node = eagle.logicalGraph().findNodeById(id); + const edge = eagle.logicalGraph().findEdgeById(id); + const object = node || edge; + + eagle.editSelection(eagle.rightWindow().mode(), object, Eagle.selectedLocation()); + } } static printTable() : void { @@ -193,6 +225,8 @@ export class Undo { "current": realCurrent === i ? "->" : "", "description": snapshot.description(), "buffer position": i, + "nodes": snapshot.data().getNodes().length, + "edges": snapshot.data().getEdges().length }); } From 9820f2fe95caa1f68995ca097cb66b0e33ec2fa3 Mon Sep 17 00:00:00 2001 From: james-strauss-uwa Date: Wed, 3 Jul 2024 14:45:08 +0800 Subject: [PATCH 2/3] Cleanup console.log --- src/Eagle.ts | 3 --- src/LogicalGraph.ts | 8 -------- src/Undo.ts | 4 ---- 3 files changed, 15 deletions(-) diff --git a/src/Eagle.ts b/src/Eagle.ts index a1ad054e5..c4bef9ef9 100644 --- a/src/Eagle.ts +++ b/src/Eagle.ts @@ -993,8 +993,6 @@ export class Eagle { } createSubgraphFromSelection = () : void => { - console.log("createSubgraphFromSelection()"); - const eagle = Eagle.getInstance() if(eagle.selectedObjects().length === 0){ Utils.showNotification('Error','At least one node must be selected!', 'warning') @@ -1024,7 +1022,6 @@ export class Eagle { // update selection node.setParentKey(parentNode.getKey()); - console.log("set parent of node", node.getKey(), node.getName(), "to", parentNode.getKey(), parentNode.getName()); } // shrink/expand subgraph node to fit children diff --git a/src/LogicalGraph.ts b/src/LogicalGraph.ts index 43aa77afe..1e70b52f6 100644 --- a/src/LogicalGraph.ts +++ b/src/LogicalGraph.ts @@ -546,11 +546,8 @@ export class LogicalGraph { // TODO: shrinkNode and normaliseNodes seem to share some common code, maybe factor out or combine? shrinkNode = (node : Node) : void => { - console.log("shrinkNode()", node.getKey()); - // abort shrink of non-group node if (!node.isGroup()){ - console.log("not group"); return; } @@ -562,8 +559,6 @@ export class LogicalGraph { // loop through all nodes, finding all children and determining minimum bounding box to contain all children for (const n of this.nodes()){ - console.log("n", n.getKey(), n.getParentKey()); - if (n.getParentKey() === node.getKey()){ numChildren += 1; @@ -584,7 +579,6 @@ export class LogicalGraph { // if no children were found, set to default size if (numChildren === 0){ - console.log("no children"); node.setRadius(GraphConfig.MINIMUM_CONSTRUCT_RADIUS); return; } @@ -599,8 +593,6 @@ export class LogicalGraph { node.setPosition(minX, minY); const maxDimension = Math.max(maxX - minX, maxY - minY); node.setRadius(maxDimension); - - console.log("Shrink node", node.getName(), node.getKey(), "to", minX, minY, maxDimension); } findMultiplicity = (node : Node) : number => { diff --git a/src/Undo.ts b/src/Undo.ts index e01a90d78..149f7a6a9 100644 --- a/src/Undo.ts +++ b/src/Undo.ts @@ -80,7 +80,6 @@ export class Undo { return; } - console.log("Undo: write to memory at", this.current()); this.memory()[this.current()] = new Snapshot(description, newContent); this.memory.valueHasMutated(); this.front((this.current() + 1) % Config.UNDO_MEMORY_SIZE); @@ -170,7 +169,6 @@ export class Undo { } _loadFromIndex = (index: number, eagle: Eagle) : void => { - console.log("_loadFromIndex()", index); const snapshot : Snapshot = this.memory()[index]; if (snapshot === null){ @@ -179,8 +177,6 @@ export class Undo { } const dataObject: LogicalGraph = snapshot.data(); - console.log("dataObject", "nodes", dataObject.getNodes().length, "edges", dataObject.getEdges().length); - eagle.logicalGraph(dataObject.clone()); } From 3039f200dfe8866d505ec648822144ae4e8274ab Mon Sep 17 00:00:00 2001 From: james-strauss-uwa Date: Thu, 4 Jul 2024 15:59:44 +0800 Subject: [PATCH 3/3] Fixed bug where selected objects were not found when updating selection --- src/Undo.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Undo.ts b/src/Undo.ts index 1dc95ab28..57f3e418f 100644 --- a/src/Undo.ts +++ b/src/Undo.ts @@ -202,6 +202,11 @@ export class Undo { const edge = eagle.logicalGraph().findEdgeById(id); const object = node || edge; + // abort if no edge or node exists fot that id + if (node === null && edge === null){ + continue; + } + eagle.editSelection(eagle.rightWindow().mode(), object, Eagle.selectedLocation()); } }