From 2f33aeac9ff14d075957d7c053dd7b384d2caa37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 13 Nov 2020 14:31:07 +0100 Subject: [PATCH 1/9] Move drawable instatiation to [SetUp] --- .../Skinning/ManiaHitObjectTestScene.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Mania.Tests/Skinning/ManiaHitObjectTestScene.cs b/osu.Game.Rulesets.Mania.Tests/Skinning/ManiaHitObjectTestScene.cs index d24c81dac6..96444fd316 100644 --- a/osu.Game.Rulesets.Mania.Tests/Skinning/ManiaHitObjectTestScene.cs +++ b/osu.Game.Rulesets.Mania.Tests/Skinning/ManiaHitObjectTestScene.cs @@ -1,7 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using osu.Framework.Allocation; +using NUnit.Framework; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Rulesets.Mania.Objects.Drawables; @@ -15,8 +15,8 @@ namespace osu.Game.Rulesets.Mania.Tests.Skinning /// public abstract class ManiaHitObjectTestScene : ManiaSkinnableTestScene { - [BackgroundDependencyLoader] - private void load() + [SetUp] + public void SetUp() => Schedule(() => { SetContents(() => new FillFlowContainer { @@ -65,7 +65,7 @@ namespace osu.Game.Rulesets.Mania.Tests.Skinning }, } }); - } + }); protected abstract DrawableManiaHitObject CreateHitObject(); } From 2ccc81ccc0ca7406d21e6283c60a77f5849bf681 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 13 Nov 2020 14:31:24 +0100 Subject: [PATCH 2/9] Add test case for fading hold note --- .../Skinning/TestSceneHoldNote.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/osu.Game.Rulesets.Mania.Tests/Skinning/TestSceneHoldNote.cs b/osu.Game.Rulesets.Mania.Tests/Skinning/TestSceneHoldNote.cs index 9c4c2b3d5b..e88ff8e2ac 100644 --- a/osu.Game.Rulesets.Mania.Tests/Skinning/TestSceneHoldNote.cs +++ b/osu.Game.Rulesets.Mania.Tests/Skinning/TestSceneHoldNote.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Framework.Bindables; @@ -26,6 +27,18 @@ namespace osu.Game.Rulesets.Mania.Tests.Skinning }); } + [Test] + public void TestFadeOnMiss() + { + AddStep("miss tick", () => + { + foreach (var holdNote in holdNotes) + holdNote.ChildrenOfType().First().MissForcefully(); + }); + } + + private IEnumerable holdNotes => CreatedDrawables.SelectMany(d => d.ChildrenOfType()); + protected override DrawableManiaHitObject CreateHitObject() { var note = new HoldNote { Duration = 1000 }; From 55a91dbbe031df2bb677a6518cf035961d4faf1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 13 Nov 2020 15:16:33 +0100 Subject: [PATCH 3/9] Add fading on hit state change --- .../Skinning/LegacyBodyPiece.cs | 46 +++++++++++++++++-- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs index c0f0fcb4af..a9cd7b592f 100644 --- a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs +++ b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs @@ -18,6 +18,8 @@ namespace osu.Game.Rulesets.Mania.Skinning { public class LegacyBodyPiece : LegacyManiaColumnElement { + private DrawableHoldNote holdNote; + private readonly IBindable direction = new Bindable(); private readonly IBindable isHitting = new Bindable(); @@ -38,6 +40,8 @@ namespace osu.Game.Rulesets.Mania.Skinning [BackgroundDependencyLoader] private void load(ISkinSource skin, IScrollingInfo scrollingInfo, DrawableHitObject drawableObject) { + holdNote = (DrawableHoldNote)drawableObject; + string imageName = GetColumnSkinConfig(skin, LegacyManiaSkinConfigurationLookups.HoldNoteBodyImage)?.Value ?? $"mania-note{FallbackColumnIndex}L"; @@ -92,11 +96,44 @@ namespace osu.Game.Rulesets.Mania.Skinning InternalChild = bodySprite; direction.BindTo(scrollingInfo.Direction); - direction.BindValueChanged(onDirectionChanged, true); - - var holdNote = (DrawableHoldNote)drawableObject; isHitting.BindTo(holdNote.IsHitting); + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + direction.BindValueChanged(onDirectionChanged, true); isHitting.BindValueChanged(onIsHittingChanged, true); + + holdNote.ApplyCustomUpdateState += applyCustomUpdateState; + applyCustomUpdateState(holdNote, holdNote.State.Value); + } + + /// + /// Stores the start time of the fade animation that plays when any of the nested + /// hitobjects of the hold note are missed. + /// + private double? missFadeTime; + + private void applyCustomUpdateState(DrawableHitObject hitObject, ArmedState state) + { + switch (state) + { + case ArmedState.Miss: + missFadeTime ??= hitObject.StateUpdateTime; + + using (BeginAbsoluteSequence(missFadeTime.Value)) + { + // colour and duration matches stable + // transforms not applied to entire hold note in order to not affect hit lighting + holdNote.Head.FadeColour(Colour4.DarkGray, 60); + bodySprite?.FadeColour(Colour4.DarkGray, 60); + holdNote.Tail.FadeColour(Colour4.DarkGray, 60); + } + + break; + } } private void onIsHittingChanged(ValueChangedEvent isHitting) @@ -162,6 +199,9 @@ namespace osu.Game.Rulesets.Mania.Skinning { base.Dispose(isDisposing); + if (holdNote != null) + holdNote.ApplyCustomUpdateState -= applyCustomUpdateState; + lightContainer?.Expire(); } } From 4777b1be8108462cad86c5b81e2865888a1972e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 13 Nov 2020 15:43:10 +0100 Subject: [PATCH 4/9] Fix fade not applying to tails sometimes --- .../Skinning/LegacyBodyPiece.cs | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs index a9cd7b592f..a0cbc47df1 100644 --- a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs +++ b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs @@ -118,21 +118,22 @@ namespace osu.Game.Rulesets.Mania.Skinning private void applyCustomUpdateState(DrawableHitObject hitObject, ArmedState state) { - switch (state) + if (state == ArmedState.Miss) + missFadeTime = hitObject.StateUpdateTime; + + if (missFadeTime == null) + return; + + // this state update could come from any nested object of the hold note. + // make sure the transforms are consistent across all affected parts + // even if they're idle. + using (BeginAbsoluteSequence(missFadeTime.Value)) { - case ArmedState.Miss: - missFadeTime ??= hitObject.StateUpdateTime; - - using (BeginAbsoluteSequence(missFadeTime.Value)) - { - // colour and duration matches stable - // transforms not applied to entire hold note in order to not affect hit lighting - holdNote.Head.FadeColour(Colour4.DarkGray, 60); - bodySprite?.FadeColour(Colour4.DarkGray, 60); - holdNote.Tail.FadeColour(Colour4.DarkGray, 60); - } - - break; + // colour and duration matches stable + // transforms not applied to entire hold note in order to not affect hit lighting + holdNote.Head.FadeColour(Colour4.DarkGray, 60); + bodySprite?.FadeColour(Colour4.DarkGray, 60); + holdNote.Tail.FadeColour(Colour4.DarkGray, 60); } } From b62bf5798d40d0df7cd0e8ed63e59215dae6a088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 13 Nov 2020 21:14:34 +0100 Subject: [PATCH 5/9] Store time of hold note break --- .../Objects/Drawables/DrawableHoldNote.cs | 8 ++++---- .../Objects/Drawables/DrawableHoldNoteTail.cs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNote.cs b/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNote.cs index 59899637f9..a64cc6dc67 100644 --- a/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNote.cs +++ b/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNote.cs @@ -51,9 +51,9 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables public double? HoldStartTime { get; private set; } /// - /// Whether the hold note has been released too early and shouldn't give full score for the release. + /// Time at which the hold note has been broken, i.e. released too early, resulting in a reduced score. /// - public bool HasBroken { get; private set; } + public double? HoldBrokenTime { get; private set; } /// /// Whether the hold note has been released potentially without having caused a break. @@ -238,7 +238,7 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables } if (Tail.Judged && !Tail.IsHit) - HasBroken = true; + HoldBrokenTime = Time.Current; } public bool OnPressed(ManiaAction action) @@ -298,7 +298,7 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables // If the key has been released too early, the user should not receive full score for the release if (!Tail.IsHit) - HasBroken = true; + HoldBrokenTime = Time.Current; releaseTime = Time.Current; } diff --git a/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNoteTail.cs b/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNoteTail.cs index c780c0836e..a4029e7893 100644 --- a/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNoteTail.cs +++ b/osu.Game.Rulesets.Mania/Objects/Drawables/DrawableHoldNoteTail.cs @@ -52,7 +52,7 @@ namespace osu.Game.Rulesets.Mania.Objects.Drawables ApplyResult(r => { // If the head wasn't hit or the hold note was broken, cap the max score to Meh. - if (result > HitResult.Meh && (!holdNote.Head.IsHit || holdNote.HasBroken)) + if (result > HitResult.Meh && (!holdNote.Head.IsHit || holdNote.HoldBrokenTime != null)) result = HitResult.Meh; r.Type = result; From a199a957cca03746a93b2ebf1e5ffa0a90b43bd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 13 Nov 2020 21:47:55 +0100 Subject: [PATCH 6/9] Use stored hold note break time to fade upon it --- .../Skinning/LegacyBodyPiece.cs | 53 +++++++++++-------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs index a0cbc47df1..9ed25115d8 100644 --- a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs +++ b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs @@ -23,6 +23,12 @@ namespace osu.Game.Rulesets.Mania.Skinning private readonly IBindable direction = new Bindable(); private readonly IBindable isHitting = new Bindable(); + /// + /// Stores the start time of the fade animation that plays when any of the nested + /// hitobjects of the hold note are missed. + /// + private readonly Bindable missFadeTime = new Bindable(); + [CanBeNull] private Drawable bodySprite; @@ -105,36 +111,16 @@ namespace osu.Game.Rulesets.Mania.Skinning direction.BindValueChanged(onDirectionChanged, true); isHitting.BindValueChanged(onIsHittingChanged, true); + missFadeTime.BindValueChanged(onMissFadeTimeChanged, true); holdNote.ApplyCustomUpdateState += applyCustomUpdateState; applyCustomUpdateState(holdNote, holdNote.State.Value); } - /// - /// Stores the start time of the fade animation that plays when any of the nested - /// hitobjects of the hold note are missed. - /// - private double? missFadeTime; - private void applyCustomUpdateState(DrawableHitObject hitObject, ArmedState state) { if (state == ArmedState.Miss) - missFadeTime = hitObject.StateUpdateTime; - - if (missFadeTime == null) - return; - - // this state update could come from any nested object of the hold note. - // make sure the transforms are consistent across all affected parts - // even if they're idle. - using (BeginAbsoluteSequence(missFadeTime.Value)) - { - // colour and duration matches stable - // transforms not applied to entire hold note in order to not affect hit lighting - holdNote.Head.FadeColour(Colour4.DarkGray, 60); - bodySprite?.FadeColour(Colour4.DarkGray, 60); - holdNote.Tail.FadeColour(Colour4.DarkGray, 60); - } + missFadeTime.Value ??= hitObject.StateUpdateTime; } private void onIsHittingChanged(ValueChangedEvent isHitting) @@ -196,6 +182,29 @@ namespace osu.Game.Rulesets.Mania.Skinning } } + private void onMissFadeTimeChanged(ValueChangedEvent missFadeTimeChange) + { + if (missFadeTimeChange.NewValue == null) + return; + + // this update could come from any nested object of the hold note (or even from an input). + // make sure the transforms are consistent across all affected parts. + using (BeginAbsoluteSequence(missFadeTimeChange.NewValue.Value)) + { + // colour and duration matches stable + // transforms not applied to entire hold note in order to not affect hit lighting + holdNote.Head.FadeColour(Colour4.DarkGray, 60); + holdNote.Tail.FadeColour(Colour4.DarkGray, 60); + bodySprite?.FadeColour(Colour4.DarkGray, 60); + } + } + + protected override void Update() + { + base.Update(); + missFadeTime.Value ??= holdNote.HoldBrokenTime; + } + protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); From ba30800bf4d242b30f0909bf97a146cced4e6ccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 13 Nov 2020 22:21:22 +0100 Subject: [PATCH 7/9] Extract constant --- osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs index 9ed25115d8..f1f72c8618 100644 --- a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs +++ b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs @@ -193,9 +193,11 @@ namespace osu.Game.Rulesets.Mania.Skinning { // colour and duration matches stable // transforms not applied to entire hold note in order to not affect hit lighting - holdNote.Head.FadeColour(Colour4.DarkGray, 60); - holdNote.Tail.FadeColour(Colour4.DarkGray, 60); - bodySprite?.FadeColour(Colour4.DarkGray, 60); + const double fade_duration = 60; + + holdNote.Head.FadeColour(Colour4.DarkGray, fade_duration); + holdNote.Tail.FadeColour(Colour4.DarkGray, fade_duration); + bodySprite?.FadeColour(Colour4.DarkGray, fade_duration); } } From e88920442c7f3a2ce09b17d346737018c54bbb8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 16 Nov 2020 20:01:10 +0100 Subject: [PATCH 8/9] Use HitStateUpdateTime instead --- osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs index f1f72c8618..328c4e547b 100644 --- a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs +++ b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs @@ -120,7 +120,7 @@ namespace osu.Game.Rulesets.Mania.Skinning private void applyCustomUpdateState(DrawableHitObject hitObject, ArmedState state) { if (state == ArmedState.Miss) - missFadeTime.Value ??= hitObject.StateUpdateTime; + missFadeTime.Value ??= hitObject.HitStateUpdateTime; } private void onIsHittingChanged(ValueChangedEvent isHitting) From 21f29e28e2a11f33b702c3dfbe92d5da32a92b36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 16 Nov 2020 20:36:56 +0100 Subject: [PATCH 9/9] Add clarification comment --- osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs index 328c4e547b..8902d82f33 100644 --- a/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs +++ b/osu.Game.Rulesets.Mania/Skinning/LegacyBodyPiece.cs @@ -119,6 +119,7 @@ namespace osu.Game.Rulesets.Mania.Skinning private void applyCustomUpdateState(DrawableHitObject hitObject, ArmedState state) { + // ensure that the hold note is also faded out when the head/tail/any tick is missed. if (state == ArmedState.Miss) missFadeTime.Value ??= hitObject.HitStateUpdateTime; }