Merge pull request #15648 from peppy/playlist-show-invalid-beatmaps

Highlight invalid playlist items during room creation

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

@@ -67,6 +67,19 @@ namespace osu.Game.Tests.Visual.Multiplayer
             AddAssert("no item selected", () => playlist.SelectedItem.Value == null);
         }
 
+        [Test]
+        public void TestMarkInvalid()
+        {
+            createPlaylist(true, true);
+
+            AddStep("mark item 0 as invalid", () => playlist.Items[0].MarkInvalid());
+
+            moveToItem(0);
+
+            AddStep("click", () => InputManager.Click(MouseButton.Left));
+            AddAssert("no item selected", () => playlist.SelectedItem.Value == null);
+        }
+
         [Test]
         public void TestSelectable()
         {

osu.Game.Tests/Visual/Playlists/TestScenePlaylistsMatchSettingsOverlay.cs

@@ -73,7 +73,7 @@ namespace osu.Game.Tests.Visual.Playlists
                 RoomManager.CreateRequested = r =>
                 {
                     createdRoom = r;
-                    return true;
+                    return string.Empty;
                 };
             });
 
@@ -82,28 +82,58 @@ namespace osu.Game.Tests.Visual.Playlists
             AddAssert("has correct duration", () => createdRoom.Duration.Value == expectedDuration);
         }
 
