Add some basic tests + fixes to seek/seekforward functionality

osu.Game.Tests/Visual/TestCaseEditorSeekSnapping.cs

@@ -0,0 +1,325 @@
+// Copyright (c) 2007-2018 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 osu.Framework.Allocation;
+using osu.Framework.Audio.Track;
+using osu.Framework.Extensions.Color4Extensions;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.Shapes;
+using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Edit;
+using osu.Game.Rulesets.Edit.Tools;
+using osu.Game.Rulesets.Osu;
+using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Tests.Beatmaps;
+using OpenTK;
+using OpenTK.Graphics;
+
+namespace osu.Game.Tests.Visual
+{
+    public class TestCaseEditorSeekSnapping : OsuTestCase
+    {
+        public override IReadOnlyList<Type> RequiredTypes => new[] { typeof(HitObjectComposer) };
+
+        private Track track;
+        private HitObjectComposer composer;
+
+        [BackgroundDependencyLoader]
+        private void load(OsuGameBase osuGame)
+        {
+            var testBeatmap = new Beatmap
+            {
+                ControlPointInfo = new ControlPointInfo
+                {
+                    TimingPoints =
+                    {
+                        new TimingControlPoint { Time = 0, BeatLength = 200},
+                        new TimingControlPoint { Time = 100, BeatLength = 400 },
+                        new TimingControlPoint { Time = 175, BeatLength = 800 },
+                        new TimingControlPoint { Time = 350, BeatLength = 200 },
+                        new TimingControlPoint { Time = 450, BeatLength = 100 }
+                    }
+                },
+                HitObjects =
+                {
+                    new HitCircle { StartTime = 0 },
+                    new HitCircle { StartTime = 5000 }
+                }
+            };
+
+            osuGame.Beatmap.Value = new TestWorkingBeatmap(testBeatmap);
+            track = osuGame.Beatmap.Value.Track;
+
+            Child = new GridContainer
+            {
+                RelativeSizeAxes = Axes.Both,
+                Content = new[]
+                {
+                    new Drawable[] { composer = new TestHitObjectComposer(new OsuRuleset()) },
+                    new Drawable[] { new TimingPointVisualiser(testBeatmap, track) },
+                },
+                RowDimensions = new[]
+                {
+                    new Dimension(GridSizeMode.Distributed),
+                    new Dimension(GridSizeMode.AutoSize),
+                }
+            };
+
+//            testSeekNoSnapping();
+//            testSeekSnappingOnBeat();
+//            testSeekSnappingInBetweenBeat();
+//            testSeekForwardNoSnapping();
+            testSeekForwardSnappingOnBeat();
+        }
+
+        /// <summary>
+        /// Tests whether time is correctly seeked without snapping.
+        /// </summary>
+        private void testSeekNoSnapping()
+        {
+            reset();
+
+            // Forwards
+            AddStep("Seek(0)", () => composer.SeekTo(0));
+            AddAssert("Time = 0", () => track.CurrentTime == 0);
+            AddStep("Seek(33)", () => composer.SeekTo(33));
+            AddAssert("Time = 33", () => track.CurrentTime == 33);
+            AddStep("Seek(89)", () => composer.SeekTo(89));
+            AddAssert("Time = 89", () => track.CurrentTime == 89);
+
+            // Backwards
+            AddStep("Seek(25)", () => composer.SeekTo(25));
+            AddAssert("Time = 25", () => track.CurrentTime == 25);
+            AddStep("Seek(0)", () => composer.SeekTo(0));
+            AddAssert("Time = 0", () => track.CurrentTime == 0);
+        }
+
+        /// <summary>
+        /// Tests whether seeking to exact beat times puts us on the beat time.
+        /// These are the white/yellow ticks on the graph.
+        /// </summary>
+        private void testSeekSnappingOnBeat()
+        {
+            reset();
+
+            AddStep("Seek(0), Snap", () => composer.SeekTo(0, true));
+            AddAssert("Time = 0", () => track.CurrentTime == 0);
+            AddStep("Seek(50), Snap", () => composer.SeekTo(50, true));
+            AddAssert("Time = 50", () => track.CurrentTime == 50);
+            AddStep("Seek(100), Snap", () => composer.SeekTo(100, true));
+            AddAssert("Time = 100", () => track.CurrentTime == 100);
+            AddStep("Seek(175), Snap", () => composer.SeekTo(175, true));
+            AddAssert("Time = 175", () => track.CurrentTime == 175);
+            AddStep("Seek(350), Snap", () => composer.SeekTo(350, true));
+            AddAssert("Time = 350", () => track.CurrentTime == 350);
+            AddStep("Seek(400), Snap", () => composer.SeekTo(400, true));
+            AddAssert("Time = 400", () => track.CurrentTime == 400);
+            AddStep("Seek(450), Snap", () => composer.SeekTo(450, true));
+            AddAssert("Time = 450", () => track.CurrentTime == 450);
+        }
+
+        /// <summary>
+        /// Tests whether seeking to somewhere in the middle between beats puts us on the expected beats.
+        /// For example, snapping between a white/yellow beat should put us on either the yellow or white, depending on which one we're closer too.
+        /// If
+        /// </summary>
+        private void testSeekSnappingInBetweenBeat()
+        {
+            reset();
+
+            AddStep("Seek(24), Snap", () => composer.SeekTo(24, true));
+            AddAssert("Time = 0", () => track.CurrentTime == 0);
+            AddStep("Seek(26), Snap", () => composer.SeekTo(26, true));
+            AddAssert("Time = 50", () => track.CurrentTime == 50);
+            AddStep("Seek(150), Snap", () => composer.SeekTo(150, true));
+            AddAssert("Time = 100", () => track.CurrentTime == 100);
+            AddStep("Seek(170), Snap", () => composer.SeekTo(170, true));
+            AddAssert("Time = 175", () => track.CurrentTime == 175);
+            AddStep("Seek(274), Snap", () => composer.SeekTo(274, true));
+            AddAssert("Time = 175", () => track.CurrentTime == 175);
+            AddStep("Seek(276), Snap", () => composer.SeekTo(276, true));
+            AddAssert("Time = 350", () => track.CurrentTime == 350);
+        }
+
+        /// <summary>
+        /// Tests that when seeking forward with no beat snapping, beats are never snapped to, nor the next timing point (if we've skipped it).
+        /// </summary>
+        private void testSeekForwardNoSnapping()
+        {
+            reset();
+
+            AddStep("SeekForward", () => composer.SeekForward());
+            AddAssert("Time = 50", () => track.CurrentTime == 50);
+            AddStep("SeekForward", () => composer.SeekForward());
+            AddAssert("Time = 100", () => track.CurrentTime == 100);
+            AddStep("SeekForward", () => composer.SeekForward());
+            AddAssert("Time = 200", () => track.CurrentTime == 200);
+            AddStep("SeekForward", () => composer.SeekForward());
+            AddAssert("Time = 400", () => track.CurrentTime == 400);
+            AddStep("SeekForward", () => composer.SeekForward());
+            AddAssert("Time = 450", () => track.CurrentTime == 450);
+        }
+
+        /// <summary>
+        /// Tests that when seeking forward with beat snapping, all beats are snapped to and timing points are never skipped.
+        /// </summary>
+        private void testSeekForwardSnappingOnBeat()
+        {
+            reset();
+
+            AddStep("SeekForward", () => composer.SeekForward(true));
+            AddAssert("Time = 50", () => track.CurrentTime == 50);
+            AddStep("SeekForward", () => composer.SeekForward(true));
+            AddAssert("Time = 100", () => track.CurrentTime == 100);
+            AddStep("SeekForward", () => composer.SeekForward(true));
+            AddAssert("Time = 175", () => track.CurrentTime == 175);
+            AddStep("SeekForward", () => composer.SeekForward(true));
+            AddAssert("Time = 350", () => track.CurrentTime == 350);
+            AddStep("SeekForward", () => composer.SeekForward(true));
+            AddAssert("Time = 400", () => track.CurrentTime == 400);
+            AddStep("SeekForward", () => composer.SeekForward(true));
+            AddAssert("Time = 450", () => track.CurrentTime == 450);
+        }
+
+        /// <summary>
+        /// Tests that when seeking forward from in-between two beats, the next beat or timing point is snapped to, and no beats are skipped.
+        /// </summary>
+        private void testSeekForwardSnappingFromInBetweenBeat()
+        {
+            reset();
+
+            AddStep("Seek(25)", () => composer.SeekTo(25));
+            AddStep("SeekForward", () => composer.SeekForward(true));
+            AddAssert("Time = 50", () => track.CurrentTime == 50);
+        }
+
+        private void reset()
+        {
+            AddStep("Reset", () => composer.SeekTo(0));
+        }
+
+        private class TestHitObjectComposer : HitObjectComposer
+        {
+            public TestHitObjectComposer(Ruleset ruleset)
+                : base(ruleset)
+            {
+            }
+
+            protected override IReadOnlyList<ICompositionTool> CompositionTools => new ICompositionTool[0];
+        }
+
+        private class TimingPointVisualiser : CompositeDrawable
+        {
+            private readonly Track track;
+
+            private readonly Drawable tracker;
+
+            public TimingPointVisualiser(Beatmap beatmap, Track track)
+            {
+                this.track = track;
+
+                Anchor = Anchor.Centre;
+                Origin = Anchor.Centre;
+
+                RelativeSizeAxes = Axes.X;
+                AutoSizeAxes = Axes.Y;
+
+                Width = 0.75f;
+
+                FillFlowContainer timelineContainer;
+
+                InternalChildren = new Drawable[]
+                {
+                    new Box
+                    {
+                        Name = "Background",
+                        RelativeSizeAxes = Axes.Both,
+                        Colour = Color4.Black.Opacity(85f)
+                    },
+                    new Container
+                    {
+                        Name = "Tracks",
+                        RelativeSizeAxes = Axes.X,
+                        AutoSizeAxes = Axes.Y,
+                        Padding = new MarginPadding(15),
+                        Children = new[]
+                        {
+                            tracker = new Box
+                            {
+                                Anchor = Anchor.CentreLeft,
+                                Origin = Anchor.Centre,
+                                RelativeSizeAxes = Axes.Y,
+                                RelativePositionAxes = Axes.X,
+                                Width = 2,
+                                Colour = Color4.Red,
+                            },
+                            timelineContainer = new FillFlowContainer
+                            {
+                                RelativeSizeAxes = Axes.X,
+                                AutoSizeAxes = Axes.Y,
+                                Spacing = new Vector2(0, 5)
+                            },
+                        }
+                    }
+                };
+
+                var timingPoints = beatmap.ControlPointInfo.TimingPoints;
+
+                for (int i = 0; i < timingPoints.Count; i++)
+                {
+                    TimingControlPoint next = i == timingPoints.Count - 1 ? null : timingPoints[i + 1];
+                    timelineContainer.Add(new TimingPointTimeline(timingPoints[i], next?.Time ?? beatmap.HitObjects.Last().StartTime, track.Length));
+                }
+            }
+
+            protected override void Update()
+            {
+                base.Update();
+
+                tracker.X = (float)(track.CurrentTime / track.Length);
+            }
+
+            private class TimingPointTimeline : CompositeDrawable
+            {
+                public TimingPointTimeline(TimingControlPoint timingPoint, double endTime, double fullDuration)
+                {
+                    RelativeSizeAxes = Axes.X;
+                    AutoSizeAxes = Axes.Y;
+
+                    Box createMainTick(double time) => new Box
+                    {
+                        Anchor = Anchor.BottomLeft,
+                        Origin = Anchor.BottomCentre,
+                        RelativePositionAxes = Axes.X,
+                        X = (float)(time / fullDuration),
+                        Height = 10,
+                        Width = 2
+                    };
+
+                    Box createBeatTick(double time) => new Box
+                    {
+                        Anchor = Anchor.BottomLeft,
+                        Origin = Anchor.BottomCentre,
+                        RelativePositionAxes = Axes.X,
+                        X = (float)(time / fullDuration),
+                        Height = 5,
+                        Width = 2,
+                        Colour = time > endTime ? Color4.Gray : Color4.Yellow
+                    };
+
+                    AddInternal(createMainTick(timingPoint.Time));
+                    AddInternal(createMainTick(endTime));
+
+                    for (double t = timingPoint.Time + timingPoint.BeatLength / 4; t < fullDuration; t += timingPoint.BeatLength / 4)
+                        AddInternal(createBeatTick(t));
+                }
+            }
+        }
+    }
+}

