Move DrawableHitObject automatic state logic to base class (#5424)

Move DrawableHitObject automatic state logic to base class

Co-authored-by: Dan Balasescu <smoogipoo@smgi.me>

osu.Game.Rulesets.Catch/Objects/Drawable/DrawableCatchHitObject.cs

@@ -3,12 +3,10 @@
 
 using System;
 using osuTK;
-using osuTK.Graphics;
 using osu.Framework.Graphics;
 using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.Scoring;
-using osu.Game.Skinning;
 
 namespace osu.Game.Rulesets.Catch.Objects.Drawable
 {
@@ -60,16 +58,11 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawable
                 ApplyResult(r => r.Type = CheckPosition.Invoke(HitObject) ? HitResult.Perfect : HitResult.Miss);
         }
 
-        protected override void SkinChanged(ISkinSource skin, bool allowFallback)
+        protected override bool UseTransformStateManagement => false;
-        {
-            base.SkinChanged(skin, allowFallback);
-
-            if (HitObject is IHasComboInformation combo)
-                AccentColour.Value = skin.GetValue<SkinConfiguration, Color4?>(s => s.ComboColours.Count > 0 ? s.ComboColours[combo.ComboIndex % s.ComboColours.Count] : (Color4?)null) ?? Color4.White;
-        }
 
         protected override void UpdateState(ArmedState state)
         {
+            // TODO: update to use new state management.
             using (BeginAbsoluteSequence(HitObject.StartTime - HitObject.TimePreempt))
                 this.FadeIn(200);
 

osu.Game.Rulesets.Mania/Objects/Drawables/DrawableManiaHitObject.cs

@@ -58,8 +58,11 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables
             HitObject = hitObject;
         }
 
+        protected override bool UseTransformStateManagement => false;
+
         protected override void UpdateState(ArmedState state)
         {
+            // TODO: update to use new state management.
             switch (state)
             {
                 case ArmedState.Miss:

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs

@@ -128,16 +128,16 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
             ApplyResult(r => r.Type = result);
         }
 
-        protected override void UpdatePreemptState()
+        protected override void UpdateInitialTransforms()
         {
-            base.UpdatePreemptState();
+            base.UpdateInitialTransforms();
 
             ApproachCircle.FadeIn(Math.Min(HitObject.TimeFadeIn * 2, HitObject.TimePreempt));
             ApproachCircle.ScaleTo(1.1f, HitObject.TimePreempt);
             ApproachCircle.Expire(true);
         }
 
-        protected override void UpdateCurrentState(ArmedState state)
+        protected override void UpdateStateTransforms(ArmedState state)
         {
             glow.FadeOut(400);
 

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs

@@ -1,15 +1,10 @@
 // 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 osu.Game.Rulesets.Objects.Drawables;
 using osu.Framework.Graphics;
 using osu.Game.Rulesets.Judgements;
-using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Rulesets.Osu.Judgements;
-using osu.Game.Rulesets.Scoring;
-using osu.Game.Skinning;
-using osuTK.Graphics;
 using osu.Game.Graphics.Containers;
 
 namespace osu.Game.Rulesets.Osu.Objects.Drawables
@@ -41,47 +36,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
 
         protected sealed override double InitialLifetimeOffset => HitObject.TimePreempt;
 
-        protected sealed override void UpdateState(ArmedState state)
+        protected override void UpdateInitialTransforms() => this.FadeIn(HitObject.TimeFadeIn);
-        {
-            double transformTime = HitObject.StartTime - HitObject.TimePreempt;
-
-            base.ApplyTransformsAt(transformTime, true);
-            base.ClearTransformsAfter(transformTime, true);
-
-            using (BeginAbsoluteSequence(transformTime, true))
-            {
-                UpdatePreemptState();
-
-                var judgementOffset = Math.Min(HitObject.HitWindows.HalfWindowFor(HitResult.Miss), Result?.TimeOffset ?? 0);
-
-                using (BeginDelayedSequence(HitObject.TimePreempt + judgementOffset, true))
-                    UpdateCurrentState(state);
-            }
-        }
-
-        protected override void SkinChanged(ISkinSource skin, bool allowFallback)
-        {
-            base.SkinChanged(skin, allowFallback);
-
-            if (HitObject is IHasComboInformation combo)
-                AccentColour.Value = skin.GetValue<SkinConfiguration, Color4?>(s => s.ComboColours.Count > 0 ? s.ComboColours[combo.ComboIndex % s.ComboColours.Count] : (Color4?)null) ?? Color4.White;
-        }
-
-        protected virtual void UpdatePreemptState() => this.FadeIn(HitObject.TimeFadeIn);
-
-        protected virtual void UpdateCurrentState(ArmedState state)
-        {
-        }
-
-        // Todo: At some point we need to move these to DrawableHitObject after ensuring that all other Rulesets apply
-        // transforms in the same way and don't rely on them not being cleared
-        public override void ClearTransformsAfter(double time, bool propagateChildren = false, string targetMember = null)
-        {
-        }
-
-        public override void ApplyTransformsAt(double time, bool propagateChildren = false)
-        {
-        }
 
         private OsuInputManager osuActionInputManager;
         internal OsuInputManager OsuActionInputManager => osuActionInputManager ?? (osuActionInputManager = GetContainingInputManager() as OsuInputManager);

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableRepeatPoint.cs

@@ -47,7 +47,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
                 ApplyResult(r => r.Type = drawableSlider.Tracking.Value ? HitResult.Great : HitResult.Miss);
         }
 
-        protected override void UpdatePreemptState()
+        protected override void UpdateInitialTransforms()
         {
             animDuration = Math.Min(150, repeatPoint.SpanDuration / 2);
 
@@ -57,7 +57,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
             );
         }
 
-        protected override void UpdateCurrentState(ArmedState state)
+        protected override void UpdateStateTransforms(ArmedState state)
         {
             switch (state)
             {

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs

@@ -190,7 +190,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
             });
         }
 
