From 37ecab3f2f3cbea1a818941bf4153c58ec087158 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Wed, 8 Jul 2020 20:44:27 +0200
Subject: [PATCH 1/5] Add assertions to make spinner tests fail
---
.../TestSceneSpinnerRotation.cs | 27 +++++++++++++++----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs
index ea006ec607..579c47f585 100644
--- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs
+++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs
@@ -14,6 +14,7 @@ using osu.Game.Rulesets.Osu.Objects.Drawables;
using osuTK;
using System.Collections.Generic;
using System.Linq;
+using osu.Framework.Graphics.Sprites;
using osu.Game.Storyboards;
using static osu.Game.Tests.Visual.OsuTestScene.ClockBackedTestWorkingBeatmap;
@@ -36,6 +37,7 @@ namespace osu.Game.Rulesets.Osu.Tests
}
private DrawableSpinner drawableSpinner;
+ private SpriteIcon spinnerSymbol => drawableSpinner.ChildrenOfType<SpriteIcon>().Single();
[SetUpSteps]
public override void SetUpSteps()
@@ -50,23 +52,38 @@ namespace osu.Game.Rulesets.Osu.Tests
public void TestSpinnerRewindingRotation()
{
addSeekStep(5000);
- AddAssert("is rotation absolute not almost 0", () => !Precision.AlmostEquals(drawableSpinner.Disc.RotationAbsolute, 0, 100));
+ AddAssert("is disc rotation not almost 0", () => !Precision.AlmostEquals(drawableSpinner.Disc.Rotation, 0, 100));
+ AddAssert("is disc rotation absolute not almost 0", () => !Precision.AlmostEquals(drawableSpinner.Disc.RotationAbsolute, 0, 100));
addSeekStep(0);
- AddAssert("is rotation absolute almost 0", () => Precision.AlmostEquals(drawableSpinner.Disc.RotationAbsolute, 0, 100));
+ AddAssert("is disc rotation almost 0", () => Precision.AlmostEquals(drawableSpinner.Disc.Rotation, 0, 100));
+ AddAssert("is disc rotation absolute almost 0", () => Precision.AlmostEquals(drawableSpinner.Disc.RotationAbsolute, 0, 100));
}
[Test]
public void TestSpinnerMiddleRewindingRotation()
{
- double estimatedRotation = 0;
+ double finalAbsoluteDiscRotation = 0, finalRelativeDiscRotation = 0, finalSpinnerSymbolRotation = 0;
addSeekStep(5000);
- AddStep("retrieve rotation", () => estimatedRotation = drawableSpinner.Disc.RotationAbsolute);
+ AddStep("retrieve disc relative rotation", () => finalRelativeDiscRotation = drawableSpinner.Disc.Rotation);
+ AddStep("retrieve disc absolute rotation", () => finalAbsoluteDiscRotation = drawableSpinner.Disc.RotationAbsolute);
+ AddStep("retrieve spinner symbol rotation", () => finalSpinnerSymbolRotation = spinnerSymbol.Rotation);
addSeekStep(2500);
+ AddUntilStep("disc rotation rewound",
+ // we want to make sure that the rotation at time 2500 is in the same direction as at time 5000, but about half-way in.
+ () => Precision.AlmostEquals(drawableSpinner.Disc.Rotation, finalRelativeDiscRotation / 2, 100));
+ AddUntilStep("symbol rotation rewound",
+ () => Precision.AlmostEquals(spinnerSymbol.Rotation, finalSpinnerSymbolRotation / 2, 100));
+
addSeekStep(5000);
- AddAssert("is rotation absolute almost same", () => Precision.AlmostEquals(drawableSpinner.Disc.RotationAbsolute, estimatedRotation, 100));
+ AddAssert("is disc rotation almost same",
+ () => Precision.AlmostEquals(drawableSpinner.Disc.Rotation, finalRelativeDiscRotation, 100));
+ AddAssert("is symbol rotation almost same",
+ () => Precision.AlmostEquals(spinnerSymbol.Rotation, finalSpinnerSymbolRotation, 100));
+ AddAssert("is disc rotation absolute almost same",
+ () => Precision.AlmostEquals(drawableSpinner.Disc.RotationAbsolute, finalAbsoluteDiscRotation, 100));
}
[Test]
From 31a1f8b9a75b944c6e52e8089f26feb671c061cb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Wed, 8 Jul 2020 22:37:45 +0200
Subject: [PATCH 2/5] Add coverage for spinning in both directions
---
.../TestSceneSpinnerRotation.cs | 43 +++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs
index 579c47f585..de06570d3c 100644
--- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs
+++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs
@@ -15,6 +15,11 @@ using osuTK;
using System.Collections.Generic;
using System.Linq;
using osu.Framework.Graphics.Sprites;
+using osu.Game.Replays;
+using osu.Game.Rulesets.Osu.Replays;
+using osu.Game.Rulesets.Osu.UI;
+using osu.Game.Rulesets.Replays;
+using osu.Game.Scoring;
using osu.Game.Storyboards;
using static osu.Game.Tests.Visual.OsuTestScene.ClockBackedTestWorkingBeatmap;
@@ -86,6 +91,44 @@ namespace osu.Game.Rulesets.Osu.Tests
() => Precision.AlmostEquals(drawableSpinner.Disc.RotationAbsolute, finalAbsoluteDiscRotation, 100));
}
+ [Test]
+ public void TestRotationDirection([Values(true, false)] bool clockwise)
+ {
+ if (clockwise)
+ {
+ 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);
+
+ AddAssert("disc spin direction correct", () => clockwise ? drawableSpinner.Disc.Rotation > 0 : drawableSpinner.Disc.Rotation < 0);
+ AddAssert("spinner symbol direction correct", () => clockwise ? spinnerSymbol.Rotation > 0 : spinnerSymbol.Rotation < 0);
+ }
+
+ private Replay flipReplay(Replay scoreReplay) => new Replay
+ {
+ Frames = scoreReplay
+ .Frames
+ .Cast<OsuReplayFrame>()
+ .Select(replayFrame =>
+ {
+ var flippedPosition = new Vector2(OsuPlayfield.BASE_SIZE.X - replayFrame.Position.X, replayFrame.Position.Y);
+ return new OsuReplayFrame(replayFrame.Time, flippedPosition, replayFrame.Actions.ToArray());
+ })
+ .Cast<ReplayFrame>()
+ .ToList()
+ };
+
[Test]
public void TestSpinPerMinuteOnRewind()
{
From 213dfac344f67f776874217bca80f7eaa2479bf4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Wed, 8 Jul 2020 20:56:47 +0200
Subject: [PATCH 3/5] Fix broken spinner rotation logic
---
osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs
index 4d37622be5..12034ad333 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs
@@ -197,7 +197,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
float targetScale = relativeCircleScale + (1 - relativeCircleScale) * Progress;
Disc.Scale = new Vector2((float)Interpolation.Lerp(Disc.Scale.X, targetScale, Math.Clamp(Math.Abs(Time.Elapsed) / 100, 0, 1)));
- symbol.Rotation = (float)Interpolation.Lerp(symbol.Rotation, Disc.RotationAbsolute / 2, Math.Clamp(Math.Abs(Time.Elapsed) / 40, 0, 1));
+ symbol.Rotation = (float)Interpolation.Lerp(symbol.Rotation, Disc.Rotation / 2, Math.Clamp(Math.Abs(Time.Elapsed) / 40, 0, 1));
}
protected override void UpdateInitialTransforms()
@@ -207,9 +207,6 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
circleContainer.ScaleTo(Spinner.Scale * 0.3f);
circleContainer.ScaleTo(Spinner.Scale, HitObject.TimePreempt / 1.4f, Easing.OutQuint);
- Disc.RotateTo(-720);
- symbol.RotateTo(-720);
-
mainContainer
.ScaleTo(0)
.ScaleTo(Spinner.Scale * circle.DrawHeight / DrawHeight * 1.4f, HitObject.TimePreempt - 150, Easing.OutQuint)
From 4cd874280cd853722d5cae76c5a2af16c99b58f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Wed, 8 Jul 2020 21:05:41 +0200
Subject: [PATCH 4/5] Add clarifying xmldoc for RotationAbsolute
---
.../Objects/Drawables/Pieces/SpinnerDisc.cs | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/SpinnerDisc.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/SpinnerDisc.cs
index d4ef039b79..408aba54d7 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/SpinnerDisc.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/SpinnerDisc.cs
@@ -73,6 +73,19 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Pieces
}
}
+ /// <summary>
+ /// The total rotation performed on the spinner disc, disregarding the spin direction.
+ /// </summary>
+ /// <remarks>
+ /// 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).
+ /// </remarks>
+ /// <example>
+ /// 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"/>).
+ /// </example>
+ public float RotationAbsolute;
+
/// <summary>
/// Whether currently in the correct time range to allow spinning.
/// </summary>
@@ -88,7 +101,6 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Pieces
private float lastAngle;
private float currentRotation;
- public float RotationAbsolute;
private int completeTick;
private bool updateCompleteTick() => completeTick != (completeTick = (int)(RotationAbsolute / 360));
From efb2c2f4aee0df8952d1efeac6817a49a6d1b391 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 9 Jul 2020 12:01:00 +0900
Subject: [PATCH 5/5] Rename variable to be more clear on purpose
---
osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs | 2 +-
osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs | 8 ++++----
.../Objects/Drawables/DrawableSpinner.cs | 4 ++--
.../Objects/Drawables/Pieces/SpinnerDisc.cs | 6 +++---
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs
index 65bed071cd..8cb7f3f4b6 100644
--- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs
+++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs
@@ -67,7 +67,7 @@ namespace osu.Game.Rulesets.Osu.Tests
if (auto && !userTriggered && Time.Current > Spinner.StartTime + Spinner.Duration / 2 && Progress < 1)
{
// force completion only once to not break human interaction
- Disc.RotationAbsolute = Spinner.SpinsRequired * 360;
+ Disc.CumulativeRotation = Spinner.SpinsRequired * 360;
auto = false;
}
diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs
index de06570d3c..6b1394d799 100644
--- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs
+++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinnerRotation.cs
@@ -58,11 +58,11 @@ namespace osu.Game.Rulesets.Osu.Tests
{
addSeekStep(5000);
AddAssert("is disc rotation not almost 0", () => !Precision.AlmostEquals(drawableSpinner.Disc.Rotation, 0, 100));
- AddAssert("is disc rotation absolute not almost 0", () => !Precision.AlmostEquals(drawableSpinner.Disc.RotationAbsolute, 0, 100));
+ AddAssert("is disc rotation absolute not almost 0", () => !Precision.AlmostEquals(drawableSpinner.Disc.CumulativeRotation, 0, 100));
addSeekStep(0);
AddAssert("is disc rotation almost 0", () => Precision.AlmostEquals(drawableSpinner.Disc.Rotation, 0, 100));
- AddAssert("is disc rotation absolute almost 0", () => Precision.AlmostEquals(drawableSpinner.Disc.RotationAbsolute, 0, 100));
+ AddAssert("is disc rotation absolute almost 0", () => Precision.AlmostEquals(drawableSpinner.Disc.CumulativeRotation, 0, 100));
}
[Test]
@@ -72,7 +72,7 @@ namespace osu.Game.Rulesets.Osu.Tests
addSeekStep(5000);
AddStep("retrieve disc relative rotation", () => finalRelativeDiscRotation = drawableSpinner.Disc.Rotation);
- AddStep("retrieve disc absolute rotation", () => finalAbsoluteDiscRotation = drawableSpinner.Disc.RotationAbsolute);
+ AddStep("retrieve disc absolute rotation", () => finalAbsoluteDiscRotation = drawableSpinner.Disc.CumulativeRotation);
AddStep("retrieve spinner symbol rotation", () => finalSpinnerSymbolRotation = spinnerSymbol.Rotation);
addSeekStep(2500);
@@ -88,7 +88,7 @@ namespace osu.Game.Rulesets.Osu.Tests
AddAssert("is symbol rotation almost same",
() => Precision.AlmostEquals(spinnerSymbol.Rotation, finalSpinnerSymbolRotation, 100));
AddAssert("is disc rotation absolute almost same",
- () => Precision.AlmostEquals(drawableSpinner.Disc.RotationAbsolute, finalAbsoluteDiscRotation, 100));
+ () => Precision.AlmostEquals(drawableSpinner.Disc.CumulativeRotation, finalAbsoluteDiscRotation, 100));
}
[Test]
diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs
index 12034ad333..be6766509c 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs
@@ -138,7 +138,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
positionBindable.BindTo(HitObject.PositionBindable);
}
- public float Progress => Math.Clamp(Disc.RotationAbsolute / 360 / Spinner.SpinsRequired, 0, 1);
+ public float Progress => Math.Clamp(Disc.CumulativeRotation / 360 / Spinner.SpinsRequired, 0, 1);
protected override void CheckForResult(bool userTriggered, double timeOffset)
{
@@ -191,7 +191,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
circle.Rotation = Disc.Rotation;
Ticks.Rotation = Disc.Rotation;
- SpmCounter.SetRotation(Disc.RotationAbsolute);
+ SpmCounter.SetRotation(Disc.CumulativeRotation);
float relativeCircleScale = Spinner.Scale * circle.DrawHeight / mainContainer.DrawHeight;
float targetScale = relativeCircleScale + (1 - relativeCircleScale) * Progress;
diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/SpinnerDisc.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/SpinnerDisc.cs
index 408aba54d7..35819cd05e 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/SpinnerDisc.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/SpinnerDisc.cs
@@ -84,7 +84,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Pieces
/// 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"/>).
/// </example>
- public float RotationAbsolute;
+ public float CumulativeRotation;
/// <summary>
/// Whether currently in the correct time range to allow spinning.
@@ -103,7 +103,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Pieces
private float currentRotation;
private int completeTick;
- private bool updateCompleteTick() => completeTick != (completeTick = (int)(RotationAbsolute / 360));
+ private bool updateCompleteTick() => completeTick != (completeTick = (int)(CumulativeRotation / 360));
private bool rotationTransferred;
@@ -161,7 +161,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Pieces
}
currentRotation += angle;
- RotationAbsolute += Math.Abs(angle) * Math.Sign(Clock.ElapsedFrameTime);
+ CumulativeRotation += Math.Abs(angle) * Math.Sign(Clock.ElapsedFrameTime);
}
}
}