diff --git a/osu.Game.Tests/NonVisual/BeatmapSetInfoEqualityTest.cs b/osu.Game.Tests/NonVisual/BeatmapSetInfoEqualityTest.cs index 938edf07c6..534983f869 100644 --- a/osu.Game.Tests/NonVisual/BeatmapSetInfoEqualityTest.cs +++ b/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] diff --git a/osu.Game/Beatmaps/BeatmapInfo.cs b/osu.Game/Beatmaps/BeatmapInfo.cs index 602bb22c40..6f45f5ec71 100644 --- a/osu.Game/Beatmaps/BeatmapInfo.cs +++ b/osu.Game/Beatmaps/BeatmapInfo.cs @@ -154,12 +154,13 @@ 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 AudioEquals(BeatmapInfo other) => other != null && BeatmapSet != null && other.BeatmapSet != null && diff --git a/osu.Game/Beatmaps/BeatmapSetInfo.cs b/osu.Game/Beatmaps/BeatmapSetInfo.cs index f42e6876e3..6dd8cc5ade 100644 --- a/osu.Game/Beatmaps/BeatmapSetInfo.cs +++ b/osu.Game/Beatmaps/BeatmapSetInfo.cs @@ -69,19 +69,13 @@ 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; } #region Implementation of IHasOnlineID diff --git a/osu.Game/Collections/BeatmapCollection.cs b/osu.Game/Collections/BeatmapCollection.cs index 7e4b15ecf9..1a739f824f 100644 --- a/osu.Game/Collections/BeatmapCollection.cs +++ b/osu.Game/Collections/BeatmapCollection.cs @@ -25,7 +25,7 @@ namespace osu.Game.Collections /// /// The beatmaps contained by the collection. /// - public readonly BindableList Beatmaps = new BindableList(); + public readonly BindableList Beatmaps = new BindableList(); /// /// The date when this collection was last modified. diff --git a/osu.Game/Collections/CollectionFilterDropdown.cs b/osu.Game/Collections/CollectionFilterDropdown.cs index 7067f82fd3..ad23874b2e 100644 --- a/osu.Game/Collections/CollectionFilterDropdown.cs +++ b/osu.Game/Collections/CollectionFilterDropdown.cs @@ -39,7 +39,7 @@ namespace osu.Game.Collections } private readonly IBindableList collections = new BindableList(); - private readonly IBindableList beatmaps = new BindableList(); + private readonly IBindableList beatmaps = new BindableList(); private readonly BindableList filters = new BindableList(); [Resolved(CanBeNull = true)] @@ -200,7 +200,7 @@ namespace osu.Game.Collections private IBindable beatmap { get; set; } [CanBeNull] - private readonly BindableList collectionBeatmaps; + private readonly BindableList collectionBeatmaps; [NotNull] private readonly Bindable collectionName; diff --git a/osu.Game/Database/IHasOnlineID.cs b/osu.Game/Database/IHasOnlineID.cs index 4e83ed8876..7a720989cd 100644 --- a/osu.Game/Database/IHasOnlineID.cs +++ b/osu.Game/Database/IHasOnlineID.cs @@ -1,11 +1,14 @@ // Copyright (c) ppy Pty Ltd . 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 + where T : IEquatable { /// /// 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). diff --git a/osu.Game/Extensions/ModelExtensions.cs b/osu.Game/Extensions/ModelExtensions.cs index d8e0938d46..48a3bac112 100644 --- a/osu.Game/Extensions/ModelExtensions.cs +++ b/osu.Game/Extensions/ModelExtensions.cs @@ -2,10 +2,13 @@ // See the LICENCE file in the repository root for full licence text. using osu.Game.Beatmaps; +using osu.Game.Database; using osu.Game.Rulesets; using osu.Game.Scoring; using osu.Game.Users; +#nullable enable + namespace osu.Game.Extensions { public static class ModelExtensions @@ -22,9 +25,9 @@ namespace osu.Game.Extensions /// extension method type inference rules cause this method to call itself and cause a stack overflow. /// /// - public static string GetDisplayString(this object model) + public static string GetDisplayString(this object? model) { - string result = null; + string? result = null; switch (model) { @@ -57,5 +60,39 @@ namespace osu.Game.Extensions result ??= model?.ToString() ?? @"null"; return result; } + + /// + /// Check whether the online ID of two instances match. + /// + /// The instance to compare. + /// The other instance to compare against. + /// Whether online IDs match. If either instance is missing an online ID, this will return false. + public static bool MatchesOnlineID(this IHasOnlineID? instance, IHasOnlineID? other) + { + if (instance == null || other == null) + return false; + + if (instance.OnlineID < 0 || other.OnlineID < 0) + return false; + + return instance.OnlineID.Equals(other.OnlineID); + } + + /// + /// Check whether the online ID of two instances match. + /// + /// The instance to compare. + /// The other instance to compare against. + /// Whether online IDs match. If either instance is missing an online ID, this will return false. + public static bool MatchesOnlineID(this IHasOnlineID? instance, IHasOnlineID? other) + { + if (instance == null || other == null) + return false; + + if (instance.OnlineID < 0 || other.OnlineID < 0) + return false; + + return instance.OnlineID.Equals(other.OnlineID); + } } } diff --git a/osu.Game/Models/RealmBeatmap.cs b/osu.Game/Models/RealmBeatmap.cs index 9311425cb7..2a197d296a 100644 --- a/osu.Game/Models/RealmBeatmap.cs +++ b/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 { [PrimaryKey] public Guid ID { get; set; } = Guid.NewGuid(); @@ -98,6 +98,14 @@ 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 AudioEquals(RealmBeatmap? other) => other != null && BeatmapSet != null && other.BeatmapSet != null diff --git a/osu.Game/Models/RealmBeatmapSet.cs b/osu.Game/Models/RealmBeatmapSet.cs index 6735510422..1747cce67a 100644 --- a/osu.Game/Models/RealmBeatmapSet.cs +++ b/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,16 @@ namespace osu.Game.Models /// The name of the file to get the storage path of. 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(); + IEnumerable IBeatmapSetInfo.Beatmaps => Beatmaps; IEnumerable IBeatmapSetInfo.Files => Files; diff --git a/osu.Game/Online/API/Requests/Responses/APIUser.cs b/osu.Game/Online/API/Requests/Responses/APIUser.cs index 49edfd036b..50f5d67796 100644 --- a/osu.Game/Online/API/Requests/Responses/APIUser.cs +++ 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); } } diff --git a/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs b/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs index 8c7846783d..1282a14c3d 100644 --- a/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanelDownloadButton.cs +++ b/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 findPredicate = null; if (SelectedBeatmap.Value != null) - findPredicate = b => b.OnlineID == SelectedBeatmap.Value.OnlineID; + findPredicate = b => b.MatchesOnlineID(SelectedBeatmap.Value); game?.PresentBeatmap(beatmapSet, findPredicate); break; diff --git a/osu.Game/Overlays/BeatmapSet/BeatmapPicker.cs b/osu.Game/Overlays/BeatmapSet/BeatmapPicker.cs index b152375062..59e8e8db3c 100644 --- a/osu.Game/Overlays/BeatmapSet/BeatmapPicker.cs +++ b/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) { diff --git a/osu.Game/Overlays/BeatmapSet/BeatmapRulesetTabItem.cs b/osu.Game/Overlays/BeatmapSet/BeatmapRulesetTabItem.cs index b3b3d1980b..e8cdc6913b 100644 --- a/osu.Game/Overlays/BeatmapSet/BeatmapRulesetTabItem.cs +++ 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); diff --git a/osu.Game/Overlays/Music/Playlist.cs b/osu.Game/Overlays/Music/Playlist.cs index 4fe338926f..5de62ebfb8 100644 --- a/osu.Game/Overlays/Music/Playlist.cs +++ b/osu.Game/Overlays/Music/Playlist.cs @@ -7,6 +7,7 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Beatmaps; +using osu.Game.Extensions; using osu.Game.Graphics.Containers; using osuTK; @@ -29,7 +30,7 @@ namespace osu.Game.Overlays.Music var items = (SearchContainer>)ListContainer; foreach (var item in items.OfType()) - item.InSelectedCollection = criteria.Collection?.Beatmaps.Any(b => b.BeatmapSet.Equals(item.Model)) ?? true; + item.InSelectedCollection = criteria.Collection?.Beatmaps.Any(b => b.MatchesOnlineID(item.Model)) ?? true; items.SearchTerm = criteria.SearchText; } diff --git a/osu.Game/Screens/OnlinePlay/Lounge/Components/RoomsContainer.cs b/osu.Game/Screens/OnlinePlay/Lounge/Components/RoomsContainer.cs index 85efdcef1a..32ae7cf859 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/Components/RoomsContainer.cs +++ b/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)); diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsPlayer.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsPlayer.cs index 22537c3ce0..35d417520e 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsPlayer.cs +++ b/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 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))) diff --git a/osu.sln.DotSettings b/osu.sln.DotSettings index f35bdfce66..3fac94b243 100644 --- a/osu.sln.DotSettings +++ b/osu.sln.DotSettings @@ -209,6 +209,8 @@ WARNING SUGGESTION DO_NOT_SHOW + + True DO_NOT_SHOW WARNING WARNING