Stop BackgroundScreenDefault from reloading beatmap background when already correct

osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenDefault.cs

@@ -0,0 +1,59 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Linq;
+using NUnit.Framework;
+using osu.Framework.Allocation;
+using osu.Framework.Screens;
+using osu.Framework.Testing;
+using osu.Game.Configuration;
+using osu.Game.Graphics.Backgrounds;
+using osu.Game.Online.API;
+using osu.Game.Screens;
+using osu.Game.Screens.Backgrounds;
+using osu.Game.Users;
+
+namespace osu.Game.Tests.Visual.Background
+{
+    [TestFixture]
+    public class TestSceneBackgroundScreenDefault : OsuTestScene
+    {
+        private BackgroundScreenStack stack;
+        private BackgroundScreenDefault screen;
+
+        private Graphics.Backgrounds.Background getCurrentBackground() => screen.ChildrenOfType<Graphics.Backgrounds.Background>().FirstOrDefault();
+
+        [Resolved]
+        private OsuConfigManager config { get; set; }
+
+        [SetUpSteps]
+        public void SetUpSteps()
+        {
+            AddStep("create background stack", () => Child = stack = new BackgroundScreenStack());
+            AddStep("push default screen", () => stack.Push(screen = new BackgroundScreenDefault(false)));
+            AddUntilStep("wait for screen to load", () => screen.IsCurrentScreen());
+        }
+
+        [Test]
+        public void TestBeatmapDoesntReloadOnNoChange()
+        {
+            BeatmapBackground last = null;
+
+            setSourceMode(BackgroundSource.Beatmap);
+            setSupporter(true);
+
+            AddUntilStep("wait for beatmap background to be loaded", () => (last = getCurrentBackground() as BeatmapBackground) != null);
+            AddAssert("next doesn't load new background", () => screen.Next() == false);
+
+            // doesn't really need to be checked but might as well.
+            AddWaitStep("wait a bit", 5);
+            AddUntilStep("ensure same background instance", () => last == getCurrentBackground());
+        }
+
+        private void setSourceMode(BackgroundSource source) =>
+            AddStep("set background mode to beatmap", () => config.SetValue(OsuSetting.MenuBackgroundSource, source));
+
+        private void setSupporter(bool isSupporter) =>
+            AddStep("set supporter", () => ((DummyAPIAccess)API).LocalUser.Value = new User { IsSupporter = isSupporter });
+    }
+}

osu.Game/Screens/Backgrounds/BackgroundScreenDefault.cs

@@ -5,8 +5,8 @@ using System.Threading;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
-using osu.Framework.Utils;
 using osu.Framework.Threading;
+using osu.Framework.Utils;
 using osu.Game.Beatmaps;
 using osu.Game.Configuration;
 using osu.Game.Graphics.Backgrounds;
@@ -53,14 +53,41 @@ namespace osu.Game.Screens.Backgrounds
             mode.ValueChanged += _ => Next();
             beatmap.ValueChanged += _ => Next();
             introSequence.ValueChanged += _ => Next();
-            seasonalBackgroundLoader.SeasonalBackgroundChanged += Next;
+            seasonalBackgroundLoader.SeasonalBackgroundChanged += () => Next();
 
             currentDisplay = RNG.Next(0, background_count);
 
             Next();
         }
 
-        private void display(Background newBackground)
+        private ScheduledDelegate nextTask;
+        private CancellationTokenSource cancellationTokenSource;
+
+        /// <summary>
+        /// Request loading the next background.
+        /// </summary>
+        /// <returns>Whether a new background was queued for load. May return false if the current background is still valid.</returns>
+        public bool Next()
+        {
+            var nextBackground = createBackground();
+
+            // in the case that the background hasn't changed, we want to avoid cancelling any tasks that could still be loading.
+            if (nextBackground == background)
+                return false;
+
+            cancellationTokenSource?.Cancel();
+            cancellationTokenSource = new CancellationTokenSource();
+
+            nextTask?.Cancel();
+            nextTask = Scheduler.AddDelayed(() =>
+            {
+                LoadComponentAsync(nextBackground, displayNext, cancellationTokenSource.Token);
+            }, 100);
+
+            return true;
+        }
+
+        private void displayNext(Background newBackground)
         {
             background?.FadeOut(800, Easing.InOutSine);
             background?.Expire();
@@ -69,68 +96,50 @@ namespace osu.Game.Screens.Backgrounds
             currentDisplay++;
         }
 
-        private ScheduledDelegate nextTask;
-        private CancellationTokenSource cancellationTokenSource;
-
-        public void Next()
-        {
-            nextTask?.Cancel();
-            cancellationTokenSource?.Cancel();
-            cancellationTokenSource = new CancellationTokenSource();
-            nextTask = Scheduler.AddDelayed(() => LoadComponentAsync(createBackground(), display, cancellationTokenSource.Token), 100);
-        }
-
         private Background createBackground()
         {
-            Background newBackground;
-            string backgroundName;
+            // seasonal background loading gets highest priority.
+            Background newBackground = seasonalBackgroundLoader.LoadNextBackground();
 
-            var seasonalBackground = seasonalBackgroundLoader.LoadNextBackground();
-
-            if (seasonalBackground != null)
-            {
-                seasonalBackground.Depth = currentDisplay;
-                return seasonalBackground;
-            }
-
-            switch (introSequence.Value)
-            {
-                case IntroSequence.Welcome:
-                    backgroundName = "Intro/Welcome/menu-background";
-                    break;
-
-                default:
-                    backgroundName = $@"Menu/menu-background-{currentDisplay % background_count + 1}";
-                    break;
-            }
-
-            if (user.Value?.IsSupporter ?? false)
+            if (newBackground == null && user.Value?.IsSupporter == true)
             {
                 switch (mode.Value)
                 {
                     case BackgroundSource.Beatmap:
-                        newBackground = new BeatmapBackground(beatmap.Value, backgroundName);
-                        break;
-
                     case BackgroundSource.BeatmapWithStoryboard:
-                        newBackground = AllowStoryboardBackground
-                            ? new BeatmapBackgroundWithStoryboard(beatmap.Value, backgroundName)
-                            : new BeatmapBackground(beatmap.Value, backgroundName);
-                        break;
+                    {
+                        // this method is called in many cases where the beatmap hasn't changed (ie. on screen transitions).
+                        // if a background is already displayed for the requested beatmap, we don't want to load it again.
+                        if ((background as BeatmapBackground)?.Beatmap == beatmap.Value)
+                            return background;
 
-                    default:
-                        newBackground = new SkinnedBackground(skin.Value, backgroundName);
+                        if (mode.Value == BackgroundSource.BeatmapWithStoryboard && AllowStoryboardBackground)
+                            newBackground = new BeatmapBackgroundWithStoryboard(beatmap.Value, getBackgroundTextureName());
+
+                        newBackground ??= new BeatmapBackground(beatmap.Value, getBackgroundTextureName());
                         break;
+                    }
                 }
             }
-            else
-                newBackground = new Background(backgroundName);
 
+            newBackground ??= new Background(getBackgroundTextureName());
             newBackground.Depth = currentDisplay;
 
             return newBackground;
         }
 
+        private string getBackgroundTextureName()
+        {
+            switch (introSequence.Value)
+            {
+                case IntroSequence.Welcome:
+                    return @"Intro/Welcome/menu-background";
+
+                default:
+                    return $@"Menu/menu-background-{currentDisplay % background_count + 1}";
+            }
+        }
+
         private class SkinnedBackground : Background
         {
             private readonly Skin skin;