Merge pull request #12588 from Naxesss/basic-compose-checks

Add unsnapped and concurrent object checks

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;

osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs

@@ -0,0 +1,194 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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<HitObject>
+            {
+                new HitCircle { StartTime = 100 },
+                new HitCircle { StartTime = 150 }
+            });
+        }
+
+        [Test]
+        public void TestCirclesConcurrent()
+        {
+            assertConcurrentSame(new List<HitObject>
+            {
+                new HitCircle { StartTime = 100 },
+                new HitCircle { StartTime = 100 }
+            });
+        }
+
+        [Test]
+        public void TestCirclesAlmostConcurrent()
+        {
+            assertConcurrentSame(new List<HitObject>
+            {
+                new HitCircle { StartTime = 100 },
+                new HitCircle { StartTime = 101 }
+            });
+        }
+
+        [Test]
+        public void TestSlidersSeparate()
+        {
+            assertOk(new List<HitObject>
+            {
+                getSliderMock(startTime: 100, endTime: 400.75d).Object,
+                getSliderMock(startTime: 500, endTime: 900.75d).Object
+            });
+        }
+
+        [Test]
+        public void TestSlidersConcurrent()
+        {
+            assertConcurrentSame(new List<HitObject>
+            {
+                getSliderMock(startTime: 100, endTime: 400.75d).Object,
+                getSliderMock(startTime: 300, endTime: 700.75d).Object
+            });
+        }
+
+        [Test]
+        public void TestSlidersAlmostConcurrent()
+        {
+            assertConcurrentSame(new List<HitObject>
+            {
+                getSliderMock(startTime: 100, endTime: 400.75d).Object,
+                getSliderMock(startTime: 402, endTime: 902.75d).Object
+            });
+        }
+
+        [Test]
+        public void TestSliderAndCircleConcurrent()
+        {
+            assertConcurrentDifferent(new List<HitObject>
+            {
+                getSliderMock(startTime: 100, endTime: 400.75d).Object,
+                new HitCircle { StartTime = 300 }
+            });
+        }
+
+        [Test]
+        public void TestManyObjectsConcurrent()
+        {
+            var hitobjects = new List<HitObject>
+            {
+                getSliderMock(startTime: 100, endTime: 400.75d).Object,
+                getSliderMock(startTime: 200, endTime: 500.75d).Object,
+                new HitCircle { StartTime = 300 }
+            };
+
+            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<HitObject>
+            {
+                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<HitObject>
+            {
+                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<HitObject>
+            {
+                getHoldNoteMock(startTime: 100, endTime: 400.75d, column: 1).Object,
+                getHoldNoteMock(startTime: 300, endTime: 700.75d, column: 1).Object
+            });
+        }
+
+        private Mock<Slider> getSliderMock(double startTime, double endTime, int repeats = 0)
+        {
+            var mock = new Mock<Slider>();
+            mock.SetupGet(s => s.StartTime).Returns(startTime);
+            mock.As<IHasRepeats>().Setup(r => r.RepeatCount).Returns(repeats);
+            mock.As<IHasDuration>().Setup(d => d.EndTime).Returns(endTime);
+
+            return mock;
+        }
+
+        private Mock<HoldNote> getHoldNoteMock(double startTime, double endTime, int column)
+        {
+            var mock = new Mock<HoldNote>();
+            mock.SetupGet(s => s.StartTime).Returns(startTime);
+            mock.As<IHasDuration>().Setup(d => d.EndTime).Returns(endTime);
+            mock.As<IHasColumn>().Setup(c => c.Column).Returns(column);
+
+            return mock;
+        }
+
+        private void assertOk(List<HitObject> hitobjects)
+        {
+            Assert.That(check.Run(getPlayableBeatmap(hitobjects), null), Is.Empty);
+        }
+
+        private void assertConcurrentSame(List<HitObject> 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<HitObject> 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<HitObject> hitobjects)
+        {
+            return new Beatmap<HitObject>
+            {
+                HitObjects = hitobjects
+            };
+        }
+    }
+}

osu.Game.Tests/Editing/Checks/CheckUnsnappedObjectsTest.cs

