Merge pull request #8368 from peppy/fix-carousel-restore-previous-selection

Fix carousel not returning to previous selection after filter

osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs

@@ -419,7 +419,7 @@ namespace osu.Game.Tests.Visual.SongSelect
         }
 
         [Test]
-        public void TestCarouselRootIsRandom()
+        public void TestCarouselRemembersSelection()
         {
             List<BeatmapSetInfo> manySets = new List<BeatmapSetInfo>();
 
@@ -429,12 +429,74 @@ namespace osu.Game.Tests.Visual.SongSelect
             loadBeatmaps(manySets);
 
             advanceSelection(direction: 1, diff: false);
-            checkNonmatchingFilter();
-            checkNonmatchingFilter();
-            checkNonmatchingFilter();
-            checkNonmatchingFilter();
-            checkNonmatchingFilter();
-            AddAssert("Selection was random", () => eagerSelectedIDs.Count > 1);
+
+            for (int i = 0; i < 5; i++)
+            {
+                AddStep("Toggle non-matching filter", () =>
+                {
+                    carousel.Filter(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }, false);
+                });
+
+                AddStep("Restore no filter", () =>
+                {
+                    carousel.Filter(new FilterCriteria(), false);
+                    eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID);
+                });
+            }
+
+            // always returns to same selection as long as it's available.
+            AddAssert("Selection was remembered", () => eagerSelectedIDs.Count == 1);
+        }
+
+        [Test]
+        public void TestRandomFallbackOnNonMatchingPrevious()
+        {
+            List<BeatmapSetInfo> manySets = new List<BeatmapSetInfo>();
+
+            AddStep("populate maps", () =>
+            {
+                for (int i = 0; i < 10; i++)
+                {
+                    var set = createTestBeatmapSet(i);
+
+                    foreach (var b in set.Beatmaps)
+                    {
+                        // all taiko except for first
+                        int ruleset = i > 0 ? 1 : 0;
+
+                        b.Ruleset = rulesets.GetRuleset(ruleset);
+                        b.RulesetID = ruleset;
+                    }
+
+                    manySets.Add(set);
+                }
+            });
+
+            loadBeatmaps(manySets);
+
+            for (int i = 0; i < 10; i++)
+            {
+                AddStep("Reset filter", () => carousel.Filter(new FilterCriteria(), false));
+
+                AddStep("select first beatmap", () => carousel.SelectBeatmap(manySets.First().Beatmaps.First()));
+
+                AddStep("Toggle non-matching filter", () =>
+                {
+                    carousel.Filter(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }, false);
+                });
+
+                AddAssert("selection lost", () => carousel.SelectedBeatmap == null);
+
+                AddStep("Restore different ruleset filter", () =>
+                {
+                    carousel.Filter(new FilterCriteria { Ruleset = rulesets.GetRuleset(1) }, false);
+                    eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID);
+                });
+
+                AddAssert("selection changed", () => carousel.SelectedBeatmap != manySets.First().Beatmaps.First());
+            }
+
+            AddAssert("Selection was random", () => eagerSelectedIDs.Count > 2);
         }
 
         [Test]
@@ -593,16 +655,6 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddAssert("Selection is visible", selectedBeatmapVisible);
         }
 
