Merge branch 'master' into move-screen-restrictions

osu.Desktop.Tests/Visual/TestCaseScrollingPlayfield.cs

@@ -3,19 +3,23 @@
 
 using System;
 using System.Collections.Generic;
+using NUnit.Framework;
 using OpenTK;
 using osu.Desktop.Tests.Beatmaps;
+using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Input;
 using osu.Framework.Timing;
 using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.ControlPoints;
 using osu.Game.Rulesets.Beatmaps;
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.Scoring;
+using osu.Game.Rulesets.Timing;
 using osu.Game.Rulesets.UI;
 
 namespace osu.Desktop.Tests.Visual
@@ -23,6 +27,7 @@ namespace osu.Desktop.Tests.Visual
     /// <summary>
     /// The most minimal implementation of a playfield with scrolling hit objects.
     /// </summary>
+    [TestFixture]
     public class TestCaseScrollingPlayfield : OsuTestCase
     {
         public TestCaseScrollingPlayfield()
@@ -64,6 +69,66 @@ namespace osu.Desktop.Tests.Visual
             });
         }
 
+        [Test]
+        public void TestSpeedAdjustmentOrdering()
+        {
+            var hitObjectContainer = new ScrollingPlayfield<TestHitObject, TestJudgement>.ScrollingHitObjectContainer(Axes.X);
+
+            var speedAdjustments = new[]
+            {
+                new SpeedAdjustmentContainer(new MultiplierControlPoint()),
+                new SpeedAdjustmentContainer(new MultiplierControlPoint(1000)
+                {
+                    TimingPoint = new TimingControlPoint { BeatLength = 500 }
+                }),
+                new SpeedAdjustmentContainer(new MultiplierControlPoint(2000)
+                {
+                    TimingPoint = new TimingControlPoint { BeatLength = 1000 },
+                    DifficultyPoint = new DifficultyControlPoint { SpeedMultiplier = 2}
+                }),
+                new SpeedAdjustmentContainer(new MultiplierControlPoint(3000)
+                {
+                    TimingPoint = new TimingControlPoint { BeatLength = 1000 },
+                    DifficultyPoint = new DifficultyControlPoint { SpeedMultiplier = 1}
+                }),
+            };
+
+            var hitObjects = new[]
+            {
+                new DrawableTestHitObject(Axes.X, new TestHitObject { StartTime = -1000 }),
+                new DrawableTestHitObject(Axes.X, new TestHitObject()),
+                new DrawableTestHitObject(Axes.X, new TestHitObject { StartTime = 1000 }),
+                new DrawableTestHitObject(Axes.X, new TestHitObject { StartTime = 2000 }),
+                new DrawableTestHitObject(Axes.X, new TestHitObject { StartTime = 3000 }),
+                new DrawableTestHitObject(Axes.X, new TestHitObject { StartTime = 4000 }),
+            };
+
+            hitObjects.ForEach(h => hitObjectContainer.Add(h));
+            speedAdjustments.ForEach(hitObjectContainer.AddSpeedAdjustment);
+
+            // The 0th index in hitObjectContainer.SpeedAdjustments is the "default" control point
+            // Check multiplier of the default speed adjustment
+            Assert.AreEqual(1, hitObjectContainer.SpeedAdjustments[0].ControlPoint.Multiplier);
+            Assert.AreEqual(1, speedAdjustments[0].ControlPoint.Multiplier);
+            Assert.AreEqual(2, speedAdjustments[1].ControlPoint.Multiplier);
+            Assert.AreEqual(2, speedAdjustments[2].ControlPoint.Multiplier);
+            Assert.AreEqual(1, speedAdjustments[3].ControlPoint.Multiplier);
+
+            // Check insertion of hit objects
+            Assert.IsTrue(hitObjectContainer.SpeedAdjustments[0].Contains(hitObjects[0]));
+            Assert.IsTrue(speedAdjustments[0].Contains(hitObjects[1]));
+            Assert.IsTrue(speedAdjustments[1].Contains(hitObjects[2]));
+            Assert.IsTrue(speedAdjustments[2].Contains(hitObjects[3]));
+            Assert.IsTrue(speedAdjustments[3].Contains(hitObjects[4]));
+            Assert.IsTrue(speedAdjustments[3].Contains(hitObjects[5]));
+
+            hitObjectContainer.RemoveSpeedAdjustment(speedAdjustments[1]);
+
+            // The hit object contained in this speed adjustment should be resorted into the previous one
+
+            Assert.IsTrue(speedAdjustments[0].Contains(hitObjects[2]));
+        }
+
         private class TestRulesetContainer : ScrollingRulesetContainer<TestPlayfield, TestHitObject, TestJudgement>
         {
             private readonly Axes scrollingAxes;
@@ -158,4 +223,4 @@ namespace osu.Desktop.Tests.Visual
             public override string MaxResultString { get { throw new NotImplementedException(); } }
         }
     }
