Merge branch 'master' into mania-distance-snap-grid

osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs

@@ -162,8 +162,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components
             if (ControlPoint == slider.Path.ControlPoints[0])
             {
                 // Special handling for the head control point - the position of the slider changes which means the snapped position and time have to be taken into account
-                var result = snapProvider?.SnapScreenSpacePositionToValidTime(e.MousePosition);
+                var result = snapProvider?.SnapScreenSpacePositionToValidTime(e.ScreenSpaceMousePosition);
-                Vector2 movementDelta = (result?.ScreenSpacePosition ?? e.MousePosition) - slider.Position;
+
+                Vector2 movementDelta = Parent.ToLocalSpace(result?.ScreenSpacePosition ?? e.ScreenSpaceMousePosition) - slider.Position;
 
                 slider.Position += movementDelta;
                 slider.StartTime = result?.Time ?? slider.StartTime;

osu.Game.Tests/NonVisual/BarLineGeneratorTest.cs

@@ -0,0 +1,73 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Collections.Generic;
+using NUnit.Framework;
+using osu.Framework.Utils;
+using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Beatmaps.Timing;
+using osu.Game.Rulesets.Objects;
+
+namespace osu.Game.Tests.NonVisual
+{
+    public class BarLineGeneratorTest
+    {
+        [Test]
+        public void TestRoundingErrorCompensation()
+        {
+            // The aim of this test is to make sure bar line generation compensates for floating-point errors.
+            // The premise of the test is that we have a single timing point that should result in bar lines
+            // that start at a time point that is a whole number every seventh beat.
+
+            // The fact it's every seventh beat is important - it's a number indivisible by 2, which makes
+            // it susceptible to rounding inaccuracies. In fact this was originally spotted in cases of maps
+            // that met exactly this criteria.
+
+            const int beat_length_numerator = 2000;
+            const int beat_length_denominator = 7;
+            const TimeSignatures signature = TimeSignatures.SimpleQuadruple;
+
+            var beatmap = new Beatmap
+            {
+                HitObjects = new List<HitObject>
+                {
+                    new HitObject { StartTime = 0 },
+                    new HitObject { StartTime = 120_000 }
+                },
+                ControlPointInfo = new ControlPointInfo()
+            };
+
+            beatmap.ControlPointInfo.Add(0, new TimingControlPoint
+            {
+                BeatLength = (double)beat_length_numerator / beat_length_denominator,
+                TimeSignature = signature
+            });
+
+            var barLines = new BarLineGenerator<BarLine>(beatmap).BarLines;
+
+            for (int i = 0; i * beat_length_denominator < barLines.Count; i++)
+            {
+                var barLine = barLines[i * beat_length_denominator];
+                var expectedTime = beat_length_numerator * (int)signature * i;
+
+                // every seventh bar's start time should be at least greater than the whole number we expect.
+                // It cannot be less, as that can affect overlapping scroll algorithms
+                // (the previous timing point might be chosen incorrectly if this is not the case)
+                Assert.GreaterOrEqual(barLine.StartTime, expectedTime);
+
+                // on the other side, make sure we don't stray too far from the expected time either.
+                Assert.IsTrue(Precision.AlmostEquals(barLine.StartTime, expectedTime));
+
+                // check major/minor lines for good measure too
+                Assert.AreEqual(i % (int)signature == 0, barLine.Major);
+            }
+        }
+
+        private class BarLine : IBarLine
+        {
+            public double StartTime { get; set; }
+            public bool Major { get; set; }
+        }
+    }
+}

osu.Game.Tests/ScrollAlgorithms/SequentialScrollTest.cs

@@ -29,8 +29,22 @@ namespace osu.Game.Tests.ScrollAlgorithms
         [Test]
         public void TestDisplayStartTime()
         {
-            // Sequential scroll algorithm approximates the start time
+            // easy cases - time range adjusted for velocity fits within control point duration
-            // This should be fixed in the future
+            Assert.AreEqual(2500, algorithm.GetDisplayStartTime(5000, 0, 2500, 1)); // 5000 - (2500 / 1)
+            Assert.AreEqual(13750, algorithm.GetDisplayStartTime(15000, 0, 2500, 1)); // 15000 - (2500 / 2)
+            Assert.AreEqual(20000, algorithm.GetDisplayStartTime(25000, 0, 2500, 1)); // 25000 - (2500 / 0.5)
+
+            // hard case - time range adjusted for velocity exceeds control point duration
+
+            // 1st multiplier point takes 10000 / 2500 = 4 scroll lengths
+            // 2nd multiplier point takes 10000 / (2500 / 2) = 8 scroll lengths
+            // 3rd multiplier point takes 2500 / (2500 * 2) = 0.5 scroll lengths up to hitobject start
+
+            // absolute position of the hitobject = 1000 * (4 + 8 + 0.5) = 12500
+            // minus one scroll length allowance = 12500 - 1000 = 11500 = 11.5 [scroll lengths]
+            // therefore the start time lies within the second multiplier point (because 11.5 < 4 + 8)
+            // its exact time position is = 10000 + 7.5 * (2500 / 2) = 19375
+            Assert.AreEqual(19375, algorithm.GetDisplayStartTime(22500, 0, 2500, 1000));
         }
 
         [Test]

osu.Game.Tests/Visual/Gameplay/TestSceneScrollingHitObjects.cs

@@ -16,6 +16,7 @@ using osu.Game.Configuration;
 using osu.Game.Rulesets.Mods;
 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.Scrolling;
 using osuTK;
@@ -77,19 +78,18 @@ namespace osu.Game.Tests.Visual.Gameplay
                 }
             };
 