@@ -0,0 +1,155 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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 CheckUnsnappedObjectsTest
+    {
+        private CheckUnsnappedObjects check;
+        private ControlPointInfo cpi;
+
+        [SetUp]
+        public void Setup()
+        {
+            check = new CheckUnsnappedObjects();
+
+            cpi = new ControlPointInfo();
+            cpi.Add(100, new TimingControlPoint { BeatLength = 100 });
+        }
+
+        [Test]
+        public void TestCircleSnapped()
+        {
+            assertOk(new List<HitObject>
+            {
+                new HitCircle { StartTime = 100 }
+            });
+        }
+
+        [Test]
+        public void TestCircleUnsnapped1Ms()
+        {
+            assert1Ms(new List<HitObject>
+            {
+                new HitCircle { StartTime = 101 }
+            });
+
+            assert1Ms(new List<HitObject>
+            {
+                new HitCircle { StartTime = 99 }
+            });
+        }
+
+        [Test]
+        public void TestCircleUnsnapped2Ms()
+        {
+            assert2Ms(new List<HitObject>
+            {
+                new HitCircle { StartTime = 102 }
+            });
+
+            assert2Ms(new List<HitObject>
+            {
+                new HitCircle { StartTime = 98 }
+            });
+        }
+
+        [Test]
+        public void TestSliderSnapped()
+        {
+            // Slider ends are naturally < 1 ms unsnapped because of how SV works.
+            assertOk(new List<HitObject>
+            {
+                getSliderMock(startTime: 100, endTime: 400.75d).Object
+            });
+        }
+
+        [Test]
+        public void TestSliderUnsnapped1Ms()
+        {
+            assert1Ms(new List<HitObject>
+            {
+                getSliderMock(startTime: 101, endTime: 401.75d).Object
+            }, count: 2);
+
+            // End is only off by 0.25 ms, hence count 1.
+            assert1Ms(new List<HitObject>
+            {
+                getSliderMock(startTime: 99, endTime: 399.75d).Object
+            }, count: 1);
+        }
+
+        [Test]
+        public void TestSliderUnsnapped2Ms()
+        {
+            assert2Ms(new List<HitObject>
+            {
+                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<HitObject>
+            {
+                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 CheckUnsnappedObjects.IssueTemplateSmallUnsnap));
+            Assert.That(issues.Any(issue => issue.Template is CheckUnsnappedObjects.IssueTemplateLargeUnsnap));
+        }
+
+        private Mock<Slider> getSliderMock(double startTime, double endTime, int repeats = 0)
+        {
+            var mockSlider = new Mock<Slider>();
+            mockSlider.SetupGet(s => s.StartTime).Returns(startTime);
+            mockSlider.As<IHasRepeats>().Setup(r => r.RepeatCount).Returns(repeats);
+            mockSlider.As<IHasDuration>().Setup(d => d.EndTime).Returns(endTime);
+
+            return mockSlider;
+        }
+
+        private void assertOk(List<HitObject> hitobjects)
+        {
+            Assert.That(check.Run(getPlayableBeatmap(hitobjects), null), Is.Empty);
+        }
+
+        private void assert1Ms(List<HitObject> 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 CheckUnsnappedObjects.IssueTemplateSmallUnsnap));
+        }
+
+        private void assert2Ms(List<HitObject> 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 CheckUnsnappedObjects.IssueTemplateLargeUnsnap));
+        }
+
+        private IBeatmap getPlayableBeatmap(List<HitObject> hitobjects)
+        {
+            return new Beatmap<HitObject>
+            {
+                ControlPointInfo = cpi,
+                HitObjects = hitobjects
+            };
+        }
+    }
+}

osu.Game.Tests/NonVisual/ClosestBeatDivisorTest.cs

@@ -0,0 +1,91 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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<double> divisors, IReadOnlyList<double> closestDivisors, ControlPointInfo cpi, double step = 1)
+        {
+            List<HitObject> hitobjects = new List<HitObject>();
+            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.ControlPointInfo.GetClosestBeatDivisor(beatmap.HitObjects[i].StartTime), $"at index {i}");
+        }
+    }
+}

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,58 @@ namespace osu.Game.Beatmaps.ControlPoints
             groups.Remove(group);
         }
 
+        /// <summary>
+        /// Returns the time on the given beat divisor closest to the given time.
+        /// </summary>
+        /// <param name="time">The time to find the closest snapped time to.</param>
+        /// <param name="beatDivisor">The beat divisor to snap to.</param>
+        /// <param name="referenceTime">An optional reference point to use for timing point lookup.</param>
+        public double GetClosestSnappedTime(double time, int beatDivisor, double? referenceTime = null)
+        {
+            var timingPoint = TimingPointAt(referenceTime ?? time);
+            return getClosestSnappedTime(timingPoint, time, beatDivisor);
+        }
+
+        /// <summary>
+        /// Returns the time on *ANY* valid beat divisor, favouring the divisor closest to the given time.
+        /// </summary>
+        /// <param name="time">The time to find the closest snapped time to.</param>
+        public double GetClosestSnappedTime(double time) => GetClosestSnappedTime(time, GetClosestBeatDivisor(time));
+
+        /// <summary>
+        /// Returns the beat snap divisor closest to the given time. If two are equally close, the smallest divisor is returned.
+        /// </summary>
+        /// <param name="time">The time to find the closest beat snap divisor to.</param>
+        /// <param name="referenceTime">An optional reference point to use for timing point lookup.</param>
+        public int GetClosestBeatDivisor(double time, double? referenceTime = null)
+        {
+            TimingControlPoint timingPoint = TimingPointAt(referenceTime ?? time);
+
+            int closestDivisor = 0;
+            double closestTime = double.MaxValue;
+
+            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;
+        }
+
         /// <summary>
         /// Binary searches one of the control point lists to find the active control point at <paramref name="time"/>.
         /// Includes logic for returning a specific point when no matching point is found.

