Merge pull request #15594 from peppy/beatmap-collection-inteface-types

Add `IBeatmapInfo`/`IBeatmapSetInfo` equality support and update `BeatmapCollection` to use interface types

osu.Game.Tests/NonVisual/BeatmapSetInfoEqualityTest.cs

@@ -3,6 +3,7 @@
 
 using NUnit.Framework;
 using osu.Game.Beatmaps;
+using osu.Game.Extensions;
 
 namespace osu.Game.Tests.NonVisual
 {
@@ -15,7 +16,8 @@ namespace osu.Game.Tests.NonVisual
             var ourInfo = new BeatmapSetInfo { OnlineID = 123 };
             var otherInfo = new BeatmapSetInfo { OnlineID = 123 };
 
-            Assert.AreEqual(ourInfo, otherInfo);
+            Assert.AreNotEqual(ourInfo, otherInfo);
+            Assert.IsTrue(ourInfo.MatchesOnlineID(otherInfo));
         }
 
         [Test]
@@ -33,7 +35,8 @@ namespace osu.Game.Tests.NonVisual
             var ourInfo = new BeatmapSetInfo { ID = 123, OnlineID = 12 };
             var otherInfo = new BeatmapSetInfo { OnlineID = 12 };
 
-            Assert.AreEqual(ourInfo, otherInfo);
+            Assert.AreNotEqual(ourInfo, otherInfo);
+            Assert.IsTrue(ourInfo.MatchesOnlineID(otherInfo));
         }
 
         [Test]

osu.Game/Beatmaps/BeatmapInfo.cs

@@ -154,14 +154,17 @@ namespace osu.Game.Beatmaps
 
         public bool Equals(BeatmapInfo other)
         {
-            if (ID == 0 || other?.ID == 0)
-                // one of the two BeatmapInfos we are comparing isn't sourced from a database.
-                // fall back to reference equality.
-                return ReferenceEquals(this, other);
+            if (ReferenceEquals(this, other)) return true;
+            if (other == null) return false;
 
-            return ID == other?.ID;
+            if (ID != 0 && other.ID != 0)
+                return ID == other.ID;
+
+            return false;
         }
 
+        public bool Equals(IBeatmapInfo other) => other is BeatmapInfo b && Equals(b);
+
         public bool AudioEquals(BeatmapInfo other) => other != null && BeatmapSet != null && other.BeatmapSet != null &&
                                                       BeatmapSet.Hash == other.BeatmapSet.Hash &&
                                                       (Metadata ?? BeatmapSet.Metadata).AudioFile == (other.Metadata ?? other.BeatmapSet.Metadata).AudioFile;

osu.Game/Beatmaps/BeatmapSetInfo.cs

@@ -69,21 +69,17 @@ namespace osu.Game.Beatmaps
 
         public bool Equals(BeatmapSetInfo other)
         {
-            if (other == null)
-                return false;
+            if (ReferenceEquals(this, other)) return true;
+            if (other == null) return false;
 
             if (ID != 0 && other.ID != 0)
                 return ID == other.ID;
 
-            if (OnlineID.HasValue && other.OnlineID.HasValue)
-                return OnlineID == other.OnlineID;
-
-            if (!string.IsNullOrEmpty(Hash) && !string.IsNullOrEmpty(other.Hash))
-                return Hash == other.Hash;
-
-            return ReferenceEquals(this, other);
+            return false;
         }
 
+        public bool Equals(IBeatmapSetInfo other) => other is BeatmapSetInfo b && Equals(b);
+
         #region Implementation of IHasOnlineID
 
         int IHasOnlineID<int>.OnlineID => OnlineID ?? -1;

osu.Game/Beatmaps/IBeatmapInfo.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;
 using osu.Game.Database;
 using osu.Game.Rulesets;
 
@@ -11,7 +12,7 @@ namespace osu.Game.Beatmaps
     /// <summary>
     /// A single beatmap difficulty.
     /// </summary>
