From 30815ace62cc0580f9868999c6cf5a3597110638 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 27 Feb 2019 16:52:34 +0900 Subject: [PATCH 1/6] Fix crossthread operations due to Track.Completed --- osu.Game/Audio/PreviewTrack.cs | 2 +- osu.Game/Overlays/MusicController.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Audio/PreviewTrack.cs b/osu.Game/Audio/PreviewTrack.cs index 5de2f2551d..fad4731f18 100644 --- a/osu.Game/Audio/PreviewTrack.cs +++ b/osu.Game/Audio/PreviewTrack.cs @@ -28,7 +28,7 @@ namespace osu.Game.Audio private void load() { track = GetTrack(); - track.Completed += Stop; + track.Completed += () => Schedule(Stop); } /// diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 6a71e91de9..7fbb7ec41e 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -351,11 +351,11 @@ namespace osu.Game.Overlays queuedDirection = null; } - private void currentTrackCompleted() + private void currentTrackCompleted() => Schedule(() => { if (!beatmap.Disabled && beatmapSets.Any()) next(); - } + }); private ScheduledDelegate pendingBeatmapSwitch; From 1006c539be2dff48cef6fb506bb6fbff1669c3b3 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 27 Feb 2019 17:03:09 +0900 Subject: [PATCH 2/6] Fix looping not being checked --- osu.Game/Overlays/MusicController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 7fbb7ec41e..6ac597451d 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -353,7 +353,7 @@ namespace osu.Game.Overlays private void currentTrackCompleted() => Schedule(() => { - if (!beatmap.Disabled && beatmapSets.Any()) + if (!current.Track.Looping && !beatmap.Disabled && beatmapSets.Any()) next(); }); From a8bc87f5d8c8181ce91c8d93c88a4a5ea6db618e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Feb 2019 18:52:39 +0900 Subject: [PATCH 3/6] Add basic skin tests Should be expanded in the future. Bare minimum for now to test fail case. --- .../Visual/TestCaseSkinReloadable.cs | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 osu.Game.Tests/Visual/TestCaseSkinReloadable.cs diff --git a/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs b/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs new file mode 100644 index 0000000000..aac9d2b812 --- /dev/null +++ b/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs @@ -0,0 +1,151 @@ +using System; +using NUnit.Framework; +using osu.Framework.Audio.Sample; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Graphics.Sprites; +using osu.Framework.Graphics.Textures; +using osu.Game.Graphics; +using osu.Game.Skinning; +using osuTK.Graphics; + +namespace osu.Game.Tests.Visual +{ + public class TestCaseSkinReloadable : OsuTestCase + { + + [Test] + public void TestInitialLoad() + { + var secondarySource = new SecondarySource(); + SkinConsumer consumer = null; + + AddStep("setup layout", () => + { + Child = new SkinSourceContainer + { + RelativeSizeAxes = Axes.Both, + Child = new LocalSkinOverrideContainer(secondarySource) + { + RelativeSizeAxes = Axes.Both, + Child = consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true) + } + }; + }); + + AddAssert("consumer using override source", () => consumer.Drawable is SecondarySourceBox); + AddAssert("skinchanged only called once", () => consumer.SkinChangedCount == 1); + } + + [Test] + public void TestOverride() + { + var secondarySource = new SecondarySource(); + + SkinConsumer consumer = null; + Container target = null; + + AddStep("setup layout", () => + { + Child = new SkinSourceContainer + { + RelativeSizeAxes = Axes.Both, + Child = target = new LocalSkinOverrideContainer(secondarySource) + { + RelativeSizeAxes = Axes.Both, + } + }; + }); + + AddStep("add permissive", () => target.Add(consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true))); + AddAssert("consumer using override source", () => consumer.Drawable is SecondarySourceBox); + AddAssert("skinchanged only called once", () => consumer.SkinChangedCount == 1); + } + + private class NamedBox : Container + { + public NamedBox(string name) + { + Children = new Drawable[] + { + new Box + { + Colour = Color4.Black, + RelativeSizeAxes = Axes.Both, + }, + new SpriteText + { + Font = OsuFont.Default.With(size: 40), + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Text = name + } + }; + } + } + + private class SkinConsumer : SkinnableDrawable + { + public new Drawable Drawable => base.Drawable; + public int SkinChangedCount { get; private set; } + + public SkinConsumer(string name, Func defaultImplementation, Func allowFallback = null, bool restrictSize = true) + : base(name, defaultImplementation, allowFallback, restrictSize) + { + } + + protected override void SkinChanged(ISkinSource skin, bool allowFallback) + { + base.SkinChanged(skin, allowFallback); + SkinChangedCount++; + } + } + + private class BaseSourceBox : NamedBox + { + public BaseSourceBox() + : base("Base Source") + { + } + } + + private class SecondarySourceBox : NamedBox + { + public SecondarySourceBox() + : base("Secondary Source") + { + } + } + + private class SecondarySource : ISkinSource + { + public event Action SourceChanged; + + public void TriggerSourceChanged() => SourceChanged?.Invoke(); + + public Drawable GetDrawableComponent(string componentName) => new SecondarySourceBox(); + + public Texture GetTexture(string componentName) => throw new NotImplementedException(); + + public SampleChannel GetSample(string sampleName) => throw new NotImplementedException(); + + public TValue GetValue(Func query) where TConfiguration : SkinConfiguration => throw new NotImplementedException(); + } + + private class SkinSourceContainer : Container, ISkinSource + { + public event Action SourceChanged; + + public void TriggerSourceChanged() => SourceChanged?.Invoke(); + + public Drawable GetDrawableComponent(string componentName) => new BaseSourceBox(); + + public Texture GetTexture(string componentName) => throw new NotImplementedException(); + + public SampleChannel GetSample(string sampleName) => throw new NotImplementedException(); + + public TValue GetValue(Func query) where TConfiguration : SkinConfiguration => throw new NotImplementedException(); + } + } +} From 9473b53226abb8021335fec61d7f786c4da56957 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Feb 2019 14:30:04 +0900 Subject: [PATCH 4/6] Avoid redundant (synchronous) skin change --- .../Skinning/LocalSkinOverrideContainer.cs | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/osu.Game/Skinning/LocalSkinOverrideContainer.cs b/osu.Game/Skinning/LocalSkinOverrideContainer.cs index 36e95f4038..349a4ea9d8 100644 --- a/osu.Game/Skinning/LocalSkinOverrideContainer.cs +++ b/osu.Game/Skinning/LocalSkinOverrideContainer.cs @@ -71,27 +71,20 @@ namespace osu.Game.Skinning var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); fallbackSource = dependencies.Get(); - dependencies.CacheAs(this); - - return dependencies; - } - - [BackgroundDependencyLoader] - private void load(OsuConfigManager config) - { - config.BindWith(OsuSetting.BeatmapSkins, beatmapSkins); - config.BindWith(OsuSetting.BeatmapHitsounds, beatmapHitsounds); - } - - protected override void LoadComplete() - { - base.LoadComplete(); - if (fallbackSource != null) fallbackSource.SourceChanged += onSourceChanged; + dependencies.CacheAs(this); + + var config = dependencies.Get(); + + config.BindWith(OsuSetting.BeatmapSkins, beatmapSkins); + config.BindWith(OsuSetting.BeatmapHitsounds, beatmapHitsounds); + beatmapSkins.BindValueChanged(_ => onSourceChanged()); - beatmapHitsounds.BindValueChanged(_ => onSourceChanged(), true); + beatmapHitsounds.BindValueChanged(_ => onSourceChanged()); + + return dependencies; } protected override void Dispose(bool isDisposing) From b555f793d9be5b51680f2b1b98e894bfbf625003 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Feb 2019 18:57:27 +0900 Subject: [PATCH 5/6] Fix whitespace --- osu.Game.Tests/Visual/TestCaseSkinReloadable.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs b/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs index aac9d2b812..05e1c58768 100644 --- a/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs +++ b/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs @@ -14,7 +14,6 @@ namespace osu.Game.Tests.Visual { public class TestCaseSkinReloadable : OsuTestCase { - [Test] public void TestInitialLoad() { From f942515fcdd4aa319ce94a0d5f63be9a6a0727b3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Feb 2019 18:59:45 +0900 Subject: [PATCH 6/6] Add licence header --- osu.Game.Tests/Visual/TestCaseSkinReloadable.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs b/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs index 05e1c58768..94f01e9d32 100644 --- a/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs +++ b/osu.Game.Tests/Visual/TestCaseSkinReloadable.cs @@ -1,3 +1,6 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + using System; using NUnit.Framework; using osu.Framework.Audio.Sample;