Merge pull request #941 from smoogipooo/better-drawablehitobject-lifetimes

Automatically determine lifetime for scrolling rulesets.

osu.Desktop.VisualTests/Tests/TestCaseScrollingHitObjects.cs

@@ -126,6 +126,8 @@ namespace osu.Desktop.VisualTests.Tests
 
         private class TestSpeedAdjustmentContainer : SpeedAdjustmentContainer
         {
+            public override bool RemoveWhenNotAlive => false;
+
             public TestSpeedAdjustmentContainer(MultiplierControlPoint controlPoint)
                 : base(controlPoint)
             {
@@ -151,11 +153,13 @@ namespace osu.Desktop.VisualTests.Tests
             }
         }
 
-        private class TestDrawableHitObject : DrawableHitObject
+        private class TestDrawableHitObject : DrawableHitObject, IScrollingHitObject
         {
             private readonly Box background;
             private const float height = 14;
 
+            public BindableDouble LifetimeOffset { get; } = new BindableDouble();
+
             public TestDrawableHitObject(HitObject hitObject)
                 : base(hitObject)
             {
@@ -196,25 +200,11 @@ namespace osu.Desktop.VisualTests.Tests
                 FadeInFromZero(250, EasingTypes.OutQuint);
             }
 
-            private bool hasExpired;
             protected override void Update()
             {
                 base.Update();
                 if (Time.Current >= HitObject.StartTime)
-                {
                     background.Colour = Color4.Red;
-
-                    if (!hasExpired)
-                    {
-                        using (BeginDelayedSequence(200))
-                        {
-                            FadeOut(200);
-                            Expire();
-                        }
-
-                        hasExpired = true;
-                    }
-                }
             }
         }
     }

osu.Game.Rulesets.Mania/Objects/Drawables/DrawableManiaHitObject.cs

@@ -6,12 +6,11 @@ using OpenTK.Input;
 using osu.Framework.Configuration;
 using osu.Framework.Graphics;
 using osu.Game.Rulesets.Mania.Judgements;