-    public interface IBeatmapInfo : IHasOnlineID<int>
+    public interface IBeatmapInfo : IHasOnlineID<int>, IEquatable<IBeatmapInfo>
     {
         /// <summary>
         /// The user-specified name given to this beatmap.

osu.Game/Beatmaps/IBeatmapSetInfo.cs

@@ -12,7 +12,7 @@ namespace osu.Game.Beatmaps
     /// <summary>
     /// A representation of a collection of beatmap difficulties, generally packaged as an ".osz" archive.
     /// </summary>
-    public interface IBeatmapSetInfo : IHasOnlineID<int>
+    public interface IBeatmapSetInfo : IHasOnlineID<int>, IEquatable<IBeatmapSetInfo>
     {
         /// <summary>
         /// The date when this beatmap was imported.

osu.Game/Collections/BeatmapCollection.cs

@@ -25,7 +25,7 @@ namespace osu.Game.Collections
         /// <summary>
         /// The beatmaps contained by the collection.
         /// </summary>
-        public readonly BindableList<BeatmapInfo> Beatmaps = new BindableList<BeatmapInfo>();
+        public readonly BindableList<IBeatmapInfo> Beatmaps = new BindableList<IBeatmapInfo>();
 
         /// <summary>
         /// The date when this collection was last modified.

osu.Game/Collections/CollectionFilterDropdown.cs

@@ -39,7 +39,7 @@ namespace osu.Game.Collections
         }
 
         private readonly IBindableList<BeatmapCollection> collections = new BindableList<BeatmapCollection>();
-        private readonly IBindableList<BeatmapInfo> beatmaps = new BindableList<BeatmapInfo>();
+        private readonly IBindableList<IBeatmapInfo> beatmaps = new BindableList<IBeatmapInfo>();
         private readonly BindableList<CollectionFilterMenuItem> filters = new BindableList<CollectionFilterMenuItem>();
 
         [Resolved(CanBeNull = true)]
@@ -200,7 +200,7 @@ namespace osu.Game.Collections
             private IBindable<WorkingBeatmap> beatmap { get; set; }
 
             [CanBeNull]
-            private readonly BindableList<BeatmapInfo> collectionBeatmaps;
+            private readonly BindableList<IBeatmapInfo> collectionBeatmaps;
 
             [NotNull]
             private readonly Bindable<string> collectionName;

osu.Game/Database/IHasOnlineID.cs

@@ -1,11 +1,14 @@
 // 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;
+
 #nullable enable
 
 namespace osu.Game.Database
 {
     public interface IHasOnlineID<out T>
+        where T : IEquatable<T>
     {
         /// <summary>
         /// The server-side ID representing this instance, if one exists. Any value 0 or less denotes a missing ID (except in special cases where autoincrement is not used, like rulesets).

osu.Game/Extensions/ModelExtensions.cs

@@ -2,10 +2,14 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using osu.Game.Beatmaps;
+using osu.Game.Database;
+using osu.Game.Online.API.Requests.Responses;
 using osu.Game.Rulesets;
 using osu.Game.Scoring;
 using osu.Game.Users;
 
+#nullable enable
+
 namespace osu.Game.Extensions
 {
     public static class ModelExtensions
@@ -22,9 +26,9 @@ namespace osu.Game.Extensions
         /// extension method type inference rules cause this method to call itself and cause a stack overflow.
         /// </para>
         /// </remarks>
-        public static string GetDisplayString(this object model)
+        public static string GetDisplayString(this object? model)
         {
-            string result = null;
+            string? result = null;
 
             switch (model)
             {
@@ -57,5 +61,59 @@ namespace osu.Game.Extensions
             result ??= model?.ToString() ?? @"null";
             return result;
         }
+
+        /// <summary>
+        /// Check whether the online ID of two <see cref="IBeatmapSetInfo"/>s match.
+        /// </summary>
+        /// <param name="instance">The instance to compare.</param>
+        /// <param name="other">The other instance to compare against.</param>
+        /// <returns>Whether online IDs match. If either instance is missing an online ID, this will return false.</returns>
+        public static bool MatchesOnlineID(this IBeatmapSetInfo? instance, IBeatmapSetInfo? other) => matchesOnlineID(instance, other);
+
+        /// <summary>
+        /// Check whether the online ID of two <see cref="IBeatmapInfo"/>s match.
+        /// </summary>
+        /// <param name="instance">The instance to compare.</param>
+        /// <param name="other">The other instance to compare against.</param>
+        /// <returns>Whether online IDs match. If either instance is missing an online ID, this will return false.</returns>
+        public static bool MatchesOnlineID(this IBeatmapInfo? instance, IBeatmapInfo? other) => matchesOnlineID(instance, other);
+
+        /// <summary>
+        /// Check whether the online ID of two <see cref="IRulesetInfo"/>s match.
+        /// </summary>
+        /// <param name="instance">The instance to compare.</param>
+        /// <param name="other">The other instance to compare against.</param>
+        /// <returns>Whether online IDs match. If either instance is missing an online ID, this will return false.</returns>
+        public static bool MatchesOnlineID(this IRulesetInfo? instance, IRulesetInfo? other) => matchesOnlineID(instance, other);
+
+        /// <summary>
+        /// Check whether the online ID of two <see cref="APIUser"/>s match.
+        /// </summary>
+        /// <param name="instance">The instance to compare.</param>
+        /// <param name="other">The other instance to compare against.</param>
+        /// <returns>Whether online IDs match. If either instance is missing an online ID, this will return false.</returns>
+        public static bool MatchesOnlineID(this APIUser? instance, APIUser? other) => matchesOnlineID(instance, other);
+
+        private static bool matchesOnlineID(this IHasOnlineID<long>? instance, IHasOnlineID<long>? other)
+        {
+            if (instance == null || other == null)
+                return false;
+
+            if (instance.OnlineID < 0 || other.OnlineID < 0)
+                return false;
+
+            return instance.OnlineID.Equals(other.OnlineID);
+        }
+
+        private static bool matchesOnlineID(this IHasOnlineID<int>? instance, IHasOnlineID<int>? other)
+        {
+            if (instance == null || other == null)
+                return false;
+
+            if (instance.OnlineID < 0 || other.OnlineID < 0)
+                return false;
+
+            return instance.OnlineID.Equals(other.OnlineID);
+        }
     }
 }

osu.Game/Models/RealmBeatmap.cs

@@ -20,7 +20,7 @@ namespace osu.Game.Models
     [ExcludeFromDynamicCompile]
     [Serializable]
     [MapTo("Beatmap")]
-    public class RealmBeatmap : RealmObject, IHasGuidPrimaryKey, IBeatmapInfo
+    public class RealmBeatmap : RealmObject, IHasGuidPrimaryKey, IBeatmapInfo, IEquatable<RealmBeatmap>
     {
         [PrimaryKey]
         public Guid ID { get; set; } = Guid.NewGuid();
@@ -98,6 +98,16 @@ namespace osu.Game.Models
 
         #endregion
 
+        public bool Equals(RealmBeatmap? other)
+        {
+            if (ReferenceEquals(this, other)) return true;
+            if (other == null) return false;
+
+            return ID == other.ID;
+        }
+
+        public bool Equals(IBeatmapInfo? other) => other is RealmBeatmap b && Equals(b);
+
         public bool AudioEquals(RealmBeatmap? other) => other != null
                                                         && BeatmapSet != null
                                                         && other.BeatmapSet != null

osu.Game/Models/RealmBeatmapSet.cs

@@ -7,6 +7,7 @@ using System.Linq;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
 using osu.Game.Database;
+using osu.Game.Extensions;
 using Realms;
 
 #nullable enable
@@ -53,25 +54,18 @@ namespace osu.Game.Models
         /// <param name="filename">The name of the file to get the storage path of.</param>
         public string? GetPathForFile(string filename) => Files.SingleOrDefault(f => string.Equals(f.Filename, filename, StringComparison.OrdinalIgnoreCase))?.File.StoragePath;
 
-        public override string ToString() => Metadata?.ToString() ?? base.ToString();
-
         public bool Equals(RealmBeatmapSet? other)
         {
-            if (other == null)
-                return false;
+            if (ReferenceEquals(this, other)) return true;
+            if (other == null) return false;
 
-            if (IsManaged && other.IsManaged)
-                return ID == other.ID;
-
-            if (OnlineID > 0 && other.OnlineID > 0)
-                return OnlineID == other.OnlineID;
-
-            if (!string.IsNullOrEmpty(Hash) && !string.IsNullOrEmpty(other.Hash))
-                return Hash == other.Hash;
-
-            return ReferenceEquals(this, other);
+            return ID == other.ID;
         }
 
+        public override string ToString() => Metadata?.GetDisplayString() ?? base.ToString();
+
+        public bool Equals(IBeatmapSetInfo? other) => other is RealmBeatmapSet b && Equals(b);
+
         IEnumerable<IBeatmapInfo> IBeatmapSetInfo.Beatmaps => Beatmaps;
 
         IEnumerable<INamedFileUsage> IBeatmapSetInfo.Files => Files;

osu.Game/Online/API/Requests/Responses/APIBeatmap.cs

@@ -4,6 +4,7 @@
 using System;
 using Newtonsoft.Json;
 using osu.Game.Beatmaps;
+using osu.Game.Extensions;
 using osu.Game.Rulesets;
 
 #nullable enable
@@ -103,5 +104,7 @@ namespace osu.Game.Online.API.Requests.Responses
         public string Hash => throw new NotImplementedException();
 
         #endregion
+
+        public bool Equals(IBeatmapInfo? other) => other is APIBeatmap b && this.MatchesOnlineID(b);
     }
 }

osu.Game/Online/API/Requests/Responses/APIBeatmapSet.cs

@@ -6,6 +6,7 @@ using System.Collections.Generic;
 using Newtonsoft.Json;
 using osu.Game.Beatmaps;
 using osu.Game.Database;
+using osu.Game.Extensions;
 
 #nullable enable
 
@@ -141,5 +142,7 @@ namespace osu.Game.Online.API.Requests.Responses
         double IBeatmapSetInfo.MaxBPM => BPM;
 
         #endregion
+
+        public bool Equals(IBeatmapSetInfo? other) => other is APIBeatmapSet b && this.MatchesOnlineID(b);
     }
 }

