Merge pull request #13556 from bdach/editor-hit-circle-animation-regression

Fix editor hit circle animation toggle not working

osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuEditorHitAnimations.cs

@@ -0,0 +1,114 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using NUnit.Framework;
+using osu.Framework.Allocation;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Transforms;
+using osu.Framework.Testing;
+using osu.Framework.Utils;
+using osu.Game.Configuration;
+using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Objects.Drawables;
+using osu.Game.Rulesets.Osu.Edit;
+using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Rulesets.Osu.Objects.Drawables;
+
+namespace osu.Game.Rulesets.Osu.Tests.Editor
+{
+    [TestFixture]
+    public class TestSceneOsuEditorHitAnimations : TestSceneOsuEditor
+    {
+        [Resolved]
+        private OsuConfigManager config { get; set; }
+
+        [Test]
+        public void TestHitCircleAnimationDisable()
+        {
+            HitCircle hitCircle = null;
+            DrawableHitCircle drawableHitCircle = null;
+
+            AddStep("retrieve first hit circle", () => hitCircle = getHitCircle(0));
+            toggleAnimations(true);
+            seekSmoothlyTo(() => hitCircle.StartTime + 10);
+
+            AddStep("retrieve drawable", () => drawableHitCircle = (DrawableHitCircle)getDrawableObjectFor(hitCircle));
+            assertFutureTransforms(() => drawableHitCircle.CirclePiece, true);
+
+            AddStep("retrieve second hit circle", () => hitCircle = getHitCircle(1));
+            toggleAnimations(false);
+            seekSmoothlyTo(() => hitCircle.StartTime + 10);
+
+            AddStep("retrieve drawable", () => drawableHitCircle = (DrawableHitCircle)getDrawableObjectFor(hitCircle));
+            assertFutureTransforms(() => drawableHitCircle.CirclePiece, false);
+            AddAssert("hit circle has longer fade-out applied", () =>
+            {
+                var alphaTransform = drawableHitCircle.Transforms.Last(t => t.TargetMember == nameof(Alpha));
+                return alphaTransform.EndTime - alphaTransform.StartTime == DrawableOsuEditorRuleset.EDITOR_HIT_OBJECT_FADE_OUT_EXTENSION;
+            });
+        }
+
+        [Test]
+        public void TestSliderAnimationDisable()
+        {
+            Slider slider = null;
+            DrawableSlider drawableSlider = null;
+            DrawableSliderRepeat sliderRepeat = null;
+
+            AddStep("retrieve first slider with repeats", () => slider = getSliderWithRepeats(0));
+            toggleAnimations(true);
+            seekSmoothlyTo(() => slider.StartTime + slider.SpanDuration + 10);
+
+            retrieveDrawables();
+            assertFutureTransforms(() => sliderRepeat, true);
+
+            AddStep("retrieve second slider with repeats", () => slider = getSliderWithRepeats(1));
+            toggleAnimations(false);
+            seekSmoothlyTo(() => slider.StartTime + slider.SpanDuration + 10);
+
+            retrieveDrawables();
+            assertFutureTransforms(() => sliderRepeat.Arrow, false);
+            seekSmoothlyTo(() => slider.GetEndTime());
+            AddAssert("slider has longer fade-out applied", () =>
+            {
+                var alphaTransform = drawableSlider.Transforms.Last(t => t.TargetMember == nameof(Alpha));
+                return alphaTransform.EndTime - alphaTransform.StartTime == DrawableOsuEditorRuleset.EDITOR_HIT_OBJECT_FADE_OUT_EXTENSION;
+            });
+
+            void retrieveDrawables() =>
+                AddStep("retrieve drawables", () =>
+                {
+                    drawableSlider = (DrawableSlider)getDrawableObjectFor(slider);
+                    sliderRepeat = (DrawableSliderRepeat)getDrawableObjectFor(slider.NestedHitObjects.OfType<SliderRepeat>().First());
+                });
+        }
+
+        private HitCircle getHitCircle(int index)
+            => EditorBeatmap.HitObjects.OfType<HitCircle>().ElementAt(index);
+
+        private Slider getSliderWithRepeats(int index)
+            => EditorBeatmap.HitObjects.OfType<Slider>().Where(s => s.RepeatCount >= 1).ElementAt(index);
+
+        private DrawableHitObject getDrawableObjectFor(HitObject hitObject)
+            => this.ChildrenOfType<DrawableHitObject>().Single(ho => ho.HitObject == hitObject);
+
+        private IEnumerable<Transform> getTransformsRecursively(Drawable drawable)
+            => drawable.ChildrenOfType<Drawable>().SelectMany(d => d.Transforms);
+
+        private void toggleAnimations(bool enabled)
+            => AddStep($"toggle animations {(enabled ? "on" : "off")}", () => config.SetValue(OsuSetting.EditorHitAnimations, enabled));
+
+        private void seekSmoothlyTo(Func<double> targetTime)
+        {
+            AddStep("seek smoothly", () => EditorClock.SeekSmoothlyTo(targetTime.Invoke()));
+            AddUntilStep("wait for seek", () => Precision.AlmostEquals(targetTime.Invoke(), EditorClock.CurrentTime));
+        }
+
+        private void assertFutureTransforms(Func<Drawable> getDrawable, bool hasFutureTransforms)
+            => AddAssert($"object {(hasFutureTransforms ? "has" : "has no")} future transforms",
+                () => getTransformsRecursively(getDrawable()).Any(t => t.EndTime >= EditorClock.CurrentTime) == hasFutureTransforms);
+    }
+}

