[DYN-4517] TuneUp will fail to register if not closed between graph runs (#43)

ivaylo-matov · web-flow · commit 2fafe3065cc0 · 2024-07-02T08:19:54.000-07:00
* TuneUp upgraded to NET8

builds without errors

* .NET8 except binaries

* updated packages

* loads in 3.1+

now loads in all versions from the Extensions menu

* readme

* subscribes only when MenuItem.IsChecked

* Subscribe/unsubscrive called from IsTuneUpActive

* ready for PR

- TuneUp active only when MenuItem.IsChecked
- switches to manual model when activated
- ViewModel.IsTuneUpChecked tracks MenuItem.IsChecked

* conflicts resolved

aligned to .csproj &amp; .props in master
diff --git a/TuneUp/TuneUpViewExtension.cs b/TuneUp/TuneUpViewExtension.cs
@@ -44,14 +44,19 @@ public override void Loaded(ViewLoadedParams p)
                 if (TuneUpMenuItem.IsChecked)
                 {
                     p.AddToExtensionsSideBar(this, TuneUpView);
-                    ViewModel.EnableProfiling();
+                    ViewModel.SwitchToManualMode();
+                    ViewModel.EnableProfiling();                    
                 }
                 else
                 {
                     p.CloseExtensioninInSideBar(this);
                 }
             };
 
+            // Bind the IsChecked property to the IsTuneUpActive property
+            TuneUpMenuItem.Checked += (sender, args) => ViewModel.IsTuneUpChecked = true;
+            TuneUpMenuItem.Unchecked += (sender, args) => ViewModel.IsTuneUpChecked = false;
+
             // Add this view extension's menu item to the Extensions tab or View tab accordingly.
             var dynamoMenuItems = p.dynamoMenu.Items.OfType<MenuItem>();
             // TODO: Investigate why TuneUpMenuItem is not added to the specified MenuItem
diff --git a/TuneUp/TuneUpWindowViewModel.cs b/TuneUp/TuneUpWindowViewModel.cs
@@ -40,6 +40,7 @@ public enum ProfiledNodeState
     public class TuneUpWindowViewModel : NotificationObject, IDisposable
     {
         #region Internal Properties
+
         private ViewLoadedParams viewLoadedParams;
         private IProfilingExecutionTimeData executionTimeData;
         private int executedNodesNum;
@@ -48,6 +49,7 @@ public class TuneUpWindowViewModel : NotificationObject, IDisposable
         private HomeWorkspaceModel currentWorkspace;
         private Dictionary<Guid, ProfiledNodeViewModel> nodeDictionary = new Dictionary<Guid, ProfiledNodeViewModel>();
         private SynchronizationContext uiContext;
+        private bool isTuneUpChecked = false;
 
         /// <summary>
         /// Name of the row to display current execution time
@@ -71,30 +73,31 @@ public class TuneUpWindowViewModel : NotificationObject, IDisposable
 
         private HomeWorkspaceModel CurrentWorkspace
         {
-            get
-            {
-                return currentWorkspace;
-            }
+            get => currentWorkspace;
             set
             {
                 // Unsubscribe from old workspace
-                if (currentWorkspace != null)
+                if (currentWorkspace != null && isTuneUpChecked)
                 {
-                    UnsubscribeWorkspaceEvents(currentWorkspace);
+                    ManageWorkspaceEvents(currentWorkspace, false);
                 }
 
                 // Subscribe to new workspace
                 if (value != null)
                 {
                     // Set new workspace
                     currentWorkspace = value;
-                    SubscribeWorkspaceEvents(currentWorkspace);
+                    if (isTuneUpChecked)
+                    {
+                        ManageWorkspaceEvents(currentWorkspace, true);
+                    }
                 }
             }
         }
         #endregion
 
         #region Public Properties
+        
         /// <summary>
         /// Is the recomputeAll button enabled in the UI. Users should not be able to force a 
         /// reset of the engine and re-execution of the graph if one is still ongoing. This causes...trouble.
@@ -111,6 +114,31 @@ private set
                 }
             }
         }