-}
+}

osu.Game/Rulesets/Timing/SpeedAdjustmentContainer.cs

@@ -1,7 +1,6 @@
 // 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.Allocation;
 using osu.Framework.Configuration;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
@@ -27,7 +26,7 @@ namespace osu.Game.Rulesets.Timing
         public readonly BindableBool Reversed = new BindableBool();
 
         protected override Container<DrawableHitObject> Content => content;
-        private Container<DrawableHitObject> content;
+        private readonly Container<DrawableHitObject> content;
 
         /// <summary>
         /// The axes which the content of this container will scroll through.
@@ -41,7 +40,7 @@ namespace osu.Game.Rulesets.Timing
         /// </summary>
         public readonly MultiplierControlPoint ControlPoint;
 
-        private ScrollingContainer scrollingContainer;
+        private readonly ScrollingContainer scrollingContainer;
 
         /// <summary>
         /// Creates a new <see cref="SpeedAdjustmentContainer"/>.
@@ -51,11 +50,7 @@ namespace osu.Game.Rulesets.Timing
         {
             ControlPoint = controlPoint;
             RelativeSizeAxes = Axes.Both;
-        }
 
-        [BackgroundDependencyLoader]
-        private void load()
-        {
             scrollingContainer = CreateScrollingContainer();
 
             scrollingContainer.ScrollingAxes = ScrollingAxes;

osu.Game/Rulesets/UI/ScrollingPlayfield.cs

@@ -62,7 +62,7 @@ namespace osu.Game.Rulesets.UI
         /// <summary>
         /// The container that contains the <see cref="SpeedAdjustmentContainer"/>s and <see cref="DrawableHitObject"/>s.
         /// </summary>
-        internal new readonly ScrollingHitObjectContainer HitObjects;
+        public new readonly ScrollingHitObjectContainer HitObjects;
 
         /// <summary>
         /// Creates a new <see cref="ScrollingPlayfield{TObject, TJudgement}"/>.
@@ -138,7 +138,7 @@ namespace osu.Game.Rulesets.UI
         /// <summary>
         /// A container that provides the foundation for sorting <see cref="DrawableHitObject"/>s into <see cref="SpeedAdjustmentContainer"/>s.
         /// </summary>
-        internal class ScrollingHitObjectContainer : HitObjectContainer
+        public class ScrollingHitObjectContainer : HitObjectContainer
         {
             /// <summary>
             /// Gets or sets the range of time that is visible by the length of the scrolling axes.
@@ -152,6 +152,9 @@ namespace osu.Game.Rulesets.UI
             public readonly BindableBool Reversed = new BindableBool();
 
             private readonly Container<SpeedAdjustmentContainer> speedAdjustments;
+            public IReadOnlyList<SpeedAdjustmentContainer> SpeedAdjustments => speedAdjustments;
+
+            private readonly SpeedAdjustmentContainer defaultSpeedAdjustment;
 
             private readonly Axes scrollingAxes;
 
@@ -166,7 +169,7 @@ namespace osu.Game.Rulesets.UI
                 AddInternal(speedAdjustments = new Container<SpeedAdjustmentContainer> { RelativeSizeAxes = Axes.Both });
 
                 // Default speed adjustment
-                AddSpeedAdjustment(new SpeedAdjustmentContainer(new MultiplierControlPoint(0)));
+                AddSpeedAdjustment(defaultSpeedAdjustment = new SpeedAdjustmentContainer(new MultiplierControlPoint(0)));
             }
 
             /// <summary>
@@ -181,18 +184,44 @@ namespace osu.Game.Rulesets.UI
                 speedAdjustments.Add(speedAdjustment);
 
                 // We now need to re-sort the hit objects in the last speed adjustment prior to this one, to see if they need a new parent
-                var previousSpeedAdjustment = speedAdjustments.LastOrDefault(s => s.ControlPoint.StartTime < speedAdjustment.ControlPoint.StartTime);
+                var previousSpeedAdjustment = speedAdjustments.LastOrDefault(s => s != speedAdjustment && s.ControlPoint.StartTime <= speedAdjustment.ControlPoint.StartTime);
                 if (previousSpeedAdjustment == null)
                     return;
 
-                foreach (DrawableHitObject h in previousSpeedAdjustment.Children)
+                for (int i = 0; i < previousSpeedAdjustment.Children.Count; i++)
                 {
-                    var newSpeedAdjustment = adjustmentContainerFor(h);
+                    DrawableHitObject hitObject = previousSpeedAdjustment[i];
+
+                    var newSpeedAdjustment = adjustmentContainerFor(hitObject);
                     if (newSpeedAdjustment == previousSpeedAdjustment)
                         continue;
 
-                    previousSpeedAdjustment.Remove(h);
+                    previousSpeedAdjustment.Remove(hitObject);
-                    newSpeedAdjustment.Add(h);
+                    newSpeedAdjustment.Add(hitObject);
+
+                    i--;
+                }
+            }
+
+            /// <summary>
+            /// Removes a <see cref="SpeedAdjustmentContainer"/> from this container, re-sorting all hit objects
+            /// which it contained into new <see cref="SpeedAdjustmentContainer"/>s.
+            /// </summary>
+            /// <param name="speedAdjustment">The <see cref="SpeedAdjustmentContainer"/> to remove.</param>
+            public void RemoveSpeedAdjustment(SpeedAdjustmentContainer speedAdjustment)
+            {
+                if (speedAdjustment == defaultSpeedAdjustment)
+                    throw new InvalidOperationException($"The default {nameof(SpeedAdjustmentContainer)} must not be removed.");
+
+                if (!speedAdjustments.Remove(speedAdjustment))
+                    return;
+
+                while (speedAdjustment.Count > 0)
+                {
+                    DrawableHitObject hitObject = speedAdjustment[0];
+
+                    speedAdjustment.Remove(hitObject);
+                    Add(hitObject);
                 }
             }
 
