Merge pull request #25551 from smoogipoo/fix-combo-handling

Fix handling of combo and combo colours around spinners

osu.Game.Rulesets.Catch/Beatmaps/CatchBeatmapProcessor.cs

@@ -23,6 +23,22 @@ namespace osu.Game.Rulesets.Catch.Beatmaps
         {
         }
 
+        public override void PreProcess()
+        {
+            IHasComboInformation? lastObj = null;
+
+            // For sanity, ensures that both the first hitobject and the first hitobject after a banana shower start a new combo.
+            // This is normally enforced by the legacy decoder, but is not enforced by the editor.
+            foreach (var obj in Beatmap.HitObjects.OfType<IHasComboInformation>())
+            {
+                if (obj is not BananaShower && (lastObj == null || lastObj is BananaShower))
+                    obj.NewCombo = true;
+                lastObj = obj;
+            }
+
+            base.PreProcess();
+        }
+
         public override void PostProcess()
         {
             base.PostProcess();

osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs

@@ -155,6 +155,33 @@ namespace osu.Game.Rulesets.Catch.Objects
             Scale = LegacyRulesetExtensions.CalculateScaleFromCircleSize(difficulty.CircleSize);
         }
 
+        public void UpdateComboInformation(IHasComboInformation? lastObj)
+        {
+            // Note that this implementation is shared with the osu! ruleset's implementation.
+            // If a change is made here, OsuHitObject.cs should also be updated.
+            ComboIndex = lastObj?.ComboIndex ?? 0;
+            ComboIndexWithOffsets = lastObj?.ComboIndexWithOffsets ?? 0;
+            IndexInCurrentCombo = (lastObj?.IndexInCurrentCombo + 1) ?? 0;
+
+            if (this is BananaShower)
+            {
+                // For the purpose of combo colours, spinners never start a new combo even if they are flagged as doing so.
+                return;
+            }
+
+            // At decode time, the first hitobject in the beatmap and the first hitobject after a banana shower are both enforced to be a new combo,
+            // but this isn't directly enforced by the editor so the extra checks against the last hitobject are duplicated here.
+            if (NewCombo || lastObj == null || lastObj is BananaShower)
+            {
+                IndexInCurrentCombo = 0;
+                ComboIndex++;
+                ComboIndexWithOffsets += ComboOffset + 1;
+
+                if (lastObj != null)
+                    lastObj.LastInCombo = true;
+            }
+        }
+
         protected override HitWindows CreateHitWindows() => HitWindows.Empty;
 
         #region Hit object conversion

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

@@ -2,9 +2,11 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Linq;
 using osu.Framework.Graphics;
 using osu.Game.Beatmaps;
 using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.Osu.Objects;
 using osuTK;
 
@@ -19,6 +21,22 @@ namespace osu.Game.Rulesets.Osu.Beatmaps
         {
         }
 
