diff --git a/osu.Game.Rulesets.Mania.Tests/Editor/Checks/CheckManiaConcurrentObjectsTest.cs b/osu.Game.Rulesets.Mania.Tests/Editor/Checks/CheckManiaConcurrentObjectsTest.cs index 5af2af9314..731263b25c 100644 --- a/osu.Game.Rulesets.Mania.Tests/Editor/Checks/CheckManiaConcurrentObjectsTest.cs +++ b/osu.Game.Rulesets.Mania.Tests/Editor/Checks/CheckManiaConcurrentObjectsTest.cs @@ -55,6 +55,16 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor.Checks }); } + [Test] + public void TestHoldNotesAlmostConcurrentOnSameColumn() + { + assertAlmostConcurrentSame(new List + { + createHoldNote(startTime: 100, endTime: 400.75d, column: 1), + createHoldNote(startTime: 408, endTime: 700.75d, column: 1) + }); + } + private void assertOk(List hitobjects) { Assert.That(check.Run(getContext(hitobjects)), Is.Empty); @@ -65,7 +75,17 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor.Checks var issues = check.Run(getContext(hitobjects)).ToList(); Assert.That(issues, Has.Count.EqualTo(count)); - Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrentSame)); + Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrent)); + Assert.That(issues.All(issue => issue.ToString().Contains("s are concurrent here"))); + } + + private void assertAlmostConcurrentSame(List hitobjects) + { + var issues = check.Run(getContext(hitobjects)).ToList(); + + Assert.That(issues, Has.Count.EqualTo(1)); + Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateAlmostConcurrent)); + Assert.That(issues.All(issue => issue.ToString().Contains("s are less than 10ms apart"))); } private BeatmapVerifierContext getContext(List hitobjects) diff --git a/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs b/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs index 569217207c..5c73a6b676 100644 --- a/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs +++ b/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs @@ -28,14 +28,18 @@ namespace osu.Game.Rulesets.Mania.Edit.Checks 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)) + // So if the next object is not concurrent or almost concurrent, then we know no future objects will be either. + if (!AreConcurrent(hitobject, nextHitobject) && !AreAlmostConcurrent(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); + if (AreConcurrent(hitobject, nextHitobject)) + { + yield return new IssueTemplateConcurrent(this).Create(hitobject, nextHitobject); + } + else if (AreAlmostConcurrent(hitobject, nextHitobject)) + { + yield return new IssueTemplateAlmostConcurrent(this).Create(hitobject, nextHitobject); + } } } } diff --git a/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs b/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs index a255f41653..fd63e1b05d 100644 --- a/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs @@ -57,6 +57,16 @@ namespace osu.Game.Tests.Editing.Checks }); } + [Test] + public void TestCirclesAlmostConcurrentWarning() + { + assertAlmostConcurrentSame(new List + { + new HitCircle { StartTime = 100 }, + new HitCircle { StartTime = 108 } + }); + } + [Test] public void TestSlidersSeparate() { @@ -97,6 +107,16 @@ namespace osu.Game.Tests.Editing.Checks }); } + [Test] + public void TestSliderAndCircleAlmostConcurrent() + { + assertAlmostConcurrentDifferent(new List + { + getSliderMock(startTime: 100, endTime: 400.75d).Object, + new HitCircle { StartTime = 408 } + }); + } + [Test] public void TestManyObjectsConcurrent() { @@ -110,8 +130,14 @@ namespace osu.Game.Tests.Editing.Checks var issues = check.Run(getContext(hitobjects)).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)); + Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrent)); + + // Should have 1 same-type concurrent (Slider & Slider) and 2 different-type concurrent (Slider & Circle) + var sameTypeIssues = issues.Where(issue => issue.ToString().Contains("s are concurrent here")).ToList(); + var differentTypeIssues = issues.Where(issue => issue.ToString().Contains(" and ") && issue.ToString().Contains("are concurrent here")).ToList(); + + Assert.That(sameTypeIssues, Has.Count.EqualTo(1)); + Assert.That(differentTypeIssues, Has.Count.EqualTo(2)); } private Mock getSliderMock(double startTime, double endTime, int repeats = 0) @@ -144,7 +170,8 @@ namespace osu.Game.Tests.Editing.Checks var issues = check.Run(getContext(hitobjects)).ToList(); Assert.That(issues, Has.Count.EqualTo(count)); - Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrentSame)); + Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrent)); + Assert.That(issues.All(issue => issue.ToString().Contains("s are concurrent here"))); } private void assertConcurrentDifferent(List hitobjects, int count = 1) @@ -152,7 +179,26 @@ namespace osu.Game.Tests.Editing.Checks var issues = check.Run(getContext(hitobjects)).ToList(); Assert.That(issues, Has.Count.EqualTo(count)); - Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrentDifferent)); + Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrent)); + Assert.That(issues.All(issue => issue.ToString().Contains(" and ") && issue.ToString().Contains("are concurrent here"))); + } + + private void assertAlmostConcurrentSame(List hitobjects) + { + var issues = check.Run(getContext(hitobjects)).ToList(); + + Assert.That(issues, Has.Count.EqualTo(1)); + Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateAlmostConcurrent)); + Assert.That(issues.All(issue => issue.ToString().Contains("s are less than 10ms apart"))); + } + + private void assertAlmostConcurrentDifferent(List hitobjects) + { + var issues = check.Run(getContext(hitobjects)).ToList(); + + Assert.That(issues, Has.Count.EqualTo(1)); + Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateAlmostConcurrent)); + Assert.That(issues.All(issue => issue.ToString().Contains(" and ") && issue.ToString().Contains("are less than 10ms apart"))); } private BeatmapVerifierContext getContext(List hitobjects) diff --git a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs index c0089e6fe2..c23a944ffb 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Objects; +using System; namespace osu.Game.Rulesets.Edit.Checks { @@ -11,13 +12,14 @@ namespace osu.Game.Rulesets.Edit.Checks { // 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; + private const double almost_concurrent_threshold = 10.0; public CheckMetadata Metadata { get; } = new CheckMetadata(CheckCategory.Compose, "Concurrent hitobjects"); public IEnumerable PossibleTemplates => new IssueTemplate[] { - new IssueTemplateConcurrentSame(this), - new IssueTemplateConcurrentDifferent(this) + new IssueTemplateConcurrent(this), + new IssueTemplateAlmostConcurrent(this) }; public virtual IEnumerable Run(BeatmapVerifierContext context) @@ -33,50 +35,66 @@ namespace osu.Game.Rulesets.Edit.Checks var nextHitobject = hitObjects[j]; // 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)) + // So if the next object is not concurrent or almost concurrent, then we know no future objects will be either. + if (!AreConcurrent(hitobject, nextHitobject) && !AreAlmostConcurrent(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); + if (AreConcurrent(hitobject, nextHitobject)) + { + yield return new IssueTemplateConcurrent(this).Create(hitobject, nextHitobject); + } + else if (AreAlmostConcurrent(hitobject, nextHitobject)) + { + yield return new IssueTemplateAlmostConcurrent(this).Create(hitobject, nextHitobject); + } } } } protected bool AreConcurrent(HitObject hitobject, HitObject nextHitobject) => nextHitobject.StartTime <= hitobject.GetEndTime() + ms_leniency; - public abstract class IssueTemplateConcurrent : IssueTemplate + protected bool AreAlmostConcurrent(HitObject hitobject, HitObject nextHitobject) => + Math.Abs(nextHitobject.StartTime - hitobject.GetEndTime()) < almost_concurrent_threshold; + + public class IssueTemplateConcurrent : IssueTemplate { - protected IssueTemplateConcurrent(ICheck check, string unformattedMessage) - : base(check, IssueType.Problem, unformattedMessage) + public IssueTemplateConcurrent(ICheck check) + : base(check, IssueType.Problem, "{0}") { } public Issue Create(HitObject hitobject, HitObject nextHitobject) { var hitobjects = new List { hitobject, nextHitobject }; - return new Issue(hitobjects, this, hitobject.GetType().Name, nextHitobject.GetType().Name) + string message = hitobject.GetType() == nextHitobject.GetType() + ? $"{hitobject.GetType().Name}s are concurrent here." + : $"{hitobject.GetType().Name} and {nextHitobject.GetType().Name} are concurrent here."; + + return new Issue(hitobjects, this, message) { Time = nextHitobject.StartTime }; } } - public class IssueTemplateConcurrentSame : IssueTemplateConcurrent + public class IssueTemplateAlmostConcurrent : IssueTemplate { - public IssueTemplateConcurrentSame(ICheck check) - : base(check, "{0}s are concurrent here.") + public IssueTemplateAlmostConcurrent(ICheck check) + : base(check, IssueType.Problem, "{0}") { } - } - public class IssueTemplateConcurrentDifferent : IssueTemplateConcurrent - { - public IssueTemplateConcurrentDifferent(ICheck check) - : base(check, "{0} and {1} are concurrent here.") + public Issue Create(HitObject hitobject, HitObject nextHitobject) { + var hitobjects = new List { hitobject, nextHitobject }; + string message = hitobject.GetType() == nextHitobject.GetType() + ? $"{hitobject.GetType().Name}s are less than {almost_concurrent_threshold}ms apart." + : $"{hitobject.GetType().Name} and {nextHitobject.GetType().Name} are less than {almost_concurrent_threshold}ms apart."; + + return new Issue(hitobjects, this, message) + { + Time = nextHitobject.StartTime + }; } } }