+        [Test]
+        public void TestInvalidBeatmapError()
+        {
+            const string not_found_prefix = "beatmaps not found:";
+
+            string errorMesage = null;
+
+            AddStep("setup", () =>
+            {
+                var beatmap = CreateBeatmap(Ruleset.Value).BeatmapInfo;
+
+                SelectedRoom.Value.Name.Value = "Test Room";
+                SelectedRoom.Value.Playlist.Add(new PlaylistItem { Beatmap = { Value = beatmap } });
+
+                errorMesage = $"{not_found_prefix} {beatmap.OnlineID}";
+
+                RoomManager.CreateRequested = _ => errorMesage;
+            });
+
+            AddAssert("error not displayed", () => !settings.ErrorText.IsPresent);
+            AddAssert("playlist item valid", () => SelectedRoom.Value.Playlist[0].Valid.Value);
+
+            AddStep("create room", () => settings.ApplyButton.Action.Invoke());
+
+            AddAssert("error displayed", () => settings.ErrorText.IsPresent);
+            AddAssert("error has custom text", () => settings.ErrorText.Text != errorMesage);
+            AddAssert("playlist item marked invalid", () => !SelectedRoom.Value.Playlist[0].Valid.Value);
+        }
+
         [Test]
         public void TestCreationFailureDisplaysError()
         {
-            bool fail;
+            const string error_message = "failed";
+
+            string failText = error_message;
 
             AddStep("setup", () =>
             {
                 SelectedRoom.Value.Name.Value = "Test Room";
                 SelectedRoom.Value.Playlist.Add(new PlaylistItem { Beatmap = { Value = CreateBeatmap(Ruleset.Value).BeatmapInfo } });
 
-                fail = true;
+                RoomManager.CreateRequested = _ => failText;
-                RoomManager.CreateRequested = _ => !fail;
             });
             AddAssert("error not displayed", () => !settings.ErrorText.IsPresent);
 
             AddStep("create room", () => settings.ApplyButton.Action.Invoke());
             AddAssert("error displayed", () => settings.ErrorText.IsPresent);
-            AddAssert("error has correct text", () => settings.ErrorText.Text == TestRoomManager.FAILED_TEXT);
+            AddAssert("error has correct text", () => settings.ErrorText.Text == error_message);
 
             AddStep("create room no fail", () =>
             {
-                fail = false;
+                failText = string.Empty;
                 settings.ApplyButton.Action.Invoke();
             });
 
@@ -132,9 +162,7 @@ namespace osu.Game.Tests.Visual.Playlists
 
         protected class TestRoomManager : IRoomManager
         {
-            public const string FAILED_TEXT = "failed";
+            public Func<Room, string> CreateRequested;
-
-            public Func<Room, bool> CreateRequested;
 
             public event Action RoomsUpdated
             {
@@ -157,8 +185,10 @@ namespace osu.Game.Tests.Visual.Playlists
                 if (CreateRequested == null)
                     return;
 
-                if (!CreateRequested.Invoke(room))
+                string error = CreateRequested.Invoke(room);
-                    onError?.Invoke(FAILED_TEXT);
+
+                if (!string.IsNullOrEmpty(error))
+                    onError?.Invoke(error);
                 else
                     onSuccess?.Invoke(room);
             }

osu.Game/Online/Rooms/PlaylistItem.cs

@@ -30,6 +30,11 @@ namespace osu.Game.Online.Rooms
         [JsonProperty("expired")]
         public bool Expired { get; set; }
 
+        [JsonIgnore]
+        public IBindable<bool> Valid => valid;
+
+        private readonly Bindable<bool> valid = new BindableBool(true);
+
         [JsonIgnore]
         public readonly Bindable<IBeatmapInfo> Beatmap = new Bindable<IBeatmapInfo>();
 
@@ -69,6 +74,8 @@ namespace osu.Game.Online.Rooms
             Ruleset.BindValueChanged(ruleset => RulesetID = ruleset.NewValue?.OnlineID ?? 0);
         }
 
+        public void MarkInvalid() => valid.Value = false;
+
         public void MapObjects(RulesetStore rulesets)
         {
             Beatmap.Value ??= apiBeatmap;

osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs

@@ -13,7 +13,6 @@ using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Input.Events;
-using osu.Framework.Threading;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.Drawables;
 using osu.Game.Graphics;
@@ -45,6 +44,8 @@ namespace osu.Game.Screens.OnlinePlay
         private ExplicitContentBeatmapPill explicitContentPill;
         private ModDisplay modDisplay;
 
+        private readonly IBindable<bool> valid = new Bindable<bool>();
+
         private readonly Bindable<IBeatmapInfo> beatmap = new Bindable<IBeatmapInfo>();
         private readonly Bindable<IRulesetInfo> ruleset = new Bindable<IRulesetInfo>();
         private readonly BindableList<Mod> requiredMods = new BindableList<Mod>();
@@ -66,14 +67,18 @@ namespace osu.Game.Screens.OnlinePlay
             this.allowSelection = allowSelection;
 
             beatmap.BindTo(item.Beatmap);
+            valid.BindTo(item.Valid);
             ruleset.BindTo(item.Ruleset);
             requiredMods.BindTo(item.RequiredMods);
 
             ShowDragHandle.Value = allowEdit;
         }
 
+        [Resolved]
+        private OsuColour colours { get; set; }
+
         [BackgroundDependencyLoader]
-        private void load(OsuColour colours)
+        private void load()
         {
             if (!allowEdit)
                 HandleColour = HandleColour.Opacity(0);
@@ -85,27 +90,43 @@ namespace osu.Game.Screens.OnlinePlay
         {
             base.LoadComplete();
 
-            SelectedItem.BindValueChanged(selected => maskingContainer.BorderThickness = selected.NewValue == Model ? 5 : 0, true);
+            SelectedItem.BindValueChanged(selected =>
+            {
+                bool isCurrent = selected.NewValue == Model;
 
-            beatmap.BindValueChanged(_ => scheduleRefresh());
+                if (!valid.Value)
-            ruleset.BindValueChanged(_ => scheduleRefresh());
+                {
+                    // Don't allow selection when not valid.
+                    if (isCurrent)
+                    {
+                        SelectedItem.Value = selected.OldValue;
+                    }
 
-            requiredMods.CollectionChanged += (_, __) => scheduleRefresh();
+                    // Don't update border when not valid (the border is displaying this fact).
+                    return;
+                }
+
+                maskingContainer.BorderThickness = isCurrent ? 5 : 0;
+            }, true);
+
+            beatmap.BindValueChanged(_ => Scheduler.AddOnce(refresh));
+            ruleset.BindValueChanged(_ => Scheduler.AddOnce(refresh));
+            valid.BindValueChanged(_ => Scheduler.AddOnce(refresh));
+            requiredMods.CollectionChanged += (_, __) => Scheduler.AddOnce(refresh);
 
             refresh();
         }
 
-        private ScheduledDelegate scheduledRefresh;
         private PanelBackground panelBackground;
 
-        private void scheduleRefresh()
-        {
-            scheduledRefresh?.Cancel();
-            scheduledRefresh = Schedule(refresh);
-        }
-
         private void refresh()
         {
+            if (!valid.Value)
+            {
+                maskingContainer.BorderThickness = 5;
+                maskingContainer.BorderColour = colours.Red;
+            }
+
             difficultyIconContainer.Child = new DifficultyIcon(Item.Beatmap.Value, ruleset.Value, requiredMods, performBackgroundDifficultyLookup: false) { Size = new Vector2(32) };
 
             panelBackground.Beatmap.Value = Item.Beatmap.Value;
@@ -278,7 +299,7 @@ namespace osu.Game.Screens.OnlinePlay
 
         protected override bool OnClick(ClickEvent e)
         {
-            if (allowSelection)
+            if (allowSelection && valid.Value)
                 SelectedItem.Value = Model;
             return true;
         }

osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Specialized;
+using System.Linq;
 using Humanizer;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
@@ -204,7 +205,7 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
                                                                 {
                                                                     new Drawable[]
                                                                     {
-                                                                        playlist = new DrawableRoomPlaylist(true, true) { RelativeSizeAxes = Axes.Both }
+                                                                        playlist = new DrawableRoomPlaylist(true, false) { RelativeSizeAxes = Axes.Both }
                                                                     },
                                                                     new Drawable[]
                                                                     {
@@ -339,9 +340,8 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
 
                 Duration.Value = DurationField.Current.Value;
 
-                manager?.CreateRoom(room, onSuccess, onError);
-
                 loadingLayer.Show();
+                manager?.CreateRoom(room, onSuccess, onError);
             }
 
             private void hideError() => ErrorText.FadeOut(50);
@@ -350,9 +350,31 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
 
             private void onError(string text)
             {
-                ErrorText.Text = text;
+                // see https://github.com/ppy/osu-web/blob/2c97aaeb64fb4ed97c747d8383a35b30f57428c7/app/Models/Multiplayer/PlaylistItem.php#L48.
-                ErrorText.FadeIn(50);
+                const string not_found_prefix = "beatmaps not found:";
 
+                if (text.StartsWith(not_found_prefix, StringComparison.Ordinal))
+                {
+                    ErrorText.Text = "One or more beatmaps were not available online. Please remove or replace the highlighted items.";
+
+                    int[] invalidBeatmapIDs = text
+                                              .Substring(not_found_prefix.Length + 1)
+                                              .Split(", ")
+                                              .Select(int.Parse)
+                                              .ToArray();
+
+                    foreach (var item in Playlist)
+                    {
+                        if (invalidBeatmapIDs.Contains(item.BeatmapID))
+                            item.MarkInvalid();
+                    }
+                }
+                else
+                {
+                    ErrorText.Text = text;
+                }
+
+                ErrorText.FadeIn(50);
                 loadingLayer.Hide();
             }
         }