From f7ec135b9b0a1bf357a3b508fea8b746a52a8c97 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Oct 2021 14:39:21 +0900 Subject: [PATCH 1/9] Fix `TestSceneLoungeRoomsContainer` crashing on selecting a room with a password --- .../Multiplayer/TestSceneLoungeRoomsContainer.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs index 99b530c2a2..1bdf3c2750 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs @@ -5,6 +5,7 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Cursor; using osu.Framework.Testing; using osu.Game.Online.Rooms; using osu.Game.Rulesets.Catch; @@ -25,12 +26,18 @@ namespace osu.Game.Tests.Visual.Multiplayer [SetUp] public new void Setup() => Schedule(() => { - Child = container = new RoomsContainer + Child = new PopoverContainer { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, Anchor = Anchor.Centre, Origin = Anchor.Centre, Width = 0.5f, - SelectedRoom = { BindTarget = SelectedRoom } + + Child = container = new RoomsContainer + { + SelectedRoom = { BindTarget = SelectedRoom } + } }; }); From d35fe5493a6638f14795ef033f09dbef7afd1234 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Oct 2021 14:50:19 +0900 Subject: [PATCH 2/9] Change default value of `DrawableLoungeRoom.matchingFilter` so they display by default --- osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs b/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs index fe7c7cc364..e787191d5d 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs @@ -103,7 +103,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge public IEnumerable FilterTerms => new[] { Room.Name.Value }; - private bool matchingFilter; + private bool matchingFilter = true; public bool MatchingFilter { From 473459d1910b1a164b2784f74531b7beb09d264f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Oct 2021 15:04:14 +0900 Subject: [PATCH 3/9] Add layout duration to `PasswordEntryPopover` to make error text look a bit smoother --- osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs b/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs index e787191d5d..79515bf352 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs @@ -200,6 +200,8 @@ namespace osu.Game.Screens.OnlinePlay.Lounge Spacing = new Vector2(5), AutoSizeAxes = Axes.Both, Direction = FillDirection.Vertical, + LayoutDuration = 500, + LayoutEasing = Easing.OutQuint, Children = new Drawable[] { new FillFlowContainer From 8944b1dd78cd5daa4b48eb706ae7f1ca2cadd0e3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Oct 2021 15:04:32 +0900 Subject: [PATCH 4/9] Add basic test coverage of `DrawableLoungeRoom` --- .../TestSceneDrawableLoungeRoom.cs | 72 +++++++++++++++++++ .../OnlinePlay/Lounge/LoungeSubScreen.cs | 2 +- 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs new file mode 100644 index 0000000000..91c65d14a7 --- /dev/null +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs @@ -0,0 +1,72 @@ +// 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.Threading; +using System.Threading.Tasks; +using Moq; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Cursor; +using osu.Framework.Testing; +using osu.Game.Online.Rooms; +using osu.Game.Overlays; +using osu.Game.Screens.OnlinePlay.Lounge; + +namespace osu.Game.Tests.Visual.Multiplayer +{ + public class TestSceneDrawableLoungeRoom : OsuTestScene + { + private readonly Room room = new Room + { + HasPassword = { Value = true } + }; + + [Cached] + protected readonly OverlayColourProvider ColourProvider = new OverlayColourProvider(OverlayColourScheme.Pink); + + [BackgroundDependencyLoader] + private void load() + { + var mockLounge = new Mock(); + mockLounge + .Setup(l => l.Join(It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny>())) + .Callback, Action>((a, b, c, d) => + { + Task.Run(() => + { + Thread.Sleep(500); + Schedule(() => d?.Invoke("Incorrect password")); + }); + }); + + Dependencies.CacheAs(mockLounge.Object); + } + + [SetUpSteps] + public void SetUpSteps() + { + AddStep("create drawable", () => + { + Child = new PopoverContainer + { + RelativeSizeAxes = Axes.Both, + Children = new Drawable[] + { + new DrawableLoungeRoom(room) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + } + } + }; + }); + } + + [Test] + public void TestFocus() + { + } + } +} diff --git a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs index 08bdd0487a..f9183ec74a 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs @@ -290,7 +290,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge popoverContainer.HidePopover(); } - public void Join(Room room, string password, Action onSuccess = null, Action onFailure = null) => Schedule(() => + public virtual void Join(Room room, string password, Action onSuccess = null, Action onFailure = null) => Schedule(() => { if (joiningRoomOperation != null) return; From 32cbf6e54be7f8cdccc0b78eec7a4f3a175e1b61 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Oct 2021 15:20:30 +0900 Subject: [PATCH 5/9] Add test coverage ensuring password popover keeps focus --- .../TestSceneDrawableLoungeRoom.cs | 103 +++++++++++++++++- 1 file changed, 99 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs index 91c65d14a7..6d969b6a83 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Moq; @@ -10,13 +11,15 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Cursor; using osu.Framework.Testing; +using osu.Game.Graphics.UserInterface; using osu.Game.Online.Rooms; using osu.Game.Overlays; using osu.Game.Screens.OnlinePlay.Lounge; +using osuTK.Input; namespace osu.Game.Tests.Visual.Multiplayer { - public class TestSceneDrawableLoungeRoom : OsuTestScene + public class TestSceneDrawableLoungeRoom : OsuManualInputManagerTestScene { private readonly Room room = new Room { @@ -26,6 +29,11 @@ namespace osu.Game.Tests.Visual.Multiplayer [Cached] protected readonly OverlayColourProvider ColourProvider = new OverlayColourProvider(OverlayColourScheme.Pink); + private DrawableLoungeRoom drawableRoom; + private SearchTextBox searchTextBox; + + private readonly ManualResetEventSlim allowResponseCallback = new ManualResetEventSlim(); + [BackgroundDependencyLoader] private void load() { @@ -36,7 +44,8 @@ namespace osu.Game.Tests.Visual.Multiplayer { Task.Run(() => { - Thread.Sleep(500); + allowResponseCallback.Wait(); + allowResponseCallback.Reset(); Schedule(() => d?.Invoke("Incorrect password")); }); }); @@ -54,7 +63,16 @@ namespace osu.Game.Tests.Visual.Multiplayer RelativeSizeAxes = Axes.Both, Children = new Drawable[] { - new DrawableLoungeRoom(room) + searchTextBox = new SearchTextBox() + { + HoldFocus = true, + Anchor = Anchor.TopCentre, + Origin = Anchor.TopCentre, + Margin = new MarginPadding(50), + Width = 500, + Depth = float.MaxValue + }, + drawableRoom = new DrawableLoungeRoom(room) { Anchor = Anchor.Centre, Origin = Anchor.Centre, @@ -65,8 +83,85 @@ namespace osu.Game.Tests.Visual.Multiplayer } [Test] - public void TestFocus() + public void TestFocusViaKeyboardCommit() { + DrawableLoungeRoom.PasswordEntryPopover popover = null; + + AddAssert("search textbox has focus", () => checkFocus(searchTextBox)); + AddStep("click room twice", () => + { + InputManager.MoveMouseTo(drawableRoom); + InputManager.Click(MouseButton.Left); + InputManager.Click(MouseButton.Left); + }); + AddUntilStep("wait for popover", () => (popover = InputManager.ChildrenOfType().SingleOrDefault()) != null); + + AddAssert("textbox has focus", () => checkFocus(popover.ChildrenOfType().Single())); + + AddStep("enter password", () => popover.ChildrenOfType().Single().Text = "password"); + AddStep("commit via enter", () => InputManager.Key(Key.Enter)); + + AddAssert("popover has focus", () => checkFocus(popover)); + + AddStep("attempt another enter", () => InputManager.Key(Key.Enter)); + + AddAssert("popover still has focus", () => checkFocus(popover)); + + AddStep("unblock response", () => allowResponseCallback.Set()); + + AddUntilStep("wait for textbox refocus", () => checkFocus(popover.ChildrenOfType().Single())); + + AddStep("press escape", () => InputManager.Key(Key.Escape)); + AddStep("press escape", () => InputManager.Key(Key.Escape)); + + AddUntilStep("search textbox has focus", () => checkFocus(searchTextBox)); } + + [Test] + public void TestFocusViaMouseCommit() + { + DrawableLoungeRoom.PasswordEntryPopover popover = null; + + AddAssert("search textbox has focus", () => checkFocus(searchTextBox)); + AddStep("click room twice", () => + { + InputManager.MoveMouseTo(drawableRoom); + InputManager.Click(MouseButton.Left); + InputManager.Click(MouseButton.Left); + }); + AddUntilStep("wait for popover", () => (popover = InputManager.ChildrenOfType().SingleOrDefault()) != null); + + AddAssert("textbox has focus", () => checkFocus(popover.ChildrenOfType().Single())); + + AddStep("enter password", () => popover.ChildrenOfType().Single().Text = "password"); + + AddStep("commit via click button", () => + { + var button = popover.ChildrenOfType().Single(); + InputManager.MoveMouseTo(button); + InputManager.Click(MouseButton.Left); + }); + + AddAssert("popover has focus", () => checkFocus(popover)); + + AddStep("attempt another click", () => InputManager.Click(MouseButton.Left)); + + AddAssert("popover still has focus", () => checkFocus(popover)); + + AddStep("unblock response", () => allowResponseCallback.Set()); + + AddUntilStep("wait for textbox refocus", () => checkFocus(popover.ChildrenOfType().Single())); + + AddStep("click away", () => + { + InputManager.MoveMouseTo(searchTextBox); + InputManager.Click(MouseButton.Left); + }); + + AddUntilStep("search textbox has focus", () => checkFocus(searchTextBox)); + } + + private bool checkFocus(Drawable expected) => + InputManager.FocusedDrawable == expected; } } From a589964dc7f50743536b020efe5006900d7f396d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Oct 2021 15:27:51 +0900 Subject: [PATCH 6/9] Centralise join logic --- .../OnlinePlay/Lounge/DrawableLoungeRoom.cs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs b/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs index 79515bf352..64bd61b960 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs @@ -232,7 +232,21 @@ namespace osu.Game.Screens.OnlinePlay.Lounge sampleJoinFail = audio.Samples.Get(@"UI/password-fail"); - joinButton.Action = () => lounge?.Join(room, passwordTextbox.Text, null, joinFailed); + joinButton.Action = performJoin; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + Schedule(() => GetContainingInputManager().ChangeFocus(passwordTextbox)); + passwordTextbox.OnCommit += (_, __) => performJoin(); + } + + private void performJoin() + { + lounge?.Join(room, passwordTextbox.Text, null, joinFailed); + GetContainingInputManager().TriggerFocusContention(passwordTextbox); } private void joinFailed(string error) @@ -252,14 +266,6 @@ namespace osu.Game.Screens.OnlinePlay.Lounge sampleJoinFail?.Play(); } - - protected override void LoadComplete() - { - base.LoadComplete(); - - Schedule(() => GetContainingInputManager().ChangeFocus(passwordTextbox)); - passwordTextbox.OnCommit += (_, __) => lounge?.Join(room, passwordTextbox.Text, null, joinFailed); - } } } } From da524261c076ca844d1f28e611de97d7ffd16c03 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Oct 2021 15:36:15 +0900 Subject: [PATCH 7/9] Fix focus potentially being transferred too far up when password is being verified --- osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs b/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs index 64bd61b960..2a68b1d9b3 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs @@ -181,6 +181,10 @@ namespace osu.Game.Screens.OnlinePlay.Lounge [Resolved(canBeNull: true)] private LoungeSubScreen lounge { get; set; } + public override bool HandleNonPositionalInput => true; + + protected override bool BlockNonPositionalInput => true; + public PasswordEntryPopover(Room room) { this.room = room; From 480dc571025924d4b4bf5fadb776d8654eb3abbc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Oct 2021 15:54:10 +0900 Subject: [PATCH 8/9] Schedule `joinFailed` callback for added safety --- osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs b/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs index 2a68b1d9b3..72574b729a 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/DrawableLoungeRoom.cs @@ -253,7 +253,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge GetContainingInputManager().TriggerFocusContention(passwordTextbox); } - private void joinFailed(string error) + private void joinFailed(string error) => Schedule(() => { passwordTextbox.Text = string.Empty; @@ -269,7 +269,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge Body.Shake(); sampleJoinFail?.Play(); - } + }); } } } From fd01a226db28023a06b578d37b1f3bdd7962b23a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Oct 2021 18:11:33 +0900 Subject: [PATCH 9/9] Remove redundant parenthesis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- .../Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs index 6d969b6a83..ea895a23d2 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs @@ -63,7 +63,7 @@ namespace osu.Game.Tests.Visual.Multiplayer RelativeSizeAxes = Axes.Both, Children = new Drawable[] { - searchTextBox = new SearchTextBox() + searchTextBox = new SearchTextBox { HoldFocus = true, Anchor = Anchor.TopCentre,