RepoMirrors/osu
1
0
Fork 0
mirror of https://github.com/ppy/osu synced 2024-12-23 23:33:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Move skin deletion logic to OsuGameBase to promote thread safety

Browse Source 
`CurrentSkinInfo` is used in multiple places expecting thread safety,
while ItemRemoved events are explicitly mentioning they are not thread
safe. As SkinManager itself doesn't have the ability to schedule to the
update thread, I've just moved the logic to `OsuGameBase`. We may want
to move the current skin bindable out of the manager class in the
future to match things like `BeatmapManager`.

Closes https://github.com/ppy/osu/issues/10837.
This commit is contained in:
Dean Herbert 2020-11-16 16:43:17 +09:00
parent d8c9db860a
commit 9a7fdb2b7e
2 changed files with 11 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
osu.Game/OsuGameBase.cs
View File

@ -194,6 +194,17 @@ namespace osu.Game
dependencies.Cache(SkinManager = new SkinManager(Storage, contextFactory, Host, Audio, new NamespacedResourceStore<byte[]>(Resources, "Skins/Legacy")));
dependencies.CacheAs<ISkinSource>(SkinManager);
// needs to be done here rather than inside SkinManager to ensure thread safety of CurrentSkinInfo.
SkinManager.ItemRemoved.BindValueChanged(weakRemovedInfo =>
{
if (weakRemovedInfo.NewValue.TryGetTarget(out var removedInfo))
{
// check the removed skin is not the current user choice. if it is, switch back to default.
if (removedInfo.ID == SkinManager.CurrentSkinInfo.Value.ID)
Schedule(() => SkinManager.CurrentSkinInfo.Value = SkinInfo.Default);
}
});
dependencies.CacheAs(API ??= new APIAccess(LocalConfig));
dependencies.CacheAs(spectatorStreaming = new SpectatorStreamingClient());

10
osu.Game/Skinning/SkinManager.cs
View File

@ -48,16 +48,6 @@ namespace osu.Game.Skinning
this.audio = audio;
this.legacyDefaultResources = legacyDefaultResources;
ItemRemoved.BindValueChanged(weakRemovedInfo =>
{
if (weakRemovedInfo.NewValue.TryGetTarget(out var removedInfo))
{
// check the removed skin is not the current user choice. if it is, switch back to default.
if (removedInfo.ID == CurrentSkinInfo.Value.ID)
CurrentSkinInfo.Value = SkinInfo.Default;
}
});
CurrentSkinInfo.ValueChanged += skin => CurrentSkin.Value = GetSkin(skin.NewValue);
CurrentSkin.ValueChanged += skin =>
{