Merge pull request #9377 from smoogipoo/fix-unsafe-mod-deserialisation

Fix crash due to unsafe mod deserialisation

osu.Game.Tests/Online/TestAPIModSerialization.cs

@@ -49,9 +49,32 @@ namespace osu.Game.Tests.Online
             Assert.That(converted.TestSetting.Value, Is.EqualTo(2));
         }
 
+        [Test]
+        public void TestDeserialiseTimeRampMod()
+        {
+            // Create the mod with values different from default.
+            var apiMod = new APIMod(new TestModTimeRamp
+            {
+                AdjustPitch = { Value = false },
+                InitialRate = { Value = 1.25 },
+                FinalRate = { Value = 0.25 }
+            });
+
+            var deserialised = JsonConvert.DeserializeObject<APIMod>(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));
+        }
+
         private class TestRuleset : Ruleset
         {
-            public override IEnumerable<Mod> GetModsFor(ModType type) => new[] { new TestMod() };
+            public override IEnumerable<Mod> GetModsFor(ModType type) => new Mod[]
+            {
+                new TestMod(),
+                new TestModTimeRamp(),
+            };
 
             public override DrawableRuleset CreateDrawableRulesetWith(IBeatmap beatmap, IReadOnlyList<Mod> mods = null) => throw new System.NotImplementedException();
 
@@ -78,5 +101,39 @@ namespace osu.Game.Tests.Online
                 Precision = 0.01,
             };
         }
+
+        private class TestModTimeRamp : ModTimeRamp
+        {
+            public override string Name => "Test Mod";
+            public override string Acronym => "TMTR";
+            public override double ScoreMultiplier => 1;
+
+            [SettingSource("Initial rate", "The starting speed of the track")]
+            public override BindableNumber<double> InitialRate { get; } = new BindableDouble
+            {
+                MinValue = 1,
+                MaxValue = 2,
+                Default = 1.5,
+                Value = 1.5,
+                Precision = 0.01,
+            };
+
+            [SettingSource("Final rate", "The speed increase to ramp towards")]
+            public override BindableNumber<double> FinalRate { get; } = new BindableDouble
+            {
+                MinValue = 0,
+                MaxValue = 1,
+                Default = 0.5,
+                Value = 0.5,
+                Precision = 0.01,
+            };
+
+            [SettingSource("Adjust pitch", "Should pitch be adjusted with speed")]
+            public override BindableBool AdjustPitch { get; } = new BindableBool
+            {
+                Default = true,
+                Value = true
+            };
+        }
     }
 }

osu.Game/Rulesets/Mods/ModTimeRamp.cs

@@ -89,9 +89,9 @@ namespace osu.Game.Rulesets.Mods
         private void applyPitchAdjustment(ValueChangedEvent<bool> adjustPitchSetting)
         {
             // remove existing old adjustment
-            track.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), SpeedChange);
+            track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), SpeedChange);
 
-            track.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), SpeedChange);
+            track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), SpeedChange);
         }
 
         private AdjustableProperty adjustmentForPitchSetting(bool adjustPitchSettingValue)

osu.Game/Screens/Multi/RoomManager.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Threading.Tasks;
 using osu.Framework.Allocation;
@@ -142,6 +143,8 @@ namespace osu.Game.Screens.Multi
             joinedRoom = null;
         }
 
+        private readonly HashSet<int> ignoredRooms = new HashSet<int>();
+
         /// <summary>
         /// Invoked when the listing of all <see cref="Room"/>s is received from the server.
         /// </summary>
@@ -163,11 +166,27 @@ namespace osu.Game.Screens.Multi
                     continue;
                 }
 
-                var r = listing[i];
+                var room = listing[i];
-                r.Position.Value = i;
 
-                update(r, r);
+                Debug.Assert(room.RoomID.Value != null);
-                addRoom(r);
+
+                if (ignoredRooms.Contains(room.RoomID.Value.Value))
+                    continue;
+
+                room.Position.Value = i;
+
+                try
+                {
+                    update(room, room);
+                    addRoom(room);
+                }
+                catch (Exception ex)
+                {
+                    Logger.Error(ex, $"Failed to update room: {room.Name.Value}.");
+
+                    ignoredRooms.Add(room.RoomID.Value.Value);
+                    rooms.Remove(room);
+                }
             }
 
             RoomsUpdated?.Invoke();