From 973f606a9e48fb5d43cbbff03af514ca8a48766a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 7 Jan 2025 13:59:26 +0100 Subject: [PATCH 1/4] Add test coverage for expected behaviour --- .../TestSceneEditorBeatmapProcessor.cs | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/osu.Game.Tests/Editing/TestSceneEditorBeatmapProcessor.cs b/osu.Game.Tests/Editing/TestSceneEditorBeatmapProcessor.cs index bbcf6aac2c..1df8f96f93 100644 --- a/osu.Game.Tests/Editing/TestSceneEditorBeatmapProcessor.cs +++ b/osu.Game.Tests/Editing/TestSceneEditorBeatmapProcessor.cs @@ -539,5 +539,78 @@ namespace osu.Game.Tests.Editing Assert.That(beatmap.Breaks[0].EndTime, Is.EqualTo(5000 - OsuHitObject.PREEMPT_MAX)); }); } + + [Test] + public void TestPuttingObjectBetweenBreakEndAndAnotherObjectForcesNewCombo() + { + var controlPoints = new ControlPointInfo(); + controlPoints.Add(0, new TimingControlPoint { BeatLength = 500 }); + var beatmap = new EditorBeatmap(new Beatmap + { + ControlPointInfo = controlPoints, + BeatmapInfo = { Ruleset = new OsuRuleset().RulesetInfo }, + Difficulty = + { + ApproachRate = 10, + }, + HitObjects = + { + new HitCircle { StartTime = 1000, NewCombo = true }, + new HitCircle { StartTime = 4500 }, + new HitCircle { StartTime = 5000, NewCombo = true }, + }, + Breaks = + { + new BreakPeriod(2000, 4000), + } + }); + + foreach (var ho in beatmap.HitObjects) + ho.ApplyDefaults(beatmap.ControlPointInfo, beatmap.Difficulty); + + var beatmapProcessor = new EditorBeatmapProcessor(beatmap, new OsuRuleset()); + beatmapProcessor.PreProcess(); + beatmapProcessor.PostProcess(); + + Assert.Multiple(() => + { + Assert.That(((HitCircle)beatmap.HitObjects[1]).NewCombo, Is.True); + Assert.That(((HitCircle)beatmap.HitObjects[2]).NewCombo, Is.True); + }); + } + + [Test] + public void TestAutomaticallyInsertedBreakForcesNewCombo() + { + var controlPoints = new ControlPointInfo(); + controlPoints.Add(0, new TimingControlPoint { BeatLength = 500 }); + var beatmap = new EditorBeatmap(new Beatmap + { + ControlPointInfo = controlPoints, + BeatmapInfo = { Ruleset = new OsuRuleset().RulesetInfo }, + Difficulty = + { + ApproachRate = 10, + }, + HitObjects = + { + new HitCircle { StartTime = 1000, NewCombo = true }, + new HitCircle { StartTime = 5000 }, + }, + }); + + foreach (var ho in beatmap.HitObjects) + ho.ApplyDefaults(beatmap.ControlPointInfo, beatmap.Difficulty); + + var beatmapProcessor = new EditorBeatmapProcessor(beatmap, new OsuRuleset()); + beatmapProcessor.PreProcess(); + beatmapProcessor.PostProcess(); + + Assert.Multiple(() => + { + Assert.That(beatmap.Breaks, Has.Count.EqualTo(1)); + Assert.That(((HitCircle)beatmap.HitObjects[1]).NewCombo, Is.True); + }); + } } } From c93b87583ac33bc9dc0bd8efc05ebc8f683fea70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 7 Jan 2025 13:59:53 +0100 Subject: [PATCH 2/4] Force new combo on objects succeeding a break No issue thread for this again. Reported internally on discord: https://discord.com/channels/90072389919997952/1259818301517725707/1320420768814727229 Placing this logic in the beatmap processor, as a post-processing step, means that the new combo force won't be visible until a placement has been committed. That can be seen as subpar, but I tried putting this logic in the placement and it sucked anyway: - While the combo number was correct, the colour looked off, because it would use the same combo colour as the already-placed objects after said break, which would only cycle to the next, correct one on placement - Not all scenarios can be handled in the placement. Refer to one of the test cases added in the preceding commit, wherein two objects are placed far apart from each other, and an automated break is inserted between them - the placement has no practical way of knowing whether it's going to have a break inserted automatically before it or not. --- .../Screens/Edit/EditorBeatmapProcessor.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/osu.Game/Screens/Edit/EditorBeatmapProcessor.cs b/osu.Game/Screens/Edit/EditorBeatmapProcessor.cs index 4fe431498f..8108f51ad1 100644 --- a/osu.Game/Screens/Edit/EditorBeatmapProcessor.cs +++ b/osu.Game/Screens/Edit/EditorBeatmapProcessor.cs @@ -41,6 +41,7 @@ namespace osu.Game.Screens.Edit rulesetBeatmapProcessor?.PostProcess(); autoGenerateBreaks(); + ensureNewComboAfterBreaks(); } private void autoGenerateBreaks() @@ -100,5 +101,31 @@ namespace osu.Game.Screens.Edit Beatmap.Breaks.Add(breakPeriod); } } + + private void ensureNewComboAfterBreaks() + { + var breakEnds = Beatmap.Breaks.Select(b => b.EndTime).OrderBy(t => t).ToList(); + + if (breakEnds.Count == 0) + return; + + int currentBreak = 0; + + for (int i = 0; i < Beatmap.HitObjects.Count; ++i) + { + var hitObject = Beatmap.HitObjects[i]; + + if (hitObject is not IHasComboInformation hasCombo) + continue; + + if (currentBreak < breakEnds.Count && hitObject.StartTime >= breakEnds[currentBreak]) + { + hasCombo.NewCombo = true; + currentBreak += 1; + } + + hasCombo.UpdateComboInformation(i > 0 ? Beatmap.HitObjects[i - 1] as IHasComboInformation : null); + } + } } } From fbfda2e04425296c8f8fb73557cc724da0ee0e6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 8 Jan 2025 10:28:04 +0100 Subject: [PATCH 3/4] Extend test coverage with combo index correctness checks --- osu.Game.Tests/Editing/TestSceneEditorBeatmapProcessor.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/osu.Game.Tests/Editing/TestSceneEditorBeatmapProcessor.cs b/osu.Game.Tests/Editing/TestSceneEditorBeatmapProcessor.cs index 1df8f96f93..c625346645 100644 --- a/osu.Game.Tests/Editing/TestSceneEditorBeatmapProcessor.cs +++ b/osu.Game.Tests/Editing/TestSceneEditorBeatmapProcessor.cs @@ -576,6 +576,10 @@ namespace osu.Game.Tests.Editing { Assert.That(((HitCircle)beatmap.HitObjects[1]).NewCombo, Is.True); Assert.That(((HitCircle)beatmap.HitObjects[2]).NewCombo, Is.True); + + Assert.That(((HitCircle)beatmap.HitObjects[0]).ComboIndex, Is.EqualTo(1)); + Assert.That(((HitCircle)beatmap.HitObjects[1]).ComboIndex, Is.EqualTo(2)); + Assert.That(((HitCircle)beatmap.HitObjects[2]).ComboIndex, Is.EqualTo(3)); }); } @@ -610,6 +614,9 @@ namespace osu.Game.Tests.Editing { Assert.That(beatmap.Breaks, Has.Count.EqualTo(1)); Assert.That(((HitCircle)beatmap.HitObjects[1]).NewCombo, Is.True); + + Assert.That(((HitCircle)beatmap.HitObjects[0]).ComboIndex, Is.EqualTo(1)); + Assert.That(((HitCircle)beatmap.HitObjects[1]).ComboIndex, Is.EqualTo(2)); }); } } From 7c70dc4dc305d7bcd421c0e1f8d83d1ab3bfd67b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 8 Jan 2025 10:28:06 +0100 Subject: [PATCH 4/4] Only update combo information when any changes happened --- .../Screens/Edit/EditorBeatmapProcessor.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/Edit/EditorBeatmapProcessor.cs b/osu.Game/Screens/Edit/EditorBeatmapProcessor.cs index 8108f51ad1..957c1d0969 100644 --- a/osu.Game/Screens/Edit/EditorBeatmapProcessor.cs +++ b/osu.Game/Screens/Edit/EditorBeatmapProcessor.cs @@ -111,20 +111,29 @@ namespace osu.Game.Screens.Edit int currentBreak = 0; - for (int i = 0; i < Beatmap.HitObjects.Count; ++i) - { - var hitObject = Beatmap.HitObjects[i]; + IHasComboInformation? lastObj = null; + bool comboInformationUpdateRequired = false; + foreach (var hitObject in Beatmap.HitObjects) + { if (hitObject is not IHasComboInformation hasCombo) continue; if (currentBreak < breakEnds.Count && hitObject.StartTime >= breakEnds[currentBreak]) { - hasCombo.NewCombo = true; + if (!hasCombo.NewCombo) + { + hasCombo.NewCombo = true; + comboInformationUpdateRequired = true; + } + currentBreak += 1; } - hasCombo.UpdateComboInformation(i > 0 ? Beatmap.HitObjects[i - 1] as IHasComboInformation : null); + if (comboInformationUpdateRequired) + hasCombo.UpdateComboInformation(lastObj); + + lastObj = hasCombo; } } }