From c6ef21f7dc9a575d5ea24c7df35a215dc01e2a00 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Wed, 15 Nov 2017 19:49:49 +0100 Subject: [PATCH 1/2] Fix switching views Switching views was happening in such a way that the new view would not get OnDataUpdate called between OnEnable and OnUI, because OnDataUpdate only gets called during layout events, and we switch the view (calling OnEnable on it) during the onmouseup event. That meant that the newly-active view would not load data until the next ui cycle, which causes exceptions to be thrown, given that the rendering code relies on data to always be there in some way (either cached or empty or whatever, but *something* needs to exist) This ensures that OnDataUpdate gets called on the new view manually. This means that the event type for OnUI is going to be mouseup... which might confuse the new view if it's trying to handle it? I think it's probably fine because by this point the event object has been marked as used, which means other controls that want to handle the event will ignore it. --- .../Editor/GitHub.Unity/UI/BranchesView.cs | 16 ++++------------ .../Editor/GitHub.Unity/UI/SettingsView.cs | 8 ++------ .../Assets/Editor/GitHub.Unity/UI/Window.cs | 12 ++++++------ 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/BranchesView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/BranchesView.cs index 1ef7aa7b2..d5bf1bb56 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/BranchesView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/BranchesView.cs @@ -85,16 +85,13 @@ public override void OnDataUpdate() } private void RepositoryOnLocalAndRemoteBranchListChanged(CacheUpdateEvent cacheUpdateEvent) + { { if (!lastLocalAndRemoteBranchListChangedEvent.Equals(cacheUpdateEvent)) { - new ActionTask(TaskManager.Token, () => - { - lastLocalAndRemoteBranchListChangedEvent = cacheUpdateEvent; - localAndRemoteBranchListHasUpdate = true; - Redraw(); - }) - { Affinity = TaskAffinity.UI }.Start(); + lastLocalAndRemoteBranchListChangedEvent = cacheUpdateEvent; + localAndRemoteBranchListHasUpdate = true; + Redraw(); } } @@ -114,16 +111,11 @@ private void MaybeUpdateData() private void AttachHandlers(IRepository repository) { - if (repository == null) - return; - repository.LocalAndRemoteBranchListChanged += RepositoryOnLocalAndRemoteBranchListChanged; } private void DetachHandlers(IRepository repository) { - if (repository == null) - return; repository.LocalAndRemoteBranchListChanged -= RepositoryOnLocalAndRemoteBranchListChanged; } diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/SettingsView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/SettingsView.cs index 91afd8fe0..5b3052399 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/SettingsView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/SettingsView.cs @@ -51,12 +51,8 @@ public override void OnEnable() userSettingsView.OnEnable(); AttachHandlers(Repository); - if (Repository != null) - { - Repository.CheckCurrentRemoteChangedEvent(lastCurrentRemoteChangedEvent); - Repository.CheckLocksChangedEvent(lastLocksChangedEvent); - } - + Repository.CheckCurrentRemoteChangedEvent(lastCurrentRemoteChangedEvent); + Repository.CheckLocksChangedEvent(lastLocksChangedEvent); metricsHasChanged = true; } diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/Window.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/Window.cs index 1171f90d2..0b2a23af8 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/Window.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/Window.cs @@ -129,9 +129,8 @@ public override void OnRepositoryChanged(IRepository oldRepository) if (Repository != null && activeTab == SubTab.InitProject) { changeTab = SubTab.History; + UpdateActiveTab(); } - - UpdateActiveTab(); } public override void OnSelectionChange() @@ -317,7 +316,6 @@ private void DoToolbarGUI() // Subtabs & toolbar GUILayout.BeginHorizontal(EditorStyles.toolbar); { - changeTab = activeTab; EditorGUI.BeginChangeCheck(); { if (HasRepository) @@ -352,7 +350,8 @@ private void UpdateActiveTab() { var fromView = ActiveView; activeTab = changeTab; - SwitchView(fromView, ActiveView); + var toView = ActiveView; + SwitchView(fromView, toView); } } @@ -363,6 +362,7 @@ private void SwitchView(Subview fromView, Subview toView) if (fromView != null) fromView.OnDisable(); toView.OnEnable(); + toView.OnDataUpdate(); // this triggers a repaint Repaint(); @@ -424,9 +424,9 @@ public void ShowNotification(GUIContent content, float timeout) base.ShowNotification(content); } - private static SubTab TabButton(SubTab tab, string title, SubTab activeTab) + private static SubTab TabButton(SubTab tab, string title, SubTab currentTab) { - return GUILayout.Toggle(activeTab == tab, title, EditorStyles.toolbarButton) ? tab : activeTab; + return GUILayout.Toggle(currentTab == tab, title, EditorStyles.toolbarButton) ? tab : currentTab; } private Subview ToView(SubTab tab) From 86966179adb9be972e33ca6ec57d5c6bcaa20827 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Wed, 15 Nov 2017 19:58:09 +0100 Subject: [PATCH 2/2] Fix bad stash merge --- src/UnityExtension/Assets/Editor/GitHub.Unity/UI/BranchesView.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/BranchesView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/BranchesView.cs index d5bf1bb56..2c4aeeac2 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/BranchesView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/BranchesView.cs @@ -85,7 +85,6 @@ public override void OnDataUpdate() } private void RepositoryOnLocalAndRemoteBranchListChanged(CacheUpdateEvent cacheUpdateEvent) - { { if (!lastLocalAndRemoteBranchListChangedEvent.Equals(cacheUpdateEvent)) {