osu.Game.Tests/osu.Game.Tests.csproj

@@ -130,6 +130,7 @@
     <Compile Include="Visual\TestCaseEditorComposeRadioButtons.cs" />
     <Compile Include="Visual\TestCaseEditorComposeTimeline.cs" />
     <Compile Include="Visual\TestCaseEditorMenuBar.cs" />
+    <Compile Include="Visual\TestCaseEditorSeekSnapping.cs" />
     <Compile Include="Visual\TestCaseEditorSummaryTimeline.cs" />
     <Compile Include="Visual\TestCaseEditorSelectionLayer.cs" />
     <Compile Include="Visual\TestCaseGamefield.cs" />

osu.Game/Rulesets/Edit/HitObjectComposer.cs

@@ -153,13 +153,43 @@ namespace osu.Game.Rulesets.Edit
         /// Seeks the current time one beat-snapped beat-length backwards.
         /// </summary>
         /// <param name="snapped">Whether to snap to the closest beat.</param>
-        public void SeekBackward(bool snapped) => seek(-1, snapped);
+        public void SeekBackward(bool snapped = false) => seek(-1, snapped);
 
         /// <summary>
         /// Seeks the current time one beat-snapped beat-length forwards.
         /// </summary>
         /// <param name="snapped">Whether to snap to the closest beat.</param>