-        private void checkNonmatchingFilter()
-        {
-            AddStep("Toggle non-matching filter", () =>
-            {
-                carousel.Filter(new FilterCriteria { SearchText = "Dingo" }, false);
-                carousel.Filter(new FilterCriteria(), false);
-                eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID);
-            });
-        }
-
         private BeatmapSetInfo createTestBeatmapSet(int id)
         {
             return new BeatmapSetInfo

osu.Game.Tournament.Tests/LadderTestScene.cs

@@ -1,16 +1,147 @@
 // 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.Utils;
+using osu.Game.Beatmaps;
+using osu.Game.Rulesets;
 using osu.Game.Tournament.Models;
+using osu.Game.Users;
 
 namespace osu.Game.Tournament.Tests
 {
     [TestFixture]
     public abstract class LadderTestScene : TournamentTestScene
     {
+        [Cached]
+        protected LadderInfo Ladder { get; private set; } = new LadderInfo();
+
         [Resolved]
-        protected LadderInfo Ladder { get; private set; }
+        private RulesetStore rulesetStore { get; set; }
+
+        [BackgroundDependencyLoader]
+        private void load()
+        {
+            if (Ladder.Ruleset.Value == null)
+                Ladder.Ruleset.Value = rulesetStore.AvailableRulesets.First();
+
+            Ruleset.BindTo(Ladder.Ruleset);
+        }
+
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            TournamentMatch match = CreateSampleMatch();
+
+            Ladder.Rounds.Add(match.Round.Value);
+            Ladder.Matches.Add(match);
+            Ladder.Teams.Add(match.Team1.Value);
+            Ladder.Teams.Add(match.Team2.Value);
+
+            Ladder.CurrentMatch.Value = match;
+        }
+
+        public static TournamentMatch CreateSampleMatch() => new TournamentMatch
+        {
+            Team1 =
+            {
+                Value = new TournamentTeam
+                {
+                    FlagName = { Value = "JP" },
+                    FullName = { Value = "Japan" },
+                    LastYearPlacing = { Value = 10 },
+                    Seed = { Value = "Low" },
+                    SeedingResults =
+                    {
+                        new SeedingResult
+                        {
+                            Mod = { Value = "NM" },
+                            Seed = { Value = 10 },
+                            Beatmaps =
+                            {
+                                new SeedingBeatmap
+                                {
+                                    BeatmapInfo = CreateSampleBeatmapInfo(),
+                                    Score = 12345672,
+                                    Seed = { Value = 24 },
+                                },
+                                new SeedingBeatmap
+                                {
+                                    BeatmapInfo = CreateSampleBeatmapInfo(),
+                                    Score = 1234567,
+                                    Seed = { Value = 12 },
+                                },
+                                new SeedingBeatmap
+                                {
+                                    BeatmapInfo = CreateSampleBeatmapInfo(),
+                                    Score = 1234567,
+                                    Seed = { Value = 16 },
+                                }
+                            }
+                        },
+                        new SeedingResult
+                        {
+                            Mod = { Value = "DT" },
+                            Seed = { Value = 5 },
+                            Beatmaps =
+                            {
+                                new SeedingBeatmap
+                                {
+                                    BeatmapInfo = CreateSampleBeatmapInfo(),
+                                    Score = 234567,
+                                    Seed = { Value = 3 },
+                                },
+                                new SeedingBeatmap
+                                {
+                                    BeatmapInfo = CreateSampleBeatmapInfo(),
+                                    Score = 234567,
+                                    Seed = { Value = 6 },
+                                },
+                                new SeedingBeatmap
+                                {
+                                    BeatmapInfo = CreateSampleBeatmapInfo(),
+                                    Score = 234567,
+                                    Seed = { Value = 12 },
+                                }
+                            }
+                        }
+                    },
+                    Players =
+                    {
+                        new User { Username = "Hello", Statistics = new UserStatistics { Ranks = new UserStatistics.UserRanks { Global = 12 } } },
+                        new User { Username = "Hello", Statistics = new UserStatistics { Ranks = new UserStatistics.UserRanks { Global = 16 } } },
+                        new User { Username = "Hello", Statistics = new UserStatistics { Ranks = new UserStatistics.UserRanks { Global = 20 } } },
+                        new User { Username = "Hello", Statistics = new UserStatistics { Ranks = new UserStatistics.UserRanks { Global = 24 } } },
+                        new User { Username = "Hello", Statistics = new UserStatistics { Ranks = new UserStatistics.UserRanks { Global = 30 } } },
+                    }
+                }
+            },
+            Team2 =
+            {
+                Value = new TournamentTeam
+                {
+                    FlagName = { Value = "US" },
+                    FullName = { Value = "United States" },
+                    Players =
+                    {
+                        new User { Username = "Hello" },
+                        new User { Username = "Hello" },
+                        new User { Username = "Hello" },
+                        new User { Username = "Hello" },
+                        new User { Username = "Hello" },
+                    }
+                }
+            },
+            Round =
+            {
+                Value = new TournamentRound { Name = { Value = "Quarterfinals" } }
+            }
+        };
+
+        public static BeatmapInfo CreateSampleBeatmapInfo() =>
+            new BeatmapInfo { Metadata = new BeatmapMetadata { Title = "Test Title", Artist = "Test Artist", ID = RNG.Next(0, 1000000) } };
     }
 }

