Fix spinner cheese by accounting for spin directionality

osu.Game.Rulesets.Osu.Tests/SpinnerSpinHistoryTest.cs

@@ -0,0 +1,148 @@
+// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using NUnit.Framework;
+using osu.Game.Rulesets.Osu.Objects.Drawables;
+
+namespace osu.Game.Rulesets.Osu.Tests
+{
+    [TestFixture]
+    public class SpinnerSpinHistoryTest
+    {
+        private SpinnerSpinHistory history = null!;
+
+        [SetUp]
+        public void Setup()
+        {
+            history = new SpinnerSpinHistory();
+        }
+
+        [TestCase(0, 0)]
+        [TestCase(10, 10)]
+        [TestCase(180, 180)]
+        [TestCase(350, 350)]
+        [TestCase(360, 360)]
+        [TestCase(370, 370)]
+        [TestCase(540, 540)]
+        [TestCase(720, 720)]
+        // ---
+        [TestCase(-0, 0)]
+        [TestCase(-10, 10)]
+        [TestCase(-180, 180)]
+        [TestCase(-350, 350)]
+        [TestCase(-360, 360)]
+        [TestCase(-370, 370)]
+        [TestCase(-540, 540)]
+        [TestCase(-720, 720)]
+        public void TestSpinOneDirection(float spin, float expectedRotation)
+        {
+            history.ReportDelta(500, spin);
+            Assert.That(history.TotalRotation, Is.EqualTo(expectedRotation));
+        }
+
+        [TestCase(0, 0, 0, 0)]
+        // ---
+        [TestCase(10, -10, 0, 10)]
+        [TestCase(-10, 10, 0, 10)]
+        // ---
+        [TestCase(10, -20, 0, 10)]
+        [TestCase(-10, 20, 0, 10)]
+        // ---
+        [TestCase(20, -10, 0, 20)]
+        [TestCase(-20, 10, 0, 20)]
+        // ---
+        [TestCase(10, -360, 0, 350)]
+        [TestCase(-10, 360, 0, 350)]
+        // ---
+        [TestCase(360, -10, 0, 370)]
+        [TestCase(360, 10, 0, 370)]
+        [TestCase(-360, 10, 0, 370)]
+        [TestCase(-360, -10, 0, 370)]
+        // ---
+        [TestCase(10, 10, 10, 30)]
+        [TestCase(10, 10, -10, 20)]
+        [TestCase(10, -10, 10, 10)]
+        [TestCase(-10, -10, -10, 30)]
+        [TestCase(-10, -10, 10, 20)]
+        [TestCase(-10, 10, 10, 10)]
+        // ---
+        [TestCase(10, -20, -350, 360)]
+        [TestCase(10, -20, 350, 340)]
+        [TestCase(-10, 20, 350, 360)]
+        [TestCase(-10, 20, -350, 340)]
+        public void TestSpinMultipleDirections(float spin1, float spin2, float spin3, float expectedRotation)
+        {
+            history.ReportDelta(500, spin1);
+            history.ReportDelta(1000, spin2);
+            history.ReportDelta(1500, spin3);
+            Assert.That(history.TotalRotation, Is.EqualTo(expectedRotation));
+        }
+
+        // One spin
+        [TestCase(370, -50, 320)]
+        [TestCase(-370, 50, 320)]
+        // Two spins
+        [TestCase(740, -420, 320)]
+        [TestCase(-740, 420, 320)]
+        public void TestRemoveAndCrossFullSpin(float deltaToAdd, float deltaToRemove, float expectedRotation)
+        {
+            history.ReportDelta(1000, deltaToAdd);
+            history.ReportDelta(500, deltaToRemove);
+            Assert.That(history.TotalRotation, Is.EqualTo(expectedRotation));
+        }
+
+        // One spin + partial
+        [TestCase(400, -30, -50, 320)]
+        [TestCase(-400, 30, 50, 320)]
+        // Two spins + partial
+        [TestCase(800, -430, -50, 320)]
+        [TestCase(-800, 430, 50, 320)]
+        public void TestRemoveAndCrossFullAndPartialSpins(float deltaToAdd1, float deltaToAdd2, float deltaToRemove, float expectedRotation)
+        {
+            history.ReportDelta(1000, deltaToAdd1);
+            history.ReportDelta(1500, deltaToAdd2);
+            history.ReportDelta(500, deltaToRemove);
+            Assert.That(history.TotalRotation, Is.EqualTo(expectedRotation));
+        }
+
+        [Test]
+        public void TestRewindMultipleFullSpins()
+        {
+            history.ReportDelta(500, 360);
+            history.ReportDelta(1000, 720);
+
+            Assert.That(history.TotalRotation, Is.EqualTo(1080));
+
+            history.ReportDelta(250, -900);
+
+            Assert.That(history.TotalRotation, Is.EqualTo(180));
+        }
+
+        [Test]
+        public void TestRewindIntoSegmentThatHasNotCrossedZero()
+        {
+            history.ReportDelta(1000, -180);
+            Assert.That(history.TotalRotation, Is.EqualTo(180));
+            history.ReportDelta(1500, 90);
+            Assert.That(history.TotalRotation, Is.EqualTo(180));
+            history.ReportDelta(2000, 450);
+            Assert.That(history.TotalRotation, Is.EqualTo(360));
+            history.ReportDelta(1750, -45);
+            Assert.That(history.TotalRotation, Is.EqualTo(315));
+        }
+
+        [Test]
+        public void TestRewindOverDirectionChange()
+        {
+            history.ReportDelta(1000, 40); // max is now CW 40 degrees
+            Assert.That(history.TotalRotation, Is.EqualTo(40));
+            history.ReportDelta(1100, -90); // max is now CCW 50 degrees
+            Assert.That(history.TotalRotation, Is.EqualTo(50));
+            history.ReportDelta(1200, 110); // max is now CW 60 degrees
+            Assert.That(history.TotalRotation, Is.EqualTo(60));
+
+            history.ReportDelta(1000, -20);
+            Assert.That(history.TotalRotation, Is.EqualTo(40));
+        }
+    }
+}

osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerInput.cs

@@ -53,7 +53,6 @@ public void Setup() => Schedule(() =>
         /// While off-centre, vibrates backwards and forwards on the x-axis, from centre-50 to centre+50, every 50ms.
         /// </summary>
         [Test]
-        [Ignore("An upcoming implementation will fix this case")]
         public void TestVibrateWithoutSpinningOffCentre()
         {
             List<ReplayFrame> frames = new List<ReplayFrame>();
@@ -81,7 +80,6 @@ public void TestVibrateWithoutSpinningOffCentre()
         /// While centred on the slider, vibrates backwards and forwards on the x-axis, from centre-50 to centre+50, every 50ms.
         /// </summary>
         [Test]
-        [Ignore("An upcoming implementation will fix this case")]
         public void TestVibrateWithoutSpinningOnCentre()
         {
             List<ReplayFrame> frames = new List<ReplayFrame>();
@@ -130,7 +128,6 @@ public void TestSpinSingleDirection(float amount, int expectedTicks)
         /// No ticks should be hit since the total rotation is -0.5 (0.5 CW + 1 CCW = 0.5 CCW).
         /// </summary>
         [Test]
-        [Ignore("An upcoming implementation will fix this case")]
         public void TestSpinHalfBothDirections()
         {
             performTest(new SpinFramesGenerator(time_spinner_start)
@@ -149,7 +146,6 @@ public void TestSpinHalfBothDirections()
         [TestCase(-180, 540, 1)]
         [TestCase(180, -900, 2)]
         [TestCase(-180, 900, 2)]
-        [Ignore("An upcoming implementation will fix this case")]
         public void TestSpinOneDirectionThenChangeDirection(float direction1, float direction2, int expectedTicks)
         {
             performTest(new SpinFramesGenerator(time_spinner_start)
@@ -162,18 +158,24 @@ public void TestSpinOneDirectionThenChangeDirection(float direction1, float dire
         }
 
         [Test]
-        [Ignore("An upcoming implementation will fix this case")]
         public void TestRewind()
         {
             AddStep("set manual clock", () => manualClock = new ManualClock { Rate = 1 });
 
-            List<ReplayFrame> frames = new SpinFramesGenerator(time_spinner_start)
-                                       .Spin(360, 500) // 2000ms -> 1 full CW spin
-                                       .Spin(-180, 500) // 2500ms -> 0.5 CCW spins
-                                       .Spin(90, 500) // 3000ms -> 0.25 CW spins
-                                       .Spin(450, 500) // 3500ms -> 1 full CW spin
-                                       .Spin(180, 500) // 4000ms -> 0.5 CW spins
-                                       .Build();
+            List<ReplayFrame> frames =
+                new SpinFramesGenerator(time_spinner_start)
+                    // 1500ms start
+                    .Spin(360, 500)
+                    // 2000ms -> 1 full CW spin
+                    .Spin(-180, 500)
+                    // 2500ms -> 1 full CW spin + 0.5 CCW spins
+                    .Spin(90, 500)
+                    // 3000ms -> 1 full CW spin + 0.25 CCW spins
+                    .Spin(450, 500)
+                    // 3500ms -> 2 full CW spins
+                    .Spin(180, 500)
+                    // 4000ms -> 2 full CW spins + 0.5 CW spins
+                    .Build();
 
             loadPlayer(frames);
 
@@ -194,11 +196,11 @@ public void TestRewind()
             assertTotalRotation(3750, 810);
             assertTotalRotation(3500, 720);
             assertTotalRotation(3250, 530);
-            assertTotalRotation(3000, 540);
+            assertTotalRotation(3000, 450);
             assertTotalRotation(2750, 540);
             assertTotalRotation(2500, 540);
-            assertTotalRotation(2250, 360);
-            assertTotalRotation(2000, 180);
+            assertTotalRotation(2250, 450);
+            assertTotalRotation(2000, 360);
             assertTotalRotation(1500, 0);
 
             void assertTotalRotation(double time, float expected)
@@ -211,7 +213,8 @@ void assertTotalRotation(double time, float expected)
             void addSeekStep(double time)
             {
                 AddStep($"seek to {time}", () => clock.Seek(time));
-                AddUntilStep("wait for seek to finish", () => drawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(time));
+                // Lenience is required due to interpolation running slightly ahead on a stalled clock.
+                AddUntilStep("wait for seek to finish", () => drawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(time).Within(40));
             }
         }
 

osu.Game.Rulesets.Osu/Judgements/OsuSpinnerJudgementResult.cs

@@ -4,6 +4,7 @@
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Rulesets.Osu.Objects.Drawables;
 
 namespace osu.Game.Rulesets.Osu.Judgements
 {
@@ -15,28 +16,15 @@ public class OsuSpinnerJudgementResult : OsuJudgementResult
         public Spinner Spinner => (Spinner)HitObject;
 
         /// <summary>
-        /// The total rotation performed on the spinner disc, disregarding the spin direction,
-        /// adjusted for the track's playback rate.
+        /// The total amount that the spinner was rotated.
         /// </summary>
-        /// <remarks>
-        /// <para>
-        /// This value is always non-negative and is monotonically increasing with time
-        /// (i.e. will only increase if time is passing forward, but can decrease during rewind).
-        /// </para>
-        /// <para>
-        /// The rotation from each frame is multiplied by the clock's current playback rate.
-        /// The reason this is done is to ensure that spinners give the same score and require the same number of spins
-        /// regardless of whether speed-modifying mods are applied.
-        /// </para>
-        /// </remarks>
-        /// <example>
-        /// Assuming no speed-modifying mods are active,
-        /// if the spinner is spun 360 degrees clockwise and then 360 degrees counter-clockwise,
-        /// this property will return the value of 720 (as opposed to 0).
-        /// If Double Time is active instead (with a speed multiplier of 1.5x),
-        /// in the same scenario the property will return 720 * 1.5 = 1080.
-        /// </example>
-        public float TotalRotation;
+        public float TotalRotation => History.TotalRotation;
+
+        /// <summary>
+        /// Stores the spinning history of the spinner.<br />
+        /// Instants of movement deltas may be added or removed from this in order to calculate the total rotation for the spinner.
+        /// </summary>
+        public readonly SpinnerSpinHistory History = new SpinnerSpinHistory();
 
         /// <summary>
         /// Time instant at which the spin was started (the first user input which caused an increase in spin).