-        public void SeekForward(bool snapped) => seek(1, snapped);
+        public void SeekForward(bool snapped = false) => seek(1, snapped);
+
+        public void SeekTo(double seekTime, bool snapped = false)
+        {
+            // Todo: This should not be a constant, but feels good for now
+            const int beat_snap_divisor = 4;
+
+            if (!snapped)
+            {
+                adjustableClock.Seek(seekTime);
+                return;
+            }
+
+            var timingPoint = beatmap.Value.Beatmap.ControlPointInfo.TimingPointAt(seekTime);
+            double beatSnapLength = timingPoint.BeatLength / beat_snap_divisor;
+
+            // We will be snapping to beats within the timing point
+            seekTime -= timingPoint.Time;
+
+            // Determine the index from the current timing point of the closest beat to seekTime
+            int closestBeat = (int)Math.Round(seekTime / beatSnapLength);
+            seekTime = timingPoint.Time + closestBeat * beatSnapLength;
+
+            // Depending on beatSnapLength, we may snap to a beat that is beyond timingPoint's end time, but we want to instead snap to
+            // the next timing point's start time
+            var nextTimingPoint = beatmap.Value.Beatmap.ControlPointInfo.TimingPoints.FirstOrDefault(t => t.Time > timingPoint.Time);
+            if (seekTime > nextTimingPoint?.Time)
+                seekTime = nextTimingPoint.Time;
+
+            adjustableClock.Seek(seekTime);
+        }
 
         private void seek(int direction, bool snapped)
         {
@@ -181,18 +211,14 @@ namespace osu.Game.Rulesets.Edit
             var firstTimingPoint = beatmap.Value.Beatmap.ControlPointInfo.TimingPoints.First();
 
             if (currentTimingPoint != firstTimingPoint && seekTime < currentTimingPoint.Time)
-                seekTime = currentTimingPoint.Time - 1; // -1 to be in the prior timing point's boundary
+                adjustableClock.Seek(currentTimingPoint.Time - 1); // -1 to be in the prior timing point's boundary
-            else if (nextTimingPoint != null && seekTime >= nextTimingPoint.Time)
+            else if (seekTime >= nextTimingPoint?.Time)
-                seekTime = nextTimingPoint.Time + 1; // +1 to be in the next timing point's boundary
+                adjustableClock.Seek(nextTimingPoint.Time); // +1 to be in the next timing point's boundary
             else
             {
-                // We will be snapping to beats within the current timing point
+                // We will be snapping to beats within the timing point
                 seekTime -= currentTimingPoint.Time;
 
-                // When rounding below, we need to ensure that abs(seekTime - currentTime) > seekAmount
-                // This is done by adding direction - a small offset, to seekTime
-                seekTime += direction;
-
                 // Determine the index from the current timing point of the closest beat to seekTime, accounting for scrolling direction
                 int closestBeat;
                 if (direction > 0)
@@ -201,9 +227,9 @@ namespace osu.Game.Rulesets.Edit
                     closestBeat = (int)Math.Ceiling(seekTime / seekAmount);
 
                 seekTime = currentTimingPoint.Time + closestBeat * seekAmount;
-            }
 
-            adjustableClock.Seek(seekTime);
+                adjustableClock.Seek(seekTime);
+            }
         }
 
         private void setCompositionTool(ICompositionTool tool) => CurrentTool = tool;