Merge pull request #3239 from smoogipoo/spinner-combo-fix

Fix spinners creating an extra combo

osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapConverter.cs

@@ -12,7 +12,7 @@ using osu.Game.Rulesets.Osu.UI;
 
 namespace osu.Game.Rulesets.Osu.Beatmaps
 {
-    internal class OsuBeatmapConverter : BeatmapConverter<OsuHitObject>
+    public class OsuBeatmapConverter : BeatmapConverter<OsuHitObject>
     {
         public OsuBeatmapConverter(IBeatmap beatmap)
             : base(beatmap)

osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs

@@ -8,7 +8,7 @@ using osu.Game.Rulesets.Osu.Objects;
 
 namespace osu.Game.Rulesets.Osu.Beatmaps
 {
-    internal class OsuBeatmapProcessor : BeatmapProcessor
+    public class OsuBeatmapProcessor : BeatmapProcessor
     {
         public OsuBeatmapProcessor(IBeatmap beatmap)
             : base(beatmap)

osu.Game.Rulesets.Osu/Objects/Spinner.cs

@@ -20,8 +20,6 @@ namespace osu.Game.Rulesets.Osu.Objects
         /// </summary>
         public int SpinsRequired { get; protected set; } = 1;
 
-        public override bool NewCombo => true;
-
         protected override void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, BeatmapDifficulty difficulty)
         {
             base.ApplyDefaultsToSelf(controlPointInfo, difficulty);

osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs

@@ -11,7 +11,9 @@ using osu.Game.Audio;
 using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Beatmaps.Formats;
 using osu.Game.Beatmaps.Timing;
+using osu.Game.Rulesets.Catch.Beatmaps;
 using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Osu.Beatmaps;
 using osu.Game.Skinning;
 
 namespace osu.Game.Tests.Beatmaps.Formats
@@ -187,14 +189,46 @@ namespace osu.Game.Tests.Beatmaps.Formats
         }
 
         [Test]
-        public void TestDecodeBeatmapComboOffsets()
+        public void TestDecodeBeatmapComboOffsetsOsu()
         {
             var decoder = new LegacyBeatmapDecoder();
             using (var resStream = Resource.OpenResource("hitobject-combo-offset.osu"))
             using (var stream = new StreamReader(resStream))
             {
                 var beatmap = decoder.Decode(stream);
-                Assert.AreEqual(3, ((IHasCombo)beatmap.HitObjects[0]).ComboOffset);
+
+                var converted = new OsuBeatmapConverter(beatmap).Convert();
+                new OsuBeatmapProcessor(converted).PreProcess();
+                new OsuBeatmapProcessor(converted).PostProcess();
+
+                Assert.AreEqual(4, ((IHasComboInformation)converted.HitObjects.ElementAt(0)).ComboIndex);
+                Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(2)).ComboIndex);
+                Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(4)).ComboIndex);
+                Assert.AreEqual(6, ((IHasComboInformation)converted.HitObjects.ElementAt(6)).ComboIndex);
+                Assert.AreEqual(11, ((IHasComboInformation)converted.HitObjects.ElementAt(8)).ComboIndex);
+                Assert.AreEqual(14, ((IHasComboInformation)converted.HitObjects.ElementAt(11)).ComboIndex);
+            }
+        }
+
+        [Test]
+        public void TestDecodeBeatmapComboOffsetsCatch()
+        {
+            var decoder = new LegacyBeatmapDecoder();
+            using (var resStream = Resource.OpenResource("hitobject-combo-offset.osu"))
+            using (var stream = new StreamReader(resStream))
+            {
+                var beatmap = decoder.Decode(stream);
+
+                var converted = new CatchBeatmapConverter(beatmap).Convert();
+                new CatchBeatmapProcessor(converted).PreProcess();
+                new CatchBeatmapProcessor(converted).PostProcess();
+
+                Assert.AreEqual(4, ((IHasComboInformation)converted.HitObjects.ElementAt(0)).ComboIndex);
+                Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(2)).ComboIndex);
+                Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(4)).ComboIndex);
+                Assert.AreEqual(6, ((IHasComboInformation)converted.HitObjects.ElementAt(6)).ComboIndex);
+                Assert.AreEqual(11, ((IHasComboInformation)converted.HitObjects.ElementAt(8)).ComboIndex);
+                Assert.AreEqual(14, ((IHasComboInformation)converted.HitObjects.ElementAt(11)).ComboIndex);
             }
         }
 

osu.Game.Tests/Resources/hitobject-combo-offset.osu