osu.Game.Rulesets.Osu/Edit/DrawableOsuEditorRuleset.cs

@@ -20,6 +20,12 @@ namespace osu.Game.Rulesets.Osu.Edit
 {
     public class DrawableOsuEditorRuleset : DrawableOsuRuleset
     {
+        /// <summary>
+        /// Hit objects are intentionally made to fade out at a constant slower rate than in gameplay.
+        /// This allows a mapper to gain better historical context and use recent hitobjects as reference / snap points.
+        /// </summary>
+        public const double EDITOR_HIT_OBJECT_FADE_OUT_EXTENSION = 700;
+
         public DrawableOsuEditorRuleset(Ruleset ruleset, IBeatmap beatmap, IReadOnlyList<Mod> mods)
             : base(ruleset, beatmap, mods)
         {
@@ -46,12 +52,6 @@ namespace osu.Game.Rulesets.Osu.Edit
                 d.ApplyCustomUpdateState += updateState;
             }
 
-            /// <summary>
-            /// Hit objects are intentionally made to fade out at a constant slower rate than in gameplay.
-            /// This allows a mapper to gain better historical context and use recent hitobjects as reference / snap points.
-            /// </summary>
-            private const double editor_hit_object_fade_out_extension = 700;
-
             private void updateState(DrawableHitObject hitObject, ArmedState state)
             {
                 if (state == ArmedState.Idle || hitAnimations.Value)
@@ -60,7 +60,7 @@ namespace osu.Game.Rulesets.Osu.Edit
                 if (hitObject is DrawableHitCircle circle)
                 {
                     circle.ApproachCircle
-                          .FadeOutFromOne(editor_hit_object_fade_out_extension * 4)
+                          .FadeOutFromOne(EDITOR_HIT_OBJECT_FADE_OUT_EXTENSION * 4)
                           .Expire();
 
                     circle.ApproachCircle.ScaleTo(1.1f, 300, Easing.OutQuint);
@@ -69,14 +69,20 @@ namespace osu.Game.Rulesets.Osu.Edit
                 if (hitObject is IHasMainCirclePiece mainPieceContainer)
                 {
                     // clear any explode animation logic.
-                    mainPieceContainer.CirclePiece.ApplyTransformsAt(hitObject.HitStateUpdateTime, true);
+                    // this is scheduled after children to ensure that the clear happens after invocations of ApplyCustomUpdateState on the circle piece's nested skinnables.
-                    mainPieceContainer.CirclePiece.ClearTransformsAfter(hitObject.HitStateUpdateTime, true);
+                    ScheduleAfterChildren(() =>
+                    {
+                        if (hitObject.HitObject == null) return;
+
+                        mainPieceContainer.CirclePiece.ApplyTransformsAt(hitObject.StateUpdateTime, true);
+                        mainPieceContainer.CirclePiece.ClearTransformsAfter(hitObject.StateUpdateTime, true);
+                    });
                 }
 
                 if (hitObject is DrawableSliderRepeat repeat)
                 {
-                    repeat.Arrow.ApplyTransformsAt(hitObject.HitStateUpdateTime, true);
+                    repeat.Arrow.ApplyTransformsAt(hitObject.StateUpdateTime, true);
-                    repeat.Arrow.ClearTransformsAfter(hitObject.HitStateUpdateTime, true);
+                    repeat.Arrow.ClearTransformsAfter(hitObject.StateUpdateTime, true);
                 }
 
                 // adjust the visuals of top-level object types to make them stay on screen for longer than usual.
@@ -93,7 +99,7 @@ namespace osu.Game.Rulesets.Osu.Edit
                         hitObject.RemoveTransform(existing);
 
                         using (hitObject.BeginAbsoluteSequence(hitObject.HitStateUpdateTime))
-                            hitObject.FadeOut(editor_hit_object_fade_out_extension).Expire();
+                            hitObject.FadeOut(EDITOR_HIT_OBJECT_FADE_OUT_EXTENSION).Expire();
                         break;
                 }
             }