Merge pull request #9826 from bdach/spinner-rotation-clock-rate

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

@@ -7,6 +7,7 @@ using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Audio;
+using osu.Framework.Bindables;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Testing;
 using osu.Framework.Timing;
@@ -69,11 +70,11 @@ namespace osu.Game.Rulesets.Osu.Tests
                 trackerRotationTolerance = Math.Abs(drawableSpinner.RotationTracker.Rotation * 0.1f);
             });
             AddAssert("is disc rotation not almost 0", () => !Precision.AlmostEquals(drawableSpinner.RotationTracker.Rotation, 0, 100));
-            AddAssert("is disc rotation absolute not almost 0", () => !Precision.AlmostEquals(drawableSpinner.RotationTracker.CumulativeRotation, 0, 100));
+            AddAssert("is disc rotation absolute not almost 0", () => !Precision.AlmostEquals(drawableSpinner.RotationTracker.RateAdjustedRotation, 0, 100));
 
             addSeekStep(0);
             AddAssert("is disc rotation almost 0", () => Precision.AlmostEquals(drawableSpinner.RotationTracker.Rotation, 0, trackerRotationTolerance));
-            AddAssert("is disc rotation absolute almost 0", () => Precision.AlmostEquals(drawableSpinner.RotationTracker.CumulativeRotation, 0, 100));
+            AddAssert("is disc rotation absolute almost 0", () => Precision.AlmostEquals(drawableSpinner.RotationTracker.RateAdjustedRotation, 0, 100));
         }
 
         [Test]
@@ -94,7 +95,7 @@ namespace osu.Game.Rulesets.Osu.Tests
                 finalSpinnerSymbolRotation = spinnerSymbol.Rotation;
                 spinnerSymbolRotationTolerance = Math.Abs(finalSpinnerSymbolRotation * 0.05f);
             });
-            AddStep("retrieve cumulative disc rotation", () => finalCumulativeTrackerRotation = drawableSpinner.RotationTracker.CumulativeRotation);
+            AddStep("retrieve cumulative disc rotation", () => finalCumulativeTrackerRotation = drawableSpinner.RotationTracker.RateAdjustedRotation);
 
             addSeekStep(2500);
             AddAssert("disc rotation rewound",
@@ -106,7 +107,7 @@ namespace osu.Game.Rulesets.Osu.Tests
                 () => Precision.AlmostEquals(spinnerSymbol.Rotation, finalSpinnerSymbolRotation / 2, spinnerSymbolRotationTolerance));
             AddAssert("is cumulative rotation rewound",
                 // cumulative rotation is not damped, so we're treating it as the "ground truth" and allowing a comparatively smaller margin of error.
-                () => Precision.AlmostEquals(drawableSpinner.RotationTracker.CumulativeRotation, finalCumulativeTrackerRotation / 2, 100));
+                () => Precision.AlmostEquals(drawableSpinner.RotationTracker.RateAdjustedRotation, finalCumulativeTrackerRotation / 2, 100));
 
             addSeekStep(5000);
             AddAssert("is disc rotation almost same",
@@ -114,26 +115,14 @@ namespace osu.Game.Rulesets.Osu.Tests
             AddAssert("is symbol rotation almost same",
                 () => Precision.AlmostEquals(spinnerSymbol.Rotation, finalSpinnerSymbolRotation, spinnerSymbolRotationTolerance));
             AddAssert("is cumulative rotation almost same",
-                () => Precision.AlmostEquals(drawableSpinner.RotationTracker.CumulativeRotation, finalCumulativeTrackerRotation, 100));
+                () => Precision.AlmostEquals(drawableSpinner.RotationTracker.RateAdjustedRotation, finalCumulativeTrackerRotation, 100));
         }
 
         [Test]
         public void TestRotationDirection([Values(true, false)] bool clockwise)
         {
             if (clockwise)
-            {
+                transformReplay(flip);
-                AddStep("flip replay", () =>
-                {
-                    var drawableRuleset = this.ChildrenOfType<DrawableOsuRuleset>().Single();
-                    var score = drawableRuleset.ReplayScore;
-                    var scoreWithFlippedReplay = new Score
-                    {
-                        ScoreInfo = score.ScoreInfo,
-                        Replay = flipReplay(score.Replay)
-                    };
-                    drawableRuleset.SetReplayScore(scoreWithFlippedReplay);
-                });
-            }
 
             addSeekStep(5000);
 
@@ -141,7 +130,7 @@ namespace osu.Game.Rulesets.Osu.Tests
             AddAssert("spinner symbol direction correct", () => clockwise ? spinnerSymbol.Rotation > 0 : spinnerSymbol.Rotation < 0);
         }
 
-        private Replay flipReplay(Replay scoreReplay) => new Replay
+        private Replay flip(Replay scoreReplay) => new Replay
         {
             Frames = scoreReplay
                      .Frames
@@ -164,7 +153,7 @@ namespace osu.Game.Rulesets.Osu.Tests
             {
                 // multipled by 2 to nullify the score multiplier. (autoplay mod selected)
                 var totalScore = ((ScoreExposedPlayer)Player).ScoreProcessor.TotalScore.Value * 2;
-                return totalScore == (int)(drawableSpinner.RotationTracker.CumulativeRotation / 360) * SpinnerTick.SCORE_PER_TICK;
+                return totalScore == (int)(drawableSpinner.RotationTracker.RateAdjustedRotation / 360) * SpinnerTick.SCORE_PER_TICK;
             });
 
             addSeekStep(0);
@@ -196,6 +185,49 @@ namespace osu.Game.Rulesets.Osu.Tests
             AddAssert("spm still valid", () => Precision.AlmostEquals(drawableSpinner.SpmCounter.SpinsPerMinute, estimatedSpm, 1.0));
         }
 