osu.Game/Online/API/Requests/Responses/APIUser.cs

@@ -7,6 +7,7 @@ using System.Linq;
 using JetBrains.Annotations;
 using Newtonsoft.Json;
 using osu.Framework.Bindables;
+using osu.Game.Extensions;
 using osu.Game.Users;
 
 namespace osu.Game.Online.API.Requests.Responses
@@ -240,6 +241,6 @@ namespace osu.Game.Online.API.Requests.Responses
 
         public int OnlineID => Id;
 
-        public bool Equals(APIUser other) => OnlineID == other?.OnlineID;
+        public bool Equals(APIUser other) => this.MatchesOnlineID(other);
     }
 }

osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs

@@ -8,6 +8,7 @@ using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Game.Beatmaps;
 using osu.Game.Configuration;
+using osu.Game.Extensions;
 using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Online;
@@ -80,7 +81,7 @@ namespace osu.Game.Overlays.BeatmapListing.Panels
                     case DownloadState.LocallyAvailable:
                         Predicate<BeatmapInfo> findPredicate = null;
                         if (SelectedBeatmap.Value != null)
-                            findPredicate = b => b.OnlineID == SelectedBeatmap.Value.OnlineID;
+                            findPredicate = b => b.MatchesOnlineID(SelectedBeatmap.Value);
 
                         game?.PresentBeatmap(beatmapSet, findPredicate);
                         break;

