Clean up code and add xmldoc and inline doc

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

@@ -26,33 +26,33 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
         /// <remarks>
         /// This is the final scoring value.
         /// </remarks>
-        public float TotalRotation => 360 * segments.Count + currentMaxRotation;
+        public float TotalRotation => 360 * completedSpins.Count + currentSpinMaxRotation;
 
-        /// <summary>
-        /// The list of all segments where either:
-        /// <list type="bullet">
-        /// <item>The spinning direction was changed.</item>
-        /// <item>A full spin of 360 degrees was performed in either direction.</item>
-        /// </list>
-        /// </summary>
-        private readonly Stack<SpinSegment> segments = new Stack<SpinSegment>();
+        private readonly Stack<CompletedSpin> completedSpins = new Stack<CompletedSpin>();
 
         /// <summary>
         /// The total accumulated rotation.
         /// </summary>
-        private float currentAbsoluteRotation;
+        private float totalAbsoluteRotation;
 
-        private float lastCompletionAbsoluteRotation;
+        private float totalAbsoluteRotationsAtLastCompletion;
 
         /// <summary>
-        /// For the current spin, represents the maximum rotation (from 0..360) achieved by the user.
+        /// For the current spin, represents the maximum absolute rotation (from 0..360) achieved by the user.
         /// </summary>
-        private float currentMaxRotation;
+        /// <remarks>
+        /// This is used to report <see cref="TotalRotation"/> in the case a user spins backwards.
+        /// Basically it allows us to not reduce the total rotation in such a case.
+        ///
+        /// This also stops spinner "cheese" where a user may rapidly change directions and cause an increase
+        /// in rotations.
+        /// </remarks>
+        private float currentSpinMaxRotation;
 
         /// <summary>
         /// The current spin, from -360..360.
         /// </summary>
-        private float currentRotation => currentAbsoluteRotation - lastCompletionAbsoluteRotation;
+        private float currentSpinRotation => totalAbsoluteRotation - totalAbsoluteRotationsAtLastCompletion;
 
         private double lastReportTime = double.NegativeInfinity;
 
@@ -63,74 +63,82 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
         /// <param name="delta">The delta of the angle moved through since the last report.</param>
         public void ReportDelta(double currentTime, float delta)
         {
-            // TODO: Debug.Assert(Math.Abs(delta) < 180);
-            // This will require important frame guarantees.
+            if (delta == 0)
+                return;
 
-            currentAbsoluteRotation += delta;
+            // Importantly, outside of tests the max delta entering here is 180 degrees.
+            // If it wasn't for tests, we could add this line:
+            //
+            // Debug.Assert(Math.Abs(delta) < 180);
+            //
+            // For this to be 101% correct, we need to add the ability for important frames to be
+            // created based on gameplay intrinsics (ie. there should be one frame for any spinner delta 90 < n < 180 degrees).
+            //
+            // But this can come later.
+
+            totalAbsoluteRotation += delta;
 
             if (currentTime >= lastReportTime)
-                addDelta(currentTime, delta);
+            {
+                currentSpinMaxRotation = Math.Max(currentSpinMaxRotation, Math.Abs(currentSpinRotation));
+
+                // Handle the case where the user has completed another spin.
+                // Note that this does could be an `if` rather than `while` if the above assertion held true.
+                // It is a `while` loop to handle tests which throw larger values at this method.
+                while (currentSpinMaxRotation >= 360)
+                {
+                    int direction = Math.Sign(currentSpinRotation);
+
+                    completedSpins.Push(new CompletedSpin(currentTime, direction));
+
+                    // Incrementing the last completion point will cause `currentSpinRotation` to
+                    // hold the remaining spin that needs to be considered.
+                    totalAbsoluteRotationsAtLastCompletion += direction * 360;
+
+                    // Reset the current max as we are entering a new spin.
+                    // Importantly, carry over the remainder (which is now stored in `currentSpinRotation`).
+                    currentSpinMaxRotation = Math.Abs(currentSpinRotation);
+                }
+            }
             else
-                rewindDelta(currentTime, delta);
+            {
+                // When rewinding, the main thing we care about is getting `totalAbsoluteRotationsAtLastCompletion`
+                // to the correct value. We can used the stored history for this.
+                while (completedSpins.TryPeek(out var segment) && segment.CompletionTime > currentTime)
+                {
+                    completedSpins.Pop();
+                    totalAbsoluteRotationsAtLastCompletion -= segment.Direction * 360;
+                }
+
+                // This is a best effort. We may not have enough data to match this 1:1, but that's okay.
+                // We know that the player is somewhere in a spin.
+                // In the worst case, this will be lower than expected, and recover in forward playback.
+                currentSpinMaxRotation = Math.Abs(currentSpinRotation);
+            }
 
             lastReportTime = currentTime;
         }
 
-        private void addDelta(double currentTime, float delta)
-        {
-            if (delta == 0)
-                return;
-
-            currentMaxRotation = Math.Max(currentMaxRotation, Math.Abs(currentRotation));
-
-            while (currentMaxRotation >= 360)
-            {
-                int direction = Math.Sign(currentRotation);
-
-                segments.Push(new SpinSegment(currentTime, direction));
-
-                lastCompletionAbsoluteRotation += direction * 360;
-                currentMaxRotation = Math.Abs(currentRotation);
-            }
-        }
-
-        private void rewindDelta(double currentTime, float delta)
-        {
-            while (segments.TryPeek(out var segment) && segment.StartTime > currentTime)
-            {
-                segments.Pop();
-                lastCompletionAbsoluteRotation -= segment.Direction * 360;
-                currentMaxRotation = Math.Abs(currentRotation);
-            }
-
-            currentMaxRotation = Math.Abs(currentRotation);
-        }
-
         /// <summary>
-        /// Represents a single segment of history.
+        /// Represents a single completed spin.
         /// </summary>
-        /// <remarks>
-        /// Each time the player changes direction, a new segment is recorded.
-        /// A segment stores the current absolute angle of rotation. Generally this would be either -360 or 360 for a completed spin, or
-        /// a number representing the last incomplete spin.
-        /// </remarks>
-        private class SpinSegment
+        private class CompletedSpin
         {
             /// <summary>
-            /// The start time of this segment, when the direction change occurred.
+            /// The time at which this spin completion occurred.
             /// </summary>
-            public readonly double StartTime;
+            public readonly double CompletionTime;
 
             /// <summary>
-            /// The direction this segment started in.
+            /// The direction this spin completed in.
             /// </summary>
             public readonly int Direction;
 
-            public SpinSegment(double startTime, int direction)
+            public CompletedSpin(double completionTime, int direction)
             {
                 Debug.Assert(direction == -1 || direction == 1);
 
-                StartTime = startTime;
+                CompletionTime = completionTime;
                 Direction = direction;
             }
         }