From 92f3c90cd46061c423ebdd23738b50c8a7101ffb Mon Sep 17 00:00:00 2001 From: Michael Zhang Date: Tue, 22 Aug 2023 14:22:08 -0500 Subject: [PATCH] Fix code review issues and add unit tests --- .../Checks/CheckSpinnerRecoveryTimeTest.cs | 95 +++++++++++++++++++ .../Edit/Checks/CheckSpinnerRecoveryTime.cs | 11 +-- 2 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckSpinnerRecoveryTimeTest.cs diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckSpinnerRecoveryTimeTest.cs b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckSpinnerRecoveryTimeTest.cs new file mode 100644 index 0000000000..331f6725dd --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/Editor/Checks/CheckSpinnerRecoveryTimeTest.cs @@ -0,0 +1,95 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Edit.Checks.Components; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Osu.Edit.Checks; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Tests.Beatmaps; + +namespace osu.Game.Rulesets.Osu.Tests.Editor.Checks +{ + [TestFixture] + public class CheckSpinnerRecoveryTimeTest + { + private CheckSpinnerRecoveryTime check = null!; + + [SetUp] + public void Setup() + { + check = new CheckSpinnerRecoveryTime(); + } + + [Test] + public void TestEasyOk() + { + Assert.That(runScenario(DifficultyRating.Easy, 1000, 3000), Is.Empty); + } + + [Test] + public void TestEasyFail() + { + var issues = runScenario(DifficultyRating.Easy, 1000, 2500).ToList(); + Assert.That(issues, Has.Count.EqualTo(1)); + Assert.That(issues.First().Template is CheckSpinnerRecoveryTime.IssueTemplateSpinnerRecoveryTooShort); + } + + [Test] + public void TestNormalOk() + { + Assert.That(runScenario(DifficultyRating.Normal, 1000, 2000), Is.Empty); + } + + [Test] + public void TestNormalFail() + { + var issues = runScenario(DifficultyRating.Normal, 1000, 1500).ToList(); + Assert.That(issues, Has.Count.EqualTo(1)); + Assert.That(issues.First().Template is CheckSpinnerRecoveryTime.IssueTemplateSpinnerRecoveryTooShort); + } + + [Test] + public void TestHardOk() + { + Assert.That(runScenario(DifficultyRating.Hard, 1000, 1500), Is.Empty); + } + + [Test] + public void TestHardFail() + { + var issues = runScenario(DifficultyRating.Hard, 1000, 1250).ToList(); + Assert.That(issues, Has.Count.EqualTo(1)); + Assert.That(issues.First().Template is CheckSpinnerRecoveryTime.IssueTemplateSpinnerRecoveryTooShort); + } + + [Test] + public void TestEverythingElseOk() + { + Assert.That(runScenario(DifficultyRating.Insane, 1000, 1001), Is.Empty); + Assert.That(runScenario(DifficultyRating.Expert, 1000, 1001), Is.Empty); + Assert.That(runScenario(DifficultyRating.ExpertPlus, 1000, 1001), Is.Empty); + } + + private IEnumerable runScenario(DifficultyRating rating, double spinnerEnd, double nextObjectStart) + { + Spinner spinner = new Spinner { StartTime = 0, EndTime = spinnerEnd }; + HitObject nextObject = new HitObject { StartTime = nextObjectStart }; + var hitObjects = new List { spinner, nextObject }; + + TimingControlPoint tp = new TimingControlPoint { BeatLength = 500 }; + ControlPointInfo controlPoints = new ControlPointInfo(); + controlPoints.Add(0, tp); + + var beatmap = new Beatmap { HitObjects = hitObjects, ControlPointInfo = controlPoints }; + var context = new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap), rating); + + return check.Run(context); + } + } +} diff --git a/osu.Game.Rulesets.Osu/Edit/Checks/CheckSpinnerRecoveryTime.cs b/osu.Game.Rulesets.Osu/Edit/Checks/CheckSpinnerRecoveryTime.cs index faeacb34ed..253e617e86 100644 --- a/osu.Game.Rulesets.Osu/Edit/Checks/CheckSpinnerRecoveryTime.cs +++ b/osu.Game.Rulesets.Osu/Edit/Checks/CheckSpinnerRecoveryTime.cs @@ -21,13 +21,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks public IEnumerable Run(BeatmapVerifierContext context) { - double? recommendedRecoveryTime_ = this.recommendedRecoveryTime(context.InterpretedDifficulty); + double? recommendedRecoveryTime = getRecommendedRecoveryTime(context.InterpretedDifficulty); - // If anything goes, we can skip this check - if (recommendedRecoveryTime_ is not double recommendedRecoveryTime) - { + if (!recommendedRecoveryTime.HasValue) yield break; - } foreach (var (firstObject, secondObject) in context.Beatmap.HitObjects.Zip(context.Beatmap.HitObjects.Skip(1))) { @@ -43,7 +40,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks if (timeDifferenceInBeats < recommendedRecoveryTime) { yield return new IssueTemplateSpinnerRecoveryTooShort(this) - .Create(spinner, timeDifferenceInBeats, recommendedRecoveryTime, context.InterpretedDifficulty); + .Create(spinner, timeDifferenceInBeats, recommendedRecoveryTime.Value, context.InterpretedDifficulty); } } @@ -55,7 +52,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Checks /// /// The difficulty to interpret the beatmap as /// Number of beats if there is a guideline for it, null if anything goes - private double? recommendedRecoveryTime(DifficultyRating interpretedDifficulty) + private double? getRecommendedRecoveryTime(DifficultyRating interpretedDifficulty) { switch (interpretedDifficulty) {