From 7b82d184bd48dd3130552596ff1115a58986c651 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 25 Jul 2019 11:07:53 +0300 Subject: [PATCH 01/33] Add bindable boolean for break times --- osu.Game/Screens/Play/BreakOverlay.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index d390787090..0941b8a452 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -2,7 +2,9 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using System.Linq; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; @@ -35,6 +37,11 @@ namespace osu.Game.Screens.Play public override bool RemoveCompletedTransforms => false; + /// + /// Whether we are currently in the break time range. + /// + public readonly BindableBool IsBreakTime = new BindableBool(); + private readonly Container remainingTimeAdjustmentBox; private readonly Container remainingTimeBox; private readonly RemainingTimeCounter remainingTimeCounter; @@ -109,6 +116,13 @@ namespace osu.Game.Screens.Play initializeBreaks(); } + protected override void Update() + { + base.Update(); + + IsBreakTime.Value = breaks.Where(b => b.HasEffect).Any(b => Clock.CurrentTime >= b.StartTime && Clock.CurrentTime <= b.EndTime); + } + private void initializeBreaks() { if (!IsLoaded) return; // we need a clock. From 0ea0a10ca45ee73e08efad09e55e98f17b1ee2da Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 25 Jul 2019 11:26:38 +0300 Subject: [PATCH 02/33] Expose as IBindable --- osu.Game/Screens/Play/BreakOverlay.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 0941b8a452..593b54afb8 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -41,6 +41,9 @@ namespace osu.Game.Screens.Play /// Whether we are currently in the break time range. /// public readonly BindableBool IsBreakTime = new BindableBool(); + public IBindable IsBreakTime => isBreakTime; + + private readonly BindableBool isBreakTime = new BindableBool(); private readonly Container remainingTimeAdjustmentBox; private readonly Container remainingTimeBox; From 69d2f57f4fadc108076365780f13b96b6e8e08cc Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 25 Jul 2019 11:27:01 +0300 Subject: [PATCH 03/33] Avoid using LINQ --- osu.Game/Screens/Play/BreakOverlay.cs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 593b54afb8..bebbbc5b68 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -123,7 +122,24 @@ namespace osu.Game.Screens.Play { base.Update(); - IsBreakTime.Value = breaks.Where(b => b.HasEffect).Any(b => Clock.CurrentTime >= b.StartTime && Clock.CurrentTime <= b.EndTime); + updateBreakTimeBindable(); + } + + private void updateBreakTimeBindable() + { + foreach (var b in breaks) + { + if (!b.HasEffect) + continue; + + if (Clock.CurrentTime >= b.StartTime && Clock.CurrentTime <= b.EndTime) + { + isBreakTime.Value = true; + return; + } + } + + isBreakTime.Value = false; } private void initializeBreaks() From 9bd66b6e7a777f9273efd5b6c7734db909c78b2f Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 25 Jul 2019 11:27:32 +0300 Subject: [PATCH 04/33] Better xmldoc --- osu.Game/Screens/Play/BreakOverlay.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index bebbbc5b68..5958ca77d8 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -37,9 +37,8 @@ namespace osu.Game.Screens.Play public override bool RemoveCompletedTransforms => false; /// - /// Whether we are currently in the break time range. + /// Whether the gameplay is currently in a break. /// - public readonly BindableBool IsBreakTime = new BindableBool(); public IBindable IsBreakTime => isBreakTime; private readonly BindableBool isBreakTime = new BindableBool(); From 172a9ce33a29a5286d23f46fc0b752e795362e12 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 25 Jul 2019 11:40:45 +0300 Subject: [PATCH 05/33] Use a for loop instead of foreach avoid allocating an iterator --- osu.Game/Screens/Play/BreakOverlay.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 5958ca77d8..918bb1e5c9 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -126,12 +126,12 @@ namespace osu.Game.Screens.Play private void updateBreakTimeBindable() { - foreach (var b in breaks) + for (int i = 0; i < breaks.Count; i++) { - if (!b.HasEffect) + if (!breaks[i].HasEffect) continue; - if (Clock.CurrentTime >= b.StartTime && Clock.CurrentTime <= b.EndTime) + if (Clock.CurrentTime >= breaks[i].StartTime && Clock.CurrentTime <= breaks[i].EndTime) { isBreakTime.Value = true; return; From 5a55433d6c7e2b76e8b7ceabd732374e0573efc1 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 25 Jul 2019 11:53:32 +0300 Subject: [PATCH 06/33] Return if breaks are null Fixes a test --- osu.Game/Screens/Play/BreakOverlay.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 918bb1e5c9..8b9431a87c 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -126,6 +126,9 @@ namespace osu.Game.Screens.Play private void updateBreakTimeBindable() { + if (breaks == null) + return; + for (int i = 0; i < breaks.Count; i++) { if (!breaks[i].HasEffect) From cdda264c491c4cef4b122ece777e04413e57cbc5 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Thu, 25 Jul 2019 12:28:21 +0300 Subject: [PATCH 07/33] Use global index and move it to find if break time Avoid using iterations --- osu.Game/Screens/Play/BreakOverlay.cs | 38 ++++++++++++++++++--------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 8b9431a87c..0470cdb0d5 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -30,6 +31,8 @@ namespace osu.Game.Screens.Play set { breaks = value; + currentBreakIndex = 0; + initializeBreaks(); } } @@ -41,6 +44,7 @@ namespace osu.Game.Screens.Play /// public IBindable IsBreakTime => isBreakTime; + private int currentBreakIndex; private readonly BindableBool isBreakTime = new BindableBool(); private readonly Container remainingTimeAdjustmentBox; @@ -126,22 +130,30 @@ namespace osu.Game.Screens.Play private void updateBreakTimeBindable() { - if (breaks == null) - return; - - for (int i = 0; i < breaks.Count; i++) + if (breaks == null || !breaks.Any()) { - if (!breaks[i].HasEffect) - continue; - - if (Clock.CurrentTime >= breaks[i].StartTime && Clock.CurrentTime <= breaks[i].EndTime) - { - isBreakTime.Value = true; - return; - } + isBreakTime.Value = false; + return; } - isBreakTime.Value = false; + int indexDirection = Clock.CurrentTime < breaks[currentBreakIndex].StartTime ? -1 : (Clock.CurrentTime > breaks[currentBreakIndex].EndTime ? 1 : 0); + + while (Clock.CurrentTime < breaks[currentBreakIndex].StartTime || Clock.CurrentTime > breaks[currentBreakIndex].EndTime) + { + currentBreakIndex += indexDirection; + + if (currentBreakIndex < 0 || currentBreakIndex >= breaks.Count) + break; + } + + if (currentBreakIndex < 0 || currentBreakIndex >= breaks.Count) + { + isBreakTime.Value = false; + currentBreakIndex = 0; + return; + } + + isBreakTime.Value = true; } private void initializeBreaks() From 5e5101280054709321e3f528312605be91843b28 Mon Sep 17 00:00:00 2001 From: Shane Woolcock Date: Thu, 25 Jul 2019 22:54:05 +0930 Subject: [PATCH 08/33] Rewrite updateBreakTimeBindable --- .../Visual/Gameplay/TestSceneBreakOverlay.cs | 26 ++++++++++++++++- osu.Game/Screens/Play/BreakOverlay.cs | 29 +++++++------------ 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs index 3cd1b8307a..ffa822d175 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs @@ -1,8 +1,13 @@ // 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 System.Collections.Generic; using NUnit.Framework; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Sprites; using osu.Game.Beatmaps.Timing; using osu.Game.Screens.Play; @@ -11,11 +16,30 @@ namespace osu.Game.Tests.Visual.Gameplay [TestFixture] public class TestSceneBreakOverlay : OsuTestScene { + public override IReadOnlyList RequiredTypes => new[] + { + typeof(BreakOverlay), + }; + private readonly BreakOverlay breakOverlay; + private readonly IBindable isBreakTimeBindable = new BindableBool(); public TestSceneBreakOverlay() { - Child = breakOverlay = new BreakOverlay(true); + SpriteText breakTimeText; + Child = new FillFlowContainer + { + RelativeSizeAxes = Axes.Both, + Direction = FillDirection.Vertical, + Children = new Drawable[] + { + breakTimeText = new SpriteText(), + breakOverlay = new BreakOverlay(true) + } + }; + + isBreakTimeBindable.BindTo(breakOverlay.IsBreakTime); + isBreakTimeBindable.BindValueChanged(e => breakTimeText.Text = $"IsBreakTime: {e.NewValue}", true); AddStep("2s break", () => startBreak(2000)); AddStep("5s break", () => startBreak(5000)); diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 0470cdb0d5..aceacdbab1 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -31,7 +31,7 @@ namespace osu.Game.Screens.Play set { breaks = value; - currentBreakIndex = 0; + nearestBreakIndex = 0; initializeBreaks(); } @@ -44,7 +44,7 @@ namespace osu.Game.Screens.Play /// public IBindable IsBreakTime => isBreakTime; - private int currentBreakIndex; + private int nearestBreakIndex; private readonly BindableBool isBreakTime = new BindableBool(); private readonly Container remainingTimeAdjustmentBox; @@ -136,24 +136,17 @@ namespace osu.Game.Screens.Play return; } - int indexDirection = Clock.CurrentTime < breaks[currentBreakIndex].StartTime ? -1 : (Clock.CurrentTime > breaks[currentBreakIndex].EndTime ? 1 : 0); + while (nearestBreakIndex < breaks.Count - 1 && Clock.CurrentTime > breaks[nearestBreakIndex].EndTime) + nearestBreakIndex++; - while (Clock.CurrentTime < breaks[currentBreakIndex].StartTime || Clock.CurrentTime > breaks[currentBreakIndex].EndTime) - { - currentBreakIndex += indexDirection; + while (nearestBreakIndex > 0 && Clock.CurrentTime < breaks[nearestBreakIndex].StartTime) + nearestBreakIndex--; - if (currentBreakIndex < 0 || currentBreakIndex >= breaks.Count) - break; - } - - if (currentBreakIndex < 0 || currentBreakIndex >= breaks.Count) - { - isBreakTime.Value = false; - currentBreakIndex = 0; - return; - } - - isBreakTime.Value = true; + // This ensures that IsBreakTime is generally consistent with the overlay's transforms during a break. + // If the overlay never shows (break.HasEffect is false), IsBreakTime should be false. + // We also assume that the overlay's fade out transform is "not break time". + var nearestBreak = breaks[nearestBreakIndex]; + isBreakTime.Value = nearestBreak.HasEffect && Clock.CurrentTime >= nearestBreak.StartTime && Clock.CurrentTime <= nearestBreak.EndTime - fade_duration; } private void initializeBreaks() From 1d6c321e14ff3c820ae2f3578f62e239cacbdd18 Mon Sep 17 00:00:00 2001 From: Shane Woolcock Date: Fri, 26 Jul 2019 08:34:18 +0930 Subject: [PATCH 09/33] Ensure we don't ping-pong nearestBreakIndex between breaks --- osu.Game/Screens/Play/BreakOverlay.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index aceacdbab1..50b2c97f59 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -136,11 +136,16 @@ namespace osu.Game.Screens.Play return; } - while (nearestBreakIndex < breaks.Count - 1 && Clock.CurrentTime > breaks[nearestBreakIndex].EndTime) - nearestBreakIndex++; - - while (nearestBreakIndex > 0 && Clock.CurrentTime < breaks[nearestBreakIndex].StartTime) - nearestBreakIndex--; + if (Clock.CurrentTime > breaks[nearestBreakIndex].EndTime) + { + while (nearestBreakIndex < breaks.Count - 1 && Clock.CurrentTime > breaks[nearestBreakIndex].EndTime) + nearestBreakIndex++; + } + else + { + while (nearestBreakIndex > 0 && Clock.CurrentTime < breaks[nearestBreakIndex].StartTime) + nearestBreakIndex--; + } // This ensures that IsBreakTime is generally consistent with the overlay's transforms during a break. // If the overlay never shows (break.HasEffect is false), IsBreakTime should be false. From a08d54eb06b2b2686152b9b04729ba3aabe65fdb Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Fri, 26 Jul 2019 03:11:59 +0300 Subject: [PATCH 10/33] Remove unnecessary checks --- osu.Game/Screens/Play/BreakOverlay.cs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 50b2c97f59..aceacdbab1 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -136,16 +136,11 @@ namespace osu.Game.Screens.Play return; } - if (Clock.CurrentTime > breaks[nearestBreakIndex].EndTime) - { - while (nearestBreakIndex < breaks.Count - 1 && Clock.CurrentTime > breaks[nearestBreakIndex].EndTime) - nearestBreakIndex++; - } - else - { - while (nearestBreakIndex > 0 && Clock.CurrentTime < breaks[nearestBreakIndex].StartTime) - nearestBreakIndex--; - } + while (nearestBreakIndex < breaks.Count - 1 && Clock.CurrentTime > breaks[nearestBreakIndex].EndTime) + nearestBreakIndex++; + + while (nearestBreakIndex > 0 && Clock.CurrentTime < breaks[nearestBreakIndex].StartTime) + nearestBreakIndex--; // This ensures that IsBreakTime is generally consistent with the overlay's transforms during a break. // If the overlay never shows (break.HasEffect is false), IsBreakTime should be false. From b4c93b1777646c59507bed626c0c79d2e5f4f82b Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Fri, 26 Jul 2019 05:11:01 +0300 Subject: [PATCH 11/33] Use lookup direction than 2 while loops --- osu.Game/Screens/Play/BreakOverlay.cs | 31 +++++++++++++++++++-------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index aceacdbab1..9091e54159 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -31,7 +31,9 @@ namespace osu.Game.Screens.Play set { breaks = value; - nearestBreakIndex = 0; + + // reset index in case the new breaks list is smaller than last one + currentBreakIndex = 0; initializeBreaks(); } @@ -44,7 +46,7 @@ namespace osu.Game.Screens.Play /// public IBindable IsBreakTime => isBreakTime; - private int nearestBreakIndex; + private int currentBreakIndex; private readonly BindableBool isBreakTime = new BindableBool(); private readonly Container remainingTimeAdjustmentBox; @@ -133,20 +135,31 @@ namespace osu.Game.Screens.Play if (breaks == null || !breaks.Any()) { isBreakTime.Value = false; + currentBreakIndex = 0; return; } - while (nearestBreakIndex < breaks.Count - 1 && Clock.CurrentTime > breaks[nearestBreakIndex].EndTime) - nearestBreakIndex++; + var lastIndex = currentBreakIndex; + var lookupDirection = Clock.CurrentTime > breaks[lastIndex].EndTime ? 1 : (Clock.CurrentTime < breaks[lastIndex].StartTime ? -1 : 0); - while (nearestBreakIndex > 0 && Clock.CurrentTime < breaks[nearestBreakIndex].StartTime) - nearestBreakIndex--; + while (Clock.CurrentTime < breaks[currentBreakIndex].StartTime || Clock.CurrentTime > breaks[currentBreakIndex].EndTime) + { + currentBreakIndex += lookupDirection; + + // restore index if out of bounds + if (currentBreakIndex < 0 || currentBreakIndex >= breaks.Count) + { + isBreakTime.Value = false; + currentBreakIndex = lastIndex; + return; + } + } // This ensures that IsBreakTime is generally consistent with the overlay's transforms during a break. - // If the overlay never shows (break.HasEffect is false), IsBreakTime should be false. + // If the current break doesn't have effects, IsBreakTime should be false. // We also assume that the overlay's fade out transform is "not break time". - var nearestBreak = breaks[nearestBreakIndex]; - isBreakTime.Value = nearestBreak.HasEffect && Clock.CurrentTime >= nearestBreak.StartTime && Clock.CurrentTime <= nearestBreak.EndTime - fade_duration; + var currentBreak = breaks[currentBreakIndex]; + isBreakTime.Value = currentBreak.HasEffect && Clock.CurrentTime <= currentBreak.EndTime - fade_duration; } private void initializeBreaks() From 91fa8a6552c4591ad78b891be086159aa556baf6 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Fri, 26 Jul 2019 08:09:18 +0300 Subject: [PATCH 12/33] Simplify null and any check --- osu.Game/Screens/Play/BreakOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 9091e54159..86a991c02e 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -132,7 +132,7 @@ namespace osu.Game.Screens.Play private void updateBreakTimeBindable() { - if (breaks == null || !breaks.Any()) + if (breaks?.Any() != true) { isBreakTime.Value = false; currentBreakIndex = 0; From 806d41daf499df87917abaf0c3dd8772ee557e75 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Fri, 26 Jul 2019 08:11:13 +0300 Subject: [PATCH 13/33] Add function to reset break index --- osu.Game/Screens/Play/BreakOverlay.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 86a991c02e..99361a5bd7 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -33,7 +33,7 @@ namespace osu.Game.Screens.Play breaks = value; // reset index in case the new breaks list is smaller than last one - currentBreakIndex = 0; + resetBreakIndex(); initializeBreaks(); } @@ -130,12 +130,17 @@ namespace osu.Game.Screens.Play updateBreakTimeBindable(); } + private void resetBreakIndex() + { + isBreakTime.Value = false; + currentBreakIndex = 0; + } + private void updateBreakTimeBindable() { if (breaks?.Any() != true) { - isBreakTime.Value = false; - currentBreakIndex = 0; + resetBreakIndex(); return; } From 3fa6804501a0d417feb4a876160fc83731101140 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Fri, 26 Jul 2019 08:12:32 +0300 Subject: [PATCH 14/33] Use better loops for moving index Easy to read, suggested by peppy --- osu.Game/Screens/Play/BreakOverlay.cs | 31 +++++++++++++++++---------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 99361a5bd7..f3decb38fb 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -144,19 +144,28 @@ namespace osu.Game.Screens.Play return; } - var lastIndex = currentBreakIndex; - var lookupDirection = Clock.CurrentTime > breaks[lastIndex].EndTime ? 1 : (Clock.CurrentTime < breaks[lastIndex].StartTime ? -1 : 0); + var time = Clock.CurrentTime; - while (Clock.CurrentTime < breaks[currentBreakIndex].StartTime || Clock.CurrentTime > breaks[currentBreakIndex].EndTime) + if (time > breaks[currentBreakIndex].EndTime) { - currentBreakIndex += lookupDirection; - - // restore index if out of bounds - if (currentBreakIndex < 0 || currentBreakIndex >= breaks.Count) + for (int i = currentBreakIndex; i < breaks.Count; i++) { - isBreakTime.Value = false; - currentBreakIndex = lastIndex; - return; + if (time <= breaks[i].EndTime) + { + currentBreakIndex = i; + break; + } + } + } + else if (time < breaks[currentBreakIndex].StartTime) + { + for (int i = currentBreakIndex; i >= 0; i--) + { + if (time >= breaks[i].StartTime) + { + currentBreakIndex = i; + break; + } } } @@ -164,7 +173,7 @@ namespace osu.Game.Screens.Play // If the current break doesn't have effects, IsBreakTime should be false. // We also assume that the overlay's fade out transform is "not break time". var currentBreak = breaks[currentBreakIndex]; - isBreakTime.Value = currentBreak.HasEffect && Clock.CurrentTime <= currentBreak.EndTime - fade_duration; + isBreakTime.Value = currentBreak.HasEffect && time >= currentBreak.StartTime && time <= currentBreak.EndTime - fade_duration; } private void initializeBreaks() From 6fac716ec704f1c07436d57a2979e95f0b504966 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Fri, 26 Jul 2019 08:15:46 +0300 Subject: [PATCH 15/33] Use container instead of fill flow --- .../Visual/Gameplay/TestSceneBreakOverlay.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs index ffa822d175..85f69bf058 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs @@ -27,13 +27,17 @@ namespace osu.Game.Tests.Visual.Gameplay public TestSceneBreakOverlay() { SpriteText breakTimeText; - Child = new FillFlowContainer + Child = new Container { RelativeSizeAxes = Axes.Both, - Direction = FillDirection.Vertical, Children = new Drawable[] { - breakTimeText = new SpriteText(), + breakTimeText = new SpriteText + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Margin = new MarginPadding { Bottom = 35 }, + }, breakOverlay = new BreakOverlay(true) } }; From 5a94a223147b4e774452154b41ed2c4794d30615 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Fri, 26 Jul 2019 09:17:39 +0300 Subject: [PATCH 16/33] Add a quick check if we're not in a break with current index --- osu.Game/Screens/Play/BreakOverlay.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index f3decb38fb..8f07b90b30 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -146,6 +146,16 @@ namespace osu.Game.Screens.Play var time = Clock.CurrentTime; + if (currentBreakIndex < breaks.Count - 1) + { + // Quick check if we're not in a break with our current index. + if (time > breaks[currentBreakIndex].EndTime && time < breaks[currentBreakIndex + 1].StartTime) + { + isBreakTime.Value = false; + return; + } + } + if (time > breaks[currentBreakIndex].EndTime) { for (int i = currentBreakIndex; i < breaks.Count; i++) From 4c9e8527d86b57c65774b89c7db6ae552c9ae64b Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Fri, 26 Jul 2019 09:24:53 +0300 Subject: [PATCH 17/33] Modify global index directly in the for loop Moves the global index to a near break if not in a break yet --- osu.Game/Screens/Play/BreakOverlay.cs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 8f07b90b30..005cdd36d7 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -158,25 +158,15 @@ namespace osu.Game.Screens.Play if (time > breaks[currentBreakIndex].EndTime) { - for (int i = currentBreakIndex; i < breaks.Count; i++) - { - if (time <= breaks[i].EndTime) - { - currentBreakIndex = i; + for (; currentBreakIndex < breaks.Count; currentBreakIndex++) + if (time <= breaks[currentBreakIndex].EndTime) break; - } - } } else if (time < breaks[currentBreakIndex].StartTime) { - for (int i = currentBreakIndex; i >= 0; i--) - { - if (time >= breaks[i].StartTime) - { - currentBreakIndex = i; + for (; currentBreakIndex >= 0; currentBreakIndex--) + if (time >= breaks[currentBreakIndex].StartTime) break; - } - } } // This ensures that IsBreakTime is generally consistent with the overlay's transforms during a break. From 46f17885c6378aaaeb19e7cd0043629cefa494ff Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sat, 27 Jul 2019 15:51:14 +0300 Subject: [PATCH 18/33] Rewrite break overlay test --- .../Visual/Gameplay/TestSceneBreakOverlay.cs | 180 +++++++++++------- 1 file changed, 107 insertions(+), 73 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs index 85f69bf058..bbebd62b26 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs @@ -3,11 +3,10 @@ using System; using System.Collections.Generic; +using System.Linq; using NUnit.Framework; -using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Sprites; +using osu.Framework.Timing; using osu.Game.Beatmaps.Timing; using osu.Game.Screens.Play; @@ -21,96 +20,131 @@ namespace osu.Game.Tests.Visual.Gameplay typeof(BreakOverlay), }; - private readonly BreakOverlay breakOverlay; - private readonly IBindable isBreakTimeBindable = new BindableBool(); + private readonly BreakOverlay breakOverlay, manualBreakOverlay; + private readonly ManualClock manualClock = new ManualClock(); + + private readonly IReadOnlyList testBreaks = new List + { + new BreakPeriod + { + StartTime = 1000, + EndTime = 5000, + }, + new BreakPeriod + { + StartTime = 6000, + EndTime = 13500, + }, + }; public TestSceneBreakOverlay() { - SpriteText breakTimeText; - Child = new Container + Add(breakOverlay = new BreakOverlay(true)); + Add(manualBreakOverlay = new BreakOverlay(true) { - RelativeSizeAxes = Axes.Both, - Children = new Drawable[] - { - breakTimeText = new SpriteText - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Margin = new MarginPadding { Bottom = 35 }, - }, - breakOverlay = new BreakOverlay(true) - } - }; - - isBreakTimeBindable.BindTo(breakOverlay.IsBreakTime); - isBreakTimeBindable.BindValueChanged(e => breakTimeText.Text = $"IsBreakTime: {e.NewValue}", true); - - AddStep("2s break", () => startBreak(2000)); - AddStep("5s break", () => startBreak(5000)); - AddStep("10s break", () => startBreak(10000)); - AddStep("15s break", () => startBreak(15000)); - AddStep("2s, 2s", startMultipleBreaks); - AddStep("0.5s, 0.7s, 1s, 2s", startAnotherMultipleBreaks); + Alpha = 0, + Clock = new FramedClock(manualClock), + }); } - private void startBreak(double duration) + [Test] + public void TestShowBreaks() { - breakOverlay.Breaks = new List + loadClockStep(false); + + addShowBreakStep(2); + addShowBreakStep(5); + addShowBreakStep(15); + } + + [Test] + public void TestNoEffectsBreak() + { + var shortBreak = new BreakPeriod { EndTime = 500 }; + + loadClockStep(true); + AddStep("start short break", () => manualBreakOverlay.Breaks = new[] { shortBreak }); + + seekBreakStep("seek back to 0", 0, false); + addBreakSeeks(shortBreak, false); + } + + [Test] + public void TestMultipleBreaks() + { + loadClockStep(true); + AddStep("start multiple breaks", () => manualBreakOverlay.Breaks = testBreaks); + + seekBreakStep("seek back to 0", 0, false); + foreach (var b in testBreaks) + addBreakSeeks(b, false); + } + + [Test] + public void TestRewindBreaks() + { + loadClockStep(true); + AddStep("start multiple breaks in rewind", () => manualBreakOverlay.Breaks = testBreaks); + + seekBreakStep("seek back to 0", 0, false); + foreach (var b in testBreaks.Reverse()) + addBreakSeeks(b, true); + } + + [Test] + public void TestSkipBreaks() + { + loadClockStep(true); + AddStep("start multiple breaks with skipping", () => manualBreakOverlay.Breaks = testBreaks); + + var b = testBreaks.Last(); + seekBreakStep("seek back to 0", 0, false); + addBreakSeeks(b, false); + } + + private void addShowBreakStep(double seconds) + { + AddStep($"show '{seconds}s' break", () => breakOverlay.Breaks = new List { new BreakPeriod { StartTime = Clock.CurrentTime, - EndTime = Clock.CurrentTime + duration, + EndTime = Clock.CurrentTime + seconds * 1000, } - }; + }); } - private void startMultipleBreaks() + private void loadClockStep(bool loadManual) { - double currentTime = Clock.CurrentTime; - - breakOverlay.Breaks = new List + AddStep($"load {(loadManual ? "manual" : "normal")} clock", () => { - new BreakPeriod - { - StartTime = currentTime, - EndTime = currentTime + 2000, - }, - new BreakPeriod - { - StartTime = currentTime + 4000, - EndTime = currentTime + 6000, - } - }; + breakOverlay.FadeTo(loadManual ? 0 : 1); + manualBreakOverlay.FadeTo(loadManual ? 1 : 0); + }); } - private void startAnotherMultipleBreaks() + private void addBreakSeeks(BreakPeriod b, bool isReversed) { - double currentTime = Clock.CurrentTime; - - breakOverlay.Breaks = new List + if (isReversed) { - new BreakPeriod // Duration is less than 650 - too short to appear - { - StartTime = currentTime, - EndTime = currentTime + 500, - }, - new BreakPeriod - { - StartTime = currentTime + 1500, - EndTime = currentTime + 2200, - }, - new BreakPeriod - { - StartTime = currentTime + 3200, - EndTime = currentTime + 4200, - }, - new BreakPeriod - { - StartTime = currentTime + 5200, - EndTime = currentTime + 7200, - } - }; + seekBreakStep("seek to break after end", b.EndTime + 500, false); + seekBreakStep("seek to break end", b.EndTime, false); + seekBreakStep("seek to break middle", b.StartTime + b.Duration / 2, b.HasEffect); + seekBreakStep("seek to break start", b.StartTime, b.HasEffect); + } + else + { + seekBreakStep("seek to break start", b.StartTime, b.HasEffect); + seekBreakStep("seek to break middle", b.StartTime + b.Duration / 2, b.HasEffect); + seekBreakStep("seek to break end", b.EndTime, false); + seekBreakStep("seek to break after end", b.EndTime + 500, false); + } + } + + private void seekBreakStep(string seekStepDescription, double time, bool onBreak) + { + AddStep(seekStepDescription, () => manualClock.CurrentTime = time); + AddAssert($"is{(!onBreak ? " not " : " ")}break time", () => manualBreakOverlay.IsBreakTime.Value == onBreak); } } } From 6c580ac9d5c4456bc91a1f8a51f4d92457167669 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sat, 27 Jul 2019 15:52:01 +0300 Subject: [PATCH 19/33] Use while loops instead --- osu.Game/Screens/Play/BreakOverlay.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index f96c176104..532109d14b 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -158,15 +158,13 @@ namespace osu.Game.Screens.Play if (time > breaks[currentBreakIndex].EndTime) { - for (; currentBreakIndex < breaks.Count; currentBreakIndex++) - if (time <= breaks[currentBreakIndex].EndTime) - break; + while (time > breaks[currentBreakIndex].EndTime && currentBreakIndex < breaks.Count - 1) + currentBreakIndex++; } else if (time < breaks[currentBreakIndex].StartTime) { - for (; currentBreakIndex >= 0; currentBreakIndex--) - if (time >= breaks[currentBreakIndex].StartTime) - break; + while (time < breaks[currentBreakIndex].StartTime && currentBreakIndex > 0) + currentBreakIndex--; } // This ensures that IsBreakTime is generally consistent with the overlay's transforms during a break. From 95b568eb4646c45b2169899d599114f86c350865 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sat, 27 Jul 2019 15:52:30 +0300 Subject: [PATCH 20/33] Remove unnecessary condition --- osu.Game/Screens/Play/BreakOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 532109d14b..7e87c44259 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -161,7 +161,7 @@ namespace osu.Game.Screens.Play while (time > breaks[currentBreakIndex].EndTime && currentBreakIndex < breaks.Count - 1) currentBreakIndex++; } - else if (time < breaks[currentBreakIndex].StartTime) + else { while (time < breaks[currentBreakIndex].StartTime && currentBreakIndex > 0) currentBreakIndex--; From 4d49b38277ec35df3723eac519e6cba80f681e1a Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sun, 28 Jul 2019 05:21:27 +0300 Subject: [PATCH 21/33] Add a function for loading a list of breaks --- .../Visual/Gameplay/TestSceneBreakOverlay.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs index bbebd62b26..c238f42b60 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs @@ -63,9 +63,8 @@ namespace osu.Game.Tests.Visual.Gameplay var shortBreak = new BreakPeriod { EndTime = 500 }; loadClockStep(true); - AddStep("start short break", () => manualBreakOverlay.Breaks = new[] { shortBreak }); + loadBreaksStep("short break", new[] { shortBreak }); - seekBreakStep("seek back to 0", 0, false); addBreakSeeks(shortBreak, false); } @@ -73,9 +72,8 @@ namespace osu.Game.Tests.Visual.Gameplay public void TestMultipleBreaks() { loadClockStep(true); - AddStep("start multiple breaks", () => manualBreakOverlay.Breaks = testBreaks); + loadBreaksStep("multiple breaks", testBreaks); - seekBreakStep("seek back to 0", 0, false); foreach (var b in testBreaks) addBreakSeeks(b, false); } @@ -84,9 +82,8 @@ namespace osu.Game.Tests.Visual.Gameplay public void TestRewindBreaks() { loadClockStep(true); - AddStep("start multiple breaks in rewind", () => manualBreakOverlay.Breaks = testBreaks); + loadBreaksStep("multiple breaks", testBreaks); - seekBreakStep("seek back to 0", 0, false); foreach (var b in testBreaks.Reverse()) addBreakSeeks(b, true); } @@ -95,10 +92,9 @@ namespace osu.Game.Tests.Visual.Gameplay public void TestSkipBreaks() { loadClockStep(true); - AddStep("start multiple breaks with skipping", () => manualBreakOverlay.Breaks = testBreaks); + loadBreaksStep("multiple breaks", testBreaks); var b = testBreaks.Last(); - seekBreakStep("seek back to 0", 0, false); addBreakSeeks(b, false); } @@ -123,6 +119,12 @@ namespace osu.Game.Tests.Visual.Gameplay }); } + private void loadBreaksStep(string breakDescription, IReadOnlyList breaks) + { + AddStep($"load {breakDescription}", () => manualBreakOverlay.Breaks = breaks); + seekBreakStep("seek back to 0", 0, false); + } + private void addBreakSeeks(BreakPeriod b, bool isReversed) { if (isReversed) From 4143bafd67ac14e61b98855797a96bec2fac4647 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sun, 28 Jul 2019 05:22:09 +0300 Subject: [PATCH 22/33] More simplifies --- osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs index c238f42b60..d2db92c855 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs @@ -21,7 +21,7 @@ namespace osu.Game.Tests.Visual.Gameplay }; private readonly BreakOverlay breakOverlay, manualBreakOverlay; - private readonly ManualClock manualClock = new ManualClock(); + private readonly ManualClock manualClock; private readonly IReadOnlyList testBreaks = new List { @@ -43,7 +43,7 @@ namespace osu.Game.Tests.Visual.Gameplay Add(manualBreakOverlay = new BreakOverlay(true) { Alpha = 0, - Clock = new FramedClock(manualClock), + Clock = new FramedClock(manualClock = new ManualClock()), }); } @@ -94,8 +94,7 @@ namespace osu.Game.Tests.Visual.Gameplay loadClockStep(true); loadBreaksStep("multiple breaks", testBreaks); - var b = testBreaks.Last(); - addBreakSeeks(b, false); + addBreakSeeks(testBreaks.Last(), false); } private void addShowBreakStep(double seconds) From 1dd3a6630082b0e0b88b43be089f471b54d19aec Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sun, 28 Jul 2019 09:16:19 +0300 Subject: [PATCH 23/33] Remove unnecessary index resets --- osu.Game/Screens/Play/BreakOverlay.cs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 7e87c44259..726c825c84 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -33,7 +33,8 @@ namespace osu.Game.Screens.Play breaks = value; // reset index in case the new breaks list is smaller than last one - resetBreakIndex(); + isBreakTime.Value = false; + currentBreakIndex = 0; initializeBreaks(); } @@ -126,23 +127,13 @@ namespace osu.Game.Screens.Play protected override void Update() { base.Update(); - updateBreakTimeBindable(); } - private void resetBreakIndex() - { - isBreakTime.Value = false; - currentBreakIndex = 0; - } - private void updateBreakTimeBindable() { if (breaks?.Any() != true) - { - resetBreakIndex(); return; - } var time = Clock.CurrentTime; From 5bf0277fd401b5d668bd8939ba7a41264a1ca6c9 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sun, 28 Jul 2019 09:17:13 +0300 Subject: [PATCH 24/33] Remove unnecessary quick check Not saving for anything --- osu.Game/Screens/Play/BreakOverlay.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 726c825c84..e3e4014eb3 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -137,16 +137,6 @@ namespace osu.Game.Screens.Play var time = Clock.CurrentTime; - if (currentBreakIndex < breaks.Count - 1) - { - // Quick check if we're not in a break with our current index. - if (time > breaks[currentBreakIndex].EndTime && time < breaks[currentBreakIndex + 1].StartTime) - { - isBreakTime.Value = false; - return; - } - } - if (time > breaks[currentBreakIndex].EndTime) { while (time > breaks[currentBreakIndex].EndTime && currentBreakIndex < breaks.Count - 1) From 37c32659429787f33f3a78005142d5803feb1ac8 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sun, 28 Jul 2019 09:21:54 +0300 Subject: [PATCH 25/33] Manually call the update function on retrieving IsBreakTime value --- .../Visual/Gameplay/TestSceneBreakOverlay.cs | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs index d2db92c855..890c575eb6 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs @@ -5,7 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; using NUnit.Framework; -using osu.Framework.Graphics; +using osu.Framework.Bindables; using osu.Framework.Timing; using osu.Game.Beatmaps.Timing; using osu.Game.Screens.Play; @@ -20,8 +20,9 @@ namespace osu.Game.Tests.Visual.Gameplay typeof(BreakOverlay), }; - private readonly BreakOverlay breakOverlay, manualBreakOverlay; private readonly ManualClock manualClock; + private readonly BreakOverlay breakOverlay; + private readonly TestBreakOverlay manualBreakOverlay; private readonly IReadOnlyList testBreaks = new List { @@ -40,7 +41,7 @@ namespace osu.Game.Tests.Visual.Gameplay public TestSceneBreakOverlay() { Add(breakOverlay = new BreakOverlay(true)); - Add(manualBreakOverlay = new BreakOverlay(true) + Add(manualBreakOverlay = new TestBreakOverlay(true) { Alpha = 0, Clock = new FramedClock(manualClock = new ManualClock()), @@ -147,5 +148,24 @@ namespace osu.Game.Tests.Visual.Gameplay AddStep(seekStepDescription, () => manualClock.CurrentTime = time); AddAssert($"is{(!onBreak ? " not " : " ")}break time", () => manualBreakOverlay.IsBreakTime.Value == onBreak); } + + private class TestBreakOverlay : BreakOverlay + { + public new IBindable IsBreakTime + { + get + { + // Manually call the update function as it might take up to 2 frames for an automatic update to happen + Update(); + + return base.IsBreakTime; + } + } + + public TestBreakOverlay(bool letterboxing) + : base(letterboxing) + { + } + } } } From c9e45f8cdc2e5b26e6d099e21bf2e7d731fa2dab Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Sun, 28 Jul 2019 09:27:02 +0300 Subject: [PATCH 26/33] Assign clocks instead of creating 2 separate overlays --- .../Visual/Gameplay/TestSceneBreakOverlay.cs | 42 +++++++++++-------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs index 890c575eb6..eaae647dbc 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs @@ -20,9 +20,7 @@ namespace osu.Game.Tests.Visual.Gameplay typeof(BreakOverlay), }; - private readonly ManualClock manualClock; - private readonly BreakOverlay breakOverlay; - private readonly TestBreakOverlay manualBreakOverlay; + private readonly TestBreakOverlay breakOverlay; private readonly IReadOnlyList testBreaks = new List { @@ -40,12 +38,7 @@ namespace osu.Game.Tests.Visual.Gameplay public TestSceneBreakOverlay() { - Add(breakOverlay = new BreakOverlay(true)); - Add(manualBreakOverlay = new TestBreakOverlay(true) - { - Alpha = 0, - Clock = new FramedClock(manualClock = new ManualClock()), - }); + Add(breakOverlay = new TestBreakOverlay(true)); } [Test] @@ -112,16 +105,12 @@ namespace osu.Game.Tests.Visual.Gameplay private void loadClockStep(bool loadManual) { - AddStep($"load {(loadManual ? "manual" : "normal")} clock", () => - { - breakOverlay.FadeTo(loadManual ? 0 : 1); - manualBreakOverlay.FadeTo(loadManual ? 1 : 0); - }); + AddStep($"load {(loadManual ? "manual" : "normal")} clock", () => breakOverlay.SwitchClock(loadManual)); } private void loadBreaksStep(string breakDescription, IReadOnlyList breaks) { - AddStep($"load {breakDescription}", () => manualBreakOverlay.Breaks = breaks); + AddStep($"load {breakDescription}", () => breakOverlay.Breaks = breaks); seekBreakStep("seek back to 0", 0, false); } @@ -145,12 +134,22 @@ namespace osu.Game.Tests.Visual.Gameplay private void seekBreakStep(string seekStepDescription, double time, bool onBreak) { - AddStep(seekStepDescription, () => manualClock.CurrentTime = time); - AddAssert($"is{(!onBreak ? " not " : " ")}break time", () => manualBreakOverlay.IsBreakTime.Value == onBreak); + AddStep(seekStepDescription, () => breakOverlay.ManualClockTime = time); + AddAssert($"is{(!onBreak ? " not " : " ")}break time", () => breakOverlay.IsBreakTime.Value == onBreak); } private class TestBreakOverlay : BreakOverlay { + private readonly FramedClock framedManualClock; + private readonly ManualClock manualClock; + private IFrameBasedClock normalClock; + + public double ManualClockTime + { + get => manualClock.CurrentTime; + set => manualClock.CurrentTime = value; + } + public new IBindable IsBreakTime { get @@ -165,6 +164,15 @@ namespace osu.Game.Tests.Visual.Gameplay public TestBreakOverlay(bool letterboxing) : base(letterboxing) { + framedManualClock = new FramedClock(manualClock = new ManualClock()); + } + + public void SwitchClock(bool setManual) => Clock = setManual ? framedManualClock : normalClock; + + protected override void LoadComplete() + { + base.LoadComplete(); + normalClock = Clock; } } } From e8e5b2742d545d1344769afe965073aaeae00e3f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Jul 2019 18:31:57 +0900 Subject: [PATCH 27/33] Fix manual clock usage --- .../Visual/Gameplay/TestSceneBreakOverlay.cs | 65 +++++++++---------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs index eaae647dbc..ed5861b47d 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Linq; using NUnit.Framework; -using osu.Framework.Bindables; using osu.Framework.Timing; using osu.Game.Beatmaps.Timing; using osu.Game.Screens.Play; @@ -44,7 +43,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestShowBreaks() { - loadClockStep(false); + setClock(false); addShowBreakStep(2); addShowBreakStep(5); @@ -56,7 +55,7 @@ namespace osu.Game.Tests.Visual.Gameplay { var shortBreak = new BreakPeriod { EndTime = 500 }; - loadClockStep(true); + setClock(true); loadBreaksStep("short break", new[] { shortBreak }); addBreakSeeks(shortBreak, false); @@ -65,7 +64,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestMultipleBreaks() { - loadClockStep(true); + setClock(true); loadBreaksStep("multiple breaks", testBreaks); foreach (var b in testBreaks) @@ -75,7 +74,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestRewindBreaks() { - loadClockStep(true); + setClock(true); loadBreaksStep("multiple breaks", testBreaks); foreach (var b in testBreaks.Reverse()) @@ -85,7 +84,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestSkipBreaks() { - loadClockStep(true); + setClock(true); loadBreaksStep("multiple breaks", testBreaks); addBreakSeeks(testBreaks.Last(), false); @@ -103,46 +102,50 @@ namespace osu.Game.Tests.Visual.Gameplay }); } - private void loadClockStep(bool loadManual) + private void setClock(bool useManual) { - AddStep($"load {(loadManual ? "manual" : "normal")} clock", () => breakOverlay.SwitchClock(loadManual)); + AddStep($"set {(useManual ? "manual" : "realtime")} clock", () => breakOverlay.SwitchClock(useManual)); } private void loadBreaksStep(string breakDescription, IReadOnlyList breaks) { AddStep($"load {breakDescription}", () => breakOverlay.Breaks = breaks); - seekBreakStep("seek back to 0", 0, false); + seekAndAssertBreak("seek back to 0", 0, false); } private void addBreakSeeks(BreakPeriod b, bool isReversed) { if (isReversed) { - seekBreakStep("seek to break after end", b.EndTime + 500, false); - seekBreakStep("seek to break end", b.EndTime, false); - seekBreakStep("seek to break middle", b.StartTime + b.Duration / 2, b.HasEffect); - seekBreakStep("seek to break start", b.StartTime, b.HasEffect); + seekAndAssertBreak("seek to break after end", b.EndTime + 500, false); + seekAndAssertBreak("seek to break end", b.EndTime, false); + seekAndAssertBreak("seek to break middle", b.StartTime + b.Duration / 2, b.HasEffect); + seekAndAssertBreak("seek to break start", b.StartTime, b.HasEffect); } else { - seekBreakStep("seek to break start", b.StartTime, b.HasEffect); - seekBreakStep("seek to break middle", b.StartTime + b.Duration / 2, b.HasEffect); - seekBreakStep("seek to break end", b.EndTime, false); - seekBreakStep("seek to break after end", b.EndTime + 500, false); + seekAndAssertBreak("seek to break start", b.StartTime, b.HasEffect); + seekAndAssertBreak("seek to break middle", b.StartTime + b.Duration / 2, b.HasEffect); + seekAndAssertBreak("seek to break end", b.EndTime, false); + seekAndAssertBreak("seek to break after end", b.EndTime + 500, false); } } - private void seekBreakStep(string seekStepDescription, double time, bool onBreak) + private void seekAndAssertBreak(string seekStepDescription, double time, bool shouldBeBreak) { AddStep(seekStepDescription, () => breakOverlay.ManualClockTime = time); - AddAssert($"is{(!onBreak ? " not " : " ")}break time", () => breakOverlay.IsBreakTime.Value == onBreak); + AddAssert($"is{(!shouldBeBreak ? " not" : string.Empty)} break time", () => + { + breakOverlay.ProgressTime(); + return breakOverlay.IsBreakTime.Value == shouldBeBreak; + }); } private class TestBreakOverlay : BreakOverlay { private readonly FramedClock framedManualClock; private readonly ManualClock manualClock; - private IFrameBasedClock normalClock; + private IFrameBasedClock originalClock; public double ManualClockTime { @@ -150,29 +153,25 @@ namespace osu.Game.Tests.Visual.Gameplay set => manualClock.CurrentTime = value; } - public new IBindable IsBreakTime - { - get - { - // Manually call the update function as it might take up to 2 frames for an automatic update to happen - Update(); - - return base.IsBreakTime; - } - } - public TestBreakOverlay(bool letterboxing) : base(letterboxing) { framedManualClock = new FramedClock(manualClock = new ManualClock()); + ProcessCustomClock = false; } - public void SwitchClock(bool setManual) => Clock = setManual ? framedManualClock : normalClock; + public void ProgressTime() + { + framedManualClock.ProcessFrame(); + Update(); + } + + public void SwitchClock(bool setManual) => Clock = setManual ? framedManualClock : originalClock; protected override void LoadComplete() { base.LoadComplete(); - normalClock = Clock; + originalClock = Clock; } } } From e6e315e07bd952625a01bdd9c1578224d41d3dd0 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Tue, 30 Jul 2019 13:29:41 +0300 Subject: [PATCH 28/33] Expose current break index --- .../Visual/Gameplay/TestSceneBreakOverlay.cs | 2 ++ osu.Game/Screens/Play/BreakOverlay.cs | 17 +++++++++-------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs index ed5861b47d..bb89c85e93 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs @@ -147,6 +147,8 @@ namespace osu.Game.Tests.Visual.Gameplay private readonly ManualClock manualClock; private IFrameBasedClock originalClock; + public new int CurrentBreakIndex => base.CurrentBreakIndex; + public double ManualClockTime { get => manualClock.CurrentTime; diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index e3e4014eb3..93267e5f2a 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -34,7 +34,7 @@ namespace osu.Game.Screens.Play // reset index in case the new breaks list is smaller than last one isBreakTime.Value = false; - currentBreakIndex = 0; + CurrentBreakIndex = 0; initializeBreaks(); } @@ -47,7 +47,8 @@ namespace osu.Game.Screens.Play /// public IBindable IsBreakTime => isBreakTime; - private int currentBreakIndex; + protected int CurrentBreakIndex; + private readonly BindableBool isBreakTime = new BindableBool(); private readonly Container remainingTimeAdjustmentBox; @@ -137,21 +138,21 @@ namespace osu.Game.Screens.Play var time = Clock.CurrentTime; - if (time > breaks[currentBreakIndex].EndTime) + if (time > breaks[CurrentBreakIndex].EndTime) { - while (time > breaks[currentBreakIndex].EndTime && currentBreakIndex < breaks.Count - 1) - currentBreakIndex++; + while (time > breaks[CurrentBreakIndex].EndTime && CurrentBreakIndex < breaks.Count - 1) + CurrentBreakIndex++; } else { - while (time < breaks[currentBreakIndex].StartTime && currentBreakIndex > 0) - currentBreakIndex--; + while (time < breaks[CurrentBreakIndex].StartTime && CurrentBreakIndex > 0) + CurrentBreakIndex--; } // This ensures that IsBreakTime is generally consistent with the overlay's transforms during a break. // If the current break doesn't have effects, IsBreakTime should be false. // We also assume that the overlay's fade out transform is "not break time". - var currentBreak = breaks[currentBreakIndex]; + var currentBreak = breaks[CurrentBreakIndex]; isBreakTime.Value = currentBreak.HasEffect && time >= currentBreak.StartTime && time <= currentBreak.EndTime - fade_duration; } From f2ab259c21aefc7900f983b1cffc087229f0a349 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Tue, 30 Jul 2019 13:30:26 +0300 Subject: [PATCH 29/33] Add an assert if current break index has skipped a break --- osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs index bb89c85e93..879e15c548 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakOverlay.cs @@ -87,7 +87,12 @@ namespace osu.Game.Tests.Visual.Gameplay setClock(true); loadBreaksStep("multiple breaks", testBreaks); - addBreakSeeks(testBreaks.Last(), false); + seekAndAssertBreak("seek to break start", testBreaks[1].StartTime, true); + AddAssert("is skipped to break #2", () => breakOverlay.CurrentBreakIndex == 1); + + seekAndAssertBreak("seek to break middle", testBreaks[1].StartTime + testBreaks[1].Duration / 2, true); + seekAndAssertBreak("seek to break end", testBreaks[1].EndTime, false); + seekAndAssertBreak("seek to break after end", testBreaks[1].EndTime + 500, false); } private void addShowBreakStep(double seconds) From d3657d82cd042e13c1ea848c003b7e094483575b Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Wed, 7 Aug 2019 16:28:16 +0300 Subject: [PATCH 30/33] Simplify final check for break time --- osu.Game/Screens/Play/BreakOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 93267e5f2a..ea9bba9c95 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -153,7 +153,7 @@ namespace osu.Game.Screens.Play // If the current break doesn't have effects, IsBreakTime should be false. // We also assume that the overlay's fade out transform is "not break time". var currentBreak = breaks[CurrentBreakIndex]; - isBreakTime.Value = currentBreak.HasEffect && time >= currentBreak.StartTime && time <= currentBreak.EndTime - fade_duration; + isBreakTime.Value = currentBreak.HasEffect && currentBreak.Contains(time); } private void initializeBreaks() From ba269a49eef92af0c1ab44dcc73950ed7a8156b4 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Wed, 7 Aug 2019 16:59:35 +0300 Subject: [PATCH 31/33] Expose break fade duration and add it in the calculation --- osu.Game/Beatmaps/Timing/BreakPeriod.cs | 4 +++- osu.Game/Screens/Play/BreakOverlay.cs | 22 +++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/osu.Game/Beatmaps/Timing/BreakPeriod.cs b/osu.Game/Beatmaps/Timing/BreakPeriod.cs index 856a5fefd4..5d79c7a86b 100644 --- a/osu.Game/Beatmaps/Timing/BreakPeriod.cs +++ b/osu.Game/Beatmaps/Timing/BreakPeriod.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using osu.Game.Screens.Play; + namespace osu.Game.Beatmaps.Timing { public class BreakPeriod @@ -35,6 +37,6 @@ namespace osu.Game.Beatmaps.Timing /// /// The time to check in milliseconds. /// Whether the time falls within this . - public bool Contains(double time) => time >= StartTime && time <= EndTime; + public bool Contains(double time) => time >= StartTime && time <= EndTime - BreakOverlay.BREAK_FADE_DURATION; } } diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index ea9bba9c95..444b1bd210 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -17,7 +17,11 @@ namespace osu.Game.Screens.Play { public class BreakOverlay : Container { - private const double fade_duration = BreakPeriod.MIN_BREAK_DURATION / 2; + /// + /// The duration of the break overlay fading. + /// + public const double BREAK_FADE_DURATION = BreakPeriod.MIN_BREAK_DURATION / 2; + private const float remaining_time_container_max_size = 0.3f; private const int vertical_margin = 25; @@ -172,25 +176,25 @@ namespace osu.Game.Screens.Play using (BeginAbsoluteSequence(b.StartTime, true)) { - fadeContainer.FadeIn(fade_duration); - breakArrows.Show(fade_duration); + fadeContainer.FadeIn(BREAK_FADE_DURATION); + breakArrows.Show(BREAK_FADE_DURATION); remainingTimeAdjustmentBox - .ResizeWidthTo(remaining_time_container_max_size, fade_duration, Easing.OutQuint) - .Delay(b.Duration - fade_duration) + .ResizeWidthTo(remaining_time_container_max_size, BREAK_FADE_DURATION, Easing.OutQuint) + .Delay(b.Duration - BREAK_FADE_DURATION) .ResizeWidthTo(0); remainingTimeBox - .ResizeWidthTo(0, b.Duration - fade_duration) + .ResizeWidthTo(0, b.Duration - BREAK_FADE_DURATION) .Then() .ResizeWidthTo(1); remainingTimeCounter.CountTo(b.Duration).CountTo(0, b.Duration); - using (BeginDelayedSequence(b.Duration - fade_duration, true)) + using (BeginDelayedSequence(b.Duration - BREAK_FADE_DURATION, true)) { - fadeContainer.FadeOut(fade_duration); - breakArrows.Hide(fade_duration); + fadeContainer.FadeOut(BREAK_FADE_DURATION); + breakArrows.Hide(BREAK_FADE_DURATION); } } } From 40a33b338201d543c27e97f7769e1731d5461352 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 8 Aug 2019 10:41:23 +0900 Subject: [PATCH 32/33] Move IsLoaded check to more correct place --- osu.Game/Screens/Play/BreakOverlay.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 444b1bd210..78690f156f 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -40,7 +40,8 @@ namespace osu.Game.Screens.Play isBreakTime.Value = false; CurrentBreakIndex = 0; - initializeBreaks(); + if (IsLoaded) + initializeBreaks(); } } @@ -162,8 +163,6 @@ namespace osu.Game.Screens.Play private void initializeBreaks() { - if (!IsLoaded) return; // we need a clock. - FinishTransforms(true); Scheduler.CancelDelayedTasks(); From 99f5ca07ce8562e87fa4dae3de9a24c77805cacf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 8 Aug 2019 10:42:54 +0900 Subject: [PATCH 33/33] Remove redundant comment --- osu.Game/Screens/Play/BreakOverlay.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 78690f156f..2d014db210 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -154,9 +154,6 @@ namespace osu.Game.Screens.Play CurrentBreakIndex--; } - // This ensures that IsBreakTime is generally consistent with the overlay's transforms during a break. - // If the current break doesn't have effects, IsBreakTime should be false. - // We also assume that the overlay's fade out transform is "not break time". var currentBreak = breaks[CurrentBreakIndex]; isBreakTime.Value = currentBreak.HasEffect && currentBreak.Contains(time); }