osu.Game.Tournament.Tests/Screens/TestSceneMapPoolScreen.cs

@@ -1,24 +1,140 @@
 // 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.Graphics.Containers;
+using osu.Framework.Testing;
+using osu.Game.Tournament.Components;
+using osu.Game.Tournament.Models;
 using osu.Game.Tournament.Screens.MapPool;
 
 namespace osu.Game.Tournament.Tests.Screens
 {
     public class TestSceneMapPoolScreen : LadderTestScene
     {
-        public override IReadOnlyList<Type> RequiredTypes => new[]
-        {
-            typeof(MapPoolScreen)
-        };
+        private MapPoolScreen screen;
 
         [BackgroundDependencyLoader]
         private void load()
         {
-            Add(new MapPoolScreen { Width = 0.7f });
+            Add(screen = new MapPoolScreen { Width = 0.7f });
+        }
+
+        [Test]
+        public void TestFewMaps()
+        {
+            AddStep("load few maps", () =>
+            {
+                Ladder.CurrentMatch.Value.Round.Value.Beatmaps.Clear();
+
+                for (int i = 0; i < 8; i++)
+                    addBeatmap();
+            });
+
+            AddStep("reset match", () =>
+            {
+                Ladder.CurrentMatch.Value = new TournamentMatch();
+                Ladder.CurrentMatch.Value = Ladder.Matches.First();
+            });
+
+            assertTwoWide();
+        }
+
+        [Test]
+        public void TestJustEnoughMaps()
+        {
+            AddStep("load just enough maps", () =>
+            {
+                Ladder.CurrentMatch.Value.Round.Value.Beatmaps.Clear();
+
+                for (int i = 0; i < 18; i++)
+                    addBeatmap();
+            });
+
+            AddStep("reset match", () =>
+            {
+                Ladder.CurrentMatch.Value = new TournamentMatch();
+                Ladder.CurrentMatch.Value = Ladder.Matches.First();
+            });
+
+            assertTwoWide();
+        }
+
+        [Test]
+        public void TestManyMaps()
+        {
+            AddStep("load many maps", () =>
+            {
+                Ladder.CurrentMatch.Value.Round.Value.Beatmaps.Clear();
+
+                for (int i = 0; i < 19; i++)
+                    addBeatmap();
+            });
+
+            AddStep("reset match", () =>
+            {
+                Ladder.CurrentMatch.Value = new TournamentMatch();
+                Ladder.CurrentMatch.Value = Ladder.Matches.First();
+            });
+
+            assertThreeWide();
+        }
+
+        [Test]
+        public void TestJustEnoughMods()
+        {
+            AddStep("load many maps", () =>
+            {
+                Ladder.CurrentMatch.Value.Round.Value.Beatmaps.Clear();
+
+                for (int i = 0; i < 11; i++)
+                    addBeatmap(i > 4 ? $"M{i}" : "NM");
+            });
+
+            AddStep("reset match", () =>
+            {
+                Ladder.CurrentMatch.Value = new TournamentMatch();
+                Ladder.CurrentMatch.Value = Ladder.Matches.First();
+            });
+
+            assertTwoWide();
+        }
+
+        private void assertTwoWide() =>
+            AddAssert("ensure layout width is 2", () => screen.ChildrenOfType<FillFlowContainer<FillFlowContainer<TournamentBeatmapPanel>>>().First().Padding.Left > 0);
+
+        private void assertThreeWide() =>
+            AddAssert("ensure layout width is 3", () => screen.ChildrenOfType<FillFlowContainer<FillFlowContainer<TournamentBeatmapPanel>>>().First().Padding.Left == 0);
+
+        [Test]
+        public void TestManyMods()
+        {
+            AddStep("load many maps", () =>
+            {
+                Ladder.CurrentMatch.Value.Round.Value.Beatmaps.Clear();
+
+                for (int i = 0; i < 12; i++)
+                    addBeatmap(i > 4 ? $"M{i}" : "NM");
+            });
+
+            AddStep("reset match", () =>
+            {
+                Ladder.CurrentMatch.Value = new TournamentMatch();
+                Ladder.CurrentMatch.Value = Ladder.Matches.First();
+            });
+
+            assertThreeWide();
+        }
+
+        private void addBeatmap(string mods = "nm")
+        {
+            Ladder.CurrentMatch.Value.Round.Value.Beatmaps.Add(new RoundBeatmap
+            {
+                BeatmapInfo = CreateSampleBeatmapInfo(),
+                Mods = mods
+            });
         }
     }
 }