-        protected override void UpdateCurrentState(ArmedState state)
+        protected override void UpdateStateTransforms(ArmedState state)
         {
             Ball.FadeIn();
             Ball.ScaleTo(HitObject.Scale);

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTick.cs

@@ -52,13 +52,13 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
                 ApplyResult(r => r.Type = Tracking ? HitResult.Great : HitResult.Miss);
         }
 
-        protected override void UpdatePreemptState()
+        protected override void UpdateInitialTransforms()
         {
             this.FadeOut().FadeIn(ANIM_DURATION);
             this.ScaleTo(0.5f).ScaleTo(1f, ANIM_DURATION * 4, Easing.OutElasticHalf);
         }
 
-        protected override void UpdateCurrentState(ArmedState state)
+        protected override void UpdateStateTransforms(ArmedState state)
         {
             switch (state)
             {

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs

@@ -196,9 +196,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
             symbol.RotateTo(Disc.Rotation / 2, 500, Easing.OutQuint);
         }
 
-        protected override void UpdatePreemptState()
+        protected override void UpdateInitialTransforms()
         {
-            base.UpdatePreemptState();
+            base.UpdateInitialTransforms();
 
             circleContainer.ScaleTo(Spinner.Scale * 0.3f);
             circleContainer.ScaleTo(Spinner.Scale, HitObject.TimePreempt / 1.4f, Easing.OutQuint);
@@ -213,7 +213,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
                 .ScaleTo(1, 500, Easing.OutQuint);
         }
 
-        protected override void UpdateCurrentState(ArmedState state)
+        protected override void UpdateStateTransforms(ArmedState state)
         {
             var sequence = this.Delay(Spinner.Duration).FadeOut(160);
 

osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs

@@ -94,6 +94,7 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables
 
         protected override void UpdateState(ArmedState state)
         {
+            // TODO: update to use new state management.
             var circlePiece = MainPiece as CirclePiece;
             circlePiece?.FlashBox.FinishTransforms();
 

osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs

@@ -121,6 +121,8 @@ namespace osu.Game.Rulesets.Taiko.Objects.Drawables
             }
         }
 
+        protected override bool UseTransformStateManagement => false;
+
         // Normal and clap samples are handled by the drum
         protected override IEnumerable<HitSampleInfo> GetSamples() => HitObject.Samples.Where(s => s.Name != HitSampleInfo.HIT_NORMAL && s.Name != HitSampleInfo.HIT_CLAP);
 

osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs

@@ -81,7 +81,9 @@ namespace osu.Game.Rulesets.Objects.Drawables
 
         public override bool IsPresent => base.IsPresent || (State.Value == ArmedState.Idle && Clock?.CurrentTime >= LifetimeStart);
 
-        public readonly Bindable<ArmedState> State = new Bindable<ArmedState>();
+        private readonly Bindable<ArmedState> state = new Bindable<ArmedState>();
+
+        public IBindable<ArmedState> State => state;
 
         protected DrawableHitObject(HitObject hitObject)
         {
@@ -116,33 +118,120 @@ namespace osu.Game.Rulesets.Objects.Drawables
             }
         }
 
