From c51a2e169d374b1cb820e2975def1cb2bbd902f7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Mar 2024 12:19:06 +0800 Subject: [PATCH 1/3] Add test coverage of crash scenario --- osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs b/osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs index e603f72bb8..24c9d1294f 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs @@ -74,6 +74,10 @@ namespace osu.Game.Tests.Visual.Menus }); AddStep("enter code", () => loginOverlay.ChildrenOfType().First().Text = "88800088"); assertAPIState(APIState.Online); + + AddStep("set failing", () => { dummyAPI.SetState(APIState.Failing); }); + AddStep("return to online", () => { dummyAPI.SetState(APIState.Online); }); + AddStep("clear handler", () => dummyAPI.HandleRequest = null); } From fef8afb833b7ddfd42b97260faff7593da514868 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Mar 2024 12:03:05 +0800 Subject: [PATCH 2/3] Fix double binding causing game crash after API enters failing state See https://sentry.ppy.sh/organizations/ppy/issues/33406/?alert_rule_id=4&alert_timestamp=1711655107332&alert_type=email&environment=production&project=2&referrer=alert_email --- osu.Game/Overlays/Login/LoginPanel.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/osu.Game/Overlays/Login/LoginPanel.cs b/osu.Game/Overlays/Login/LoginPanel.cs index 25bf612bc3..d5c7ed29b8 100644 --- a/osu.Game/Overlays/Login/LoginPanel.cs +++ b/osu.Game/Overlays/Login/LoginPanel.cs @@ -30,7 +30,7 @@ namespace osu.Game.Overlays.Login [Resolved] private OsuColour colours { get; set; } = null!; - private UserDropdown dropdown = null!; + private UserDropdown? dropdown; /// /// Called to request a hide of a parent displaying this container. @@ -68,6 +68,14 @@ namespace osu.Game.Overlays.Login apiState.BindValueChanged(onlineStateChanged, true); } + protected override void LoadComplete() + { + base.LoadComplete(); + + userStatus.BindTo(api.LocalUser.Value.Status); + userStatus.BindValueChanged(e => updateDropdownCurrent(e.NewValue), true); + } + private void onlineStateChanged(ValueChangedEvent state) => Schedule(() => { form = null; @@ -144,9 +152,6 @@ namespace osu.Game.Overlays.Login }, }; - userStatus.BindTo(api.LocalUser.Value.Status); - userStatus.BindValueChanged(e => updateDropdownCurrent(e.NewValue), true); - dropdown.Current.BindValueChanged(action => { switch (action.NewValue) @@ -171,6 +176,7 @@ namespace osu.Game.Overlays.Login break; } }, true); + break; } @@ -180,6 +186,9 @@ namespace osu.Game.Overlays.Login private void updateDropdownCurrent(UserStatus? status) { + if (dropdown == null) + return; + switch (status) { case UserStatus.Online: From d9cf5b5440ff3146e94d1c5da5c3c7c11bd53dd8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Mar 2024 15:44:01 +0800 Subject: [PATCH 3/3] Fix bindable not being correctly re-bound across local user changes --- .../Visual/Menus/TestSceneLoginOverlay.cs | 12 +++++++++ osu.Game/Overlays/Login/LoginPanel.cs | 25 +++++++++++-------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs b/osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs index 24c9d1294f..460d7814e0 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs @@ -15,6 +15,7 @@ using osu.Game.Online.API.Requests; using osu.Game.Overlays; using osu.Game.Overlays.Login; using osu.Game.Overlays.Settings; +using osu.Game.Users; using osu.Game.Users.Drawables; using osuTK.Input; @@ -72,13 +73,24 @@ namespace osu.Game.Tests.Visual.Menus return false; }); + AddStep("enter code", () => loginOverlay.ChildrenOfType().First().Text = "88800088"); assertAPIState(APIState.Online); + assertDropdownState(UserAction.Online); AddStep("set failing", () => { dummyAPI.SetState(APIState.Failing); }); AddStep("return to online", () => { dummyAPI.SetState(APIState.Online); }); AddStep("clear handler", () => dummyAPI.HandleRequest = null); + + assertDropdownState(UserAction.Online); + AddStep("change user state", () => dummyAPI.LocalUser.Value.Status.Value = UserStatus.DoNotDisturb); + assertDropdownState(UserAction.DoNotDisturb); + } + + private void assertDropdownState(UserAction state) + { + AddAssert($"dropdown state is {state}", () => loginOverlay.ChildrenOfType().First().Current.Value, () => Is.EqualTo(state)); } private void assertAPIState(APIState expected) => diff --git a/osu.Game/Overlays/Login/LoginPanel.cs b/osu.Game/Overlays/Login/LoginPanel.cs index d5c7ed29b8..a8adf4ce8c 100644 --- a/osu.Game/Overlays/Login/LoginPanel.cs +++ b/osu.Game/Overlays/Login/LoginPanel.cs @@ -15,6 +15,7 @@ using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; using osu.Game.Localisation; using osu.Game.Online.API; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Overlays.Settings; using osu.Game.Users; using osuTK; @@ -37,8 +38,10 @@ namespace osu.Game.Overlays.Login /// public Action? RequestHide; + private IBindable user = null!; + private readonly Bindable status = new Bindable(); + private readonly IBindable apiState = new Bindable(); - private readonly Bindable userStatus = new Bindable(); [Resolved] private IAPIProvider api { get; set; } = null!; @@ -61,19 +64,21 @@ namespace osu.Game.Overlays.Login AutoSizeAxes = Axes.Y; } - [BackgroundDependencyLoader] - private void load() - { - apiState.BindTo(api.State); - apiState.BindValueChanged(onlineStateChanged, true); - } - protected override void LoadComplete() { base.LoadComplete(); - userStatus.BindTo(api.LocalUser.Value.Status); - userStatus.BindValueChanged(e => updateDropdownCurrent(e.NewValue), true); + apiState.BindTo(api.State); + apiState.BindValueChanged(onlineStateChanged, true); + + user = api.LocalUser.GetBoundCopy(); + user.BindValueChanged(u => + { + status.UnbindBindings(); + status.BindTo(u.NewValue.Status); + }, true); + + status.BindValueChanged(e => updateDropdownCurrent(e.NewValue), true); } private void onlineStateChanged(ValueChangedEvent state) => Schedule(() =>