osu.Game.Tournament.Tests/Screens/TestSceneSeedingEditorScreen.cs

@@ -14,7 +14,7 @@ namespace osu.Game.Tournament.Tests.Screens
 
         public TestSceneSeedingEditorScreen()
         {
-            var match = TestSceneSeedingScreen.CreateSampleSeededMatch();
+            var match = CreateSampleMatch();
 
             Add(new SeedingEditorScreen(match.Team1.Value)
             {

osu.Game.Tournament.Tests/Screens/TestSceneSeedingScreen.cs

@@ -3,10 +3,8 @@
 
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
-using osu.Game.Beatmaps;
 using osu.Game.Tournament.Models;
 using osu.Game.Tournament.Screens.TeamIntro;
-using osu.Game.Users;
 
 namespace osu.Game.Tournament.Tests.Screens
 {
@@ -18,110 +16,11 @@ namespace osu.Game.Tournament.Tests.Screens
         [BackgroundDependencyLoader]
         private void load()
         {
-            ladder.CurrentMatch.Value = CreateSampleSeededMatch();
-
             Add(new SeedingScreen
             {
                 FillMode = FillMode.Fit,
                 FillAspectRatio = 16 / 9f
             });
         }
-
-        public static TournamentMatch CreateSampleSeededMatch() => new TournamentMatch
-        {
-            Team1 =
-            {
-                Value = new TournamentTeam
-                {
-                    FlagName = { Value = "JP" },
-                    FullName = { Value = "Japan" },
-                    LastYearPlacing = { Value = 10 },
-                    Seed = { Value = "Low" },
-                    SeedingResults =
-                    {
-                        new SeedingResult
-                        {
-                            Mod = { Value = "NM" },
-                            Seed = { Value = 10 },
-                            Beatmaps =
-                            {
-                                new SeedingBeatmap
-                                {
-                                    BeatmapInfo = new BeatmapInfo { Metadata = new BeatmapMetadata { Title = "Test Title", Artist = "Test Artist" } },
-                                    Score = 12345672,
-                                    Seed = { Value = 24 },
-                                },
-                                new SeedingBeatmap
-                                {
-                                    BeatmapInfo = new BeatmapInfo { Metadata = new BeatmapMetadata { Title = "Test Title", Artist = "Test Artist" } },
-                                    Score = 1234567,
-                                    Seed = { Value = 12 },
-                                },
-                                new SeedingBeatmap
-                                {
-                                    BeatmapInfo = new BeatmapInfo { Metadata = new BeatmapMetadata { Title = "Test Title", Artist = "Test Artist" } },
-                                    Score = 1234567,
-                                    Seed = { Value = 16 },
-                                }
-                            }
-                        },
-                        new SeedingResult
-                        {
-                            Mod = { Value = "DT" },
-                            Seed = { Value = 5 },
-                            Beatmaps =
-                            {
-                                new SeedingBeatmap
-                                {
-                                    BeatmapInfo = new BeatmapInfo { Metadata = new BeatmapMetadata { Title = "Test Title", Artist = "Test Artist" } },
-                                    Score = 234567,
-                                    Seed = { Value = 3 },
-                                },
-                                new SeedingBeatmap
-                                {
-                                    BeatmapInfo = new BeatmapInfo { Metadata = new BeatmapMetadata { Title = "Test Title", Artist = "Test Artist" } },
-                                    Score = 234567,
-                                    Seed = { Value = 6 },
-                                },
-                                new SeedingBeatmap
-                                {
-                                    BeatmapInfo = new BeatmapInfo { Metadata = new BeatmapMetadata { Title = "Test Title", Artist = "Test Artist" } },
-                                    Score = 234567,
-                                    Seed = { Value = 12 },
-                                }
-                            }
-                        }
-                    },
-                    Players =
-                    {
-                        new User { Username = "Hello", Statistics = new UserStatistics { Ranks = new UserStatistics.UserRanks { Global = 12 } } },
-                        new User { Username = "Hello", Statistics = new UserStatistics { Ranks = new UserStatistics.UserRanks { Global = 16 } } },
-                        new User { Username = "Hello", Statistics = new UserStatistics { Ranks = new UserStatistics.UserRanks { Global = 20 } } },
-                        new User { Username = "Hello", Statistics = new UserStatistics { Ranks = new UserStatistics.UserRanks { Global = 24 } } },
-                        new User { Username = "Hello", Statistics = new UserStatistics { Ranks = new UserStatistics.UserRanks { Global = 30 } } },
-                    }
-                }
-            },
-            Team2 =
-            {
-                Value = new TournamentTeam
-                {
-                    FlagName = { Value = "US" },
-                    FullName = { Value = "United States" },
-                    Players =
-                    {
-                        new User { Username = "Hello" },
-                        new User { Username = "Hello" },
-                        new User { Username = "Hello" },
-                        new User { Username = "Hello" },
-                        new User { Username = "Hello" },
-                    }
-                }
-            },
-            Round =
-            {
-                Value = new TournamentRound { Name = { Value = "Quarterfinals" } }
-            }
-        };
     }
 }

osu.Game.Tournament/Screens/MapPool/MapPoolScreen.cs

@@ -50,11 +50,11 @@ namespace osu.Game.Tournament.Screens.MapPool
                 new MatchHeader(),
                 mapFlows = new FillFlowContainer<FillFlowContainer<TournamentBeatmapPanel>>
                 {
-                    Y = 140,
+                    Y = 160,
                     Spacing = new Vector2(10, 10),
-                    Padding = new MarginPadding(25),
                     Direction = FillDirection.Vertical,
-                    RelativeSizeAxes = Axes.Both,
+                    RelativeSizeAxes = Axes.X,
+                    AutoSizeAxes = Axes.Y,
                 },
                 new ControlPanel
                 {
@@ -95,6 +95,7 @@ namespace osu.Game.Tournament.Screens.MapPool
                             Text = "Reset",
                             Action = reset
                         },
+                        new ControlPanel.Spacer(),
                     }
                 }
             };
