Merge pull request #11342 from bdach/fix-beatmap-not-reverting

Fix user changes in multi song select not reverting on exit without confirmation

osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs

@@ -0,0 +1,145 @@
+// 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;
+using System.Collections.Generic;
+using System.Linq;
+using NUnit.Framework;
+using osu.Framework.Allocation;
+using osu.Framework.Audio;
+using osu.Framework.Extensions;
+using osu.Framework.Platform;
+using osu.Framework.Screens;
+using osu.Framework.Utils;
+using osu.Game.Beatmaps;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Catch;
+using osu.Game.Rulesets.Osu;
+using osu.Game.Rulesets.Osu.Mods;
+using osu.Game.Rulesets.Taiko;
+using osu.Game.Rulesets.Taiko.Mods;
+using osu.Game.Screens.OnlinePlay.Multiplayer;
+using osu.Game.Screens.Select;
+
+namespace osu.Game.Tests.Visual.Multiplayer
+{
+    public class TestSceneMultiplayerMatchSongSelect : RoomTestScene
+    {
+        private BeatmapManager manager;
+        private RulesetStore rulesets;
+
+        private List<BeatmapInfo> beatmaps;
+
+        private TestMultiplayerMatchSongSelect songSelect;
+
+        [BackgroundDependencyLoader]
+        private void load(GameHost host, AudioManager audio)
+        {
+            Dependencies.Cache(rulesets = new RulesetStore(ContextFactory));
+            Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, host, Beatmap.Default));
+
+            beatmaps = new List<BeatmapInfo>();
+
+            for (int i = 0; i < 8; ++i)
+            {
+                int beatmapId = 10 * 10 + i;
+
+                int length = RNG.Next(30000, 200000);
+                double bpm = RNG.NextSingle(80, 200);
+
+                beatmaps.Add(new BeatmapInfo
+                {
+                    Ruleset = rulesets.GetRuleset(i % 4),
+                    OnlineBeatmapID = beatmapId,
+                    Length = length,
+                    BPM = bpm,
+                    BaseDifficulty = new BeatmapDifficulty()
+                });
+            }
+
+            manager.Import(new BeatmapSetInfo
+            {
+                OnlineBeatmapSetID = 10,
+                Hash = Guid.NewGuid().ToString().ComputeMD5Hash(),
+                Metadata = new BeatmapMetadata
+                {
+                    Artist = "Some Artist",
+                    Title = "Some Beatmap",
+                    AuthorString = "Some Author"
+                },
+                Beatmaps = beatmaps,
+                DateAdded = DateTimeOffset.UtcNow
+            }).Wait();
+        }
+
+        public override void SetUpSteps()
+        {
+            base.SetUpSteps();
+
+            AddStep("reset", () =>
+            {
+                Ruleset.Value = new OsuRuleset().RulesetInfo;
+                Beatmap.SetDefault();
+                SelectedMods.SetDefault();
+            });
+
+            AddStep("create song select", () => LoadScreen(songSelect = new TestMultiplayerMatchSongSelect()));
+            AddUntilStep("wait for present", () => songSelect.IsCurrentScreen());
+        }
+
+        [Test]
+        public void TestBeatmapRevertedOnExitIfNoSelection()
+        {
+            BeatmapInfo selectedBeatmap = null;
+
+            AddStep("select beatmap",
+                () => songSelect.Carousel.SelectBeatmap(selectedBeatmap = beatmaps.Where(beatmap => beatmap.RulesetID == new OsuRuleset().LegacyID).ElementAt(1)));
+            AddUntilStep("wait for selection", () => Beatmap.Value.BeatmapInfo.Equals(selectedBeatmap));
+
+            AddStep("exit song select", () => songSelect.Exit());
+            AddAssert("beatmap reverted", () => Beatmap.IsDefault);
+        }
+
+        [Test]
+        public void TestModsRevertedOnExitIfNoSelection()
+        {
+            AddStep("change mods", () => SelectedMods.Value = new[] { new OsuModDoubleTime() });
+
+            AddStep("exit song select", () => songSelect.Exit());
+            AddAssert("mods reverted", () => SelectedMods.Value.Count == 0);
+        }
+
+        [Test]
+        public void TestRulesetRevertedOnExitIfNoSelection()
+        {
+            AddStep("change ruleset", () => Ruleset.Value = new CatchRuleset().RulesetInfo);
+
+            AddStep("exit song select", () => songSelect.Exit());
+            AddAssert("ruleset reverted", () => Ruleset.Value.Equals(new OsuRuleset().RulesetInfo));
+        }
+
+        [Test]
+        public void TestBeatmapConfirmed()
+        {
+            BeatmapInfo selectedBeatmap = null;
+
+            AddStep("change ruleset", () => Ruleset.Value = new TaikoRuleset().RulesetInfo);
+            AddStep("select beatmap",
+                () => songSelect.Carousel.SelectBeatmap(selectedBeatmap = beatmaps.First(beatmap => beatmap.RulesetID == new TaikoRuleset().LegacyID)));
+            AddUntilStep("wait for selection", () => Beatmap.Value.BeatmapInfo.Equals(selectedBeatmap));
+            AddStep("set mods", () => SelectedMods.Value = new[] { new TaikoModDoubleTime() });
+
+            AddStep("confirm selection", () => songSelect.FinaliseSelection());
+            AddStep("exit song select", () => songSelect.Exit());
+
+            AddAssert("beatmap not changed", () => Beatmap.Value.BeatmapInfo.Equals(selectedBeatmap));
+            AddAssert("ruleset not changed", () => Ruleset.Value.Equals(new TaikoRuleset().RulesetInfo));
+            AddAssert("mods not changed", () => SelectedMods.Value.Single() is TaikoModDoubleTime);
+        }
+
+        private class TestMultiplayerMatchSongSelect : MultiplayerMatchSongSelect
+        {
+            public new BeatmapCarousel Carousel => base.Carousel;
+        }
+    }
+}

osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSongSelect.cs

@@ -1,6 +1,7 @@
 // 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.Collections.Generic;
 using System.Linq;
 using Humanizer;
 using osu.Framework.Allocation;
@@ -8,9 +9,12 @@ using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Logging;
 using osu.Framework.Screens;
+using osu.Game.Beatmaps;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Online.Rooms;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Mods;
 using osu.Game.Screens.Select;
 
 namespace osu.Game.Screens.OnlinePlay.Multiplayer
@@ -29,6 +33,12 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
 
         private LoadingLayer loadingLayer;
 
+        private WorkingBeatmap initialBeatmap;
+        private RulesetInfo initialRuleset;
+        private IReadOnlyList<Mod> initialMods;
+
+        private bool itemSelected;
+
         public MultiplayerMatchSongSelect()
         {
             Padding = new MarginPadding { Horizontal = HORIZONTAL_OVERFLOW_PADDING };
@@ -38,10 +48,14 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
         private void load()
         {
             AddInternal(loadingLayer = new LoadingLayer(Carousel));
+            initialBeatmap = Beatmap.Value;
+            initialRuleset = Ruleset.Value;
+            initialMods = Mods.Value.ToList();
         }
 
         protected override bool OnStart()
         {
+            itemSelected = true;
             var item = new PlaylistItem();
 
             item.Beatmap.Value = Beatmap.Value.BeatmapInfo;
@@ -82,6 +96,18 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
             return true;
         }
 
+        public override bool OnExiting(IScreen next)
+        {
+            if (!itemSelected)
+            {
+                Beatmap.Value = initialBeatmap;
+                Ruleset.Value = initialRuleset;
+                Mods.Value = initialMods;
+            }
+
+            return base.OnExiting(next);
+        }
+
         protected override BeatmapDetailArea CreateBeatmapDetailArea() => new PlayBeatmapDetailArea();
     }
 }