-using osu.Game.Rulesets.Mania.UI;
 using osu.Game.Rulesets.Objects.Drawables;
 
 namespace osu.Game.Rulesets.Mania.Objects.Drawables
 {
-    public abstract class DrawableManiaHitObject<TObject> : DrawableHitObject<ManiaHitObject, ManiaJudgement>
+    public abstract class DrawableManiaHitObject<TObject> : DrawableScrollingHitObject<ManiaHitObject, ManiaJudgement>
         where TObject : ManiaHitObject
     {
         /// <summary>
@@ -33,13 +32,6 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables
             Y = (float)HitObject.StartTime;
         }
 
-        protected override void LoadComplete()
-        {
-            base.LoadComplete();
-
-            LifetimeStart = HitObject.StartTime - ManiaPlayfield.TIME_SPAN_MAX;
-        }
-
         public override Color4 AccentColour
         {
             get { return base.AccentColour; }

osu.Game.Rulesets.Mania/Objects/Drawables/DrawableNote.cs

@@ -23,7 +23,7 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables
         public DrawableNote(Note hitObject, Bindable<Key> key = null)
             : base(hitObject, key)
         {
-            RelativeSizeAxes = Axes.Both;
+            RelativeSizeAxes = Axes.X;
             Height = 100;
 
             Add(headPiece = new NotePiece

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

@@ -30,8 +30,8 @@ namespace osu.Game.Rulesets.Mania.UI
         public const float HIT_TARGET_POSITION = 50;
 
         private const double time_span_default = 1500;
-        public const double TIME_SPAN_MIN = 50;
+        private const double time_span_min = 50;
-        public const double TIME_SPAN_MAX = 10000;
+        private const double time_span_max = 10000;
         private const double time_span_step = 50;
 
         /// <summary>
@@ -60,8 +60,8 @@ namespace osu.Game.Rulesets.Mania.UI
 
         private readonly BindableDouble visibleTimeRange = new BindableDouble(time_span_default)
         {
-            MinValue = TIME_SPAN_MIN,
+            MinValue = time_span_min,
-            MaxValue = TIME_SPAN_MAX
+            MaxValue = time_span_max
         };
 
         private readonly SpeedAdjustmentCollection barLineContainer;

osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs

@@ -184,7 +184,7 @@ namespace osu.Game.Rulesets.Objects.Drawables
         private List<DrawableHitObject<TObject, TJudgement>> nestedHitObjects;
         protected IEnumerable<DrawableHitObject<TObject, TJudgement>> NestedHitObjects => nestedHitObjects;
 
-        protected void AddNested(DrawableHitObject<TObject, TJudgement> h)
+        protected virtual void AddNested(DrawableHitObject<TObject, TJudgement> h)
         {
             if (nestedHitObjects == null)
                 nestedHitObjects = new List<DrawableHitObject<TObject, TJudgement>>();

osu.Game/Rulesets/Objects/Drawables/DrawableScrollingHitObject.cs

@@ -0,0 +1,49 @@
+// 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 osu.Framework.Configuration;
+using osu.Game.Rulesets.Judgements;
+using osu.Game.Rulesets.Objects.Types;
+
+namespace osu.Game.Rulesets.Objects.Drawables
+{
+    /// <summary>
+    /// A basic class that overrides <see cref="DrawableHitObject{TObject, TJudgement}"/> and implements <see cref="IScrollingHitObject"/>.
+    /// </summary>
+    public abstract class DrawableScrollingHitObject<TObject, TJudgement> : DrawableHitObject<TObject, TJudgement>, IScrollingHitObject
+        where TObject : HitObject
+        where TJudgement : Judgement
+    {
+        public BindableDouble LifetimeOffset { get; } = new BindableDouble();
+
+        protected DrawableScrollingHitObject(TObject hitObject)
+            : base(hitObject)
+        {
+        }
+
+        public override double LifetimeStart
+        {
+            get { return Math.Min(HitObject.StartTime - LifetimeOffset, base.LifetimeStart); }
+            set { base.LifetimeStart = value; }
+        }
+
+        public override double LifetimeEnd
+        {
+            get
+            {
+                var endTime = (HitObject as IHasEndTime)?.EndTime ?? HitObject.StartTime;
+                return Math.Max(endTime + LifetimeOffset, base.LifetimeEnd);
+            }
+            set { base.LifetimeEnd = value; }
+        }
+
+        protected override void AddNested(DrawableHitObject<TObject, TJudgement> h)
+        {
+            var scrollingHitObject = h as IScrollingHitObject;
+            scrollingHitObject?.LifetimeOffset.BindTo(LifetimeOffset);
+
+            base.AddNested(h);
+        }
+    }
+}

osu.Game/Rulesets/Objects/Drawables/IScrollingHitObject.cs

@@ -0,0 +1,25 @@
+// 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.Configuration;
+using osu.Framework.Graphics;
+
+namespace osu.Game.Rulesets.Objects.Drawables
+{
+    /// <summary>
+    /// An interface that exposes properties required for scrolling hit objects to be properly displayed.
+    /// </summary>
+    public interface IScrollingHitObject : IDrawable
+    {
+        /// <summary>
+        /// Time offset before the hit object start time at which this <see cref="IScrollingHitObject"/> becomes visible and the time offset
+        /// after the hit object's end time after which it expires.
+        ///
+        /// <para>
+        /// This provides only a default life time range, however classes inheriting from <see cref="IScrollingHitObject"/> should override
+        /// their life times if more tight control is desired.
+        /// </para>
+        /// </summary>
+        BindableDouble LifetimeOffset { get; }
+    }
+}

osu.Game/Rulesets/Timing/DrawableTimingSection.cs

@@ -10,6 +10,7 @@ using osu.Framework.Graphics.Containers;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Drawables;
 using OpenTK;
+using osu.Game.Rulesets.Objects.Types;
 
 namespace osu.Game.Rulesets.Timing
 {
@@ -44,14 +45,17 @@ namespace osu.Game.Rulesets.Timing
             set { visibleTimeRange.BindTo(value); }
         }
 
-        protected override IComparer<Drawable> DepthComparer => new HitObjectReverseStartTimeComparer();
-
         /// <summary>
-        /// Axes through which this timing section scrolls. This is set from <see cref="SpeedAdjustmentContainer"/>.
+        /// Axes through which this timing section scrolls. This is set by the <see cref="SpeedAdjustmentContainer"/>.
         /// </summary>
         internal Axes ScrollingAxes;
 
-        private Cached layout = new Cached();
+        /// <summary>
+        /// The control point that provides the speed adjustments for this container. This is set by the <see cref="SpeedAdjustmentContainer"/>.
+        /// </summary>
+        internal MultiplierControlPoint ControlPoint;
+
+        protected override IComparer<Drawable> DepthComparer => new HitObjectReverseStartTimeComparer();
 
         /// <summary>
         /// Creates a new <see cref="DrawableTimingSection"/>.
@@ -71,41 +75,58 @@ namespace osu.Game.Rulesets.Timing
                 return;
             }
 
-            layout.Invalidate();
+            durationBacking.Invalidate();
 
             base.InvalidateFromChild(invalidation);
         }
 
-        protected override void UpdateAfterChildren()
+        private Cached<double> durationBacking = new Cached<double>();
+        /// <summary>
+        /// The maximum duration of any one hit object inside this <see cref="DrawableTimingSection"/>. This is calculated as the maximum
+        /// end time between all hit objects relative to this <see cref="DrawableTimingSection"/>'s <see cref="MultiplierControlPoint.StartTime"/>.
+        /// </summary>
+        public double Duration => durationBacking.EnsureValid()
+            ? durationBacking.Value
+            : durationBacking.Refresh(() =>
         {
-            base.UpdateAfterChildren();
+            if (!Children.Any())
+                return 0;
 
-            if (!layout.EnsureValid())
+            double baseDuration = Children.Max(c => (c.HitObject as IHasEndTime)?.EndTime ?? c.HitObject.StartTime) - ControlPoint.StartTime;
-            {
-                layout.Refresh(() =>
-                {
-                    if (!Children.Any())
-                        return;
 
-                    //double maxDuration = Children.Select(c => (c.HitObject as IHasEndTime)?.EndTime ?? c.HitObject.StartTime).Max();
+            // If we have a singular hit object at the timing section's start time, let's set a sane default duration
-                    //float width = (float)maxDuration - RelativeChildOffset.X;
+            if (baseDuration == 0)
-                    //float height = (float)maxDuration - RelativeChildOffset.Y;
+                baseDuration = 1;
 
+            // Scrolling ruleset hit objects typically have anchors+origins set to the hit object's start time, but if the hit object doesn't implement IHasEndTime and lies on the control point
+            // then the baseDuration above will be 0. This will cause problems with masking when it is further set as the value for Size in Update(). We _want_ the timing section bounds to
+            // completely enclose the hit object to avoid the masking optimisations.
+            //
+            // To do this we need to find a duration that corresponds to the absolute size of the element that extrudes beyond the timing section's bounds and add that to baseDuration.
+            // We can utilize the fact that the Size and RelativeChildSpace are 1:1, meaning that an change in duration for the timing section has no change to the hit object's positioning
+            // and simply find the largest absolutely-sized element in this timing section. This introduces a little bit of error, but will never under-estimate the duration.
 
-                    // Auto-size to the total size of our children
+            // Find the largest element that is absolutely-sized along ScrollingAxes
-                    // This ends up being the total duration of our children, however for now this is a more sure-fire way to calculate this
+            float maxAbsoluteSize = Children.Where(c => (c.RelativeSizeAxes & ScrollingAxes) == 0)
-                    // than the above due to some undesired masking optimisations causing some hit objects to be culled...
+                                            .Select(c => (ScrollingAxes & Axes.X) > 0 ? c.Width : c.Height)
-                    // Todo: When this is investigated more we should use the above method as it is a little more exact
+                                            .DefaultIfEmpty().Max();
-                    // Todo: This is not working correctly in the case that hit objects are absolutely-sized - needs a proper looking into in osu!framework
-                    float width = Children.Select(child => child.X + child.Width).Max() - RelativeChildOffset.X;
-                    float height = Children.Select(child => child.Y + child.Height).Max() - RelativeChildOffset.Y;
 
-                    // Consider that width/height are time values. To have ourselves span these time values 1:1, we first need to set our size
+            float ourAbsoluteSize = (ScrollingAxes & Axes.X) > 0 ? DrawWidth : DrawHeight;
-                    Size = new Vector2((ScrollingAxes & Axes.X) > 0 ? width : Size.X, (ScrollingAxes & Axes.Y) > 0 ? height : Size.Y);
+
-                    // Then to make our position-space be time values again, we need our relative child size to follow our size
+            // Add the extra duration to account for the absolute size
-                    RelativeChildSize = Size;
+            baseDuration *= 1 + maxAbsoluteSize / ourAbsoluteSize;
-                });
+
-            }
+            return baseDuration;
+        });
+
+        protected override void Update()
+        {
+            base.Update();
+
+            // We want our size and position-space along ScrollingAxes to span our duration to completely enclose all the hit objects
+            Size = new Vector2((ScrollingAxes & Axes.X) > 0 ? (float)Duration : Size.X, (ScrollingAxes & Axes.Y) > 0 ? (float)Duration : Size.Y);
+            // And we need to make sure the hit object's position-space doesn't change due to our resizing
+            RelativeChildSize = Size;
         }
     }
 }

osu.Game/Rulesets/Timing/SpeedAdjustmentCollection.cs

@@ -51,6 +51,13 @@ namespace osu.Game.Rulesets.Timing
             this.scrollingAxes = scrollingAxes;
         }
 