@@ -211,11 +212,15 @@ namespace osu.Game.Tournament.Screens.MapPool
         {
             mapFlows.Clear();
 
+            int totalRows = 0;
+
             if (match.NewValue.Round.Value != null)
             {
                 FillFlowContainer<TournamentBeatmapPanel> currentFlow = null;
                 string currentMod = null;
 
+                int flowCount = 0;
+
                 foreach (var b in match.NewValue.Round.Value.Beatmaps)
                 {
                     if (currentFlow == null || currentMod != b.Mods)
@@ -229,6 +234,15 @@ namespace osu.Game.Tournament.Screens.MapPool
                         });
 
                         currentMod = b.Mods;
+
+                        totalRows++;
+                        flowCount = 0;
+                    }
+
+                    if (++flowCount > 2)
+                    {
+                        totalRows++;
+                        flowCount = 1;
                     }
 
                     currentFlow.Add(new TournamentBeatmapPanel(b.BeatmapInfo, b.Mods)
@@ -239,6 +253,12 @@ namespace osu.Game.Tournament.Screens.MapPool
                     });
                 }
             }
+
+            mapFlows.Padding = new MarginPadding(5)
+            {
+                // remove horizontal padding to increase flow width to 3 panels
+                Horizontal = totalRows > 9 ? 0 : 100
+            };
         }
     }
 }

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -333,8 +333,7 @@ namespace osu.Game.Screens.Select
             else
                 set = visibleSets.ElementAt(RNG.Next(visibleSets.Count));
 
-            var visibleBeatmaps = set.Beatmaps.Where(s => !s.Filtered.Value).ToList();
-            select(visibleBeatmaps[RNG.Next(visibleBeatmaps.Count)]);
+            select(set);
             return true;
         }
 
@@ -756,7 +755,7 @@ namespace osu.Game.Screens.Select
 
             protected override void PerformSelection()
             {
-                if (LastSelected == null)
+                if (LastSelected == null || LastSelected.Filtered.Value)
                     carousel.SelectNextRandom();
                 else
                     base.PerformSelection();

osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs

@@ -104,7 +104,8 @@ namespace osu.Game.Screens.Select.Carousel
 
         private void updateSelected(CarouselItem newSelection)
         {
-            LastSelected = newSelection;
+            if (newSelection != null)
+                LastSelected = newSelection;
             updateSelectedIndex();
         }