+
+        /// <summary>
+        /// Gets or sets a value indicating whether the TuneUp extension is active.
+        /// When activated, it subscribes to workspace events to enable profiling. 
+        /// When deactivated, it unsubscribes from those events.
+        /// </summary>
+        public bool IsTuneUpChecked
+        {
+            get => isTuneUpChecked;
+            set
+            {
+                if (isTuneUpChecked != value)
+                {
+                    isTuneUpChecked = value;
+                    RaisePropertyChanged(nameof(IsTuneUpChecked));
+
+                    // Subscribe if activated, unsubscribe if deactivated
+                    if (currentWorkspace != null)
+                    {
+                        ManageWorkspaceEvents(currentWorkspace, isTuneUpChecked);
+                    }
+                }
+            }
+        }
+
         /// <summary>
         /// Collection of profiling data for nodes in the current workspace
         /// </summary>
@@ -153,11 +181,14 @@ public TuneUpWindowViewModel(ViewLoadedParams p)
                 CurrentWorkspace = p.CurrentWorkspaceModel as HomeWorkspaceModel;
             }
         }
-
         #endregion
 
         #region ProfilingMethods
 
+        /// <summary>
+        /// Resets the profiling data for all nodes in the current workspace. Clears the existing
+        /// profiling data and re-initializes it based on the nodes present in the current workspace.
+        /// </summary>
         internal void ResetProfiledNodes()
         {
             if (CurrentWorkspace == null)
@@ -186,13 +217,13 @@ internal void ResetProfiledNodes()
         }
 
         /// <summary>
-        /// The hanlder for force-recompute the graph
+        /// The handler for force-recompute the graph
         /// </summary>
         internal void ResetProfiling()
         {
             // Put the graph into manual mode as there is no guarantee that nodes will be marked
-            // dirty in topologically sorted oreder during a reset.
-            CurrentWorkspace.RunSettings.RunType = Dynamo.Models.RunType.Manual;
+            // dirty in topologically sorted order during a reset.
+            SwitchToManualMode();
             // TODO: need a way to do this from an extension and not cause a run.
             // DynamoModel interface or a more specific reset command.
             (viewLoadedParams.DynamoWindow.DataContext as DynamoViewModel).Model.ResetEngine(true);
@@ -205,6 +236,15 @@ internal void ResetProfiling()
             executionTimeData = CurrentWorkspace.EngineController.ExecutionTimeData;
         }
 
+        /// <summary>
+        /// Switches the current workspace's run mode to manual. Prevents the workspace from 
+        /// running automatically and allows for manual control of execution.
+        /// </summary>
+        internal void SwitchToManualMode()
+        {
+            CurrentWorkspace.RunSettings.RunType = Dynamo.Models.RunType.Manual;
+        }
+
         /// <summary>
         /// Enable profiling when it is disabled temporarily.
         /// </summary>
@@ -357,42 +397,43 @@ private void OnCurrentWorkspaceCleared(IWorkspaceModel workspace)
 
         /// <summary>
         /// When switching workspaces or closing TuneUp extension,
-        /// unsubscribe workspace events for profiling
+        /// subscribe (true)/unsubscribe (false) workspace events for profiling
         /// </summary>