+        public override void PreProcess()
+        {
+            IHasComboInformation? lastObj = null;
+
+            // For sanity, ensures that both the first hitobject and the first hitobject after a spinner start a new combo.
+            // This is normally enforced by the legacy decoder, but is not enforced by the editor.
+            foreach (var obj in Beatmap.HitObjects.OfType<IHasComboInformation>())
+            {
+                if (obj is not Spinner && (lastObj == null || lastObj is Spinner))
+                    obj.NewCombo = true;
+                lastObj = obj;
+            }
+
+            base.PreProcess();
+        }
+
         public override void PostProcess()
         {
             base.PostProcess();
@@ -95,15 +113,15 @@ namespace osu.Game.Rulesets.Osu.Beatmaps
             {
                 int n = i;
                 /* We should check every note which has not yet got a stack.
-                    * Consider the case we have two interwound stacks and this will make sense.
+                 * Consider the case we have two interwound stacks and this will make sense.
-                    *
+                 *
-                    * o <-1      o <-2
+                 * o <-1      o <-2
-                    *  o <-3      o <-4
+                 *  o <-3      o <-4
-                    *
+                 *
-                    * We first process starting from 4 and handle 2,
+                 * We first process starting from 4 and handle 2,
-                    * then we come backwards on the i loop iteration until we reach 3 and handle 1.
+                 * then we come backwards on the i loop iteration until we reach 3 and handle 1.
-                    * 2 and 1 will be ignored in the i loop because they already have a stack value.
+                 * 2 and 1 will be ignored in the i loop because they already have a stack value.
-                    */
+                 */
 
                 OsuHitObject objectI = beatmap.HitObjects[i];
                 if (objectI.StackHeight != 0 || objectI is Spinner) continue;
@@ -111,9 +129,9 @@ namespace osu.Game.Rulesets.Osu.Beatmaps
                 double stackThreshold = objectI.TimePreempt * beatmap.BeatmapInfo.StackLeniency;
 
                 /* If this object is a hitcircle, then we enter this "special" case.
-                    * It either ends with a stack of hitcircles only, or a stack of hitcircles that are underneath a slider.
+                 * It either ends with a stack of hitcircles only, or a stack of hitcircles that are underneath a slider.
-                    * Any other case is handled by the "is Slider" code below this.
+                 * Any other case is handled by the "is Slider" code below this.
-                    */
+                 */
                 if (objectI is HitCircle)
                 {
                     while (--n >= 0)
@@ -135,10 +153,10 @@ namespace osu.Game.Rulesets.Osu.Beatmaps
                         }
 
                         /* This is a special case where hticircles are moved DOWN and RIGHT (negative stacking) if they are under the *last* slider in a stacked pattern.
-                            *    o==o <- slider is at original location
+                         *    o==o <- slider is at original location
-                            *        o <- hitCircle has stack of -1
+                         *        o <- hitCircle has stack of -1
-                            *         o <- hitCircle has stack of -2
+                         *         o <- hitCircle has stack of -2
-                            */
+                         */
                         if (objectN is Slider && Vector2Extensions.Distance(objectN.EndPosition, objectI.Position) < stack_distance)
                         {
                             int offset = objectI.StackHeight - objectN.StackHeight + 1;
@@ -169,8 +187,8 @@ namespace osu.Game.Rulesets.Osu.Beatmaps
                 else if (objectI is Slider)
                 {
                     /* We have hit the first slider in a possible stack.
-                        * From this point on, we ALWAYS stack positive regardless.
+                     * From this point on, we ALWAYS stack positive regardless.
-                        */
+                     */
                     while (--n >= startIndex)
                     {
                         OsuHitObject objectN = beatmap.HitObjects[n];

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

@@ -159,6 +159,33 @@ namespace osu.Game.Rulesets.Osu.Objects
             Scale = LegacyRulesetExtensions.CalculateScaleFromCircleSize(difficulty.CircleSize, true);
         }
 
+        public void UpdateComboInformation(IHasComboInformation? lastObj)
+        {
+            // Note that this implementation is shared with the osu!catch ruleset's implementation.
+            // If a change is made here, CatchHitObject.cs should also be updated.
+            ComboIndex = lastObj?.ComboIndex ?? 0;
+            ComboIndexWithOffsets = lastObj?.ComboIndexWithOffsets ?? 0;
+            IndexInCurrentCombo = (lastObj?.IndexInCurrentCombo + 1) ?? 0;
+
+            if (this is Spinner)
+            {
+                // For the purpose of combo colours, spinners never start a new combo even if they are flagged as doing so.
+                return;
+            }
+
+            // At decode time, the first hitobject in the beatmap and the first hitobject after a spinner are both enforced to be a new combo,
+            // but this isn't directly enforced by the editor so the extra checks against the last hitobject are duplicated here.
+            if (NewCombo || lastObj == null || lastObj is Spinner)
+            {
+                IndexInCurrentCombo = 0;
+                ComboIndex++;
+                ComboIndexWithOffsets += ComboOffset + 1;
+
+                if (lastObj != null)
+                    lastObj.LastInCombo = true;
+            }
+        }
+
         protected override HitWindows CreateHitWindows() => new OsuHitWindows();
     }
 }

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

@@ -12,6 +12,7 @@ using osu.Game.Beatmaps.Formats;
 using osu.Game.Beatmaps.Legacy;
 using osu.Game.Beatmaps.Timing;
 using osu.Game.IO;
+using osu.Game.Rulesets;
 using osu.Game.Rulesets.Catch;
 using osu.Game.Rulesets.Catch.Beatmaps;
 using osu.Game.Rulesets.Mods;
@@ -433,12 +434,12 @@ namespace osu.Game.Tests.Beatmaps.Formats
                 new OsuBeatmapProcessor(converted).PreProcess();
                 new OsuBeatmapProcessor(converted).PostProcess();
 
-                Assert.AreEqual(4, ((IHasComboInformation)converted.HitObjects.ElementAt(0)).ComboIndexWithOffsets);
+                Assert.AreEqual(1, ((IHasComboInformation)converted.HitObjects.ElementAt(0)).ComboIndexWithOffsets);
-                Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(2)).ComboIndexWithOffsets);
+                Assert.AreEqual(2, ((IHasComboInformation)converted.HitObjects.ElementAt(2)).ComboIndexWithOffsets);
-                Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(4)).ComboIndexWithOffsets);
+                Assert.AreEqual(3, ((IHasComboInformation)converted.HitObjects.ElementAt(4)).ComboIndexWithOffsets);
-                Assert.AreEqual(6, ((IHasComboInformation)converted.HitObjects.ElementAt(6)).ComboIndexWithOffsets);
+                Assert.AreEqual(4, ((IHasComboInformation)converted.HitObjects.ElementAt(6)).ComboIndexWithOffsets);
-                Assert.AreEqual(11, ((IHasComboInformation)converted.HitObjects.ElementAt(8)).ComboIndexWithOffsets);
+                Assert.AreEqual(8, ((IHasComboInformation)converted.HitObjects.ElementAt(8)).ComboIndexWithOffsets);
-                Assert.AreEqual(14, ((IHasComboInformation)converted.HitObjects.ElementAt(11)).ComboIndexWithOffsets);
+                Assert.AreEqual(9, ((IHasComboInformation)converted.HitObjects.ElementAt(11)).ComboIndexWithOffsets);
             }
         }
 
