From 91f86adb66250b20062ebbe3fbe1d7aede2b1b2d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 22 Jul 2019 15:05:56 +0900 Subject: [PATCH 1/6] Move DrawableHitObject state management to base class --- .../Drawable/DrawableCatchHitObject.cs | 10 --- .../Objects/Drawables/DrawableOsuHitObject.cs | 49 +---------- .../Objects/Drawables/DrawableHitObject.cs | 82 ++++++++++++++++--- 3 files changed, 75 insertions(+), 66 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/Drawable/DrawableCatchHitObject.cs b/osu.Game.Rulesets.Catch/Objects/Drawable/DrawableCatchHitObject.cs index 2ccb01a3e3..f5dd7cd255 100644 --- a/osu.Game.Rulesets.Catch/Objects/Drawable/DrawableCatchHitObject.cs +++ b/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,14 +58,6 @@ 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) - { - base.SkinChanged(skin, allowFallback); - - if (HitObject is IHasComboInformation combo) - AccentColour.Value = skin.GetValue(s => s.ComboColours.Count > 0 ? s.ComboColours[combo.ComboIndex % s.ComboColours.Count] : (Color4?)null) ?? Color4.White; - } - protected override void UpdateState(ArmedState state) { using (BeginAbsoluteSequence(HitObject.StartTime - HitObject.TimePreempt)) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs index 30cf09c9d7..145be253bc 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs @@ -1,15 +1,10 @@ // Copyright (c) ppy Pty Ltd . 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 @@ -39,49 +34,11 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables protected override void ClearInternal(bool disposeChildren = true) => shakeContainer.Clear(disposeChildren); protected override bool RemoveInternal(Drawable drawable) => shakeContainer.Remove(drawable); + protected override bool UseTransformStateManagement => true; + protected sealed override double InitialLifetimeOffset => HitObject.TimePreempt; - protected sealed override void UpdateState(ArmedState state) - { - 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(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) - { - } + protected override void UpdatePreemptState() => this.FadeIn(HitObject.TimeFadeIn); private OsuInputManager osuActionInputManager; internal OsuInputManager OsuActionInputManager => osuActionInputManager ?? (osuActionInputManager = GetContainingInputManager() as OsuInputManager); diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 1fd6176f4f..06ea82746f 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -79,10 +79,12 @@ namespace osu.Game.Rulesets.Objects.Drawables public override bool RemoveCompletedTransforms => false; protected override bool RequiresChildrenUpdate => true; - public override bool IsPresent => base.IsPresent || (State.Value == ArmedState.Idle && Clock?.CurrentTime >= LifetimeStart); + public override bool IsPresent => base.IsPresent || (state.Value == ArmedState.Idle && Clock?.CurrentTime >= LifetimeStart); public readonly Bindable State = new Bindable(); + private readonly Bindable state = new Bindable(); + protected DrawableHitObject(HitObject hitObject) { HitObject = hitObject; @@ -122,21 +124,81 @@ namespace osu.Game.Rulesets.Objects.Drawables { base.LoadComplete(); - State.ValueChanged += armed => + state.BindValueChanged(armed => { - UpdateState(armed.NewValue); + updateState(armed.NewValue); // apply any custom state overrides ApplyCustomUpdateState?.Invoke(this, armed.NewValue); if (armed.NewValue == ArmedState.Hit) PlaySamples(); - }; - - State.TriggerChange(); + }, true); } - protected abstract void UpdateState(ArmedState state); + protected virtual bool UseTransformStateManagement => false; + + private void updateState(ArmedState state) + { + if (UseTransformStateManagement) + { + double transformTime = HitObject.StartTime - InitialLifetimeOffset; + + base.ApplyTransformsAt(transformTime, true); + base.ClearTransformsAfter(transformTime, true); + + using (BeginAbsoluteSequence(transformTime, true)) + { + UpdatePreemptState(); + + var judgementOffset = Math.Min(HitObject.HitWindows?.HalfWindowFor(HitResult.Miss) ?? double.MaxValue, Result?.TimeOffset ?? 0); + + using (BeginDelayedSequence(InitialLifetimeOffset + judgementOffset, true)) + { + UpdateCurrentState(state); + State.Value = state; + } + } + } + else + { + State.Value = state; + } + + UpdateState(state); + } + + protected override void SkinChanged(ISkinSource skin, bool allowFallback) + { + base.SkinChanged(skin, allowFallback); + + if (HitObject is IHasComboInformation combo) + AccentColour.Value = skin.GetValue(s => s.ComboColours.Count > 0 ? s.ComboColours[combo.ComboIndex % s.ComboColours.Count] : (Color4?)null) ?? Color4.White; + } + + protected virtual void UpdatePreemptState() + { + } + + protected virtual void UpdateCurrentState(ArmedState state) + { + } + + public override void ClearTransformsAfter(double time, bool propagateChildren = false, string targetMember = null) + { + if (!UseTransformStateManagement) + base.ClearTransformsAfter(time, propagateChildren, targetMember); + } + + public override void ApplyTransformsAt(double time, bool propagateChildren = false) + { + if (!UseTransformStateManagement) + base.ApplyTransformsAt(time, propagateChildren); + } + + protected virtual void UpdateState(ArmedState state) + { + } /// /// Bind to apply a custom state which can override the default implementation. @@ -163,7 +225,7 @@ namespace osu.Game.Rulesets.Objects.Drawables Result.TimeOffset = 0; Result.Type = HitResult.None; - State.Value = ArmedState.Idle; + state.Value = ArmedState.Idle; } } } @@ -243,11 +305,11 @@ namespace osu.Game.Rulesets.Objects.Drawables break; case HitResult.Miss: - State.Value = ArmedState.Miss; + state.Value = ArmedState.Miss; break; default: - State.Value = ArmedState.Hit; + state.Value = ArmedState.Hit; break; } From be170b412496834986c5d214dd4660b4d26115a2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 22 Jul 2019 15:33:12 +0900 Subject: [PATCH 2/6] Naming and documentation improvements --- .../Objects/Drawables/DrawableHitCircle.cs | 6 +-- .../Objects/Drawables/DrawableOsuHitObject.cs | 2 +- .../Objects/Drawables/DrawableRepeatPoint.cs | 4 +- .../Objects/Drawables/DrawableSlider.cs | 2 +- .../Objects/Drawables/DrawableSliderTick.cs | 4 +- .../Objects/Drawables/DrawableSpinner.cs | 6 +-- .../Objects/Drawables/DrawableHitObject.cs | 39 ++++++++++++------- 7 files changed, 37 insertions(+), 26 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs index a83f1b5e56..d3d763daf3 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs +++ b/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); diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs index 145be253bc..8107d366b3 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs @@ -38,7 +38,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables protected sealed override double InitialLifetimeOffset => HitObject.TimePreempt; - protected override void UpdatePreemptState() => this.FadeIn(HitObject.TimeFadeIn); + protected override void UpdateInitialTransforms() => this.FadeIn(HitObject.TimeFadeIn); private OsuInputManager osuActionInputManager; internal OsuInputManager OsuActionInputManager => osuActionInputManager ?? (osuActionInputManager = GetContainingInputManager() as OsuInputManager); diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableRepeatPoint.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableRepeatPoint.cs index cce6dfe106..1e2c0ae59f 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableRepeatPoint.cs +++ b/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) { diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index e06287f527..d2089c05f3 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/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); diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTick.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTick.cs index ec294a1630..3e128e9f15 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTick.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTick.cs @@ -55,13 +55,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) { diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs index 1794da54b7..a0bd301fdb 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSpinner.cs +++ b/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); diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 06ea82746f..26d51e3809 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -118,8 +118,6 @@ 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(); @@ -136,8 +134,19 @@ namespace osu.Game.Rulesets.Objects.Drawables }, true); } + #region State / Transform Management + + /// + /// Enables automatic transform management of this hitobject. Implementation of transforms should be done in and only. Rewinding and removing previous states is done automatically. + /// + /// + /// Going forward, this is the preferred way of implementing s. Previous functionality + /// is offered as a compatibility layer until all rulesets have been migrated across. + /// protected virtual bool UseTransformStateManagement => false; + protected override void ClearInternal(bool disposeChildren = true) => throw new InvalidOperationException($"Should never clear a {nameof(DrawableHitObject)}"); + private void updateState(ArmedState state) { if (UseTransformStateManagement) @@ -149,13 +158,13 @@ namespace osu.Game.Rulesets.Objects.Drawables using (BeginAbsoluteSequence(transformTime, true)) { - UpdatePreemptState(); + UpdateInitialTransforms(); var judgementOffset = Math.Min(HitObject.HitWindows?.HalfWindowFor(HitResult.Miss) ?? double.MaxValue, Result?.TimeOffset ?? 0); using (BeginDelayedSequence(InitialLifetimeOffset + judgementOffset, true)) { - UpdateCurrentState(state); + UpdateStateTransforms(state); State.Value = state; } } @@ -168,19 +177,11 @@ namespace osu.Game.Rulesets.Objects.Drawables UpdateState(state); } - protected override void SkinChanged(ISkinSource skin, bool allowFallback) - { - base.SkinChanged(skin, allowFallback); - - if (HitObject is IHasComboInformation combo) - AccentColour.Value = skin.GetValue(s => s.ComboColours.Count > 0 ? s.ComboColours[combo.ComboIndex % s.ComboColours.Count] : (Color4?)null) ?? Color4.White; - } - - protected virtual void UpdatePreemptState() + protected virtual void UpdateInitialTransforms() { } - protected virtual void UpdateCurrentState(ArmedState state) + protected virtual void UpdateStateTransforms(ArmedState state) { } @@ -200,6 +201,16 @@ namespace osu.Game.Rulesets.Objects.Drawables { } + #endregion + + protected override void SkinChanged(ISkinSource skin, bool allowFallback) + { + base.SkinChanged(skin, allowFallback); + + if (HitObject is IHasComboInformation combo) + AccentColour.Value = skin.GetValue(s => s.ComboColours.Count > 0 ? s.ComboColours[combo.ComboIndex % s.ComboColours.Count] : (Color4?)null) ?? Color4.White; + } + /// /// Bind to apply a custom state which can override the default implementation. /// From c3b81bef4ab8e9160e73f0013067082d4239706f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 22 Jul 2019 15:55:38 +0900 Subject: [PATCH 3/6] Flip default to the preferred method going forward --- .../Objects/Drawable/DrawableCatchHitObject.cs | 3 +++ .../Objects/Drawables/DrawableManiaHitObject.cs | 3 +++ .../Objects/Drawables/DrawableOsuHitObject.cs | 2 -- osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs | 1 + .../Objects/Drawables/DrawableTaikoHitObject.cs | 2 ++ osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 2 +- 6 files changed, 10 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/Drawable/DrawableCatchHitObject.cs b/osu.Game.Rulesets.Catch/Objects/Drawable/DrawableCatchHitObject.cs index f5dd7cd255..a1279e8443 100644 --- a/osu.Game.Rulesets.Catch/Objects/Drawable/DrawableCatchHitObject.cs +++ b/osu.Game.Rulesets.Catch/Objects/Drawable/DrawableCatchHitObject.cs @@ -58,8 +58,11 @@ namespace osu.Game.Rulesets.Catch.Objects.Drawable ApplyResult(r => r.Type = CheckPosition.Invoke(HitObject) ? HitResult.Perfect : HitResult.Miss); } + protected override bool UseTransformStateManagement => false; + protected override void UpdateState(ArmedState state) { + // TODO: update to use new state management. using (BeginAbsoluteSequence(HitObject.StartTime - HitObject.TimePreempt)) this.FadeIn(200); diff --git a/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableManiaHitObject.cs b/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableManiaHitObject.cs index 0873f753be..db6b53e76d 100644 --- a/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableManiaHitObject.cs +++ b/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: diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs index 8107d366b3..579f16e0d4 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuHitObject.cs @@ -34,8 +34,6 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables protected override void ClearInternal(bool disposeChildren = true) => shakeContainer.Clear(disposeChildren); protected override bool RemoveInternal(Drawable drawable) => shakeContainer.Remove(drawable); - protected override bool UseTransformStateManagement => true; - protected sealed override double InitialLifetimeOffset => HitObject.TimePreempt; protected override void UpdateInitialTransforms() => this.FadeIn(HitObject.TimeFadeIn); diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs index 4c8d5d5204..34ae7db984 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableHit.cs +++ b/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(); diff --git a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs b/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs index bd45b52d7b..b46738c69a 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Drawables/DrawableTaikoHitObject.cs +++ b/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 GetSamples() => HitObject.Samples.Where(s => s.Name != HitSampleInfo.HIT_NORMAL && s.Name != HitSampleInfo.HIT_CLAP); diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 26d51e3809..aef163cda7 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -143,7 +143,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// Going forward, this is the preferred way of implementing s. Previous functionality /// is offered as a compatibility layer until all rulesets have been migrated across. /// - protected virtual bool UseTransformStateManagement => false; + protected virtual bool UseTransformStateManagement => true; protected override void ClearInternal(bool disposeChildren = true) => throw new InvalidOperationException($"Should never clear a {nameof(DrawableHitObject)}"); From d4d286c9880a4ed3a08c9245e5f5491d87f5c9f0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 22 Jul 2019 16:08:38 +0900 Subject: [PATCH 4/6] Add full documentation --- .../Objects/Drawables/DrawableHitObject.cs | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index aef163cda7..d10f829dae 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -136,6 +136,11 @@ namespace osu.Game.Rulesets.Objects.Drawables #region State / Transform Management + /// + /// Bind to apply a custom state which can override the default implementation. + /// + public event Action ApplyCustomUpdateState; + /// /// Enables automatic transform management of this hitobject. Implementation of transforms should be done in and only. Rewinding and removing previous states is done automatically. /// @@ -177,26 +182,45 @@ namespace osu.Game.Rulesets.Objects.Drawables UpdateState(state); } + /// + /// Apply (generally fade-in) transforms. + /// The local drawable hierarchy is recursively delayed to for convenience. + /// + /// + /// This is called once before every . This is to ensure a good state in the case + /// the was negative and potentially altered the pre-hit transforms. + /// protected virtual void UpdateInitialTransforms() { } + /// + /// Apply transforms based on the current . Previous states are automatically cleared. + /// + /// The new armed state. protected virtual void UpdateStateTransforms(ArmedState state) { } public override void ClearTransformsAfter(double time, bool propagateChildren = false, string targetMember = null) { + // When we are using automatic state menement, 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 menement, parent calls to this should be blocked for safety. if (!UseTransformStateManagement) base.ApplyTransformsAt(time, propagateChildren); } + /// + /// Legacy method to handle state changes. + /// Should generally not be used when is true; use instead. + /// + /// The new armed state. protected virtual void UpdateState(ArmedState state) { } @@ -211,11 +235,6 @@ namespace osu.Game.Rulesets.Objects.Drawables AccentColour.Value = skin.GetValue(s => s.ComboColours.Count > 0 ? s.ComboColours[combo.ComboIndex % s.ComboColours.Count] : (Color4?)null) ?? Color4.White; } - /// - /// Bind to apply a custom state which can override the default implementation. - /// - public event Action ApplyCustomUpdateState; - /// /// Plays all the hit sounds for this . /// This is invoked automatically when this is hit. @@ -268,6 +287,7 @@ namespace osu.Game.Rulesets.Objects.Drawables /// /// /// This is only used as an optimisation to delay the initial update of this and may be tuned more aggressively if required. + /// It is indirectly used to decide the automatic transform offset provided to . /// A more accurate should be set inside for an state. /// protected virtual double InitialLifetimeOffset => 10000; From 74b09c72fa044d9fc0f21f19f03da216df2bf291 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 23 Jul 2019 21:08:41 +0900 Subject: [PATCH 5/6] Refactor state updates to convert State into an IBindable --- .../Objects/Drawables/DrawableHitObject.cs | 46 +++++++++---------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index d10f829dae..3253302c71 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -79,12 +79,12 @@ namespace osu.Game.Rulesets.Objects.Drawables public override bool RemoveCompletedTransforms => false; protected override bool RequiresChildrenUpdate => true; - public override bool IsPresent => base.IsPresent || (state.Value == ArmedState.Idle && Clock?.CurrentTime >= LifetimeStart); - - public readonly Bindable State = new Bindable(); + public override bool IsPresent => base.IsPresent || (State.Value == ArmedState.Idle && Clock?.CurrentTime >= LifetimeStart); private readonly Bindable state = new Bindable(); + public IBindable State => state; + protected DrawableHitObject(HitObject hitObject) { HitObject = hitObject; @@ -121,17 +121,7 @@ namespace osu.Game.Rulesets.Objects.Drawables protected override void LoadComplete() { base.LoadComplete(); - - state.BindValueChanged(armed => - { - updateState(armed.NewValue); - - // apply any custom state overrides - ApplyCustomUpdateState?.Invoke(this, armed.NewValue); - - if (armed.NewValue == ArmedState.Hit) - PlaySamples(); - }, true); + updateState(ArmedState.Idle, true); } #region State / Transform Management @@ -152,8 +142,17 @@ namespace osu.Game.Rulesets.Objects.Drawables protected override void ClearInternal(bool disposeChildren = true) => throw new InvalidOperationException($"Should never clear a {nameof(DrawableHitObject)}"); - private void updateState(ArmedState state) + 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; @@ -169,17 +168,15 @@ namespace osu.Game.Rulesets.Objects.Drawables using (BeginDelayedSequence(InitialLifetimeOffset + judgementOffset, true)) { - UpdateStateTransforms(state); - State.Value = state; + UpdateStateTransforms(newState); + state.Value = newState; } } } else - { - State.Value = state; - } + state.Value = newState; - UpdateState(state); + UpdateState(newState); } /// @@ -255,7 +252,8 @@ namespace osu.Game.Rulesets.Objects.Drawables Result.TimeOffset = 0; Result.Type = HitResult.None; - state.Value = ArmedState.Idle; + + updateState(ArmedState.Idle); } } } @@ -336,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; } From 4e7e2d1d5230b76bb25edfbceffb8b51c060f475 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 23 Jul 2019 21:15:55 +0900 Subject: [PATCH 6/6] Adjust comments --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index 3253302c71..1d9d885527 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -180,7 +180,7 @@ namespace osu.Game.Rulesets.Objects.Drawables } /// - /// Apply (generally fade-in) transforms. + /// Apply (generally fade-in) transforms leading into the start time. /// The local drawable hierarchy is recursively delayed to for convenience. /// /// @@ -201,14 +201,14 @@ namespace osu.Game.Rulesets.Objects.Drawables public override void ClearTransformsAfter(double time, bool propagateChildren = false, string targetMember = null) { - // When we are using automatic state menement, parent calls to this should be blocked for safety. + // 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 menement, parent calls to this should be blocked for safety. + // When we are using automatic state management, parent calls to this should be blocked for safety. if (!UseTransformStateManagement) base.ApplyTransformsAt(time, propagateChildren); }