Merge pull request #27754 from peppy/multi-binding-fix

Fix double binding causing game crash after API enters failing state
ppy · Mar 29, 2024 · 5c7fdec · 5c7fdec
2 parents 21201e6 + d9cf5b5
commit 5c7fdec
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 7 deletions.
diff --git a/osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs b/osu.Game.Tests/Visual/Menus/TestSceneLoginOverlay.cs
@@ -15,6 +15,7 @@
 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,9 +73,24 @@ public void TestLoginSuccess()
 
                 return false;
             });
+
             AddStep("enter code", () => loginOverlay.ChildrenOfType<OsuTextBox>().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<UserDropdown>().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
@@ -15,6 +15,7 @@
 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;
@@ -30,15 +31,17 @@ public partial class LoginPanel : Container
         [Resolved]
         private OsuColour colours { get; set; } = null!;
 
-        private UserDropdown dropdown = null!;
+        private UserDropdown? dropdown;
 
         /// <summary>
         /// Called to request a hide of a parent displaying this container.
         /// </summary>
         public Action? RequestHide;
 
+        private IBindable<APIUser> user = null!;
+        private readonly Bindable<UserStatus?> status = new Bindable<UserStatus?>();
+
         private readonly IBindable<APIState> apiState = new Bindable<APIState>();
-        private readonly Bindable<UserStatus?> userStatus = new Bindable<UserStatus?>();
 
         [Resolved]
         private IAPIProvider api { get; set; } = null!;
@@ -61,11 +64,21 @@ public LoginPanel()
             AutoSizeAxes = Axes.Y;
         }
 
-        [BackgroundDependencyLoader]
-        private void load()
+        protected override void LoadComplete()
         {
+            base.LoadComplete();
+
             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<APIState> state) => Schedule(() =>
@@ -144,9 +157,6 @@ private void load()
                         },
                     };
 
-                    userStatus.BindTo(api.LocalUser.Value.Status);
-                    userStatus.BindValueChanged(e => updateDropdownCurrent(e.NewValue), true);
-
                     dropdown.Current.BindValueChanged(action =>
                     {
                         switch (action.NewValue)
@@ -171,6 +181,7 @@ private void load()
                                 break;
                         }
                     }, true);
+
                     break;
             }
 
@@ -180,6 +191,9 @@ private void load()
 
         private void updateDropdownCurrent(UserStatus? status)
         {
+            if (dropdown == null)
+                return;
+
             switch (status)
             {
                 case UserStatus.Online: