Refactor beatmap converters.

osu.Game.Modes.Catch/Beatmaps/CatchBeatmapConverter.cs

@@ -7,19 +7,17 @@ using System.Collections.Generic;
 using System;
 using osu.Game.Modes.Objects.Types;
 using osu.Game.Modes.Beatmaps;
+using osu.Game.Modes.Objects;
 
 namespace osu.Game.Modes.Catch.Beatmaps
 {
     internal class CatchBeatmapConverter : BeatmapConverter<CatchBaseHit>
     {
-        public override IEnumerable<Type> ValidConversionTypes { get; } = new[] { typeof(IHasXPosition) };
+        protected override IEnumerable<Type> ValidConversionTypes { get; } = new[] { typeof(IHasXPosition) };
 
-        public override Beatmap<CatchBaseHit> Convert(Beatmap original)
+        protected override IEnumerable<CatchBaseHit> ConvertHitObject(HitObject original, Beatmap beatmap)
         {
-            return new Beatmap<CatchBaseHit>(original)
+            yield return null;
-            {
-                HitObjects = new List<CatchBaseHit>() // Todo: Convert HitObjects
-            };
         }
     }
 }

osu.Game.Modes.Mania/Beatmaps/ManiaBeatmapConverter.cs

@@ -7,19 +7,17 @@ using System.Collections.Generic;
 using System;
 using osu.Game.Modes.Objects.Types;
 using osu.Game.Modes.Beatmaps;
+using osu.Game.Modes.Objects;
 
 namespace osu.Game.Modes.Mania.Beatmaps
 {
     internal class ManiaBeatmapConverter : BeatmapConverter<ManiaBaseHit>
     {
-        public override IEnumerable<Type> ValidConversionTypes { get; } = new[] { typeof(IHasXPosition) };
+        protected override IEnumerable<Type> ValidConversionTypes { get; } = new[] { typeof(IHasXPosition) };
 
-        public override Beatmap<ManiaBaseHit> Convert(Beatmap original)
+        protected override IEnumerable<ManiaBaseHit> ConvertHitObject(HitObject original, Beatmap beatmap)
         {
-            return new Beatmap<ManiaBaseHit>(original)
+            yield return null;
-            {
-                HitObjects = new List<ManiaBaseHit>() // Todo: Implement
-            };
         }
     }
 }

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

@@ -5,10 +5,8 @@ using OpenTK;
 using osu.Game.Beatmaps;
 using osu.Game.Modes.Objects;
 using osu.Game.Modes.Osu.Objects;
-using osu.Game.Modes.Osu.Objects.Drawables;
 using System.Collections.Generic;
 using osu.Game.Modes.Objects.Types;
-using System.Linq;
 using System;
 using osu.Game.Modes.Osu.UI;
 using osu.Game.Modes.Beatmaps;
@@ -17,31 +15,10 @@ namespace osu.Game.Modes.Osu.Beatmaps
 {
     internal class OsuBeatmapConverter : BeatmapConverter<OsuHitObject>
     {
-        public override IEnumerable<Type> ValidConversionTypes { get; } = new[] { typeof(IHasPosition) };
+        protected override IEnumerable<Type> ValidConversionTypes { get; } = new[] { typeof(IHasPosition) };
 
-        public override Beatmap<OsuHitObject> Convert(Beatmap original)
+        protected override IEnumerable<OsuHitObject> ConvertHitObject(HitObject original, Beatmap beatmap)
         {
-            return new Beatmap<OsuHitObject>(original)
-            {
-                HitObjects = convertHitObjects(original.HitObjects, original.BeatmapInfo?.StackLeniency ?? 0.7f)
-            };
-        }
-
-        private List<OsuHitObject> convertHitObjects(List<HitObject> hitObjects, float stackLeniency)
-        {
-            List<OsuHitObject> converted = hitObjects.Select(convertHitObject).ToList();
-
-            updateStacking(converted, stackLeniency);
-
-            return converted;
-        }
-
-        private OsuHitObject convertHitObject(HitObject original)
-        {
-            OsuHitObject originalOsu = original as OsuHitObject;
-            if (originalOsu != null)
-                return originalOsu;
-
             IHasCurve curveData = original as IHasCurve;
             IHasEndTime endTimeData = original as IHasEndTime;
             IHasPosition positionData = original as IHasPosition;
@@ -49,7 +26,7 @@ namespace osu.Game.Modes.Osu.Beatmaps
 
             if (curveData != null)
             {
-                return new Slider
+                yield return new Slider
                 {
                     StartTime = original.StartTime,
                     Samples = original.Samples,
@@ -58,10 +35,9 @@ namespace osu.Game.Modes.Osu.Beatmaps
                     NewCombo = comboData?.NewCombo ?? false
                 };
             }
-
+            else if (endTimeData != null)
-            if (endTimeData != null)
             {
-                return new Spinner
+                yield return new Spinner
                 {
                     StartTime = original.StartTime,
                     Samples = original.Samples,
@@ -70,161 +46,15 @@ namespace osu.Game.Modes.Osu.Beatmaps
                     Position = positionData?.Position ?? OsuPlayfield.BASE_SIZE / 2,
                 };
             }
-
+            else
-            return new HitCircle
             {
-                StartTime = original.StartTime,
+                yield return new HitCircle
-                Samples = original.Samples,
-                Position = positionData?.Position ?? Vector2.Zero,
-                NewCombo = comboData?.NewCombo ?? false
-            };
-        }
-
-        private void updateStacking(List<OsuHitObject> hitObjects, float stackLeniency, int startIndex = 0, int endIndex = -1)
-        {
-            if (endIndex == -1)
-                endIndex = hitObjects.Count - 1;
-
-            const int stack_distance = 3;
-            float stackThreshold = DrawableOsuHitObject.TIME_PREEMPT * stackLeniency;
-
-            // Reset stacking inside the update range
-            for (int i = startIndex; i <= endIndex; i++)
-                hitObjects[i].StackHeight = 0;
-
-            // Extend the end index to include objects they are stacked on
-            int extendedEndIndex = endIndex;
-            for (int i = endIndex; i >= startIndex; i--)
-            {
-                int stackBaseIndex = i;
-                for (int n = stackBaseIndex + 1; n < hitObjects.Count; n++)
                 {
-                    OsuHitObject stackBaseObject = hitObjects[stackBaseIndex];
+                    StartTime = original.StartTime,
-                    if (stackBaseObject is Spinner) break;
+                    Samples = original.Samples,
-
+                    Position = positionData?.Position ?? Vector2.Zero,
-                    OsuHitObject objectN = hitObjects[n];
+                    NewCombo = comboData?.NewCombo ?? false
-                    if (objectN is Spinner)
+                };
-                        continue;
-
-                    double endTime = (stackBaseObject as IHasEndTime)?.EndTime ?? stackBaseObject.StartTime;
-
-                    if (objectN.StartTime - endTime > stackThreshold)
-                        //We are no longer within stacking range of the next object.
-                        break;
-
-                    if (Vector2.Distance(stackBaseObject.Position, objectN.Position) < stack_distance ||
-                        stackBaseObject is Slider && Vector2.Distance(stackBaseObject.EndPosition, objectN.Position) < stack_distance)
-                    {
-                        stackBaseIndex = n;
-
-                        // HitObjects after the specified update range haven't been reset yet
-                        objectN.StackHeight = 0;
-                    }
-                }
-
-                if (stackBaseIndex > extendedEndIndex)
-                {
-                    extendedEndIndex = stackBaseIndex;
-                    if (extendedEndIndex == hitObjects.Count - 1)
-                        break;
-                }
-            }
-
-            //Reverse pass for stack calculation.
-            int extendedStartIndex = startIndex;
-            for (int i = extendedEndIndex; i > startIndex; i--)
-            {
-                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.
-                    *
-                    * o <-1      o <-2
-                    *  o <-3      o <-4
-                    *
-                    * 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.
-                    * 2 and 1 will be ignored in the i loop because they already have a stack value.
-                    */
-
-                OsuHitObject objectI = hitObjects[i];
-                if (objectI.StackHeight != 0 || objectI is Spinner) continue;
-
-                /* 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.
-                    * Any other case is handled by the "is Slider" code below this.
-                    */
-                if (objectI is HitCircle)
-                {
-                    while (--n >= 0)
-                    {
-                        OsuHitObject objectN = hitObjects[n];
-                        if (objectN is Spinner) continue;
-
-                        double endTime = (objectN as IHasEndTime)?.EndTime ?? objectN.StartTime;
-
-                        if (objectI.StartTime - endTime > stackThreshold)
-                            //We are no longer within stacking range of the previous object.
-                            break;
-
-                        // HitObjects before the specified update range haven't been reset yet
-                        if (n < extendedStartIndex)
-                        {
-                            objectN.StackHeight = 0;
-                            extendedStartIndex = n;
-                        }
-
-                        /* 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 <- hitCircle has stack of -1
-                            *         o <- hitCircle has stack of -2
-                            */
-                        if (objectN is Slider && Vector2.Distance(objectN.EndPosition, objectI.Position) < stack_distance)
-                        {
-                            int offset = objectI.StackHeight - objectN.StackHeight + 1;
-                            for (int j = n + 1; j <= i; j++)
-                            {
-                                //For each object which was declared under this slider, we will offset it to appear *below* the slider end (rather than above).
-                                OsuHitObject objectJ = hitObjects[j];
-                                if (Vector2.Distance(objectN.EndPosition, objectJ.Position) < stack_distance)
-                                    objectJ.StackHeight -= offset;
-                            }
-
-                            //We have hit a slider.  We should restart calculation using this as the new base.
-                            //Breaking here will mean that the slider still has StackCount of 0, so will be handled in the i-outer-loop.
-                            break;
-                        }
-
-                        if (Vector2.Distance(objectN.Position, objectI.Position) < stack_distance)
-                        {
-                            //Keep processing as if there are no sliders.  If we come across a slider, this gets cancelled out.
-                            //NOTE: Sliders with start positions stacking are a special case that is also handled here.
-
-                            objectN.StackHeight = objectI.StackHeight + 1;
-                            objectI = objectN;
-                        }
-                    }
-                }
-                else if (objectI is Slider)
-                {
-                    /* We have hit the first slider in a possible stack.
-                        * From this point on, we ALWAYS stack positive regardless.
-                        */
-                    while (--n >= startIndex)
-                    {
-                        OsuHitObject objectN = hitObjects[n];
-                        if (objectN is Spinner) continue;
-
-                        if (objectI.StartTime - objectN.StartTime > stackThreshold)
-                            //We are no longer within stacking range of the previous object.
-                            break;
-
-                        if (Vector2.Distance(objectN.EndPosition, objectI.Position) < stack_distance)
-                        {
-                            objectN.StackHeight = objectI.StackHeight + 1;
-                            objectI = objectN;
-                        }
-                    }
-                }
             }
         }
     }

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

