Merge pull request #17704 from peppy/bindable-safety-skin-editor

Fix multiple issues with bindable safety in `SkinEditor` components
ppy · Apr 7, 2022 · d256bd2 · d256bd2
2 parents 926710b + 3016ce7
commit d256bd2
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 8 deletions.
diff --git a/osu.Game/Skinning/Editor/SkinBlueprintContainer.cs b/osu.Game/Skinning/Editor/SkinBlueprintContainer.cs
@@ -21,21 +21,20 @@ public class SkinBlueprintContainer : BlueprintContainer<ISkinnableDrawable>
 
         private readonly List<BindableList<ISkinnableDrawable>> targetComponents = new List<BindableList<ISkinnableDrawable>>();
 
+        [Resolved]
+        private SkinEditor editor { get; set; }
+
         public SkinBlueprintContainer(Drawable target)
         {
             this.target = target;
         }
 
-        [BackgroundDependencyLoader(true)]
-        private void load(SkinEditor editor)
-        {
-            SelectedItems.BindTo(editor.SelectedComponents);
-        }
-
         protected override void LoadComplete()
         {
             base.LoadComplete();
 
+            SelectedItems.BindTo(editor.SelectedComponents);
+
             // track each target container on the current screen.
             var targetContainers = target.ChildrenOfType<ISkinnableTarget>().ToArray();
 
@@ -56,7 +55,7 @@ protected override void LoadComplete()
             }
         }
 
-        private void componentsChanged(object sender, NotifyCollectionChangedEventArgs e)
+        private void componentsChanged(object sender, NotifyCollectionChangedEventArgs e) => Schedule(() =>
         {
             switch (e.Action)
             {
@@ -79,7 +78,7 @@ private void componentsChanged(object sender, NotifyCollectionChangedEventArgs e
                         AddBlueprintFor(item);
                     break;
             }
-        }
+        });
 
         protected override void AddBlueprintFor(ISkinnableDrawable item)
         {
@@ -93,5 +92,13 @@ protected override void AddBlueprintFor(ISkinnableDrawable item)
 
         protected override SelectionBlueprint<ISkinnableDrawable> CreateBlueprintFor(ISkinnableDrawable component)
             => new SkinBlueprint(component);
+
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+
+            foreach (var list in targetComponents)
+                list.UnbindAll();
+        }
     }
 }
diff --git a/osu.Game/Skinning/Editor/SkinEditor.cs b/osu.Game/Skinning/Editor/SkinEditor.cs
@@ -203,6 +203,9 @@ public void UpdateTargetScreen(Drawable targetScreen)
 
             SelectedComponents.Clear();
 
+            // Immediately clear the previous blueprint container to ensure it doesn't try to interact with the old target.
+            content?.Clear();
+
             Scheduler.AddOnce(loadBlueprintContainer);
             Scheduler.AddOnce(populateSettings);