From 00656962481e77d56ecb7b622426fce4f82522dc Mon Sep 17 00:00:00 2001 From: Derrick Timmermans Date: Sun, 25 May 2025 14:19:23 +0200 Subject: [PATCH 1/5] Make CheckConcurrentObject ruleset specific --- .../Edit/CatchBeatmapVerifier.cs | 5 +++++ .../Edit/Checks/CheckManiaConcurrentObjects.cs | 15 +++++++++++++++ .../Edit/ManiaBeatmapVerifier.cs | 3 +++ osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs | 2 ++ .../Edit/TaikoBeatmapVerifier.cs | 5 +++++ osu.Game/Rulesets/Edit/BeatmapVerifier.cs | 1 - .../Edit/Checks/CheckConcurrentObjects.cs | 12 ++++++++---- 7 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs diff --git a/osu.Game.Rulesets.Catch/Edit/CatchBeatmapVerifier.cs b/osu.Game.Rulesets.Catch/Edit/CatchBeatmapVerifier.cs index 71da6d5014..374ab16633 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchBeatmapVerifier.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchBeatmapVerifier.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using osu.Game.Rulesets.Catch.Edit.Checks; using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Edit.Checks; using osu.Game.Rulesets.Edit.Checks.Components; namespace osu.Game.Rulesets.Catch.Edit @@ -13,7 +14,11 @@ namespace osu.Game.Rulesets.Catch.Edit { private readonly List checks = new List { + // Compose new CheckBananaShowerGap(), + new CheckConcurrentObjects(), + + // Settings new CheckCatchAbnormalDifficultySettings(), }; diff --git a/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs b/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs new file mode 100644 index 0000000000..a69a00a42e --- /dev/null +++ b/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs @@ -0,0 +1,15 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Game.Rulesets.Edit.Checks; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; + +namespace osu.Game.Rulesets.Mania.Edit.Checks +{ + public class CheckManiaConcurrentObjects : CheckConcurrentObjects + { + // Mania hitobjects are only considered concurrent if they also share the same column. + protected override bool ConcurrentCondition(HitObject first, HitObject second) => (first as IHasColumn)?.Column != (second as IHasColumn)?.Column; + } +} diff --git a/osu.Game.Rulesets.Mania/Edit/ManiaBeatmapVerifier.cs b/osu.Game.Rulesets.Mania/Edit/ManiaBeatmapVerifier.cs index 4adabfa4d7..efb1d354af 100644 --- a/osu.Game.Rulesets.Mania/Edit/ManiaBeatmapVerifier.cs +++ b/osu.Game.Rulesets.Mania/Edit/ManiaBeatmapVerifier.cs @@ -13,6 +13,9 @@ namespace osu.Game.Rulesets.Mania.Edit { private readonly List checks = new List { + // Compose + new CheckManiaConcurrentObjects(), + // Settings new CheckKeyCount(), new CheckManiaAbnormalDifficultySettings(), diff --git a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs index 4b01a1fc39..c3796124b8 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuBeatmapVerifier.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Edit.Checks; using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Osu.Edit.Checks; @@ -16,6 +17,7 @@ namespace osu.Game.Rulesets.Osu.Edit // Compose new CheckOffscreenObjects(), new CheckTooShortSpinners(), + new CheckConcurrentObjects(), // Spread new CheckTimeDistanceEquality(), diff --git a/osu.Game.Rulesets.Taiko/Edit/TaikoBeatmapVerifier.cs b/osu.Game.Rulesets.Taiko/Edit/TaikoBeatmapVerifier.cs index f5c3f1846d..8f695c4834 100644 --- a/osu.Game.Rulesets.Taiko/Edit/TaikoBeatmapVerifier.cs +++ b/osu.Game.Rulesets.Taiko/Edit/TaikoBeatmapVerifier.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Edit.Checks; using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Taiko.Edit.Checks; @@ -13,6 +14,10 @@ namespace osu.Game.Rulesets.Taiko.Edit { private readonly List checks = new List { + // Compose + new CheckConcurrentObjects(), + + // Settings new CheckTaikoAbnormalDifficultySettings(), }; diff --git a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs index 642b878a7b..e1c0815dac 100644 --- a/osu.Game/Rulesets/Edit/BeatmapVerifier.cs +++ b/osu.Game/Rulesets/Edit/BeatmapVerifier.cs @@ -36,7 +36,6 @@ namespace osu.Game.Rulesets.Edit // Compose new CheckUnsnappedObjects(), - new CheckConcurrentObjects(), new CheckZeroLengthObjects(), new CheckDrainLength(), new CheckUnusedAudioAtEnd(), diff --git a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs index ba5fbcf58d..5f5f0e2f6f 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Objects; -using osu.Game.Rulesets.Objects.Types; namespace osu.Game.Rulesets.Edit.Checks { @@ -21,6 +20,12 @@ namespace osu.Game.Rulesets.Edit.Checks new IssueTemplateConcurrentDifferent(this) }; + /// + /// Determines whether two hitobjects can be considered concurrent based on ruleset requirements. + /// + /// Whether the two hitobjects can be concurrent. + protected virtual bool ConcurrentCondition(HitObject first, HitObject second) => true; + public IEnumerable Run(BeatmapVerifierContext context) { var hitObjects = context.Beatmap.HitObjects; @@ -33,9 +38,8 @@ namespace osu.Game.Rulesets.Edit.Checks { var nextHitobject = 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) + // Some rulesets impose additional requirements for concurrency, such as Mania only considering hitobjects in the same column. + if (!ConcurrentCondition(hitobject, nextHitobject)) continue; // Two hitobjects cannot be concurrent without also being concurrent with all objects in between. From 53340fe4b7470cbd2f7335be6ad11a4dc0795a6b Mon Sep 17 00:00:00 2001 From: Derrick Timmermans Date: Sun, 25 May 2025 14:35:59 +0200 Subject: [PATCH 2/5] Flip equality in CheckManiaConcurrentObjects Bad copy and paste. --- .../Edit/Checks/CheckManiaConcurrentObjects.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs b/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs index a69a00a42e..00ba2d655f 100644 --- a/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs +++ b/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs @@ -10,6 +10,6 @@ namespace osu.Game.Rulesets.Mania.Edit.Checks public class CheckManiaConcurrentObjects : CheckConcurrentObjects { // Mania hitobjects are only considered concurrent if they also share the same column. - protected override bool ConcurrentCondition(HitObject first, HitObject second) => (first as IHasColumn)?.Column != (second as IHasColumn)?.Column; + protected override bool ConcurrentCondition(HitObject first, HitObject second) => (first as IHasColumn)?.Column == (second as IHasColumn)?.Column; } } From 9a254afa0a3d72d88660859c04ac733a8779a86a Mon Sep 17 00:00:00 2001 From: Derrick Timmermans Date: Sun, 25 May 2025 17:24:04 +0200 Subject: [PATCH 3/5] Move concurrent hold note tests to mania tests The general check no longer considers mania. --- .../Checks/CheckManiaConcurrentObjectsTest.cs | 87 +++++++++++++++++++ .../Checks/CheckConcurrentObjectsTest.cs | 30 ------- 2 files changed, 87 insertions(+), 30 deletions(-) create mode 100644 osu.Game.Rulesets.Mania.Tests/Editor/Checks/CheckManiaConcurrentObjectsTest.cs diff --git a/osu.Game.Rulesets.Mania.Tests/Editor/Checks/CheckManiaConcurrentObjectsTest.cs b/osu.Game.Rulesets.Mania.Tests/Editor/Checks/CheckManiaConcurrentObjectsTest.cs new file mode 100644 index 0000000000..5af2af9314 --- /dev/null +++ b/osu.Game.Rulesets.Mania.Tests/Editor/Checks/CheckManiaConcurrentObjectsTest.cs @@ -0,0 +1,87 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Edit.Checks; +using osu.Game.Rulesets.Mania.Edit.Checks; +using osu.Game.Rulesets.Mania.Objects; +using osu.Game.Rulesets.Objects; +using osu.Game.Tests.Beatmaps; + +namespace osu.Game.Rulesets.Mania.Tests.Editor.Checks +{ + [TestFixture] + public class CheckManiaConcurrentObjectsTest + { + private CheckConcurrentObjects check = null!; + + [SetUp] + public void Setup() + { + check = new CheckManiaConcurrentObjects(); + } + + [Test] + public void TestHoldNotesSeparateOnSameColumn() + { + assertOk(new List + { + createHoldNote(startTime: 100, endTime: 400.75d, column: 1), + createHoldNote(startTime: 500, endTime: 900.75d, column: 1) + }); + } + + [Test] + public void TestHoldNotesConcurrentOnDifferentColumns() + { + assertOk(new List + { + createHoldNote(startTime: 100, endTime: 400.75d, column: 1), + createHoldNote(startTime: 300, endTime: 700.75d, column: 2) + }); + } + + [Test] + public void TestHoldNotesConcurrentOnSameColumn() + { + assertConcurrentSame(new List + { + createHoldNote(startTime: 100, endTime: 400.75d, column: 1), + createHoldNote(startTime: 300, endTime: 700.75d, column: 1) + }); + } + + private void assertOk(List hitobjects) + { + Assert.That(check.Run(getContext(hitobjects)), Is.Empty); + } + + private void assertConcurrentSame(List hitobjects, int count = 1) + { + var issues = check.Run(getContext(hitobjects)).ToList(); + + Assert.That(issues, Has.Count.EqualTo(count)); + Assert.That(issues.All(issue => issue.Template is CheckConcurrentObjects.IssueTemplateConcurrentSame)); + } + + private BeatmapVerifierContext getContext(List hitobjects) + { + var beatmap = new Beatmap { HitObjects = hitobjects }; + return new BeatmapVerifierContext(beatmap, new TestWorkingBeatmap(beatmap)); + } + + private HoldNote createHoldNote(double startTime, double endTime, int column) + { + return new HoldNote + { + StartTime = startTime, + EndTime = endTime, + Column = column + }; + } + } +} diff --git a/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs b/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs index b5c6568583..a255f41653 100644 --- a/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs +++ b/osu.Game.Tests/Editing/Checks/CheckConcurrentObjectsTest.cs @@ -114,36 +114,6 @@ namespace osu.Game.Tests.Editing.Checks 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(); From 257767b3118d42f979925c1ddf786d24c6bf9357 Mon Sep 17 00:00:00 2001 From: Derrick Timmermans Date: Mon, 26 May 2025 10:32:43 +0200 Subject: [PATCH 4/5] Rename `ConcurrentCondition` to `ConcurrencyPrecondition` Better describes the intent of the method. --- .../Edit/Checks/CheckManiaConcurrentObjects.cs | 3 ++- .../Rulesets/Edit/Checks/CheckConcurrentObjects.cs | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs b/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs index 00ba2d655f..983553b078 100644 --- a/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs +++ b/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs @@ -10,6 +10,7 @@ namespace osu.Game.Rulesets.Mania.Edit.Checks public class CheckManiaConcurrentObjects : CheckConcurrentObjects { // Mania hitobjects are only considered concurrent if they also share the same column. - protected override bool ConcurrentCondition(HitObject first, HitObject second) => (first as IHasColumn)?.Column == (second as IHasColumn)?.Column; + protected override bool ConcurrencyPrecondition(HitObject first, HitObject second) + => (first as IHasColumn)?.Column == (second as IHasColumn)?.Column; } } diff --git a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs index 5f5f0e2f6f..7595adc9dd 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs @@ -21,10 +21,10 @@ namespace osu.Game.Rulesets.Edit.Checks }; /// - /// Determines whether two hitobjects can be considered concurrent based on ruleset requirements. + /// The conditions that must hold true for any two hitobjects to be considered for the concurrency check. /// - /// Whether the two hitobjects can be concurrent. - protected virtual bool ConcurrentCondition(HitObject first, HitObject second) => true; + /// Whether the two hitobjects could be concurrent. + protected virtual bool ConcurrencyPrecondition(HitObject first, HitObject second) => true; public IEnumerable Run(BeatmapVerifierContext context) { @@ -38,8 +38,8 @@ namespace osu.Game.Rulesets.Edit.Checks { var nextHitobject = hitObjects[j]; - // Some rulesets impose additional requirements for concurrency, such as Mania only considering hitobjects in the same column. - if (!ConcurrentCondition(hitobject, nextHitobject)) + // Some rulesets impose additional preconditions for concurrency, such as Mania only considering hitobjects in the same column. + if (!ConcurrencyPrecondition(hitobject, nextHitobject)) continue; // Two hitobjects cannot be concurrent without also being concurrent with all objects in between. From 0495a036079f6e7a0962e6a609d8197fd028f822 Mon Sep 17 00:00:00 2001 From: Derrick Timmermans Date: Mon, 26 May 2025 10:57:20 +0200 Subject: [PATCH 5/5] Make `CheckConcurrentObjects.Run` virtual And let implementers copy-paste common code. --- .../Checks/CheckManiaConcurrentObjects.cs | 35 ++++++++++++++++--- .../Edit/Checks/CheckConcurrentObjects.cs | 16 ++------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs b/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs index 983553b078..569217207c 100644 --- a/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs +++ b/osu.Game.Rulesets.Mania/Edit/Checks/CheckManiaConcurrentObjects.cs @@ -1,16 +1,43 @@ // 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.Rulesets.Edit; using osu.Game.Rulesets.Edit.Checks; -using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Edit.Checks.Components; using osu.Game.Rulesets.Objects.Types; namespace osu.Game.Rulesets.Mania.Edit.Checks { public class CheckManiaConcurrentObjects : CheckConcurrentObjects { - // Mania hitobjects are only considered concurrent if they also share the same column. - protected override bool ConcurrencyPrecondition(HitObject first, HitObject second) - => (first as IHasColumn)?.Column == (second as IHasColumn)?.Column; + public override IEnumerable Run(BeatmapVerifierContext context) + { + var hitObjects = context.Beatmap.HitObjects; + + for (int i = 0; i < hitObjects.Count - 1; ++i) + { + var hitobject = hitObjects[i]; + + for (int j = i + 1; j < hitObjects.Count; ++j) + { + var nextHitobject = hitObjects[j]; + + // Mania hitobjects are only considered concurrent if they also share 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); + } + } + } } } diff --git a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs index 7595adc9dd..c0089e6fe2 100644 --- a/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs +++ b/osu.Game/Rulesets/Edit/Checks/CheckConcurrentObjects.cs @@ -20,13 +20,7 @@ namespace osu.Game.Rulesets.Edit.Checks new IssueTemplateConcurrentDifferent(this) }; - /// - /// The conditions that must hold true for any two hitobjects to be considered for the concurrency check. - /// - /// Whether the two hitobjects could be concurrent. - protected virtual bool ConcurrencyPrecondition(HitObject first, HitObject second) => true; - - public IEnumerable Run(BeatmapVerifierContext context) + public virtual IEnumerable Run(BeatmapVerifierContext context) { var hitObjects = context.Beatmap.HitObjects; @@ -38,13 +32,9 @@ namespace osu.Game.Rulesets.Edit.Checks { var nextHitobject = hitObjects[j]; - // Some rulesets impose additional preconditions for concurrency, such as Mania only considering hitobjects in the same column. - if (!ConcurrencyPrecondition(hitobject, nextHitobject)) - 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)) + if (!AreConcurrent(hitobject, nextHitobject)) break; if (hitobject.GetType() == nextHitobject.GetType()) @@ -55,7 +45,7 @@ namespace osu.Game.Rulesets.Edit.Checks } } - private bool areConcurrent(HitObject hitobject, HitObject nextHitobject) => nextHitobject.StartTime <= hitobject.GetEndTime() + ms_leniency; + protected bool AreConcurrent(HitObject hitobject, HitObject nextHitobject) => nextHitobject.StartTime <= hitobject.GetEndTime() + ms_leniency; public abstract class IssueTemplateConcurrent : IssueTemplate {