osu.Game/Overlays/BeatmapSet/BeatmapPicker.cs

@@ -14,6 +14,7 @@ using osu.Framework.Graphics.Sprites;
 using osu.Framework.Input.Events;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.Drawables;
+using osu.Game.Extensions;
 using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.Sprites;
@@ -166,7 +167,7 @@ namespace osu.Game.Overlays.BeatmapSet
             if (BeatmapSet != null)
             {
                 Difficulties.ChildrenEnumerable = BeatmapSet.Beatmaps
-                                                            .Where(b => b.Ruleset.OnlineID == ruleset.Value?.OnlineID)
+                                                            .Where(b => b.Ruleset.MatchesOnlineID(ruleset.Value))
                                                             .OrderBy(b => b.StarRating)
                                                             .Select(b => new DifficultySelectorButton(b)
                                                             {

osu.Game/Overlays/BeatmapSet/BeatmapRulesetTabItem.cs

@@ -7,6 +7,7 @@ using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
+using osu.Game.Extensions;
 using osu.Game.Graphics;
 using osu.Game.Graphics.Sprites;
 using osu.Game.Online.API.Requests.Responses;
@@ -64,7 +65,7 @@ namespace osu.Game.Overlays.BeatmapSet
 
             BeatmapSet.BindValueChanged(setInfo =>
             {
-                int beatmapsCount = setInfo.NewValue?.Beatmaps.Count(b => b.Ruleset.OnlineID == Value.OnlineID) ?? 0;
+                int beatmapsCount = setInfo.NewValue?.Beatmaps.Count(b => b.Ruleset.MatchesOnlineID(Value)) ?? 0;
 
                 count.Text = beatmapsCount.ToString();
                 countContainer.FadeTo(beatmapsCount > 0 ? 1 : 0);

osu.Game/Overlays/Music/Playlist.cs

@@ -29,7 +29,7 @@ namespace osu.Game.Overlays.Music
             var items = (SearchContainer<RearrangeableListItem<BeatmapSetInfo>>)ListContainer;
 
             foreach (var item in items.OfType<PlaylistItem>())
-                item.InSelectedCollection = criteria.Collection?.Beatmaps.Any(b => b.BeatmapSet.Equals(item.Model)) ?? true;
+                item.InSelectedCollection = criteria.Collection?.Beatmaps.Any(b => item.Model.Equals(b.BeatmapSet)) ?? true;
 
             items.SearchTerm = criteria.SearchText;
         }

osu.Game/Screens/OnlinePlay/Lounge/Components/RoomsContainer.cs

@@ -82,7 +82,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge.Components
                 {
                     bool matchingFilter = true;
 
-                    matchingFilter &= r.Room.Playlist.Count == 0 || criteria.Ruleset == null || r.Room.Playlist.Any(i => i.Ruleset.Value.Equals(criteria.Ruleset));
+                    matchingFilter &= r.Room.Playlist.Count == 0 || criteria.Ruleset == null || r.Room.Playlist.Any(i => i.Ruleset.Value.MatchesOnlineID(criteria.Ruleset));
 
                     if (!string.IsNullOrEmpty(criteria.SearchString))
                         matchingFilter &= r.FilterTerms.Any(term => term.Contains(criteria.SearchString, StringComparison.InvariantCultureIgnoreCase));

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

@@ -8,6 +8,7 @@ using System.Threading.Tasks;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Screens;
+using osu.Game.Extensions;
 using osu.Game.Online.Rooms;
 using osu.Game.Rulesets;
 using osu.Game.Scoring;
@@ -32,10 +33,10 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
         private void load(IBindable<RulesetInfo> ruleset)
         {
             // Sanity checks to ensure that PlaylistsPlayer matches the settings for the current PlaylistItem
-            if (Beatmap.Value.BeatmapInfo.OnlineID != PlaylistItem.Beatmap.Value.OnlineID)
+            if (!Beatmap.Value.BeatmapInfo.MatchesOnlineID(PlaylistItem.Beatmap.Value))
                 throw new InvalidOperationException("Current Beatmap does not match PlaylistItem's Beatmap");
 
-            if (ruleset.Value.ID != PlaylistItem.Ruleset.Value.ID)
+            if (!ruleset.Value.MatchesOnlineID(PlaylistItem.Ruleset.Value))
                 throw new InvalidOperationException("Current Ruleset does not match PlaylistItem's Ruleset");
 
             if (!PlaylistItem.RequiredMods.All(m => Mods.Value.Any(m.Equals)))

osu.sln.DotSettings

@@ -209,6 +209,8 @@
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=StructCanBeMadeReadOnly/@EntryIndexedValue">WARNING</s:String>
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuggestVarOrType_005FBuiltInTypes/@EntryIndexedValue">SUGGESTION</s:String>
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuggestVarOrType_005FSimpleTypes/@EntryIndexedValue">DO_NOT_SHOW</s:String>
+	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuspiciousTypeConversion_002EGlobal/@EntryIndexedValue"></s:String>
+	<s:Boolean x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuspiciousTypeConversion_002EGlobal/@EntryIndexRemoved">True</s:Boolean>
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SwitchStatementMissingSomeCases/@EntryIndexedValue">DO_NOT_SHOW</s:String>
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=TabsAndSpacesMismatch/@EntryIndexedValue">WARNING</s:String>
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=TabsAreDisallowed/@EntryIndexedValue">WARNING</s:String>