-            setUpHitObjects();
+            hitObjectSpawnDelegate?.Cancel();
         });
 
-        private void setUpHitObjects()
+        private void setUpHitObjects() => AddStep("set up hit objects", () =>
         {
             scrollContainers.ForEach(c => c.ControlPoints.Add(new MultiplierControlPoint(0)));
 
             for (int i = spawn_rate / 2; i <= time_range; i += spawn_rate)
                 addHitObject(Time.Current + i);
 
-            hitObjectSpawnDelegate?.Cancel();
             hitObjectSpawnDelegate = Scheduler.AddDelayed(() => addHitObject(Time.Current + time_range), spawn_rate, true);
-        }
+        });
 
         private IList<MultiplierControlPoint> testControlPoints => new List<MultiplierControlPoint>
         {
@@ -101,6 +101,8 @@ namespace osu.Game.Tests.Visual.Gameplay
         [Test]
         public void TestScrollAlgorithms()
         {
+            setUpHitObjects();
+
             AddStep("constant scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Constant));
             AddStep("overlapping scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Overlapping));
             AddStep("sequential scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Sequential));
@@ -113,6 +115,8 @@ namespace osu.Game.Tests.Visual.Gameplay
         [Test]
         public void TestConstantScrollLifetime()
         {
+            setUpHitObjects();
+
             AddStep("set constant scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Constant));
             // scroll container time range must be less than the rate of spawning hitobjects
             // otherwise the hitobjects will spawn already partly visible on screen and look wrong
@@ -122,14 +126,40 @@ namespace osu.Game.Tests.Visual.Gameplay
         [Test]
         public void TestSequentialScrollLifetime()
         {
+            setUpHitObjects();
+
             AddStep("set sequential scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Sequential));
             AddStep("set time range", () => scrollContainers.ForEach(c => c.TimeRange = time_range / 2.0));
             AddStep("add control points", () => addControlPoints(testControlPoints, Time.Current));
         }
 
+        [Test]
+        public void TestSlowSequentialScroll()
+        {
+            AddStep("set sequential scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Sequential));
+            AddStep("set time range", () => scrollContainers.ForEach(c => c.TimeRange = time_range));
+            AddStep("add control points", () => addControlPoints(
+                new List<MultiplierControlPoint>
+                {
+                    new MultiplierControlPoint { Velocity = 0.1 }
+                },
+                Time.Current + time_range));
+
+            // All of the hit objects added below should be immediately visible on screen
+            AddStep("add hit objects", () =>
+            {
+                for (int i = 0; i < 20; ++i)
+                {
+                    addHitObject(Time.Current + time_range * (2 + 0.1 * i));
+                }
+            });
+        }
+
         [Test]
         public void TestOverlappingScrollLifetime()
         {
+            setUpHitObjects();
+
             AddStep("set overlapping scroll", () => setScrollAlgorithm(ScrollVisualisationMethod.Overlapping));
             AddStep("set time range", () => scrollContainers.ForEach(c => c.TimeRange = time_range / 2.0));
             AddStep("add control points", () => addControlPoints(testControlPoints, Time.Current));
@@ -221,7 +251,7 @@ namespace osu.Game.Tests.Visual.Gameplay
         private class TestDrawableControlPoint : DrawableHitObject<HitObject>
         {
             public TestDrawableControlPoint(ScrollingDirection direction, double time)
-                : base(new HitObject { StartTime = time })
+                : base(new HitObject { StartTime = time, HitWindows = HitWindows.Empty })
             {
                 Origin = Anchor.Centre;
 
@@ -252,7 +282,7 @@ namespace osu.Game.Tests.Visual.Gameplay
         private class TestDrawableHitObject : DrawableHitObject<HitObject>
         {
             public TestDrawableHitObject(double time)
-                : base(new HitObject { StartTime = time })
+                : base(new HitObject { StartTime = time, HitWindows = HitWindows.Empty })
             {
                 Origin = Anchor.Custom;
                 OriginPosition = new Vector2(75 / 4.0f);

osu.Game/Rulesets/Edit/PlacementBlueprint.cs

@@ -64,9 +64,7 @@ namespace osu.Game.Rulesets.Edit
         /// <param name="commitStart">Whether this call is committing a value for HitObject.StartTime and continuing with further adjustments.</param>
         protected void BeginPlacement(bool commitStart = false)
         {
-            // applies snapping to above time
             placementHandler.BeginPlacement(HitObject);
-
             PlacementActive |= commitStart;
         }
 

osu.Game/Rulesets/Objects/BarLineGenerator.cs

@@ -1,6 +1,7 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using osu.Framework.Utils;
@@ -46,6 +47,16 @@ namespace osu.Game.Rulesets.Objects
 
                 for (double t = currentTimingPoint.Time; Precision.DefinitelyBigger(endTime, t); t += barLength, currentBeat++)
                 {
+                    var roundedTime = Math.Round(t, MidpointRounding.AwayFromZero);
+
+                    // in the case of some bar lengths, rounding errors can cause t to be slightly less than
+                    // the expected whole number value due to floating point inaccuracies.
+                    // if this is the case, apply rounding.
+                    if (Precision.AlmostEquals(t, roundedTime))
+                    {
+                        t = roundedTime;
+                    }
+
                     BarLines.Add(new TBarLine
                     {
                         StartTime = t,

osu.Game/Rulesets/UI/Scrolling/Algorithms/SequentialScrollAlgorithm.cs

@@ -22,8 +22,7 @@ namespace osu.Game.Rulesets.UI.Scrolling.Algorithms
 
         public double GetDisplayStartTime(double originTime, float offset, double timeRange, float scrollLength)
         {
-            double adjustedTime = TimeAt(-offset, originTime, timeRange, scrollLength);
+            return TimeAt(-(scrollLength + offset), originTime, timeRange, scrollLength);
-            return adjustedTime - timeRange - 1000;
         }
 
         public float GetLength(double startTime, double endTime, double timeRange, float scrollLength)

osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs

@@ -181,8 +181,8 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
         [Resolved]
         private IBeatSnapProvider beatSnapProvider { get; set; }
 
-        public SnapResult SnapScreenSpacePositionToValidTime(Vector2 position) =>
+        public SnapResult SnapScreenSpacePositionToValidTime(Vector2 screenSpacePosition) =>
-            new SnapResult(position, beatSnapProvider.SnapTime(getTimeFromPosition(Content.ToLocalSpace(position))));
+            new SnapResult(screenSpacePosition, beatSnapProvider.SnapTime(getTimeFromPosition(Content.ToLocalSpace(screenSpacePosition))));
 
         private double getTimeFromPosition(Vector2 localPosition) =>
             (localPosition.X / Content.DrawWidth) * track.Length;

osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs

@@ -282,7 +282,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
                         case IHasRepeats repeatHitObject:
                             // find the number of repeats which can fit in the requested time.
                             var lengthOfOneRepeat = repeatHitObject.Duration / (repeatHitObject.RepeatCount + 1);
-                            var proposedCount = Math.Max(0, (int)((time - hitObject.StartTime) / lengthOfOneRepeat) - 1);
+                            var proposedCount = Math.Max(0, (int)Math.Round((time - hitObject.StartTime) / lengthOfOneRepeat) - 1);
 
                             if (proposedCount == repeatHitObject.RepeatCount)
                                 return;