@@ -1,4 +1,32 @@
 osu file format v14
 
 [HitObjects]
-255,193,2170,49,0,0:0:0:0:
+// Circle with combo offset (3)
+255,193,1000,49,0,0:0:0:0:
+// Combo index = 4
+
+// Slider with new combo followed by circle with no new combo
+256,192,2000,12,0,2000,0:0:0:0:
+255,193,3000,1,0,0:0:0:0:
+// Combo index = 5
+
+// Slider without new combo followed by circle with no new combo
+256,192,4000,8,0,5000,0:0:0:0:
+255,193,6000,1,0,0:0:0:0:
+// Combo index = 5
+
+// Slider without new combo followed by circle with new combo
+256,192,7000,8,0,8000,0:0:0:0:
+255,193,9000,5,0,0:0:0:0:
+// Combo index = 6
+
+// Slider with new combo and offset (1) followed by circle with new combo and offset (3)
+256,192,10000,28,0,11000,0:0:0:0:
+255,193,12000,53,0,0:0:0:0:
+// Combo index = 11
+
+// Slider with new combo and offset (2) followed by slider with no new combo followed by circle with no new combo
+256,192,13000,44,0,14000,0:0:0:0:
+256,192,15000,8,0,16000,0:0:0:0:
+255,193,17000,1,0,0:0:0:0:
+// Combo index = 14

osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs

@@ -18,8 +18,17 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch
         {
         }
 
+        private bool forceNewCombo;
+        private int extraComboOffset;
+
         protected override HitObject CreateHit(Vector2 position, bool newCombo, int comboOffset)
         {
+            newCombo |= forceNewCombo;
+            comboOffset += extraComboOffset;
+
+            forceNewCombo = false;
+            extraComboOffset = 0;
+
             return new ConvertHit
             {
                 X = position.X,
@@ -30,6 +39,12 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch
 
         protected override HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, List<Vector2> controlPoints, double length, CurveType curveType, int repeatCount, List<List<SampleInfo>> repeatSamples)
         {
+            newCombo |= forceNewCombo;
+            comboOffset += extraComboOffset;
+
+            forceNewCombo = false;
+            extraComboOffset = 0;
+
             return new ConvertSlider
             {
                 X = position.X,
@@ -45,11 +60,14 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch
 
         protected override HitObject CreateSpinner(Vector2 position, bool newCombo, int comboOffset, double endTime)
         {
+            // Convert spinners don't create the new combo themselves, but force the next non-spinner hitobject to create a new combo
+            // Their combo offset is still added to that next hitobject's combo index
+            forceNewCombo |= FormatVersion <= 8 || newCombo;
+            extraComboOffset += comboOffset;
+
             return new ConvertSpinner
             {
-                EndTime = endTime,
+                EndTime = endTime
-                NewCombo = FirstObject || newCombo,
-                ComboOffset = comboOffset
             };
         }
 

osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs

@@ -19,8 +19,17 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
         {
         }
 
+        private bool forceNewCombo;
+        private int extraComboOffset;
+
         protected override HitObject CreateHit(Vector2 position, bool newCombo, int comboOffset)
         {
+            newCombo |= forceNewCombo;
+            comboOffset += extraComboOffset;
+
+            forceNewCombo = false;
+            extraComboOffset = 0;
+
             return new ConvertHit
             {
                 Position = position,
@@ -31,6 +40,12 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
 
         protected override HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, List<Vector2> controlPoints, double length, CurveType curveType, int repeatCount, List<List<SampleInfo>> repeatSamples)
         {
+            newCombo |= forceNewCombo;
+            comboOffset += extraComboOffset;
+
+            forceNewCombo = false;
+            extraComboOffset = 0;
+
             return new ConvertSlider
             {
                 Position = position,
@@ -46,12 +61,15 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
 
         protected override HitObject CreateSpinner(Vector2 position, bool newCombo, int comboOffset, double endTime)
         {
+            // Convert spinners don't create the new combo themselves, but force the next non-spinner hitobject to create a new combo
+            // Their combo offset is still added to that next hitobject's combo index
+            forceNewCombo |= FormatVersion <= 8 || newCombo;
+            extraComboOffset += comboOffset;
+
             return new ConvertSpinner
             {
                 Position = position,
-                EndTime = endTime,
+                EndTime = endTime
-                NewCombo = FormatVersion <= 8 || FirstObject || newCombo,
-                ComboOffset = comboOffset
             };
         }