-        protected override void ClearInternal(bool disposeChildren = true) => throw new InvalidOperationException($"Should never clear a {nameof(DrawableHitObject)}");
-
         protected override void LoadComplete()
         {
             base.LoadComplete();
-
+            updateState(ArmedState.Idle, true);
-            State.ValueChanged += armed =>
-            {
-                UpdateState(armed.NewValue);
-
-                // apply any custom state overrides
-                ApplyCustomUpdateState?.Invoke(this, armed.NewValue);
-
-                if (armed.NewValue == ArmedState.Hit)
-                    PlaySamples();
-            };
-
-            State.TriggerChange();
         }
 
-        protected abstract void UpdateState(ArmedState state);
+        #region State / Transform Management
 
         /// <summary>
         /// Bind to apply a custom state which can override the default implementation.
         /// </summary>
         public event Action<DrawableHitObject, ArmedState> ApplyCustomUpdateState;
 
+        /// <summary>
+        /// Enables automatic transform management of this hitobject. Implementation of transforms should be done in <see cref="UpdateInitialTransforms"/> and <see cref="UpdateStateTransforms"/> only. Rewinding and removing previous states is done automatically.
+        /// </summary>
+        /// <remarks>
+        /// Going forward, this is the preferred way of implementing <see cref="DrawableHitObject"/>s. Previous functionality
+        /// is offered as a compatibility layer until all rulesets have been migrated across.
+        /// </remarks>
+        protected virtual bool UseTransformStateManagement => true;
+
+        protected override void ClearInternal(bool disposeChildren = true) => throw new InvalidOperationException($"Should never clear a {nameof(DrawableHitObject)}");
+
+        private void updateState(ArmedState newState, bool force = false)
+        {
+            if (State.Value == newState && !force)
+                return;
+
+            // apply any custom state overrides
+            ApplyCustomUpdateState?.Invoke(this, newState);
+
+            if (newState == ArmedState.Hit)
+                PlaySamples();
+
+            if (UseTransformStateManagement)
+            {
+                double transformTime = HitObject.StartTime - InitialLifetimeOffset;
+
+                base.ApplyTransformsAt(transformTime, true);
+                base.ClearTransformsAfter(transformTime, true);
+
+                using (BeginAbsoluteSequence(transformTime, true))
+                {
+                    UpdateInitialTransforms();
+
+                    var judgementOffset = Math.Min(HitObject.HitWindows?.HalfWindowFor(HitResult.Miss) ?? double.MaxValue, Result?.TimeOffset ?? 0);
+
+                    using (BeginDelayedSequence(InitialLifetimeOffset + judgementOffset, true))
+                    {
+                        UpdateStateTransforms(newState);
+                        state.Value = newState;
+                    }
+                }
+            }
+            else
+                state.Value = newState;
+
+            UpdateState(newState);
+        }
+
+        /// <summary>
+        /// Apply (generally fade-in) transforms leading into the <see cref="HitObject"/> start time.
+        /// The local drawable hierarchy is recursively delayed to <see cref="LifetimeStart"/> for convenience.
+        /// </summary>
+        /// <remarks>
+        /// This is called once before every <see cref="UpdateStateTransforms"/>. This is to ensure a good state in the case
+        /// the <see cref="JudgementResult.TimeOffset"/> was negative and potentially altered the pre-hit transforms.
+        /// </remarks>
+        protected virtual void UpdateInitialTransforms()
+        {
+        }
+
+        /// <summary>
+        /// Apply transforms based on the current <see cref="ArmedState"/>. Previous states are automatically cleared.
+        /// </summary>
+        /// <param name="state">The new armed state.</param>
+        protected virtual void UpdateStateTransforms(ArmedState state)
+        {
+        }
+
+        public override void ClearTransformsAfter(double time, bool propagateChildren = false, string targetMember = null)
+        {
+            // When we are using automatic state management, parent calls to this should be blocked for safety.
+            if (!UseTransformStateManagement)
+                base.ClearTransformsAfter(time, propagateChildren, targetMember);
+        }
+
+        public override void ApplyTransformsAt(double time, bool propagateChildren = false)
+        {
+            // When we are using automatic state management, parent calls to this should be blocked for safety.
+            if (!UseTransformStateManagement)
+                base.ApplyTransformsAt(time, propagateChildren);
+        }
+
+        /// <summary>
+        /// Legacy method to handle state changes.
+        /// Should generally not be used when <see cref="UseTransformStateManagement"/> is true; use <see cref="UpdateStateTransforms"/> instead.
+        /// </summary>
+        /// <param name="state">The new armed state.</param>
+        protected virtual void UpdateState(ArmedState state)
+        {
+        }
+
+        #endregion
+
+        protected override void SkinChanged(ISkinSource skin, bool allowFallback)
+        {
+            base.SkinChanged(skin, allowFallback);
+
+            if (HitObject is IHasComboInformation combo)
+                AccentColour.Value = skin.GetValue<SkinConfiguration, Color4?>(s => s.ComboColours.Count > 0 ? s.ComboColours[combo.ComboIndex % s.ComboColours.Count] : (Color4?)null) ?? Color4.White;
+        }
+
         /// <summary>
         /// Plays all the hit sounds for this <see cref="DrawableHitObject"/>.
         /// This is invoked automatically when this <see cref="DrawableHitObject"/> is hit.
