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

Fix multiple issues with bindable safety in SkinEditor components #17704

Merged
merged 3 commits into from Apr 7, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 15 additions & 8 deletions osu.Game/Skinning/Editor/SkinBlueprintContainer.cs
Expand Up @@ -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();

Expand All @@ -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)
{
Expand All @@ -79,7 +78,7 @@ private void componentsChanged(object sender, NotifyCollectionChangedEventArgs e
AddBlueprintFor(item);
break;
}
}
});

protected override void AddBlueprintFor(ISkinnableDrawable item)
{
Expand All @@ -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();
}
}
}
3 changes: 3 additions & 0 deletions osu.Game/Skinning/Editor/SkinEditor.cs
Expand Up @@ -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);

Expand Down