Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binary Expressions deleted by live runner make their first input symbols null. #10512

Merged
merged 5 commits into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/Engine/ProtoScript/Runners/LiveRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,8 @@ private bool CompileToSSA(Guid guid, List<AssociativeNode> astList, out List<Ass
}

/// <summary>
/// Creates a list of null assignment statements where the lhs is retrieved from an ast list
/// Creates a list of null assignment statements where the lhs is retrieved from an ast list.
/// Any expressions which are not assignments are not modified.
/// </summary>
/// <param name="astList"></param>
/// <returns></returns>
Expand All @@ -982,7 +983,7 @@ private List<BinaryExpressionNode> BuildNullAssignments(List<AssociativeNode> as
}

IdentifierNode leftNode = bNode.LeftNode as IdentifierNode;
if (leftNode == null || leftNode.ArrayDimensions != null)
if (leftNode == null || leftNode.ArrayDimensions != null || bNode.Optr != Operator.assign)
Copy link
Contributor

@aparajit-pratap aparajit-pratap Apr 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but why not have this check on line 980?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right, that seems like a more appropriate place, where the binary node is validated, I can move it.

{
continue;
}
Expand Down
20 changes: 20 additions & 0 deletions test/DynamoCoreTests/DSEvaluationModelTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,26 @@ public void Regression_Magn_10015()
RunModel(dynFilePath);
AssertPreviewValue("deb457c6-1b4b-4703-9476-db312b34a8e2", new object[] { null, null, null, null });
}

[Test]
public void LogicUINodesDeleted()
{
RunModel(@"core\dsevaluation\testuilogicnodes.dyn");
this.CurrentDynamoModel.CurrentWorkspace.RequestRun();
var codeblock = this.CurrentDynamoModel.CurrentWorkspace.Nodes.Where(x => x.Name == "Code Block").First();
var uiANDnode = this.CurrentDynamoModel.CurrentWorkspace.Nodes.Where(x => x.Name == "And").First();
var ztANDnode = this.CurrentDynamoModel.CurrentWorkspace.Nodes.Where(x => x.Name == "&&").First();
AssertPreviewValue(ztANDnode.GUID.ToString(), true);


//delete binary expression AND node
this.CurrentDynamoModel.CurrentWorkspace.RemoveAndDisposeNode(uiANDnode);
this.CurrentDynamoModel.CurrentWorkspace.RequestRun();

//assert other node value is still valid
AssertPreviewValue(ztANDnode.GUID.ToString(), true);
AssertPreviewValue(codeblock.GUID.ToString(), true);
}
}

[Category("GithubIssues")]
Expand Down
87 changes: 87 additions & 0 deletions test/Engine/ProtoTest/LiveRunnerTests/MicroFeatureTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,93 @@ public void GraphILTest_DeletedNode01()

}

[Test]
public void GraphILTest_DeletedBinaryExpresionDoesNotEffectReferences()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effectively this test is simulating the same thing that would happen in the Dynamo UI when running the graph, deleting the node, and rerunning it or is it testing something additionally?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simulating the same thing as in the UI - I verified it failed before this change.

{
//====================================
// Create a = true;
// b = true;
// c = a & b;
// d = a & b;
// Execute and verify a = true
// b= true
// c = true
// d = true
// Delete c = a&b
//
// Execute and verify d = true;
//====================================

// Create a = true;
// b = true;
// c = a & b;
// d = a & b;
BinaryExpressionNode assign1 = new BinaryExpressionNode(
new IdentifierNode("a"),
new BooleanNode(true),
Operator.assign);

BinaryExpressionNode assign2 = new BinaryExpressionNode(
new IdentifierNode("b"),
new BooleanNode(true),
Operator.assign);

BinaryExpressionNode assign3 = new BinaryExpressionNode(
new IdentifierNode("c"),
new BinaryExpressionNode(new IdentifierNode("a"), new IdentifierNode("b")
, Operator.and)
,Operator.assign);

BinaryExpressionNode assign4 = new BinaryExpressionNode(
new IdentifierNode("d"),
new BinaryExpressionNode(new IdentifierNode("a"), new IdentifierNode("b")
, Operator.and)
,Operator.assign);

List<AssociativeNode> astList = new List<AssociativeNode>() {assign1,assign2,assign3,assign4};



Guid guid = System.Guid.NewGuid();


// Instantiate GraphSyncData
List<Subtree> addedList = new List<Subtree>();
addedList.Add(new Subtree(astList,guid));

GraphSyncData syncData = new GraphSyncData(null, addedList, null);

// emit the DS code from the AST tree
liveRunner = new ProtoScript.Runners.LiveRunner();
liveRunner.UpdateGraph(syncData);

// Execute and verify c = true
// Execute and verify d = true
RuntimeMirror mirrorc = liveRunner.InspectNodeValue("c");
Assert.IsTrue((bool)mirrorc.GetData().Data == true);
RuntimeMirror mirrord = liveRunner.InspectNodeValue("d");
Assert.IsTrue((bool)mirrord.GetData().Data == true);



// Delete c = a&b
var deleteGuid = Guid.NewGuid();
var deletedsb = new Subtree(new List<AssociativeNode> { assign3 }, deleteGuid);
List<Subtree> deletedList = new List<Subtree>();
deletedList.Add(deletedsb);
syncData = new GraphSyncData(deletedList, null, null);
liveRunner.UpdateGraph(syncData);

// Execute and verify c = true
// Execute and verify d = true
RuntimeMirror mirrorc2 = liveRunner.InspectNodeValue("c");
Assert.IsTrue(mirrorc2.GetData().Data == null);
RuntimeMirror mirrord2 = liveRunner.InspectNodeValue("d");
Assert.IsTrue((bool)mirrord2.GetData().Data == true);


}

