From 46af4bce32eb176c459514b04aac08a95cf44e35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Mar 2020 19:42:45 +0100 Subject: [PATCH 1/5] Cover regression in autoplay test --- osu.Game.Tests/Visual/Gameplay/TestSceneAutoplay.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneAutoplay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneAutoplay.cs index 8108ce0864..5ee17aeea2 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneAutoplay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneAutoplay.cs @@ -6,6 +6,7 @@ using System.Linq; using osu.Framework.Testing; using osu.Game.Rulesets; using osu.Game.Screens.Play; +using osu.Game.Screens.Play.Break; namespace osu.Game.Tests.Visual.Gameplay { @@ -27,7 +28,8 @@ namespace osu.Game.Tests.Visual.Gameplay AddStep("seek to break time", () => Player.GameplayClockContainer.Seek(Player.ChildrenOfType().First().Breaks.First().StartTime)); AddUntilStep("wait for seek to complete", () => Player.HUDOverlay.Progress.ReferenceClock.CurrentTime >= Player.BreakOverlay.Breaks.First().StartTime); - AddAssert("test keys not counting", () => !Player.HUDOverlay.KeyCounter.IsCounting); + AddAssert("keys not counting", () => !Player.HUDOverlay.KeyCounter.IsCounting); + AddAssert("overlay displays 100% accuracy", () => Player.BreakOverlay.ChildrenOfType().Single().AccuracyDisplay.Current.Value == 1); AddStep("rewind", () => Player.GameplayClockContainer.Seek(-80000)); AddUntilStep("key counter reset", () => Player.HUDOverlay.KeyCounter.Children.All(kc => kc.CountPresses == 0)); } From adc759771ff73e3b3b023b624348cf9655c9f017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Mar 2020 19:47:42 +0100 Subject: [PATCH 2/5] Hook up score processor in player --- osu.Game/Screens/Play/Player.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 118cea324c..8693035103 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -308,7 +308,7 @@ namespace osu.Game.Screens.Play }, }, failAnimation = new FailAnimation(DrawableRuleset) { OnComplete = onFailComplete, }, - BreakOverlay = new BreakOverlay(working.Beatmap.BeatmapInfo.LetterboxInBreaks) + BreakOverlay = new BreakOverlay(working.Beatmap.BeatmapInfo.LetterboxInBreaks, ScoreProcessor) { Clock = DrawableRuleset.FrameStableClock, ProcessCustomClock = false, From 3a3bfe9a5ea14477da9fdc67d42b9f6fe16598e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Mar 2020 21:19:49 +0100 Subject: [PATCH 3/5] Reorder children to fix pause overlay z-order --- osu.Game/Screens/Play/Player.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 8693035103..63ec3b0d2d 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -251,6 +251,12 @@ namespace osu.Game.Screens.Play { target.AddRange(new[] { + BreakOverlay = new BreakOverlay(working.Beatmap.BeatmapInfo.LetterboxInBreaks, ScoreProcessor) + { + Clock = DrawableRuleset.FrameStableClock, + ProcessCustomClock = false, + Breaks = working.Beatmap.Breaks + }, // display the cursor above some HUD elements. DrawableRuleset.Cursor?.CreateProxy() ?? new Container(), DrawableRuleset.ResumeOverlay?.CreateProxy() ?? new Container(), @@ -308,12 +314,6 @@ namespace osu.Game.Screens.Play }, }, failAnimation = new FailAnimation(DrawableRuleset) { OnComplete = onFailComplete, }, - BreakOverlay = new BreakOverlay(working.Beatmap.BeatmapInfo.LetterboxInBreaks, ScoreProcessor) - { - Clock = DrawableRuleset.FrameStableClock, - ProcessCustomClock = false, - Breaks = working.Beatmap.Breaks - }, }); } From 15fb1a099e4d96e725a6c46072fdf6b782bfd529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 28 Mar 2020 00:42:51 +0100 Subject: [PATCH 4/5] Modify assert to avoid false failures In headless tests it was possible for TestInstantLoad() to erroneously fail. There were two scenarios in which LoadingSpinner could be null: 1. If the test runner was quick enough, the assert could end up running even before Loader.OnEntering() had even had a chance to, meaning that the spinner was never even actually assigned to or instantiated at that point in time. 2. Even if Loader.OnEntering() had managed to run, there was also a possibility that the spinner itself wasn't loaded at the point of checking the assertion. As the spinner is accessed through ChildrenOfType(), which only checks InternalChildren and ignores all currently-loading drawables, it would therefore return null. As null != 0, both of these cases would actually fail the test (this is best seen running headless, preferably with a [Repeat] attribute attached). To resolve, allow the spinner to be null at the point of asserting and duplicate the assertion step at the end. This weakens the test, as case (1) should probably be waited for and case (2) could be solved with exposition as protected in the base, but when attempting to wait for the loader itself to be loaded there were also cases where the appropriate until step would take so much time that the spinner would actually become visible in line with the delayed display logic, so this is a best-effort attempt to address both points without radical changes. --- osu.Game.Tests/Visual/Menus/TestSceneLoader.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneLoader.cs b/osu.Game.Tests/Visual/Menus/TestSceneLoader.cs index b3064ba9be..c44363d9ea 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneLoader.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneLoader.cs @@ -36,8 +36,6 @@ namespace osu.Game.Tests.Visual.Menus [Test] public void TestInstantLoad() { - // visual only, very impossible to test this using asserts. - AddStep("load immediately", () => { loader = new TestLoader(); @@ -46,12 +44,17 @@ namespace osu.Game.Tests.Visual.Menus LoadScreen(loader); }); - AddAssert("spinner did not display", () => loader.LoadingSpinner?.Alpha == 0); + spinnerNotPresentOrHidden(); AddUntilStep("loaded", () => loader.ScreenLoaded); AddUntilStep("not current", () => !loader.IsCurrentScreen()); + + spinnerNotPresentOrHidden(); } + private void spinnerNotPresentOrHidden() => + AddAssert("spinner did not display", () => loader.LoadingSpinner == null || loader.LoadingSpinner.Alpha == 0); + [Test] public void TestDelayedLoad() { From a317ef65b8b5d301689e526bec8efd6b453743bb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 28 Mar 2020 12:18:28 +0900 Subject: [PATCH 5/5] Remove default for argument --- osu.Game.Tests/Visual/Gameplay/TestSceneBreakTracker.cs | 2 +- osu.Game/Screens/Play/BreakOverlay.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakTracker.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakTracker.cs index d46b4ea289..ff25e609c1 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBreakTracker.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBreakTracker.cs @@ -43,7 +43,7 @@ namespace osu.Game.Tests.Visual.Gameplay AddRange(new Drawable[] { breakTracker = new TestBreakTracker(), - breakOverlay = new BreakOverlay(true) + breakOverlay = new BreakOverlay(true, null) { ProcessCustomClock = false, } diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs index 89f51315f2..c978f4e96d 100644 --- a/osu.Game/Screens/Play/BreakOverlay.cs +++ b/osu.Game/Screens/Play/BreakOverlay.cs @@ -45,7 +45,7 @@ namespace osu.Game.Screens.Play private readonly RemainingTimeCounter remainingTimeCounter; private readonly BreakArrows breakArrows; - public BreakOverlay(bool letterboxing, ScoreProcessor scoreProcessor = null) + public BreakOverlay(bool letterboxing, ScoreProcessor scoreProcessor) { RelativeSizeAxes = Axes.Both;