@@ -456,12 +457,12 @@ namespace osu.Game.Tests.Beatmaps.Formats
                 new CatchBeatmapProcessor(converted).PreProcess();
                 new CatchBeatmapProcessor(converted).PostProcess();
 
-                Assert.AreEqual(4, ((IHasComboInformation)converted.HitObjects.ElementAt(0)).ComboIndexWithOffsets);
+                Assert.AreEqual(1, ((IHasComboInformation)converted.HitObjects.ElementAt(0)).ComboIndexWithOffsets);
-                Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(2)).ComboIndexWithOffsets);
+                Assert.AreEqual(2, ((IHasComboInformation)converted.HitObjects.ElementAt(2)).ComboIndexWithOffsets);
-                Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(4)).ComboIndexWithOffsets);
+                Assert.AreEqual(3, ((IHasComboInformation)converted.HitObjects.ElementAt(4)).ComboIndexWithOffsets);
-                Assert.AreEqual(6, ((IHasComboInformation)converted.HitObjects.ElementAt(6)).ComboIndexWithOffsets);
+                Assert.AreEqual(4, ((IHasComboInformation)converted.HitObjects.ElementAt(6)).ComboIndexWithOffsets);
-                Assert.AreEqual(11, ((IHasComboInformation)converted.HitObjects.ElementAt(8)).ComboIndexWithOffsets);
+                Assert.AreEqual(8, ((IHasComboInformation)converted.HitObjects.ElementAt(8)).ComboIndexWithOffsets);
-                Assert.AreEqual(14, ((IHasComboInformation)converted.HitObjects.ElementAt(11)).ComboIndexWithOffsets);
+                Assert.AreEqual(9, ((IHasComboInformation)converted.HitObjects.ElementAt(11)).ComboIndexWithOffsets);
             }
         }
 
@@ -1093,5 +1094,67 @@ namespace osu.Game.Tests.Beatmaps.Formats
                 Assert.That(hitObject.Samples.Select(s => s.Volume), Has.All.EqualTo(70));
             }
         }
