From cb4568c4a1e7d12fea7dda4a643878e9c2bae675 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 16 Nov 2023 20:21:11 +0900 Subject: [PATCH 1/4] Fix first object after break not starting a new combo --- .../Formats/LegacyBeatmapDecoderTest.cs | 15 ++++++++++++ .../TestSceneHitObjectAccentColour.cs | 3 --- .../Resources/break-between-objects.osu | 15 ++++++++++++ .../Beatmaps/Formats/LegacyBeatmapDecoder.cs | 23 +++++++++++++++++++ .../Objects/Legacy/Catch/ConvertHit.cs | 6 +---- .../Objects/Legacy/Catch/ConvertSlider.cs | 6 +---- .../Objects/Legacy/Catch/ConvertSpinner.cs | 6 +---- .../Objects/Legacy/ConvertHitObject.cs | 7 +++++- .../Rulesets/Objects/Legacy/Osu/ConvertHit.cs | 6 +---- .../Objects/Legacy/Osu/ConvertSlider.cs | 6 +---- .../Objects/Legacy/Osu/ConvertSpinner.cs | 6 +---- 11 files changed, 65 insertions(+), 34 deletions(-) create mode 100644 osu.Game.Tests/Resources/break-between-objects.osu diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index 66151a51e6..be1993957f 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -1093,5 +1093,20 @@ namespace osu.Game.Tests.Beatmaps.Formats Assert.That(hitObject.Samples.Select(s => s.Volume), Has.All.EqualTo(70)); } } + + [Test] + public void TestNewComboAfterBreak() + { + var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; + + using (var resStream = TestResources.OpenResource("break-between-objects.osu")) + using (var stream = new LineBufferedReader(resStream)) + { + var beatmap = decoder.Decode(stream); + Assert.That(((IHasCombo)beatmap.HitObjects[0]).NewCombo, Is.True); + Assert.That(((IHasCombo)beatmap.HitObjects[1]).NewCombo, Is.True); + Assert.That(((IHasCombo)beatmap.HitObjects[2]).NewCombo, Is.False); + } + } } } diff --git a/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs b/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs index f38c2c9416..acb14f86fc 100644 --- a/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs +++ b/osu.Game.Tests/Gameplay/TestSceneHitObjectAccentColour.cs @@ -94,9 +94,6 @@ namespace osu.Game.Tests.Gameplay private class TestHitObjectWithCombo : ConvertHitObject, IHasComboInformation { - public bool NewCombo { get; set; } - public int ComboOffset => 0; - public Bindable IndexInCurrentComboBindable { get; } = new Bindable(); public int IndexInCurrentCombo diff --git a/osu.Game.Tests/Resources/break-between-objects.osu b/osu.Game.Tests/Resources/break-between-objects.osu new file mode 100644 index 0000000000..91821e2c58 --- /dev/null +++ b/osu.Game.Tests/Resources/break-between-objects.osu @@ -0,0 +1,15 @@ +osu file format v14 + +[General] +Mode: 0 + +[Events] +2,200,1200 + +[TimingPoints] +0,307.692307692308,4,2,1,60,1,0 + +[HitObjects] +142,99,0,1,0,0:0:0:0: +323,88,3000,1,0,0:0:0:0: +323,88,4000,1,0,0:0:0:0: diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 8c5e4971d5..1ee4670ae2 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -93,6 +93,8 @@ namespace osu.Game.Beatmaps.Formats // The parsing order of hitobjects matters in mania difficulty calculation this.beatmap.HitObjects = this.beatmap.HitObjects.OrderBy(h => h.StartTime).ToList(); + postProcessBreaks(this.beatmap); + foreach (var hitObject in this.beatmap.HitObjects) { applyDefaults(hitObject); @@ -100,6 +102,27 @@ namespace osu.Game.Beatmaps.Formats } } + /// + /// Processes the beatmap such that a new combo is started the first hitobject following each break. + /// + private void postProcessBreaks(Beatmap beatmap) + { + int currentBreak = 0; + bool forceNewCombo = false; + + foreach (var h in beatmap.HitObjects.OfType()) + { + while (currentBreak < beatmap.Breaks.Count && beatmap.Breaks[currentBreak].EndTime < h.StartTime) + { + forceNewCombo = true; + currentBreak++; + } + + h.NewCombo |= forceNewCombo; + forceNewCombo = false; + } + } + private void applyDefaults(HitObject hitObject) { DifficultyControlPoint difficultyControlPoint = (beatmap.ControlPointInfo as LegacyControlPointInfo)?.DifficultyPointAt(hitObject.StartTime) ?? DifficultyControlPoint.DEFAULT; diff --git a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHit.cs b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHit.cs index 12b4812824..96c779e79b 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHit.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHit.cs @@ -9,16 +9,12 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch /// /// Legacy osu!catch Hit-type, used for parsing Beatmaps. /// - internal sealed class ConvertHit : ConvertHitObject, IHasPosition, IHasCombo + internal sealed class ConvertHit : ConvertHitObject, IHasPosition { public float X => Position.X; public float Y => Position.Y; public Vector2 Position { get; set; } - - public bool NewCombo { get; set; } - - public int ComboOffset { get; set; } } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertSlider.cs b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertSlider.cs index fb1afed3b4..bcf1c7fae2 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertSlider.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertSlider.cs @@ -9,16 +9,12 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch /// /// Legacy osu!catch Slider-type, used for parsing Beatmaps. /// - internal sealed class ConvertSlider : Legacy.ConvertSlider, IHasPosition, IHasCombo + internal sealed class ConvertSlider : Legacy.ConvertSlider, IHasPosition { public float X => Position.X; public float Y => Position.Y; public Vector2 Position { get; set; } - - public bool NewCombo { get; set; } - - public int ComboOffset { get; set; } } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertSpinner.cs b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertSpinner.cs index 014494ec54..5ef3d51cb3 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertSpinner.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertSpinner.cs @@ -8,16 +8,12 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch /// /// Legacy osu!catch Spinner-type, used for parsing Beatmaps. /// - internal sealed class ConvertSpinner : ConvertHitObject, IHasDuration, IHasXPosition, IHasCombo + internal sealed class ConvertSpinner : ConvertHitObject, IHasDuration, IHasXPosition { public double EndTime => StartTime + Duration; public double Duration { get; set; } public float X => 256; // Required for CatchBeatmapConverter - - public bool NewCombo { get; set; } - - public int ComboOffset { get; set; } } } diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObject.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObject.cs index 54dbd28c76..bb36aab0b3 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObject.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObject.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using osu.Game.Rulesets.Judgements; +using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Scoring; namespace osu.Game.Rulesets.Objects.Legacy @@ -9,8 +10,12 @@ namespace osu.Game.Rulesets.Objects.Legacy /// /// A hit object only used for conversion, not actual gameplay. /// - internal abstract class ConvertHitObject : HitObject + internal abstract class ConvertHitObject : HitObject, IHasCombo { + public bool NewCombo { get; set; } + + public int ComboOffset { get; set; } + public override Judgement CreateJudgement() => new IgnoreJudgement(); protected override HitWindows CreateHitWindows() => HitWindows.Empty; diff --git a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHit.cs b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHit.cs index 069366bad3..b7cd4b0dcc 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHit.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHit.cs @@ -9,16 +9,12 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu /// /// Legacy osu! Hit-type, used for parsing Beatmaps. /// - internal sealed class ConvertHit : ConvertHitObject, IHasPosition, IHasCombo + internal sealed class ConvertHit : ConvertHitObject, IHasPosition { public Vector2 Position { get; set; } public float X => Position.X; public float Y => Position.Y; - - public bool NewCombo { get; set; } - - public int ComboOffset { get; set; } } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSlider.cs b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSlider.cs index 790af6cfc1..8c37154f95 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSlider.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSlider.cs @@ -9,7 +9,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu /// /// Legacy osu! Slider-type, used for parsing Beatmaps. /// - internal sealed class ConvertSlider : Legacy.ConvertSlider, IHasPosition, IHasCombo, IHasGenerateTicks + internal sealed class ConvertSlider : Legacy.ConvertSlider, IHasPosition, IHasGenerateTicks { public Vector2 Position { get; set; } @@ -17,10 +17,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu public float Y => Position.Y; - public bool NewCombo { get; set; } - - public int ComboOffset { get; set; } - public bool GenerateTicks { get; set; } = true; } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSpinner.cs b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSpinner.cs index e9e5ca8c94..d6e24b6bbf 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSpinner.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSpinner.cs @@ -9,7 +9,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu /// /// Legacy osu! Spinner-type, used for parsing Beatmaps. /// - internal sealed class ConvertSpinner : ConvertHitObject, IHasDuration, IHasPosition, IHasCombo + internal sealed class ConvertSpinner : ConvertHitObject, IHasDuration, IHasPosition { public double Duration { get; set; } @@ -20,9 +20,5 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu public float X => Position.X; public float Y => Position.Y; - - public bool NewCombo { get; set; } - - public int ComboOffset { get; set; } } } From 7998204cfe1e529bc684b54f64356882ec2c81a2 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 23 Nov 2023 13:41:01 +0900 Subject: [PATCH 2/4] Fix combo/combo colouring issues around spinners --- .../Beatmaps/CatchBeatmapProcessor.cs | 16 ++++++ .../Objects/CatchHitObject.cs | 25 +++++++++ .../Beatmaps/OsuBeatmapProcessor.cs | 54 ++++++++++++------- osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs | 25 +++++++++ .../Formats/LegacyBeatmapDecoderTest.cs | 48 +++++++++++++++++ .../Resources/spinner-between-objects.osu | 38 +++++++++++++ osu.Game/Beatmaps/BeatmapProcessor.cs | 6 --- .../Legacy/Catch/ConvertHitObjectParser.cs | 38 ++++--------- .../Legacy/Osu/ConvertHitObjectParser.cs | 38 ++++--------- 9 files changed, 210 insertions(+), 78 deletions(-) create mode 100644 osu.Game.Tests/Resources/spinner-between-objects.osu diff --git a/osu.Game.Rulesets.Catch/Beatmaps/CatchBeatmapProcessor.cs b/osu.Game.Rulesets.Catch/Beatmaps/CatchBeatmapProcessor.cs index ab61b14ac4..7c81ca03d1 100644 --- a/osu.Game.Rulesets.Catch/Beatmaps/CatchBeatmapProcessor.cs +++ b/osu.Game.Rulesets.Catch/Beatmaps/CatchBeatmapProcessor.cs @@ -23,6 +23,22 @@ namespace osu.Game.Rulesets.Catch.Beatmaps { } + public override void PreProcess() + { + IHasComboInformation? lastObj = null; + + // For sanity, ensures that both the first hitobject and the first hitobject after a banana shower start a new combo. + // This is normally enforced by the legacy decoder, but is not enforced by the editor. + foreach (var obj in Beatmap.HitObjects.OfType()) + { + if (obj is not BananaShower && (lastObj == null || lastObj is BananaShower)) + obj.NewCombo = true; + lastObj = obj; + } + + base.PreProcess(); + } + public override void PostProcess() { base.PostProcess(); diff --git a/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs b/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs index b9fef6bf8c..d122758a4e 100644 --- a/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs +++ b/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs @@ -155,6 +155,31 @@ namespace osu.Game.Rulesets.Catch.Objects Scale = LegacyRulesetExtensions.CalculateScaleFromCircleSize(difficulty.CircleSize); } + public void UpdateComboInformation(IHasComboInformation? lastObj) + { + ComboIndex = lastObj?.ComboIndex ?? 0; + ComboIndexWithOffsets = lastObj?.ComboIndexWithOffsets ?? 0; + IndexInCurrentCombo = (lastObj?.IndexInCurrentCombo + 1) ?? 0; + + if (this is BananaShower) + { + // For the purpose of combo colours, spinners never start a new combo even if they are flagged as doing so. + return; + } + + // At decode time, the first hitobject in the beatmap and the first hitobject after a banana shower are both enforced to be a new combo, + // but this isn't directly enforced by the editor so the extra checks against the last hitobject are duplicated here. + if (NewCombo || lastObj == null || lastObj is BananaShower) + { + IndexInCurrentCombo = 0; + ComboIndex++; + ComboIndexWithOffsets += ComboOffset + 1; + + if (lastObj != null) + lastObj.LastInCombo = true; + } + } + protected override HitWindows CreateHitWindows() => HitWindows.Empty; #region Hit object conversion diff --git a/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs b/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs index c081df3ac6..835c67ff19 100644 --- a/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs +++ b/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs @@ -2,9 +2,11 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Linq; using osu.Framework.Graphics; using osu.Game.Beatmaps; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Osu.Objects; using osuTK; @@ -19,6 +21,22 @@ namespace osu.Game.Rulesets.Osu.Beatmaps { } + public override void PreProcess() + { + IHasComboInformation? lastObj = null; + + // For sanity, ensures that both the first hitobject and the first hitobject after a spinner start a new combo. + // This is normally enforced by the legacy decoder, but is not enforced by the editor. + foreach (var obj in Beatmap.HitObjects.OfType()) + { + if (obj is not Spinner && (lastObj == null || lastObj is Spinner)) + obj.NewCombo = true; + lastObj = obj; + } + + base.PreProcess(); + } + public override void PostProcess() { base.PostProcess(); @@ -95,15 +113,15 @@ namespace osu.Game.Rulesets.Osu.Beatmaps { int n = i; /* We should check every note which has not yet got a stack. - * Consider the case we have two interwound stacks and this will make sense. - * - * o <-1 o <-2 - * o <-3 o <-4 - * - * We first process starting from 4 and handle 2, - * then we come backwards on the i loop iteration until we reach 3 and handle 1. - * 2 and 1 will be ignored in the i loop because they already have a stack value. - */ + * Consider the case we have two interwound stacks and this will make sense. + * + * o <-1 o <-2 + * o <-3 o <-4 + * + * We first process starting from 4 and handle 2, + * then we come backwards on the i loop iteration until we reach 3 and handle 1. + * 2 and 1 will be ignored in the i loop because they already have a stack value. + */ OsuHitObject objectI = beatmap.HitObjects[i]; if (objectI.StackHeight != 0 || objectI is Spinner) continue; @@ -111,9 +129,9 @@ namespace osu.Game.Rulesets.Osu.Beatmaps double stackThreshold = objectI.TimePreempt * beatmap.BeatmapInfo.StackLeniency; /* If this object is a hitcircle, then we enter this "special" case. - * It either ends with a stack of hitcircles only, or a stack of hitcircles that are underneath a slider. - * Any other case is handled by the "is Slider" code below this. - */ + * It either ends with a stack of hitcircles only, or a stack of hitcircles that are underneath a slider. + * Any other case is handled by the "is Slider" code below this. + */ if (objectI is HitCircle) { while (--n >= 0) @@ -135,10 +153,10 @@ namespace osu.Game.Rulesets.Osu.Beatmaps } /* This is a special case where hticircles are moved DOWN and RIGHT (negative stacking) if they are under the *last* slider in a stacked pattern. - * o==o <- slider is at original location - * o <- hitCircle has stack of -1 - * o <- hitCircle has stack of -2 - */ + * o==o <- slider is at original location + * o <- hitCircle has stack of -1 + * o <- hitCircle has stack of -2 + */ if (objectN is Slider && Vector2Extensions.Distance(objectN.EndPosition, objectI.Position) < stack_distance) { int offset = objectI.StackHeight - objectN.StackHeight + 1; @@ -169,8 +187,8 @@ namespace osu.Game.Rulesets.Osu.Beatmaps else if (objectI is Slider) { /* We have hit the first slider in a possible stack. - * From this point on, we ALWAYS stack positive regardless. - */ + * From this point on, we ALWAYS stack positive regardless. + */ while (--n >= startIndex) { OsuHitObject objectN = beatmap.HitObjects[n]; diff --git a/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs b/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs index d74d28c748..716c34d024 100644 --- a/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs +++ b/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs @@ -159,6 +159,31 @@ namespace osu.Game.Rulesets.Osu.Objects Scale = LegacyRulesetExtensions.CalculateScaleFromCircleSize(difficulty.CircleSize, true); } + public void UpdateComboInformation(IHasComboInformation? lastObj) + { + ComboIndex = lastObj?.ComboIndex ?? 0; + ComboIndexWithOffsets = lastObj?.ComboIndexWithOffsets ?? 0; + IndexInCurrentCombo = (lastObj?.IndexInCurrentCombo + 1) ?? 0; + + if (this is Spinner) + { + // For the purpose of combo colours, spinners never start a new combo even if they are flagged as doing so. + return; + } + + // At decode time, the first hitobject in the beatmap and the first hitobject after a spinner are both enforced to be a new combo, + // but this isn't directly enforced by the editor so the extra checks against the last hitobject are duplicated here. + if (NewCombo || lastObj == null || lastObj is Spinner) + { + IndexInCurrentCombo = 0; + ComboIndex++; + ComboIndexWithOffsets += ComboOffset + 1; + + if (lastObj != null) + lastObj.LastInCombo = true; + } + } + protected override HitWindows CreateHitWindows() => new OsuHitWindows(); } } diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index be1993957f..20f59384ab 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -12,6 +12,7 @@ using osu.Game.Beatmaps.Formats; using osu.Game.Beatmaps.Legacy; using osu.Game.Beatmaps.Timing; using osu.Game.IO; +using osu.Game.Rulesets; using osu.Game.Rulesets.Catch; using osu.Game.Rulesets.Catch.Beatmaps; using osu.Game.Rulesets.Mods; @@ -1108,5 +1109,52 @@ namespace osu.Game.Tests.Beatmaps.Formats Assert.That(((IHasCombo)beatmap.HitObjects[2]).NewCombo, Is.False); } } + + /// + /// Test cases that involve a spinner between two hitobjects. + /// + [Test] + public void TestSpinnerNewComboBetweenObjects([Values("osu", "catch")] string rulesetName) + { + var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; + + using (var resStream = TestResources.OpenResource("spinner-between-objects.osu")) + using (var stream = new LineBufferedReader(resStream)) + { + Ruleset ruleset; + + switch (rulesetName) + { + case "osu": + ruleset = new OsuRuleset(); + break; + + case "catch": + ruleset = new CatchRuleset(); + break; + + default: + throw new ArgumentOutOfRangeException(nameof(rulesetName), rulesetName, null); + } + + var working = new TestWorkingBeatmap(decoder.Decode(stream)); + var playable = working.GetPlayableBeatmap(ruleset.RulesetInfo, Array.Empty()); + + // There's no good way to figure out these values other than to compare (in code) with osu!stable... + + Assert.That(((IHasComboInformation)playable.HitObjects[0]).ComboIndexWithOffsets, Is.EqualTo(1)); + Assert.That(((IHasComboInformation)playable.HitObjects[2]).ComboIndexWithOffsets, Is.EqualTo(2)); + Assert.That(((IHasComboInformation)playable.HitObjects[3]).ComboIndexWithOffsets, Is.EqualTo(2)); + Assert.That(((IHasComboInformation)playable.HitObjects[5]).ComboIndexWithOffsets, Is.EqualTo(3)); + Assert.That(((IHasComboInformation)playable.HitObjects[6]).ComboIndexWithOffsets, Is.EqualTo(3)); + Assert.That(((IHasComboInformation)playable.HitObjects[8]).ComboIndexWithOffsets, Is.EqualTo(4)); + Assert.That(((IHasComboInformation)playable.HitObjects[9]).ComboIndexWithOffsets, Is.EqualTo(4)); + Assert.That(((IHasComboInformation)playable.HitObjects[11]).ComboIndexWithOffsets, Is.EqualTo(5)); + Assert.That(((IHasComboInformation)playable.HitObjects[12]).ComboIndexWithOffsets, Is.EqualTo(6)); + Assert.That(((IHasComboInformation)playable.HitObjects[14]).ComboIndexWithOffsets, Is.EqualTo(7)); + Assert.That(((IHasComboInformation)playable.HitObjects[15]).ComboIndexWithOffsets, Is.EqualTo(8)); + Assert.That(((IHasComboInformation)playable.HitObjects[17]).ComboIndexWithOffsets, Is.EqualTo(9)); + } + } } } diff --git a/osu.Game.Tests/Resources/spinner-between-objects.osu b/osu.Game.Tests/Resources/spinner-between-objects.osu new file mode 100644 index 0000000000..03e61d965c --- /dev/null +++ b/osu.Game.Tests/Resources/spinner-between-objects.osu @@ -0,0 +1,38 @@ +osu file format v14 + +[General] +Mode: 0 + +[TimingPoints] +0,571.428571428571,4,2,1,5,1,0 + +[HitObjects] +// +C -> +C -> +C +104,95,0,5,0,0:0:0:0: +256,192,1000,12,0,2000,0:0:0:0: +178,171,3000,5,0,0:0:0:0: + +// -C -> +C -> +C +178,171,4000,1,0,0:0:0:0: +256,192,5000,12,0,6000,0:0:0:0: +178,171,7000,5,0,0:0:0:0: + +// -C -> -C -> +C +178,171,8000,1,0,0:0:0:0: +256,192,9000,8,0,10000,0:0:0:0: +178,171,11000,5,0,0:0:0:0: + +// -C -> -C -> -C +178,171,12000,1,0,0:0:0:0: +256,192,13000,8,0,14000,0:0:0:0: +178,171,15000,1,0,0:0:0:0: + +// +C -> -C -> -C +178,171,16000,5,0,0:0:0:0: +256,192,17000,8,0,18000,0:0:0:0: +178,171,19000,1,0,0:0:0:0: + +// +C -> +C -> -C +178,171,20000,5,0,0:0:0:0: +256,192,21000,12,0,22000,0:0:0:0: +178,171,23000,1,0,0:0:0:0: \ No newline at end of file diff --git a/osu.Game/Beatmaps/BeatmapProcessor.cs b/osu.Game/Beatmaps/BeatmapProcessor.cs index fb5313469f..89d6e9d3f8 100644 --- a/osu.Game/Beatmaps/BeatmapProcessor.cs +++ b/osu.Game/Beatmaps/BeatmapProcessor.cs @@ -24,12 +24,6 @@ namespace osu.Game.Beatmaps foreach (var obj in Beatmap.HitObjects.OfType()) { - if (lastObj == null) - { - // first hitobject should always be marked as a new combo for sanity. - obj.NewCombo = true; - } - obj.UpdateComboInformation(lastObj); lastObj = obj; } diff --git a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs index 4861e8b3f7..0ed5aef0cf 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs @@ -14,26 +14,19 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch /// public class ConvertHitObjectParser : Legacy.ConvertHitObjectParser { + private ConvertHitObject lastObject; + public ConvertHitObjectParser(double offset, int formatVersion) : base(offset, formatVersion) { } - private bool forceNewCombo; - private int extraComboOffset; - protected override HitObject CreateHit(Vector2 position, bool newCombo, int comboOffset) { - newCombo |= forceNewCombo; - comboOffset += extraComboOffset; - - forceNewCombo = false; - extraComboOffset = 0; - - return new ConvertHit + return lastObject = new ConvertHit { Position = position, - NewCombo = newCombo, + NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo, ComboOffset = comboOffset }; } @@ -41,16 +34,10 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch protected override HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, PathControlPoint[] controlPoints, double? length, int repeatCount, IList> nodeSamples) { - newCombo |= forceNewCombo; - comboOffset += extraComboOffset; - - forceNewCombo = false; - extraComboOffset = 0; - - return new ConvertSlider + return lastObject = new ConvertSlider { Position = position, - NewCombo = FirstObject || newCombo, + NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo, ComboOffset = comboOffset, Path = new SliderPath(controlPoints, length), NodeSamples = nodeSamples, @@ -60,20 +47,17 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch protected override HitObject CreateSpinner(Vector2 position, bool newCombo, int comboOffset, double duration) { - // Convert spinners don't create the new combo themselves, but force the next non-spinner hitobject to create a new combo - // Their combo offset is still added to that next hitobject's combo index - forceNewCombo |= FormatVersion <= 8 || newCombo; - extraComboOffset += comboOffset; - - return new ConvertSpinner + return lastObject = new ConvertSpinner { - Duration = duration + Duration = duration, + NewCombo = newCombo + // Spinners cannot have combo offset. }; } protected override HitObject CreateHold(Vector2 position, bool newCombo, int comboOffset, double duration) { - return null; + return lastObject = null; } } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs index 7a88a31bd5..8bb9600a7d 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs @@ -14,26 +14,19 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu /// public class ConvertHitObjectParser : Legacy.ConvertHitObjectParser { + private ConvertHitObject lastObject; + public ConvertHitObjectParser(double offset, int formatVersion) : base(offset, formatVersion) { } - private bool forceNewCombo; - private int extraComboOffset; - protected override HitObject CreateHit(Vector2 position, bool newCombo, int comboOffset) { - newCombo |= forceNewCombo; - comboOffset += extraComboOffset; - - forceNewCombo = false; - extraComboOffset = 0; - - return new ConvertHit + return lastObject = new ConvertHit { Position = position, - NewCombo = FirstObject || newCombo, + NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo, ComboOffset = comboOffset }; } @@ -41,16 +34,10 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu protected override HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, PathControlPoint[] controlPoints, double? length, int repeatCount, IList> nodeSamples) { - newCombo |= forceNewCombo; - comboOffset += extraComboOffset; - - forceNewCombo = false; - extraComboOffset = 0; - - return new ConvertSlider + return lastObject = new ConvertSlider { Position = position, - NewCombo = FirstObject || newCombo, + NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo, ComboOffset = comboOffset, Path = new SliderPath(controlPoints, length), NodeSamples = nodeSamples, @@ -60,21 +47,18 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu protected override HitObject CreateSpinner(Vector2 position, bool newCombo, int comboOffset, double duration) { - // Convert spinners don't create the new combo themselves, but force the next non-spinner hitobject to create a new combo - // Their combo offset is still added to that next hitobject's combo index - forceNewCombo |= FormatVersion <= 8 || newCombo; - extraComboOffset += comboOffset; - - return new ConvertSpinner + return lastObject = new ConvertSpinner { Position = position, - Duration = duration + Duration = duration, + NewCombo = newCombo + // Spinners cannot have combo offset. }; } protected override HitObject CreateHold(Vector2 position, bool newCombo, int comboOffset, double duration) { - return null; + return lastObject = null; } } } From 10e16e4b04c58774573c1b8fff66ee46b717bcdc Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 24 Nov 2023 09:46:06 +0900 Subject: [PATCH 3/4] Fix handling of combo offset without new combo, and incorrect lazer tests --- .../Formats/LegacyBeatmapDecoderTest.cs | 24 +++++++++---------- .../Resources/hitobject-combo-offset.osu | 12 +++++----- .../Legacy/Catch/ConvertHitObjectParser.cs | 4 ++-- .../Legacy/Osu/ConvertHitObjectParser.cs | 4 ++-- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index cccceaf58e..dcfe8ecb41 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -434,12 +434,12 @@ namespace osu.Game.Tests.Beatmaps.Formats new OsuBeatmapProcessor(converted).PreProcess(); new OsuBeatmapProcessor(converted).PostProcess(); - Assert.AreEqual(4, ((IHasComboInformation)converted.HitObjects.ElementAt(0)).ComboIndexWithOffsets); - Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(2)).ComboIndexWithOffsets); - Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(4)).ComboIndexWithOffsets); - Assert.AreEqual(6, ((IHasComboInformation)converted.HitObjects.ElementAt(6)).ComboIndexWithOffsets); - Assert.AreEqual(11, ((IHasComboInformation)converted.HitObjects.ElementAt(8)).ComboIndexWithOffsets); - Assert.AreEqual(14, ((IHasComboInformation)converted.HitObjects.ElementAt(11)).ComboIndexWithOffsets); + Assert.AreEqual(1, ((IHasComboInformation)converted.HitObjects.ElementAt(0)).ComboIndexWithOffsets); + Assert.AreEqual(2, ((IHasComboInformation)converted.HitObjects.ElementAt(2)).ComboIndexWithOffsets); + Assert.AreEqual(3, ((IHasComboInformation)converted.HitObjects.ElementAt(4)).ComboIndexWithOffsets); + Assert.AreEqual(4, ((IHasComboInformation)converted.HitObjects.ElementAt(6)).ComboIndexWithOffsets); + Assert.AreEqual(8, ((IHasComboInformation)converted.HitObjects.ElementAt(8)).ComboIndexWithOffsets); + Assert.AreEqual(9, ((IHasComboInformation)converted.HitObjects.ElementAt(11)).ComboIndexWithOffsets); } } @@ -457,12 +457,12 @@ namespace osu.Game.Tests.Beatmaps.Formats new CatchBeatmapProcessor(converted).PreProcess(); new CatchBeatmapProcessor(converted).PostProcess(); - Assert.AreEqual(4, ((IHasComboInformation)converted.HitObjects.ElementAt(0)).ComboIndexWithOffsets); - Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(2)).ComboIndexWithOffsets); - Assert.AreEqual(5, ((IHasComboInformation)converted.HitObjects.ElementAt(4)).ComboIndexWithOffsets); - Assert.AreEqual(6, ((IHasComboInformation)converted.HitObjects.ElementAt(6)).ComboIndexWithOffsets); - Assert.AreEqual(11, ((IHasComboInformation)converted.HitObjects.ElementAt(8)).ComboIndexWithOffsets); - Assert.AreEqual(14, ((IHasComboInformation)converted.HitObjects.ElementAt(11)).ComboIndexWithOffsets); + Assert.AreEqual(1, ((IHasComboInformation)converted.HitObjects.ElementAt(0)).ComboIndexWithOffsets); + Assert.AreEqual(2, ((IHasComboInformation)converted.HitObjects.ElementAt(2)).ComboIndexWithOffsets); + Assert.AreEqual(3, ((IHasComboInformation)converted.HitObjects.ElementAt(4)).ComboIndexWithOffsets); + Assert.AreEqual(4, ((IHasComboInformation)converted.HitObjects.ElementAt(6)).ComboIndexWithOffsets); + Assert.AreEqual(8, ((IHasComboInformation)converted.HitObjects.ElementAt(8)).ComboIndexWithOffsets); + Assert.AreEqual(9, ((IHasComboInformation)converted.HitObjects.ElementAt(11)).ComboIndexWithOffsets); } } diff --git a/osu.Game.Tests/Resources/hitobject-combo-offset.osu b/osu.Game.Tests/Resources/hitobject-combo-offset.osu index d39a3e8548..9f39229d87 100644 --- a/osu.Game.Tests/Resources/hitobject-combo-offset.osu +++ b/osu.Game.Tests/Resources/hitobject-combo-offset.osu @@ -3,30 +3,30 @@ osu file format v14 [HitObjects] // Circle with combo offset (3) 255,193,1000,49,0,0:0:0:0: -// Combo index = 4 +// Combo index = 1 // Spinner with new combo followed by circle with no new combo 256,192,2000,12,0,2000,0:0:0:0: 255,193,3000,1,0,0:0:0:0: -// Combo index = 5 +// Combo index = 2 // Spinner without new combo followed by circle with no new combo 256,192,4000,8,0,5000,0:0:0:0: 255,193,6000,1,0,0:0:0:0: -// Combo index = 5 +// Combo index = 3 // Spinner without new combo followed by circle with new combo 256,192,7000,8,0,8000,0:0:0:0: 255,193,9000,5,0,0:0:0:0: -// Combo index = 6 +// Combo index = 4 // Spinner with new combo and offset (1) followed by circle with new combo and offset (3) 256,192,10000,28,0,11000,0:0:0:0: 255,193,12000,53,0,0:0:0:0: -// Combo index = 11 +// Combo index = 8 // Spinner with new combo and offset (2) followed by slider with no new combo followed by circle with no new combo 256,192,13000,44,0,14000,0:0:0:0: 256,192,15000,8,0,16000,0:0:0:0: 255,193,17000,1,0,0:0:0:0: -// Combo index = 14 \ No newline at end of file +// Combo index = 9 \ No newline at end of file diff --git a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs index 0ed5aef0cf..a5c1a73fa7 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs @@ -27,7 +27,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch { Position = position, NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo, - ComboOffset = comboOffset + ComboOffset = newCombo ? comboOffset : 0 }; } @@ -38,7 +38,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch { Position = position, NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo, - ComboOffset = comboOffset, + ComboOffset = newCombo ? comboOffset : 0, Path = new SliderPath(controlPoints, length), NodeSamples = nodeSamples, RepeatCount = repeatCount diff --git a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs index 8bb9600a7d..43c346b621 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs @@ -27,7 +27,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu { Position = position, NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo, - ComboOffset = comboOffset + ComboOffset = newCombo ? comboOffset : 0 }; } @@ -38,7 +38,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu { Position = position, NewCombo = FirstObject || lastObject is ConvertSpinner || newCombo, - ComboOffset = comboOffset, + ComboOffset = newCombo ? comboOffset : 0, Path = new SliderPath(controlPoints, length), NodeSamples = nodeSamples, RepeatCount = repeatCount From 039f8e62421d13d23f532e146cadd97ec89e78d1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 24 Nov 2023 10:25:23 +0900 Subject: [PATCH 4/4] Add note about shared code --- osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs | 2 ++ osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs b/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs index d122758a4e..17ff8afb87 100644 --- a/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs +++ b/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs @@ -157,6 +157,8 @@ namespace osu.Game.Rulesets.Catch.Objects public void UpdateComboInformation(IHasComboInformation? lastObj) { + // Note that this implementation is shared with the osu! ruleset's implementation. + // If a change is made here, OsuHitObject.cs should also be updated. ComboIndex = lastObj?.ComboIndex ?? 0; ComboIndexWithOffsets = lastObj?.ComboIndexWithOffsets ?? 0; IndexInCurrentCombo = (lastObj?.IndexInCurrentCombo + 1) ?? 0; diff --git a/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs b/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs index 716c34d024..21f7b4b22d 100644 --- a/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs +++ b/osu.Game.Rulesets.Osu/Objects/OsuHitObject.cs @@ -161,6 +161,8 @@ namespace osu.Game.Rulesets.Osu.Objects public void UpdateComboInformation(IHasComboInformation? lastObj) { + // Note that this implementation is shared with the osu!catch ruleset's implementation. + // If a change is made here, CatchHitObject.cs should also be updated. ComboIndex = lastObj?.ComboIndex ?? 0; ComboIndexWithOffsets = lastObj?.ComboIndexWithOffsets ?? 0; IndexInCurrentCombo = (lastObj?.IndexInCurrentCombo + 1) ?? 0;