Skip to content

Commit

Permalink
Do not set execution hint on undo move (#11519) (#11524)
Browse files Browse the repository at this point in the history
There were a couple of issues that were causing a node to be set to be
executed on undoing an action that consisted of just a move:

1. The 'UsingDefaultValue' property was triggering a property changed
even if the deserialized value was the same as the current. This was
fixed by checking if the value is the same and not doing anything in
that case.

2. Even if the 'IsFrozen' property for the current node had not changed
an undo would mark the current and all downstream nodes as if they were
modified. This was fixed by relying on the 'IsFrozen' property
setter, which correctly handles the cases when the previous should be
done.
  • Loading branch information
mmisol authored Mar 2, 2021
1 parent 6c9f89c commit 1e4040c
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 12 deletions.
9 changes: 1 addition & 8 deletions src/DynamoCore/Graph/Nodes/NodeModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2234,7 +2234,7 @@ protected override void DeserializeCore(XmlElement nodeElement, SaveContext cont
argumentLacing = helper.ReadEnum("lacing", LacingStrategy.Disabled);
IsSetAsInput = helper.ReadBoolean("isSelectedInput", false);
IsSetAsOutput = helper.ReadBoolean("isSelectedOutput", false);
isFrozenExplicitly = helper.ReadBoolean("IsFrozen", false);
IsFrozen = helper.ReadBoolean("IsFrozen", false);
PreviewPinned = helper.ReadBoolean("isPinned", false);

var portInfoProcessed = new HashSet<int>();
Expand Down Expand Up @@ -2306,13 +2306,6 @@ var port in
RaisePropertyChanged(nameof(ArgumentLacing));
RaisePropertyChanged(nameof(IsVisible));
RaisePropertyChanged(nameof(DisplayLabels));
RaisePropertyChanged(nameof(IsSetAsInput));
RaisePropertyChanged(nameof(IsSetAsOutput));
//we need to modify the downstream nodes manually in case the
//undo is for toggling freeze. This is ONLY modifying the execution hint.
// this does not run the graph.
RaisePropertyChanged(nameof(IsFrozen));
MarkDownStreamNodesAsModified(this);

// Notify listeners that the position of the node has changed,
// then all connected connectors will also redraw themselves.
Expand Down
9 changes: 6 additions & 3 deletions src/DynamoCore/Graph/Nodes/PortModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,12 @@ public bool UsingDefaultValue
get { return usingDefaultValue; }
set
{
usingDefaultValue = value;
RaisePropertyChanged("UsingDefaultValue");
RaisePropertyChanged("ToolTip");
if (usingDefaultValue != value)
{
usingDefaultValue = value;
RaisePropertyChanged("UsingDefaultValue");
RaisePropertyChanged("ToolTip");
}
}
}

Expand Down
9 changes: 8 additions & 1 deletion src/Libraries/PythonNodeModels/PythonNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,14 @@ protected override void DeserializeCore(XmlElement nodeElement, SaveContext cont

if (engineNode != null)
{
this.Engine = (PythonEngineVersion)Enum.Parse(typeof(PythonEngineVersion),engineNode.InnerText);
var oldEngine = Engine;
Engine = (PythonEngineVersion)Enum.Parse(typeof(PythonEngineVersion), engineNode.InnerText);
if (context == SaveContext.Undo && oldEngine != this.Engine)
{
// For Python nodes, changing the Engine property does not set the node Modified flag.
// This is done externally in all event handlers, so for Undo we do it here.
OnNodeModified();
}
}
}

Expand Down
23 changes: 23 additions & 0 deletions test/DynamoCoreTests/CoreTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,29 @@ public void PasteInputAndOutputNodeInHomeWorkspace()
Assert.IsInstanceOf<CodeBlockNodeModel>(homeNodes.ElementAt(1));
}

[Test]
public void UndoMoveDoesNotForceExecution()
{
string openPath = Path.Combine(TestDirectory, @"core\LacingTest.dyn");
OpenModel(openPath);
RunCurrentModel();

var sumNodeGuid = new Guid("088365b5-4543-4e73-9430-463475147e19");
var sumNode = CurrentDynamoModel.CurrentWorkspace.Nodes.First(node => node.GUID == sumNodeGuid);
Assert.IsFalse(sumNode.IsModified);

var oldX = sumNode.X;
var newX = sumNode.X + 100;
var newPosition = $"{newX};{sumNode.Y}";
CurrentDynamoModel.ExecuteCommand(new DynCmd.UpdateModelValueCommand(Guid.Empty, sumNodeGuid, nameof(NodeModel.Position), newPosition));
Assert.AreEqual(newX, sumNode.X);

CurrentDynamoModel.ExecuteCommand(new DynCmd.UndoRedoCommand(DynCmd.UndoRedoCommand.Operation.Undo));
Assert.AreEqual(oldX, sumNode.X);

Assert.IsFalse(sumNode.IsModified);
}

[Test]
public void TestFileDirtyOnLacingChange()
{
Expand Down

0 comments on commit 1e4040c

Please sign in to comment.