+
+        [Test]
+        public void TestNewComboAfterBreak()
+        {
+            var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false };
+
+            using (var resStream = TestResources.OpenResource("break-between-objects.osu"))
+            using (var stream = new LineBufferedReader(resStream))
+            {
+                var beatmap = decoder.Decode(stream);
+                Assert.That(((IHasCombo)beatmap.HitObjects[0]).NewCombo, Is.True);
+                Assert.That(((IHasCombo)beatmap.HitObjects[1]).NewCombo, Is.True);
+                Assert.That(((IHasCombo)beatmap.HitObjects[2]).NewCombo, Is.False);
+            }
+        }
+
+        /// <summary>
+        /// Test cases that involve a spinner between two hitobjects.
+        /// </summary>
+        [Test]
+        public void TestSpinnerNewComboBetweenObjects([Values("osu", "catch")] string rulesetName)
+        {
+            var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false };
+
+            using (var resStream = TestResources.OpenResource("spinner-between-objects.osu"))
+            using (var stream = new LineBufferedReader(resStream))
+            {
+                Ruleset ruleset;
+
+                switch (rulesetName)
+                {
+                    case "osu":
+                        ruleset = new OsuRuleset();
+                        break;
+
+                    case "catch":
+                        ruleset = new CatchRuleset();
+                        break;
+
+                    default:
+                        throw new ArgumentOutOfRangeException(nameof(rulesetName), rulesetName, null);
+                }
+
+                var working = new TestWorkingBeatmap(decoder.Decode(stream));
+                var playable = working.GetPlayableBeatmap(ruleset.RulesetInfo, Array.Empty<Mod>());
+
+                // There's no good way to figure out these values other than to compare (in code) with osu!stable...
+
+                Assert.That(((IHasComboInformation)playable.HitObjects[0]).ComboIndexWithOffsets, Is.EqualTo(1));
+                Assert.That(((IHasComboInformation)playable.HitObjects[2]).ComboIndexWithOffsets, Is.EqualTo(2));
+                Assert.That(((IHasComboInformation)playable.HitObjects[3]).ComboIndexWithOffsets, Is.EqualTo(2));
+                Assert.That(((IHasComboInformation)playable.HitObjects[5]).ComboIndexWithOffsets, Is.EqualTo(3));
+                Assert.That(((IHasComboInformation)playable.HitObjects[6]).ComboIndexWithOffsets, Is.EqualTo(3));
+                Assert.That(((IHasComboInformation)playable.HitObjects[8]).ComboIndexWithOffsets, Is.EqualTo(4));
+                Assert.That(((IHasComboInformation)playable.HitObjects[9]).ComboIndexWithOffsets, Is.EqualTo(4));
+                Assert.That(((IHasComboInformation)playable.HitObjects[11]).ComboIndexWithOffsets, Is.EqualTo(5));
+                Assert.That(((IHasComboInformation)playable.HitObjects[12]).ComboIndexWithOffsets, Is.EqualTo(6));
+                Assert.That(((IHasComboInformation)playable.HitObjects[14]).ComboIndexWithOffsets, Is.EqualTo(7));
+                Assert.That(((IHasComboInformation)playable.HitObjects[15]).ComboIndexWithOffsets, Is.EqualTo(8));
+                Assert.That(((IHasComboInformation)playable.HitObjects[17]).ComboIndexWithOffsets, Is.EqualTo(9));
+            }
+        }
     }
 }

osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs

@@ -94,9 +94,6 @@ namespace osu.Game.Tests.Gameplay
 
         private class TestHitObjectWithCombo : ConvertHitObject, IHasComboInformation
         {
-            public bool NewCombo { get; set; }
-            public int ComboOffset => 0;
-
             public Bindable<int> IndexInCurrentComboBindable { get; } = new Bindable<int>();
 
             public int IndexInCurrentCombo

osu.Game.Tests/Resources/break-between-objects.osu

@@ -0,0 +1,15 @@
+osu file format v14
+
+[General]
+Mode: 0
+
+[Events]
+2,200,1200
+
+[TimingPoints]
+0,307.692307692308,4,2,1,60,1,0
+
+[HitObjects]
+142,99,0,1,0,0:0:0:0:
+323,88,3000,1,0,0:0:0:0:
+323,88,4000,1,0,0:0:0:0:

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

@@ -3,30 +3,30 @@ osu file format v14
 [HitObjects]
 // Circle with combo offset (3)
 255,193,1000,49,0,0:0:0:0:
-// Combo index = 4
+// Combo index = 1
 
 // Spinner 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
+// Combo index = 2
 
 // Spinner 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
+// Combo index = 3
 
 // Spinner 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
+// Combo index = 4
 
 // Spinner 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
+// Combo index = 8
 
 // Spinner 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
+// Combo index = 9

osu.Game.Tests/Resources/spinner-between-objects.osu

@@ -0,0 +1,38 @@
+osu file format v14
+
+[General]
+Mode: 0
+
+[TimingPoints]
+0,571.428571428571,4,2,1,5,1,0
+
+[HitObjects]
+// +C -> +C -> +C
+104,95,0,5,0,0:0:0:0:
+256,192,1000,12,0,2000,0:0:0:0:
+178,171,3000,5,0,0:0:0:0:
+
+// -C -> +C -> +C
+178,171,4000,1,0,0:0:0:0:
+256,192,5000,12,0,6000,0:0:0:0:
+178,171,7000,5,0,0:0:0:0:
+
+// -C -> -C -> +C
+178,171,8000,1,0,0:0:0:0:
+256,192,9000,8,0,10000,0:0:0:0:
+178,171,11000,5,0,0:0:0:0:
+
+// -C -> -C -> -C
+178,171,12000,1,0,0:0:0:0:
+256,192,13000,8,0,14000,0:0:0:0:
+178,171,15000,1,0,0:0:0:0:
+
+// +C -> -C -> -C
+178,171,16000,5,0,0:0:0:0:
+256,192,17000,8,0,18000,0:0:0:0:
+178,171,19000,1,0,0:0:0:0:
+
+// +C -> +C -> -C
+178,171,20000,5,0,0:0:0:0:
+256,192,21000,12,0,22000,0:0:0:0:
+178,171,23000,1,0,0:0:0:0:

osu.Game/Beatmaps/BeatmapProcessor.cs

@@ -24,12 +24,6 @@ namespace osu.Game.Beatmaps
 
             foreach (var obj in Beatmap.HitObjects.OfType<IHasComboInformation>())
             {
-                if (lastObj == null)
-                {
-                    // first hitobject should always be marked as a new combo for sanity.
-                    obj.NewCombo = true;
-                }
-
                 obj.UpdateComboInformation(lastObj);
                 lastObj = obj;
             }

osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs

@@ -93,6 +93,8 @@ namespace osu.Game.Beatmaps.Formats
             // The parsing order of hitobjects matters in mania difficulty calculation
             this.beatmap.HitObjects = this.beatmap.HitObjects.OrderBy(h => h.StartTime).ToList();
 
+            postProcessBreaks(this.beatmap);
+
             foreach (var hitObject in this.beatmap.HitObjects)
             {
                 applyDefaults(hitObject);
@@ -100,6 +102,27 @@ namespace osu.Game.Beatmaps.Formats
             }
         }
 
+        /// <summary>
+        /// Processes the beatmap such that a new combo is started the first hitobject following each break.
+        /// </summary>
+        private void postProcessBreaks(Beatmap beatmap)
+        {
+            int currentBreak = 0;
+            bool forceNewCombo = false;
+
+            foreach (var h in beatmap.HitObjects.OfType<ConvertHitObject>())
+            {
+                while (currentBreak < beatmap.Breaks.Count && beatmap.Breaks[currentBreak].EndTime < h.StartTime)
+                {
+                    forceNewCombo = true;
+                    currentBreak++;
+                }
+
+                h.NewCombo |= forceNewCombo;
+                forceNewCombo = false;
+            }
+        }
+
         private void applyDefaults(HitObject hitObject)
         {
             DifficultyControlPoint difficultyControlPoint = (beatmap.ControlPointInfo as LegacyControlPointInfo)?.DifficultyPointAt(hitObject.StartTime) ?? DifficultyControlPoint.DEFAULT;

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

@@ -9,16 +9,12 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch
     /// <summary>
     /// Legacy osu!catch Hit-type, used for parsing Beatmaps.
     /// </summary>
-    internal sealed class ConvertHit : ConvertHitObject, IHasPosition, IHasCombo
+    internal sealed class ConvertHit : ConvertHitObject, IHasPosition
     {
         public float X => Position.X;
 
         public float Y => Position.Y;
 
         public Vector2 Position { get; set; }
-
-        public bool NewCombo { get; set; }
-
-        public int ComboOffset { get; set; }
     }
 }

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