@@ -163,7 +252,8 @@ namespace osu.Game.Rulesets.Objects.Drawables
 
                     Result.TimeOffset = 0;
                     Result.Type = HitResult.None;
-                    State.Value = ArmedState.Idle;
+
+                    updateState(ArmedState.Idle);
                 }
             }
         }
@@ -195,6 +285,7 @@ namespace osu.Game.Rulesets.Objects.Drawables
         /// </summary>
         /// <remarks>
         /// This is only used as an optimisation to delay the initial update of this <see cref="DrawableHitObject"/> and may be tuned more aggressively if required.
+        /// It is indirectly used to decide the automatic transform offset provided to <see cref="UpdateInitialTransforms"/>.
         /// A more accurate <see cref="LifetimeStart"/> should be set inside <see cref="UpdateState"/> for an <see cref="ArmedState.Idle"/> state.
         /// </remarks>
         protected virtual double InitialLifetimeOffset => 10000;
@@ -243,11 +334,11 @@ namespace osu.Game.Rulesets.Objects.Drawables
                     break;
 
                 case HitResult.Miss:
-                    State.Value = ArmedState.Miss;
+                    updateState(ArmedState.Miss);
                     break;
 
                 default:
-                    State.Value = ArmedState.Hit;
+                    updateState(ArmedState.Hit);
                     break;
             }