+        public override void Add(SpeedAdjustmentContainer speedAdjustment)
+        {
+            speedAdjustment.VisibleTimeRange.BindTo(VisibleTimeRange);
+            speedAdjustment.ScrollingAxes = scrollingAxes;
+            base.Add(speedAdjustment);
+        }
+
         /// <summary>
         /// Adds a hit object to this <see cref="SpeedAdjustmentCollection"/>. The hit objects will be kept in a queue
         /// and will be processed when new <see cref="SpeedAdjustmentContainer"/>s are added to this <see cref="SpeedAdjustmentCollection"/>.
@@ -58,14 +65,10 @@ namespace osu.Game.Rulesets.Timing
         /// <param name="hitObject">The hit object to add.</param>
         public void Add(DrawableHitObject hitObject)
         {
-            queuedHitObjects.Enqueue(hitObject);
+            if (!(hitObject is IScrollingHitObject))
-        }
+                throw new InvalidOperationException($"Hit objects added to a {nameof(SpeedAdjustmentCollection)} must implement {nameof(IScrollingHitObject)}.");
 
-        public override void Add(SpeedAdjustmentContainer speedAdjustment)
+            queuedHitObjects.Enqueue(hitObject);
-        {
-            speedAdjustment.VisibleTimeRange.BindTo(VisibleTimeRange);
-            speedAdjustment.ScrollingAxes = scrollingAxes;
-            base.Add(speedAdjustment);
         }
 
         protected override void Update()

