From 447371478ea6f4843b57a409ebbfab1db1d15e78 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 14 May 2021 12:03:06 +0900 Subject: [PATCH 01/22] Switch null assignment to non-nullable warnings on --- osu.sln.DotSettings | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.sln.DotSettings b/osu.sln.DotSettings index aa8f8739c1..4ac796ccd0 100644 --- a/osu.sln.DotSettings +++ b/osu.sln.DotSettings @@ -18,7 +18,7 @@ WARNING HINT DO_NOT_SHOW - HINT + WARNING WARNING WARNING WARNING From b36c991ba1ca714d9b47e34e705fba071e477e22 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 14 May 2021 12:04:38 +0900 Subject: [PATCH 02/22] Fix single case of incorrect usage --- osu.Game/Screens/Edit/Editor.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 434683a016..5ac3401720 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -635,6 +635,9 @@ namespace osu.Game.Screens.Edit case EditorScreenMode.Verify: currentScreen = new VerifyScreen(); break; + + default: + throw new InvalidOperationException("Editor menu bar switched to an unsupported mode"); } LoadComponentAsync(currentScreen, newScreen => From 32ff40628973ff325c213cd51a32f2324b722df6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 14 May 2021 15:40:29 +0900 Subject: [PATCH 03/22] Add database tracking of beatmap creator `user_id`s --- ...BeatmapManager_BeatmapOnlineLookupQueue.cs | 26 +- osu.Game/Beatmaps/BeatmapMetadata.cs | 21 +- ...9_AddAuthorIdToBeatmapMetadata.Designer.cs | 511 ++++++++++++++++++ ...0514062639_AddAuthorIdToBeatmapMetadata.cs | 23 + .../Migrations/OsuDbContextModelSnapshot.cs | 3 + 5 files changed, 570 insertions(+), 14 deletions(-) create mode 100644 osu.Game/Migrations/20210514062639_AddAuthorIdToBeatmapMetadata.Designer.cs create mode 100644 osu.Game/Migrations/20210514062639_AddAuthorIdToBeatmapMetadata.cs diff --git a/osu.Game/Beatmaps/BeatmapManager_BeatmapOnlineLookupQueue.cs b/osu.Game/Beatmaps/BeatmapManager_BeatmapOnlineLookupQueue.cs index 7c4b344c9e..5dff4fe282 100644 --- a/osu.Game/Beatmaps/BeatmapManager_BeatmapOnlineLookupQueue.cs +++ b/osu.Game/Beatmaps/BeatmapManager_BeatmapOnlineLookupQueue.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; using System.Threading; @@ -83,6 +82,12 @@ namespace osu.Game.Beatmaps beatmap.BeatmapSet.OnlineBeatmapSetID = res.OnlineBeatmapSetID; beatmap.OnlineBeatmapID = res.OnlineBeatmapID; + if (beatmap.Metadata != null) + beatmap.Metadata.AuthorID = res.AuthorID; + + if (beatmap.BeatmapSet.Metadata != null) + beatmap.BeatmapSet.Metadata.AuthorID = res.AuthorID; + LogForModel(set, $"Online retrieval mapped {beatmap} to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}."); } } @@ -157,7 +162,7 @@ namespace osu.Game.Beatmaps using (var cmd = db.CreateCommand()) { - cmd.CommandText = "SELECT beatmapset_id, beatmap_id, approved FROM osu_beatmaps WHERE checksum = @MD5Hash OR beatmap_id = @OnlineBeatmapID OR filename = @Path"; + cmd.CommandText = "SELECT beatmapset_id, beatmap_id, approved, user_id FROM osu_beatmaps WHERE checksum = @MD5Hash OR beatmap_id = @OnlineBeatmapID OR filename = @Path"; cmd.Parameters.Add(new SqliteParameter("@MD5Hash", beatmap.MD5Hash)); cmd.Parameters.Add(new SqliteParameter("@OnlineBeatmapID", beatmap.OnlineBeatmapID ?? (object)DBNull.Value)); @@ -174,6 +179,12 @@ namespace osu.Game.Beatmaps beatmap.BeatmapSet.OnlineBeatmapSetID = reader.GetInt32(0); beatmap.OnlineBeatmapID = reader.GetInt32(1); + if (beatmap.Metadata != null) + beatmap.Metadata.AuthorID = reader.GetInt32(3); + + if (beatmap.BeatmapSet.Metadata != null) + beatmap.BeatmapSet.Metadata.AuthorID = reader.GetInt32(3); + LogForModel(set, $"Cached local retrieval for {beatmap}."); return true; } @@ -194,17 +205,6 @@ namespace osu.Game.Beatmaps cacheDownloadRequest?.Dispose(); updateScheduler?.Dispose(); } - - [Serializable] - [SuppressMessage("ReSharper", "InconsistentNaming")] - private class CachedOnlineBeatmapLookup - { - public int approved { get; set; } - - public int? beatmapset_id { get; set; } - - public int? beatmap_id { get; set; } - } } } } diff --git a/osu.Game/Beatmaps/BeatmapMetadata.cs b/osu.Game/Beatmaps/BeatmapMetadata.cs index 858da8e602..9540a216fc 100644 --- a/osu.Game/Beatmaps/BeatmapMetadata.cs +++ b/osu.Game/Beatmaps/BeatmapMetadata.cs @@ -34,6 +34,21 @@ namespace osu.Game.Beatmaps [JsonIgnore] public List BeatmapSets { get; set; } + /// + /// Helper property to deserialize a username to . + /// + [JsonProperty(@"user_id")] + [Column("AuthorID")] + public int AuthorID + { + get => Author?.Id ?? 1; + set + { + Author ??= new User(); + Author.Id = value; + } + } + /// /// Helper property to deserialize a username to . /// @@ -42,7 +57,11 @@ namespace osu.Game.Beatmaps public string AuthorString { get => Author?.Username; - set => Author = new User { Username = value }; + set + { + Author ??= new User(); + Author.Username = value; + } } /// diff --git a/osu.Game/Migrations/20210514062639_AddAuthorIdToBeatmapMetadata.Designer.cs b/osu.Game/Migrations/20210514062639_AddAuthorIdToBeatmapMetadata.Designer.cs new file mode 100644 index 0000000000..89bab3a0fa --- /dev/null +++ b/osu.Game/Migrations/20210514062639_AddAuthorIdToBeatmapMetadata.Designer.cs @@ -0,0 +1,511 @@ +// +using System; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; +using osu.Game.Database; + +namespace osu.Game.Migrations +{ + [DbContext(typeof(OsuDbContext))] + [Migration("20210514062639_AddAuthorIdToBeatmapMetadata")] + partial class AddAuthorIdToBeatmapMetadata + { + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasAnnotation("ProductVersion", "2.2.6-servicing-10079"); + + modelBuilder.Entity("osu.Game.Beatmaps.BeatmapDifficulty", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("ApproachRate"); + + b.Property("CircleSize"); + + b.Property("DrainRate"); + + b.Property("OverallDifficulty"); + + b.Property("SliderMultiplier"); + + b.Property("SliderTickRate"); + + b.HasKey("ID"); + + b.ToTable("BeatmapDifficulty"); + }); + + modelBuilder.Entity("osu.Game.Beatmaps.BeatmapInfo", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("AudioLeadIn"); + + b.Property("BPM"); + + b.Property("BaseDifficultyID"); + + b.Property("BeatDivisor"); + + b.Property("BeatmapSetInfoID"); + + b.Property("Countdown"); + + b.Property("DistanceSpacing"); + + b.Property("EpilepsyWarning"); + + b.Property("GridSize"); + + b.Property("Hash"); + + b.Property("Hidden"); + + b.Property("Length"); + + b.Property("LetterboxInBreaks"); + + b.Property("MD5Hash"); + + b.Property("MetadataID"); + + b.Property("OnlineBeatmapID"); + + b.Property("Path"); + + b.Property("RulesetID"); + + b.Property("SpecialStyle"); + + b.Property("StackLeniency"); + + b.Property("StarDifficulty"); + + b.Property("Status"); + + b.Property("StoredBookmarks"); + + b.Property("TimelineZoom"); + + b.Property("Version"); + + b.Property("WidescreenStoryboard"); + + b.HasKey("ID"); + + b.HasIndex("BaseDifficultyID"); + + b.HasIndex("BeatmapSetInfoID"); + + b.HasIndex("Hash"); + + b.HasIndex("MD5Hash"); + + b.HasIndex("MetadataID"); + + b.HasIndex("OnlineBeatmapID") + .IsUnique(); + + b.HasIndex("RulesetID"); + + b.ToTable("BeatmapInfo"); + }); + + modelBuilder.Entity("osu.Game.Beatmaps.BeatmapMetadata", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("Artist"); + + b.Property("ArtistUnicode"); + + b.Property("AudioFile"); + + b.Property("AuthorID") + .HasColumnName("AuthorID"); + + b.Property("AuthorString") + .HasColumnName("Author"); + + b.Property("BackgroundFile"); + + b.Property("PreviewTime"); + + b.Property("Source"); + + b.Property("Tags"); + + b.Property("Title"); + + b.Property("TitleUnicode"); + + b.HasKey("ID"); + + b.ToTable("BeatmapMetadata"); + }); + + modelBuilder.Entity("osu.Game.Beatmaps.BeatmapSetFileInfo", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("BeatmapSetInfoID"); + + b.Property("FileInfoID"); + + b.Property("Filename") + .IsRequired(); + + b.HasKey("ID"); + + b.HasIndex("BeatmapSetInfoID"); + + b.HasIndex("FileInfoID"); + + b.ToTable("BeatmapSetFileInfo"); + }); + + modelBuilder.Entity("osu.Game.Beatmaps.BeatmapSetInfo", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("DateAdded"); + + b.Property("DeletePending"); + + b.Property("Hash"); + + b.Property("MetadataID"); + + b.Property("OnlineBeatmapSetID"); + + b.Property("Protected"); + + b.Property("Status"); + + b.HasKey("ID"); + + b.HasIndex("DeletePending"); + + b.HasIndex("Hash") + .IsUnique(); + + b.HasIndex("MetadataID"); + + b.HasIndex("OnlineBeatmapSetID") + .IsUnique(); + + b.ToTable("BeatmapSetInfo"); + }); + + modelBuilder.Entity("osu.Game.Configuration.DatabasedSetting", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("Key") + .HasColumnName("Key"); + + b.Property("RulesetID"); + + b.Property("SkinInfoID"); + + b.Property("StringValue") + .HasColumnName("Value"); + + b.Property("Variant"); + + b.HasKey("ID"); + + b.HasIndex("SkinInfoID"); + + b.HasIndex("RulesetID", "Variant"); + + b.ToTable("Settings"); + }); + + modelBuilder.Entity("osu.Game.IO.FileInfo", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("Hash"); + + b.Property("ReferenceCount"); + + b.HasKey("ID"); + + b.HasIndex("Hash") + .IsUnique(); + + b.HasIndex("ReferenceCount"); + + b.ToTable("FileInfo"); + }); + + modelBuilder.Entity("osu.Game.Input.Bindings.DatabasedKeyBinding", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("IntAction") + .HasColumnName("Action"); + + b.Property("KeysString") + .HasColumnName("Keys"); + + b.Property("RulesetID"); + + b.Property("Variant"); + + b.HasKey("ID"); + + b.HasIndex("IntAction"); + + b.HasIndex("RulesetID", "Variant"); + + b.ToTable("KeyBinding"); + }); + + modelBuilder.Entity("osu.Game.Rulesets.RulesetInfo", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("Available"); + + b.Property("InstantiationInfo"); + + b.Property("Name"); + + b.Property("ShortName"); + + b.HasKey("ID"); + + b.HasIndex("Available"); + + b.HasIndex("ShortName") + .IsUnique(); + + b.ToTable("RulesetInfo"); + }); + + modelBuilder.Entity("osu.Game.Scoring.ScoreFileInfo", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("FileInfoID"); + + b.Property("Filename") + .IsRequired(); + + b.Property("ScoreInfoID"); + + b.HasKey("ID"); + + b.HasIndex("FileInfoID"); + + b.HasIndex("ScoreInfoID"); + + b.ToTable("ScoreFileInfo"); + }); + + modelBuilder.Entity("osu.Game.Scoring.ScoreInfo", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("Accuracy") + .HasColumnType("DECIMAL(1,4)"); + + b.Property("BeatmapInfoID"); + + b.Property("Combo"); + + b.Property("Date"); + + b.Property("DeletePending"); + + b.Property("Hash"); + + b.Property("MaxCombo"); + + b.Property("ModsJson") + .HasColumnName("Mods"); + + b.Property("OnlineScoreID"); + + b.Property("PP"); + + b.Property("Rank"); + + b.Property("RulesetID"); + + b.Property("StatisticsJson") + .HasColumnName("Statistics"); + + b.Property("TotalScore"); + + b.Property("UserID") + .HasColumnName("UserID"); + + b.Property("UserString") + .HasColumnName("User"); + + b.HasKey("ID"); + + b.HasIndex("BeatmapInfoID"); + + b.HasIndex("OnlineScoreID") + .IsUnique(); + + b.HasIndex("RulesetID"); + + b.ToTable("ScoreInfo"); + }); + + modelBuilder.Entity("osu.Game.Skinning.SkinFileInfo", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("FileInfoID"); + + b.Property("Filename") + .IsRequired(); + + b.Property("SkinInfoID"); + + b.HasKey("ID"); + + b.HasIndex("FileInfoID"); + + b.HasIndex("SkinInfoID"); + + b.ToTable("SkinFileInfo"); + }); + + modelBuilder.Entity("osu.Game.Skinning.SkinInfo", b => + { + b.Property("ID") + .ValueGeneratedOnAdd(); + + b.Property("Creator"); + + b.Property("DeletePending"); + + b.Property("Hash"); + + b.Property("InstantiationInfo"); + + b.Property("Name"); + + b.HasKey("ID"); + + b.HasIndex("DeletePending"); + + b.HasIndex("Hash") + .IsUnique(); + + b.ToTable("SkinInfo"); + }); + + modelBuilder.Entity("osu.Game.Beatmaps.BeatmapInfo", b => + { + b.HasOne("osu.Game.Beatmaps.BeatmapDifficulty", "BaseDifficulty") + .WithMany() + .HasForeignKey("BaseDifficultyID") + .OnDelete(DeleteBehavior.Cascade); + + b.HasOne("osu.Game.Beatmaps.BeatmapSetInfo", "BeatmapSet") + .WithMany("Beatmaps") + .HasForeignKey("BeatmapSetInfoID") + .OnDelete(DeleteBehavior.Cascade); + + b.HasOne("osu.Game.Beatmaps.BeatmapMetadata", "Metadata") + .WithMany("Beatmaps") + .HasForeignKey("MetadataID"); + + b.HasOne("osu.Game.Rulesets.RulesetInfo", "Ruleset") + .WithMany() + .HasForeignKey("RulesetID") + .OnDelete(DeleteBehavior.Cascade); + }); + + modelBuilder.Entity("osu.Game.Beatmaps.BeatmapSetFileInfo", b => + { + b.HasOne("osu.Game.Beatmaps.BeatmapSetInfo") + .WithMany("Files") + .HasForeignKey("BeatmapSetInfoID") + .OnDelete(DeleteBehavior.Cascade); + + b.HasOne("osu.Game.IO.FileInfo", "FileInfo") + .WithMany() + .HasForeignKey("FileInfoID") + .OnDelete(DeleteBehavior.Cascade); + }); + + modelBuilder.Entity("osu.Game.Beatmaps.BeatmapSetInfo", b => + { + b.HasOne("osu.Game.Beatmaps.BeatmapMetadata", "Metadata") + .WithMany("BeatmapSets") + .HasForeignKey("MetadataID"); + }); + + modelBuilder.Entity("osu.Game.Configuration.DatabasedSetting", b => + { + b.HasOne("osu.Game.Skinning.SkinInfo") + .WithMany("Settings") + .HasForeignKey("SkinInfoID"); + }); + + modelBuilder.Entity("osu.Game.Scoring.ScoreFileInfo", b => + { + b.HasOne("osu.Game.IO.FileInfo", "FileInfo") + .WithMany() + .HasForeignKey("FileInfoID") + .OnDelete(DeleteBehavior.Cascade); + + b.HasOne("osu.Game.Scoring.ScoreInfo") + .WithMany("Files") + .HasForeignKey("ScoreInfoID"); + }); + + modelBuilder.Entity("osu.Game.Scoring.ScoreInfo", b => + { + b.HasOne("osu.Game.Beatmaps.BeatmapInfo", "Beatmap") + .WithMany("Scores") + .HasForeignKey("BeatmapInfoID") + .OnDelete(DeleteBehavior.Cascade); + + b.HasOne("osu.Game.Rulesets.RulesetInfo", "Ruleset") + .WithMany() + .HasForeignKey("RulesetID") + .OnDelete(DeleteBehavior.Cascade); + }); + + modelBuilder.Entity("osu.Game.Skinning.SkinFileInfo", b => + { + b.HasOne("osu.Game.IO.FileInfo", "FileInfo") + .WithMany() + .HasForeignKey("FileInfoID") + .OnDelete(DeleteBehavior.Cascade); + + b.HasOne("osu.Game.Skinning.SkinInfo") + .WithMany("Files") + .HasForeignKey("SkinInfoID") + .OnDelete(DeleteBehavior.Cascade); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/osu.Game/Migrations/20210514062639_AddAuthorIdToBeatmapMetadata.cs b/osu.Game/Migrations/20210514062639_AddAuthorIdToBeatmapMetadata.cs new file mode 100644 index 0000000000..98fe9b5e13 --- /dev/null +++ b/osu.Game/Migrations/20210514062639_AddAuthorIdToBeatmapMetadata.cs @@ -0,0 +1,23 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +namespace osu.Game.Migrations +{ + public partial class AddAuthorIdToBeatmapMetadata : Migration + { + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.AddColumn( + name: "AuthorID", + table: "BeatmapMetadata", + nullable: false, + defaultValue: 0); + } + + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropColumn( + name: "AuthorID", + table: "BeatmapMetadata"); + } + } +} diff --git a/osu.Game/Migrations/OsuDbContextModelSnapshot.cs b/osu.Game/Migrations/OsuDbContextModelSnapshot.cs index d4bde50b60..f518cfb42b 100644 --- a/osu.Game/Migrations/OsuDbContextModelSnapshot.cs +++ b/osu.Game/Migrations/OsuDbContextModelSnapshot.cs @@ -126,6 +126,9 @@ namespace osu.Game.Migrations b.Property("AudioFile"); + b.Property("AuthorID") + .HasColumnName("AuthorID"); + b.Property("AuthorString") .HasColumnName("Author"); From d09da02673186d308e95da4482a06dade52a2a2f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 14 May 2021 16:03:22 +0900 Subject: [PATCH 04/22] Fix deleting skin elements not saving out to skin Closes https://github.com/ppy/osu/issues/12786. --- osu.Game/Skinning/Editor/SkinEditor.cs | 15 +++++++++++--- .../Skinning/Editor/SkinSelectionHandler.cs | 14 ++++++------- osu.Game/Skinning/ISkinnableTarget.cs | 9 ++++++++- osu.Game/Skinning/SkinnableTargetContainer.cs | 20 +++++++++++++++---- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/osu.Game/Skinning/Editor/SkinEditor.cs b/osu.Game/Skinning/Editor/SkinEditor.cs index 6427d6298b..c594f8d271 100644 --- a/osu.Game/Skinning/Editor/SkinEditor.cs +++ b/osu.Game/Skinning/Editor/SkinEditor.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -193,14 +194,16 @@ namespace osu.Game.Skinning.Editor SelectedComponents.Add(component); } + private IEnumerable availableTargets => targetScreen.ChildrenOfType(); + private ISkinnableTarget getTarget(SkinnableTarget target) { - return targetScreen.ChildrenOfType().FirstOrDefault(c => c.Target == target); + return availableTargets.FirstOrDefault(c => c.Target == target); } private void revert() { - SkinnableTargetContainer[] targetContainers = targetScreen.ChildrenOfType().ToArray(); + ISkinnableTarget[] targetContainers = availableTargets.ToArray(); foreach (var t in targetContainers) { @@ -216,7 +219,7 @@ namespace osu.Game.Skinning.Editor if (!hasBegunMutating) return; - SkinnableTargetContainer[] targetContainers = targetScreen.ChildrenOfType().ToArray(); + ISkinnableTarget[] targetContainers = availableTargets.ToArray(); foreach (var t in targetContainers) currentSkin.Value.UpdateDrawableTarget(t); @@ -237,5 +240,11 @@ namespace osu.Game.Skinning.Editor { this.FadeOut(TRANSITION_DURATION, Easing.OutQuint); } + + public void DeleteItems(ISkinnableDrawable[] items) + { + foreach (var item in items.ToArray()) + availableTargets.FirstOrDefault(t => t.Components.Contains(item))?.Remove(item); + } } } diff --git a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs index 9bcdc6e08b..7931a5ec41 100644 --- a/osu.Game/Skinning/Editor/SkinSelectionHandler.cs +++ b/osu.Game/Skinning/Editor/SkinSelectionHandler.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using osu.Framework.Allocation; using osu.Framework.Extensions.EnumExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.UserInterface; @@ -17,6 +18,9 @@ namespace osu.Game.Skinning.Editor { public class SkinSelectionHandler : SelectionHandler { + [Resolved] + private SkinEditor skinEditor { get; set; } + public override bool HandleRotation(float angle) { // TODO: this doesn't correctly account for origin/anchor specs being different in a multi-selection. @@ -72,14 +76,8 @@ namespace osu.Game.Skinning.Editor SelectionBox.CanReverse = false; } - protected override void DeleteItems(IEnumerable items) - { - foreach (var i in items) - { - ((Drawable)i).Expire(); - SelectedItems.Remove(i); - } - } + protected override void DeleteItems(IEnumerable items) => + skinEditor.DeleteItems(items.ToArray()); protected override IEnumerable GetContextMenuItemsForSelection(IEnumerable> selection) { diff --git a/osu.Game/Skinning/ISkinnableTarget.cs b/osu.Game/Skinning/ISkinnableTarget.cs index 65b8ba7b17..15cad2c817 100644 --- a/osu.Game/Skinning/ISkinnableTarget.cs +++ b/osu.Game/Skinning/ISkinnableTarget.cs @@ -37,8 +37,15 @@ namespace osu.Game.Skinning void Reload(); /// - /// Add the provided item to this target. + /// Add a new skinnable component to this target. /// + /// The component to add. void Add(ISkinnableDrawable drawable); + + /// + /// Remove an existing skinnable component to this target. + /// + /// The component to add. + public void Remove(ISkinnableDrawable component); } } diff --git a/osu.Game/Skinning/SkinnableTargetContainer.cs b/osu.Game/Skinning/SkinnableTargetContainer.cs index a4d7f621eb..f055fb2533 100644 --- a/osu.Game/Skinning/SkinnableTargetContainer.cs +++ b/osu.Game/Skinning/SkinnableTargetContainer.cs @@ -43,10 +43,7 @@ namespace osu.Game.Skinning } } - /// - /// Add a new skinnable component to this target. - /// - /// The component to add. + /// /// Thrown when attempting to add an element to a target which is not supported by the current skin. /// Thrown if the provided instance is not a . public void Add(ISkinnableDrawable component) @@ -61,6 +58,21 @@ namespace osu.Game.Skinning components.Add(component); } + /// + /// Thrown when attempting to add an element to a target which is not supported by the current skin. + /// Thrown if the provided instance is not a . + public void Remove(ISkinnableDrawable component) + { + if (content == null) + throw new NotSupportedException("Attempting to add a new component to a target container which is not supported by the current skin."); + + if (!(component is Drawable drawable)) + throw new ArgumentException($"Provided argument must be of type {nameof(Drawable)}.", nameof(drawable)); + + content.Remove(drawable); + components.Remove(component); + } + protected override void SkinChanged(ISkinSource skin, bool allowFallback) { base.SkinChanged(skin, allowFallback); From 876f53bf3beb7c2f212b1634579326a5b2c7c6e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 22:15:43 +0200 Subject: [PATCH 05/22] Fix copy-paste oversights in xmldoc & exception messages --- osu.Game/Skinning/ISkinnableTarget.cs | 4 ++-- osu.Game/Skinning/SkinnableTargetContainer.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Skinning/ISkinnableTarget.cs b/osu.Game/Skinning/ISkinnableTarget.cs index 15cad2c817..8d4f4dd0c3 100644 --- a/osu.Game/Skinning/ISkinnableTarget.cs +++ b/osu.Game/Skinning/ISkinnableTarget.cs @@ -43,9 +43,9 @@ namespace osu.Game.Skinning void Add(ISkinnableDrawable drawable); /// - /// Remove an existing skinnable component to this target. + /// Remove an existing skinnable component from this target. /// - /// The component to add. + /// The component to remove. public void Remove(ISkinnableDrawable component); } } diff --git a/osu.Game/Skinning/SkinnableTargetContainer.cs b/osu.Game/Skinning/SkinnableTargetContainer.cs index f055fb2533..c195a5516f 100644 --- a/osu.Game/Skinning/SkinnableTargetContainer.cs +++ b/osu.Game/Skinning/SkinnableTargetContainer.cs @@ -64,7 +64,7 @@ namespace osu.Game.Skinning public void Remove(ISkinnableDrawable component) { if (content == null) - throw new NotSupportedException("Attempting to add a new component to a target container which is not supported by the current skin."); + throw new NotSupportedException("Attempting to remove a new component from a target container which is not supported by the current skin."); if (!(component is Drawable drawable)) throw new ArgumentException($"Provided argument must be of type {nameof(Drawable)}.", nameof(drawable)); From 743b4fbff15bb8d506f4848b1c1ed9a803339253 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 22:16:37 +0200 Subject: [PATCH 06/22] Pass correct member name to `ArgumentException`s --- osu.Game/Skinning/SkinnableTargetContainer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/SkinnableTargetContainer.cs b/osu.Game/Skinning/SkinnableTargetContainer.cs index c195a5516f..d454e199dc 100644 --- a/osu.Game/Skinning/SkinnableTargetContainer.cs +++ b/osu.Game/Skinning/SkinnableTargetContainer.cs @@ -52,7 +52,7 @@ namespace osu.Game.Skinning throw new NotSupportedException("Attempting to add a new component to a target container which is not supported by the current skin."); if (!(component is Drawable drawable)) - throw new ArgumentException($"Provided argument must be of type {nameof(Drawable)}.", nameof(drawable)); + throw new ArgumentException($"Provided argument must be of type {nameof(Drawable)}.", nameof(component)); content.Add(drawable); components.Add(component); @@ -67,7 +67,7 @@ namespace osu.Game.Skinning throw new NotSupportedException("Attempting to remove a new component from a target container which is not supported by the current skin."); if (!(component is Drawable drawable)) - throw new ArgumentException($"Provided argument must be of type {nameof(Drawable)}.", nameof(drawable)); + throw new ArgumentException($"Provided argument must be of type {nameof(Drawable)}.", nameof(component)); content.Remove(drawable); components.Remove(component); From 3d3c5028e6df652f53706e24d7b8ee0f58d09c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 22:33:26 +0200 Subject: [PATCH 07/22] Trim unnecessary array copy --- osu.Game/Skinning/Editor/SkinEditor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/Editor/SkinEditor.cs b/osu.Game/Skinning/Editor/SkinEditor.cs index c594f8d271..f24b0c71c0 100644 --- a/osu.Game/Skinning/Editor/SkinEditor.cs +++ b/osu.Game/Skinning/Editor/SkinEditor.cs @@ -243,7 +243,7 @@ namespace osu.Game.Skinning.Editor public void DeleteItems(ISkinnableDrawable[] items) { - foreach (var item in items.ToArray()) + foreach (var item in items) availableTargets.FirstOrDefault(t => t.Components.Contains(item))?.Remove(item); } } From 044770f1a265f6a7f1b4d34858384dee6a43c4a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:29:34 +0200 Subject: [PATCH 08/22] Locally suppress warning in `SerializationReader` `SerializationReader` is not written in a form that would support turning nullability checking on for the entire class. The biggest problem there is the inner `DynamicDeserializer` static class, whose members are initialised via an `initialize()` method, which the compiler knows nothing about. For this reason, just opt to suppress the single inspection about returning a `null` from a method with a return type of `string` (rider expects `string?`). It would have been also viable to enable nullability checking for this one method, but that's pretty much the same thing and adds no safety anyways, so just disable the warning to minimise surprise. --- osu.Game/IO/Legacy/SerializationReader.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/IO/Legacy/SerializationReader.cs b/osu.Game/IO/Legacy/SerializationReader.cs index 17cbd19838..00f90f78e3 100644 --- a/osu.Game/IO/Legacy/SerializationReader.cs +++ b/osu.Game/IO/Legacy/SerializationReader.cs @@ -38,6 +38,7 @@ namespace osu.Game.IO.Legacy /// Reads a string from the buffer. Overrides the base implementation so it can cope with nulls. public override string ReadString() { + // ReSharper disable once AssignNullToNotNullAttribute if (ReadByte() == 0) return null; return base.ReadString(); From aaa7c7eb0593c9b04c728aee778ff1de5a08f6bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:33:17 +0200 Subject: [PATCH 09/22] Handle null case explicitly in `SpectatorState.Equals()` Uses the usual pattern of two `ReferenceEquals` checks against `this` and `null` before proceeding to inspect field values. Doing this causes the compiler to infer that at the point that field values are checked, `other` can no longer viably be `null`. --- osu.Game/Online/Spectator/SpectatorState.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/osu.Game/Online/Spectator/SpectatorState.cs b/osu.Game/Online/Spectator/SpectatorState.cs index 96a875bc14..ebb91e4dd2 100644 --- a/osu.Game/Online/Spectator/SpectatorState.cs +++ b/osu.Game/Online/Spectator/SpectatorState.cs @@ -24,7 +24,13 @@ namespace osu.Game.Online.Spectator [Key(2)] public IEnumerable Mods { get; set; } = Enumerable.Empty(); - public bool Equals(SpectatorState other) => BeatmapID == other?.BeatmapID && Mods.SequenceEqual(other?.Mods) && RulesetID == other?.RulesetID; + public bool Equals(SpectatorState other) + { + if (ReferenceEquals(null, other)) return false; + if (ReferenceEquals(this, other)) return true; + + return BeatmapID == other.BeatmapID && Mods.SequenceEqual(other.Mods) && RulesetID == other.RulesetID; + } public override string ToString() => $"Beatmap:{BeatmapID} Mods:{string.Join(',', Mods)} Ruleset:{RulesetID}"; } From c9facf70f9503bd3928b3ddad35807d21df633b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:35:06 +0200 Subject: [PATCH 10/22] Use conditional nullability attribute As it turns out, C# 8 provides an attribute that allows annotating that an `out` parameter's nullability depends on the method's return value, which is exactly what is desired here. --- osu.Game.Tests/Mods/ModUtilsTest.cs | 2 +- osu.Game/Utils/ModUtils.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Mods/ModUtilsTest.cs b/osu.Game.Tests/Mods/ModUtilsTest.cs index 7dcaabca3d..7384471c41 100644 --- a/osu.Game.Tests/Mods/ModUtilsTest.cs +++ b/osu.Game.Tests/Mods/ModUtilsTest.cs @@ -146,7 +146,7 @@ namespace osu.Game.Tests.Mods if (isValid) Assert.IsNull(invalid); else - Assert.That(invalid?.Select(t => t.GetType()), Is.EquivalentTo(expectedInvalid)); + Assert.That(invalid.Select(t => t.GetType()), Is.EquivalentTo(expectedInvalid)); } public abstract class CustomMod1 : Mod diff --git a/osu.Game/Utils/ModUtils.cs b/osu.Game/Utils/ModUtils.cs index 596880f2e7..1c3558fc90 100644 --- a/osu.Game/Utils/ModUtils.cs +++ b/osu.Game/Utils/ModUtils.cs @@ -92,7 +92,7 @@ namespace osu.Game.Utils /// The mods to check. /// Invalid mods, if any were found. Can be null if all mods were valid. /// Whether the input mods were all valid. If false, will contain all invalid entries. - public static bool CheckValidForGameplay(IEnumerable mods, out List? invalidMods) + public static bool CheckValidForGameplay(IEnumerable mods, [NotNullWhen(false)] out List? invalidMods) { mods = mods.ToArray(); From f716fb09686682f8b9061b2b532866848db0fdf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:38:44 +0200 Subject: [PATCH 11/22] Remove bogus `InternalChildren` null-check `InternalChildren` can't viably be `null`, and if it were, we have bigger problems. The removed null-check was triggering false-positive inspections further down. --- osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyTaikoScroller.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyTaikoScroller.cs b/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyTaikoScroller.cs index eb92097204..6fc59ea0e8 100644 --- a/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyTaikoScroller.cs +++ b/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyTaikoScroller.cs @@ -57,7 +57,7 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Legacy base.Update(); // store X before checking wide enough so if we perform layout there is no positional discrepancy. - float currentX = (InternalChildren?.FirstOrDefault()?.X ?? 0) - (float)Clock.ElapsedFrameTime * 0.1f; + float currentX = (InternalChildren.FirstOrDefault()?.X ?? 0) - (float)Clock.ElapsedFrameTime * 0.1f; // ensure we have enough sprites if (!InternalChildren.Any() From 483e0dd9431efe347f01ac38ef86e1190b8d8f6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:39:36 +0200 Subject: [PATCH 12/22] Pass placeholder hitobject instead of `null` --- .../Skinning/TestSceneTaikoScroller.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Taiko.Tests/Skinning/TestSceneTaikoScroller.cs b/osu.Game.Rulesets.Taiko.Tests/Skinning/TestSceneTaikoScroller.cs index 4ae3cbd418..14c3599fcd 100644 --- a/osu.Game.Rulesets.Taiko.Tests/Skinning/TestSceneTaikoScroller.cs +++ b/osu.Game.Rulesets.Taiko.Tests/Skinning/TestSceneTaikoScroller.cs @@ -5,6 +5,7 @@ using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Testing; using osu.Framework.Timing; using osu.Game.Rulesets.Judgements; +using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Scoring; using osu.Game.Rulesets.Taiko.Skinning.Legacy; using osu.Game.Skinning; @@ -27,7 +28,7 @@ namespace osu.Game.Rulesets.Taiko.Tests.Skinning })); AddToggleStep("Toggle passing", passing => this.ChildrenOfType().ForEach(s => s.LastResult.Value = - new JudgementResult(null, new Judgement()) { Type = passing ? HitResult.Great : HitResult.Miss })); + new JudgementResult(new HitObject(), new Judgement()) { Type = passing ? HitResult.Great : HitResult.Miss })); AddToggleStep("toggle playback direction", reversed => this.reversed = reversed); } From f2d0f7db9928b60f84b23b14c07488751df86917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:40:17 +0200 Subject: [PATCH 13/22] Remove list null-checks in `LogoTrackingContainer` test If the null-checks were tripped, the test would crash anyway. It is not possible to call `.Any()` and get a valid result instead of an exception on a null reference. --- .../Visual/UserInterface/TestSceneLogoTrackingContainer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneLogoTrackingContainer.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneLogoTrackingContainer.cs index 5582cc6826..b46d35a84d 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneLogoTrackingContainer.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneLogoTrackingContainer.cs @@ -264,7 +264,7 @@ namespace osu.Game.Tests.Visual.UserInterface private void moveLogoFacade() { - if (!(logoFacade?.Transforms).Any() && !(transferContainer?.Transforms).Any()) + if (!logoFacade.Transforms.Any() && !transferContainer.Transforms.Any()) { Random random = new Random(); trackingContainer.Delay(500).MoveTo(new Vector2(random.Next(0, (int)logo.Parent.DrawWidth), random.Next(0, (int)logo.Parent.DrawHeight)), 300); From 43c73f9583e31d55cc384ca8fa445011c449ec6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:42:56 +0200 Subject: [PATCH 14/22] Mark access to exception if task faulted as safe There are seemingly no C#-side compile-time guarantees that it is safe, but if the task's state is `Faulted` (as is checked right before), the exception cannot be null as per the documentation. --- osu.Game/Extensions/TaskExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Extensions/TaskExtensions.cs b/osu.Game/Extensions/TaskExtensions.cs index 76a76c0c52..17f1a491f8 100644 --- a/osu.Game/Extensions/TaskExtensions.cs +++ b/osu.Game/Extensions/TaskExtensions.cs @@ -6,6 +6,7 @@ using System; using System.Threading; using System.Threading.Tasks; +using osu.Framework.Extensions.ObjectExtensions; namespace osu.Game.Extensions { @@ -50,7 +51,7 @@ namespace osu.Game.Extensions } else if (continuationTask.IsFaulted) { - tcs.TrySetException(continuationTask.Exception); + tcs.TrySetException(continuationTask.Exception.AsNonNull()); } else { From 628e7a71edc56fb2ff2f1aec5d44646b3b1a343f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:45:58 +0200 Subject: [PATCH 15/22] Ignore possible nulls in `Type.GetType()` calls They're mostly used in extensibility scenarios, so everything happens in runtime. There is no better resolution than to crash with a null reference exception. --- osu.Game/IO/Serialization/Converters/TypedListConverter.cs | 3 ++- osu.Game/Rulesets/RulesetInfo.cs | 3 ++- osu.Game/Rulesets/RulesetStore.cs | 3 ++- osu.Game/Skinning/SkinInfo.cs | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/osu.Game/IO/Serialization/Converters/TypedListConverter.cs b/osu.Game/IO/Serialization/Converters/TypedListConverter.cs index 50b28ea74b..174fbf9983 100644 --- a/osu.Game/IO/Serialization/Converters/TypedListConverter.cs +++ b/osu.Game/IO/Serialization/Converters/TypedListConverter.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using Newtonsoft.Json; using Newtonsoft.Json.Linq; +using osu.Framework.Extensions.ObjectExtensions; namespace osu.Game.IO.Serialization.Converters { @@ -60,7 +61,7 @@ namespace osu.Game.IO.Serialization.Converters throw new JsonException("Expected $type token."); var typeName = lookupTable[(int)tok["$type"]]; - var instance = (T)Activator.CreateInstance(Type.GetType(typeName)); + var instance = (T)Activator.CreateInstance(Type.GetType(typeName).AsNonNull()); serializer.Populate(itemReader, instance); list.Add(instance); diff --git a/osu.Game/Rulesets/RulesetInfo.cs b/osu.Game/Rulesets/RulesetInfo.cs index 702bf35fa8..59ec9cdd7e 100644 --- a/osu.Game/Rulesets/RulesetInfo.cs +++ b/osu.Game/Rulesets/RulesetInfo.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics.CodeAnalysis; using Newtonsoft.Json; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Testing; namespace osu.Game.Rulesets @@ -27,7 +28,7 @@ namespace osu.Game.Rulesets { if (!Available) return null; - var ruleset = (Ruleset)Activator.CreateInstance(Type.GetType(InstantiationInfo)); + var ruleset = (Ruleset)Activator.CreateInstance(Type.GetType(InstantiationInfo).AsNonNull()); // overwrite the pre-populated RulesetInfo with a potentially database attached copy. ruleset.RulesetInfo = this; diff --git a/osu.Game/Rulesets/RulesetStore.cs b/osu.Game/Rulesets/RulesetStore.cs index 4261ee3d47..0a34ca9598 100644 --- a/osu.Game/Rulesets/RulesetStore.cs +++ b/osu.Game/Rulesets/RulesetStore.cs @@ -7,6 +7,7 @@ using System.IO; using System.Linq; using System.Reflection; using osu.Framework; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Database; @@ -111,7 +112,7 @@ namespace osu.Game.Rulesets { try { - var instanceInfo = ((Ruleset)Activator.CreateInstance(Type.GetType(r.InstantiationInfo))).RulesetInfo; + var instanceInfo = ((Ruleset)Activator.CreateInstance(Type.GetType(r.InstantiationInfo).AsNonNull())).RulesetInfo; r.Name = instanceInfo.Name; r.ShortName = instanceInfo.ShortName; diff --git a/osu.Game/Skinning/SkinInfo.cs b/osu.Game/Skinning/SkinInfo.cs index bc57a8e71c..55760876e3 100644 --- a/osu.Game/Skinning/SkinInfo.cs +++ b/osu.Game/Skinning/SkinInfo.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.IO.Stores; using osu.Game.Configuration; using osu.Game.Database; @@ -32,7 +33,7 @@ namespace osu.Game.Skinning var type = string.IsNullOrEmpty(InstantiationInfo) // handle the case of skins imported before InstantiationInfo was added. ? typeof(LegacySkin) - : Type.GetType(InstantiationInfo); + : Type.GetType(InstantiationInfo).AsNonNull(); if (type == typeof(DefaultLegacySkin)) return (Skin)Activator.CreateInstance(type, this, legacyDefaultResources, resources); From 5b2b701915f1c387856742d93a19a620a2ffb368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:47:35 +0200 Subject: [PATCH 16/22] Ignore possible null in `GetResponseString()` A null there indicates a deserialisation error and therefore due to the catch block immediately succeeding the changed line everything will continue to work as intended. --- osu.Game/Online/API/APIAccess.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/API/APIAccess.cs b/osu.Game/Online/API/APIAccess.cs index 944525c119..1686595512 100644 --- a/osu.Game/Online/API/APIAccess.cs +++ b/osu.Game/Online/API/APIAccess.cs @@ -270,7 +270,7 @@ namespace osu.Game.Online.API { try { - return JObject.Parse(req.GetResponseString()).SelectToken("form_error", true).AsNonNull().ToObject(); + return JObject.Parse(req.GetResponseString().AsNonNull()).SelectToken("form_error", true).AsNonNull().ToObject(); } catch { From fa6b5515b7752932c0ffb9f035659f3c2ab1aedb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:49:41 +0200 Subject: [PATCH 17/22] Ignore possible null from `JsonConvert.DeserializeObject()` Nothing better can be done if a `null` is indeed returned. --- osu.Game/Skinning/Skin.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 2944c7a8ec..f43283f624 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -8,6 +8,7 @@ using System.Text; using Newtonsoft.Json; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.OpenGL.Textures; using osu.Framework.Graphics.Textures; @@ -57,7 +58,7 @@ namespace osu.Game.Skinning string jsonContent = Encoding.UTF8.GetString(bytes); - DrawableComponentInfo[skinnableTarget] = JsonConvert.DeserializeObject>(jsonContent).ToArray(); + DrawableComponentInfo[skinnableTarget] = JsonConvert.DeserializeObject>(jsonContent).AsNonNull().ToArray(); } } From b51d0380882c33bae1723af63036533f6ed2d8b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:50:25 +0200 Subject: [PATCH 18/22] Ignore possible path-related nulls They're all in test code anyway, so any issue there will cause a test to fail. --- osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs | 3 ++- osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs | 3 ++- osu.Game/Tests/Beatmaps/DifficultyCalculatorTest.cs | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index 0c35e9471d..0d117f8755 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -12,6 +12,7 @@ using osu.Framework.Platform; using osu.Game.IPC; using osu.Framework.Allocation; using osu.Framework.Extensions; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Logging; using osu.Game.Beatmaps; using osu.Game.Database; @@ -264,7 +265,7 @@ namespace osu.Game.Tests.Beatmaps.IO // change filename var firstFile = new FileInfo(Directory.GetFiles(extractedFolder).First()); - firstFile.MoveTo(Path.Combine(firstFile.DirectoryName, $"{firstFile.Name}-changed{firstFile.Extension}")); + firstFile.MoveTo(Path.Combine(firstFile.DirectoryName.AsNonNull(), $"{firstFile.Name}-changed{firstFile.Extension}")); using (var zip = ZipArchive.Create()) { diff --git a/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs b/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs index 1c5e551042..a97f6defe9 100644 --- a/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs +++ b/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs @@ -9,6 +9,7 @@ using System.Reflection; using Newtonsoft.Json; using NUnit.Framework; using osu.Framework.Audio.Track; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics.Textures; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; @@ -164,7 +165,7 @@ namespace osu.Game.Tests.Beatmaps private Stream openResource(string name) { - var localPath = Path.GetDirectoryName(Uri.UnescapeDataString(new UriBuilder(Assembly.GetExecutingAssembly().CodeBase).Path)); + var localPath = Path.GetDirectoryName(Uri.UnescapeDataString(new UriBuilder(Assembly.GetExecutingAssembly().CodeBase).Path)).AsNonNull(); return Assembly.LoadFrom(Path.Combine(localPath, $"{ResourceAssembly}.dll")).GetManifestResourceStream($@"{ResourceAssembly}.Resources.{name}"); } diff --git a/osu.Game/Tests/Beatmaps/DifficultyCalculatorTest.cs b/osu.Game/Tests/Beatmaps/DifficultyCalculatorTest.cs index 748a52d1c5..e10bf08da4 100644 --- a/osu.Game/Tests/Beatmaps/DifficultyCalculatorTest.cs +++ b/osu.Game/Tests/Beatmaps/DifficultyCalculatorTest.cs @@ -5,6 +5,7 @@ using System; using System.IO; using System.Reflection; using NUnit.Framework; +using osu.Framework.Extensions.ObjectExtensions; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; using osu.Game.IO; @@ -41,7 +42,7 @@ namespace osu.Game.Tests.Beatmaps private Stream openResource(string name) { - var localPath = Path.GetDirectoryName(Uri.UnescapeDataString(new UriBuilder(Assembly.GetExecutingAssembly().CodeBase).Path)); + var localPath = Path.GetDirectoryName(Uri.UnescapeDataString(new UriBuilder(Assembly.GetExecutingAssembly().CodeBase).Path)).AsNonNull(); return Assembly.LoadFrom(Path.Combine(localPath, $"{ResourceAssembly}.dll")).GetManifestResourceStream($@"{ResourceAssembly}.Resources.{name}"); } From e62e473bb262f368988dcfdbd417ca272f8a748a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:51:06 +0200 Subject: [PATCH 19/22] Ignore possible null in multiplayer test A null value will fail the test anyhow. --- .../NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs b/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs index 14589f8e6c..adc1d6aede 100644 --- a/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs +++ b/osu.Game.Tests/NonVisual/Multiplayer/StatefulMultiplayerClientTest.cs @@ -4,6 +4,7 @@ using System.Linq; using Humanizer; using NUnit.Framework; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Testing; using osu.Game.Online.Multiplayer; using osu.Game.Tests.Visual.Multiplayer; @@ -34,7 +35,7 @@ namespace osu.Game.Tests.NonVisual.Multiplayer changeState(6, MultiplayerUserState.WaitingForLoad); checkPlayingUserCount(6); - AddStep("another user left", () => Client.RemoveUser(Client.Room?.Users.Last().User)); + AddStep("another user left", () => Client.RemoveUser((Client.Room?.Users.Last().User).AsNonNull())); checkPlayingUserCount(5); AddStep("leave room", () => Client.LeaveRoom()); From d581e0a2524c6c584667d5017e9ab92e81a8de0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:52:16 +0200 Subject: [PATCH 20/22] Ignore possible nulls in `NotifyCollectionChangedArgs` Safe to access by the virtue of the preceding case labels on `args.Action`. And they're in test code anyways. --- .../Visual/Online/TestSceneLeaderboardModSelector.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneLeaderboardModSelector.cs b/osu.Game.Tests/Visual/Online/TestSceneLeaderboardModSelector.cs index 54e655d4ec..fc438ce6dd 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneLeaderboardModSelector.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneLeaderboardModSelector.cs @@ -13,6 +13,7 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Bindables; +using osu.Framework.Extensions.ObjectExtensions; using osu.Game.Graphics.Sprites; using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; @@ -45,14 +46,14 @@ namespace osu.Game.Tests.Visual.Online switch (args.Action) { case NotifyCollectionChangedAction.Add: - args.NewItems.Cast().ForEach(mod => selectedMods.Add(new OsuSpriteText + args.NewItems.AsNonNull().Cast().ForEach(mod => selectedMods.Add(new OsuSpriteText { Text = mod.Acronym, })); break; case NotifyCollectionChangedAction.Remove: - args.OldItems.Cast().ForEach(mod => + args.OldItems.AsNonNull().Cast().ForEach(mod => { foreach (var selected in selectedMods) { From ae71389ebe18bcd3f3c1ca3793088c13fe041fcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 14 May 2021 23:53:37 +0200 Subject: [PATCH 21/22] Ignore possible nulls from stream reader in IPC Any failures will be caught. They're not logged, but they also weren't before. Error handling can be improved at a future date, this series of changes is primarily intending to unblock a new inspection. --- osu.Game.Tournament/IPC/FileBasedIPC.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tournament/IPC/FileBasedIPC.cs b/osu.Game.Tournament/IPC/FileBasedIPC.cs index 71417d1cc6..f538d4a7d9 100644 --- a/osu.Game.Tournament/IPC/FileBasedIPC.cs +++ b/osu.Game.Tournament/IPC/FileBasedIPC.cs @@ -7,6 +7,7 @@ using System.Linq; using JetBrains.Annotations; using Microsoft.Win32; using osu.Framework.Allocation; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Framework.Threading; @@ -77,8 +78,8 @@ namespace osu.Game.Tournament.IPC using (var stream = IPCStorage.GetStream(file_ipc_filename)) using (var sr = new StreamReader(stream)) { - var beatmapId = int.Parse(sr.ReadLine()); - var mods = int.Parse(sr.ReadLine()); + var beatmapId = int.Parse(sr.ReadLine().AsNonNull()); + var mods = int.Parse(sr.ReadLine().AsNonNull()); if (lastBeatmapId != beatmapId) { @@ -124,7 +125,7 @@ namespace osu.Game.Tournament.IPC using (var stream = IPCStorage.GetStream(file_ipc_state_filename)) using (var sr = new StreamReader(stream)) { - State.Value = (TourneyState)Enum.Parse(typeof(TourneyState), sr.ReadLine()); + State.Value = (TourneyState)Enum.Parse(typeof(TourneyState), sr.ReadLine().AsNonNull()); } } catch (Exception) From 69fc072429bc31ad23a67a1dacf165cf341d6fde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 15 May 2021 01:01:27 +0200 Subject: [PATCH 22/22] Ignore skin component json data if deserialisation fails instead Crashing was not really the best thing to do there given the preceding code that already allowed a few continues in case of a missing file. --- osu.Game/Skinning/Skin.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index f43283f624..b6cb8fc7a4 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -8,7 +8,6 @@ using System.Text; using Newtonsoft.Json; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; -using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.OpenGL.Textures; using osu.Framework.Graphics.Textures; @@ -57,8 +56,12 @@ namespace osu.Game.Skinning continue; string jsonContent = Encoding.UTF8.GetString(bytes); + var deserializedContent = JsonConvert.DeserializeObject>(jsonContent); - DrawableComponentInfo[skinnableTarget] = JsonConvert.DeserializeObject>(jsonContent).AsNonNull().ToArray(); + if (deserializedContent == null) + continue; + + DrawableComponentInfo[skinnableTarget] = deserializedContent.ToArray(); } }