@@ -14,44 +14,31 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch
     /// </summary>
     public class ConvertHitObjectParser : Legacy.ConvertHitObjectParser
     {
+        private ConvertHitObject lastObject;
+
         public ConvertHitObjectParser(double offset, int formatVersion)
             : base(offset, formatVersion)
         {
         }
 
-        private bool forceNewCombo;
-        private int extraComboOffset;
-
         protected override HitObject CreateHit(Vector2 position, bool newCombo, int comboOffset)
         {
-            newCombo |= forceNewCombo;
+            return lastObject = new ConvertHit
-            comboOffset += extraComboOffset;
-
-            forceNewCombo = false;
-            extraComboOffset = 0;
-
-            return new ConvertHit
             {
                 Position = position,
-                NewCombo = newCombo,
+                NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo,
-                ComboOffset = comboOffset
+                ComboOffset = newCombo ? comboOffset : 0
             };
         }
 
         protected override HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, PathControlPoint[] controlPoints, double? length, int repeatCount,
                                                   IList<IList<HitSampleInfo>> nodeSamples)
         {
-            newCombo |= forceNewCombo;
+            return lastObject = new ConvertSlider
-            comboOffset += extraComboOffset;
-
-            forceNewCombo = false;
-            extraComboOffset = 0;
-
-            return new ConvertSlider
             {
                 Position = position,
-                NewCombo = FirstObject || newCombo,
+                NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo,
-                ComboOffset = comboOffset,
+                ComboOffset = newCombo ? comboOffset : 0,
                 Path = new SliderPath(controlPoints, length),
                 NodeSamples = nodeSamples,
                 RepeatCount = repeatCount
@@ -60,20 +47,17 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch
 
         protected override HitObject CreateSpinner(Vector2 position, bool newCombo, int comboOffset, double duration)
         {
-            // Convert spinners don't create the new combo themselves, but force the next non-spinner hitobject to create a new combo
+            return lastObject = new ConvertSpinner
-            // Their combo offset is still added to that next hitobject's combo index
-            forceNewCombo |= FormatVersion <= 8 || newCombo;
-            extraComboOffset += comboOffset;
-
-            return new ConvertSpinner
             {
-                Duration = duration
+                Duration = duration,
+                NewCombo = newCombo
+                // Spinners cannot have combo offset.
             };
         }
 
         protected override HitObject CreateHold(Vector2 position, bool newCombo, int comboOffset, double duration)
         {
-            return null;
+            return lastObject = null;
         }
     }
 }

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

@@ -9,16 +9,12 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch
     /// <summary>
     /// Legacy osu!catch Slider-type, used for parsing Beatmaps.
     /// </summary>
-    internal sealed class ConvertSlider : Legacy.ConvertSlider, IHasPosition, IHasCombo
+    internal sealed class ConvertSlider : Legacy.ConvertSlider, IHasPosition
     {
         public float X => Position.X;
 
         public float Y => Position.Y;
 
         public Vector2 Position { get; set; }
-
-        public bool NewCombo { get; set; }
-
-        public int ComboOffset { get; set; }
     }
 }

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

@@ -8,16 +8,12 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch
     /// <summary>
     /// Legacy osu!catch Spinner-type, used for parsing Beatmaps.
     /// </summary>
-    internal sealed class ConvertSpinner : ConvertHitObject, IHasDuration, IHasXPosition, IHasCombo
+    internal sealed class ConvertSpinner : ConvertHitObject, IHasDuration, IHasXPosition
     {
         public double EndTime => StartTime + Duration;
 
         public double Duration { get; set; }
 
         public float X => 256; // Required for CatchBeatmapConverter
-
-        public bool NewCombo { get; set; }
-
-        public int ComboOffset { get; set; }
     }
 }

osu.Game/Rulesets/Objects/Legacy/ConvertHitObject.cs

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using osu.Game.Rulesets.Judgements;
+using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.Scoring;
 
 namespace osu.Game.Rulesets.Objects.Legacy
@@ -9,8 +10,12 @@ namespace osu.Game.Rulesets.Objects.Legacy
     /// <summary>
     /// A hit object only used for conversion, not actual gameplay.
     /// </summary>
-    internal abstract class ConvertHitObject : HitObject
+    internal abstract class ConvertHitObject : HitObject, IHasCombo
     {
+        public bool NewCombo { get; set; }
+
+        public int ComboOffset { get; set; }
+
         public override Judgement CreateJudgement() => new IgnoreJudgement();
 
         protected override HitWindows CreateHitWindows() => HitWindows.Empty;

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

@@ -9,16 +9,12 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
     /// <summary>
     /// Legacy osu! Hit-type, used for parsing Beatmaps.
     /// </summary>
-    internal sealed class ConvertHit : ConvertHitObject, IHasPosition, IHasCombo
+    internal sealed class ConvertHit : ConvertHitObject, IHasPosition
     {
         public Vector2 Position { get; set; }
 
         public float X => Position.X;
 
         public float Y => Position.Y;
-
-        public bool NewCombo { get; set; }
-
-        public int ComboOffset { get; set; }
     }
 }

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