@@ -208,11 +237,7 @@ namespace osu.Game.Rulesets.UI
                 if (!(hitObject is IScrollingHitObject))
                     throw new InvalidOperationException($"Hit objects added to a {nameof(ScrollingHitObjectContainer)} must implement {nameof(IScrollingHitObject)}.");
 
-                var target = adjustmentContainerFor(hitObject);
+                adjustmentContainerFor(hitObject).Add(hitObject);
-                if (target == null)
-                    throw new InvalidOperationException($"A {nameof(SpeedAdjustmentContainer)} to container {hitObject} could not be found.");
-
-                target.Add(hitObject);
             }
 
             public override bool Remove(DrawableHitObject hitObject) => speedAdjustments.Any(s => s.Remove(hitObject));
@@ -224,7 +249,7 @@ namespace osu.Game.Rulesets.UI
             /// </summary>
             /// <param name="hitObject">The hit object to find the active <see cref="SpeedAdjustmentContainer"/> for.</param>
             /// <returns>The <see cref="SpeedAdjustmentContainer"/> active at <paramref name="hitObject"/>'s start time. Null if there are no speed adjustments.</returns>
-            private SpeedAdjustmentContainer adjustmentContainerFor(DrawableHitObject hitObject) => speedAdjustments.FirstOrDefault(c => c.CanContain(hitObject)) ?? speedAdjustments.LastOrDefault();
+            private SpeedAdjustmentContainer adjustmentContainerFor(DrawableHitObject hitObject) => speedAdjustments.LastOrDefault(c => c.CanContain(hitObject)) ?? defaultSpeedAdjustment;
 
             /// <summary>
             /// Finds the <see cref="SpeedAdjustmentContainer"/> which provides the speed adjustment active at a time.
@@ -232,7 +257,7 @@ namespace osu.Game.Rulesets.UI
             /// </summary>
             /// <param name="time">The time to find the active <see cref="SpeedAdjustmentContainer"/> at.</param>
             /// <returns>The <see cref="SpeedAdjustmentContainer"/> active at <paramref name="time"/>. Null if there are no speed adjustments.</returns>
-            private SpeedAdjustmentContainer adjustmentContainerAt(double time) => speedAdjustments.FirstOrDefault(c => c.CanContain(time)) ?? speedAdjustments.LastOrDefault();
+            private SpeedAdjustmentContainer adjustmentContainerAt(double time) => speedAdjustments.LastOrDefault(c => c.CanContain(time)) ?? defaultSpeedAdjustment;
         }
     }
 }