@@ -1,9 +1,12 @@
 // Copyright (c) 2007-2017 ppy Pty Ltd <contact@ppy.sh>.
 // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
 
+using OpenTK;
 using osu.Game.Beatmaps;
 using osu.Game.Modes.Beatmaps;
+using osu.Game.Modes.Objects.Types;
 using osu.Game.Modes.Osu.Objects;
+using osu.Game.Modes.Osu.Objects.Drawables;
 
 namespace osu.Game.Modes.Osu.Beatmaps
 {
@@ -11,6 +14,8 @@ namespace osu.Game.Modes.Osu.Beatmaps
     {
         public override void PostProcess(Beatmap<OsuHitObject> beatmap)
         {
+            applyStacking(beatmap);
+
             if (beatmap.ComboColors.Count == 0)
                 return;
 
@@ -29,5 +34,150 @@ namespace osu.Game.Modes.Osu.Beatmaps
                 obj.ComboColour = beatmap.ComboColors[colourIndex];
             }
         }
+
+        private void applyStacking(Beatmap<OsuHitObject> beatmap)
+        {
+            const int stack_distance = 3;
+            float stackThreshold = DrawableOsuHitObject.TIME_PREEMPT * beatmap.BeatmapInfo?.StackLeniency ?? 0.7f;
+
+            // Reset stacking
+            for (int i = 0; i <= beatmap.HitObjects.Count - 1; i++)
+                beatmap.HitObjects[i].StackHeight = 0;
+
+            // Extend the end index to include objects they are stacked on
+            int extendedEndIndex = beatmap.HitObjects.Count - 1;
+            for (int i = beatmap.HitObjects.Count - 1; i >= 0; i--)
+            {
+                int stackBaseIndex = i;
+                for (int n = stackBaseIndex + 1; n < beatmap.HitObjects.Count; n++)
+                {
+                    OsuHitObject stackBaseObject = beatmap.HitObjects[stackBaseIndex];
+                    if (stackBaseObject is Spinner) break;
+
+                    OsuHitObject objectN = beatmap.HitObjects[n];
+                    if (objectN is Spinner)
+                        continue;
+
+                    double endTime = (stackBaseObject as IHasEndTime)?.EndTime ?? stackBaseObject.StartTime;
+
+                    if (objectN.StartTime - endTime > stackThreshold)
+                        //We are no longer within stacking range of the next object.
+                        break;
+
+                    if (Vector2.Distance(stackBaseObject.Position, objectN.Position) < stack_distance ||
+                        stackBaseObject is Slider && Vector2.Distance(stackBaseObject.EndPosition, objectN.Position) < stack_distance)
+                    {
+                        stackBaseIndex = n;
+
+                        // HitObjects after the specified update range haven't been reset yet
+                        objectN.StackHeight = 0;
+                    }
+                }
+
+                if (stackBaseIndex > extendedEndIndex)
+                {
+                    extendedEndIndex = stackBaseIndex;
+                    if (extendedEndIndex == beatmap.HitObjects.Count - 1)
+                        break;
+                }
+            }
+
+            //Reverse pass for stack calculation.
+            int extendedStartIndex = 0;
+            for (int i = extendedEndIndex; i > 0; i--)
+            {
+                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.
+                    *
+                    * o <-1      o <-2
+                    *  o <-3      o <-4
+                    *
+                    * 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.
+                    * 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;
+
+                /* 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.
+                    * Any other case is handled by the "is Slider" code below this.
+                    */
+                if (objectI is HitCircle)
+                {
+                    while (--n >= 0)
+                    {
+                        OsuHitObject objectN = beatmap.HitObjects[n];
+                        if (objectN is Spinner) continue;
+
+                        double endTime = (objectN as IHasEndTime)?.EndTime ?? objectN.StartTime;
+
+                        if (objectI.StartTime - endTime > stackThreshold)
+                            //We are no longer within stacking range of the previous object.
+                            break;
+
+                        // HitObjects before the specified update range haven't been reset yet
+                        if (n < extendedStartIndex)
+                        {
+                            objectN.StackHeight = 0;
+                            extendedStartIndex = n;
+                        }
+
+                        /* 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 <- hitCircle has stack of -1
+                            *         o <- hitCircle has stack of -2
+                            */
+                        if (objectN is Slider && Vector2.Distance(objectN.EndPosition, objectI.Position) < stack_distance)
+                        {
+                            int offset = objectI.StackHeight - objectN.StackHeight + 1;
+                            for (int j = n + 1; j <= i; j++)
+                            {
+                                //For each object which was declared under this slider, we will offset it to appear *below* the slider end (rather than above).
+                                OsuHitObject objectJ = beatmap.HitObjects[j];
+                                if (Vector2.Distance(objectN.EndPosition, objectJ.Position) < stack_distance)
+                                    objectJ.StackHeight -= offset;
+                            }
+
+                            //We have hit a slider.  We should restart calculation using this as the new base.
+                            //Breaking here will mean that the slider still has StackCount of 0, so will be handled in the i-outer-loop.
+                            break;
+                        }
+
+                        if (Vector2.Distance(objectN.Position, objectI.Position) < stack_distance)
+                        {
+                            //Keep processing as if there are no sliders.  If we come across a slider, this gets cancelled out.
+                            //NOTE: Sliders with start positions stacking are a special case that is also handled here.
+
+                            objectN.StackHeight = objectI.StackHeight + 1;
+                            objectI = objectN;
+                        }
+                    }
+                }
+                else if (objectI is Slider)
+                {
+                    /* We have hit the first slider in a possible stack.
+                        * From this point on, we ALWAYS stack positive regardless.
+                        */
+                    while (--n >= 0)
+                    {
+                        OsuHitObject objectN = beatmap.HitObjects[n];
+                        if (objectN is Spinner) continue;
+
+                        if (objectI.StartTime - objectN.StartTime > stackThreshold)
+                            //We are no longer within stacking range of the previous object.
+                            break;
+
+                        if (Vector2.Distance(objectN.EndPosition, objectI.Position) < stack_distance)
+                        {
+                            objectN.StackHeight = objectI.StackHeight + 1;
+                            objectI = objectN;
+                        }
+                    }
+                }
+            }
+        }
     }
 }

