From 9e6994166c0eef43cb63b6d1d34e70e0cb324634 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Dec 2020 20:59:12 +0100 Subject: [PATCH 01/21] Add helper to track ongoing operations in UI --- .../OnlinePlay/OngoingOperationTracker.cs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs diff --git a/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs b/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs new file mode 100644 index 0000000000..f2d943e14f --- /dev/null +++ b/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs @@ -0,0 +1,47 @@ +// 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 osu.Framework.Bindables; + +namespace osu.Game.Screens.OnlinePlay +{ + /// + /// Utility class to track ongoing online operations' progress. + /// Can be used to disable interactivity while waiting for a response from online sources. + /// + public class OngoingOperationTracker + { + /// + /// Whether there is an online operation in progress. + /// + public IBindable InProgress => inProgress; + + private readonly Bindable inProgress = new BindableBool(); + + private LeasedBindable leasedInProgress; + + /// + /// Begins tracking a new online operation. + /// + /// An operation has already been started. + public void BeginOperation() + { + if (leasedInProgress != null) + throw new InvalidOperationException("Cannot begin operation while another is in progress."); + + leasedInProgress = inProgress.BeginLease(true); + leasedInProgress.Value = true; + } + + /// + /// Ends tracking an online operation. + /// Does nothing if an operation has not been begun yet. + /// + public void EndOperation() + { + leasedInProgress?.Return(); + leasedInProgress = null; + } + } +} From 47ab7c9fd67270079d39e8ad9f45fb3e986c76df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Dec 2020 20:59:38 +0100 Subject: [PATCH 02/21] Disable ready button after host click --- .../TestSceneMultiplayerReadyButton.cs | 22 +++++++++++++++---- .../Match/MultiplayerReadyButton.cs | 13 ++++++++++- .../Multiplayer/MultiplayerMatchSubScreen.cs | 4 ++++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs index 6b11613f1c..958c6d218b 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs @@ -7,11 +7,14 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Graphics; using osu.Framework.Platform; +using osu.Framework.Testing; using osu.Framework.Utils; using osu.Game.Beatmaps; +using osu.Game.Graphics.UserInterface; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; using osu.Game.Rulesets; +using osu.Game.Screens.OnlinePlay; using osu.Game.Screens.OnlinePlay.Multiplayer.Match; using osu.Game.Tests.Resources; using osu.Game.Users; @@ -27,6 +30,9 @@ namespace osu.Game.Tests.Visual.Multiplayer private BeatmapManager beatmaps; private RulesetStore rulesets; + [Cached] + private OngoingOperationTracker gameplayStartTracker = new OngoingOperationTracker(); + [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) { @@ -89,8 +95,7 @@ namespace osu.Game.Tests.Visual.Multiplayer addClickButtonStep(); AddAssert("user is ready", () => Client.Room?.Users[0].State == MultiplayerUserState.Ready); - addClickButtonStep(); - AddAssert("match started", () => Client.Room?.Users[0].State == MultiplayerUserState.WaitingForLoad); + verifyGameplayStartFlow(); } [Test] @@ -105,8 +110,7 @@ namespace osu.Game.Tests.Visual.Multiplayer addClickButtonStep(); AddStep("make user host", () => Client.TransferHost(Client.Room?.Users[0].UserID ?? 0)); - addClickButtonStep(); - AddAssert("match started", () => Client.Room?.Users[0].State == MultiplayerUserState.WaitingForLoad); + verifyGameplayStartFlow(); } [Test] @@ -160,5 +164,15 @@ namespace osu.Game.Tests.Visual.Multiplayer InputManager.MoveMouseTo(button); InputManager.Click(MouseButton.Left); }); + + private void verifyGameplayStartFlow() + { + addClickButtonStep(); + AddAssert("user waiting for load", () => Client.Room?.Users[0].State == MultiplayerUserState.WaitingForLoad); + AddAssert("ready button disabled", () => !button.ChildrenOfType().Single().Enabled.Value); + + AddStep("transitioned to gameplay", () => gameplayStartTracker.EndOperation()); + AddAssert("ready button enabled", () => button.ChildrenOfType().Single().Enabled.Value); + } } } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs index 281e92404c..5009b435f4 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs @@ -33,11 +33,15 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match [Resolved] private OsuColour colours { get; set; } + [Resolved] + private OngoingOperationTracker gameplayStartTracker { get; set; } + private SampleChannel sampleReadyCount; private readonly ButtonWithTrianglesExposed button; private int countReady; + private IBindable gameplayStartInProgress; public MultiplayerReadyButton() { @@ -54,6 +58,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match private void load(AudioManager audio) { sampleReadyCount = audio.Samples.Get(@"SongSelect/select-difficulty"); + + gameplayStartInProgress = gameplayStartTracker.InProgress.GetBoundCopy(); + gameplayStartInProgress.BindValueChanged(_ => updateState()); } protected override void OnRoomUpdated() @@ -63,7 +70,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match // this method is called on leaving the room, so the local user may not exist in the room any more. localUser = Room?.Users.SingleOrDefault(u => u.User?.Id == api.LocalUser.Value.Id); - button.Enabled.Value = Client.Room?.State == MultiplayerRoomState.Open; updateState(); } @@ -100,6 +106,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match break; } + button.Enabled.Value = Client.Room?.State == MultiplayerRoomState.Open && !gameplayStartInProgress.Value; + if (newCountReady != countReady) { countReady = newCountReady; @@ -142,7 +150,10 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match else { if (Room?.Host?.Equals(localUser) == true) + { + gameplayStartTracker.BeginOperation(); Client.StartMatch().CatchUnobservedExceptions(true); + } else Client.ChangeState(MultiplayerUserState.Idle).CatchUnobservedExceptions(true); } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 58314c3774..93db913ce0 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -31,6 +31,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer [Resolved] private StatefulMultiplayerClient client { get; set; } + [Cached] + private OngoingOperationTracker gameplayStartTracker = new OngoingOperationTracker(); + private MultiplayerMatchSettingsOverlay settingsOverlay; private IBindable isConnected; @@ -203,6 +206,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer int[] userIds = client.Room.Users.Where(u => u.State >= MultiplayerUserState.WaitingForLoad).Select(u => u.UserID).ToArray(); StartPlay(() => new MultiplayerPlayer(SelectedItem.Value, userIds)); + gameplayStartTracker.EndOperation(); } protected override void Dispose(bool isDisposing) From af66e4531196853215e8625cf858d4a0d221aaec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Dec 2020 21:39:11 +0100 Subject: [PATCH 03/21] Disable create room button after triggering join --- .../TestSceneCreateMultiplayerMatchButton.cs | 52 +++++++++++++++++++ .../OnlinePlay/Lounge/LoungeSubScreen.cs | 22 ++++---- .../CreateMultiplayerMatchButton.cs | 19 ++++++- .../Screens/OnlinePlay/OnlinePlayScreen.cs | 3 ++ 4 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 osu.Game.Tests/Visual/Multiplayer/TestSceneCreateMultiplayerMatchButton.cs diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneCreateMultiplayerMatchButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneCreateMultiplayerMatchButton.cs new file mode 100644 index 0000000000..2a549e5262 --- /dev/null +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneCreateMultiplayerMatchButton.cs @@ -0,0 +1,52 @@ +// 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.Graphics; +using osu.Game.Screens.OnlinePlay; +using osu.Game.Screens.OnlinePlay.Multiplayer; + +namespace osu.Game.Tests.Visual.Multiplayer +{ + public class TestSceneCreateMultiplayerMatchButton : MultiplayerTestScene + { + [Cached] + private OngoingOperationTracker joiningRoomTracker = new OngoingOperationTracker(); + + private CreateMultiplayerMatchButton button; + + public override void SetUpSteps() + { + base.SetUpSteps(); + AddStep("create button", () => Child = button = new CreateMultiplayerMatchButton + { + Width = 200, + Height = 100, + Anchor = Anchor.Centre, + Origin = Anchor.Centre + }); + } + + [Test] + public void TestButtonEnableStateChanges() + { + assertButtonEnableState(true); + + AddStep("begin joining room", () => joiningRoomTracker.BeginOperation()); + assertButtonEnableState(false); + + AddStep("end joining room", () => joiningRoomTracker.EndOperation()); + assertButtonEnableState(true); + + AddStep("disconnect client", () => Client.Disconnect()); + assertButtonEnableState(false); + + AddStep("re-connect client", () => Client.Connect()); + assertButtonEnableState(true); + } + + private void assertButtonEnableState(bool enabled) + => AddAssert($"button {(enabled ? "enabled" : "disabled")}", () => button.Enabled.Value == enabled); + } +} diff --git a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs index 79f5dfdee1..2730693f73 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs @@ -26,6 +26,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge protected override UserActivity InitialActivity => new UserActivity.SearchingForLobby(); private readonly IBindable initialRoomsReceived = new Bindable(); + private readonly IBindable joiningRoom = new Bindable(); private FilterControl filter; private Container content; @@ -37,7 +38,8 @@ namespace osu.Game.Screens.OnlinePlay.Lounge [Resolved] private MusicController music { get; set; } - private bool joiningRoom; + [Resolved] + private OngoingOperationTracker joiningRoomTracker { get; set; } [BackgroundDependencyLoader] private void load() @@ -98,7 +100,10 @@ namespace osu.Game.Screens.OnlinePlay.Lounge base.LoadComplete(); initialRoomsReceived.BindTo(RoomManager.InitialRoomsReceived); - initialRoomsReceived.BindValueChanged(onInitialRoomsReceivedChanged, true); + initialRoomsReceived.BindValueChanged(_ => updateLoadingLayer()); + + joiningRoom.BindTo(joiningRoomTracker.InProgress); + joiningRoom.BindValueChanged(_ => updateLoadingLayer(), true); } protected override void UpdateAfterChildren() @@ -156,26 +161,21 @@ namespace osu.Game.Screens.OnlinePlay.Lounge private void joinRequested(Room room) { - joiningRoom = true; - updateLoadingLayer(); + joiningRoomTracker.BeginOperation(); RoomManager?.JoinRoom(room, r => { Open(room); - joiningRoom = false; - updateLoadingLayer(); + joiningRoomTracker.EndOperation(); }, _ => { - joiningRoom = false; - updateLoadingLayer(); + joiningRoomTracker.EndOperation(); }); } - private void onInitialRoomsReceivedChanged(ValueChangedEvent received) => updateLoadingLayer(); - private void updateLoadingLayer() { - if (joiningRoom || !initialRoomsReceived.Value) + if (joiningRoom.Value || !initialRoomsReceived.Value) loadingLayer.Show(); else loadingLayer.Hide(); diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs index 163efd9c20..7518a4e71b 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs @@ -10,14 +10,29 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer { public class CreateMultiplayerMatchButton : PurpleTriangleButton { + private IBindable isConnected; + private IBindable joiningRoom; + + [Resolved] + private StatefulMultiplayerClient multiplayerClient { get; set; } + + [Resolved] + private OngoingOperationTracker joiningRoomTracker { get; set; } + [BackgroundDependencyLoader] - private void load(StatefulMultiplayerClient multiplayerClient) + private void load() { Triangles.TriangleScale = 1.5f; Text = "Create room"; - ((IBindable)Enabled).BindTo(multiplayerClient.IsConnected); + isConnected = multiplayerClient.IsConnected.GetBoundCopy(); + isConnected.BindValueChanged(_ => updateState()); + + joiningRoom = joiningRoomTracker.InProgress.GetBoundCopy(); + joiningRoom.BindValueChanged(_ => updateState(), true); } + + private void updateState() => Enabled.Value = isConnected.Value && !joiningRoom.Value; } } diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs index 4074dd1573..92665c498b 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs @@ -52,6 +52,9 @@ namespace osu.Game.Screens.OnlinePlay [Cached] private readonly Bindable currentFilter = new Bindable(new FilterCriteria()); + [Cached] + private readonly OngoingOperationTracker joiningRoomTracker = new OngoingOperationTracker(); + [Resolved(CanBeNull = true)] private MusicController music { get; set; } From 6dc0f6af50a3befd707a7f9d4a5fe10cd0862413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Dec 2020 21:44:58 +0100 Subject: [PATCH 04/21] Disable setting apply button for duration of operation --- .../Match/MultiplayerMatchSettingsOverlay.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs index ae03d384f6..de25cff546 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs @@ -68,6 +68,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match [Resolved] private Bindable ruleset { get; set; } + private readonly OngoingOperationTracker applyingSettingsTracker = new OngoingOperationTracker(); + [BackgroundDependencyLoader] private void load(OsuColour colours) { @@ -274,13 +276,21 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match Type.BindValueChanged(type => TypePicker.Current.Value = type.NewValue, true); MaxParticipants.BindValueChanged(count => MaxParticipantsField.Text = count.NewValue?.ToString(), true); RoomID.BindValueChanged(roomId => initialBeatmapControl.Alpha = roomId.NewValue == null ? 1 : 0, true); + + applyingSettingsTracker.InProgress.BindValueChanged(v => + { + if (v.NewValue) + loadingLayer.Show(); + else + loadingLayer.Hide(); + }); } protected override void Update() { base.Update(); - ApplyButton.Enabled.Value = Playlist.Count > 0 && NameField.Text.Length > 0; + ApplyButton.Enabled.Value = Playlist.Count > 0 && NameField.Text.Length > 0 && !applyingSettingsTracker.InProgress.Value; } private void apply() @@ -289,7 +299,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match return; hideError(); - loadingLayer.Show(); + applyingSettingsTracker.BeginOperation(); // If the client is already in a room, update via the client. // Otherwise, update the room directly in preparation for it to be submitted to the API on match creation. @@ -322,7 +332,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match private void onSuccess(Room room) { - loadingLayer.Hide(); + applyingSettingsTracker.EndOperation(); SettingsApplied?.Invoke(); } @@ -330,8 +340,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match { ErrorText.Text = text; ErrorText.FadeIn(50); - - loadingLayer.Hide(); + applyingSettingsTracker.EndOperation(); } } From 540dec2e7ccbb96342683545d744ffa74b4011d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Dec 2020 22:54:52 +0100 Subject: [PATCH 05/21] Allow null tracker in lounge screen for tests --- .../Screens/OnlinePlay/Lounge/LoungeSubScreen.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs index 2730693f73..f25835f56a 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs @@ -38,7 +38,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge [Resolved] private MusicController music { get; set; } - [Resolved] + [Resolved(CanBeNull = true)] private OngoingOperationTracker joiningRoomTracker { get; set; } [BackgroundDependencyLoader] @@ -102,8 +102,11 @@ namespace osu.Game.Screens.OnlinePlay.Lounge initialRoomsReceived.BindTo(RoomManager.InitialRoomsReceived); initialRoomsReceived.BindValueChanged(_ => updateLoadingLayer()); - joiningRoom.BindTo(joiningRoomTracker.InProgress); - joiningRoom.BindValueChanged(_ => updateLoadingLayer(), true); + if (joiningRoomTracker != null) + { + joiningRoom.BindTo(joiningRoomTracker.InProgress); + joiningRoom.BindValueChanged(_ => updateLoadingLayer(), true); + } } protected override void UpdateAfterChildren() @@ -161,15 +164,15 @@ namespace osu.Game.Screens.OnlinePlay.Lounge private void joinRequested(Room room) { - joiningRoomTracker.BeginOperation(); + joiningRoomTracker?.BeginOperation(); RoomManager?.JoinRoom(room, r => { Open(room); - joiningRoomTracker.EndOperation(); + joiningRoomTracker?.EndOperation(); }, _ => { - joiningRoomTracker.EndOperation(); + joiningRoomTracker?.EndOperation(); }); } From 903dca875e7aa364277dc336e52c9c350a6f363a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 29 Dec 2020 07:46:22 +0100 Subject: [PATCH 06/21] Make localUser a client property --- .../Online/Multiplayer/StatefulMultiplayerClient.cs | 5 +++++ .../Multiplayer/Match/MultiplayerReadyButton.cs | 10 ++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 39d119b2a4..0f44085134 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -65,6 +65,11 @@ namespace osu.Game.Online.Multiplayer /// public readonly BindableList CurrentMatchPlayingUserIds = new BindableList(); + /// + /// The corresponding to the local player, if available. + /// + public MultiplayerRoomUser? LocalUser => Room?.Users.SingleOrDefault(u => u.User?.Id == api.LocalUser.Value.Id); + [Resolved] private UserLookupCache userLookupCache { get; set; } = null!; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs index 5009b435f4..9b406ff52c 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs @@ -27,9 +27,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match [Resolved] private IAPIProvider api { get; set; } - [CanBeNull] - private MultiplayerRoomUser localUser; - [Resolved] private OsuColour colours { get; set; } @@ -67,14 +64,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match { base.OnRoomUpdated(); - // this method is called on leaving the room, so the local user may not exist in the room any more. - localUser = Room?.Users.SingleOrDefault(u => u.User?.Id == api.LocalUser.Value.Id); - updateState(); } private void updateState() { + var localUser = Client.LocalUser; + if (localUser == null) return; @@ -142,6 +138,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match private void onClick() { + var localUser = Client.LocalUser; + if (localUser == null) return; From 9ff214023297f4300a439e5c2427281a0cb37bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 29 Dec 2020 07:51:46 +0100 Subject: [PATCH 07/21] Move ready-up logic to match sub-screen --- .../Match/MultiplayerMatchFooter.cs | 9 +++++- .../Match/MultiplayerReadyButton.cs | 30 ++++--------------- .../Multiplayer/MultiplayerMatchSubScreen.cs | 28 ++++++++++++++++- 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs index a52f62fe00..bd1ca1377e 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.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 System; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; @@ -19,7 +20,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match public readonly Bindable SelectedItem = new Bindable(); + public Action OnReady + { + set => readyButton.OnReady = value; + } + private readonly Drawable background; + private readonly MultiplayerReadyButton readyButton; public MultiplayerMatchFooter() { @@ -29,7 +36,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match InternalChildren = new[] { background = new Box { RelativeSizeAxes = Axes.Both }, - new MultiplayerReadyButton + readyButton = new MultiplayerReadyButton { Anchor = Anchor.Centre, Origin = Anchor.Centre, diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs index 9b406ff52c..96470cf070 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs @@ -1,15 +1,14 @@ // 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.Diagnostics; using System.Linq; -using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Game.Extensions; using osu.Game.Graphics; using osu.Game.Graphics.Backgrounds; using osu.Game.Online.API; @@ -24,6 +23,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match { public Bindable SelectedItem => button.SelectedItem; + public Action OnReady + { + set => button.Action = value; + } + [Resolved] private IAPIProvider api { get; set; } @@ -47,7 +51,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match RelativeSizeAxes = Axes.Both, Size = Vector2.One, Enabled = { Value = true }, - Action = onClick }; } @@ -136,27 +139,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match } } - private void onClick() - { - var localUser = Client.LocalUser; - - if (localUser == null) - return; - - if (localUser.State == MultiplayerUserState.Idle) - Client.ChangeState(MultiplayerUserState.Ready).CatchUnobservedExceptions(true); - else - { - if (Room?.Host?.Equals(localUser) == true) - { - gameplayStartTracker.BeginOperation(); - Client.StartMatch().CatchUnobservedExceptions(true); - } - else - Client.ChangeState(MultiplayerUserState.Idle).CatchUnobservedExceptions(true); - } - } - private class ButtonWithTrianglesExposed : ReadyButton { public new Triangles Triangles => base.Triangles; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 8eafd7c7df..a87471c8af 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -9,6 +9,7 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Screens; +using osu.Game.Extensions; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; using osu.Game.Screens.OnlinePlay.Components; @@ -153,7 +154,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer }, new Drawable[] { - new MultiplayerMatchFooter { SelectedItem = { BindTarget = SelectedItem } } + new MultiplayerMatchFooter + { + SelectedItem = { BindTarget = SelectedItem }, + OnReady = onReady + } } }, RowDimensions = new[] @@ -199,6 +204,27 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer private void onPlaylistChanged(object sender, NotifyCollectionChangedEventArgs e) => SelectedItem.Value = Playlist.FirstOrDefault(); + private void onReady() + { + var localUser = client.LocalUser; + + if (localUser == null) + return; + + if (localUser.State == MultiplayerUserState.Idle) + client.ChangeState(MultiplayerUserState.Ready).CatchUnobservedExceptions(true); + else + { + if (client.Room?.Host?.Equals(localUser) == true) + { + gameplayStartTracker.BeginOperation(); + client.StartMatch().CatchUnobservedExceptions(true); + } + else + client.ChangeState(MultiplayerUserState.Idle).CatchUnobservedExceptions(true); + } + } + private void onLoadRequested() { Debug.Assert(client.Room != null); From f59ba799d30390f030fcac0e12ac99a1893f78f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 29 Dec 2020 07:54:27 +0100 Subject: [PATCH 08/21] Adjust operation tracker implementation --- osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs b/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs index f2d943e14f..6a340d7954 100644 --- a/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs +++ b/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using osu.Framework.Allocation; using osu.Framework.Bindables; namespace osu.Game.Screens.OnlinePlay @@ -24,21 +25,26 @@ namespace osu.Game.Screens.OnlinePlay /// /// Begins tracking a new online operation. /// + /// + /// An that will automatically mark the operation as ended on disposal. + /// /// An operation has already been started. - public void BeginOperation() + public IDisposable BeginOperation() { if (leasedInProgress != null) throw new InvalidOperationException("Cannot begin operation while another is in progress."); leasedInProgress = inProgress.BeginLease(true); leasedInProgress.Value = true; + + return new InvokeOnDisposal(endOperation); } /// /// Ends tracking an online operation. /// Does nothing if an operation has not been begun yet. /// - public void EndOperation() + private void endOperation() { leasedInProgress?.Return(); leasedInProgress = null; From db52255bbec7a284b07baac967280e2cf5b3d9a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 29 Dec 2020 08:20:43 +0100 Subject: [PATCH 09/21] Adjust tracker usages to match new API --- .../TestSceneCreateMultiplayerMatchButton.cs | 9 ++++-- .../TestSceneMultiplayerReadyButton.cs | 9 ++++-- .../OnlinePlay/Lounge/LoungeSubScreen.cs | 21 +++++++++---- .../CreateMultiplayerMatchButton.cs | 10 +++--- .../Match/MultiplayerMatchSettingsOverlay.cs | 31 +++++++++++++++---- .../Match/MultiplayerReadyButton.cs | 10 +++--- .../Multiplayer/MultiplayerMatchSubScreen.cs | 19 +++++++++--- .../Screens/OnlinePlay/OnlinePlayScreen.cs | 2 +- 8 files changed, 79 insertions(+), 32 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneCreateMultiplayerMatchButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneCreateMultiplayerMatchButton.cs index 2a549e5262..381270c5aa 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneCreateMultiplayerMatchButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneCreateMultiplayerMatchButton.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 System; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; @@ -12,7 +13,7 @@ namespace osu.Game.Tests.Visual.Multiplayer public class TestSceneCreateMultiplayerMatchButton : MultiplayerTestScene { [Cached] - private OngoingOperationTracker joiningRoomTracker = new OngoingOperationTracker(); + private OngoingOperationTracker ongoingOperationTracker = new OngoingOperationTracker(); private CreateMultiplayerMatchButton button; @@ -31,12 +32,14 @@ namespace osu.Game.Tests.Visual.Multiplayer [Test] public void TestButtonEnableStateChanges() { + IDisposable joiningRoomOperation = null; + assertButtonEnableState(true); - AddStep("begin joining room", () => joiningRoomTracker.BeginOperation()); + AddStep("begin joining room", () => joiningRoomOperation = ongoingOperationTracker.BeginOperation()); assertButtonEnableState(false); - AddStep("end joining room", () => joiningRoomTracker.EndOperation()); + AddStep("end joining room", () => joiningRoomOperation.Dispose()); assertButtonEnableState(true); AddStep("disconnect client", () => Client.Disconnect()); diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs index 958c6d218b..de02c6c844 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.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 System; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; @@ -31,7 +32,7 @@ namespace osu.Game.Tests.Visual.Multiplayer private RulesetStore rulesets; [Cached] - private OngoingOperationTracker gameplayStartTracker = new OngoingOperationTracker(); + private OngoingOperationTracker ongoingOperationTracker = new OngoingOperationTracker(); [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) @@ -167,11 +168,15 @@ namespace osu.Game.Tests.Visual.Multiplayer private void verifyGameplayStartFlow() { + IDisposable gameplayStartOperation = null; + + AddStep("hook up tracker", () => button.OnReady = () => gameplayStartOperation = ongoingOperationTracker.BeginOperation()); + addClickButtonStep(); AddAssert("user waiting for load", () => Client.Room?.Users[0].State == MultiplayerUserState.WaitingForLoad); AddAssert("ready button disabled", () => !button.ChildrenOfType().Single().Enabled.Value); - AddStep("transitioned to gameplay", () => gameplayStartTracker.EndOperation()); + AddStep("transitioned to gameplay", () => gameplayStartOperation.Dispose()); AddAssert("ready button enabled", () => button.ChildrenOfType().Single().Enabled.Value); } } diff --git a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs index f25835f56a..2a16a62714 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs @@ -1,7 +1,10 @@ // 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.Diagnostics; using System.Linq; +using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -39,7 +42,10 @@ namespace osu.Game.Screens.OnlinePlay.Lounge private MusicController music { get; set; } [Resolved(CanBeNull = true)] - private OngoingOperationTracker joiningRoomTracker { get; set; } + private OngoingOperationTracker ongoingOperationTracker { get; set; } + + [CanBeNull] + private IDisposable joiningRoomOperation { get; set; } [BackgroundDependencyLoader] private void load() @@ -102,9 +108,9 @@ namespace osu.Game.Screens.OnlinePlay.Lounge initialRoomsReceived.BindTo(RoomManager.InitialRoomsReceived); initialRoomsReceived.BindValueChanged(_ => updateLoadingLayer()); - if (joiningRoomTracker != null) + if (ongoingOperationTracker != null) { - joiningRoom.BindTo(joiningRoomTracker.InProgress); + joiningRoom.BindTo(ongoingOperationTracker.InProgress); joiningRoom.BindValueChanged(_ => updateLoadingLayer(), true); } } @@ -164,15 +170,18 @@ namespace osu.Game.Screens.OnlinePlay.Lounge private void joinRequested(Room room) { - joiningRoomTracker?.BeginOperation(); + Debug.Assert(joiningRoomOperation == null); + joiningRoomOperation = ongoingOperationTracker?.BeginOperation(); RoomManager?.JoinRoom(room, r => { Open(room); - joiningRoomTracker?.EndOperation(); + joiningRoomOperation?.Dispose(); + joiningRoomOperation = null; }, _ => { - joiningRoomTracker?.EndOperation(); + joiningRoomOperation?.Dispose(); + joiningRoomOperation = null; }); } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs index 7518a4e71b..3785dfc29f 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs @@ -11,13 +11,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer public class CreateMultiplayerMatchButton : PurpleTriangleButton { private IBindable isConnected; - private IBindable joiningRoom; + private IBindable operationInProgress; [Resolved] private StatefulMultiplayerClient multiplayerClient { get; set; } [Resolved] - private OngoingOperationTracker joiningRoomTracker { get; set; } + private OngoingOperationTracker ongoingOperationTracker { get; set; } [BackgroundDependencyLoader] private void load() @@ -29,10 +29,10 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer isConnected = multiplayerClient.IsConnected.GetBoundCopy(); isConnected.BindValueChanged(_ => updateState()); - joiningRoom = joiningRoomTracker.InProgress.GetBoundCopy(); - joiningRoom.BindValueChanged(_ => updateState(), true); + operationInProgress = ongoingOperationTracker.InProgress.GetBoundCopy(); + operationInProgress.BindValueChanged(_ => updateState(), true); } - private void updateState() => Enabled.Value = isConnected.Value && !joiningRoom.Value; + private void updateState() => Enabled.Value = isConnected.Value && !operationInProgress.Value; } } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs index de25cff546..59b138123d 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs @@ -2,6 +2,8 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Diagnostics; +using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; @@ -68,7 +70,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match [Resolved] private Bindable ruleset { get; set; } - private readonly OngoingOperationTracker applyingSettingsTracker = new OngoingOperationTracker(); + [Resolved] + private OngoingOperationTracker ongoingOperationTracker { get; set; } + + private readonly IBindable operationInProgress = new BindableBool(); + + [CanBeNull] + private IDisposable applyingSettingsOperation; [BackgroundDependencyLoader] private void load(OsuColour colours) @@ -277,7 +285,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match MaxParticipants.BindValueChanged(count => MaxParticipantsField.Text = count.NewValue?.ToString(), true); RoomID.BindValueChanged(roomId => initialBeatmapControl.Alpha = roomId.NewValue == null ? 1 : 0, true); - applyingSettingsTracker.InProgress.BindValueChanged(v => + operationInProgress.BindTo(ongoingOperationTracker.InProgress); + operationInProgress.BindValueChanged(v => { if (v.NewValue) loadingLayer.Show(); @@ -290,7 +299,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match { base.Update(); - ApplyButton.Enabled.Value = Playlist.Count > 0 && NameField.Text.Length > 0 && !applyingSettingsTracker.InProgress.Value; + ApplyButton.Enabled.Value = Playlist.Count > 0 && NameField.Text.Length > 0 && !operationInProgress.Value; } private void apply() @@ -299,7 +308,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match return; hideError(); - applyingSettingsTracker.BeginOperation(); + + Debug.Assert(applyingSettingsOperation == null); + applyingSettingsOperation = ongoingOperationTracker.BeginOperation(); // If the client is already in a room, update via the client. // Otherwise, update the room directly in preparation for it to be submitted to the API on match creation. @@ -332,15 +343,23 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match private void onSuccess(Room room) { - applyingSettingsTracker.EndOperation(); + Debug.Assert(applyingSettingsOperation != null); + SettingsApplied?.Invoke(); + + applyingSettingsOperation.Dispose(); + applyingSettingsOperation = null; } private void onError(string text) { + Debug.Assert(applyingSettingsOperation != null); + ErrorText.Text = text; ErrorText.FadeIn(50); - applyingSettingsTracker.EndOperation(); + + applyingSettingsOperation.Dispose(); + applyingSettingsOperation = null; } } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs index 96470cf070..b145cd46c1 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs @@ -35,14 +35,14 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match private OsuColour colours { get; set; } [Resolved] - private OngoingOperationTracker gameplayStartTracker { get; set; } + private OngoingOperationTracker ongoingOperationTracker { get; set; } private SampleChannel sampleReadyCount; private readonly ButtonWithTrianglesExposed button; private int countReady; - private IBindable gameplayStartInProgress; + private IBindable operationInProgress; public MultiplayerReadyButton() { @@ -59,8 +59,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match { sampleReadyCount = audio.Samples.Get(@"SongSelect/select-difficulty"); - gameplayStartInProgress = gameplayStartTracker.InProgress.GetBoundCopy(); - gameplayStartInProgress.BindValueChanged(_ => updateState()); + operationInProgress = ongoingOperationTracker.InProgress.GetBoundCopy(); + operationInProgress.BindValueChanged(_ => updateState()); } protected override void OnRoomUpdated() @@ -105,7 +105,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match break; } - button.Enabled.Value = Client.Room?.State == MultiplayerRoomState.Open && !gameplayStartInProgress.Value; + button.Enabled.Value = Client.Room?.State == MultiplayerRoomState.Open && !operationInProgress.Value; if (newCountReady != countReady) { diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index a87471c8af..b82bf508ce 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -1,9 +1,11 @@ // 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.Collections.Specialized; using System.Diagnostics; using System.Linq; +using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -32,13 +34,16 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer [Resolved] private StatefulMultiplayerClient client { get; set; } - [Cached] - private OngoingOperationTracker gameplayStartTracker = new OngoingOperationTracker(); + [Resolved] + private OngoingOperationTracker ongoingOperationTracker { get; set; } private MultiplayerMatchSettingsOverlay settingsOverlay; private IBindable isConnected; + [CanBeNull] + private IDisposable gameplayStartOperation; + public MultiplayerMatchSubScreen(Room room) { Title = room.RoomID.Value == null ? "New room" : room.Name.Value; @@ -217,7 +222,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer { if (client.Room?.Host?.Equals(localUser) == true) { - gameplayStartTracker.BeginOperation(); + Debug.Assert(gameplayStartOperation == null); + gameplayStartOperation = ongoingOperationTracker.BeginOperation(); + client.StartMatch().CatchUnobservedExceptions(true); } else @@ -232,7 +239,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer int[] userIds = client.CurrentMatchPlayingUserIds.ToArray(); StartPlay(() => new MultiplayerPlayer(SelectedItem.Value, userIds)); - gameplayStartTracker.EndOperation(); + + Debug.Assert(gameplayStartOperation != null); + + gameplayStartOperation.Dispose(); + gameplayStartOperation = null; } protected override void Dispose(bool isDisposing) diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs index 92665c498b..13ebc1912e 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs @@ -53,7 +53,7 @@ namespace osu.Game.Screens.OnlinePlay private readonly Bindable currentFilter = new Bindable(new FilterCriteria()); [Cached] - private readonly OngoingOperationTracker joiningRoomTracker = new OngoingOperationTracker(); + private readonly OngoingOperationTracker ongoingOperationTracker = new OngoingOperationTracker(); [Resolved(CanBeNull = true)] private MusicController music { get; set; } From e9b0652359e9bcffed8c4e84f95a35f3397eef45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 29 Dec 2020 09:09:47 +0100 Subject: [PATCH 10/21] Move ready-up operation logic again to client To salvage ready up button tests. --- .../TestSceneMultiplayerReadyButton.cs | 16 ++++++--- .../Multiplayer/StatefulMultiplayerClient.cs | 33 +++++++++++++++++++ .../Match/MultiplayerMatchFooter.cs | 4 +-- .../Match/MultiplayerReadyButton.cs | 2 +- .../Multiplayer/MultiplayerMatchSubScreen.cs | 33 ++++++++----------- 5 files changed, 61 insertions(+), 27 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs index de02c6c844..86398b205d 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs @@ -34,6 +34,8 @@ namespace osu.Game.Tests.Visual.Multiplayer [Cached] private OngoingOperationTracker ongoingOperationTracker = new OngoingOperationTracker(); + private IDisposable toggleReadyOperation; + [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) { @@ -61,6 +63,14 @@ namespace osu.Game.Tests.Visual.Multiplayer Beatmap = { Value = beatmap }, Ruleset = { Value = beatmap.Ruleset } } + }, + OnToggleReady = async () => + { + toggleReadyOperation = ongoingOperationTracker.BeginOperation(); + + bool gameplayStarted = await Client.ToggleReady(); + if (!gameplayStarted) + toggleReadyOperation.Dispose(); } }; }); @@ -168,15 +178,11 @@ namespace osu.Game.Tests.Visual.Multiplayer private void verifyGameplayStartFlow() { - IDisposable gameplayStartOperation = null; - - AddStep("hook up tracker", () => button.OnReady = () => gameplayStartOperation = ongoingOperationTracker.BeginOperation()); - addClickButtonStep(); AddAssert("user waiting for load", () => Client.Room?.Users[0].State == MultiplayerUserState.WaitingForLoad); AddAssert("ready button disabled", () => !button.ChildrenOfType().Single().Enabled.Value); - AddStep("transitioned to gameplay", () => gameplayStartOperation.Dispose()); + AddStep("transitioned to gameplay", () => toggleReadyOperation.Dispose()); AddAssert("ready button enabled", () => button.ChildrenOfType().Single().Enabled.Value); } } diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 0f44085134..ee42e56d75 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -183,6 +183,39 @@ namespace osu.Game.Online.Multiplayer }); } + /// + /// Toggles the 's ready state. + /// + /// true if this toggle triggered a gameplay start; false otherwise. + /// If a toggle of ready state is not valid at this time. + public async Task ToggleReady() + { + var localUser = LocalUser; + + if (localUser == null) + return false; + + switch (localUser.State) + { + case MultiplayerUserState.Idle: + await ChangeState(MultiplayerUserState.Ready); + return false; + + case MultiplayerUserState.Ready: + if (Room?.Host?.Equals(localUser) == true) + { + await StartMatch(); + return true; + } + + await ChangeState(MultiplayerUserState.Idle); + return false; + + default: + throw new InvalidOperationException($"Cannot toggle ready when in {localUser.State}"); + } + } + public abstract Task TransferHost(int userId); public abstract Task ChangeSettings(MultiplayerRoomSettings settings); diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs index bd1ca1377e..a5e7650e31 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs @@ -20,9 +20,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match public readonly Bindable SelectedItem = new Bindable(); - public Action OnReady + public Action OnToggleReady { - set => readyButton.OnReady = value; + set => readyButton.OnToggleReady = value; } private readonly Drawable background; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs index b145cd46c1..251b5b30ae 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs @@ -23,7 +23,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match { public Bindable SelectedItem => button.SelectedItem; - public Action OnReady + public Action OnToggleReady { set => button.Action = value; } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index b82bf508ce..88b4c8a1bf 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -162,7 +162,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer new MultiplayerMatchFooter { SelectedItem = { BindTarget = SelectedItem }, - OnReady = onReady + OnToggleReady = onToggleReady } } }, @@ -209,27 +209,22 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer private void onPlaylistChanged(object sender, NotifyCollectionChangedEventArgs e) => SelectedItem.Value = Playlist.FirstOrDefault(); - private void onReady() + private void onToggleReady() { - var localUser = client.LocalUser; + Debug.Assert(gameplayStartOperation == null); + gameplayStartOperation = ongoingOperationTracker.BeginOperation(); - if (localUser == null) - return; + client.ToggleReady() + .ContinueWith(t => + { + // if gameplay was started, the button will be unblocked on load requested. + if (t.Result) return; - if (localUser.State == MultiplayerUserState.Idle) - client.ChangeState(MultiplayerUserState.Ready).CatchUnobservedExceptions(true); - else - { - if (client.Room?.Host?.Equals(localUser) == true) - { - Debug.Assert(gameplayStartOperation == null); - gameplayStartOperation = ongoingOperationTracker.BeginOperation(); - - client.StartMatch().CatchUnobservedExceptions(true); - } - else - client.ChangeState(MultiplayerUserState.Idle).CatchUnobservedExceptions(true); - } + // gameplay was not started; unblock button. + gameplayStartOperation?.Dispose(); + gameplayStartOperation = null; + }) + .CatchUnobservedExceptions(); } private void onLoadRequested() From 274730de34336b646ca57027c6820922cd9fc0ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 29 Dec 2020 09:15:33 +0100 Subject: [PATCH 11/21] Cache tracker in test scene to resolve test fails --- .../Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs index 8869718fd1..2344ebea0e 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSubScreen.cs @@ -3,10 +3,12 @@ using System.Linq; using NUnit.Framework; +using osu.Framework.Allocation; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Online.Rooms; using osu.Game.Rulesets.Osu; +using osu.Game.Screens.OnlinePlay; using osu.Game.Screens.OnlinePlay.Multiplayer; using osu.Game.Screens.OnlinePlay.Multiplayer.Match; using osu.Game.Tests.Beatmaps; @@ -18,6 +20,9 @@ namespace osu.Game.Tests.Visual.Multiplayer { private MultiplayerMatchSubScreen screen; + [Cached] + private OngoingOperationTracker ongoingOperationTracker = new OngoingOperationTracker(); + public TestSceneMultiplayerMatchSubScreen() : base(false) { From cafa241ef3e7d73952956fda2ec83b04ef519da4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 29 Dec 2020 09:41:32 +0100 Subject: [PATCH 12/21] Fix ready-up button getting stuck if server operation fails --- .../OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 88b4c8a1bf..5676bb22b4 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -11,7 +11,6 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Screens; -using osu.Game.Extensions; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; using osu.Game.Screens.OnlinePlay.Components; @@ -218,13 +217,14 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer .ContinueWith(t => { // if gameplay was started, the button will be unblocked on load requested. - if (t.Result) return; + // accessing Exception here also silences any potential errors from the antecedent task + // (we still want to unblock the button if the ready-up fails). + if (t.Exception == null && t.Result) return; // gameplay was not started; unblock button. gameplayStartOperation?.Dispose(); gameplayStartOperation = null; - }) - .CatchUnobservedExceptions(); + }); } private void onLoadRequested() From 59f2017a13672be09b527873470d528717d4ae70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Dec 2020 16:22:11 +0100 Subject: [PATCH 13/21] Move BindValueChanged subscriptions to LoadComplete --- .../Multiplayer/CreateMultiplayerMatchButton.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs index 3785dfc29f..87b0e49b5b 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs @@ -27,9 +27,14 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer Text = "Create room"; isConnected = multiplayerClient.IsConnected.GetBoundCopy(); - isConnected.BindValueChanged(_ => updateState()); - operationInProgress = ongoingOperationTracker.InProgress.GetBoundCopy(); + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + isConnected.BindValueChanged(_ => updateState()); operationInProgress.BindValueChanged(_ => updateState(), true); } From dd87478690096232e4833dc69116006b1bf1e57e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Dec 2020 16:29:19 +0100 Subject: [PATCH 14/21] Add helper IsHost property to Client --- .../Online/Multiplayer/StatefulMultiplayerClient.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index ee42e56d75..290765dc35 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -70,6 +70,18 @@ namespace osu.Game.Online.Multiplayer /// public MultiplayerRoomUser? LocalUser => Room?.Users.SingleOrDefault(u => u.User?.Id == api.LocalUser.Value.Id); + /// + /// Whether the is the host in . + /// + public bool IsHost + { + get + { + var localUser = LocalUser; + return localUser != null && Room?.Host != null && localUser.Equals(Room.Host); + } + } + [Resolved] private UserLookupCache userLookupCache { get; set; } = null!; From d34609b98ed925ccb181f2fc6d47101e3c1623a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Dec 2020 16:29:36 +0100 Subject: [PATCH 15/21] Rename On{ToggleReady -> ReadyClick} --- .../Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs | 2 +- .../OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs | 4 ++-- .../OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs | 2 +- .../OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs index 86398b205d..1d82b29a91 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs @@ -64,7 +64,7 @@ namespace osu.Game.Tests.Visual.Multiplayer Ruleset = { Value = beatmap.Ruleset } } }, - OnToggleReady = async () => + OnReadyClick = async () => { toggleReadyOperation = ongoingOperationTracker.BeginOperation(); diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs index a5e7650e31..bbf861fac3 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchFooter.cs @@ -20,9 +20,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match public readonly Bindable SelectedItem = new Bindable(); - public Action OnToggleReady + public Action OnReadyClick { - set => readyButton.OnToggleReady = value; + set => readyButton.OnReadyClick = value; } private readonly Drawable background; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs index 251b5b30ae..c6018a79e9 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs @@ -23,7 +23,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match { public Bindable SelectedItem => button.SelectedItem; - public Action OnToggleReady + public Action OnReadyClick { set => button.Action = value; } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 5676bb22b4..b00ab065fb 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -161,7 +161,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer new MultiplayerMatchFooter { SelectedItem = { BindTarget = SelectedItem }, - OnToggleReady = onToggleReady + OnReadyClick = onToggleReady } } }, From f800448c87f163346a2ce4284ace782646816823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Dec 2020 18:00:57 +0100 Subject: [PATCH 16/21] Move game start logic to a higher level --- .../TestSceneMultiplayerReadyButton.cs | 17 ++++--- .../Multiplayer/StatefulMultiplayerClient.cs | 15 ++---- .../Multiplayer/MultiplayerMatchSubScreen.cs | 51 ++++++++++++------- 3 files changed, 48 insertions(+), 35 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs index 1d82b29a91..a6037dcbf2 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs @@ -34,7 +34,7 @@ namespace osu.Game.Tests.Visual.Multiplayer [Cached] private OngoingOperationTracker ongoingOperationTracker = new OngoingOperationTracker(); - private IDisposable toggleReadyOperation; + private IDisposable readyClickOperation; [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) @@ -66,11 +66,16 @@ namespace osu.Game.Tests.Visual.Multiplayer }, OnReadyClick = async () => { - toggleReadyOperation = ongoingOperationTracker.BeginOperation(); + readyClickOperation = ongoingOperationTracker.BeginOperation(); - bool gameplayStarted = await Client.ToggleReady(); - if (!gameplayStarted) - toggleReadyOperation.Dispose(); + if (Client.IsHost && Client.LocalUser?.State == MultiplayerUserState.Ready) + { + await Client.StartMatch(); + return; + } + + await Client.ToggleReady(); + readyClickOperation.Dispose(); } }; }); @@ -182,7 +187,7 @@ namespace osu.Game.Tests.Visual.Multiplayer AddAssert("user waiting for load", () => Client.Room?.Users[0].State == MultiplayerUserState.WaitingForLoad); AddAssert("ready button disabled", () => !button.ChildrenOfType().Single().Enabled.Value); - AddStep("transitioned to gameplay", () => toggleReadyOperation.Dispose()); + AddStep("transitioned to gameplay", () => readyClickOperation.Dispose()); AddAssert("ready button enabled", () => button.ChildrenOfType().Single().Enabled.Value); } } diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 290765dc35..5c6baa4fea 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -198,30 +198,23 @@ namespace osu.Game.Online.Multiplayer /// /// Toggles the 's ready state. /// - /// true if this toggle triggered a gameplay start; false otherwise. /// If a toggle of ready state is not valid at this time. - public async Task ToggleReady() + public async Task ToggleReady() { var localUser = LocalUser; if (localUser == null) - return false; + return; switch (localUser.State) { case MultiplayerUserState.Idle: await ChangeState(MultiplayerUserState.Ready); - return false; + return; case MultiplayerUserState.Ready: - if (Room?.Host?.Equals(localUser) == true) - { - await StartMatch(); - return true; - } - await ChangeState(MultiplayerUserState.Idle); - return false; + return; default: throw new InvalidOperationException($"Cannot toggle ready when in {localUser.State}"); diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index b00ab065fb..1bef8841f5 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -11,6 +11,7 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Screens; +using osu.Game.Extensions; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; using osu.Game.Screens.OnlinePlay.Components; @@ -41,7 +42,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer private IBindable isConnected; [CanBeNull] - private IDisposable gameplayStartOperation; + private IDisposable readyClickOperation; public MultiplayerMatchSubScreen(Room room) { @@ -161,7 +162,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer new MultiplayerMatchFooter { SelectedItem = { BindTarget = SelectedItem }, - OnReadyClick = onToggleReady + OnReadyClick = onReadyClick } } }, @@ -208,23 +209,37 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer private void onPlaylistChanged(object sender, NotifyCollectionChangedEventArgs e) => SelectedItem.Value = Playlist.FirstOrDefault(); - private void onToggleReady() + private void onReadyClick() { - Debug.Assert(gameplayStartOperation == null); - gameplayStartOperation = ongoingOperationTracker.BeginOperation(); + Debug.Assert(readyClickOperation == null); + readyClickOperation = ongoingOperationTracker.BeginOperation(); + + if (client.IsHost && client.LocalUser?.State == MultiplayerUserState.Ready) + { + client.StartMatch() + .ContinueWith(t => + { + // accessing Exception here silences any potential errors from the antecedent task + if (t.Exception != null) + { + // gameplay was not started due to an exception; unblock button. + endOperation(); + } + + // gameplay is starting, the button will be unblocked on load requested. + }); + return; + } client.ToggleReady() - .ContinueWith(t => - { - // if gameplay was started, the button will be unblocked on load requested. - // accessing Exception here also silences any potential errors from the antecedent task - // (we still want to unblock the button if the ready-up fails). - if (t.Exception == null && t.Result) return; + .ContinueWith(_ => endOperation()) + .CatchUnobservedExceptions(); - // gameplay was not started; unblock button. - gameplayStartOperation?.Dispose(); - gameplayStartOperation = null; - }); + void endOperation() + { + readyClickOperation?.Dispose(); + readyClickOperation = null; + } } private void onLoadRequested() @@ -235,10 +250,10 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer StartPlay(() => new MultiplayerPlayer(SelectedItem.Value, userIds)); - Debug.Assert(gameplayStartOperation != null); + Debug.Assert(readyClickOperation != null); - gameplayStartOperation.Dispose(); - gameplayStartOperation = null; + readyClickOperation.Dispose(); + readyClickOperation = null; } protected override void Dispose(bool isDisposing) From 7d9a61fbc1b7afad59df89d3cf13cea4cce8be8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 31 Dec 2020 11:50:59 +0100 Subject: [PATCH 17/21] Handle unobserved exceptions from ready button properly --- .../OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 1bef8841f5..39323d9db9 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -222,6 +222,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer // accessing Exception here silences any potential errors from the antecedent task if (t.Exception != null) { + t.CatchUnobservedExceptions(true); // will run immediately. // gameplay was not started due to an exception; unblock button. endOperation(); } @@ -232,8 +233,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer } client.ToggleReady() - .ContinueWith(_ => endOperation()) - .CatchUnobservedExceptions(); + .ContinueWith(t => + { + t.CatchUnobservedExceptions(true); // will run immediately. + endOperation(); + }); void endOperation() { From edd328c8fecdb2566c8f68f087f2c8089219bb77 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 8 Jan 2021 17:24:55 +0900 Subject: [PATCH 18/21] Move bindable closer to source class --- .../OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs index c6018a79e9..04030cdbfd 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs @@ -37,12 +37,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match [Resolved] private OngoingOperationTracker ongoingOperationTracker { get; set; } + private IBindable operationInProgress; + private SampleChannel sampleReadyCount; private readonly ButtonWithTrianglesExposed button; private int countReady; - private IBindable operationInProgress; public MultiplayerReadyButton() { From dad5dd36676de0c965c645b2564e496adb334908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 8 Jan 2021 22:17:37 +0100 Subject: [PATCH 19/21] Remove unnecessary permissiveness wrt null --- .../OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs | 3 ++- osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs | 9 ++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs index 39323d9db9..e539b315e4 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs @@ -241,7 +241,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer void endOperation() { - readyClickOperation?.Dispose(); + Debug.Assert(readyClickOperation != null); + readyClickOperation.Dispose(); readyClickOperation = null; } } diff --git a/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs b/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs index 6a340d7954..c34d39136e 100644 --- a/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs +++ b/osu.Game/Screens/OnlinePlay/OngoingOperationTracker.cs @@ -40,13 +40,12 @@ namespace osu.Game.Screens.OnlinePlay return new InvokeOnDisposal(endOperation); } - /// - /// Ends tracking an online operation. - /// Does nothing if an operation has not been begun yet. - /// private void endOperation() { - leasedInProgress?.Return(); + if (leasedInProgress == null) + throw new InvalidOperationException("Cannot end operation multiple times."); + + leasedInProgress.Return(); leasedInProgress = null; } } From c2eeb822b84d007beec6190cda094ca3ea1ba062 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 8 Jan 2021 22:23:38 +0100 Subject: [PATCH 20/21] Rename {joiningRoom -> operationInProgress} --- osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs index 2a16a62714..9b4e78543c 100644 --- a/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Lounge/LoungeSubScreen.cs @@ -29,7 +29,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge protected override UserActivity InitialActivity => new UserActivity.SearchingForLobby(); private readonly IBindable initialRoomsReceived = new Bindable(); - private readonly IBindable joiningRoom = new Bindable(); + private readonly IBindable operationInProgress = new Bindable(); private FilterControl filter; private Container content; @@ -110,8 +110,8 @@ namespace osu.Game.Screens.OnlinePlay.Lounge if (ongoingOperationTracker != null) { - joiningRoom.BindTo(ongoingOperationTracker.InProgress); - joiningRoom.BindValueChanged(_ => updateLoadingLayer(), true); + operationInProgress.BindTo(ongoingOperationTracker.InProgress); + operationInProgress.BindValueChanged(_ => updateLoadingLayer(), true); } } @@ -187,7 +187,7 @@ namespace osu.Game.Screens.OnlinePlay.Lounge private void updateLoadingLayer() { - if (joiningRoom.Value || !initialRoomsReceived.Value) + if (operationInProgress.Value || !initialRoomsReceived.Value) loadingLayer.Show(); else loadingLayer.Hide(); From ff60d652ed6094054125990375dc9edc8ea05313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 8 Jan 2021 22:28:21 +0100 Subject: [PATCH 21/21] Move out test ongoing operation tracker to higher level --- .../Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs | 6 +----- osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs | 3 +++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs index a6037dcbf2..8b9bffcee1 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs @@ -15,7 +15,6 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; using osu.Game.Rulesets; -using osu.Game.Screens.OnlinePlay; using osu.Game.Screens.OnlinePlay.Multiplayer.Match; using osu.Game.Tests.Resources; using osu.Game.Users; @@ -31,9 +30,6 @@ namespace osu.Game.Tests.Visual.Multiplayer private BeatmapManager beatmaps; private RulesetStore rulesets; - [Cached] - private OngoingOperationTracker ongoingOperationTracker = new OngoingOperationTracker(); - private IDisposable readyClickOperation; [BackgroundDependencyLoader] @@ -66,7 +62,7 @@ namespace osu.Game.Tests.Visual.Multiplayer }, OnReadyClick = async () => { - readyClickOperation = ongoingOperationTracker.BeginOperation(); + readyClickOperation = OngoingOperationTracker.BeginOperation(); if (Client.IsHost && Client.LocalUser?.State == MultiplayerUserState.Ready) { diff --git a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs index da0e39d965..75ddb34685 100644 --- a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs +++ b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs @@ -23,6 +23,9 @@ namespace osu.Game.Tests.Visual.Multiplayer [Cached] public Bindable Filter { get; } + [Cached] + public OngoingOperationTracker OngoingOperationTracker { get; } = new OngoingOperationTracker(); + protected override Container Content => content; private readonly TestMultiplayerRoomContainer content;