From 50f9d810dd49ed45dfdfea70db321430f981d3d2 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 15 Mar 2018 15:40:52 +0900 Subject: [PATCH] Add more testcases + fix up seeking a bit more --- .../Visual/TestCaseEditorSeekSnapping.cs | 146 +++++++++++++++--- osu.Game/Rulesets/Edit/HitObjectComposer.cs | 58 ++++--- 2 files changed, 163 insertions(+), 41 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCaseEditorSeekSnapping.cs b/osu.Game.Tests/Visual/TestCaseEditorSeekSnapping.cs index 4757e6c857..2d60c6848d 100644 --- a/osu.Game.Tests/Visual/TestCaseEditorSeekSnapping.cs +++ b/osu.Game.Tests/Visual/TestCaseEditorSeekSnapping.cs @@ -43,7 +43,8 @@ namespace osu.Game.Tests.Visual new TimingControlPoint { Time = 100, BeatLength = 400 }, new TimingControlPoint { Time = 175, BeatLength = 800 }, new TimingControlPoint { Time = 350, BeatLength = 200 }, - new TimingControlPoint { Time = 450, BeatLength = 100 } + new TimingControlPoint { Time = 450, BeatLength = 100 }, + new TimingControlPoint { Time = 500, BeatLength = 307.69230769230802 } } }, HitObjects = @@ -71,12 +72,15 @@ namespace osu.Game.Tests.Visual } }; -// testSeekNoSnapping(); -// testSeekSnappingOnBeat(); -// testSeekSnappingInBetweenBeat(); -// testSeekForwardNoSnapping(); -// testSeekForwardSnappingOnBeat(); + testSeekNoSnapping(); + testSeekSnappingOnBeat(); + testSeekSnappingInBetweenBeat(); + testSeekForwardNoSnapping(); + testSeekForwardSnappingOnBeat(); testSeekForwardSnappingFromInBetweenBeat(); + testSeekBackwardSnappingOnBeat(); + testSeekBackwardSnappingFromInBetweenBeat(); + testSeekingWithFloatingPointBeatLength(); } /// @@ -149,7 +153,7 @@ namespace osu.Game.Tests.Visual } /// - /// Tests that when seeking forward with no beat snapping, beats are never snapped to, nor the next timing point (if we've skipped it). + /// Tests that when seeking forward with no beat snapping, beats are never explicitly snapped to, nor the next timing point (if we've skipped it). /// private void testSeekForwardNoSnapping() { @@ -174,17 +178,17 @@ namespace osu.Game.Tests.Visual { reset(); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 50", () => track.CurrentTime == 50); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 100", () => track.CurrentTime == 100); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 175", () => track.CurrentTime == 175); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 350", () => track.CurrentTime == 350); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 400", () => track.CurrentTime == 400); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 450", () => track.CurrentTime == 450); } @@ -197,31 +201,131 @@ namespace osu.Game.Tests.Visual reset(); AddStep("Seek(49)", () => composer.SeekTo(49)); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 50", () => track.CurrentTime == 50); AddStep("Seek(49.999)", () => composer.SeekTo(49.999)); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 50", () => track.CurrentTime == 50); AddStep("Seek(99)", () => composer.SeekTo(99)); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 100", () => track.CurrentTime == 100); AddStep("Seek(99.999)", () => composer.SeekTo(99.999)); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 100", () => track.CurrentTime == 100); AddStep("Seek(174)", () => composer.SeekTo(174)); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 175", () => track.CurrentTime == 175); AddStep("Seek(349)", () => composer.SeekTo(349)); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 350", () => track.CurrentTime == 350); AddStep("Seek(399)", () => composer.SeekTo(399)); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 400", () => track.CurrentTime == 400); AddStep("Seek(449)", () => composer.SeekTo(449)); - AddStep("SeekForward", () => composer.SeekForward(true)); + AddStep("SeekForward, Snap", () => composer.SeekForward(true)); AddAssert("Time = 450", () => track.CurrentTime == 450); } + /// + /// Tests that when seeking backward with no beat snapping, beats are never explicitly snapped to, nor the next timing point (if we've skipped it). + /// + private void testSeekBackwardNoSnapping() + { + reset(); + + AddStep("Seek(450)", () => composer.SeekTo(450)); + AddStep("SeekBackward", () => composer.SeekBackward()); + AddAssert("Time = 425", () => track.CurrentTime == 425); + AddStep("SeekBackward", () => composer.SeekBackward()); + AddAssert("Time = 375", () => track.CurrentTime == 375); + AddStep("SeekBackward", () => composer.SeekBackward()); + AddAssert("Time = 325", () => track.CurrentTime == 325); + AddStep("SeekBackward", () => composer.SeekBackward()); + AddAssert("Time = 125", () => track.CurrentTime == 125); + AddStep("SeekBackward", () => composer.SeekBackward()); + AddAssert("Time = 25", () => track.CurrentTime == 25); + AddStep("SeekBackward", () => composer.SeekBackward()); + AddAssert("Time = 0", () => track.CurrentTime == 0); + } + + /// + /// Tests that when seeking backward with beat snapping, all beats are snapped to and timing points are never skipped. + /// + private void testSeekBackwardSnappingOnBeat() + { + reset(); + + AddStep("Seek(450)", () => composer.SeekTo(450)); + AddStep("SeekBackward, Snap", () => composer.SeekBackward(true)); + AddAssert("Time = 400", () => track.CurrentTime == 400); + AddStep("SeekBackward, Snap", () => composer.SeekBackward(true)); + AddAssert("Time = 350", () => track.CurrentTime == 350); + AddStep("SeekBackward, Snap", () => composer.SeekBackward(true)); + AddAssert("Time = 175", () => track.CurrentTime == 175); + AddStep("SeekBackward, Snap", () => composer.SeekBackward(true)); + AddAssert("Time = 100", () => track.CurrentTime == 100); + AddStep("SeekBackward, Snap", () => composer.SeekBackward(true)); + AddAssert("Time = 50", () => track.CurrentTime == 50); + AddStep("SeekBackward, Snap", () => composer.SeekBackward(true)); + AddAssert("Time = 0", () => track.CurrentTime == 0); + } + + /// + /// Tests that when seeking backward from in-between two beats, the previous beat or timing point is snapped to, and no beats are skipped. + /// This will also test being extremely close to the previous beat/timing point, to ensure rounding is not an issue. + /// + private void testSeekBackwardSnappingFromInBetweenBeat() + { + reset(); + + AddStep("Seek(451)", () => composer.SeekTo(451)); + AddStep("SeekBackward, Snap", () => composer.SeekBackward(true)); + AddAssert("Time = 450", () => track.CurrentTime == 450); + AddStep("Seek(450.999)", () => composer.SeekTo(450.999)); + AddStep("SeekBackward, Snap", () => composer.SeekBackward(true)); + AddAssert("Time = 450", () => track.CurrentTime == 450); + AddStep("Seek(401)", () => composer.SeekTo(401)); + AddStep("SeekBackward, Snap", () => composer.SeekBackward(true)); + AddAssert("Time = 400", () => track.CurrentTime == 400); + AddStep("Seek(401.999)", () => composer.SeekTo(401.999)); + AddStep("SeekBackward, Snap", () => composer.SeekBackward(true)); + AddAssert("Time = 400", () => track.CurrentTime == 400); + } + + /// + /// Tests that there are no rounding issues when snapping to beats within a timing point with a floating-point beatlength. + /// + private void testSeekingWithFloatingPointBeatLength() + { + reset(); + + double lastTime = 0; + + AddStep("Seek(0)", () => composer.SeekTo(0)); + + for (int i = 0; i < 20; i++) + { + AddStep("SeekForward, Snap", () => + { + lastTime = track.CurrentTime; + composer.SeekForward(true); + }); + AddAssert("Time > lastTime", () => track.CurrentTime > lastTime); + } + + for (int i = 0; i < 20; i++) + { + AddStep("SeekBackward, Snap", () => + { + lastTime = track.CurrentTime; + composer.SeekBackward(true); + }); + AddAssert("Time < lastTime", () => track.CurrentTime < lastTime); + } + + AddAssert("Time = 0", () => track.CurrentTime == 0); + } + private void reset() { AddStep("Reset", () => composer.SeekTo(0)); diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index 952b553835..94851e8ef5 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -10,6 +10,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Input; using osu.Framework.Logging; +using osu.Framework.MathUtils; using osu.Framework.Timing; using osu.Game.Beatmaps; using osu.Game.Rulesets.Edit.Layers; @@ -196,40 +197,57 @@ namespace osu.Game.Rulesets.Edit // Todo: This should not be a constant, but feels good for now const int beat_snap_divisor = 4; - var currentTimingPoint = beatmap.Value.Beatmap.ControlPointInfo.TimingPointAt(adjustableClock.CurrentTime); - double seekAmount = currentTimingPoint.BeatLength / beat_snap_divisor; + var cpi = beatmap.Value.Beatmap.ControlPointInfo; + var timingPoint = cpi.TimingPointAt(adjustableClock.CurrentTime); + if (direction < 0 && timingPoint.Time == adjustableClock.CurrentTime) + { + // When going backwards, we care about the timing point that was _previously_ active at the current time + int activeIndex = cpi.TimingPoints.IndexOf(timingPoint); + while (activeIndex > 0 && adjustableClock.CurrentTime == timingPoint.Time) + timingPoint = cpi.TimingPoints[--activeIndex]; + } + + double seekAmount = timingPoint.BeatLength / beat_snap_divisor; double seekTime = adjustableClock.CurrentTime + seekAmount * direction; - if (!snapped) + if (!snapped || cpi.TimingPoints.Count == 0) { adjustableClock.Seek(seekTime); return; } - var nextTimingPoint = beatmap.Value.Beatmap.ControlPointInfo.TimingPoints.FirstOrDefault(t => t.Time > currentTimingPoint.Time); - var firstTimingPoint = beatmap.Value.Beatmap.ControlPointInfo.TimingPoints.First(); + // We will be snapping to beats within timingPoint + seekTime -= timingPoint.Time; - if (currentTimingPoint != firstTimingPoint && seekTime < currentTimingPoint.Time) - adjustableClock.Seek(currentTimingPoint.Time - 1); // -1 to be in the prior timing point's boundary - else if (seekTime >= nextTimingPoint?.Time) - adjustableClock.Seek(nextTimingPoint.Time); // +1 to be in the next timing point's boundary + // Determine the index from timingPoint of the closest beat to seekTime, accounting for scrolling direction + int closestBeat; + if (direction > 0) + closestBeat = (int)Math.Floor(seekTime / seekAmount); else + closestBeat = (int)Math.Ceiling(seekTime / seekAmount); + + seekTime = timingPoint.Time + closestBeat * seekAmount; + + // Due to the rounding above, we may end up on the same beat. This will effectively cause 0 seeking to happen + // so we can just seek to the next beat in the direction if this is the case + if (Math.Abs(adjustableClock.CurrentTime - seekTime) == 0) { - // We will be snapping to beats within the timing point - seekTime -= currentTimingPoint.Time; - - // Determine the index from the current timing point of the closest beat to seekTime, accounting for scrolling direction - int closestBeat; if (direction > 0) - closestBeat = (int)Math.Floor(seekTime / seekAmount); + closestBeat++; else - closestBeat = (int)Math.Ceiling(seekTime / seekAmount); - - seekTime = currentTimingPoint.Time + closestBeat * seekAmount; - - adjustableClock.Seek(seekTime); + closestBeat--; + seekTime = timingPoint.Time + closestBeat * seekAmount; } + + if (seekTime < timingPoint.Time) + seekTime = timingPoint.Time; + + var nextTimingPoint = cpi.TimingPoints.FirstOrDefault(t => t.Time > timingPoint.Time); + if (seekTime >= nextTimingPoint?.Time) + seekTime = nextTimingPoint.Time; + + adjustableClock.Seek(seekTime); } private void setCompositionTool(ICompositionTool tool) => CurrentTool = tool;