From f927cb59285ff730f7c8ad7426775f992c0f1352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 31 Jan 2024 12:29:18 +0100 Subject: [PATCH 1/3] Increase repeat count for better coverage of flip operations --- .../OsuHitObjectGenerationUtilsTest.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/OsuHitObjectGenerationUtilsTest.cs b/osu.Game.Rulesets.Osu.Tests/OsuHitObjectGenerationUtilsTest.cs index d78c32aa6a..ffb9ad1784 100644 --- a/osu.Game.Rulesets.Osu.Tests/OsuHitObjectGenerationUtilsTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/OsuHitObjectGenerationUtilsTest.cs @@ -31,7 +31,7 @@ namespace osu.Game.Rulesets.Osu.Tests new PathControlPoint(new Vector2(-128, 0), PathType.LINEAR) // absolute position: (0, 128) } }, - RepeatCount = 1 + RepeatCount = 2 }; slider.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); return slider; @@ -45,7 +45,7 @@ namespace osu.Game.Rulesets.Osu.Tests OsuHitObjectGenerationUtils.ReflectHorizontallyAlongPlayfield(slider); Assert.That(slider.Position, Is.EqualTo(new Vector2(OsuPlayfield.BASE_SIZE.X - 128, 128))); - Assert.That(slider.NestedHitObjects.OfType().Single().Position, Is.EqualTo(new Vector2(OsuPlayfield.BASE_SIZE.X - 0, 128))); + Assert.That(slider.NestedHitObjects.OfType().First().Position, Is.EqualTo(new Vector2(OsuPlayfield.BASE_SIZE.X - 0, 128))); Assert.That(slider.Path.ControlPoints.Select(point => point.Position), Is.EquivalentTo(new[] { new Vector2(), @@ -62,7 +62,7 @@ namespace osu.Game.Rulesets.Osu.Tests OsuHitObjectGenerationUtils.ReflectVerticallyAlongPlayfield(slider); Assert.That(slider.Position, Is.EqualTo(new Vector2(128, OsuPlayfield.BASE_SIZE.Y - 128))); - Assert.That(slider.NestedHitObjects.OfType().Single().Position, Is.EqualTo(new Vector2(0, OsuPlayfield.BASE_SIZE.Y - 128))); + Assert.That(slider.NestedHitObjects.OfType().First().Position, Is.EqualTo(new Vector2(0, OsuPlayfield.BASE_SIZE.Y - 128))); Assert.That(slider.Path.ControlPoints.Select(point => point.Position), Is.EquivalentTo(new[] { new Vector2(), @@ -79,7 +79,7 @@ namespace osu.Game.Rulesets.Osu.Tests OsuHitObjectGenerationUtils.FlipSliderInPlaceHorizontally(slider); Assert.That(slider.Position, Is.EqualTo(new Vector2(128, 128))); - Assert.That(slider.NestedHitObjects.OfType().Single().Position, Is.EqualTo(new Vector2(256, 128))); + Assert.That(slider.NestedHitObjects.OfType().First().Position, Is.EqualTo(new Vector2(256, 128))); Assert.That(slider.Path.ControlPoints.Select(point => point.Position), Is.EquivalentTo(new[] { new Vector2(), From a934556bb8e4d266b67c35bbc8c328d540ee35cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 31 Jan 2024 12:34:38 +0100 Subject: [PATCH 2/3] Add failing assertions for head/tail positions after flip --- .../OsuHitObjectGenerationUtilsTest.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/OsuHitObjectGenerationUtilsTest.cs b/osu.Game.Rulesets.Osu.Tests/OsuHitObjectGenerationUtilsTest.cs index ffb9ad1784..77ef4627cb 100644 --- a/osu.Game.Rulesets.Osu.Tests/OsuHitObjectGenerationUtilsTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/OsuHitObjectGenerationUtilsTest.cs @@ -45,7 +45,9 @@ namespace osu.Game.Rulesets.Osu.Tests OsuHitObjectGenerationUtils.ReflectHorizontallyAlongPlayfield(slider); Assert.That(slider.Position, Is.EqualTo(new Vector2(OsuPlayfield.BASE_SIZE.X - 128, 128))); + Assert.That(slider.NestedHitObjects.OfType().Single().Position, Is.EqualTo(new Vector2(OsuPlayfield.BASE_SIZE.X - 128, 128))); Assert.That(slider.NestedHitObjects.OfType().First().Position, Is.EqualTo(new Vector2(OsuPlayfield.BASE_SIZE.X - 0, 128))); + Assert.That(slider.NestedHitObjects.OfType().Single().Position, Is.EqualTo(new Vector2(OsuPlayfield.BASE_SIZE.X, 128))); Assert.That(slider.Path.ControlPoints.Select(point => point.Position), Is.EquivalentTo(new[] { new Vector2(), @@ -62,7 +64,9 @@ namespace osu.Game.Rulesets.Osu.Tests OsuHitObjectGenerationUtils.ReflectVerticallyAlongPlayfield(slider); Assert.That(slider.Position, Is.EqualTo(new Vector2(128, OsuPlayfield.BASE_SIZE.Y - 128))); + Assert.That(slider.NestedHitObjects.OfType().Single().Position, Is.EqualTo(new Vector2(128, OsuPlayfield.BASE_SIZE.Y - 128))); Assert.That(slider.NestedHitObjects.OfType().First().Position, Is.EqualTo(new Vector2(0, OsuPlayfield.BASE_SIZE.Y - 128))); + Assert.That(slider.NestedHitObjects.OfType().Single().Position, Is.EqualTo(new Vector2(0, OsuPlayfield.BASE_SIZE.Y - 128))); Assert.That(slider.Path.ControlPoints.Select(point => point.Position), Is.EquivalentTo(new[] { new Vector2(), @@ -79,7 +83,9 @@ namespace osu.Game.Rulesets.Osu.Tests OsuHitObjectGenerationUtils.FlipSliderInPlaceHorizontally(slider); Assert.That(slider.Position, Is.EqualTo(new Vector2(128, 128))); + Assert.That(slider.NestedHitObjects.OfType().Single().Position, Is.EqualTo(new Vector2(128, 128))); Assert.That(slider.NestedHitObjects.OfType().First().Position, Is.EqualTo(new Vector2(256, 128))); + Assert.That(slider.NestedHitObjects.OfType().Single().Position, Is.EqualTo(new Vector2(256, 128))); Assert.That(slider.Path.ControlPoints.Select(point => point.Position), Is.EquivalentTo(new[] { new Vector2(), From dfea2ade6d7d1dda0b080fca826cec767a480199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 31 Jan 2024 12:43:24 +0100 Subject: [PATCH 3/3] Revert incorrect end position optimisation Closes https://github.com/ppy/osu/issues/26867. Reverts 882f49039029b7dc3e287ccc302d04de89de10df and ce643aa68f35369be1a975bb1ceb69fb54192cf2. The applied optimisation may have been valid as long as it was constrained to `Slider`. But it is not, as `SliderTailCircle` stores a local copy of the object position. And as the commit message of ce643aa68f35369be1a975bb1ceb69fb54192cf2 states, this could be bypassed by some pretty hacky delegation from `SliderTailCircle.Position` to the slider, but it'd also be pretty hacky because it would make flows like `PositionBindable` break down. Long-term solution is to probably remove bindables from hitobjects. --- osu.Game.Rulesets.Osu/Objects/Slider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Slider.cs b/osu.Game.Rulesets.Osu/Objects/Slider.cs index 032f105ded..506145568e 100644 --- a/osu.Game.Rulesets.Osu/Objects/Slider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Slider.cs @@ -98,7 +98,7 @@ namespace osu.Game.Rulesets.Osu.Objects set { repeatCount = value; - endPositionCache.Invalidate(); + updateNestedPositions(); } } @@ -165,7 +165,7 @@ namespace osu.Game.Rulesets.Osu.Objects public Slider() { SamplesBindable.CollectionChanged += (_, _) => UpdateNestedSamples(); - Path.Version.ValueChanged += _ => endPositionCache.Invalidate(); + Path.Version.ValueChanged += _ => updateNestedPositions(); } protected override void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, IBeatmapDifficultyInfo difficulty)