Merge pull request #16178 from bdach/lounge-background-screen-correct

Fix lounge screen showing information sourced from wrong playlist item

osu.Game.Tests/OnlinePlay/PlaylistExtensionsTest.cs

@@ -0,0 +1,100 @@
+// 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 NUnit.Framework;
+using osu.Game.Online.Rooms;
+
+namespace osu.Game.Tests.OnlinePlay
+{
+    [TestFixture]
+    public class PlaylistExtensionsTest
+    {
+        [Test]
+        public void TestEmpty()
+        {
+            // mostly an extreme edge case, i.e. during room creation.
+            var items = Array.Empty<PlaylistItem>();
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(items.GetHistoricalItems(), Is.Empty);
+                Assert.That(items.GetCurrentItem(), Is.Null);
+                Assert.That(items.GetUpcomingItems(), Is.Empty);
+            });
+        }
+
+        [Test]
+        public void TestPlaylistItemsInOrder()
+        {
+            var items = new[]
+            {
+                new PlaylistItem { ID = 1, BeatmapID = 1001, PlaylistOrder = 1 },
+                new PlaylistItem { ID = 2, BeatmapID = 1002, PlaylistOrder = 2 },
+                new PlaylistItem { ID = 3, BeatmapID = 1003, PlaylistOrder = 3 },
+            };
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(items.GetHistoricalItems(), Is.Empty);
+                Assert.That(items.GetCurrentItem(), Is.EqualTo(items[0]));
+                Assert.That(items.GetUpcomingItems(), Is.EquivalentTo(items));
+            });
+        }
+
+        [Test]
+        public void TestPlaylistItemsOutOfOrder()
+        {
+            var items = new[]
+            {
+                new PlaylistItem { ID = 2, BeatmapID = 1002, PlaylistOrder = 2 },
+                new PlaylistItem { ID = 1, BeatmapID = 1001, PlaylistOrder = 1 },
+                new PlaylistItem { ID = 3, BeatmapID = 1003, PlaylistOrder = 3 },
+            };
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(items.GetHistoricalItems(), Is.Empty);
+                Assert.That(items.GetCurrentItem(), Is.EqualTo(items[1]));
+                Assert.That(items.GetUpcomingItems(), Is.EquivalentTo(new[] { items[1], items[0], items[2] }));
+            });
+        }
+
+        [Test]
+        public void TestExpiredPlaylistItemsSkipped()
+        {
+            var items = new[]
+            {
+                new PlaylistItem { ID = 1, BeatmapID = 1001, Expired = true, PlayedAt = new DateTimeOffset(2021, 12, 21, 7, 55, 0, TimeSpan.Zero) },
+                new PlaylistItem { ID = 2, BeatmapID = 1002, Expired = true, PlayedAt = new DateTimeOffset(2021, 12, 21, 7, 53, 0, TimeSpan.Zero) },
+                new PlaylistItem { ID = 3, BeatmapID = 1003, PlaylistOrder = 3 },
+            };
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(items.GetHistoricalItems(), Is.EquivalentTo(new[] { items[1], items[0] }));
+                Assert.That(items.GetCurrentItem(), Is.EqualTo(items[2]));
+                Assert.That(items.GetUpcomingItems(), Is.EquivalentTo(new[] { items[2] }));
+            });
+        }
+
+        [Test]
+        public void TestAllItemsExpired()
+        {
+            var items = new[]
+            {
+                new PlaylistItem { ID = 1, BeatmapID = 1001, Expired = true, PlayedAt = new DateTimeOffset(2021, 12, 21, 7, 55, 0, TimeSpan.Zero) },
+                new PlaylistItem { ID = 2, BeatmapID = 1002, Expired = true, PlayedAt = new DateTimeOffset(2021, 12, 21, 7, 53, 0, TimeSpan.Zero) },
+                new PlaylistItem { ID = 3, BeatmapID = 1002, Expired = true, PlayedAt = new DateTimeOffset(2021, 12, 21, 7, 57, 0, TimeSpan.Zero) },
+            };
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(items.GetHistoricalItems(), Is.EquivalentTo(new[] { items[1], items[0], items[2] }));
+                // if all items are expired, the last-played item is expected to be returned.
+                Assert.That(items.GetCurrentItem(), Is.EqualTo(items[2]));
+                Assert.That(items.GetUpcomingItems(), Is.Empty);
+            });
+        }
+    }
+}

osu.Game/Online/Rooms/PlaylistExtensions.cs

@@ -1,6 +1,9 @@
 // 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.
 
+#nullable enable
+
+using System.Collections.Generic;
 using System.Linq;
 using Humanizer;
 using Humanizer.Localisation;