osu.Game/Rulesets/Timing/SpeedAdjustmentContainer.cs

@@ -42,6 +42,8 @@ namespace osu.Game.Rulesets.Timing
 
         public readonly MultiplierControlPoint ControlPoint;
 
+        private DrawableTimingSection timingSection;
+
         /// <summary>
         /// Creates a new <see cref="SpeedAdjustmentContainer"/>.
         /// </summary>
@@ -56,9 +58,10 @@ namespace osu.Game.Rulesets.Timing
         [BackgroundDependencyLoader]
         private void load()
         {
-            DrawableTimingSection timingSection = CreateTimingSection();
+            timingSection = CreateTimingSection();
 
             timingSection.ScrollingAxes = ScrollingAxes;
+            timingSection.ControlPoint = ControlPoint;
             timingSection.VisibleTimeRange.BindTo(VisibleTimeRange);
             timingSection.RelativeChildOffset = new Vector2((ScrollingAxes & Axes.X) > 0 ? (float)ControlPoint.StartTime : 0, (ScrollingAxes & Axes.Y) > 0 ? (float)ControlPoint.StartTime : 0);
 
@@ -74,6 +77,17 @@ namespace osu.Game.Rulesets.Timing
             RelativeChildSize = new Vector2((ScrollingAxes & Axes.X) > 0 ? (float)VisibleTimeRange : 1, (ScrollingAxes & Axes.Y) > 0 ? (float)VisibleTimeRange : 1);
         }
 
+        public override double LifetimeStart => ControlPoint.StartTime - VisibleTimeRange;
+        public override double LifetimeEnd => ControlPoint.StartTime + timingSection.Duration + VisibleTimeRange;
+
+        public override void Add(DrawableHitObject drawable)
+        {
+            var scrollingHitObject = drawable as IScrollingHitObject;
+            scrollingHitObject?.LifetimeOffset.BindTo(VisibleTimeRange);
+
+            base.Add(drawable);
+        }
+
         /// <summary>
         /// Whether this speed adjustment can contain a hit object. This is true if the hit object occurs after this speed adjustment with respect to time.
         /// </summary>

osu.Game/osu.Game.csproj

@@ -185,6 +185,7 @@
     <Compile Include="Rulesets\Objects\SliderCurve.cs" />
     <Compile Include="Rulesets\Objects\Types\CurveType.cs" />
     <Compile Include="Rulesets\Objects\Drawables\IDrawableHitObjectWithProxiedApproach.cs" />
+    <Compile Include="Rulesets\Objects\Drawables\IScrollingHitObject.cs" />
     <Compile Include="Rulesets\Judgements\Judgement.cs" />
     <Compile Include="Rulesets\Objects\HitObjectParser.cs" />
     <Compile Include="Rulesets\Objects\HitObjectStartTimeComparer.cs" />
@@ -223,6 +224,7 @@
     <Compile Include="Beatmaps\Drawables\DifficultyColouredContainer.cs" />
     <Compile Include="Beatmaps\Drawables\Panel.cs" />
     <Compile Include="Rulesets\Objects\Drawables\DrawableHitObject.cs" />
+    <Compile Include="Rulesets\Objects\Drawables\DrawableScrollingHitObject.cs" />
     <Compile Include="Rulesets\Objects\HitObject.cs" />
     <Compile Include="Configuration\OsuConfigManager.cs" />
     <Compile Include="Overlays\Notifications\IHasCompletionTarget.cs" />