From 3a6a3a067b4816b81bf1851b75640a8e73349221 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Dec 2020 14:54:21 +0100 Subject: [PATCH 1/2] Rewrite test to cover failure case --- .../Online/TestSceneAccountCreationOverlay.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs index dcfe0432a8..3d65e7e4ba 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.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 NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -13,13 +14,12 @@ namespace osu.Game.Tests.Visual.Online public class TestSceneAccountCreationOverlay : OsuTestScene { private readonly Container userPanelArea; + private readonly AccountCreationOverlay accountCreation; private IBindable localUser; public TestSceneAccountCreationOverlay() { - AccountCreationOverlay accountCreation; - Children = new Drawable[] { accountCreation = new AccountCreationOverlay(), @@ -31,8 +31,6 @@ namespace osu.Game.Tests.Visual.Online Origin = Anchor.TopRight, }, }; - - AddStep("show", () => accountCreation.Show()); } [BackgroundDependencyLoader] @@ -42,8 +40,19 @@ namespace osu.Game.Tests.Visual.Online localUser = API.LocalUser.GetBoundCopy(); localUser.BindValueChanged(user => { userPanelArea.Child = new UserGridPanel(user.NewValue) { Width = 200 }; }, true); + } - AddStep("logout", API.Logout); + [Test] + public void TestOverlayVisibility() + { + AddStep("start hidden", () => accountCreation.Hide()); + AddStep("log out", API.Logout); + + AddStep("show manually", () => accountCreation.Show()); + AddUntilStep("overlay is visible", () => accountCreation.State.Value == Visibility.Visible); + + AddStep("log back in", () => API.Login("dummy", "password")); + AddUntilStep("overlay is hidden", () => accountCreation.State.Value == Visibility.Hidden); } } } From 0d8fb83d0a0a56359947ac8e52ff9e10a953db66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 25 Dec 2020 15:37:14 +0100 Subject: [PATCH 2/2] Ensure account creation overlay is shown after logout Scheduling the entire API state change callback caused the scheduled hide to fire the first time the user attempted to display the account creation overlay after a logout, because the drawable wasn't present before that (so its scheduler wasn't running). It is not theoretically safe to run `Hide()` unscheduled at its present call site (as the value change callbacks are fired on the background API thread). This could also be fixed by setting `AlwaysPresent = true`, but that's a pretty ugly and unperformant change to make in general. --- osu.Game/Overlays/AccountCreationOverlay.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/osu.Game/Overlays/AccountCreationOverlay.cs b/osu.Game/Overlays/AccountCreationOverlay.cs index 58ede5502a..3084c7475a 100644 --- a/osu.Game/Overlays/AccountCreationOverlay.cs +++ b/osu.Game/Overlays/AccountCreationOverlay.cs @@ -9,6 +9,7 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Effects; using osu.Framework.Graphics.Shapes; using osu.Framework.Screens; +using osu.Framework.Threading; using osu.Game.Graphics; using osu.Game.Graphics.Containers; using osu.Game.Online.API; @@ -93,6 +94,11 @@ namespace osu.Game.Overlays if (welcomeScreen.GetChildScreen() != null) welcomeScreen.MakeCurrent(); + + // there might be a stale scheduled hide from a previous API state change. + // cancel it here so that the overlay is not hidden again after one frame. + scheduledHide?.Cancel(); + scheduledHide = null; } protected override void PopOut() @@ -101,7 +107,9 @@ namespace osu.Game.Overlays this.FadeOut(100); } - private void apiStateChanged(ValueChangedEvent state) => Schedule(() => + private ScheduledDelegate scheduledHide; + + private void apiStateChanged(ValueChangedEvent state) { switch (state.NewValue) { @@ -113,9 +121,10 @@ namespace osu.Game.Overlays break; case APIState.Online: - Hide(); + scheduledHide?.Cancel(); + scheduledHide = Schedule(Hide); break; } - }); + } } }