osu.Game.Modes.Taiko/Beatmaps/TaikoBeatmapConverter.cs

@@ -39,33 +39,30 @@ namespace osu.Game.Modes.Taiko.Beatmaps
         /// </summary>
         private const float taiko_base_distance = 100;
 
-        public override IEnumerable<Type> ValidConversionTypes { get; } = new[] { typeof(HitObject) };
+        protected override IEnumerable<Type> ValidConversionTypes { get; } = new[] { typeof(HitObject) };
 
-        public override Beatmap<TaikoHitObject> Convert(Beatmap original)
+        protected override Beatmap<TaikoHitObject> ConvertBeatmap(Beatmap original)
         {
+            // Rewrite the beatmap info to add the slider velocity multiplier
             BeatmapInfo info = original.BeatmapInfo.DeepClone<BeatmapInfo>();
             info.Difficulty.SliderMultiplier *= legacy_velocity_multiplier;
 
-            return new Beatmap<TaikoHitObject>(original)
+            Beatmap<TaikoHitObject> converted = base.ConvertBeatmap(original);
+
+            // Post processing step to transform hit objects with the same start time into strong hits
+            converted.HitObjects = converted.HitObjects.GroupBy(t => t.StartTime).Select(x =>
             {
-                BeatmapInfo = info,
+                TaikoHitObject first = x.First();
-                HitObjects = original.HitObjects.SelectMany(h => convertHitObject(h, original)).GroupBy(t => t.StartTime).Select(x =>
+                if (x.Skip(1).Any())
-                {
+                    first.IsStrong = true;
-                    TaikoHitObject first = x.First();
+                return first;
-                    if (x.Skip(1).Any())
+            }).ToList();
-                        first.IsStrong = true;
+
-                    return first;
+            return converted;
-                }).ToList()
-            };
         }
 
