From f64fa65fd5c2297f9f0d0355c189feaffb3ec3bb Mon Sep 17 00:00:00 2001 From: Jason Won Date: Tue, 26 Oct 2021 14:52:15 -0400 Subject: [PATCH 01/14] right click on unselected object shows context menu --- .../Editing/TestSceneComposerSelection.cs | 24 +++++++++++++++++++ .../Compose/Components/BlueprintContainer.cs | 12 +++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs b/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs index a2a7b72283..36663591f2 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs @@ -42,6 +42,30 @@ namespace osu.Game.Tests.Visual.Editing }); } + [Test] + public void TestSelectAndShowContextMenu() + { + var addedObject = new HitCircle { StartTime = 100, Position = new Vector2(100, 100) }; + AddStep("add hitobject", () => EditorBeatmap.Add(addedObject)); + + moveMouseToObject(() => addedObject); + AddStep("right click", () => InputManager.Click(MouseButton.Right)); + + AddAssert("hitobject selected", () => EditorBeatmap.SelectedHitObjects.Single() == addedObject); + + AddStep("delete from context menu", () => + { + var pos = blueprintContainer.SelectionBlueprints + .First(s => s.Item == addedObject) + .ChildrenOfType() + .First(); + + InputManager.MoveMouseTo(pos, new Vector2(50, 120)); + InputManager.Click(MouseButton.Left); + }); + AddAssert("no hitobjects in beatmap", () => EditorBeatmap.HitObjects.Count == 0); + } + [Test] public void TestNudgeSelection() { diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index 75d4d13f94..0de99831f2 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -108,11 +108,17 @@ namespace osu.Game.Screens.Edit.Compose.Components protected override bool OnMouseDown(MouseDownEvent e) { bool selectionPerformed = performMouseDownActions(e); - - // even if a selection didn't occur, a drag event may still move the selection. bool movementPossible = prepareSelectionMovement(); - return selectionPerformed || (e.Button == MouseButton.Left && movementPossible); + // check if selection has occurred + if (selectionPerformed) + { + // propagate right click to show context menu on selection + return e.Button != MouseButton.Right; + } + + // even if a selection didn't occur, a drag event may still move the selection. + return e.Button == MouseButton.Left && movementPossible; } protected SelectionBlueprint ClickedBlueprint { get; private set; } From 4efa1f23bb7c0ce836d0d59b8748cc4b8583d962 Mon Sep 17 00:00:00 2001 From: Jason Won Date: Tue, 26 Oct 2021 15:29:50 -0400 Subject: [PATCH 02/14] fix context menu test --- .../Editing/TestSceneComposerSelection.cs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs b/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs index 36663591f2..9aae595808 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs @@ -5,7 +5,10 @@ using System; using System.Linq; using NUnit.Framework; using osu.Framework.Testing; +using osu.Framework.Graphics.Cursor; +using osu.Framework.Graphics.UserInterface; using osu.Game.Beatmaps; +using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu; @@ -29,6 +32,9 @@ namespace osu.Game.Tests.Visual.Editing private ComposeBlueprintContainer blueprintContainer => Editor.ChildrenOfType().First(); + private ContextMenuContainer contextMenuContainer + => Editor.ChildrenOfType().First(); + private void moveMouseToObject(Func targetFunc) { AddStep("move mouse to object", () => @@ -52,18 +58,7 @@ namespace osu.Game.Tests.Visual.Editing AddStep("right click", () => InputManager.Click(MouseButton.Right)); AddAssert("hitobject selected", () => EditorBeatmap.SelectedHitObjects.Single() == addedObject); - - AddStep("delete from context menu", () => - { - var pos = blueprintContainer.SelectionBlueprints - .First(s => s.Item == addedObject) - .ChildrenOfType() - .First(); - - InputManager.MoveMouseTo(pos, new Vector2(50, 120)); - InputManager.Click(MouseButton.Left); - }); - AddAssert("no hitobjects in beatmap", () => EditorBeatmap.HitObjects.Count == 0); + AddAssert("context menu is visible", () => contextMenuContainer.ChildrenOfType().Single().State == MenuState.Open); } [Test] From c633e2e9521b9c9ae86defae674a0d65b16efabd Mon Sep 17 00:00:00 2001 From: Jason Won Date: Tue, 26 Oct 2021 16:24:53 -0400 Subject: [PATCH 03/14] only propagate unmodified right click --- .../Visual/Editing/TestSceneComposerSelection.cs | 4 ++-- .../Screens/Edit/Compose/Components/BlueprintContainer.cs | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs b/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs index 9aae595808..dddd9f07ab 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs @@ -57,8 +57,8 @@ namespace osu.Game.Tests.Visual.Editing moveMouseToObject(() => addedObject); AddStep("right click", () => InputManager.Click(MouseButton.Right)); - AddAssert("hitobject selected", () => EditorBeatmap.SelectedHitObjects.Single() == addedObject); - AddAssert("context menu is visible", () => contextMenuContainer.ChildrenOfType().Single().State == MenuState.Open); + AddUntilStep("hitobject selected", () => EditorBeatmap.SelectedHitObjects.Single() == addedObject); + AddUntilStep("context menu is visible", () => contextMenuContainer.ChildrenOfType().Single().State == MenuState.Open); } [Test] diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index 0de99831f2..7391c1a642 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -113,8 +113,11 @@ namespace osu.Game.Screens.Edit.Compose.Components // check if selection has occurred if (selectionPerformed) { - // propagate right click to show context menu on selection - return e.Button != MouseButton.Right; + // only unmodified right click should show context menu + var shouldShowContextMenu = e.Button == MouseButton.Right && !e.ShiftPressed && !e.AltPressed && !e.SuperPressed; + + // stop propagation if not showing context menu + return !shouldShowContextMenu; } // even if a selection didn't occur, a drag event may still move the selection. From 772b6c6dcde694476895e786fef502e237b46c77 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 4 Nov 2021 12:11:42 +0900 Subject: [PATCH 04/14] Enable nullable support on `BeatmapMetadata` --- osu.Game/Beatmaps/BeatmapMetadata.cs | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapMetadata.cs b/osu.Game/Beatmaps/BeatmapMetadata.cs index 711533e118..5d3b4007ae 100644 --- a/osu.Game/Beatmaps/BeatmapMetadata.cs +++ b/osu.Game/Beatmaps/BeatmapMetadata.cs @@ -9,6 +9,8 @@ using osu.Framework.Testing; using osu.Game.Database; using osu.Game.Users; +#nullable enable + namespace osu.Game.Beatmaps { [ExcludeFromDynamicCompile] @@ -17,21 +19,21 @@ namespace osu.Game.Beatmaps { public int ID { get; set; } - public string Title { get; set; } + public string Title { get; set; } = string.Empty; [JsonProperty("title_unicode")] - public string TitleUnicode { get; set; } + public string TitleUnicode { get; set; } = string.Empty; - public string Artist { get; set; } + public string Artist { get; set; } = string.Empty; [JsonProperty("artist_unicode")] - public string ArtistUnicode { get; set; } + public string ArtistUnicode { get; set; } = string.Empty; [JsonIgnore] - public List Beatmaps { get; set; } + public List Beatmaps { get; set; } = new List(); [JsonIgnore] - public List BeatmapSets { get; set; } + public List BeatmapSets { get; set; } = new List(); /// /// Helper property to deserialize a username to . @@ -55,7 +57,7 @@ namespace osu.Game.Beatmaps [Column("Author")] public string AuthorString { - get => Author?.Username; + get => Author?.Username ?? string.Empty; set { Author ??= new User(); @@ -67,22 +69,22 @@ namespace osu.Game.Beatmaps /// The author of the beatmaps in this set. /// [JsonIgnore] - public User Author; + public User? Author; - public string Source { get; set; } + public string Source { get; set; } = string.Empty; [JsonProperty(@"tags")] - public string Tags { get; set; } + public string Tags { get; set; } = string.Empty; /// /// The time in milliseconds to begin playing the track for preview purposes. /// If -1, the track should begin playing at 40% of its length. /// - public int PreviewTime { get; set; } + public int PreviewTime { get; set; } = -1; - public string AudioFile { get; set; } + public string AudioFile { get; set; } = string.Empty; - public string BackgroundFile { get; set; } + public string BackgroundFile { get; set; } = string.Empty; public bool Equals(BeatmapMetadata other) => ((IBeatmapMetadataInfo)this).Equals(other); From 67d9590a79f695b38f999dfdcc1f105238162920 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 4 Nov 2021 13:59:40 +0900 Subject: [PATCH 05/14] Fix new inspections --- osu.Game.Tests/Editing/Checks/CheckAudioQualityTest.cs | 2 +- osu.Game/Beatmaps/BeatmapManager.cs | 8 ++++---- osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs | 10 +++++----- osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/Editing/Checks/CheckAudioQualityTest.cs b/osu.Game.Tests/Editing/Checks/CheckAudioQualityTest.cs index 39fbf11d51..e22dfa5f8b 100644 --- a/osu.Game.Tests/Editing/Checks/CheckAudioQualityTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckAudioQualityTest.cs @@ -35,7 +35,7 @@ namespace osu.Game.Tests.Editing.Checks public void TestMissing() { // While this is a problem, it is out of scope for this check and is caught by a different one. - beatmap.Metadata.AudioFile = null; + beatmap.Metadata.AudioFile = string.Empty; var mock = new Mock(); mock.SetupGet(w => w.Beatmap).Returns(beatmap); diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 2fa8bd4990..7a9f3e0a45 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -258,10 +258,10 @@ namespace osu.Game.Beatmaps OnlineBeatmapSetID = model.OnlineID, Metadata = new BeatmapMetadata { - Title = model.Metadata?.Title, - Artist = model.Metadata?.Artist, - TitleUnicode = model.Metadata?.TitleUnicode, - ArtistUnicode = model.Metadata?.ArtistUnicode, + Title = model.Metadata?.Title ?? string.Empty, + Artist = model.Metadata?.Artist ?? string.Empty, + TitleUnicode = model.Metadata?.TitleUnicode ?? string.Empty, + ArtistUnicode = model.Metadata?.ArtistUnicode ?? string.Empty, Author = new User { Username = model.Metadata?.Author }, } }, minimiseDownloadSize); diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs index 89541a0845..3ed5055b7f 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs @@ -82,7 +82,7 @@ namespace osu.Game.Beatmaps.Formats { writer.WriteLine("[General]"); - if (beatmap.Metadata.AudioFile != null) writer.WriteLine(FormattableString.Invariant($"AudioFilename: {Path.GetFileName(beatmap.Metadata.AudioFile)}")); + if (!string.IsNullOrEmpty(beatmap.Metadata.AudioFile)) writer.WriteLine(FormattableString.Invariant($"AudioFilename: {Path.GetFileName(beatmap.Metadata.AudioFile)}")); writer.WriteLine(FormattableString.Invariant($"AudioLeadIn: {beatmap.BeatmapInfo.AudioLeadIn}")); writer.WriteLine(FormattableString.Invariant($"PreviewTime: {beatmap.Metadata.PreviewTime}")); writer.WriteLine(FormattableString.Invariant($"Countdown: {(int)beatmap.BeatmapInfo.Countdown}")); @@ -126,13 +126,13 @@ namespace osu.Game.Beatmaps.Formats writer.WriteLine("[Metadata]"); writer.WriteLine(FormattableString.Invariant($"Title: {beatmap.Metadata.Title}")); - if (beatmap.Metadata.TitleUnicode != null) writer.WriteLine(FormattableString.Invariant($"TitleUnicode: {beatmap.Metadata.TitleUnicode}")); + if (!string.IsNullOrEmpty(beatmap.Metadata.TitleUnicode)) writer.WriteLine(FormattableString.Invariant($"TitleUnicode: {beatmap.Metadata.TitleUnicode}")); writer.WriteLine(FormattableString.Invariant($"Artist: {beatmap.Metadata.Artist}")); - if (beatmap.Metadata.ArtistUnicode != null) writer.WriteLine(FormattableString.Invariant($"ArtistUnicode: {beatmap.Metadata.ArtistUnicode}")); + if (!string.IsNullOrEmpty(beatmap.Metadata.ArtistUnicode)) writer.WriteLine(FormattableString.Invariant($"ArtistUnicode: {beatmap.Metadata.ArtistUnicode}")); writer.WriteLine(FormattableString.Invariant($"Creator: {beatmap.Metadata.AuthorString}")); writer.WriteLine(FormattableString.Invariant($"Version: {beatmap.BeatmapInfo.Version}")); - if (beatmap.Metadata.Source != null) writer.WriteLine(FormattableString.Invariant($"Source: {beatmap.Metadata.Source}")); - if (beatmap.Metadata.Tags != null) writer.WriteLine(FormattableString.Invariant($"Tags: {beatmap.Metadata.Tags}")); + if (!string.IsNullOrEmpty(beatmap.Metadata.Source)) writer.WriteLine(FormattableString.Invariant($"Source: {beatmap.Metadata.Source}")); + if (!string.IsNullOrEmpty(beatmap.Metadata.Tags)) writer.WriteLine(FormattableString.Invariant($"Tags: {beatmap.Metadata.Tags}")); if (beatmap.BeatmapInfo.OnlineBeatmapID != null) writer.WriteLine(FormattableString.Invariant($"BeatmapID: {beatmap.BeatmapInfo.OnlineBeatmapID}")); if (beatmap.BeatmapInfo.BeatmapSet?.OnlineBeatmapSetID != null) writer.WriteLine(FormattableString.Invariant($"BeatmapSetID: {beatmap.BeatmapInfo.BeatmapSet.OnlineBeatmapSetID}")); } diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs index e465f423bc..d41cb73a29 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs @@ -69,7 +69,7 @@ namespace osu.Game.Screens.Select.Carousel return string.Compare(BeatmapSet.Metadata.Title, otherSet.BeatmapSet.Metadata.Title, StringComparison.OrdinalIgnoreCase); case SortMode.Author: - return string.Compare(BeatmapSet.Metadata.Author.Username, otherSet.BeatmapSet.Metadata.Author.Username, StringComparison.OrdinalIgnoreCase); + return string.Compare(BeatmapSet.Metadata.Author?.Username, otherSet.BeatmapSet.Metadata.Author?.Username, StringComparison.OrdinalIgnoreCase); case SortMode.Source: return string.Compare(BeatmapSet.Metadata.Source, otherSet.BeatmapSet.Metadata.Source, StringComparison.OrdinalIgnoreCase); From 41854f2e16890d34a1ece354e507119556660697 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 4 Nov 2021 14:01:01 +0900 Subject: [PATCH 06/14] Fix incorrect check on `AudioFile` presence --- osu.Game/Beatmaps/WorkingBeatmapCache.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmapCache.cs b/osu.Game/Beatmaps/WorkingBeatmapCache.cs index 65f84984c2..40ae627619 100644 --- a/osu.Game/Beatmaps/WorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/WorkingBeatmapCache.cs @@ -165,7 +165,7 @@ namespace osu.Game.Beatmaps protected override Track GetBeatmapTrack() { - if (Metadata?.AudioFile == null) + if (string.IsNullOrEmpty(Metadata?.AudioFile)) return null; try @@ -181,7 +181,7 @@ namespace osu.Game.Beatmaps protected override Waveform GetWaveform() { - if (Metadata?.AudioFile == null) + if (string.IsNullOrEmpty(Metadata?.AudioFile)) return null; try From 1e73b09e578cb949bfee28829aef68042216f201 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 4 Nov 2021 14:11:21 +0900 Subject: [PATCH 07/14] Fix another couple of cases of incorrect string null/empty checking --- osu.Game/Beatmaps/WorkingBeatmapCache.cs | 2 +- osu.Game/Rulesets/Edit/Checks/CheckAudioQuality.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmapCache.cs b/osu.Game/Beatmaps/WorkingBeatmapCache.cs index 40ae627619..11257e3abc 100644 --- a/osu.Game/Beatmaps/WorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/WorkingBeatmapCache.cs @@ -149,7 +149,7 @@ namespace osu.Game.Beatmaps protected override Texture GetBackground() { - if (Metadata?.BackgroundFile == null) + if (string.IsNullOrEmpty(Metadata?.BackgroundFile)) return null; try diff --git a/osu.Game/Rulesets/Edit/Checks/CheckAudioQuality.cs b/osu.Game/Rulesets/Edit/Checks/CheckAudioQuality.cs index 08e0312b64..ec2ff68aad 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckAudioQuality.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckAudioQuality.cs @@ -28,7 +28,7 @@ namespace osu.Game.Rulesets.Edit.Checks public IEnumerable Run(BeatmapVerifierContext context) { string audioFile = context.Beatmap.Metadata?.AudioFile; - if (audioFile == null) + if (string.IsNullOrEmpty(audioFile)) yield break; var track = context.WorkingBeatmap.Track; From d1e6d1cb987a3d825a4c3923eceeeadfc544ec3e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 4 Nov 2021 14:50:39 +0900 Subject: [PATCH 08/14] Update some other missed incorrect null/empty usages --- .../Editing/Checks/CheckBackgroundQualityTest.cs | 2 +- .../Editing/Checks/CheckFilePresenceTest.cs | 2 +- .../NonVisual/Filtering/FilterMatchingTest.cs | 2 +- .../Visual/Editing/TestSceneMetadataSection.cs | 12 ++++++------ .../Visual/Gameplay/TestSceneReplayRecording.cs | 4 ++-- .../Select/Carousel/DrawableCarouselBeatmap.cs | 2 +- osu.Game/Storyboards/Storyboard.cs | 4 ++-- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/osu.Game.Tests/Editing/Checks/CheckBackgroundQualityTest.cs b/osu.Game.Tests/Editing/Checks/CheckBackgroundQualityTest.cs index 3424cfe732..2918dde2db 100644 --- a/osu.Game.Tests/Editing/Checks/CheckBackgroundQualityTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckBackgroundQualityTest.cs @@ -53,7 +53,7 @@ namespace osu.Game.Tests.Editing.Checks public void TestMissing() { // While this is a problem, it is out of scope for this check and is caught by a different one. - beatmap.Metadata.BackgroundFile = null; + beatmap.Metadata.BackgroundFile = string.Empty; var context = getContext(null, System.Array.Empty()); Assert.That(check.Run(context), Is.Empty); diff --git a/osu.Game.Tests/Editing/Checks/CheckFilePresenceTest.cs b/osu.Game.Tests/Editing/Checks/CheckFilePresenceTest.cs index 39a1d76d83..70e4c76b19 100644 --- a/osu.Game.Tests/Editing/Checks/CheckFilePresenceTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckFilePresenceTest.cs @@ -65,7 +65,7 @@ namespace osu.Game.Tests.Editing.Checks [Test] public void TestBackgroundNotSet() { - beatmap.Metadata.BackgroundFile = null; + beatmap.Metadata.BackgroundFile = string.Empty; var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap)); var issues = check.Run(context).ToList(); diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs index ed86daf8b6..41939cec3f 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs @@ -189,7 +189,7 @@ namespace osu.Game.Tests.NonVisual.Filtering public void TestCriteriaMatchingArtistWithNullUnicodeName(string artistName, bool filtered) { var exampleBeatmapInfo = getExampleBeatmap(); - exampleBeatmapInfo.Metadata.ArtistUnicode = null; + exampleBeatmapInfo.Metadata.ArtistUnicode = string.Empty; var criteria = new FilterCriteria { diff --git a/osu.Game.Tests/Visual/Editing/TestSceneMetadataSection.cs b/osu.Game.Tests/Visual/Editing/TestSceneMetadataSection.cs index 19081f3281..4621436cc6 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneMetadataSection.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneMetadataSection.cs @@ -23,10 +23,10 @@ namespace osu.Game.Tests.Visual.Editing AddStep("set metadata", () => { editorBeatmap.Metadata.Artist = "Example Artist"; - editorBeatmap.Metadata.ArtistUnicode = null; + editorBeatmap.Metadata.ArtistUnicode = string.Empty; editorBeatmap.Metadata.Title = "Example Title"; - editorBeatmap.Metadata.TitleUnicode = null; + editorBeatmap.Metadata.TitleUnicode = string.Empty; }); createSection(); @@ -44,10 +44,10 @@ namespace osu.Game.Tests.Visual.Editing AddStep("set metadata", () => { editorBeatmap.Metadata.ArtistUnicode = "＊なみりん"; - editorBeatmap.Metadata.Artist = null; + editorBeatmap.Metadata.Artist = string.Empty; editorBeatmap.Metadata.TitleUnicode = "コイシテイク・プラネット"; - editorBeatmap.Metadata.Title = null; + editorBeatmap.Metadata.Title = string.Empty; }); createSection(); @@ -86,10 +86,10 @@ namespace osu.Game.Tests.Visual.Editing AddStep("set metadata", () => { editorBeatmap.Metadata.ArtistUnicode = "＊なみりん"; - editorBeatmap.Metadata.Artist = null; + editorBeatmap.Metadata.Artist = string.Empty; editorBeatmap.Metadata.TitleUnicode = "コイシテイク・プラネット"; - editorBeatmap.Metadata.Title = null; + editorBeatmap.Metadata.Title = string.Empty; }); createSection(); diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecording.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecording.cs index 3545fc96e8..08578168d6 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecording.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecording.cs @@ -54,7 +54,7 @@ namespace osu.Game.Tests.Visual.Gameplay ScoreInfo = { BeatmapInfo = gameplayState.Beatmap.BeatmapInfo } }) { - ScreenSpaceToGamefield = pos => recordingManager.ToLocalSpace(pos) + ScreenSpaceToGamefield = pos => recordingManager?.ToLocalSpace(pos) ?? Vector2.Zero, }, Child = new Container { @@ -84,7 +84,7 @@ namespace osu.Game.Tests.Visual.Gameplay { ReplayInputHandler = new TestFramedReplayInputHandler(replay) { - GamefieldToScreenSpace = pos => playbackManager.ToScreenSpace(pos), + GamefieldToScreenSpace = pos => playbackManager?.ToScreenSpace(pos) ?? Vector2.Zero, }, Child = new Container { diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs index 5940911d4a..8a5dde961f 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs @@ -142,7 +142,7 @@ namespace osu.Game.Screens.Select.Carousel }, new OsuSpriteText { - Text = $"{(beatmapInfo.Metadata ?? beatmapInfo.BeatmapSet.Metadata).Author.Username}", + Text = $"{(beatmapInfo.Metadata ?? beatmapInfo.BeatmapSet.Metadata).Author?.Username ?? string.Empty}", Font = OsuFont.GetFont(italics: true), Anchor = Anchor.BottomLeft, Origin = Anchor.BottomLeft diff --git a/osu.Game/Storyboards/Storyboard.cs b/osu.Game/Storyboards/Storyboard.cs index a25bf24491..99123627bd 100644 --- a/osu.Game/Storyboards/Storyboard.cs +++ b/osu.Game/Storyboards/Storyboard.cs @@ -77,8 +77,8 @@ namespace osu.Game.Storyboards { get { - string backgroundPath = BeatmapInfo.BeatmapSet?.Metadata?.BackgroundFile?.ToLowerInvariant(); - if (backgroundPath == null) + string backgroundPath = BeatmapInfo.BeatmapSet?.Metadata?.BackgroundFile.ToLowerInvariant(); + if (string.IsNullOrEmpty(backgroundPath)) return false; return GetLayer("Background").Elements.Any(e => e.Path.ToLowerInvariant() == backgroundPath); From 09f9731d74fb021188b98190dac6bfba0b105cc4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 4 Nov 2021 15:09:04 +0900 Subject: [PATCH 09/14] Add temporary disable of failing r# inspection --- osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index d37e09aa29..7a95856c36 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -673,6 +673,8 @@ namespace osu.Game.Tests.Beatmaps.Formats Assert.That(first.ControlPoints[1].Position, Is.EqualTo(new Vector2(161, -244))); Assert.That(first.ControlPoints[1].Type, Is.EqualTo(null)); + // ReSharper disable once HeuristicUnreachableCode + // weird one, see https://youtrack.jetbrains.com/issue/RIDER-70159. Assert.That(first.ControlPoints[2].Position, Is.EqualTo(new Vector2(376, -3))); Assert.That(first.ControlPoints[2].Type, Is.EqualTo(PathType.Bezier)); Assert.That(first.ControlPoints[3].Position, Is.EqualTo(new Vector2(68, 15))); From e43d91ad5d82cc338802f312411a24abe411f3bc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 4 Nov 2021 15:18:10 +0900 Subject: [PATCH 10/14] Fix another case of incorrect null checking in editor verification processing --- osu.Game/Rulesets/Edit/Checks/CheckFilePresence.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckFilePresence.cs b/osu.Game/Rulesets/Edit/Checks/CheckFilePresence.cs index abedee143a..33bcac1e75 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckFilePresence.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckFilePresence.cs @@ -25,7 +25,7 @@ namespace osu.Game.Rulesets.Edit.Checks { string filename = GetFilename(context.Beatmap); - if (filename == null) + if (string.IsNullOrEmpty(filename)) { yield return new IssueTemplateNoneSet(this).Create(TypeOfFile); From 09701d0af12f8ae0f05fdf7fc6e947216ed40388 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 4 Nov 2021 16:02:37 +0900 Subject: [PATCH 11/14] Use explicit primitive type specification --- osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index 42792bdf5b..29e3f12d03 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -114,7 +114,7 @@ namespace osu.Game.Screens.Edit.Compose.Components if (selectionPerformed) { // only unmodified right click should show context menu - var shouldShowContextMenu = e.Button == MouseButton.Right && !e.ShiftPressed && !e.AltPressed && !e.SuperPressed; + bool shouldShowContextMenu = e.Button == MouseButton.Right && !e.ShiftPressed && !e.AltPressed && !e.SuperPressed; // stop propagation if not showing context menu return !shouldShowContextMenu; From 0c14d8af23cd4f0b89846da362dea637ce3d8b0e Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 4 Nov 2021 16:03:15 +0900 Subject: [PATCH 12/14] Upgrade to nvika 1.0.4 --- .config/dotnet-tools.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index 1b06aa4d17..d64ad6ad21 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -15,7 +15,7 @@ ] }, "smoogipoo.nvika": { - "version": "1.0.3", + "version": "1.0.4", "commands": [ "nvika" ] From 317506d4d7a472bedba1586bb2ebbad7e089d7f6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 4 Nov 2021 16:11:23 +0900 Subject: [PATCH 13/14] Fix a few more "maybe null" inspections --- .../Online/TestAPIModJsonSerialization.cs | 15 +++++++++------ osu.Game/Screens/Select/BeatmapDeleteDialog.cs | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Online/TestAPIModJsonSerialization.cs b/osu.Game.Tests/Online/TestAPIModJsonSerialization.cs index c8848ab7d8..656e333073 100644 --- a/osu.Game.Tests/Online/TestAPIModJsonSerialization.cs +++ b/osu.Game.Tests/Online/TestAPIModJsonSerialization.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.Collections.Generic; using Newtonsoft.Json; using NUnit.Framework; @@ -67,9 +68,11 @@ namespace osu.Game.Tests.Online var deserialised = JsonConvert.DeserializeObject(JsonConvert.SerializeObject(apiMod)); var converted = (TestModTimeRamp)deserialised?.ToMod(new TestRuleset()); - Assert.That(converted?.AdjustPitch.Value, Is.EqualTo(false)); - Assert.That(converted?.InitialRate.Value, Is.EqualTo(1.25)); - Assert.That(converted?.FinalRate.Value, Is.EqualTo(0.25)); + Assert.That(converted, Is.Not.Null); + + Assert.That(converted.AdjustPitch.Value, Is.EqualTo(false)); + Assert.That(converted.InitialRate.Value, Is.EqualTo(1.25)); + Assert.That(converted.FinalRate.Value, Is.EqualTo(0.25)); } [Test] @@ -121,11 +124,11 @@ namespace osu.Game.Tests.Online new TestModDifficultyAdjust() }; - public override DrawableRuleset CreateDrawableRulesetWith(IBeatmap beatmap, IReadOnlyList mods = null) => throw new System.NotImplementedException(); + public override DrawableRuleset CreateDrawableRulesetWith(IBeatmap beatmap, IReadOnlyList mods = null) => throw new NotImplementedException(); - public override IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap) => throw new System.NotImplementedException(); + public override IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap) => throw new NotImplementedException(); - public override DifficultyCalculator CreateDifficultyCalculator(WorkingBeatmap beatmap) => throw new System.NotImplementedException(); + public override DifficultyCalculator CreateDifficultyCalculator(WorkingBeatmap beatmap) => throw new NotImplementedException(); public override string Description { get; } = string.Empty; public override string ShortName { get; } = string.Empty; diff --git a/osu.Game/Screens/Select/BeatmapDeleteDialog.cs b/osu.Game/Screens/Select/BeatmapDeleteDialog.cs index 5fb72e4151..307c2352e3 100644 --- a/osu.Game/Screens/Select/BeatmapDeleteDialog.cs +++ b/osu.Game/Screens/Select/BeatmapDeleteDialog.cs @@ -29,7 +29,7 @@ namespace osu.Game.Screens.Select new PopupDialogOkButton { Text = @"Yes. Totally. Delete it.", - Action = () => manager.Delete(beatmap), + Action = () => manager?.Delete(beatmap), }, new PopupDialogCancelButton { From b80c02b757346a99012949d9d227fb876c8f3bcd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 4 Nov 2021 17:24:40 +0900 Subject: [PATCH 14/14] Fix crash on gameplay startup if beatmap has no background --- osu.Game/Storyboards/Storyboard.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Storyboards/Storyboard.cs b/osu.Game/Storyboards/Storyboard.cs index 99123627bd..9e3d3df915 100644 --- a/osu.Game/Storyboards/Storyboard.cs +++ b/osu.Game/Storyboards/Storyboard.cs @@ -77,10 +77,14 @@ namespace osu.Game.Storyboards { get { - string backgroundPath = BeatmapInfo.BeatmapSet?.Metadata?.BackgroundFile.ToLowerInvariant(); + string backgroundPath = BeatmapInfo.BeatmapSet?.Metadata?.BackgroundFile; + if (string.IsNullOrEmpty(backgroundPath)) return false; + // Importantly, do this after the NullOrEmpty because EF may have stored the non-nullable value as null to the database, bypassing compile-time constraints. + backgroundPath = backgroundPath.ToLowerInvariant(); + return GetLayer("Background").Elements.Any(e => e.Path.ToLowerInvariant() == backgroundPath); } } @@ -93,7 +97,7 @@ namespace osu.Game.Storyboards Drawable drawable = null; string storyboardPath = BeatmapInfo.BeatmapSet?.Files.Find(f => f.Filename.Equals(path, StringComparison.OrdinalIgnoreCase))?.FileInfo.StoragePath; - if (storyboardPath != null) + if (!string.IsNullOrEmpty(storyboardPath)) drawable = new Sprite { Texture = textureStore.Get(storyboardPath) }; // if the texture isn't available locally in the beatmap, some storyboards choose to source from the underlying skin lookup hierarchy. else if (UseSkinSprites)