-        /// <param name="workspace">target workspace</param>
-        private void UnsubscribeWorkspaceEvents(HomeWorkspaceModel workspace)
+        /// <param name="workspace"></param>
+        /// <param name="subscribe"></param>
+        private void ManageWorkspaceEvents(HomeWorkspaceModel workspace, bool subscribe)
         {
-            workspace.NodeAdded -= CurrentWorkspaceModel_NodeAdded;
-            workspace.NodeRemoved -= CurrentWorkspaceModel_NodeRemoved;
-            workspace.EvaluationStarted -= CurrentWorkspaceModel_EvaluationStarted;
-            workspace.EvaluationCompleted -= CurrentWorkspaceModel_EvaluationCompleted;
+            if (workspace == null) return;
 
-            foreach (var node in workspace.Nodes)
+            // Subscribe from workspace events 
+            if (subscribe)
             {
-                node.NodeExecutionBegin -= OnNodeExecutionBegin;
-                node.NodeExecutionEnd -= OnNodeExecutionEnd;
-            }
-            executedNodesNum = 0;
-        }
-
-        /// <summary>
-        /// When switching workspaces or closing TuneUp extension,
-        /// subscribe workspace events for profiling
-        /// </summary>
-        /// <param name="workspace">target workspace</param>
-        private void SubscribeWorkspaceEvents(HomeWorkspaceModel workspace)
-        {
-            workspace.NodeAdded += CurrentWorkspaceModel_NodeAdded;
-            workspace.NodeRemoved += CurrentWorkspaceModel_NodeRemoved;
-            workspace.EvaluationStarted += CurrentWorkspaceModel_EvaluationStarted;
-            workspace.EvaluationCompleted += CurrentWorkspaceModel_EvaluationCompleted;
+                workspace.NodeAdded += CurrentWorkspaceModel_NodeAdded;
+                workspace.NodeRemoved += CurrentWorkspaceModel_NodeRemoved;
+                workspace.EvaluationStarted += CurrentWorkspaceModel_EvaluationStarted;
+                workspace.EvaluationCompleted += CurrentWorkspaceModel_EvaluationCompleted;
 
-            foreach (var node in workspace.Nodes)
-            {
-                node.NodeExecutionBegin += OnNodeExecutionBegin;
-                node.NodeExecutionEnd += OnNodeExecutionEnd;
+                foreach (var node in workspace.Nodes)
+                {
+                    node.NodeExecutionBegin += OnNodeExecutionBegin;
+                    node.NodeExecutionEnd += OnNodeExecutionEnd;
+                }
+                ResetProfiledNodes();                
             }
-            ResetProfiledNodes();
+            // Unsubscribe to workspace events
+            else
+            {
+                workspace.NodeAdded -= CurrentWorkspaceModel_NodeAdded;
+                workspace.NodeRemoved -= CurrentWorkspaceModel_NodeRemoved;
+                workspace.EvaluationStarted -= CurrentWorkspaceModel_EvaluationStarted;
+                workspace.EvaluationCompleted -= CurrentWorkspaceModel_EvaluationCompleted;
+
+                foreach (var node in workspace.Nodes)
+                {
+                    node.NodeExecutionBegin -= OnNodeExecutionBegin;
+                    node.NodeExecutionEnd -= OnNodeExecutionEnd;
+                }
+            }            
             executedNodesNum = 0;
         }
 
@@ -401,7 +442,7 @@ private void SubscribeWorkspaceEvents(HomeWorkspaceModel workspace)
         /// </summary>
         public void Dispose()
         {
-            UnsubscribeWorkspaceEvents(CurrentWorkspace);
+            ManageWorkspaceEvents(CurrentWorkspace, false);
             viewLoadedParams.CurrentWorkspaceChanged -= OnCurrentWorkspaceChanged;
             viewLoadedParams.CurrentWorkspaceCleared -= OnCurrentWorkspaceCleared;
         }

Original file line number	Diff line number	Diff line change
`@@ -44,14 +44,19 @@ public override void Loaded(ViewLoadedParams p)`
`44`	`44`	`if (TuneUpMenuItem.IsChecked)`
`45`	`45`	`{`
`46`	`46`	`p.AddToExtensionsSideBar(this, TuneUpView);`
`47`		`- ViewModel.EnableProfiling();`
	`47`	`+ ViewModel.SwitchToManualMode();`
	`48`	`+ ViewModel.EnableProfiling();`
`48`	`49`	`}`
`49`	`50`	`else`
`50`	`51`	`{`
`51`	`52`	`p.CloseExtensioninInSideBar(this);`
`52`	`53`	`}`
`53`	`54`	`};`
`54`	`55`
	`56`	`+ // Bind the IsChecked property to the IsTuneUpActive property`
	`57`	`+ TuneUpMenuItem.Checked += (sender, args) => ViewModel.IsTuneUpChecked = true;`
	`58`	`+ TuneUpMenuItem.Unchecked += (sender, args) => ViewModel.IsTuneUpChecked = false;`
	`59`	`+`
`55`	`60`	`// Add this view extension's menu item to the Extensions tab or View tab accordingly.`
`56`	`61`	`var dynamoMenuItems = p.dynamoMenu.Items.OfType<MenuItem>();`
`57`	`62`	`// TODO: Investigate why TuneUpMenuItem is not added to the specified MenuItem`