-        private IEnumerable<TaikoHitObject> convertHitObject(HitObject obj, Beatmap beatmap)
+        protected override IEnumerable<TaikoHitObject> ConvertHitObject(HitObject obj, Beatmap beatmap)
         {
-            // Check if this HitObject is already a TaikoHitObject, and return it if so
-            var originalTaiko = obj as TaikoHitObject;
-            if (originalTaiko != null)
-                yield return originalTaiko;
-
             var distanceData = obj as IHasDistance;
             var repeatsData = obj as IHasRepeats;
             var endTimeData = obj as IHasEndTime;

osu.Game/Beatmaps/Beatmap.cs

@@ -56,6 +56,7 @@ namespace osu.Game.Beatmaps
         public Beatmap(Beatmap original = null)
             : base(original)
         {
+            HitObjects = original?.HitObjects;
         }
     }
 }

osu.Game/Modes/Beatmaps/BeatmapConverter.cs

@@ -15,23 +15,68 @@ namespace osu.Game.Modes.Beatmaps
     /// <typeparam name="T">The type of HitObject stored in the Beatmap.</typeparam>
     public abstract class BeatmapConverter<T> where T : HitObject
     {
-        /// <summary>
-        /// The types of HitObjects that can be converted to be used for this Beatmap.
-        /// </summary>
-        public abstract IEnumerable<Type> ValidConversionTypes { get; }
-
-        /// <summary>
-        /// Converts a Beatmap to another mode.
-        /// </summary>
-        /// <param name="original">The original Beatmap.</param>
-        /// <returns>The converted Beatmap.</returns>
-        public abstract Beatmap<T> Convert(Beatmap original);
-
         /// <summary>
         /// Checks if a Beatmap can be converted using this Beatmap Converter.
         /// </summary>
         /// <param name="beatmap">The Beatmap to check.</param>
         /// <returns>Whether the Beatmap can be converted using this Beatmap Converter.</returns>
         public bool CanConvert(Beatmap beatmap) => ValidConversionTypes.All(t => beatmap.HitObjects.Any(t.IsInstanceOfType));
+
+        /// <summary>
+        /// Converts a Beatmap using this Beatmap Converter.
+        /// </summary>
+        /// <param name="original">The un-converted Beatmap.</param>
+        /// <returns>The converted Beatmap.</returns>
+        public Beatmap<T> Convert(Beatmap original)
+        {
+            // We always operate on a clone of the original beatmap, to not modify it game-wide
+            return ConvertBeatmap(new Beatmap(original));
+        }
+
+        /// <summary>
+        /// Performs the conversion of a Beatmap using this Beatmap Converter.
+        /// </summary>
+        /// <param name="original">The un-converted Beatmap.</param>
+        /// <returns>The converted Beatmap.</returns>
+        protected virtual Beatmap<T> ConvertBeatmap(Beatmap original)
+        {
+            return new Beatmap<T>
+            {
+                BeatmapInfo = original.BeatmapInfo,
+                TimingInfo = original.TimingInfo,
+                HitObjects = original.HitObjects.SelectMany(h => convert(h, original)).ToList()
+            };
+        }
+
+        /// <summary>
+        /// Converts a hit object.
+        /// </summary>
+        /// <param name="original">The hit object to convert.</param>
+        /// <param name="beatmap">The un-converted Beatmap.</param>
+        /// <returns>The converted hit object.</returns>
+        private IEnumerable<T> convert(HitObject original, Beatmap beatmap)
+        {
+            // Check if the hitobject is already the converted type
+            T tObject = original as T;
+            if (tObject != null)
+                yield return tObject;
+            
+            // Convert the hit object
+            foreach (var obj in ConvertHitObject(original, beatmap))
+                yield return obj;
+        }
+
+        /// <summary>
+        /// The types of HitObjects that can be converted to be used for this Beatmap.
+        /// </summary>
+        protected abstract IEnumerable<Type> ValidConversionTypes { get; }
+
+        /// <summary>
+        /// Performs the conversion of a hit object.
+        /// </summary>
+        /// <param name="original">The hit object to convert.</param>
+        /// <param name="beatmap">The un-converted Beatmap.</param>
+        /// <returns>The converted hit object.</returns>
+        protected abstract IEnumerable<T> ConvertHitObject(HitObject original, Beatmap beatmap);
     }
 }