private void AssertValue(string varname, object value)
{
var mirror = liveRunner.InspectNodeValue(varname);
Expand Down
203 changes: 203 additions & 0 deletions test/core/dsevaluation/testuilogicnodes.dyn
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
{
"Uuid": "f92363c3-5707-41ec-8a63-06acf36b45ae",
"IsCustomNode": false,
"Description": null,
"Name": "testuilogicnodes",
"ElementResolver": {
"ResolutionMap": {}
},
"Inputs": [],
"Outputs": [],
"Nodes": [
{
"ConcreteType": "Dynamo.Graph.Nodes.CodeBlockNodeModel, DynamoCore",
"NodeType": "CodeBlockNode",
"Code": "true;\ntrue;",
"Id": "85bc989a337a45f1b5848734338ae673",
"Inputs": [],
"Outputs": [
{
"Id": "5683b294f10446c8b5839420e34dab9c",
"Name": "",
"Description": "Value of expression at line 1",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
},
{
"Id": "30a412eb4e714626a97c37a8a69d19ed",
"Name": "",
"Description": "Value of expression at line 2",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
}
],
"Replication": "Disabled",
"Description": "Allows for DesignScript code to be authored directly"
},
{
"ConcreteType": "CoreNodeModels.Logic.And, CoreNodeModels",
"VariableInputPorts": true,
"NodeType": "ExtensionNode",
"Id": "d6fcc6b732b44a8b90a2ad58876ac009",
"Inputs": [
{
"Id": "7fb42091df0d4b8abe4c5ccdf6fe435f",
"Name": "bool0",
"Description": "operand",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
},
{
"Id": "44d8b83a99ef4b4094e09613b4e635c7",
"Name": "bool1",
"Description": "operand",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
}
],
"Outputs": [
{
"Id": "51e23dadf91f4cd2ba97fa4b953a9b7e",
"Name": "",
"Description": "result",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
}
],
"Replication": "Disabled",
"Description": "Boolean AND: Returns true only if both of the inputs are true. If either is false, returns false."
},
{
"ConcreteType": "Dynamo.Graph.Nodes.ZeroTouch.DSFunction, DynamoCore",
"NodeType": "FunctionNode",
"FunctionSignature": "&&@var[]..[],var[]..[]",
"Id": "f206cdb1d1be467fbffdfb7138d14162",
"Inputs": [
{
"Id": "31bf8f9d6a0a4be7a15a5b2955ce181a",
"Name": "x",
"Description": "x value.\n\nvar[]..[]",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
},
{
"Id": "8b45739b667749cb8bba69cab5fc985f",
"Name": "y",
"Description": "y value.\n\nvar[]..[]",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
}
],
"Outputs": [
{
"Id": "a767c3b1a709403c881f6c9bedaf3c2d",
"Name": "var[]..[]",
"Description": "var[]..[]",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
}
],
"Replication": "Auto",
"Description": "x and y?\n\n&& (x: var[]..[], y: var[]..[]): var[]..[]"
}
],
"Connectors": [
{
"Start": "5683b294f10446c8b5839420e34dab9c",
"End": "7fb42091df0d4b8abe4c5ccdf6fe435f",
"Id": "08f517ed5b874462b5715116b8dd7acf"
},
{
"Start": "5683b294f10446c8b5839420e34dab9c",
"End": "31bf8f9d6a0a4be7a15a5b2955ce181a",
"Id": "cfe744626fde4fdf81926898e7b5269d"
},
{
"Start": "30a412eb4e714626a97c37a8a69d19ed",
"End": "8b45739b667749cb8bba69cab5fc985f",
"Id": "25fe7c834a644402a87135db0681882d"
},
{
"Start": "30a412eb4e714626a97c37a8a69d19ed",
"End": "44d8b83a99ef4b4094e09613b4e635c7",
"Id": "5ccd3af80efd4a5e960dcd0166bd7ebf"
}
],
"Dependencies": [],
"NodeLibraryDependencies": [],
"Bindings": [],
"View": {
"Dynamo": {
"ScaleFactor": 1.0,
"HasRunWithoutCrash": true,
"IsVisibleInDynamoLibrary": true,
"Version": "2.7.0.8216",
"RunType": "Automatic",
"RunPeriod": "1000"
},
"Camera": {
"Name": "Background Preview",
"EyeX": -17.0,
"EyeY": 24.0,
"EyeZ": 50.0,
"LookX": 12.0,
"LookY": -13.0,
"LookZ": -58.0,
"UpX": 0.0,
"UpY": 1.0,
"UpZ": 0.0
},
"NodeViews": [
{
"ShowGeometry": true,
"Name": "Code Block",
"Id": "85bc989a337a45f1b5848734338ae673",
"IsSetAsInput": false,
"IsSetAsOutput": false,
"Excluded": false,
"X": 235.0,
"Y": 265.0
},
{
"ShowGeometry": true,
"Name": "And",
"Id": "d6fcc6b732b44a8b90a2ad58876ac009",
"IsSetAsInput": false,
"IsSetAsOutput": false,
"Excluded": false,
"X": 505.5,
"Y": 233.0
},
{
"ShowGeometry": true,
"Name": "&&",
"Id": "f206cdb1d1be467fbffdfb7138d14162",
"IsSetAsInput": false,
"IsSetAsOutput": false,
"Excluded": false,
"X": 506.5,
"Y": 365.0
}
],
"Annotations": [],
"X": 0.0,
"Y": 0.0,
"Zoom": 1.0
}
}