diff --git a/src/DynamoCore/Engine/EngineController.cs b/src/DynamoCore/Engine/EngineController.cs index 7deb7a4bf9e..9adbe0d789d 100644 --- a/src/DynamoCore/Engine/EngineController.cs +++ b/src/DynamoCore/Engine/EngineController.cs @@ -43,11 +43,6 @@ public class EngineController : LogSourceBase, IAstNodeContainer, IDisposable /// internal static event Action VMLibrariesReset; - /// - /// This flag is used to check if any packages are currently being loaded, and to disable any executions that are triggered before the package loading is completed. See DYN-2101 for more info. - /// - internal static Boolean DisableRun = false; - /// /// This event is fired when is completed. /// @@ -518,8 +513,16 @@ private void OnLibraryLoaded() /// private void LibraryLoaded(object sender, LibraryServices.LibraryLoadedEventArgs e) { - if(e.LibraryPaths.Any()) + if (e.LibraryPaths.Any()) + { OnLibraryLoaded(); + } + } + + internal event EventHandler RequestCustomNodeRegistration; + internal void OnRequestCustomNodeRegistration() + { + RequestCustomNodeRegistration?.Invoke(null, EventArgs.Empty); } #region Implement IAstNodeContainer interface diff --git a/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs b/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs index 6d3869bb044..bd26515b1cf 100644 --- a/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs +++ b/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs @@ -324,6 +324,8 @@ protected override void OnNodeRemoved(NodeModel node) private void LibraryLoaded(object sender, LibraryServices.LibraryLoadedEventArgs e) { + // Need to make compiled custom nodes available before running the graph. + EngineController.OnRequestCustomNodeRegistration(); // Mark all nodes as dirty so that AST for the whole graph will be // regenerated. MarkNodesAsModifiedAndRequestRun(Nodes); @@ -346,15 +348,7 @@ internal override void RequestRun() // Make this RunSettings.RunEnabled private, introduce the new flag and remove the "executingTask" variable. if (RunSettings.RunEnabled || executingTask) { - // skip the execution if runs have been disabled - currently this flag is only set by the Package Loader - if (!EngineController.DisableRun) - { - Run(); - } - else - { - this.Log("Run has been disabled in the Engine Controller"); - } + Run(); } } } diff --git a/src/DynamoCore/Models/DynamoModel.cs b/src/DynamoCore/Models/DynamoModel.cs index 159e1111a77..eddd8969b4d 100644 --- a/src/DynamoCore/Models/DynamoModel.cs +++ b/src/DynamoCore/Models/DynamoModel.cs @@ -1039,6 +1039,7 @@ private void OnAsyncTaskStateChanged(DynamoScheduler sender, TaskStateChangedEve public void Dispose() { EngineController.TraceReconcliationComplete -= EngineController_TraceReconcliationComplete; + EngineController.RequestCustomNodeRegistration -= EngineController_RequestCustomNodeRegistration; ExtensionManager.Dispose(); extensionManager.MessageLogged -= LogMessage; @@ -1454,6 +1455,7 @@ protected void ResetEngineInternal() { if (EngineController != null) { + EngineController.RequestCustomNodeRegistration -= EngineController_RequestCustomNodeRegistration; EngineController.TraceReconcliationComplete -= EngineController_TraceReconcliationComplete; EngineController.MessageLogged -= LogMessage; EngineController.Dispose(); @@ -1467,11 +1469,18 @@ protected void ResetEngineInternal() EngineController.MessageLogged += LogMessage; EngineController.TraceReconcliationComplete += EngineController_TraceReconcliationComplete; + EngineController.RequestCustomNodeRegistration += EngineController_RequestCustomNodeRegistration; foreach (var def in CustomNodeManager.LoadedDefinitions) RegisterCustomNodeDefinitionWithEngine(def); } + private void EngineController_RequestCustomNodeRegistration(object sender, EventArgs e) + { + foreach (var def in CustomNodeManager.LoadedDefinitions) + RegisterCustomNodeDefinitionWithEngine(def); + } + /// /// Forces an evaluation of the current workspace by resetting the DesignScript VM. /// @@ -1775,8 +1784,6 @@ private void ReloadDummyNodes() // If the resolved node is also a dummy node, then skip it else replace the dummy node with the resolved version of the node. if (!(resolvedNode is DummyNode)) { - // Disable graph runs temporarily while the dummy node is replaced with the resolved version of that node. - EngineController.DisableRun = true; currentWorkspace.RemoveAndDisposeNode(dn); currentWorkspace.AddAndRegisterNode(resolvedNode, false); @@ -1799,7 +1806,6 @@ private void ReloadDummyNodes() connectorModel.Delete(); ConnectorModel.Make(startNode, endNode, connectorModel.Start.Index, connectorModel.End.Index, connectorModel.GUID); } - EngineController.DisableRun = false ; resolvedDummyNode = true; } } diff --git a/src/DynamoPackages/PackageLoader.cs b/src/DynamoPackages/PackageLoader.cs index 3048a09aaba..b3178339527 100644 --- a/src/DynamoPackages/PackageLoader.cs +++ b/src/DynamoPackages/PackageLoader.cs @@ -296,18 +296,6 @@ public void LoadPackages(IEnumerable packages) { var packageList = packages.ToList(); - // This fix is in reference to the crash reported in task: https://jira.autodesk.com/browse/DYN-2101 - // TODO: https://jira.autodesk.com/browse/DYN-2120. we will be re-evaluating this workflow, to find the best clean solution. - - // The reason for this crash is, when a new package is being loaded into the dynamo, it will reload - // all the libraries into the VM. Since the graph execution runs are triggered asynchronously, it causes - // an exception as the VM is reinitialized during the execution run. To avoid this, we disable the execution run's that - // are triggered while the package is still being loaded. Once the package is completely loaded and the VM is reinitialized, - // a final run is triggered that would execute the nodes in the workspace after resolving them. - - // Disabling the run here since new packages are being loaded. - EngineController.DisableRun = true; - foreach (var pkg in packageList) { // If the pkg is null, then don't load that package into the Library. @@ -317,9 +305,6 @@ public void LoadPackages(IEnumerable packages) } } - // Setting back the DisableRun property back to false, as the package loading is completed. - EngineController.DisableRun = false; - var assemblies = packageList .SelectMany(p => p.LoadedAssemblies.Where(y => y.IsNodeLibrary)) .Select(a => a.Assembly) diff --git a/test/Libraries/PackageManagerTests/PackageLoaderTests.cs b/test/Libraries/PackageManagerTests/PackageLoaderTests.cs index 2da011dfc6f..8ef97eba3f5 100644 --- a/test/Libraries/PackageManagerTests/PackageLoaderTests.cs +++ b/test/Libraries/PackageManagerTests/PackageLoaderTests.cs @@ -4,6 +4,7 @@ using System.Linq; using Dynamo.Engine; using Dynamo.Extensions; +using Dynamo.Graph.Nodes; using Dynamo.Graph.Nodes.CustomNodes; using Dynamo.Graph.Workspaces; using Dynamo.Search.SearchElements; @@ -487,34 +488,21 @@ public void GetOwnerPackageReturnsNullForInvalidFunction() Assert.IsNull(foundPkg); } - /// This test is added for this task: https://jira.autodesk.com/browse/DYN-2101. - /// A followup task is added https://jira.autodesk.com/browse/DYN-2120 to refactor the approach to this solution. - /// This test needs to be modified in that case. [Test] - [Category("TechDebt")] public void PackageLoadExceptionTest() { - Boolean RunDisabledWhilePackageLoading = false; - string openPath = Path.Combine(TestDirectory, @"core\PackageLoadExceptionTest.dyn"); OpenModel(openPath); var loader = GetPackageLoader(); - loader.PackgeLoaded += (package) => - { - RunDisabledWhilePackageLoading = EngineController.DisableRun; - }; // Load the package when the graph is open in the workspace. string packageDirectory = Path.Combine(PackagesDirectory, "Ampersand"); var pkg = loader.ScanPackageDirectory(packageDirectory); loader.LoadPackages(new List { pkg }); - // Assert that the Run is disabled temporarily when the package is still loading. - Assert.IsTrue(RunDisabledWhilePackageLoading); - - // Assert that the DisableRun flag is set back to false, once the package loading is completed. - Assert.IsFalse(EngineController.DisableRun); + // Dummy nodes are resolved, and more importantly, no exception was thrown. + Assert.AreEqual(0, CurrentDynamoModel.CurrentWorkspace.Nodes.OfType().Count()); } /// @@ -545,7 +533,32 @@ public void MixedPackageLoadTest() // Assert value of loaded CN is non-null. AssertNonNull("576f11ed5837460d80f2e354d853de68"); + } + + [Test] + public void LoadingAPackageWithBinariesDoesNotAffectCustomNodesUsedInHomeWorkspace() + { + // Open a custom node definition and a workspace where this custom node is used. + OpenModel(Path.Combine(TestDirectory, @"core\PackageLoadReset\test.dyf")); + OpenModel(Path.Combine(TestDirectory, @"core\PackageLoadReset\MissingNode.dyn")); + + // Get the custom node. + var functionNodes = CurrentDynamoModel.CurrentWorkspace.Nodes.OfType(); + Assert.AreEqual(1, functionNodes.Count()); + var functionNode = functionNodes.First(); + + // Custom node should be good before loading the package. + Assert.AreEqual(ElementState.Active, functionNode.State); + AssertPreviewValue(functionNode.AstIdentifierGuid, 7); + + // Load a package which contains binaries, when the graph is open in the workspace. + var loader = GetPackageLoader(); + var pkg = loader.ScanPackageDirectory(Path.Combine(PackagesDirectory, "Mixed Package")); + loader.LoadPackages(new List { pkg }); + // Custom node should remain good after loading the package. + Assert.AreEqual(ElementState.Active, functionNode.State); + AssertPreviewValue(functionNode.AstIdentifierGuid, 7); } [Test] diff --git a/test/core/PackageLoadReset/MissingNode.dyn b/test/core/PackageLoadReset/MissingNode.dyn new file mode 100644 index 00000000000..cdde07966fb --- /dev/null +++ b/test/core/PackageLoadReset/MissingNode.dyn @@ -0,0 +1,117 @@ +{ + "Uuid": "63001d03-558d-4fae-a32c-767af1871c59", + "IsCustomNode": false, + "Description": null, + "Name": "MissingNode", + "ElementResolver": { + "ResolutionMap": {} + }, + "Inputs": [], + "Outputs": [], + "Nodes": [ + { + "ConcreteType": "Dynamo.Graph.Nodes.ZeroTouch.DSFunction, DynamoCore", + "NodeType": "FunctionNode", + "FunctionSignature": "QRImage.Color.Colors.GetAllColors", + "Id": "b3203366f0c048d8b1804f7b2dc3a6ae", + "Inputs": [], + "Outputs": [ + { + "Id": "f23c1ee57e2140bd834b229eb3f7f6a9", + "Name": "var[]..[]", + "Description": "var[]..[]", + "UsingDefaultValue": false, + "Level": 2, + "UseLevels": false, + "KeepListStructure": false + } + ], + "Replication": "Auto", + "Description": "Return All Color\n\nColors.GetAllColors ( ): var[]..[]" + }, + { + "ConcreteType": "Dynamo.Graph.Nodes.CustomNodes.Function, DynamoCore", + "FunctionSignature": "96dbab16-f1ce-4dfb-8e1c-1ec930eb7507", + "FunctionType": "Graph", + "NodeType": "FunctionNode", + "Id": "6f22a3d1c1294846aef586034146e89d", + "Inputs": [], + "Outputs": [ + { + "Id": "6208c6fc61f04171af7433d1e483acc8", + "Name": "LuckyNumber", + "Description": "return value", + "UsingDefaultValue": false, + "Level": 2, + "UseLevels": false, + "KeepListStructure": false + } + ], + "Replication": "Auto", + "Description": "" + } + ], + "Connectors": [], + "Dependencies": [ + "96dbab16-f1ce-4dfb-8e1c-1ec930eb7507" + ], + "NodeLibraryDependencies": [ + { + "Name": "QRImage", + "Version": "0.0.4", + "ReferenceType": "Package", + "Nodes": [ + "b3203366f0c048d8b1804f7b2dc3a6ae" + ] + } + ], + "Bindings": [], + "View": { + "Dynamo": { + "ScaleFactor": 1.0, + "HasRunWithoutCrash": true, + "IsVisibleInDynamoLibrary": true, + "Version": "2.7.0.8435", + "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": "Colors.GetAllColors", + "Id": "b3203366f0c048d8b1804f7b2dc3a6ae", + "IsSetAsInput": false, + "IsSetAsOutput": false, + "Excluded": false, + "X": 280.5, + "Y": 321.5 + }, + { + "ShowGeometry": true, + "Name": "test", + "Id": "6f22a3d1c1294846aef586034146e89d", + "IsSetAsInput": false, + "IsSetAsOutput": false, + "Excluded": false, + "X": 125.80000000000001, + "Y": 110.80000000000001 + } + ], + "Annotations": [], + "X": 0.0, + "Y": 0.0, + "Zoom": 1.0 + } +} \ No newline at end of file diff --git a/test/core/PackageLoadReset/test.dyf b/test/core/PackageLoadReset/test.dyf new file mode 100644 index 00000000000..34f978d1a0b --- /dev/null +++ b/test/core/PackageLoadReset/test.dyf @@ -0,0 +1,114 @@ +{ + "Uuid": "96dbab16-f1ce-4dfb-8e1c-1ec930eb7507", + "IsCustomNode": true, + "Category": "test", + "Description": "", + "Name": "test", + "ElementResolver": { + "ResolutionMap": {} + }, + "Inputs": [], + "Outputs": [], + "Nodes": [ + { + "ConcreteType": "CoreNodeModels.Input.DoubleInput, CoreNodeModels", + "NodeType": "NumberInputNode", + "NumberType": "Double", + "InputValue": 7.0, + "Id": "e82f21117d8a405385d8231f218d1e98", + "Inputs": [], + "Outputs": [ + { + "Id": "b5f72d10a0e943a5b339accbdefab399", + "Name": "", + "Description": "Double", + "UsingDefaultValue": false, + "Level": 2, + "UseLevels": false, + "KeepListStructure": false + } + ], + "Replication": "Disabled", + "Description": "Creates a number." + }, + { + "ConcreteType": "Dynamo.Graph.Nodes.CustomNodes.Output, DynamoCore", + "NodeType": "OutputNode", + "ElementResolver": null, + "Symbol": "LuckyNumber", + "Id": "c640aa10ffb94f5b97b77b4891a515af", + "Inputs": [ + { + "Id": "308b304db2dc4e74ae1453814c32ea0b", + "Name": "", + "Description": "", + "UsingDefaultValue": false, + "Level": 2, + "UseLevels": false, + "KeepListStructure": false + } + ], + "Outputs": [], + "Replication": "Disabled", + "Description": "A function output, use with custom nodes" + } + ], + "Connectors": [ + { + "Start": "b5f72d10a0e943a5b339accbdefab399", + "End": "308b304db2dc4e74ae1453814c32ea0b", + "Id": "ab4726cfced847f68601875fc653577b" + } + ], + "Dependencies": [], + "NodeLibraryDependencies": [], + "Bindings": [], + "View": { + "Dynamo": { + "ScaleFactor": 1.0, + "HasRunWithoutCrash": false, + "IsVisibleInDynamoLibrary": true, + "Version": "2.7.0.8435", + "RunType": "Manual", + "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": "Number", + "Id": "e82f21117d8a405385d8231f218d1e98", + "IsSetAsInput": false, + "IsSetAsOutput": false, + "Excluded": false, + "X": 117.99999999999989, + "Y": 54.8 + }, + { + "ShowGeometry": true, + "Name": "Output", + "Id": "c640aa10ffb94f5b97b77b4891a515af", + "IsSetAsInput": false, + "IsSetAsOutput": false, + "Excluded": false, + "X": 261.4, + "Y": 61.2 + } + ], + "Annotations": [], + "X": 0.0, + "Y": 0.0, + "Zoom": 1.0 + } +} \ No newline at end of file