osu.Game/Rulesets/Edit/BeatmapVerifier.cs

@@ -22,7 +22,11 @@ namespace osu.Game.Rulesets.Edit
 
             // Audio
             new CheckAudioPresence(),
-            new CheckAudioQuality()
+            new CheckAudioQuality(),
+
+            // Compose
+            new CheckUnsnappedObjects(),
+            new CheckConcurrentObjects()
         };
 
         public IEnumerable<Issue> Run(IBeatmap playableBeatmap, WorkingBeatmap workingBeatmap)

osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs

@@ -0,0 +1,88 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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 = CheckUnsnappedObjects.UNSNAP_MS_THRESHOLD;
+
+        public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Concurrent hitobjects");
+
+        public IEnumerable<IssueTemplate> PossibleTemplates => new IssueTemplate[]
+        {
+            new IssueTemplateConcurrentSame(this),
+            new IssueTemplateConcurrentDifferent(this)
+        };
+
+        public IEnumerable<Issue> 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);
+                }
+            }
+        }
+
+        private 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> { 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.")
+            {
+            }
+        }
+    }
+}

osu.Game/Rulesets/Edit/Checks/CheckUnsnappedObjects.cs

@@ -0,0 +1,100 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+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 CheckUnsnappedObjects : ICheck
+    {
+        public const double UNSNAP_MS_THRESHOLD = 2;
+
+        public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Timing, "Unsnapped hitobjects");
+
+        public IEnumerable<IssueTemplate> PossibleTemplates => new IssueTemplate[]
+        {
+            new IssueTemplateLargeUnsnap(this),
+            new IssueTemplateSmallUnsnap(this)
+        };
+
+        public IEnumerable<Issue> Run(IBeatmap playableBeatmap, IWorkingBeatmap workingBeatmap)
+        {
+            var controlPointInfo = playableBeatmap.ControlPointInfo;
+
+            foreach (var hitobject in playableBeatmap.HitObjects)
+            {
+                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;
+
+                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 - controlPointInfo.GetClosestSnappedTime(repeatTime);
+                        foreach (var issue in getUnsnapIssues(hitobject, repeatUnsnap, repeatTime, "repeat"))
+                            yield return issue;
+                    }
+                }
+
+                if (hitobject is IHasDuration hasDuration)
+                {
+                    double endUnsnap = hasDuration.EndTime - controlPointInfo.GetClosestSnappedTime(hasDuration.EndTime);
+                    foreach (var issue in getUnsnapIssues(hitobject, endUnsnap, hasDuration.EndTime, "end"))
+                        yield return issue;
+                }
+            }
+        }
+
+        private IEnumerable<Issue> getUnsnapIssues(HitObject hitobject, double unsnap, double time, string postfix = "")
+        {
+            if (Math.Abs(unsnap) >= UNSNAP_MS_THRESHOLD)
+                yield return new IssueTemplateLargeUnsnap(this).Create(hitobject, unsnap, time, postfix);
+            else if (Math.Abs(unsnap) >= 1)
+                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.
+        }
+
+        public abstract class IssueTemplateUnsnap : IssueTemplate
+        {
+            protected IssueTemplateUnsnap(ICheck check, IssueType type)
+                : base(check, type, "{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 IssueTemplateLargeUnsnap : IssueTemplateUnsnap
+        {
+            public IssueTemplateLargeUnsnap(ICheck check)
+                : base(check, IssueType.Problem)
+            {
+            }
+        }
+
+        public class IssueTemplateSmallUnsnap : IssueTemplateUnsnap
+        {
+            public IssueTemplateSmallUnsnap(ICheck check)
+                : base(check, IssueType.Negligible)
+            {
+            }
+        }
+    }
+}

osu.Game/Rulesets/Objects/Types/IHasColumn.cs

@@ -1,7 +1,7 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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
 {
     /// <summary>
     /// A type of hit object which lies in one of a number of predetermined columns.

osu.Game/Screens/Edit/EditorBeatmap.cs

@@ -301,13 +301,7 @@ namespace osu.Game.Screens.Edit
             return list.Count - 1;
         }
 
-        public double SnapTime(double time, double? referenceTime)
-        {
-            var timingPoint = ControlPointInfo.TimingPointAt(referenceTime ?? time);
-            var beatLength = timingPoint.BeatLength / BeatDivisor;
-
-            return timingPoint.Time + (int)Math.Round((time - timingPoint.Time) / beatLength, MidpointRounding.AwayFromZero) * beatLength;
-        }
+        public double SnapTime(double time, double? referenceTime) => ControlPointInfo.GetClosestSnappedTime(time, BeatDivisor, referenceTime);
 
         public double GetBeatLengthAtTime(double referenceTime) => ControlPointInfo.TimingPointAt(referenceTime).BeatLength / BeatDivisor;