Super huge abstraction changes to (hopefully) make this TimingChangeContainer usable for non-mania rulesets.

Also includes commenting improvements/fixes.

osu-framework

@@ -1 +1 @@
-Subproject commit 8ba3f5079e904f72e02b02909b681b5d64c81466
+Subproject commit 1164dd486e71841eb6dad9724be9087994cb24d6

osu.Game.Rulesets.Mania/Timing/Drawables/DrawableGravityTimingChange.cs

@@ -3,7 +3,7 @@
 
 namespace osu.Game.Rulesets.Mania.Timing.Drawables
 {
-    public class DrawableGravityTimingChange : DrawableTimingChange
+    public class DrawableGravityTimingChange : DrawableManiaTimingChange
     {
         public DrawableGravityTimingChange(TimingChange timingChange)
             : base(timingChange)

osu.Game.Rulesets.Mania/Timing/Drawables/DrawableManiaTimingChange.cs

@@ -0,0 +1,28 @@
+// Copyright (c) 2007-2017 ppy Pty Ltd <contact@ppy.sh>.
+// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
+
+using osu.Framework.Graphics;
+
+namespace osu.Game.Rulesets.Mania.Timing.Drawables
+{
+    public class DrawableManiaTimingChange : DrawableTimingChange
+    {
+        public DrawableManiaTimingChange(TimingChange timingChange)
+            : base(timingChange, Axes.Y)
+        {
+        }
+
+        protected override void UpdateAfterChildren()
+        {
+            base.UpdateAfterChildren();
+
+            var parent = Parent as IHasTimeSpan;
+
+            if (parent == null)
+                return;
+
+            LifetimeStart = TimingChange.Time - parent.TimeSpan.Y;
+            LifetimeEnd = TimingChange.Time + Content.RelativeCoordinateSpace.Y * 2;
+        }
+    }
+}

osu.Game.Rulesets.Mania/Timing/Drawables/DrawableScrollingTimingChange.cs

@@ -3,7 +3,7 @@
 
 namespace osu.Game.Rulesets.Mania.Timing.Drawables
 {
-    public class DrawableScrollingTimingChange : DrawableTimingChange
+    public class DrawableScrollingTimingChange : DrawableManiaTimingChange
     {
         public DrawableScrollingTimingChange(TimingChange timingChange)
             : base(timingChange)

osu.Game.Rulesets.Mania/Timing/Drawables/DrawableTimingChange.cs

@@ -1,6 +1,7 @@
 // Copyright (c) 2007-2017 ppy Pty Ltd <contact@ppy.sh>.
 // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using OpenTK;
@@ -11,6 +12,9 @@ using osu.Game.Rulesets.Objects.Drawables;
 
 namespace osu.Game.Rulesets.Mania.Timing.Drawables
 {
+    /// <summary>
+    /// Represents a container in which contains hit objects and moves relative to the current time.
+    /// </summary>
     public abstract class DrawableTimingChange : Container<DrawableHitObject>
     {
         public readonly TimingChange TimingChange;
@@ -18,59 +22,89 @@ namespace osu.Game.Rulesets.Mania.Timing.Drawables
         protected override Container<DrawableHitObject> Content => content;
         private readonly Container<DrawableHitObject> content;
 
-        protected DrawableTimingChange(TimingChange timingChange)
+        private readonly Axes scrollingAxes;
+
+        /// <summary>
+        /// Creates a new drawable timing change which contains hit objects and scrolls relative to the current time.
+        /// </summary>
+        /// <param name="timingChange">The encapsulated timing change that provides the speed changes.</param>
+        /// <param name="scrollingAxes">The axes through which this timing change scrolls.</param>
+        protected DrawableTimingChange(TimingChange timingChange, Axes scrollingAxes)
         {
+            this.scrollingAxes = scrollingAxes;
+
             TimingChange = timingChange;
 
-            RelativeSizeAxes = Axes.Both;
-
-            AddInternal(content = new RelativeCoordinateAutoSizingContainer
+            // We have to proxy the hit objects to an internal container since we're
+            // going to be modifying our height to apply speed changes
+            AddInternal(content = new RelativeCoordinateAutoSizingContainer(scrollingAxes)
             {
                 RelativeSizeAxes = Axes.Both,
-                RelativePositionAxes = Axes.Both,
-                Y = (float)timingChange.Time
+                RelativePositionAxes = Axes.Both
             });
         }
 
+        public override Axes RelativeSizeAxes
+        {
+            get { return Axes.Both; }
+            set { throw new InvalidOperationException($"{nameof(DrawableTimingChange)} must always be relatively-sized."); }
+        }
+
         protected override void Update()
         {
-            var parent = (TimingChangeContainer)Parent;
+            var parent = Parent as IHasTimeSpan;
 
-            // Adjust our height to account for the speed changes
-            Height = (float)(1000 / TimingChange.BeatLength / TimingChange.SpeedMultiplier);
-            RelativeCoordinateSpace = new Vector2(1, (float)parent.TimeSpan);
+            if (parent == null)
+                return;
+
+            // Adjust our size to account for the speed changes
+            float speedAdjustedSize = (float)(1000 / TimingChange.BeatLength / TimingChange.SpeedMultiplier);
+
+            Size = new Vector2((scrollingAxes & Axes.X) > 0 ? speedAdjustedSize : 1,
+                               (scrollingAxes & Axes.Y) > 0 ? speedAdjustedSize : 1);
+
+            RelativeCoordinateSpace = new Vector2((scrollingAxes & Axes.X) > 0 ? (float)parent.TimeSpan.X : 1,
+                                                  (scrollingAxes & Axes.Y) > 0 ? (float)parent.TimeSpan.Y : 1);
         }
 
-        protected override void UpdateAfterChildren()
+        public override void Add(DrawableHitObject hitObject)
         {
-            base.UpdateAfterChildren();
-
-            var parent = (TimingChangeContainer)Parent;
-
-            LifetimeStart = TimingChange.Time - parent.TimeSpan;
-            LifetimeEnd = TimingChange.Time + Content.RelativeCoordinateSpace.Y * 2;
-        }
-
-        public override void Add(DrawableHitObject drawable)
-        {
-            // The previously relatively-positioned drawable will now become relative to content, but since the drawable has no knowledge of content,
-            // we need to offset it back by content's position position so that it becomes correctly relatively-positioned to content
+            // The previously relatively-positioned hit object will now become relative to content, but since the hit object has no knowledge of content,
+            // we need to offset it back by content's position (timing change time) so that it becomes correctly relatively-positioned to content
             // This can be removed if hit objects were stored such that either their StartTime or their "beat offset" was relative to the timing change
             // they belonged to, but this requires a radical change to the beatmap format which we're not ready to do just yet
-            drawable.Y -= (float)TimingChange.Time;
+            hitObject.Position = new Vector2((scrollingAxes & Axes.X) > 0 ? hitObject.X - (float)TimingChange.Time : hitObject.X,
+                                             (scrollingAxes & Axes.Y) > 0 ? hitObject.Y - (float)TimingChange.Time : hitObject.Y);
 
-            base.Add(drawable);
+            base.Add(hitObject);
         }
 
         /// <summary>
-        /// Whether this timing change can contain a drawable. This is true if the drawable occurs "after" after this timing change.
+        /// Whether this timing change can contain a hit object. This is true if the hit object occurs "after" after this timing change.
         /// </summary>
         public bool CanContain(DrawableHitObject hitObject) => TimingChange.Time <= hitObject.HitObject.StartTime;
 
+        /// <summary>
+        /// A container which cann be relatively-sized while auto-sizing to its children on desired axes. The relative coordinate space of
+        /// this container follows its auto-sized height.
+        /// </summary>
         private class RelativeCoordinateAutoSizingContainer : Container<DrawableHitObject>
         {
             protected override IComparer<Drawable> DepthComparer => new HitObjectReverseStartTimeComparer();
 
+            private readonly Axes autoSizingAxes;
+
+            /// <summary>
+            /// The axes which this container should calculate its size from its children on.
+            /// Note that this is not the same as <see cref="AutoSizeAxes"/>, because that would not allow this container
+            /// to be relatively sized - desired in the case where the playfield re-defines <see cref="RelativeCoordinateSpace"/>.
+            /// </summary>
+            /// <param name="autoSizingAxes"></param>
+            public RelativeCoordinateAutoSizingContainer(Axes autoSizingAxes)
+            {
+                this.autoSizingAxes = autoSizingAxes;
+            }
+
             public override void InvalidateFromChild(Invalidation invalidation)
             {
                 // We only want to re-compute our size when a child's size or position has changed
@@ -84,9 +118,13 @@ namespace osu.Game.Rulesets.Mania.Timing.Drawables
                     return;
 
                 float height = Children.Select(child => child.Y + child.Height).Max();
+                float width = Children.Select(child => child.X + child.Width).Max();
 
-                Height = height;
-                RelativeCoordinateSpace = new Vector2(1, height);
+                Size = new Vector2((autoSizingAxes & Axes.X) > 0 ? width : base.Size.X,
+                                   (autoSizingAxes & Axes.Y) > 0 ? height : base.Size.Y);
+
+                RelativeCoordinateSpace = new Vector2((autoSizingAxes & Axes.X) > 0 ? width : 1,
+                                                      (autoSizingAxes & Axes.Y) > 0 ? height : 1);
 
                 base.InvalidateFromChild(invalidation);
             }

osu.Game.Rulesets.Mania/Timing/IHasTimeSpan.cs

@@ -0,0 +1,13 @@
+using OpenTK;
+using osu.Framework.Graphics.Containers;
+
+namespace osu.Game.Rulesets.Mania.Timing
+{
+    public interface IHasTimeSpan : IContainer
+    {
+        /// <summary>
+        /// The amount of time which this container spans. Drawables can be relatively positioned to this value.
+        /// </summary>
+        Vector2 TimeSpan { get; }
+    }
+}

osu.Game.Rulesets.Mania/Timing/TimingChangeContainer.cs

@@ -8,15 +8,13 @@ using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.Mania.Timing.Drawables;
+using OpenTK;
 
 namespace osu.Game.Rulesets.Mania.Timing
 {
-    public class TimingChangeContainer : Container<DrawableTimingChange>
+    public class TimingChangeContainer : Container<DrawableTimingChange>, IHasTimeSpan
     {
-        /// <summary>
-        /// The amount of time which this container spans.
-        /// </summary>
-        public double TimeSpan { get; set; }
+        public Vector2 TimeSpan { get; set; }
 
         /// <summary>
         /// Adds a hit object to the most applicable timing change in this container.
@@ -35,10 +33,11 @@ namespace osu.Game.Rulesets.Mania.Timing
         protected override IComparer<Drawable> DepthComparer => new TimingChangeReverseStartTimeComparer();
 
         /// <summary>
-        /// Finds the most applicable timing change that can contain a hit object.
+        /// Finds the most applicable timing change that can contain a hit object. If the hit object occurs before the first (time-wise)
+        /// timing change, then the timing change returned is the first (time-wise) timing change.
         /// </summary>
         /// <param name="hitObject">The hit object to contain.</param>
-        /// <returns>The last timing change which can contain <paramref name="hitObject"/>. Null if no timing change can contain the hit object.</returns>
+        /// <returns>The last (time-wise) timing change which can contain <paramref name="hitObject"/>. Null if no timing change exists.</returns>
         private DrawableTimingChange timingChangeFor(DrawableHitObject hitObject) => Children.FirstOrDefault(c => c.CanContain(hitObject)) ?? Children.LastOrDefault();
     }
 

osu.Game.Rulesets.Mania/UI/Column.cs

@@ -185,10 +185,10 @@ namespace osu.Game.Rulesets.Mania.UI
             }
         }
 
-        public double TimeSpan
+        public float TimeSpan
         {
-            get { return timingChanges.TimeSpan; }
-            set { timingChanges.TimeSpan = value; }
+            get { return timingChanges.TimeSpan.Y; }
+            set { timingChanges.TimeSpan = new Vector2(1, value); }
         }
 
         public void Add(DrawableTimingChange timingChange) => timingChanges.Add(timingChange);

osu.Game.Rulesets.Mania/UI/ManiaHitRenderer.cs

@@ -32,12 +32,11 @@ namespace osu.Game.Rulesets.Mania.UI
     public class ManiaHitRenderer : HitRenderer<ManiaHitObject, ManiaJudgement>
     {
         private int? columns;
-
         public int Columns
         {
             get { return columns ?? (int)Math.Round(Beatmap.BeatmapInfo.Difficulty.CircleSize); }
             set { columns = value; }
-        } 
+        }
 
         public Dictionary<int, List<DrawableTimingChange>> HitObjectTimingChanges;
         public List<DrawableTimingChange> BarlineTimingChanges;
@@ -45,10 +44,10 @@ namespace osu.Game.Rulesets.Mania.UI
         public ManiaHitRenderer(WorkingBeatmap beatmap, bool isForCurrentRuleset)
             : base(beatmap, isForCurrentRuleset)
         {
-            generateTimingChanges();
+            generateDefaultTimingChanges();
         }
 
-        private void generateTimingChanges()
+        private void generateDefaultTimingChanges()
         {
             if (HitObjectTimingChanges != null || BarlineTimingChanges != null)
                 return;

osu.Game.Rulesets.Mania/UI/ManiaPlayfield.cs

@@ -230,11 +230,11 @@ namespace osu.Game.Rulesets.Mania.UI
             return false;
         }
 
-        private double timeSpan;
+        private float timeSpan;
         /// <summary>
         /// The amount of time which the length of the playfield spans.
         /// </summary>
-        public double TimeSpan
+        public float TimeSpan
         {
             get { return timeSpan; }
             set
@@ -245,7 +245,7 @@ namespace osu.Game.Rulesets.Mania.UI
 
                 timeSpan = MathHelper.Clamp(timeSpan, time_span_min, time_span_max);
 
-                barLineContainer.TimeSpan = value;
+                barLineContainer.TimeSpan = new Vector2(1, value);
                 Columns.ForEach(c => c.TimeSpan = value);
             }
         }
@@ -281,7 +281,7 @@ namespace osu.Game.Rulesets.Mania.UI
                 base.Apply(d);
 
                 var p = (ManiaPlayfield)d;
-                p.TimeSpan = CurrentValue;
+                p.TimeSpan = (float)CurrentValue;
             }
         }
     }

osu.Game.Rulesets.Mania/osu.Game.Rulesets.Mania.csproj

@@ -85,8 +85,10 @@
     <Compile Include="Mods\ManiaModGravity.cs" />
     <Compile Include="UI\SpecialColumnPosition.cs" />
     <Compile Include="Timing\Drawables\DrawableGravityTimingChange.cs" />
+    <Compile Include="Timing\Drawables\DrawableManiaTimingChange.cs" />
     <Compile Include="Timing\Drawables\DrawableScrollingTimingChange.cs" />
     <Compile Include="Timing\Drawables\DrawableTimingChange.cs" />
+    <Compile Include="Timing\IHasTimeSpan.cs" />
     <Compile Include="Timing\TimingChange.cs" />
     <Compile Include="Timing\TimingChangeContainer.cs" />
   </ItemGroup>