From 6fd77e536df6bfcab9a3c2ab8d8c19fdc822fb8b Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Sun, 25 Apr 2021 05:34:54 +0200 Subject: [PATCH 01/25] Add unsnap check --- osu.Game/Rulesets/Edit/BeatmapVerifier.cs | 5 +- osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs | 119 ++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs diff --git a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs index f33feac971..aa3459a01a 100644 --- a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs +++ b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs @@ -22,7 +22,10 @@ namespace osu.Game.Rulesets.Edit // Audio new CheckAudioPresence(), - new CheckAudioQuality() + new CheckAudioQuality(), + + // Compose + new CheckUnsnaps() }; public IEnumerable Run(IBeatmap playableBeatmap, WorkingBeatmap workingBeatmap) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs b/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs new file mode 100644 index 0000000000..835c4bdb69 --- /dev/null +++ b/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs @@ -0,0 +1,119 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using System.Linq; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Rulesets.Edit.Checks.Components; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; + +namespace osu.Game.Rulesets.Edit.Checks +{ + public class CheckUnsnaps : ICheck + { + private const double unsnap_ms_threshold = 2; + + private static readonly int[] greatest_common_divisors = { 16, 12, 9, 7, 5 }; + + public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Unsnapped hitobjects"); + + public IEnumerable PossibleTemplates => new IssueTemplate[] + { + new IssueTemplate2MsOrMore(this), + new IssueTemplate1MsOrMore(this) + }; + + public IEnumerable Run(IBeatmap playableBeatmap, IWorkingBeatmap workingBeatmap) + { + foreach (var hitobject in playableBeatmap.HitObjects) + { + double startUnsnap = hitobject.StartTime - closestSnapTime(playableBeatmap, hitobject.StartTime); + string startPostfix = hitobject is IHasDuration ? "start" : ""; + foreach (var issue in getUnsnapIssues(hitobject, startUnsnap, hitobject.StartTime, startPostfix)) + yield return issue; + + if (hitobject is IHasRepeats hasRepeats) + { + for (int repeatIndex = 0; repeatIndex < hasRepeats.RepeatCount; ++repeatIndex) + { + double spanDuration = hasRepeats.Duration / (hasRepeats.RepeatCount + 1); + double repeatTime = hitobject.StartTime + spanDuration * (repeatIndex + 1); + double repeatUnsnap = repeatTime - closestSnapTime(playableBeatmap, repeatTime); + foreach (var issue in getUnsnapIssues(hitobject, repeatUnsnap, repeatTime, "repeat")) + yield return issue; + } + } + + if (hitobject is IHasDuration hasDuration) + { + double endUnsnap = hasDuration.EndTime - closestSnapTime(playableBeatmap, hasDuration.EndTime); + foreach (var issue in getUnsnapIssues(hitobject, endUnsnap, hasDuration.EndTime, "end")) + yield return issue; + } + } + } + + private IEnumerable getUnsnapIssues(HitObject hitobject, double unsnap, double time, string postfix = "") + { + if (Math.Abs(unsnap) >= unsnap_ms_threshold) + yield return new IssueTemplate2MsOrMore(this).Create(hitobject, unsnap, time, postfix); + else if (Math.Abs(unsnap) >= 1) + yield return new IssueTemplate1MsOrMore(this).Create(hitobject, unsnap, time, postfix); + + // We don't care about unsnaps < 1 ms, as all object ends have these due to the way SV works. + } + + private int closestSnapTime(IBeatmap playableBeatmap, double time) + { + var timingPoint = playableBeatmap.ControlPointInfo.TimingPointAt(time); + double smallestUnsnap = greatest_common_divisors.Select(divisor => Math.Abs(time - snapTime(timingPoint, time, divisor))).Min(); + + return (int)Math.Round(time + smallestUnsnap); + } + + private int snapTime(TimingControlPoint timingPoint, double time, int beatDivisor) + { + double beatLength = timingPoint.BeatLength / beatDivisor; + int beatLengths = (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero); + + // Casting to int matches the editor in both stable and lazer. + return (int)(timingPoint.Time + beatLengths * beatLength); + } + + public abstract class IssueTemplateUnsnap : IssueTemplate + { + protected IssueTemplateUnsnap(ICheck check, IssueType type) + : base(check, type, "{0:0.##} is unsnapped by {1:0.##} ms.") + { + } + + public Issue Create(HitObject hitobject, double unsnap, double time, string postfix = "") + { + string objectName = hitobject.GetType().Name; + if (!string.IsNullOrEmpty(postfix)) + objectName += " " + postfix; + + return new Issue(hitobject, this, objectName, unsnap) { Time = time }; + } + } + + public class IssueTemplate2MsOrMore : IssueTemplateUnsnap + { + public IssueTemplate2MsOrMore(ICheck check) + : base(check, IssueType.Problem) + { + } + } + + public class IssueTemplate1MsOrMore : IssueTemplateUnsnap + { + public IssueTemplate1MsOrMore(ICheck check) + : base(check, IssueType.Negligible) + { + } + } + } +} From 9178aa1d7d745f3057f0080ec704e86ced17f534 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 26 Apr 2021 04:48:56 +0200 Subject: [PATCH 02/25] Add unsnap check tests --- .../Editing/Checks/CheckUnsnapsTest.cs | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs diff --git a/osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs b/osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs new file mode 100644 index 0000000000..88939c43ce --- /dev/null +++ b/osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs @@ -0,0 +1,160 @@ +// 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 Moq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Rulesets.Edit.Checks; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; +using osu.Game.Rulesets.Osu.Objects; + +namespace osu.Game.Tests.Editing.Checks +{ + [TestFixture] + public class CheckUnsnapsTest + { + private CheckUnsnaps check; + private ControlPointInfo cpi; + + [SetUp] + public void Setup() + { + check = new CheckUnsnaps(); + + cpi = new ControlPointInfo(); + cpi.Add(100, new TimingControlPoint { BeatLength = 100 }); + } + + [Test] + public void TestCircleSnapped() + { + assertOk(new List + { + new HitCircle { StartTime = 100 } + }); + } + + [Test] + public void TestCircleUnsnapped1Ms() + { + assert1Ms(new List + { + new HitCircle { StartTime = 101 } + }); + + assert1Ms(new List + { + new HitCircle { StartTime = 99 } + }); + } + + [Test] + public void TestCircleUnsnapped2Ms() + { + assert2Ms(new List + { + new HitCircle { StartTime = 102 } + }); + + assert2Ms(new List + { + new HitCircle { StartTime = 98 } + }); + } + + [Test] + public void TestSliderSnapped() + { + // Slider ends are naturally < 1 ms unsnapped because of how SV works. + var mockSlider = new Mock(); + mockSlider.SetupGet(s => s.StartTime).Returns(100); + mockSlider.As().Setup(r => r.RepeatCount).Returns(0); + mockSlider.As().Setup(d => d.Duration).Returns(400.75d); + + assertOk(new List + { + mockSlider.Object + }); + } + + [Test] + public void TestSliderUnsnapped1Ms() + { + assert1Ms(new List + { + getSliderMock(startTime: 101, endTime: 401.75d).Object + }, count: 2); + + // End is only off by 0.25 ms, hence count 1. + assert1Ms(new List + { + getSliderMock(startTime: 99, endTime: 399.75d).Object + }, count: 1); + } + + [Test] + public void TestSliderUnsnapped2Ms() + { + assert2Ms(new List + { + getSliderMock(startTime: 102, endTime: 402.75d).Object + }, count: 2); + + // Start and end are 2 ms and 1.25 ms off respectively, hence two different issues in one object. + var hitobjects = new List + { + getSliderMock(startTime: 98, endTime: 398.75d).Object + }; + + var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); + + Assert.That(issues, Has.Count.EqualTo(2)); + Assert.That(issues.Any(issue => issue.Template is CheckUnsnaps.IssueTemplate1MsOrMore)); + Assert.That(issues.Any(issue => issue.Template is CheckUnsnaps.IssueTemplate2MsOrMore)); + } + + private Mock getSliderMock(double startTime, double endTime, int repeats = 0) + { + var mockSlider = new Mock(); + mockSlider.SetupGet(s => s.StartTime).Returns(startTime); + mockSlider.As().Setup(r => r.RepeatCount).Returns(repeats); + mockSlider.As().Setup(d => d.EndTime).Returns(endTime); + + return mockSlider; + } + + private void assertOk(List hitobjects) + { + Assert.That(check.Run(getPlayableBeatmap(hitobjects), null), Is.Empty); + } + + private void assert1Ms(List hitobjects, int count = 1) + { + var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); + + Assert.That(issues, Has.Count.EqualTo(count)); + Assert.That(issues.All(issue => issue.Template is CheckUnsnaps.IssueTemplate1MsOrMore)); + } + + private void assert2Ms(List hitobjects, int count = 1) + { + var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); + + Assert.That(issues, Has.Count.EqualTo(count)); + Assert.That(issues.All(issue => issue.Template is CheckUnsnaps.IssueTemplate2MsOrMore)); + } + + private IBeatmap getPlayableBeatmap(List hitobjects) + { + return new Beatmap + { + ControlPointInfo = cpi, + HitObjects = hitobjects + }; + } + } +} From 049e42fa854f2ec8bb0493246d34a8d43cb9209a Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 26 Apr 2021 05:07:24 +0200 Subject: [PATCH 03/25] Move snapping responsibility to `IBeatmap` Seems `EditorBeatmap` already implements a different kind of `SnapTime` from `IBeatSnapProvider`, so method names here aren't great. This is very similar to what https://github.com/ppy/osu/pull/12558 is doing, so may need to do some duplicate resolution later, especially surrounding `ClosestBeatSnapDivisor`. Worth noting that this change makes 1/7, 1/5, etc unsupported for now, as we now rely on `BindableBeatDivisor.VALID_DIVISORS`. --- osu.Game/Beatmaps/Beatmap.cs | 26 ++++++++++++++++++ osu.Game/Beatmaps/IBeatmap.cs | 22 +++++++++++++++ osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs | 27 +++---------------- osu.Game/Screens/Edit/EditorBeatmap.cs | 13 +++++---- osu.Game/Screens/Play/GameplayBeatmap.cs | 9 +++++++ 5 files changed, 68 insertions(+), 29 deletions(-) diff --git a/osu.Game/Beatmaps/Beatmap.cs b/osu.Game/Beatmaps/Beatmap.cs index e5b6a4bc44..1ce01aee24 100644 --- a/osu.Game/Beatmaps/Beatmap.cs +++ b/osu.Game/Beatmaps/Beatmap.cs @@ -9,6 +9,7 @@ using System.Linq; using osu.Game.Beatmaps.ControlPoints; using Newtonsoft.Json; using osu.Game.IO.Serialization.Converters; +using osu.Game.Screens.Edit; namespace osu.Game.Beatmaps { @@ -74,6 +75,31 @@ namespace osu.Game.Beatmaps return mostCommon.beatLength; } + public int SnapTimeForDivisor(double time, int beatDivisor, double? referenceTime = null) + { + var timingPoint = ControlPointInfo.TimingPointAt(referenceTime ?? time); + var beatLength = timingPoint.BeatLength / beatDivisor; + var beatLengths = (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero); + + return (int)(timingPoint.Time + beatLengths * beatLength); + } + + public int SnapTimeAnyDivisor(double time, double? referenceTime = null) + { + return SnapTimeForDivisor(time, ClosestBeatSnapDivisor(time, referenceTime), referenceTime); + } + + public int ClosestBeatSnapDivisor(double time, double? referenceTime = null) + { + double getUnsnap(int divisor) => Math.Abs(time - SnapTimeForDivisor(time, divisor, referenceTime)); + + int[] divisors = BindableBeatDivisor.VALID_DIVISORS; + double smallestUnsnap = divisors.Min(getUnsnap); + int closestDivisor = divisors.FirstOrDefault(divisor => getUnsnap(divisor) == smallestUnsnap); + + return closestDivisor; + } + IBeatmap IBeatmap.Clone() => Clone(); public Beatmap Clone() => (Beatmap)MemberwiseClone(); diff --git a/osu.Game/Beatmaps/IBeatmap.cs b/osu.Game/Beatmaps/IBeatmap.cs index 769b33009a..3b043cb59b 100644 --- a/osu.Game/Beatmaps/IBeatmap.cs +++ b/osu.Game/Beatmaps/IBeatmap.cs @@ -51,6 +51,28 @@ namespace osu.Game.Beatmaps /// double GetMostCommonBeatLength(); + /// + /// Returns the time on the given beat divisor closest to the given time. + /// + /// The time to find the closest snapped time to. + /// The beat divisor to snap to. + /// The time at which the timing point is retrieved, by default same as time. + int SnapTimeForDivisor(double time, int beatDivisor, double? referenceTime = null); + + /// + /// Returns the time on any valid beat divisor closest to the given time. + /// + /// The time to find the closest snapped time to. + /// The time at which the timing point is retrieved, by default same as time. + int SnapTimeAnyDivisor(double time, double? referenceTime = null); + + /// + /// Returns the beat snap divisor closest to the given time. If two are equally close, the smallest is returned. + /// + /// The time to find the closest beat snap divisor to. + /// The time at which the timing point is retrieved, by default same as time. + int ClosestBeatSnapDivisor(double time, double? referenceTime = null); + /// /// Creates a shallow-clone of this beatmap and returns it. /// diff --git a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs b/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs index 835c4bdb69..d15dc6f179 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs @@ -3,9 +3,7 @@ using System; using System.Collections.Generic; -using System.Linq; using osu.Game.Beatmaps; -using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; @@ -16,8 +14,6 @@ namespace osu.Game.Rulesets.Edit.Checks { private const double unsnap_ms_threshold = 2; - private static readonly int[] greatest_common_divisors = { 16, 12, 9, 7, 5 }; - public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Unsnapped hitobjects"); public IEnumerable PossibleTemplates => new IssueTemplate[] @@ -30,7 +26,7 @@ namespace osu.Game.Rulesets.Edit.Checks { foreach (var hitobject in playableBeatmap.HitObjects) { - double startUnsnap = hitobject.StartTime - closestSnapTime(playableBeatmap, hitobject.StartTime); + double startUnsnap = hitobject.StartTime - playableBeatmap.SnapTimeAnyDivisor(hitobject.StartTime); string startPostfix = hitobject is IHasDuration ? "start" : ""; foreach (var issue in getUnsnapIssues(hitobject, startUnsnap, hitobject.StartTime, startPostfix)) yield return issue; @@ -41,7 +37,7 @@ namespace osu.Game.Rulesets.Edit.Checks { double spanDuration = hasRepeats.Duration / (hasRepeats.RepeatCount + 1); double repeatTime = hitobject.StartTime + spanDuration * (repeatIndex + 1); - double repeatUnsnap = repeatTime - closestSnapTime(playableBeatmap, repeatTime); + double repeatUnsnap = repeatTime - playableBeatmap.SnapTimeAnyDivisor(repeatTime); foreach (var issue in getUnsnapIssues(hitobject, repeatUnsnap, repeatTime, "repeat")) yield return issue; } @@ -49,7 +45,7 @@ namespace osu.Game.Rulesets.Edit.Checks if (hitobject is IHasDuration hasDuration) { - double endUnsnap = hasDuration.EndTime - closestSnapTime(playableBeatmap, hasDuration.EndTime); + double endUnsnap = hasDuration.EndTime - playableBeatmap.SnapTimeAnyDivisor(hasDuration.EndTime); foreach (var issue in getUnsnapIssues(hitobject, endUnsnap, hasDuration.EndTime, "end")) yield return issue; } @@ -66,23 +62,6 @@ namespace osu.Game.Rulesets.Edit.Checks // We don't care about unsnaps < 1 ms, as all object ends have these due to the way SV works. } - private int closestSnapTime(IBeatmap playableBeatmap, double time) - { - var timingPoint = playableBeatmap.ControlPointInfo.TimingPointAt(time); - double smallestUnsnap = greatest_common_divisors.Select(divisor => Math.Abs(time - snapTime(timingPoint, time, divisor))).Min(); - - return (int)Math.Round(time + smallestUnsnap); - } - - private int snapTime(TimingControlPoint timingPoint, double time, int beatDivisor) - { - double beatLength = timingPoint.BeatLength / beatDivisor; - int beatLengths = (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero); - - // Casting to int matches the editor in both stable and lazer. - return (int)(timingPoint.Time + beatLengths * beatLength); - } - public abstract class IssueTemplateUnsnap : IssueTemplate { protected IssueTemplateUnsnap(ICheck check, IssueType type) diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index 4bf4a3b8f3..9334814e2a 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -301,14 +301,17 @@ namespace osu.Game.Screens.Edit return list.Count - 1; } - public double SnapTime(double time, double? referenceTime) + public int SnapTimeForDivisor(double time, int beatDivisor, double? referenceTime = null) { - var timingPoint = ControlPointInfo.TimingPointAt(referenceTime ?? time); - var beatLength = timingPoint.BeatLength / BeatDivisor; - - return timingPoint.Time + (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero) * beatLength; + return PlayableBeatmap.SnapTimeForDivisor(time, beatDivisor, referenceTime); } + public int SnapTimeAnyDivisor(double time, double? referenceTime = null) => PlayableBeatmap.SnapTimeAnyDivisor(time, referenceTime); + + public int ClosestBeatSnapDivisor(double time, double? referenceTime = null) => PlayableBeatmap.ClosestBeatSnapDivisor(time, referenceTime); + + public double SnapTime(double time, double? referenceTime) => SnapTimeForDivisor(time, BeatDivisor, referenceTime); + public double GetBeatLengthAtTime(double referenceTime) => ControlPointInfo.TimingPointAt(referenceTime).BeatLength / BeatDivisor; public int BeatDivisor => beatDivisor?.Value ?? 1; diff --git a/osu.Game/Screens/Play/GameplayBeatmap.cs b/osu.Game/Screens/Play/GameplayBeatmap.cs index 74fbe540fa..a3a2bbd41b 100644 --- a/osu.Game/Screens/Play/GameplayBeatmap.cs +++ b/osu.Game/Screens/Play/GameplayBeatmap.cs @@ -45,6 +45,15 @@ namespace osu.Game.Screens.Play public double GetMostCommonBeatLength() => PlayableBeatmap.GetMostCommonBeatLength(); + public int SnapTimeForDivisor(double time, int beatDivisor, double? referenceTime = null) + { + return PlayableBeatmap.SnapTimeForDivisor(time, beatDivisor, referenceTime); + } + + public int SnapTimeAnyDivisor(double time, double? referenceTime = null) => PlayableBeatmap.SnapTimeAnyDivisor(time, referenceTime); + + public int ClosestBeatSnapDivisor(double time, double? referenceTime = null) => PlayableBeatmap.ClosestBeatSnapDivisor(time, referenceTime); + public IBeatmap Clone() => PlayableBeatmap.Clone(); private readonly Bindable lastJudgementResult = new Bindable(); From 7b9ed924be4ab084101d24fa551ddaee8297f410 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 26 Apr 2021 16:07:30 +0200 Subject: [PATCH 04/25] Rename snapping methods Further separates them from `IBeatSnapProvider`'s `SnapTime`, and groups them together more, to prevent confusion between the two interfaces. Also changes the xmldoc of the reference time to that of `IBeatSnapProvider` for consistency. --- osu.Game/Beatmaps/Beatmap.cs | 10 +++++----- osu.Game/Beatmaps/IBeatmap.cs | 12 ++++++------ osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs | 6 +++--- osu.Game/Screens/Edit/EditorBeatmap.cs | 10 +++++----- osu.Game/Screens/Play/GameplayBeatmap.cs | 8 ++++---- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/osu.Game/Beatmaps/Beatmap.cs b/osu.Game/Beatmaps/Beatmap.cs index 1ce01aee24..66b8f169ef 100644 --- a/osu.Game/Beatmaps/Beatmap.cs +++ b/osu.Game/Beatmaps/Beatmap.cs @@ -75,7 +75,7 @@ namespace osu.Game.Beatmaps return mostCommon.beatLength; } - public int SnapTimeForDivisor(double time, int beatDivisor, double? referenceTime = null) + public int ClosestSnapTime(double time, int beatDivisor, double? referenceTime = null) { var timingPoint = ControlPointInfo.TimingPointAt(referenceTime ?? time); var beatLength = timingPoint.BeatLength / beatDivisor; @@ -84,14 +84,14 @@ namespace osu.Game.Beatmaps return (int)(timingPoint.Time + beatLengths * beatLength); } - public int SnapTimeAnyDivisor(double time, double? referenceTime = null) + public int ClosestSnapTime(double time, double? referenceTime = null) { - return SnapTimeForDivisor(time, ClosestBeatSnapDivisor(time, referenceTime), referenceTime); + return ClosestSnapTime(time, ClosestBeatDivisor(time, referenceTime), referenceTime); } - public int ClosestBeatSnapDivisor(double time, double? referenceTime = null) + public int ClosestBeatDivisor(double time, double? referenceTime = null) { - double getUnsnap(int divisor) => Math.Abs(time - SnapTimeForDivisor(time, divisor, referenceTime)); + double getUnsnap(int divisor) => Math.Abs(time - ClosestSnapTime(time, divisor, referenceTime)); int[] divisors = BindableBeatDivisor.VALID_DIVISORS; double smallestUnsnap = divisors.Min(getUnsnap); diff --git a/osu.Game/Beatmaps/IBeatmap.cs b/osu.Game/Beatmaps/IBeatmap.cs index 3b043cb59b..679d639fd1 100644 --- a/osu.Game/Beatmaps/IBeatmap.cs +++ b/osu.Game/Beatmaps/IBeatmap.cs @@ -56,22 +56,22 @@ namespace osu.Game.Beatmaps /// /// The time to find the closest snapped time to. /// The beat divisor to snap to. - /// The time at which the timing point is retrieved, by default same as time. - int SnapTimeForDivisor(double time, int beatDivisor, double? referenceTime = null); + /// An optional reference point to use for timing point lookup. + int ClosestSnapTime(double time, int beatDivisor, double? referenceTime = null); /// /// Returns the time on any valid beat divisor closest to the given time. /// /// The time to find the closest snapped time to. - /// The time at which the timing point is retrieved, by default same as time. - int SnapTimeAnyDivisor(double time, double? referenceTime = null); + /// An optional reference point to use for timing point lookup. + int ClosestSnapTime(double time, double? referenceTime = null); /// /// Returns the beat snap divisor closest to the given time. If two are equally close, the smallest is returned. /// /// The time to find the closest beat snap divisor to. - /// The time at which the timing point is retrieved, by default same as time. - int ClosestBeatSnapDivisor(double time, double? referenceTime = null); + /// An optional reference point to use for timing point lookup. + int ClosestBeatDivisor(double time, double? referenceTime = null); /// /// Creates a shallow-clone of this beatmap and returns it. diff --git a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs b/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs index d15dc6f179..ca268652a9 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs @@ -26,7 +26,7 @@ namespace osu.Game.Rulesets.Edit.Checks { foreach (var hitobject in playableBeatmap.HitObjects) { - double startUnsnap = hitobject.StartTime - playableBeatmap.SnapTimeAnyDivisor(hitobject.StartTime); + double startUnsnap = hitobject.StartTime - playableBeatmap.ClosestSnapTime(hitobject.StartTime); string startPostfix = hitobject is IHasDuration ? "start" : ""; foreach (var issue in getUnsnapIssues(hitobject, startUnsnap, hitobject.StartTime, startPostfix)) yield return issue; @@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Edit.Checks { double spanDuration = hasRepeats.Duration / (hasRepeats.RepeatCount + 1); double repeatTime = hitobject.StartTime + spanDuration * (repeatIndex + 1); - double repeatUnsnap = repeatTime - playableBeatmap.SnapTimeAnyDivisor(repeatTime); + double repeatUnsnap = repeatTime - playableBeatmap.ClosestSnapTime(repeatTime); foreach (var issue in getUnsnapIssues(hitobject, repeatUnsnap, repeatTime, "repeat")) yield return issue; } @@ -45,7 +45,7 @@ namespace osu.Game.Rulesets.Edit.Checks if (hitobject is IHasDuration hasDuration) { - double endUnsnap = hasDuration.EndTime - playableBeatmap.SnapTimeAnyDivisor(hasDuration.EndTime); + double endUnsnap = hasDuration.EndTime - playableBeatmap.ClosestSnapTime(hasDuration.EndTime); foreach (var issue in getUnsnapIssues(hitobject, endUnsnap, hasDuration.EndTime, "end")) yield return issue; } diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index 9334814e2a..72fb0ac9e9 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -301,16 +301,16 @@ namespace osu.Game.Screens.Edit return list.Count - 1; } - public int SnapTimeForDivisor(double time, int beatDivisor, double? referenceTime = null) + public int ClosestSnapTime(double time, int beatDivisor, double? referenceTime = null) { - return PlayableBeatmap.SnapTimeForDivisor(time, beatDivisor, referenceTime); + return PlayableBeatmap.ClosestSnapTime(time, beatDivisor, referenceTime); } - public int SnapTimeAnyDivisor(double time, double? referenceTime = null) => PlayableBeatmap.SnapTimeAnyDivisor(time, referenceTime); + public int ClosestSnapTime(double time, double? referenceTime = null) => PlayableBeatmap.ClosestSnapTime(time, referenceTime); - public int ClosestBeatSnapDivisor(double time, double? referenceTime = null) => PlayableBeatmap.ClosestBeatSnapDivisor(time, referenceTime); + public int ClosestBeatDivisor(double time, double? referenceTime = null) => PlayableBeatmap.ClosestBeatDivisor(time, referenceTime); - public double SnapTime(double time, double? referenceTime) => SnapTimeForDivisor(time, BeatDivisor, referenceTime); + public double SnapTime(double time, double? referenceTime) => ClosestSnapTime(time, BeatDivisor, referenceTime); public double GetBeatLengthAtTime(double referenceTime) => ControlPointInfo.TimingPointAt(referenceTime).BeatLength / BeatDivisor; diff --git a/osu.Game/Screens/Play/GameplayBeatmap.cs b/osu.Game/Screens/Play/GameplayBeatmap.cs index a3a2bbd41b..92f58c8759 100644 --- a/osu.Game/Screens/Play/GameplayBeatmap.cs +++ b/osu.Game/Screens/Play/GameplayBeatmap.cs @@ -45,14 +45,14 @@ namespace osu.Game.Screens.Play public double GetMostCommonBeatLength() => PlayableBeatmap.GetMostCommonBeatLength(); - public int SnapTimeForDivisor(double time, int beatDivisor, double? referenceTime = null) + public int ClosestSnapTime(double time, int beatDivisor, double? referenceTime = null) { - return PlayableBeatmap.SnapTimeForDivisor(time, beatDivisor, referenceTime); + return PlayableBeatmap.ClosestSnapTime(time, beatDivisor, referenceTime); } - public int SnapTimeAnyDivisor(double time, double? referenceTime = null) => PlayableBeatmap.SnapTimeAnyDivisor(time, referenceTime); + public int ClosestSnapTime(double time, double? referenceTime = null) => PlayableBeatmap.ClosestSnapTime(time, referenceTime); - public int ClosestBeatSnapDivisor(double time, double? referenceTime = null) => PlayableBeatmap.ClosestBeatSnapDivisor(time, referenceTime); + public int ClosestBeatDivisor(double time, double? referenceTime = null) => PlayableBeatmap.ClosestBeatDivisor(time, referenceTime); public IBeatmap Clone() => PlayableBeatmap.Clone(); From 9b9c473616c6d29a550881e17b9f94cc6271ade8 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 26 Apr 2021 16:17:38 +0200 Subject: [PATCH 05/25] Remove redundant string formatting --- osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs b/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs index ca268652a9..ff270b6d60 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs @@ -65,7 +65,7 @@ namespace osu.Game.Rulesets.Edit.Checks public abstract class IssueTemplateUnsnap : IssueTemplate { protected IssueTemplateUnsnap(ICheck check, IssueType type) - : base(check, type, "{0:0.##} is unsnapped by {1:0.##} ms.") + : base(check, type, "{0} is unsnapped by {1:0.##} ms.") { } From 71f880aa94eaf3f9defd3b943c1f949293c32631 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 26 Apr 2021 17:44:46 +0200 Subject: [PATCH 06/25] Fix duplicate code in unsnap test --- osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs b/osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs index 88939c43ce..bac3c41cb0 100644 --- a/osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs @@ -70,14 +70,9 @@ namespace osu.Game.Tests.Editing.Checks public void TestSliderSnapped() { // Slider ends are naturally < 1 ms unsnapped because of how SV works. - var mockSlider = new Mock(); - mockSlider.SetupGet(s => s.StartTime).Returns(100); - mockSlider.As().Setup(r => r.RepeatCount).Returns(0); - mockSlider.As().Setup(d => d.Duration).Returns(400.75d); - assertOk(new List { - mockSlider.Object + getSliderMock(startTime: 100, endTime: 400.75d).Object }); } From a3570e18dd3c860b0a3942d5c0ba3cf8593471c8 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 26 Apr 2021 20:17:18 +0200 Subject: [PATCH 07/25] Add concurrent objects check Here we use `IHasColumn` to support rulesets with columns, and so I moved that interface out into `osu.Game` from `osu.Game.Rulesets.Mania`. We also use the same threshold as the unsnap check to ensure that no problems slip through. Specifically where an object is simultaneously not concurrent and not unsnapped but still on the same tick. --- .../Objects/ManiaHitObject.cs | 1 - .../Edit/Checks/CheckConcurrentObjects.cs | 88 +++++++++++++++++++ osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs | 4 +- .../Rulesets}/Objects/Types/IHasColumn.cs | 2 +- 4 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs rename {osu.Game.Rulesets.Mania => osu.Game/Rulesets}/Objects/Types/IHasColumn.cs (90%) diff --git a/osu.Game.Rulesets.Mania/Objects/ManiaHitObject.cs b/osu.Game.Rulesets.Mania/Objects/ManiaHitObject.cs index 27bf50493d..6289744df1 100644 --- a/osu.Game.Rulesets.Mania/Objects/ManiaHitObject.cs +++ b/osu.Game.Rulesets.Mania/Objects/ManiaHitObject.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Bindables; -using osu.Game.Rulesets.Mania.Objects.Types; using osu.Game.Rulesets.Mania.Scoring; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; diff --git a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs new file mode 100644 index 0000000000..7c41569fab --- /dev/null +++ b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs @@ -0,0 +1,88 @@ +// 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 osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit.Checks.Components; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; + +namespace osu.Game.Rulesets.Edit.Checks +{ + public class CheckConcurrentObjects : ICheck + { + // We guarantee that the objects are either treated as concurrent or unsnapped when near the same beat divisor. + private const double ms_leniency = CheckUnsnaps.UNSNAP_MS_THRESHOLD; + + public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Concurrent hitobjects"); + + public virtual IEnumerable PossibleTemplates => new IssueTemplate[] + { + new IssueTemplateConcurrentSame(this), + new IssueTemplateConcurrentDifferent(this) + }; + + public virtual IEnumerable Run(IBeatmap playableBeatmap, IWorkingBeatmap workingBeatmap) + { + for (int i = 0; i < playableBeatmap.HitObjects.Count - 1; ++i) + { + var hitobject = playableBeatmap.HitObjects[i]; + + for (int j = i + 1; j < playableBeatmap.HitObjects.Count; ++j) + { + var nextHitobject = playableBeatmap.HitObjects[j]; + + // Accounts for rulesets with hitobjects separated by columns, such as Mania. + // In these cases we only care about concurrent objects within the same column. + if ((hitobject as IHasColumn)?.Column != (nextHitobject as IHasColumn)?.Column) + continue; + + // Two hitobjects cannot be concurrent without also being concurrent with all objects in between. + // So if the next object is not concurrent, then we know no future objects will be either. + if (!areConcurrent(hitobject, nextHitobject)) + break; + + if (hitobject.GetType() == nextHitobject.GetType()) + yield return new IssueTemplateConcurrentSame(this).Create(hitobject, nextHitobject); + else + yield return new IssueTemplateConcurrentDifferent(this).Create(hitobject, nextHitobject); + } + } + } + + protected bool areConcurrent(HitObject hitobject, HitObject nextHitobject) => nextHitobject.StartTime <= hitobject.GetEndTime() + ms_leniency; + + public abstract class IssueTemplateConcurrent : IssueTemplate + { + protected IssueTemplateConcurrent(ICheck check, string unformattedMessage) + : base(check, IssueType.Problem, unformattedMessage) + { + } + + public Issue Create(HitObject hitobject, HitObject nextHitobject) + { + var hitobjects = new List { hitobject, nextHitobject }; + return new Issue(hitobjects, this, hitobject.GetType().Name, nextHitobject.GetType().Name) + { + Time = nextHitobject.StartTime + }; + } + } + + public class IssueTemplateConcurrentSame : IssueTemplateConcurrent + { + public IssueTemplateConcurrentSame(ICheck check) + : base(check, "{0}s are concurrent here.") + { + } + } + + public class IssueTemplateConcurrentDifferent : IssueTemplateConcurrent + { + public IssueTemplateConcurrentDifferent(ICheck check) + : base(check, "{0} and {1} are concurrent here.") + { + } + } + } +} diff --git a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs b/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs index ff270b6d60..564ef13d8f 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets.Edit.Checks { public class CheckUnsnaps : ICheck { - private const double unsnap_ms_threshold = 2; + public const double UNSNAP_MS_THRESHOLD = 2; public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Unsnapped hitobjects"); @@ -54,7 +54,7 @@ namespace osu.Game.Rulesets.Edit.Checks private IEnumerable getUnsnapIssues(HitObject hitobject, double unsnap, double time, string postfix = "") { - if (Math.Abs(unsnap) >= unsnap_ms_threshold) + if (Math.Abs(unsnap) >= UNSNAP_MS_THRESHOLD) yield return new IssueTemplate2MsOrMore(this).Create(hitobject, unsnap, time, postfix); else if (Math.Abs(unsnap) >= 1) yield return new IssueTemplate1MsOrMore(this).Create(hitobject, unsnap, time, postfix); diff --git a/osu.Game.Rulesets.Mania/Objects/Types/IHasColumn.cs b/osu.Game/Rulesets/Objects/Types/IHasColumn.cs similarity index 90% rename from osu.Game.Rulesets.Mania/Objects/Types/IHasColumn.cs rename to osu.Game/Rulesets/Objects/Types/IHasColumn.cs index 1ea3138828..dc07cfbb6a 100644 --- a/osu.Game.Rulesets.Mania/Objects/Types/IHasColumn.cs +++ b/osu.Game/Rulesets/Objects/Types/IHasColumn.cs @@ -1,7 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -namespace osu.Game.Rulesets.Mania.Objects.Types +namespace osu.Game.Rulesets.Objects.Types { /// /// A type of hit object which lies in one of a number of predetermined columns. From b8cdcf56c03ef5f4dc27e204c7efece3ffd4572f Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 26 Apr 2021 20:22:24 +0200 Subject: [PATCH 08/25] Add concurrent object check tests --- .../Checks/CheckConcurrentObjectsTest.cs | 194 ++++++++++++++++++ 1 file changed, 194 insertions(+) create mode 100644 osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs diff --git a/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs b/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs new file mode 100644 index 0000000000..0f771427ee --- /dev/null +++ b/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs @@ -0,0 +1,194 @@ +// 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 Moq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit.Checks; +using osu.Game.Rulesets.Mania.Objects; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; +using osu.Game.Rulesets.Osu.Objects; + +namespace osu.Game.Tests.Editing.Checks +{ + [TestFixture] + public class CheckConcurrentObjectsTest + { + private CheckConcurrentObjects check; + + [SetUp] + public void Setup() + { + check = new CheckConcurrentObjects(); + } + + [Test] + public void TestCirclesSeparate() + { + assertOk(new List + { + new HitCircle { StartTime = 100 }, + new HitCircle { StartTime = 150 } + }); + } + + [Test] + public void TestCirclesConcurrent() + { + assertConcurrentSame(new List + { + new HitCircle { StartTime = 100 }, + new HitCircle { StartTime = 100 } + }); + } + + [Test] + public void TestCirclesAlmostConcurrent() + { + assertConcurrentSame(new List + { + new HitCircle { StartTime = 100 }, + new HitCircle { StartTime = 101 } + }); + } + + [Test] + public void TestSlidersSeparate() + { + assertOk(new List + { + getSliderMock(startTime: 100, endTime: 400.75d).Object, + getSliderMock(startTime: 500, endTime: 900.75d).Object + }); + } + + [Test] + public void TestSlidersConcurrent() + { + assertConcurrentSame(new List + { + getSliderMock(startTime: 100, endTime: 400.75d).Object, + getSliderMock(startTime: 300, endTime: 700.75d).Object + }); + } + + [Test] + public void TestSlidersAlmostConcurrent() + { + assertConcurrentSame(new List + { + getSliderMock(startTime: 100, endTime: 400.75d).Object, + getSliderMock(startTime: 402, endTime: 902.75d).Object + }); + } + + [Test] + public void TestSliderAndCircleConcurrent() + { + assertConcurrentDifferent(new List + { + getSliderMock(startTime: 100, endTime: 400.75d).Object, + new HitCircle { StartTime = 300 } + }); + } + + [Test] + public void TestManyObjectsConcurrent() + { + var hitobjects = new List + { + getSliderMock(startTime: 100, endTime: 400.75d).Object, + new HitCircle { StartTime = 300 }, + getSliderMock(startTime: 200, endTime: 500.75d).Object + }; + + var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); + + Assert.That(issues, Has.Count.EqualTo(3)); + Assert.That(issues.Where(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrentDifferent).ToList(), Has.Count.EqualTo(2)); + Assert.That(issues.Any(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrentSame)); + } + + [Test] + public void TestHoldNotesSeparateOnSameColumn() + { + assertOk(new List + { + getHoldNoteMock(startTime: 100, endTime: 400.75d, column: 1).Object, + getHoldNoteMock(startTime: 500, endTime: 900.75d, column: 1).Object + }); + } + + [Test] + public void TestHoldNotesConcurrentOnDifferentColumns() + { + assertOk(new List + { + getHoldNoteMock(startTime: 100, endTime: 400.75d, column: 1).Object, + getHoldNoteMock(startTime: 300, endTime: 700.75d, column: 2).Object + }); + } + + [Test] + public void TestHoldNotesConcurrentOnSameColumn() + { + assertConcurrentSame(new List + { + getHoldNoteMock(startTime: 100, endTime: 400.75d, column: 1).Object, + getHoldNoteMock(startTime: 300, endTime: 700.75d, column: 1).Object + }); + } + + private Mock getSliderMock(double startTime, double endTime, int repeats = 0) + { + var mock = new Mock(); + mock.SetupGet(s => s.StartTime).Returns(startTime); + mock.As().Setup(r => r.RepeatCount).Returns(repeats); + mock.As().Setup(d => d.EndTime).Returns(endTime); + + return mock; + } + + private Mock getHoldNoteMock(double startTime, double endTime, int column) + { + var mock = new Mock(); + mock.SetupGet(s => s.StartTime).Returns(startTime); + mock.As().Setup(d => d.EndTime).Returns(endTime); + mock.As().Setup(c => c.Column).Returns(column); + + return mock; + } + + private void assertOk(List hitobjects) + { + Assert.That(check.Run(getPlayableBeatmap(hitobjects), null), Is.Empty); + } + + private void assertConcurrentSame(List hitobjects, int count = 1) + { + var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); + + Assert.That(issues, Has.Count.EqualTo(count)); + Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrentSame)); + } + + private void assertConcurrentDifferent(List hitobjects, int count = 1) + { + var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); + + Assert.That(issues, Has.Count.EqualTo(count)); + Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrentDifferent)); + } + + private IBeatmap getPlayableBeatmap(List hitobjects) + { + return new Beatmap + { + HitObjects = hitobjects + }; + } + } +} From b9e4f73f78fa5bf3cdacefdd10d3a186079913be Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 26 Apr 2021 20:28:59 +0200 Subject: [PATCH 09/25] Add concurrent objects check to `BeatmapVerifier` --- osu.Game/Rulesets/Edit/BeatmapVerifier.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs index aa3459a01a..6754d62a11 100644 --- a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs +++ b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs @@ -25,7 +25,8 @@ namespace osu.Game.Rulesets.Edit new CheckAudioQuality(), // Compose - new CheckUnsnaps() + new CheckUnsnaps(), + new CheckConcurrentObjects() }; public IEnumerable Run(IBeatmap playableBeatmap, WorkingBeatmap workingBeatmap) From ce258febf6de3ec3e3e1b53cdd240107ca46028c Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Mon, 26 Apr 2021 20:32:44 +0200 Subject: [PATCH 10/25] Rename `CheckUnsnaps` -> `CheckUnsnappedObjects` Will potentially have `CheckUnsnappedKiai` or similar later, so this is worth specifying. Also consistent with `CheckConcurrentObjects`, which will likely have a `CheckConcurrentLines` later. --- ...UnsnapsTest.cs => CheckUnsnappedObjectsTest.cs} | 14 +++++++------- osu.Game/Rulesets/Edit/BeatmapVerifier.cs | 2 +- .../Rulesets/Edit/Checks/CheckConcurrentObjects.cs | 2 +- .../{CheckUnsnaps.cs => CheckUnsnappedObjects.cs} | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) rename osu.Game.Tests/Editing/Checks/{CheckUnsnapsTest.cs => CheckUnsnappedObjectsTest.cs} (93%) rename osu.Game/Rulesets/Edit/Checks/{CheckUnsnaps.cs => CheckUnsnappedObjects.cs} (98%) diff --git a/osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs b/osu.Game.Tests/Editing/Checks/CheckUnsnappedObjectsTest.cs similarity index 93% rename from osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs rename to osu.Game.Tests/Editing/Checks/CheckUnsnappedObjectsTest.cs index bac3c41cb0..f8cac331bc 100644 --- a/osu.Game.Tests/Editing/Checks/CheckUnsnapsTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckUnsnappedObjectsTest.cs @@ -15,15 +15,15 @@ using osu.Game.Rulesets.Osu.Objects; namespace osu.Game.Tests.Editing.Checks { [TestFixture] - public class CheckUnsnapsTest + public class CheckUnsnappedObjectsTest { - private CheckUnsnaps check; + private CheckUnsnappedObjects check; private ControlPointInfo cpi; [SetUp] public void Setup() { - check = new CheckUnsnaps(); + check = new CheckUnsnappedObjects(); cpi = new ControlPointInfo(); cpi.Add(100, new TimingControlPoint { BeatLength = 100 }); @@ -108,8 +108,8 @@ namespace osu.Game.Tests.Editing.Checks var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); Assert.That(issues, Has.Count.EqualTo(2)); - Assert.That(issues.Any(issue => issue.Template is CheckUnsnaps.IssueTemplate1MsOrMore)); - Assert.That(issues.Any(issue => issue.Template is CheckUnsnaps.IssueTemplate2MsOrMore)); + Assert.That(issues.Any(issue => issue.Template is CheckUnsnappedObjects.IssueTemplate1MsOrMore)); + Assert.That(issues.Any(issue => issue.Template is CheckUnsnappedObjects.IssueTemplate2MsOrMore)); } private Mock getSliderMock(double startTime, double endTime, int repeats = 0) @@ -132,7 +132,7 @@ namespace osu.Game.Tests.Editing.Checks var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); Assert.That(issues, Has.Count.EqualTo(count)); - Assert.That(issues.All(issue => issue.Template is CheckUnsnaps.IssueTemplate1MsOrMore)); + Assert.That(issues.All(issue => issue.Template is CheckUnsnappedObjects.IssueTemplate1MsOrMore)); } private void assert2Ms(List hitobjects, int count = 1) @@ -140,7 +140,7 @@ namespace osu.Game.Tests.Editing.Checks var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); Assert.That(issues, Has.Count.EqualTo(count)); - Assert.That(issues.All(issue => issue.Template is CheckUnsnaps.IssueTemplate2MsOrMore)); + Assert.That(issues.All(issue => issue.Template is CheckUnsnappedObjects.IssueTemplate2MsOrMore)); } private IBeatmap getPlayableBeatmap(List hitobjects) diff --git a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs index 6754d62a11..2f7b7b0ab8 100644 --- a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs +++ b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs @@ -25,7 +25,7 @@ namespace osu.Game.Rulesets.Edit new CheckAudioQuality(), // Compose - new CheckUnsnaps(), + new CheckUnsnappedObjects(), new CheckConcurrentObjects() }; diff --git a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs index 7c41569fab..bcc8fead18 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets.Edit.Checks public class CheckConcurrentObjects : ICheck { // We guarantee that the objects are either treated as concurrent or unsnapped when near the same beat divisor. - private const double ms_leniency = CheckUnsnaps.UNSNAP_MS_THRESHOLD; + private const double ms_leniency = CheckUnsnappedObjects.UNSNAP_MS_THRESHOLD; public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Concurrent hitobjects"); diff --git a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs b/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs similarity index 98% rename from osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs rename to osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs index 564ef13d8f..cdcf8a6b80 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckUnsnaps.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs @@ -10,7 +10,7 @@ using osu.Game.Rulesets.Objects.Types; namespace osu.Game.Rulesets.Edit.Checks { - public class CheckUnsnaps : ICheck + public class CheckUnsnappedObjects : ICheck { public const double UNSNAP_MS_THRESHOLD = 2; From 0f0870c8b875451c2f7ce3e724fb63ee05b776ec Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 27 Apr 2021 00:36:26 +0200 Subject: [PATCH 11/25] Sort objects by time in concurrent check test --- osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs b/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs index 0f771427ee..ffe5d34e67 100644 --- a/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs @@ -101,8 +101,8 @@ namespace osu.Game.Tests.Editing.Checks var hitobjects = new List { getSliderMock(startTime: 100, endTime: 400.75d).Object, - new HitCircle { StartTime = 300 }, - getSliderMock(startTime: 200, endTime: 500.75d).Object + getSliderMock(startTime: 200, endTime: 500.75d).Object, + new HitCircle { StartTime = 300 } }; var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); From 6d5883abcb6293c54a362c7e765e07a8441b70fd Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 27 Apr 2021 01:19:38 +0200 Subject: [PATCH 12/25] Return result of local variable instead --- osu.Game/Beatmaps/Beatmap.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/Beatmap.cs b/osu.Game/Beatmaps/Beatmap.cs index 66b8f169ef..e3a11e2326 100644 --- a/osu.Game/Beatmaps/Beatmap.cs +++ b/osu.Game/Beatmaps/Beatmap.cs @@ -95,9 +95,8 @@ namespace osu.Game.Beatmaps int[] divisors = BindableBeatDivisor.VALID_DIVISORS; double smallestUnsnap = divisors.Min(getUnsnap); - int closestDivisor = divisors.FirstOrDefault(divisor => getUnsnap(divisor) == smallestUnsnap); - return closestDivisor; + return divisors.FirstOrDefault(divisor => getUnsnap(divisor) == smallestUnsnap); } IBeatmap IBeatmap.Clone() => Clone(); From 217ff8238ea50abeea0ccb62319cd785a91d39f3 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 27 Apr 2021 01:23:03 +0200 Subject: [PATCH 13/25] Add snapping time comment --- osu.Game/Beatmaps/Beatmap.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Beatmaps/Beatmap.cs b/osu.Game/Beatmaps/Beatmap.cs index e3a11e2326..6515540527 100644 --- a/osu.Game/Beatmaps/Beatmap.cs +++ b/osu.Game/Beatmaps/Beatmap.cs @@ -81,6 +81,7 @@ namespace osu.Game.Beatmaps var beatLength = timingPoint.BeatLength / beatDivisor; var beatLengths = (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero); + // Casting to int matches stable. return (int)(timingPoint.Time + beatLengths * beatLength); } From a3c1b1fd52d1a74df5df536d05df6550852c9ab5 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 27 Apr 2021 01:24:38 +0200 Subject: [PATCH 14/25] Fix accessibility of `areConcurrent` --- osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs index bcc8fead18..6e8355024e 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs @@ -50,7 +50,7 @@ namespace osu.Game.Rulesets.Edit.Checks } } - protected bool areConcurrent(HitObject hitobject, HitObject nextHitobject) => nextHitobject.StartTime <= hitobject.GetEndTime() + ms_leniency; + private bool areConcurrent(HitObject hitobject, HitObject nextHitobject) => nextHitobject.StartTime <= hitobject.GetEndTime() + ms_leniency; public abstract class IssueTemplateConcurrent : IssueTemplate { From 9e49ecb57311b3d8e83c9c8655cca8e13e0342bb Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 27 Apr 2021 02:23:06 +0200 Subject: [PATCH 15/25] Remove unused `virtual` keywords Added these in a previous iteration, where I had the mania variant inherit this class. No longer necessary as `IHasColumn` was used to make this check more generic. --- osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs index 6e8355024e..ddebe2923a 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs @@ -16,13 +16,13 @@ namespace osu.Game.Rulesets.Edit.Checks public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Concurrent hitobjects"); - public virtual IEnumerable PossibleTemplates => new IssueTemplate[] + public IEnumerable PossibleTemplates => new IssueTemplate[] { new IssueTemplateConcurrentSame(this), new IssueTemplateConcurrentDifferent(this) }; - public virtual IEnumerable Run(IBeatmap playableBeatmap, IWorkingBeatmap workingBeatmap) + public IEnumerable Run(IBeatmap playableBeatmap, IWorkingBeatmap workingBeatmap) { for (int i = 0; i < playableBeatmap.HitObjects.Count - 1; ++i) { From 7a6e9e5070b63d7b1563108f170ca676994e8b5f Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 27 Apr 2021 02:32:57 +0200 Subject: [PATCH 16/25] Change category of unsnap check to timing Makes more sense, as this is typically the result of timing changes. --- osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs index cdcf8a6b80..74a2ce2fd7 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs @@ -14,7 +14,7 @@ namespace osu.Game.Rulesets.Edit.Checks { public const double UNSNAP_MS_THRESHOLD = 2; - public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Unsnapped hitobjects"); + public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Timing, "Unsnapped hitobjects"); public IEnumerable PossibleTemplates => new IssueTemplate[] { From 200352b7507cb234f6fce5f287e06492caf87606 Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 27 Apr 2021 13:56:05 +0200 Subject: [PATCH 17/25] Rename unsnap check templates --- .../Editing/Checks/CheckUnsnappedObjectsTest.cs | 8 ++++---- .../Edit/Checks/CheckUnsnappedObjects.cs | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/osu.Game.Tests/Editing/Checks/CheckUnsnappedObjectsTest.cs b/osu.Game.Tests/Editing/Checks/CheckUnsnappedObjectsTest.cs index f8cac331bc..5e65b263f2 100644 --- a/osu.Game.Tests/Editing/Checks/CheckUnsnappedObjectsTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckUnsnappedObjectsTest.cs @@ -108,8 +108,8 @@ namespace osu.Game.Tests.Editing.Checks var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); Assert.That(issues, Has.Count.EqualTo(2)); - Assert.That(issues.Any(issue => issue.Template is CheckUnsnappedObjects.IssueTemplate1MsOrMore)); - Assert.That(issues.Any(issue => issue.Template is CheckUnsnappedObjects.IssueTemplate2MsOrMore)); + Assert.That(issues.Any(issue => issue.Template is CheckUnsnappedObjects.IssueTemplateSmallUnsnap)); + Assert.That(issues.Any(issue => issue.Template is CheckUnsnappedObjects.IssueTemplateLargeUnsnap)); } private Mock getSliderMock(double startTime, double endTime, int repeats = 0) @@ -132,7 +132,7 @@ namespace osu.Game.Tests.Editing.Checks var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); Assert.That(issues, Has.Count.EqualTo(count)); - Assert.That(issues.All(issue => issue.Template is CheckUnsnappedObjects.IssueTemplate1MsOrMore)); + Assert.That(issues.All(issue => issue.Template is CheckUnsnappedObjects.IssueTemplateSmallUnsnap)); } private void assert2Ms(List hitobjects, int count = 1) @@ -140,7 +140,7 @@ namespace osu.Game.Tests.Editing.Checks var issues = check.Run(getPlayableBeatmap(hitobjects), null).ToList(); Assert.That(issues, Has.Count.EqualTo(count)); - Assert.That(issues.All(issue => issue.Template is CheckUnsnappedObjects.IssueTemplate2MsOrMore)); + Assert.That(issues.All(issue => issue.Template is CheckUnsnappedObjects.IssueTemplateLargeUnsnap)); } private IBeatmap getPlayableBeatmap(List hitobjects) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs index 74a2ce2fd7..8b6bb7d461 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs @@ -18,8 +18,8 @@ namespace osu.Game.Rulesets.Edit.Checks public IEnumerable PossibleTemplates => new IssueTemplate[] { - new IssueTemplate2MsOrMore(this), - new IssueTemplate1MsOrMore(this) + new IssueTemplateLargeUnsnap(this), + new IssueTemplateSmallUnsnap(this) }; public IEnumerable Run(IBeatmap playableBeatmap, IWorkingBeatmap workingBeatmap) @@ -55,9 +55,9 @@ namespace osu.Game.Rulesets.Edit.Checks private IEnumerable getUnsnapIssues(HitObject hitobject, double unsnap, double time, string postfix = "") { if (Math.Abs(unsnap) >= UNSNAP_MS_THRESHOLD) - yield return new IssueTemplate2MsOrMore(this).Create(hitobject, unsnap, time, postfix); + yield return new IssueTemplateLargeUnsnap(this).Create(hitobject, unsnap, time, postfix); else if (Math.Abs(unsnap) >= 1) - yield return new IssueTemplate1MsOrMore(this).Create(hitobject, unsnap, time, postfix); + yield return new IssueTemplateSmallUnsnap(this).Create(hitobject, unsnap, time, postfix); // We don't care about unsnaps < 1 ms, as all object ends have these due to the way SV works. } @@ -79,17 +79,17 @@ namespace osu.Game.Rulesets.Edit.Checks } } - public class IssueTemplate2MsOrMore : IssueTemplateUnsnap + public class IssueTemplateLargeUnsnap : IssueTemplateUnsnap { - public IssueTemplate2MsOrMore(ICheck check) + public IssueTemplateLargeUnsnap(ICheck check) : base(check, IssueType.Problem) { } } - public class IssueTemplate1MsOrMore : IssueTemplateUnsnap + public class IssueTemplateSmallUnsnap : IssueTemplateUnsnap { - public IssueTemplate1MsOrMore(ICheck check) + public IssueTemplateSmallUnsnap(ICheck check) : base(check, IssueType.Negligible) { } From b8b6d0e861bd0269b841ceacfaab3c361128bc3b Mon Sep 17 00:00:00 2001 From: Naxess <30292137+Naxesss@users.noreply.github.com> Date: Tue, 27 Apr 2021 16:54:47 +0200 Subject: [PATCH 18/25] Add tests for `ClosestBeatDivisor` Used https://github.com/ppy/osu/pull/12558/files#diff-5c1f04c5b262ca3abbaf867aa91b62a60b66691323c286ad5aa0b75c153cc6ca as reference. --- .../NonVisual/ClosestBeatDivisorTest.cs | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs diff --git a/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs b/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs new file mode 100644 index 0000000000..4d6986f5d2 --- /dev/null +++ b/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs @@ -0,0 +1,91 @@ +// 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 NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Rulesets.Objects; + +namespace osu.Game.Tests.NonVisual +{ + public class ClosestBeatDivisorTest + { + [Test] + public void TestExactDivisors() + { + var cpi = new ControlPointInfo(); + cpi.Add(-1000, new TimingControlPoint { BeatLength = 1000 }); + + double[] divisors = { 3, 1, 16, 12, 8, 6, 4, 3, 2, 1 }; + + assertClosestDivisors(divisors, divisors, cpi); + } + + [Test] + public void TestExactDivisorWithTempoChanges() + { + int offset = 0; + int[] beatLengths = { 1000, 200, 100, 50 }; + + var cpi = new ControlPointInfo(); + + foreach (int beatLength in beatLengths) + { + cpi.Add(offset, new TimingControlPoint { BeatLength = beatLength }); + offset += beatLength * 2; + } + + double[] divisors = { 3, 1, 16, 12, 8, 6, 4, 3 }; + + assertClosestDivisors(divisors, divisors, cpi); + } + + [Test] + public void TestExactDivisorsHighBPMStream() + { + var cpi = new ControlPointInfo(); + cpi.Add(-50, new TimingControlPoint { BeatLength = 50 }); // 1200 BPM 1/4 (limit testing) + + // A 1/4 stream should land on 1/1, 1/2 and 1/4 divisors. + double[] divisors = { 4, 4, 4, 4, 4, 4, 4, 4 }; + double[] closestDivisors = { 4, 2, 4, 1, 4, 2, 4, 1 }; + + assertClosestDivisors(divisors, closestDivisors, cpi, step: 1 / 4d); + } + + [Test] + public void TestApproximateDivisors() + { + var cpi = new ControlPointInfo(); + cpi.Add(-1000, new TimingControlPoint { BeatLength = 1000 }); + + double[] divisors = { 3.03d, 0.97d, 14, 13, 7.94d, 6.08d, 3.93d, 2.96d, 2.02d, 64 }; + double[] closestDivisors = { 3, 1, 16, 12, 8, 6, 4, 3, 2, 1 }; + + assertClosestDivisors(divisors, closestDivisors, cpi); + } + + private void assertClosestDivisors(IReadOnlyList divisors, IReadOnlyList closestDivisors, ControlPointInfo cpi, double step = 1) + { + List hitobjects = new List(); + double offset = cpi.TimingPoints[0].Time; + + for (int i = 0; i < divisors.Count; ++i) + { + double beatLength = cpi.TimingPointAt(offset).BeatLength; + hitobjects.Add(new HitObject { StartTime = offset + beatLength / divisors[i] }); + offset += beatLength * step; + } + + var beatmap = new Beatmap + { + HitObjects = hitobjects, + ControlPointInfo = cpi + }; + + for (int i = 0; i < divisors.Count; ++i) + Assert.AreEqual(closestDivisors[i], beatmap.ClosestBeatDivisor(beatmap.HitObjects[i].StartTime), $"at index {i}"); + } + } +} From 48d6c9ac4bee70169ac982611545c9d276126988 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Apr 2021 16:47:30 +0900 Subject: [PATCH 19/25] Move snap/divisor helper methods to inside `ControlPointInfo` --- .../NonVisual/ClosestBeatDivisorTest.cs | 2 +- osu.Game/Beatmaps/Beatmap.cs | 26 ------------ .../ControlPoints/ControlPointInfo.cs | 42 +++++++++++++++++++ osu.Game/Beatmaps/IBeatmap.cs | 22 ---------- .../Edit/Checks/CheckUnsnappedObjects.cs | 8 ++-- osu.Game/Screens/Edit/EditorBeatmap.cs | 11 +---- osu.Game/Screens/Play/GameplayBeatmap.cs | 9 ---- 7 files changed, 49 insertions(+), 71 deletions(-) diff --git a/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs b/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs index 4d6986f5d2..5ac121f5bc 100644 --- a/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs +++ b/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs @@ -85,7 +85,7 @@ namespace osu.Game.Tests.NonVisual }; for (int i = 0; i < divisors.Count; ++i) - Assert.AreEqual(closestDivisors[i], beatmap.ClosestBeatDivisor(beatmap.HitObjects[i].StartTime), $"at index {i}"); + Assert.AreEqual(closestDivisors[i], beatmap.ControlPointInfo.ClosestBeatDivisor(beatmap.HitObjects[i].StartTime), $"at index {i}"); } } } diff --git a/osu.Game/Beatmaps/Beatmap.cs b/osu.Game/Beatmaps/Beatmap.cs index 6515540527..e5b6a4bc44 100644 --- a/osu.Game/Beatmaps/Beatmap.cs +++ b/osu.Game/Beatmaps/Beatmap.cs @@ -9,7 +9,6 @@ using System.Linq; using osu.Game.Beatmaps.ControlPoints; using Newtonsoft.Json; using osu.Game.IO.Serialization.Converters; -using osu.Game.Screens.Edit; namespace osu.Game.Beatmaps { @@ -75,31 +74,6 @@ namespace osu.Game.Beatmaps return mostCommon.beatLength; } - public int ClosestSnapTime(double time, int beatDivisor, double? referenceTime = null) - { - var timingPoint = ControlPointInfo.TimingPointAt(referenceTime ?? time); - var beatLength = timingPoint.BeatLength / beatDivisor; - var beatLengths = (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero); - - // Casting to int matches stable. - return (int)(timingPoint.Time + beatLengths * beatLength); - } - - public int ClosestSnapTime(double time, double? referenceTime = null) - { - return ClosestSnapTime(time, ClosestBeatDivisor(time, referenceTime), referenceTime); - } - - public int ClosestBeatDivisor(double time, double? referenceTime = null) - { - double getUnsnap(int divisor) => Math.Abs(time - ClosestSnapTime(time, divisor, referenceTime)); - - int[] divisors = BindableBeatDivisor.VALID_DIVISORS; - double smallestUnsnap = divisors.Min(getUnsnap); - - return divisors.FirstOrDefault(divisor => getUnsnap(divisor) == smallestUnsnap); - } - IBeatmap IBeatmap.Clone() => Clone(); public Beatmap Clone() => (Beatmap)MemberwiseClone(); diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index 5cc60a5758..d1a04061b9 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -7,6 +7,7 @@ using System.Linq; using Newtonsoft.Json; using osu.Framework.Bindables; using osu.Framework.Lists; +using osu.Game.Screens.Edit; namespace osu.Game.Beatmaps.ControlPoints { @@ -160,6 +161,47 @@ namespace osu.Game.Beatmaps.ControlPoints groups.Remove(group); } + /// + /// Returns the time on the given beat divisor closest to the given time. + /// + /// The time to find the closest snapped time to. + /// The beat divisor to snap to. + /// An optional reference point to use for timing point lookup. + public int ClosestSnapTime(double time, int beatDivisor, double? referenceTime = null) + { + var timingPoint = TimingPointAt(referenceTime ?? time); + var beatLength = timingPoint.BeatLength / beatDivisor; + var beatLengths = (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero); + + // Casting to int matches stable. + return (int)(timingPoint.Time + beatLengths * beatLength); + } + + /// + /// Returns the time on any valid beat divisor closest to the given time. + /// + /// The time to find the closest snapped time to. + /// An optional reference point to use for timing point lookup. + public int ClosestSnapTime(double time, double? referenceTime = null) + { + return ClosestSnapTime(time, ClosestBeatDivisor(time, referenceTime), referenceTime); + } + + /// + /// Returns the beat snap divisor closest to the given time. If two are equally close, the smallest is returned. + /// + /// The time to find the closest beat snap divisor to. + /// An optional reference point to use for timing point lookup. + public int ClosestBeatDivisor(double time, double? referenceTime = null) + { + double getUnsnap(int divisor) => Math.Abs(time - ClosestSnapTime(time, divisor, referenceTime)); + + int[] divisors = BindableBeatDivisor.VALID_DIVISORS; + double smallestUnsnap = divisors.Min(getUnsnap); + + return divisors.FirstOrDefault(divisor => getUnsnap(divisor) == smallestUnsnap); + } + /// /// Binary searches one of the control point lists to find the active control point at . /// Includes logic for returning a specific point when no matching point is found. diff --git a/osu.Game/Beatmaps/IBeatmap.cs b/osu.Game/Beatmaps/IBeatmap.cs index 679d639fd1..769b33009a 100644 --- a/osu.Game/Beatmaps/IBeatmap.cs +++ b/osu.Game/Beatmaps/IBeatmap.cs @@ -51,28 +51,6 @@ namespace osu.Game.Beatmaps /// double GetMostCommonBeatLength(); - /// - /// Returns the time on the given beat divisor closest to the given time. - /// - /// The time to find the closest snapped time to. - /// The beat divisor to snap to. - /// An optional reference point to use for timing point lookup. - int ClosestSnapTime(double time, int beatDivisor, double? referenceTime = null); - - /// - /// Returns the time on any valid beat divisor closest to the given time. - /// - /// The time to find the closest snapped time to. - /// An optional reference point to use for timing point lookup. - int ClosestSnapTime(double time, double? referenceTime = null); - - /// - /// Returns the beat snap divisor closest to the given time. If two are equally close, the smallest is returned. - /// - /// The time to find the closest beat snap divisor to. - /// An optional reference point to use for timing point lookup. - int ClosestBeatDivisor(double time, double? referenceTime = null); - /// /// Creates a shallow-clone of this beatmap and returns it. /// diff --git a/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs index 8b6bb7d461..cc5ea2a988 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs @@ -24,9 +24,11 @@ namespace osu.Game.Rulesets.Edit.Checks public IEnumerable Run(IBeatmap playableBeatmap, IWorkingBeatmap workingBeatmap) { + var controlPointInfo = playableBeatmap.ControlPointInfo; + foreach (var hitobject in playableBeatmap.HitObjects) { - double startUnsnap = hitobject.StartTime - playableBeatmap.ClosestSnapTime(hitobject.StartTime); + double startUnsnap = hitobject.StartTime - controlPointInfo.ClosestSnapTime(hitobject.StartTime); string startPostfix = hitobject is IHasDuration ? "start" : ""; foreach (var issue in getUnsnapIssues(hitobject, startUnsnap, hitobject.StartTime, startPostfix)) yield return issue; @@ -37,7 +39,7 @@ namespace osu.Game.Rulesets.Edit.Checks { double spanDuration = hasRepeats.Duration / (hasRepeats.RepeatCount + 1); double repeatTime = hitobject.StartTime + spanDuration * (repeatIndex + 1); - double repeatUnsnap = repeatTime - playableBeatmap.ClosestSnapTime(repeatTime); + double repeatUnsnap = repeatTime - controlPointInfo.ClosestSnapTime(repeatTime); foreach (var issue in getUnsnapIssues(hitobject, repeatUnsnap, repeatTime, "repeat")) yield return issue; } @@ -45,7 +47,7 @@ namespace osu.Game.Rulesets.Edit.Checks if (hitobject is IHasDuration hasDuration) { - double endUnsnap = hasDuration.EndTime - playableBeatmap.ClosestSnapTime(hasDuration.EndTime); + double endUnsnap = hasDuration.EndTime - controlPointInfo.ClosestSnapTime(hasDuration.EndTime); foreach (var issue in getUnsnapIssues(hitobject, endUnsnap, hasDuration.EndTime, "end")) yield return issue; } diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index 72fb0ac9e9..f1262daab3 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -301,16 +301,7 @@ namespace osu.Game.Screens.Edit return list.Count - 1; } - public int ClosestSnapTime(double time, int beatDivisor, double? referenceTime = null) - { - return PlayableBeatmap.ClosestSnapTime(time, beatDivisor, referenceTime); - } - - public int ClosestSnapTime(double time, double? referenceTime = null) => PlayableBeatmap.ClosestSnapTime(time, referenceTime); - - public int ClosestBeatDivisor(double time, double? referenceTime = null) => PlayableBeatmap.ClosestBeatDivisor(time, referenceTime); - - public double SnapTime(double time, double? referenceTime) => ClosestSnapTime(time, BeatDivisor, referenceTime); + public double SnapTime(double time, double? referenceTime) => ControlPointInfo.ClosestSnapTime(time, BeatDivisor, referenceTime); public double GetBeatLengthAtTime(double referenceTime) => ControlPointInfo.TimingPointAt(referenceTime).BeatLength / BeatDivisor; diff --git a/osu.Game/Screens/Play/GameplayBeatmap.cs b/osu.Game/Screens/Play/GameplayBeatmap.cs index 92f58c8759..74fbe540fa 100644 --- a/osu.Game/Screens/Play/GameplayBeatmap.cs +++ b/osu.Game/Screens/Play/GameplayBeatmap.cs @@ -45,15 +45,6 @@ namespace osu.Game.Screens.Play public double GetMostCommonBeatLength() => PlayableBeatmap.GetMostCommonBeatLength(); - public int ClosestSnapTime(double time, int beatDivisor, double? referenceTime = null) - { - return PlayableBeatmap.ClosestSnapTime(time, beatDivisor, referenceTime); - } - - public int ClosestSnapTime(double time, double? referenceTime = null) => PlayableBeatmap.ClosestSnapTime(time, referenceTime); - - public int ClosestBeatDivisor(double time, double? referenceTime = null) => PlayableBeatmap.ClosestBeatDivisor(time, referenceTime); - public IBeatmap Clone() => PlayableBeatmap.Clone(); private readonly Bindable lastJudgementResult = new Bindable(); From f3c7694eeb8f78cd6368fd5701e22ed291e000ca Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Apr 2021 16:57:52 +0900 Subject: [PATCH 20/25] Rename methods to match generally how these find-methods are named elsewhere --- osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs | 2 +- osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs | 10 +++++----- osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs | 6 +++--- osu.Game/Screens/Edit/EditorBeatmap.cs | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs b/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs index 5ac121f5bc..08cd80dcfa 100644 --- a/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs +++ b/osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs @@ -85,7 +85,7 @@ namespace osu.Game.Tests.NonVisual }; for (int i = 0; i < divisors.Count; ++i) - Assert.AreEqual(closestDivisors[i], beatmap.ControlPointInfo.ClosestBeatDivisor(beatmap.HitObjects[i].StartTime), $"at index {i}"); + Assert.AreEqual(closestDivisors[i], beatmap.ControlPointInfo.GetClosestBeatDivisor(beatmap.HitObjects[i].StartTime), $"at index {i}"); } } } diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index d1a04061b9..60a8a40f06 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -167,7 +167,7 @@ namespace osu.Game.Beatmaps.ControlPoints /// The time to find the closest snapped time to. /// The beat divisor to snap to. /// An optional reference point to use for timing point lookup. - public int ClosestSnapTime(double time, int beatDivisor, double? referenceTime = null) + public int GetClosestSnappedTime(double time, int beatDivisor, double? referenceTime = null) { var timingPoint = TimingPointAt(referenceTime ?? time); var beatLength = timingPoint.BeatLength / beatDivisor; @@ -182,9 +182,9 @@ namespace osu.Game.Beatmaps.ControlPoints /// /// The time to find the closest snapped time to. /// An optional reference point to use for timing point lookup. - public int ClosestSnapTime(double time, double? referenceTime = null) + public int GetClosestSnappedTime(double time, double? referenceTime = null) { - return ClosestSnapTime(time, ClosestBeatDivisor(time, referenceTime), referenceTime); + return GetClosestSnappedTime(time, GetClosestBeatDivisor(time, referenceTime), referenceTime); } /// @@ -192,9 +192,9 @@ namespace osu.Game.Beatmaps.ControlPoints /// /// The time to find the closest beat snap divisor to. /// An optional reference point to use for timing point lookup. - public int ClosestBeatDivisor(double time, double? referenceTime = null) + public int GetClosestBeatDivisor(double time, double? referenceTime = null) { - double getUnsnap(int divisor) => Math.Abs(time - ClosestSnapTime(time, divisor, referenceTime)); + double getUnsnap(int divisor) => Math.Abs(time - GetClosestSnappedTime(time, divisor, referenceTime)); int[] divisors = BindableBeatDivisor.VALID_DIVISORS; double smallestUnsnap = divisors.Min(getUnsnap); diff --git a/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs index cc5ea2a988..cdf3f05465 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs @@ -28,7 +28,7 @@ namespace osu.Game.Rulesets.Edit.Checks foreach (var hitobject in playableBeatmap.HitObjects) { - double startUnsnap = hitobject.StartTime - controlPointInfo.ClosestSnapTime(hitobject.StartTime); + double startUnsnap = hitobject.StartTime - controlPointInfo.GetClosestSnappedTime(hitobject.StartTime); string startPostfix = hitobject is IHasDuration ? "start" : ""; foreach (var issue in getUnsnapIssues(hitobject, startUnsnap, hitobject.StartTime, startPostfix)) yield return issue; @@ -39,7 +39,7 @@ namespace osu.Game.Rulesets.Edit.Checks { double spanDuration = hasRepeats.Duration / (hasRepeats.RepeatCount + 1); double repeatTime = hitobject.StartTime + spanDuration * (repeatIndex + 1); - double repeatUnsnap = repeatTime - controlPointInfo.ClosestSnapTime(repeatTime); + double repeatUnsnap = repeatTime - controlPointInfo.GetClosestSnappedTime(repeatTime); foreach (var issue in getUnsnapIssues(hitobject, repeatUnsnap, repeatTime, "repeat")) yield return issue; } @@ -47,7 +47,7 @@ namespace osu.Game.Rulesets.Edit.Checks if (hitobject is IHasDuration hasDuration) { - double endUnsnap = hasDuration.EndTime - controlPointInfo.ClosestSnapTime(hasDuration.EndTime); + double endUnsnap = hasDuration.EndTime - controlPointInfo.GetClosestSnappedTime(hasDuration.EndTime); foreach (var issue in getUnsnapIssues(hitobject, endUnsnap, hasDuration.EndTime, "end")) yield return issue; } diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index f1262daab3..be53abbd55 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -301,7 +301,7 @@ namespace osu.Game.Screens.Edit return list.Count - 1; } - public double SnapTime(double time, double? referenceTime) => ControlPointInfo.ClosestSnapTime(time, BeatDivisor, referenceTime); + public double SnapTime(double time, double? referenceTime) => ControlPointInfo.GetClosestSnappedTime(time, BeatDivisor, referenceTime); public double GetBeatLengthAtTime(double referenceTime) => ControlPointInfo.TimingPointAt(referenceTime).BeatLength / BeatDivisor; From c5186b6a693e7ff3ff1b7737a5b4f81ba0cf1124 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Apr 2021 16:59:49 +0900 Subject: [PATCH 21/25] Revert return values to non-rounded doubles --- osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index 60a8a40f06..fa1c59bb08 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -167,14 +167,13 @@ namespace osu.Game.Beatmaps.ControlPoints /// The time to find the closest snapped time to. /// The beat divisor to snap to. /// An optional reference point to use for timing point lookup. - public int GetClosestSnappedTime(double time, int beatDivisor, double? referenceTime = null) + public double GetClosestSnappedTime(double time, int beatDivisor, double? referenceTime = null) { var timingPoint = TimingPointAt(referenceTime ?? time); var beatLength = timingPoint.BeatLength / beatDivisor; var beatLengths = (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero); - // Casting to int matches stable. - return (int)(timingPoint.Time + beatLengths * beatLength); + return timingPoint.Time + beatLengths * beatLength; } /// @@ -182,7 +181,7 @@ namespace osu.Game.Beatmaps.ControlPoints /// /// The time to find the closest snapped time to. /// An optional reference point to use for timing point lookup. - public int GetClosestSnappedTime(double time, double? referenceTime = null) + public double GetClosestSnappedTime(double time, double? referenceTime = null) { return GetClosestSnappedTime(time, GetClosestBeatDivisor(time, referenceTime), referenceTime); } From 859898d98f09b9f10e928eca51206be5519b3fbd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Apr 2021 17:16:05 +0900 Subject: [PATCH 22/25] Refactor lookup methods to avoid linq and reduce `TimingPointAt` calls --- .../ControlPoints/ControlPointInfo.cs | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs index fa1c59bb08..e47d48edcf 100644 --- a/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs +++ b/osu.Game/Beatmaps/ControlPoints/ControlPointInfo.cs @@ -170,35 +170,47 @@ namespace osu.Game.Beatmaps.ControlPoints public double GetClosestSnappedTime(double time, int beatDivisor, double? referenceTime = null) { var timingPoint = TimingPointAt(referenceTime ?? time); - var beatLength = timingPoint.BeatLength / beatDivisor; - var beatLengths = (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero); - - return timingPoint.Time + beatLengths * beatLength; + return getClosestSnappedTime(timingPoint, time, beatDivisor); } /// - /// Returns the time on any valid beat divisor closest to the given time. + /// Returns the time on *ANY* valid beat divisor, favouring the divisor closest to the given time. /// /// The time to find the closest snapped time to. - /// An optional reference point to use for timing point lookup. - public double GetClosestSnappedTime(double time, double? referenceTime = null) - { - return GetClosestSnappedTime(time, GetClosestBeatDivisor(time, referenceTime), referenceTime); - } + public double GetClosestSnappedTime(double time) => GetClosestSnappedTime(time, GetClosestBeatDivisor(time)); /// - /// Returns the beat snap divisor closest to the given time. If two are equally close, the smallest is returned. + /// Returns the beat snap divisor closest to the given time. If two are equally close, the smallest divisor is returned. /// /// The time to find the closest beat snap divisor to. /// An optional reference point to use for timing point lookup. public int GetClosestBeatDivisor(double time, double? referenceTime = null) { - double getUnsnap(int divisor) => Math.Abs(time - GetClosestSnappedTime(time, divisor, referenceTime)); + TimingControlPoint timingPoint = TimingPointAt(referenceTime ?? time); - int[] divisors = BindableBeatDivisor.VALID_DIVISORS; - double smallestUnsnap = divisors.Min(getUnsnap); + int closestDivisor = 0; + double closestTime = double.MaxValue; - return divisors.FirstOrDefault(divisor => getUnsnap(divisor) == smallestUnsnap); + foreach (int divisor in BindableBeatDivisor.VALID_DIVISORS) + { + double distanceFromSnap = Math.Abs(time - getClosestSnappedTime(timingPoint, time, divisor)); + + if (distanceFromSnap < closestTime) + { + closestDivisor = divisor; + closestTime = distanceFromSnap; + } + } + + return closestDivisor; + } + + private static double getClosestSnappedTime(TimingControlPoint timingPoint, double time, int beatDivisor) + { + var beatLength = timingPoint.BeatLength / beatDivisor; + var beatLengths = (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero); + + return timingPoint.Time + beatLengths * beatLength; } /// From 126056c43626cfbe5fcb9906ceb8d8e8425029a9 Mon Sep 17 00:00:00 2001 From: PercyDan54 <50285552+PercyDan54@users.noreply.github.com> Date: Wed, 28 Apr 2021 19:27:18 +0800 Subject: [PATCH 23/25] Fix precision loss on exporting legacy replays --- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index 56c4e75864..144aeeebce 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -95,8 +95,9 @@ namespace osu.Game.Scoring.Legacy foreach (var f in score.Replay.Frames.OfType().Select(f => f.ToLegacy(beatmap))) { - replayData.Append(FormattableString.Invariant($"{(int)Math.Round(f.Time - lastF.Time)}|{f.MouseX ?? 0}|{f.MouseY ?? 0}|{(int)f.ButtonState},")); + replayData.Append(FormattableString.Invariant($"{Math.Round(f.Time - lastF.Time)}|{f.MouseX ?? 0}|{f.MouseY ?? 0}|{(int)f.ButtonState},")); lastF = f; + lastF.Time = Math.Round(f.Time); } } From 4fe1497f63ec6f65badc27505821a311690226db Mon Sep 17 00:00:00 2001 From: PercyDan54 <50285552+PercyDan54@users.noreply.github.com> Date: Wed, 28 Apr 2021 20:23:56 +0800 Subject: [PATCH 24/25] Add comment & remove lastF --- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index 144aeeebce..13876f1648 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -91,13 +91,13 @@ namespace osu.Game.Scoring.Legacy if (score.Replay != null) { - LegacyReplayFrame lastF = new LegacyReplayFrame(0, 0, 0, ReplayButtonState.None); - + int lastTimeRounded = 0; foreach (var f in score.Replay.Frames.OfType().Select(f => f.ToLegacy(beatmap))) { - replayData.Append(FormattableString.Invariant($"{Math.Round(f.Time - lastF.Time)}|{f.MouseX ?? 0}|{f.MouseY ?? 0}|{(int)f.ButtonState},")); - lastF = f; - lastF.Time = Math.Round(f.Time); + // Rounding because stable could only parse integral values + int timeRounded = (int)Math.Round(f.Time); + replayData.Append(FormattableString.Invariant($"{timeRounded - lastTimeRounded}|{f.MouseX ?? 0}|{f.MouseY ?? 0}|{(int)f.ButtonState},")); + lastTimeRounded = timeRounded; } } From e716162ac242a5a36f2b435339a54472707e9ffd Mon Sep 17 00:00:00 2001 From: PercyDan54 <50285552+PercyDan54@users.noreply.github.com> Date: Wed, 28 Apr 2021 20:55:20 +0800 Subject: [PATCH 25/25] Fix formatting --- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index 13876f1648..f8dd6953ad 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -8,7 +8,6 @@ using System.Text; using osu.Framework.Extensions; using osu.Game.Beatmaps; using osu.Game.IO.Legacy; -using osu.Game.Replays.Legacy; using osu.Game.Rulesets.Replays.Types; using SharpCompress.Compressors.LZMA; @@ -91,13 +90,14 @@ namespace osu.Game.Scoring.Legacy if (score.Replay != null) { - int lastTimeRounded = 0; + int lastTime = 0; + foreach (var f in score.Replay.Frames.OfType().Select(f => f.ToLegacy(beatmap))) { // Rounding because stable could only parse integral values - int timeRounded = (int)Math.Round(f.Time); - replayData.Append(FormattableString.Invariant($"{timeRounded - lastTimeRounded}|{f.MouseX ?? 0}|{f.MouseY ?? 0}|{(int)f.ButtonState},")); - lastTimeRounded = timeRounded; + int time = (int)Math.Round(f.Time); + replayData.Append(FormattableString.Invariant($"{time - lastTime}|{f.MouseX ?? 0}|{f.MouseY ?? 0}|{(int)f.ButtonState},")); + lastTime = time; } }