+        [TestCase(0.5)]
+        [TestCase(2.0)]
+        public void TestSpinUnaffectedByClockRate(double rate)
+        {
+            double expectedProgress = 0;
+            double expectedSpm = 0;
+
+            addSeekStep(1000);
+            AddStep("retrieve spinner state", () =>
+            {
+                expectedProgress = drawableSpinner.Progress;
+                expectedSpm = drawableSpinner.SpmCounter.SpinsPerMinute;
+            });
+
+            addSeekStep(0);
+
+            AddStep("adjust track rate", () => track.AddAdjustment(AdjustableProperty.Tempo, new BindableDouble(rate)));
+            // autoplay replay frames use track time;
+            // if a spin takes 1000ms in track time and we're playing with a 2x rate adjustment, the spin will take 500ms of *real* time.
+            // therefore we need to apply the rate adjustment to the replay itself to change from track time to real time,
+            // as real time is what we care about for spinners
+            // (so we're making the spin take 1000ms in real time *always*, regardless of the track clock's rate).
+            transformReplay(replay => applyRateAdjustment(replay, rate));
+
+            addSeekStep(1000);
+            AddAssert("progress almost same", () => Precision.AlmostEquals(expectedProgress, drawableSpinner.Progress, 0.05));
+            AddAssert("spm almost same", () => Precision.AlmostEquals(expectedSpm, drawableSpinner.SpmCounter.SpinsPerMinute, 2.0));
+        }
+
+        private Replay applyRateAdjustment(Replay scoreReplay, double rate) => new Replay
+        {
+            Frames = scoreReplay
+                     .Frames
+                     .Cast<OsuReplayFrame>()
+                     .Select(replayFrame =>
+                     {
+                         var adjustedTime = replayFrame.Time * rate;
+                         return new OsuReplayFrame(adjustedTime, replayFrame.Position, replayFrame.Actions.ToArray());
+                     })
+                     .Cast<ReplayFrame>()
+                     .ToList()
+        };
+
         private void addSeekStep(double time)
         {
             AddStep($"seek to {time}", () => track.Seek(time));
@@ -203,6 +235,18 @@ namespace osu.Game.Rulesets.Osu.Tests
             AddUntilStep("wait for seek to finish", () => Precision.AlmostEquals(time, Player.DrawableRuleset.FrameStableClock.CurrentTime, 100));
         }
 
+        private void transformReplay(Func<Replay, Replay> replayTransformation) => AddStep("set replay", () =>
+        {
+            var drawableRuleset = this.ChildrenOfType<DrawableOsuRuleset>().Single();
+            var score = drawableRuleset.ReplayScore;
+            var transformedScore = new Score
+            {
+                ScoreInfo = score.ScoreInfo,
+                Replay = replayTransformation.Invoke(score.Replay)
+            };
+            drawableRuleset.SetReplayScore(transformedScore);
+        });
+
         protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new Beatmap
         {
             HitObjects = new List<HitObject>

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs

@@ -185,7 +185,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
                     // these become implicitly hit.
                     return 1;
 
-                return Math.Clamp(RotationTracker.CumulativeRotation / 360 / Spinner.SpinsRequired, 0, 1);
+                return Math.Clamp(RotationTracker.RateAdjustedRotation / 360 / Spinner.SpinsRequired, 0, 1);
             }
         }
 
@@ -233,7 +233,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
 
             if (!SpmCounter.IsPresent && RotationTracker.Tracking)
                 SpmCounter.FadeIn(HitObject.TimeFadeIn);
-            SpmCounter.SetRotation(RotationTracker.CumulativeRotation);
+            SpmCounter.SetRotation(RotationTracker.RateAdjustedRotation);
 
             updateBonusScore();
         }
@@ -245,7 +245,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
             if (ticks.Count == 0)
                 return;
 
-            int spins = (int)(RotationTracker.CumulativeRotation / 360);
+            int spins = (int)(RotationTracker.RateAdjustedRotation / 360);
 
             if (spins < wholeSpins)
             {

osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/DefaultSpinnerDisc.cs

@@ -177,7 +177,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Pieces
         {
             get
             {
-                int rotations = (int)(drawableSpinner.RotationTracker.CumulativeRotation / 360);
+                int rotations = (int)(drawableSpinner.RotationTracker.RateAdjustedRotation / 360);
 
                 if (wholeRotationCount == rotations) return false;
 

osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/SpinnerRotationTracker.cs

@@ -31,17 +31,28 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Pieces
         public readonly BindableBool Complete = new BindableBool();
 
         /// <summary>
-        /// The total rotation performed on the spinner disc, disregarding the spin direction.
+        /// The total rotation performed on the spinner disc, disregarding the spin direction,
+        /// adjusted for the track's playback rate.
         /// </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>
-        /// If the spinner is spun 360 degrees clockwise and then 360 degrees counter-clockwise,
+        /// 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 for <see cref="Drawable.Rotation"/>).
+        /// 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 CumulativeRotation { get; private set; }
+        public float RateAdjustedRotation { get; private set; }
 
         /// <summary>
         /// Whether the spinning is spinning at a reasonable speed to be considered visually spinning.
@@ -113,7 +124,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Pieces
             }
 
             currentRotation += angle;
-            CumulativeRotation += Math.Abs(angle) * Math.Sign(Clock.ElapsedFrameTime);
+            // rate has to be applied each frame, because it's not guaranteed to be constant throughout playback
+            // (see: ModTimeRamp)
+            RateAdjustedRotation += (float)(Math.Abs(angle) * Clock.Rate);
         }
     }
 }