mirror of https://github.com/ppy/osu
Change `ToMod` to return an `UnknownMod` rather than throw if a mod isn't available
As seen by this kind of crash, having the `.ToMod` method throw can be very problematic and also hidden (as it is used inside of models in places where exceptions are not expected to occur). Given there are tens of usages of this method, returning a placeholder mod seems like a better idea than outright throwing. ``` An unhandled has occurred. System.InvalidOperationException: There is no mod in the ruleset (osu) matching the acronym AS. at osu.Game.Online.API.APIMod.ToMod(Ruleset ruleset) in /Users/dean/Projects/osu/osu.Game/Online/API/APIMod.cs:line 54 at osu.Game.Scoring.ScoreInfo.<get_Mods>b__117_0(APIMod m) in /Users/dean/Projects/osu/osu.Game/Scoring/ScoreInfo.cs:line 193 at System.Linq.Enumerable.SelectArrayIterator`2.ToArray() at osu.Game.Scoring.ScoreInfo.get_Mods() in /Users/dean/Projects/osu/osu.Game/Scoring/ScoreInfo.cs:line 193 at osu.Game.Screens.Select.Leaderboards.BeatmapLeaderboard.<>c.<subscribeToLocalScores>b__40_2(ScoreInfo s) in /Users/dean/Projects/osu/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs:line 199 at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext() at osu.Game.Database.RealmObjectExtensions.Detach[T](IEnumerable`1 items) in /Users/dean/Projects/osu/osu.Game/Database/RealmObjectExtensions.cs:line 180 at osu.Game.Screens.Select.Leaderboards.BeatmapLeaderboard.<>c__DisplayClass40_0.<subscribeToLocalScores>g__localScoresChanged|1(IRealmCollection`1 sender, ChangeSet changes, Exception exception) in /Users/dean/Projects/osu/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs:line 209 at Realms.RealmCollectionBase`1.Realms.INotifiable<Realms.NotifiableObjectHandleBase.CollectionChangeSet>.NotifyCallbacks(Nullable`1 changes, Nullable`1 exception) at Realms.NotifiableObjectHandleBase.NotifyObjectChanged(IntPtr managedHandle, IntPtr changes, IntPtr exception) ```
This commit is contained in:
parent
aad9e856b1
commit
0267aed846
|
@ -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()
|
||||
{
|
||||
|
|
|
@ -8,6 +8,7 @@
|
|||
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 @@ public Mod ToMod(Ruleset ruleset)
|
|||
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 @@ public bool Equals(KeyValuePair<string, object> x, KeyValuePair<string, object>
|
|||
public int GetHashCode(KeyValuePair<string, object> obj) => HashCode.Combine(obj.Key, ModUtils.GetSettingUnderlyingValue(obj.Value));
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -0,0 +1,27 @@
|
|||
// 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 ModType Type => ModType.System;
|
||||
|
||||
public UnknownMod(string acronym)
|
||||
{
|
||||
OriginalAcronym = acronym;
|
||||
}
|
||||
|
||||
public override Mod DeepClone() => new UnknownMod(OriginalAcronym);
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue