Merge pull request #17191 from peppy/fix-mod-conversion-exceptions

Change `ToMod` to return an `UnknownMod` rather than throw if a mod isn't available

osu.Game.Tests/Online/TestAPIModJsonSerialization.cs

@@ -24,6 +24,20 @@ namespace osu.Game.Tests.Online
     [TestFixture]
     public class TestAPIModJsonSerialization
     {
+        [Test]
+        public void TestUnknownMod()
+        {
+            var apiMod = new APIMod { Acronym = "WNG" };
+
+            var deserialized = JsonConvert.DeserializeObject<APIMod>(JsonConvert.SerializeObject(apiMod));
+
+            var converted = deserialized?.ToMod(new TestRuleset());
+
+            Assert.That(converted, Is.TypeOf(typeof(UnknownMod)));
+            Assert.That(converted?.Type, Is.EqualTo(ModType.System));
+            Assert.That(converted?.Acronym, Is.EqualTo("WNG??"));
+        }
+
         [Test]
         public void TestAcronymIsPreserved()
         {

osu.Game.Tests/Visual/Gameplay/TestSceneUnknownMod.cs

@@ -0,0 +1,29 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using NUnit.Framework;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Mods;
+using osu.Game.Rulesets.Osu;
+
+namespace osu.Game.Tests.Visual.Gameplay
+{
+    public class TestSceneUnknownMod : ModTestScene
+    {
+        protected override Ruleset CreatePlayerRuleset() => new OsuRuleset();
+
+        /// <summary>
+        /// This test also covers the scenario of exiting Player after an unsuccessful beatmap load.
+        /// </summary>
+        [Test]
+        public void TestUnknownModDoesntEnterGameplay()
+        {
+            CreateModTest(new ModTestData
+            {
+                Beatmap = CreateWorkingBeatmap(new OsuRuleset().RulesetInfo).Beatmap,
+                Mod = new UnknownMod("WNG"),
+                PassCondition = () => Player.IsLoaded && !Player.LoadedBeatmapSuccessfully
+            });
+        }
+    }
+}

osu.Game/Online/API/APIMod.cs

@@ -8,6 +8,7 @@ using Humanizer;
 using MessagePack;
 using Newtonsoft.Json;
 using osu.Framework.Bindables;
+using osu.Framework.Logging;
 using osu.Game.Configuration;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
@@ -51,7 +52,10 @@ namespace osu.Game.Online.API
             Mod resultMod = ruleset.CreateModFromAcronym(Acronym);
 
             if (resultMod == null)
-                throw new InvalidOperationException($"There is no mod in the ruleset ({ruleset.ShortName}) matching the acronym {Acronym}.");
+            {
+                Logger.Log($"There is no mod in the ruleset ({ruleset.ShortName}) matching the acronym {Acronym}.");
+                return new UnknownMod(Acronym);
+            }
 
             if (Settings.Count > 0)
             {
@@ -98,4 +102,5 @@ namespace osu.Game.Online.API
             public int GetHashCode(KeyValuePair<string, object> obj) => HashCode.Combine(obj.Key, ModUtils.GetSettingUnderlyingValue(obj.Value));
         }
     }
+
 }

osu.Game/Rulesets/Mods/UnknownMod.cs

@@ -0,0 +1,29 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+namespace osu.Game.Rulesets.Mods
+{
+    public class UnknownMod : Mod
+    {
+        /// <summary>
+        /// The acronym of the mod which could not be resolved.
+        /// </summary>
+        public readonly string OriginalAcronym;
+
+        public override string Name => $"Unknown mod ({OriginalAcronym})";
+        public override string Acronym => $"{OriginalAcronym}??";
+        public override string Description => "This mod could not be resolved by the game.";
+        public override double ScoreMultiplier => 0;
+
+        public override bool UserPlayable => false;
+
+        public override ModType Type => ModType.System;
+
+        public UnknownMod(string acronym)
+        {
+            OriginalAcronym = acronym;
+        }
+
+        public override Mod DeepClone() => new UnknownMod(OriginalAcronym);
+    }
+}

osu.Game/Screens/Play/Player.cs

@@ -185,6 +185,12 @@ namespace osu.Game.Screens.Play
         {
             var gameplayMods = Mods.Value.Select(m => m.DeepClone()).ToArray();
 
+            if (gameplayMods.Any(m => m is UnknownMod))
+            {
+                Logger.Log("Gameplay was started with an unknown mod applied.", level: LogLevel.Important);
+                return;
+            }
+
             if (Beatmap.Value is DummyWorkingBeatmap)
                 return;
 
@@ -988,24 +994,27 @@ namespace osu.Game.Screens.Play
 
         public override bool OnExiting(IScreen next)
         {
-            if (!GameplayState.HasPassed && !GameplayState.HasFailed)
-                GameplayState.HasQuit = true;
-
             screenSuspension?.RemoveAndDisposeImmediately();
             failAnimationLayer?.RemoveFilters();
 
-            // if arriving here and the results screen preparation task hasn't run, it's safe to say the user has not completed the beatmap.
+            if (LoadedBeatmapSuccessfully)
-            if (prepareScoreForDisplayTask == null)
             {
-                Score.ScoreInfo.Passed = false;
+                if (!GameplayState.HasPassed && !GameplayState.HasFailed)
-                // potentially should be ScoreRank.F instead? this is the best alternative for now.
+                    GameplayState.HasQuit = true;
-                Score.ScoreInfo.Rank = ScoreRank.D;
-            }
 
-            // EndPlaying() is typically called from ReplayRecorder.Dispose(). Disposal is currently asynchronous.
+                // if arriving here and the results screen preparation task hasn't run, it's safe to say the user has not completed the beatmap.
-            // To resolve test failures, forcefully end playing synchronously when this screen exits.
+                if (prepareScoreForDisplayTask == null)
-            // Todo: Replace this with a more permanent solution once osu-framework has a synchronous cleanup method.
+                {
-            spectatorClient.EndPlaying(GameplayState);
+                    Score.ScoreInfo.Passed = false;
+                    // potentially should be ScoreRank.F instead? this is the best alternative for now.
+                    Score.ScoreInfo.Rank = ScoreRank.D;
+                }
+
+                // EndPlaying() is typically called from ReplayRecorder.Dispose(). Disposal is currently asynchronous.
+                // To resolve test failures, forcefully end playing synchronously when this screen exits.
+                // Todo: Replace this with a more permanent solution once osu-framework has a synchronous cleanup method.
+                spectatorClient.EndPlaying(GameplayState);
+            }
 
             // GameplayClockContainer performs seeks / start / stop operations on the beatmap's track.
             // as we are no longer the current screen, we cannot guarantee the track is still usable.

osu.Game/Screens/Play/SubmittingPlayer.cs

@@ -119,7 +119,8 @@ namespace osu.Game.Screens.Play
         {
             bool exiting = base.OnExiting(next);
 
-            submitScore(Score.DeepClone());
+            if (LoadedBeatmapSuccessfully)
+                submitScore(Score.DeepClone());
 
             return exiting;
         }