@@ -10,6 +13,33 @@ namespace osu.Game.Online.Rooms
 {
     public static class PlaylistExtensions
     {
+        /// <summary>
+        /// Returns all historical/expired items from the <paramref name="playlist"/>, in the order in which they were played.
+        /// </summary>
+        public static IEnumerable<PlaylistItem> GetHistoricalItems(this IEnumerable<PlaylistItem> playlist)
+            => playlist.Where(item => item.Expired).OrderBy(item => item.PlayedAt);
+
+        /// <summary>
+        /// Returns all non-expired items from the <paramref name="playlist"/>, in the order in which they are to be played.
+        /// </summary>
+        public static IEnumerable<PlaylistItem> GetUpcomingItems(this IEnumerable<PlaylistItem> playlist)
+            => playlist.Where(item => !item.Expired).OrderBy(item => item.PlaylistOrder);
+
+        /// <summary>
+        /// Returns the first non-expired <see cref="PlaylistItem"/> in playlist order from the supplied <paramref name="playlist"/>,
+        /// or the last-played <see cref="PlaylistItem"/> if all items are expired,
+        /// or <see langword="null"/> if <paramref name="playlist"/> was empty.
+        /// </summary>
+        public static PlaylistItem? GetCurrentItem(this ICollection<PlaylistItem> playlist)
+        {
+            if (playlist.Count == 0)
+                return null;
+
+            return playlist.All(item => item.Expired)
+                ? GetHistoricalItems(playlist).Last()
+                : GetUpcomingItems(playlist).First();
+        }
+
         public static string GetTotalDuration(this BindableList<PlaylistItem> playlist) =>
             playlist.Select(p => p.Beatmap.Value.Length).Sum().Milliseconds().Humanize(minUnit: TimeUnit.Second, maxUnit: TimeUnit.Hour, precision: 2);
     }

osu.Game/Screens/OnlinePlay/Components/OnlinePlayBackgroundSprite.cs

@@ -1,10 +1,10 @@
 // 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 osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Game.Beatmaps.Drawables;
+using osu.Game.Online.Rooms;
 
 namespace osu.Game.Screens.OnlinePlay.Components
 {
@@ -30,7 +30,7 @@ namespace osu.Game.Screens.OnlinePlay.Components
 
         private void updateBeatmap()
         {
-            sprite.Beatmap.Value = Playlist.FirstOrDefault()?.Beatmap.Value;
+            sprite.Beatmap.Value = Playlist.GetCurrentItem()?.Beatmap.Value;
         }
 
         protected virtual UpdateableBeatmapBackgroundSprite CreateBackgroundSprite() => new UpdateableBeatmapBackgroundSprite(BeatmapSetCoverType) { RelativeSizeAxes = Axes.Both };

osu.Game/Screens/OnlinePlay/Lounge/LoungeBackgroundScreen.cs

@@ -3,7 +3,6 @@
 
 #nullable enable
 
-using System.Linq;
 using osu.Framework.Bindables;
 using osu.Framework.Screens;
 using osu.Game.Online.Rooms;
@@ -20,7 +19,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge
         public LoungeBackgroundScreen()
         {
             SelectedRoom.BindValueChanged(onSelectedRoomChanged);
-            playlist.BindCollectionChanged((_, __) => PlaylistItem = playlist.FirstOrDefault());
+            playlist.BindCollectionChanged((_, __) => PlaylistItem = playlist.GetCurrentItem());
         }
 
         private void onSelectedRoomChanged(ValueChangedEvent<Room> room)

osu.Game/Screens/OnlinePlay/OnlinePlayComposite.cs

@@ -2,7 +2,6 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
-using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics.Containers;
@@ -73,7 +72,7 @@ namespace osu.Game.Screens.OnlinePlay
         private IBindable<PlaylistItem> subScreenSelectedItem { get; set; }
 
         /// <summary>
-        /// The currently selected item in the <see cref="RoomSubScreen"/>, or the last item from <see cref="Playlist"/>
+        /// The currently selected item in the <see cref="RoomSubScreen"/>, or the current item from <see cref="Playlist"/>
         /// if this <see cref="OnlinePlayComposite"/> is not within a <see cref="RoomSubScreen"/>.
         /// </summary>
         protected readonly Bindable<PlaylistItem> SelectedItem = new Bindable<PlaylistItem>();
@@ -88,7 +87,7 @@ namespace osu.Game.Screens.OnlinePlay
 
         protected virtual void UpdateSelectedItem()
             => SelectedItem.Value = RoomID.Value == null || subScreenSelectedItem == null
-                ? Playlist.LastOrDefault()
+                ? Playlist.GetCurrentItem()
                 : subScreenSelectedItem.Value;
     }
 }