From 447371478ea6f4843b57a409ebbfab1db1d15e78 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 14 May 2021 12:03:06 +0900 Subject: [PATCH 01/17] 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/17] 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 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 03/17] 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 04/17] 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 05/17] 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 06/17] 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 07/17] 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 08/17] 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 09/17] 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 10/17] 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 11/17] 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 12/17] 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 13/17] 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 14/17] 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 15/17] 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 16/17] 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 17/17] 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(); } }