@@ -14,44 +14,31 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
     /// </summary>
     public class ConvertHitObjectParser : Legacy.ConvertHitObjectParser
     {
+        private ConvertHitObject lastObject;
+
         public ConvertHitObjectParser(double offset, int formatVersion)
             : base(offset, formatVersion)
         {
         }
 
-        private bool forceNewCombo;
-        private int extraComboOffset;
-
         protected override HitObject CreateHit(Vector2 position, bool newCombo, int comboOffset)
         {
-            newCombo |= forceNewCombo;
+            return lastObject = new ConvertHit
-            comboOffset += extraComboOffset;
-
-            forceNewCombo = false;
-            extraComboOffset = 0;
-
-            return new ConvertHit
             {
                 Position = position,
-                NewCombo = FirstObject || newCombo,
+                NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo,
-                ComboOffset = comboOffset
+                ComboOffset = newCombo ? comboOffset : 0
             };
         }
 
         protected override HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, PathControlPoint[] controlPoints, double? length, int repeatCount,
                                                   IList<IList<HitSampleInfo>> nodeSamples)
         {
-            newCombo |= forceNewCombo;
+            return lastObject = new ConvertSlider
-            comboOffset += extraComboOffset;
-
-            forceNewCombo = false;
-            extraComboOffset = 0;
-
-            return new ConvertSlider
             {
                 Position = position,
-                NewCombo = FirstObject || newCombo,
+                NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo,
-                ComboOffset = comboOffset,
+                ComboOffset = newCombo ? comboOffset : 0,
                 Path = new SliderPath(controlPoints, length),
                 NodeSamples = nodeSamples,
                 RepeatCount = repeatCount
@@ -60,21 +47,18 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
 
         protected override HitObject CreateSpinner(Vector2 position, bool newCombo, int comboOffset, double duration)
         {
-            // Convert spinners don't create the new combo themselves, but force the next non-spinner hitobject to create a new combo
+            return lastObject = new ConvertSpinner
-            // Their combo offset is still added to that next hitobject's combo index
-            forceNewCombo |= FormatVersion <= 8 || newCombo;
-            extraComboOffset += comboOffset;
-
-            return new ConvertSpinner
             {
                 Position = position,
-                Duration = duration
+                Duration = duration,
+                NewCombo = newCombo
+                // Spinners cannot have combo offset.
             };
         }
 
         protected override HitObject CreateHold(Vector2 position, bool newCombo, int comboOffset, double duration)
         {
-            return null;
+            return lastObject = null;
         }
     }
 }

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

@@ -9,7 +9,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
     /// <summary>
     /// Legacy osu! Slider-type, used for parsing Beatmaps.
     /// </summary>
-    internal sealed class ConvertSlider : Legacy.ConvertSlider, IHasPosition, IHasCombo, IHasGenerateTicks
+    internal sealed class ConvertSlider : Legacy.ConvertSlider, IHasPosition, IHasGenerateTicks
     {
         public Vector2 Position { get; set; }
 
@@ -17,10 +17,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
 
         public float Y => Position.Y;
 
-        public bool NewCombo { get; set; }
-
-        public int ComboOffset { get; set; }
-
         public bool GenerateTicks { get; set; } = true;
     }
 }

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

@@ -9,7 +9,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
     /// <summary>
     /// Legacy osu! Spinner-type, used for parsing Beatmaps.
     /// </summary>
-    internal sealed class ConvertSpinner : ConvertHitObject, IHasDuration, IHasPosition, IHasCombo
+    internal sealed class ConvertSpinner : ConvertHitObject, IHasDuration, IHasPosition
     {
         public double Duration { get; set; }
 
@@ -20,9 +20,5 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
         public float X => Position.X;
 
         public float Y => Position.Y;
-
-        public bool NewCombo { get; set; }
-
-        public int ComboOffset { get; set; }
     }
 }