From c78d90ccbdc92bb090c9840934ce70986c409a08 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 20 May 2022 14:43:07 +0300 Subject: [PATCH 1/7] Refactor track transferring logic for ability to override and disallow --- osu.Game/Beatmaps/WorkingBeatmap.cs | 16 ++++++++++++---- osu.Game/Overlays/MusicController.cs | 9 +-------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index bb64ec796c..a1a8306b76 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -126,11 +126,19 @@ namespace osu.Game.Beatmaps } /// - /// Transfer a valid audio track into this working beatmap. Used as an optimisation to avoid reload / track swap - /// across difficulties in the same beatmap set. + /// Attempts to transfer the audio track to a target working beatmap, if valid for transferring. + /// Used as an optimisation to avoid reload / track swap across difficulties in the same beatmap set. /// - /// The track to transfer. - public void TransferTrack([NotNull] Track track) => this.track = track ?? throw new ArgumentNullException(nameof(track)); + /// The target working beatmap to transfer this track to. + /// Whether the track is valid and has been transferred to this working beatmap. + public virtual bool TryTransferTrack([NotNull] WorkingBeatmap target) + { + if (BeatmapInfo?.AudioEquals(target.BeatmapInfo) != true) + return false; + + target.track = track; + return true; + } /// /// Get the loaded audio track instance. must have first been called. diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 65b06eb864..509da4302d 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -290,15 +290,8 @@ namespace osu.Game.Overlays current = newWorking; - if (!audioEquals || CurrentTrack.IsDummyDevice) - { + if (lastWorking == null || !lastWorking.TryTransferTrack(current)) changeTrack(); - } - else - { - // transfer still valid track to new working beatmap - current.TransferTrack(lastWorking.Track); - } TrackChanged?.Invoke(current, direction); From cef99fd0205ba17f515a95530adb2ad0a6296bed Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 20 May 2022 14:43:50 +0300 Subject: [PATCH 2/7] Disallow transferring track from test `WorkingBeatmap`s which have local stores --- osu.Game.Tests/WaveformTestBeatmap.cs | 7 +++++++ osu.Game/Tests/Visual/OsuTestScene.cs | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/osu.Game.Tests/WaveformTestBeatmap.cs b/osu.Game.Tests/WaveformTestBeatmap.cs index 9c85fa0c9c..ab7bf7fb73 100644 --- a/osu.Game.Tests/WaveformTestBeatmap.cs +++ b/osu.Game.Tests/WaveformTestBeatmap.cs @@ -59,6 +59,13 @@ namespace osu.Game.Tests protected override Track GetBeatmapTrack() => trackStore.Get(firstAudioFile); + public override bool TryTransferTrack(WorkingBeatmap target) + { + // Our track comes from a local track store that's disposed on finalizer, + // therefore it's unsafe to transfer it to another working beatmap. + return false; + } + private string firstAudioFile { get diff --git a/osu.Game/Tests/Visual/OsuTestScene.cs b/osu.Game/Tests/Visual/OsuTestScene.cs index f2d280417e..1582bdfca4 100644 --- a/osu.Game/Tests/Visual/OsuTestScene.cs +++ b/osu.Game/Tests/Visual/OsuTestScene.cs @@ -373,6 +373,13 @@ namespace osu.Game.Tests.Visual protected override Track GetBeatmapTrack() => track; + public override bool TryTransferTrack(WorkingBeatmap target) + { + // Our track comes from a local track store that's disposed on finalizer, + // therefore it's unsafe to transfer it to another working beatmap. + return false; + } + public class TrackVirtualStore : AudioCollectionManager, ITrackStore { private readonly IFrameBasedClock referenceClock; From abf9039109389dd8b869fed1c2778dedd0c76b5c Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 20 May 2022 14:44:07 +0300 Subject: [PATCH 3/7] Use `==` instead of `??` --- 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 509da4302d..aa09ff6b97 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -267,7 +267,7 @@ namespace osu.Game.Overlays TrackChangeDirection direction = TrackChangeDirection.None; - bool audioEquals = newWorking?.BeatmapInfo?.AudioEquals(current?.BeatmapInfo) ?? false; + bool audioEquals = newWorking?.BeatmapInfo?.AudioEquals(current?.BeatmapInfo) == true; if (current != null) { From 466ed3c791507796151fb504ad353902a71c1f32 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 21 May 2022 16:43:31 +0300 Subject: [PATCH 4/7] Fix wrong return xmldoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game/Beatmaps/WorkingBeatmap.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index a1a8306b76..5551e0b3e5 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -130,7 +130,7 @@ namespace osu.Game.Beatmaps /// Used as an optimisation to avoid reload / track swap across difficulties in the same beatmap set. /// /// The target working beatmap to transfer this track to. - /// Whether the track is valid and has been transferred to this working beatmap. + /// Whether the track has been transferred to the . public virtual bool TryTransferTrack([NotNull] WorkingBeatmap target) { if (BeatmapInfo?.AudioEquals(target.BeatmapInfo) != true) From a42f5ea34ec5c88b002037692029422931a4dcb8 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 21 May 2022 16:50:40 +0300 Subject: [PATCH 5/7] Bring back virtual track condition given its cheapness Will still keep the override in `ClockBackedTestWorkingBeatmap` because it still relies on a local track store and will fail the moment it uses a non-virtual track. --- osu.Game/Beatmaps/WorkingBeatmap.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 5551e0b3e5..5fc579b47c 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -133,7 +133,7 @@ namespace osu.Game.Beatmaps /// Whether the track has been transferred to the . public virtual bool TryTransferTrack([NotNull] WorkingBeatmap target) { - if (BeatmapInfo?.AudioEquals(target.BeatmapInfo) != true) + if (BeatmapInfo?.AudioEquals(target.BeatmapInfo) != true || track.IsDummyDevice) return false; target.track = track; From a17eed64f96c2557bbf632d894694fd57bc1d00c Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 21 May 2022 16:51:04 +0300 Subject: [PATCH 6/7] Use `Track` to ensure its loaded before transferring --- osu.Game/Beatmaps/WorkingBeatmap.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 5fc579b47c..09072ec897 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -133,10 +133,10 @@ namespace osu.Game.Beatmaps /// Whether the track has been transferred to the . public virtual bool TryTransferTrack([NotNull] WorkingBeatmap target) { - if (BeatmapInfo?.AudioEquals(target.BeatmapInfo) != true || track.IsDummyDevice) + if (BeatmapInfo?.AudioEquals(target.BeatmapInfo) != true || Track.IsDummyDevice) return false; - target.track = track; + target.track = Track; return true; } From 21e1576b2fccaf00eaf980265c8b6590103d4767 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 21 May 2022 20:49:29 +0200 Subject: [PATCH 7/7] Fix appearance of sheared button borders after click The border would previously get brighter after click, but then dim instantly when the flash layer has fully faded out. The underlying issue there is https://github.com/ppy/osu-framework/issues/5191, but `ShearedButton` was placing the flashing layer incorrectly anyway, as the intent was that it should also apply to the border. --- .../Graphics/UserInterface/ShearedButton.cs | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/ShearedButton.cs b/osu.Game/Graphics/UserInterface/ShearedButton.cs index dea44e6d99..66c6eedd0c 100644 --- a/osu.Game/Graphics/UserInterface/ShearedButton.cs +++ b/osu.Game/Graphics/UserInterface/ShearedButton.cs @@ -5,6 +5,7 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; +using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; using osu.Framework.Input.Events; using osu.Framework.Localisation; @@ -68,6 +69,7 @@ namespace osu.Game.Graphics.UserInterface private Colour4? lighterColour; private Colour4? textColour; + private readonly Container backgroundLayer; private readonly Box flashLayer; /// @@ -85,24 +87,35 @@ namespace osu.Game.Graphics.UserInterface Height = 50; Padding = new MarginPadding { Horizontal = shear * 50 }; - Content.CornerRadius = 7; + const float corner_radius = 7; + + Content.CornerRadius = corner_radius; Content.Shear = new Vector2(shear, 0); Content.Masking = true; - Content.BorderThickness = 2; Content.Anchor = Content.Origin = Anchor.Centre; Children = new Drawable[] { - background = new Box + backgroundLayer = new Container { - RelativeSizeAxes = Axes.Both - }, - text = new OsuSpriteText - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Font = OsuFont.TorusAlternate.With(size: 17), - Shear = new Vector2(-shear, 0) + RelativeSizeAxes = Axes.Both, + CornerRadius = corner_radius, + Masking = true, + BorderThickness = 2, + Children = new Drawable[] + { + background = new Box + { + RelativeSizeAxes = Axes.Both + }, + text = new OsuSpriteText + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Font = OsuFont.TorusAlternate.With(size: 17), + Shear = new Vector2(-shear, 0) + }, + } }, flashLayer = new Box { @@ -186,7 +199,7 @@ namespace osu.Game.Graphics.UserInterface } background.FadeColour(colourDark, 150, Easing.OutQuint); - Content.TransformTo(nameof(BorderColour), ColourInfo.GradientVertical(colourDark, colourLight), 150, Easing.OutQuint); + backgroundLayer.TransformTo(nameof(BorderColour), ColourInfo.GradientVertical(colourDark, colourLight), 150, Easing.OutQuint); if (!Enabled.Value) colourText = colourText.Opacity(0.6f);