From ce3c2f07dc8f36d6cfea1af94943cc74093833d7 Mon Sep 17 00:00:00 2001 From: vmaggioli Date: Tue, 19 Jan 2021 20:13:21 -0500 Subject: [PATCH 01/30] Fix zero length spinners and sliders --- .../TestSceneSliderSelectionBlueprint.cs | 21 +++++++++++++++++++ .../Sliders/SliderSelectionBlueprint.cs | 5 ++++- .../Timeline/TimelineHitObjectBlueprint.cs | 2 +- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs index f6e1be693b..55b3707c38 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs @@ -161,6 +161,27 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor checkControlPointSelected(1, false); } + [Test] + public void TestZeroLengthSliderNotAllowed() + { + moveMouseToControlPoint(1); + AddStep("drag control point 1 to control point 0", () => + { + InputManager.PressButton(MouseButton.Left); + moveMouseToControlPoint(0); + InputManager.ReleaseButton(MouseButton.Left); + }); + moveMouseToControlPoint(2); + AddStep("drag control point 2 to control point 0", () => + { + InputManager.PressButton(MouseButton.Left); + moveMouseToControlPoint(0); + InputManager.ReleaseButton(MouseButton.Left); + }); + checkPositions(); + + } + private void moveHitObject() { AddStep("move hitobject", () => diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index 3d3dff653a..99edcd2149 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -226,7 +226,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders private void updatePath() { - HitObject.Path.ExpectedDistance.Value = composer?.GetSnappedDistanceFromDistance(HitObject.StartTime, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance; + float expectedDistance = composer?.GetSnappedDistanceFromDistance(HitObject.StartTime, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance; + if (expectedDistance < 1) + return; + HitObject.Path.ExpectedDistance.Value = expectedDistance; editorBeatmap?.Update(HitObject); } diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs index ae2a82fa10..1dc37510ad 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs @@ -387,7 +387,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline case IHasDuration endTimeHitObject: var snappedTime = Math.Max(hitObject.StartTime, beatSnapProvider.SnapTime(time)); - if (endTimeHitObject.EndTime == snappedTime) + if (endTimeHitObject.EndTime == snappedTime || (snappedTime - hitObject.StartTime) < 1) return; endTimeHitObject.Duration = snappedTime - hitObject.StartTime; From d42773ebb2b731e93b03feab519fa951cbc60421 Mon Sep 17 00:00:00 2001 From: vmaggioli Date: Wed, 20 Jan 2021 12:36:31 -0500 Subject: [PATCH 02/30] Fix preceeding space --- .../Editor/TestSceneSliderSelectionBlueprint.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs index 55b3707c38..ce1c13dac5 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs @@ -179,7 +179,6 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor InputManager.ReleaseButton(MouseButton.Left); }); checkPositions(); - } private void moveHitObject() From 5ee3a5f230435a166c85fdb8d542f4dc34be1f91 Mon Sep 17 00:00:00 2001 From: vmaggioli Date: Wed, 20 Jan 2021 13:00:25 -0500 Subject: [PATCH 03/30] Use AlmostEquals --- .../Compose/Components/Timeline/TimelineHitObjectBlueprint.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs index 1dc37510ad..301543b3c1 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs @@ -13,6 +13,7 @@ using osu.Framework.Graphics.Effects; using osu.Framework.Graphics.Primitives; using osu.Framework.Graphics.Shapes; using osu.Framework.Input.Events; +using osu.Framework.Utils; using osu.Game.Graphics; using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; @@ -387,7 +388,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline case IHasDuration endTimeHitObject: var snappedTime = Math.Max(hitObject.StartTime, beatSnapProvider.SnapTime(time)); - if (endTimeHitObject.EndTime == snappedTime || (snappedTime - hitObject.StartTime) < 1) + if (endTimeHitObject.EndTime == snappedTime || Precision.AlmostEquals(snappedTime, hitObject.StartTime, 1)) return; endTimeHitObject.Duration = snappedTime - hitObject.StartTime; From e4b59c7317b7788f03b184ccc900aee717153658 Mon Sep 17 00:00:00 2001 From: vmaggioli Date: Thu, 21 Jan 2021 11:54:26 -0500 Subject: [PATCH 04/30] Test setup --- .../TestSceneTimelineHitObjectBlueprint.cs | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs diff --git a/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs b/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs new file mode 100644 index 0000000000..ed427c2020 --- /dev/null +++ b/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs @@ -0,0 +1,55 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using NUnit.Framework; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Rulesets.Osu.Objects.Drawables; +using osu.Game.Screens.Edit.Compose.Components.Timeline; +using osuTK; + +namespace osu.Game.Tests.Visual.Editing +{ + public class TestSceneTimelineHitObjectBlueprint : TimelineTestScene + { + private Spinner spinner; + private TimelineHitObjectBlueprint blueprint; + + public TestSceneTimelineHitObjectBlueprint() + { + var spinner = new Spinner + { + Position = new Vector2(256, 256), + StartTime = -1000, + EndTime = 2000 + }; + + spinner.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty { CircleSize = 2 }); + Add(new Container + { + RelativeSizeAxes = Axes.Both, + Size = new Vector2(0.5f), + Child = _ = new DrawableSpinner(spinner) + }); + } + + protected override void LoadComplete() + { + base.LoadComplete(); + Clock.Seek(10000); + } + + public override Drawable CreateTestComponent() => blueprint = new TimelineHitObjectBlueprint(spinner); + + [Test] + public void TestDisallowZeroLengthSpinners() + { + + } + } +} From a5f866d95ce0a6e97c8620b22266b5bf3f470173 Mon Sep 17 00:00:00 2001 From: vmaggioli Date: Thu, 21 Jan 2021 15:14:24 -0500 Subject: [PATCH 05/30] Test updates --- .../TestSceneTimelineHitObjectBlueprint.cs | 37 ++++++++----------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs b/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs index ed427c2020..1fa37470cb 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs @@ -1,28 +1,27 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; +using System.Linq; using NUnit.Framework; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Rulesets.Osu.Objects.Drawables; using osu.Game.Screens.Edit.Compose.Components.Timeline; using osuTK; +using osuTK.Input; +using static osu.Game.Screens.Edit.Compose.Components.Timeline.TimelineHitObjectBlueprint; namespace osu.Game.Tests.Visual.Editing { public class TestSceneTimelineHitObjectBlueprint : TimelineTestScene { private Spinner spinner; - private TimelineHitObjectBlueprint blueprint; public TestSceneTimelineHitObjectBlueprint() { - var spinner = new Spinner + spinner = new Spinner { Position = new Vector2(256, 256), StartTime = -1000, @@ -30,26 +29,22 @@ namespace osu.Game.Tests.Visual.Editing }; spinner.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty { CircleSize = 2 }); - Add(new Container - { - RelativeSizeAxes = Axes.Both, - Size = new Vector2(0.5f), - Child = _ = new DrawableSpinner(spinner) - }); } - protected override void LoadComplete() - { - base.LoadComplete(); - Clock.Seek(10000); - } - - public override Drawable CreateTestComponent() => blueprint = new TimelineHitObjectBlueprint(spinner); + public override Drawable CreateTestComponent() => new TimelineHitObjectBlueprint(spinner); [Test] public void TestDisallowZeroLengthSpinners() { - + DragBar dragBar = this.ChildrenOfType().First(); + Circle circle = this.ChildrenOfType().First(); + InputManager.MoveMouseTo(dragBar.ScreenSpaceDrawQuad.TopRight); + AddStep("drag dragbar to hit object", () => + { + InputManager.PressButton(MouseButton.Left); + InputManager.MoveMouseTo(circle.ScreenSpaceDrawQuad.TopLeft); + InputManager.ReleaseButton(MouseButton.Left); + }); } } } From 7046f64e7f7eaec8d9aa65cdac54aca13d01bc06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 21 Jan 2021 22:07:08 +0100 Subject: [PATCH 06/30] Rewrite test scene --- .../TestSceneTimelineHitObjectBlueprint.cs | 54 ++++++++++--------- .../Visual/Editing/TimelineTestScene.cs | 10 ++-- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs b/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs index 1fa37470cb..15fa1d995b 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs @@ -1,13 +1,9 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; using osu.Framework.Testing; -using osu.Game.Beatmaps; -using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Osu.Objects; -using osu.Game.Rulesets.Osu.Objects.Drawables; using osu.Game.Screens.Edit.Compose.Components.Timeline; using osuTK; using osuTK.Input; @@ -17,34 +13,40 @@ namespace osu.Game.Tests.Visual.Editing { public class TestSceneTimelineHitObjectBlueprint : TimelineTestScene { - private Spinner spinner; - - public TestSceneTimelineHitObjectBlueprint() - { - spinner = new Spinner - { - Position = new Vector2(256, 256), - StartTime = -1000, - EndTime = 2000 - }; - - spinner.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty { CircleSize = 2 }); - } - - public override Drawable CreateTestComponent() => new TimelineHitObjectBlueprint(spinner); + public override Drawable CreateTestComponent() => new TimelineBlueprintContainer(Composer); [Test] - public void TestDisallowZeroLengthSpinners() + public void TestDisallowZeroDurationObjects() { - DragBar dragBar = this.ChildrenOfType().First(); - Circle circle = this.ChildrenOfType().First(); - InputManager.MoveMouseTo(dragBar.ScreenSpaceDrawQuad.TopRight); - AddStep("drag dragbar to hit object", () => + DragBar dragBar; + + AddStep("add spinner", () => { + EditorBeatmap.Clear(); + EditorBeatmap.Add(new Spinner + { + Position = new Vector2(256, 256), + StartTime = 150, + Duration = 500 + }); + }); + + AddStep("hold down drag bar", () => + { + // distinguishes between the actual drag bar and its "underlay shadow". + dragBar = this.ChildrenOfType().Single(bar => bar.HandlePositionalInput); + InputManager.MoveMouseTo(dragBar); InputManager.PressButton(MouseButton.Left); - InputManager.MoveMouseTo(circle.ScreenSpaceDrawQuad.TopLeft); + }); + + AddStep("try to drag bar past start", () => + { + var blueprint = this.ChildrenOfType().Single(); + InputManager.MoveMouseTo(blueprint.SelectionQuad.TopLeft - new Vector2(100, 0)); InputManager.ReleaseButton(MouseButton.Left); }); + + AddAssert("object has non-zero duration", () => EditorBeatmap.HitObjects.OfType().Single().Duration > 0); } } } diff --git a/osu.Game.Tests/Visual/Editing/TimelineTestScene.cs b/osu.Game.Tests/Visual/Editing/TimelineTestScene.cs index 63bb018d6e..d6db171cf0 100644 --- a/osu.Game.Tests/Visual/Editing/TimelineTestScene.cs +++ b/osu.Game.Tests/Visual/Editing/TimelineTestScene.cs @@ -23,22 +23,24 @@ namespace osu.Game.Tests.Visual.Editing protected HitObjectComposer Composer { get; private set; } + protected EditorBeatmap EditorBeatmap { get; private set; } + [BackgroundDependencyLoader] private void load(AudioManager audio) { Beatmap.Value = new WaveformTestBeatmap(audio); var playable = Beatmap.Value.GetPlayableBeatmap(Beatmap.Value.BeatmapInfo.Ruleset); - var editorBeatmap = new EditorBeatmap(playable); + EditorBeatmap = new EditorBeatmap(playable); - Dependencies.Cache(editorBeatmap); - Dependencies.CacheAs(editorBeatmap); + Dependencies.Cache(EditorBeatmap); + Dependencies.CacheAs(EditorBeatmap); Composer = playable.BeatmapInfo.Ruleset.CreateInstance().CreateHitObjectComposer().With(d => d.Alpha = 0); AddRange(new Drawable[] { - editorBeatmap, + EditorBeatmap, Composer, new FillFlowContainer { From a71f769cce26eb9a010e2dd67d2ee637f74cd932 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 21 Jan 2021 22:09:42 +0100 Subject: [PATCH 07/30] Add missing license header --- .../Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs b/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs index 15fa1d995b..35f394fe1d 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneTimelineHitObjectBlueprint.cs @@ -1,4 +1,7 @@ -using System.Linq; +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; using NUnit.Framework; using osu.Framework.Graphics; using osu.Framework.Testing; From e4c5e5ba17ae1b7359d26b628fc92a9565f4bcfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 21 Jan 2021 22:11:51 +0100 Subject: [PATCH 08/30] Separate return statement with blank line --- .../Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index 99edcd2149..508783a499 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -229,6 +229,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders float expectedDistance = composer?.GetSnappedDistanceFromDistance(HitObject.StartTime, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance; if (expectedDistance < 1) return; + HitObject.Path.ExpectedDistance.Value = expectedDistance; editorBeatmap?.Update(HitObject); } From d0fd2ae432700dc8a7d125b827e6931ab69940c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 21 Jan 2021 22:20:07 +0100 Subject: [PATCH 09/30] Fix added zero-length slider test not working properly --- .../TestSceneSliderSelectionBlueprint.cs | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs index ce1c13dac5..4edf778bfd 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs @@ -165,20 +165,12 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor public void TestZeroLengthSliderNotAllowed() { moveMouseToControlPoint(1); - AddStep("drag control point 1 to control point 0", () => - { - InputManager.PressButton(MouseButton.Left); - moveMouseToControlPoint(0); - InputManager.ReleaseButton(MouseButton.Left); - }); + dragMouseToControlPoint(0); + moveMouseToControlPoint(2); - AddStep("drag control point 2 to control point 0", () => - { - InputManager.PressButton(MouseButton.Left); - moveMouseToControlPoint(0); - InputManager.ReleaseButton(MouseButton.Left); - }); - checkPositions(); + dragMouseToControlPoint(0); + + AddAssert("slider has non-zero duration", () => slider.Duration > 0); } private void moveHitObject() @@ -209,6 +201,13 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor }); } + private void dragMouseToControlPoint(int index) + { + AddStep("hold down mouse button", () => InputManager.PressButton(MouseButton.Left)); + moveMouseToControlPoint(index); + AddStep("release mouse button", () => InputManager.ReleaseButton(MouseButton.Left)); + } + private void checkControlPointSelected(int index, bool selected) => AddAssert($"control point {index} {(selected ? "selected" : "not selected")}", () => blueprint.ControlPointVisualiser.Pieces[index].IsSelected.Value == selected); From 9f89b4e6d79822f7e7eb382767818add29bda6db Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Jan 2021 14:25:21 +0900 Subject: [PATCH 10/30] Rewrite connection logic to better handle failure cases The main goal here is to ensure the connection is built each connection attempt. Previously, the access token would never be updated, leading to outdated tokens failing repeatedly (in the connection retry loop) and never being able to establish a new connection as a result. Due to threading considerations, this isn't as simple as I would hope it to be. I'm open to proposals as to a better way of handling this. Also, keep in mind that this logic will need to be abstracted and (re)used in `SpectatorClient` as well. I've intentionally not done that yet until we agree that this is a good direction forward. --- .../Online/Multiplayer/MultiplayerClient.cs | 141 ++++++++++++------ 1 file changed, 93 insertions(+), 48 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 50dc8f661c..34616a45a5 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -4,7 +4,6 @@ #nullable enable using System; -using System.Diagnostics; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.SignalR.Client; @@ -30,6 +29,8 @@ namespace osu.Game.Online.Multiplayer private HubConnection? connection; + private CancellationTokenSource connectCancelSource = new CancellationTokenSource(); + private readonly string endpoint; public MultiplayerClient(EndpointConfiguration endpoints) @@ -50,8 +51,7 @@ namespace osu.Game.Online.Multiplayer { case APIState.Failing: case APIState.Offline: - connection?.StopAsync(); - connection = null; + Task.Run(Disconnect); break; case APIState.Online: @@ -60,70 +60,57 @@ namespace osu.Game.Online.Multiplayer } } - protected virtual async Task Connect() + private readonly SemaphoreSlim connectionLock = new SemaphoreSlim(1); + + public Task Disconnect() => disconnect(true); + + protected async Task Connect() { - if (connection != null) - return; + cancelExistingConnect(); - connection = new HubConnectionBuilder() - .WithUrl(endpoint, options => - { - options.Headers.Add("Authorization", $"Bearer {api.AccessToken}"); - }) - .AddNewtonsoftJsonProtocol(options => { options.PayloadSerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; }) - .Build(); + await connectionLock.WaitAsync(); - // this is kind of SILLY - // https://github.com/dotnet/aspnetcore/issues/15198 - connection.On(nameof(IMultiplayerClient.RoomStateChanged), ((IMultiplayerClient)this).RoomStateChanged); - connection.On(nameof(IMultiplayerClient.UserJoined), ((IMultiplayerClient)this).UserJoined); - connection.On(nameof(IMultiplayerClient.UserLeft), ((IMultiplayerClient)this).UserLeft); - connection.On(nameof(IMultiplayerClient.HostChanged), ((IMultiplayerClient)this).HostChanged); - connection.On(nameof(IMultiplayerClient.SettingsChanged), ((IMultiplayerClient)this).SettingsChanged); - connection.On(nameof(IMultiplayerClient.UserStateChanged), ((IMultiplayerClient)this).UserStateChanged); - connection.On(nameof(IMultiplayerClient.LoadRequested), ((IMultiplayerClient)this).LoadRequested); - connection.On(nameof(IMultiplayerClient.MatchStarted), ((IMultiplayerClient)this).MatchStarted); - connection.On(nameof(IMultiplayerClient.ResultsReady), ((IMultiplayerClient)this).ResultsReady); - - connection.Closed += async ex => + try { - isConnected.Value = false; + await disconnect(false); - Logger.Log(ex != null - ? $"Multiplayer client lost connection: {ex}" - : "Multiplayer client disconnected", LoggingTarget.Network); + // this token will be valid for the scope of this connection. + // if cancelled, we can be sure that a disconnect or reconnect is handled elsewhere. + var cancellationToken = connectCancelSource.Token; - if (connection != null) - await tryUntilConnected(); - }; - - await tryUntilConnected(); - - async Task tryUntilConnected() - { - Logger.Log("Multiplayer client connecting...", LoggingTarget.Network); - - while (api.State.Value == APIState.Online) + while (api.State.Value == APIState.Online && !cancellationToken.IsCancellationRequested) { + Logger.Log("Multiplayer client connecting...", LoggingTarget.Network); + try { - Debug.Assert(connection != null); + // importantly, rebuild the connection each attempt to get an updated access token. + connection = createConnection(cancellationToken); + + await connection.StartAsync(cancellationToken); - // reconnect on any failure - await connection.StartAsync(); Logger.Log("Multiplayer client connected!", LoggingTarget.Network); - - // Success. isConnected.Value = true; - break; + return; + } + catch (OperationCanceledException) + { + //connection process was cancelled. + return; } catch (Exception e) { Logger.Log($"Multiplayer client connection error: {e}", LoggingTarget.Network); - await Task.Delay(5000); + + // retry on any failure. + await Task.Delay(5000, cancellationToken); } } } + finally + { + connectionLock.Release(); + } } protected override Task JoinRoom(long roomId) @@ -189,5 +176,63 @@ namespace osu.Game.Online.Multiplayer return connection.InvokeAsync(nameof(IMultiplayerServer.StartMatch)); } + + private async Task disconnect(bool takeLock) + { + cancelExistingConnect(); + + if (takeLock) + await connectionLock.WaitAsync(); + + try + { + if (connection != null) + await connection.StopAsync(); + } + finally + { + connection = null; + if (takeLock) + connectionLock.Release(); + } + } + + private void cancelExistingConnect() + { + connectCancelSource.Cancel(); + connectCancelSource = new CancellationTokenSource(); + } + + private HubConnection createConnection(CancellationToken cancellationToken) + { + var newConnection = new HubConnectionBuilder() + .WithUrl(endpoint, options => { options.Headers.Add("Authorization", $"Bearer {api.AccessToken}"); }) + .AddNewtonsoftJsonProtocol(options => { options.PayloadSerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; }) + .Build(); + + // this is kind of SILLY + // https://github.com/dotnet/aspnetcore/issues/15198 + newConnection.On(nameof(IMultiplayerClient.RoomStateChanged), ((IMultiplayerClient)this).RoomStateChanged); + newConnection.On(nameof(IMultiplayerClient.UserJoined), ((IMultiplayerClient)this).UserJoined); + newConnection.On(nameof(IMultiplayerClient.UserLeft), ((IMultiplayerClient)this).UserLeft); + newConnection.On(nameof(IMultiplayerClient.HostChanged), ((IMultiplayerClient)this).HostChanged); + newConnection.On(nameof(IMultiplayerClient.SettingsChanged), ((IMultiplayerClient)this).SettingsChanged); + newConnection.On(nameof(IMultiplayerClient.UserStateChanged), ((IMultiplayerClient)this).UserStateChanged); + newConnection.On(nameof(IMultiplayerClient.LoadRequested), ((IMultiplayerClient)this).LoadRequested); + newConnection.On(nameof(IMultiplayerClient.MatchStarted), ((IMultiplayerClient)this).MatchStarted); + newConnection.On(nameof(IMultiplayerClient.ResultsReady), ((IMultiplayerClient)this).ResultsReady); + + newConnection.Closed += async ex => + { + isConnected.Value = false; + + Logger.Log(ex != null ? $"Multiplayer client lost connection: {ex}" : "Multiplayer client disconnected", LoggingTarget.Network); + + // make sure a disconnect wasn't triggered (and this is still the active connection). + if (!cancellationToken.IsCancellationRequested) + await Connect(); + }; + return newConnection; + } } } From d24d23646875b0bca4755c9d5b8a0caa24f30cae Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Jan 2021 14:34:58 +0900 Subject: [PATCH 11/30] Make OperationCanceledException throwing behaviour consistent --- .../Online/Multiplayer/MultiplayerClient.cs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 34616a45a5..aa2305c991 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -12,6 +12,7 @@ using Newtonsoft.Json; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Logging; +using osu.Game.Extensions; using osu.Game.Online.API; using osu.Game.Online.Rooms; @@ -51,11 +52,11 @@ namespace osu.Game.Online.Multiplayer { case APIState.Failing: case APIState.Offline: - Task.Run(Disconnect); + Task.Run(Disconnect).CatchUnobservedExceptions(); break; case APIState.Online: - Task.Run(Connect); + Task.Run(Connect).CatchUnobservedExceptions(); break; } } @@ -78,8 +79,10 @@ namespace osu.Game.Online.Multiplayer // if cancelled, we can be sure that a disconnect or reconnect is handled elsewhere. var cancellationToken = connectCancelSource.Token; - while (api.State.Value == APIState.Online && !cancellationToken.IsCancellationRequested) + while (api.State.Value == APIState.Online) { + cancellationToken.ThrowIfCancellationRequested(); + Logger.Log("Multiplayer client connecting...", LoggingTarget.Network); try @@ -96,7 +99,7 @@ namespace osu.Game.Online.Multiplayer catch (OperationCanceledException) { //connection process was cancelled. - return; + throw; } catch (Exception e) { @@ -222,7 +225,7 @@ namespace osu.Game.Online.Multiplayer newConnection.On(nameof(IMultiplayerClient.MatchStarted), ((IMultiplayerClient)this).MatchStarted); newConnection.On(nameof(IMultiplayerClient.ResultsReady), ((IMultiplayerClient)this).ResultsReady); - newConnection.Closed += async ex => + newConnection.Closed += ex => { isConnected.Value = false; @@ -230,7 +233,9 @@ namespace osu.Game.Online.Multiplayer // make sure a disconnect wasn't triggered (and this is still the active connection). if (!cancellationToken.IsCancellationRequested) - await Connect(); + Task.Run(Connect, default).CatchUnobservedExceptions(); + + return Task.CompletedTask; }; return newConnection; } From c05ae3497afa8c5a7dc551496e189620ccbd11ad Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jan 2021 17:02:24 +0900 Subject: [PATCH 12/30] Make connect/disconnect private --- osu.Game/Online/Multiplayer/MultiplayerClient.cs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index aa2305c991..7dc4919d23 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -25,6 +25,8 @@ namespace osu.Game.Online.Multiplayer private readonly Bindable isConnected = new Bindable(); private readonly IBindable apiState = new Bindable(); + private readonly SemaphoreSlim connectionLock = new SemaphoreSlim(1); + [Resolved] private IAPIProvider api { get; set; } = null!; @@ -52,20 +54,16 @@ namespace osu.Game.Online.Multiplayer { case APIState.Failing: case APIState.Offline: - Task.Run(Disconnect).CatchUnobservedExceptions(); + Task.Run(() => disconnect(true)).CatchUnobservedExceptions(); break; case APIState.Online: - Task.Run(Connect).CatchUnobservedExceptions(); + Task.Run(connect).CatchUnobservedExceptions(); break; } } - private readonly SemaphoreSlim connectionLock = new SemaphoreSlim(1); - - public Task Disconnect() => disconnect(true); - - protected async Task Connect() + private async Task connect() { cancelExistingConnect(); @@ -233,7 +231,7 @@ namespace osu.Game.Online.Multiplayer // make sure a disconnect wasn't triggered (and this is still the active connection). if (!cancellationToken.IsCancellationRequested) - Task.Run(Connect, default).CatchUnobservedExceptions(); + Task.Run(connect, default).CatchUnobservedExceptions(); return Task.CompletedTask; }; From 994fb2667dc22d06d0fc7aca26387ef03ec04859 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jan 2021 17:11:04 +0900 Subject: [PATCH 13/30] Call DisposeAsync instead of StopAsync --- osu.Game/Online/Multiplayer/MultiplayerClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 7dc4919d23..ffed2b57fe 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -188,7 +188,7 @@ namespace osu.Game.Online.Multiplayer try { if (connection != null) - await connection.StopAsync(); + await connection.DisposeAsync(); } finally { From 0f09a7feb9ee12c05c1d53464db9e6194e8818d0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jan 2021 17:17:04 +0900 Subject: [PATCH 14/30] Avoid semaphore potentially getting held forever --- osu.Game/Online/Multiplayer/MultiplayerClient.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index ffed2b57fe..391658f0d0 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -67,7 +67,7 @@ namespace osu.Game.Online.Multiplayer { cancelExistingConnect(); - await connectionLock.WaitAsync(); + await connectionLock.WaitAsync(10000); try { @@ -183,7 +183,7 @@ namespace osu.Game.Online.Multiplayer cancelExistingConnect(); if (takeLock) - await connectionLock.WaitAsync(); + await connectionLock.WaitAsync(10000); try { @@ -237,5 +237,12 @@ namespace osu.Game.Online.Multiplayer }; return newConnection; } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + cancelExistingConnect(); + } } } From b573c96c079f9065ee4690131f4677eeeb2998be Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jan 2021 18:59:42 +0900 Subject: [PATCH 15/30] Move disconnect logic inside connection loop to ensure previous connection is disposed --- osu.Game/Online/Multiplayer/MultiplayerClient.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 391658f0d0..cbb91c0832 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -71,14 +71,15 @@ namespace osu.Game.Online.Multiplayer try { - await disconnect(false); - // this token will be valid for the scope of this connection. // if cancelled, we can be sure that a disconnect or reconnect is handled elsewhere. var cancellationToken = connectCancelSource.Token; while (api.State.Value == APIState.Online) { + // ensure any previous connection was disposed. + await disconnect(false); + cancellationToken.ThrowIfCancellationRequested(); Logger.Log("Multiplayer client connecting...", LoggingTarget.Network); From 4d4d97661e7a8bbe924bdf4d2d5a414f017edb3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 26 Jan 2021 21:26:50 +0100 Subject: [PATCH 16/30] Fix connection loop always getting a cancelled token --- osu.Game/Online/Multiplayer/MultiplayerClient.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index cbb91c0832..319a8f7170 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -71,15 +71,16 @@ namespace osu.Game.Online.Multiplayer try { - // this token will be valid for the scope of this connection. - // if cancelled, we can be sure that a disconnect or reconnect is handled elsewhere. - var cancellationToken = connectCancelSource.Token; - while (api.State.Value == APIState.Online) { // ensure any previous connection was disposed. + // this will also create a new cancellation token source. await disconnect(false); + // this token will be valid for the scope of this connection. + // if cancelled, we can be sure that a disconnect or reconnect is handled elsewhere. + var cancellationToken = connectCancelSource.Token; + cancellationToken.ThrowIfCancellationRequested(); Logger.Log("Multiplayer client connecting...", LoggingTarget.Network); From abdd417eb6ace553d81a11c32b6bc723247557b8 Mon Sep 17 00:00:00 2001 From: vmaggioli Date: Wed, 3 Feb 2021 10:03:38 -0500 Subject: [PATCH 17/30] Remove slider changes --- .../TestSceneSliderSelectionBlueprint.cs | 19 ------------------- .../Sliders/SliderSelectionBlueprint.cs | 6 +----- 2 files changed, 1 insertion(+), 24 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs index 4edf778bfd..f6e1be693b 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderSelectionBlueprint.cs @@ -161,18 +161,6 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor checkControlPointSelected(1, false); } - [Test] - public void TestZeroLengthSliderNotAllowed() - { - moveMouseToControlPoint(1); - dragMouseToControlPoint(0); - - moveMouseToControlPoint(2); - dragMouseToControlPoint(0); - - AddAssert("slider has non-zero duration", () => slider.Duration > 0); - } - private void moveHitObject() { AddStep("move hitobject", () => @@ -201,13 +189,6 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor }); } - private void dragMouseToControlPoint(int index) - { - AddStep("hold down mouse button", () => InputManager.PressButton(MouseButton.Left)); - moveMouseToControlPoint(index); - AddStep("release mouse button", () => InputManager.ReleaseButton(MouseButton.Left)); - } - private void checkControlPointSelected(int index, bool selected) => AddAssert($"control point {index} {(selected ? "selected" : "not selected")}", () => blueprint.ControlPointVisualiser.Pieces[index].IsSelected.Value == selected); diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index 508783a499..3d3dff653a 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -226,11 +226,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders private void updatePath() { - float expectedDistance = composer?.GetSnappedDistanceFromDistance(HitObject.StartTime, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance; - if (expectedDistance < 1) - return; - - HitObject.Path.ExpectedDistance.Value = expectedDistance; + HitObject.Path.ExpectedDistance.Value = composer?.GetSnappedDistanceFromDistance(HitObject.StartTime, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance; editorBeatmap?.Update(HitObject); } From c5fa818630c25ee7b3912de0d1bfeca9194bc7a3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 5 Feb 2021 14:08:11 +0900 Subject: [PATCH 18/30] Actually handle case of failing to achieve lock on SemaphoreSlim --- osu.Game/Online/Multiplayer/MultiplayerClient.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 9067c9a738..36cdf3bc8f 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -67,7 +67,8 @@ namespace osu.Game.Online.Multiplayer { cancelExistingConnect(); - await connectionLock.WaitAsync(10000); + if (!await connectionLock.WaitAsync(10000)) + throw new TimeoutException("Could not obtain a lock to connect. A previous attempt is likely stuck."); var builder = new HubConnectionBuilder() .WithUrl(endpoint, options => { options.Headers.Add("Authorization", $"Bearer {api.AccessToken}"); }); @@ -199,7 +200,10 @@ namespace osu.Game.Online.Multiplayer cancelExistingConnect(); if (takeLock) - await connectionLock.WaitAsync(10000); + { + if (!await connectionLock.WaitAsync(10000)) + throw new TimeoutException("Could not obtain a lock to disconnect. A previous attempt is likely stuck."); + } try { From 730e66f0ee00aec2b6c9f2a2f03ca56ec152a136 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 5 Feb 2021 09:07:59 +0300 Subject: [PATCH 19/30] Make pausing on window focus lose instant --- .../Screens/Play/HUD/HoldForMenuButton.cs | 40 ------------------- osu.Game/Screens/Play/Player.cs | 21 +++++++--- 2 files changed, 16 insertions(+), 45 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/HoldForMenuButton.cs b/osu.Game/Screens/Play/HUD/HoldForMenuButton.cs index 387c0e587b..284ac899ed 100644 --- a/osu.Game/Screens/Play/HUD/HoldForMenuButton.cs +++ b/osu.Game/Screens/Play/HUD/HoldForMenuButton.cs @@ -88,11 +88,6 @@ namespace osu.Game.Screens.Play.HUD return base.OnMouseMove(e); } - public bool PauseOnFocusLost - { - set => button.PauseOnFocusLost = value; - } - protected override void Update() { base.Update(); @@ -120,8 +115,6 @@ namespace osu.Game.Screens.Play.HUD public Action HoverGained; public Action HoverLost; - private readonly IBindable gameActive = new Bindable(true); - [BackgroundDependencyLoader] private void load(OsuColour colours, Framework.Game game) { @@ -164,14 +157,6 @@ namespace osu.Game.Screens.Play.HUD }; bind(); - - gameActive.BindTo(game.IsActive); - } - - protected override void LoadComplete() - { - base.LoadComplete(); - gameActive.BindValueChanged(_ => updateActive(), true); } private void bind() @@ -221,31 +206,6 @@ namespace osu.Game.Screens.Play.HUD base.OnHoverLost(e); } - private bool pauseOnFocusLost = true; - - public bool PauseOnFocusLost - { - set - { - if (pauseOnFocusLost == value) - return; - - pauseOnFocusLost = value; - if (IsLoaded) - updateActive(); - } - } - - private void updateActive() - { - if (!pauseOnFocusLost || IsPaused.Value) return; - - if (gameActive.Value) - AbortConfirm(); - else - BeginConfirm(); - } - public bool OnPressed(GlobalAction action) { switch (action) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index b622f11775..556964bca4 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -59,6 +59,8 @@ namespace osu.Game.Screens.Play // We are managing our own adjustments (see OnEntering/OnExiting). public override bool AllowRateAdjustments => false; + private readonly IBindable gameActive = new Bindable(true); + private readonly Bindable samplePlaybackDisabled = new Bindable(); /// @@ -154,6 +156,9 @@ namespace osu.Game.Screens.Play // replays should never be recorded or played back when autoplay is enabled if (!Mods.Value.Any(m => m is ModAutoplay)) PrepareReplay(); + + // needs to be bound here as the last binding, otherwise starting a replay while not focused causes player to exit. + gameActive.BindValueChanged(_ => updatePauseOnFocusLostState(), true); } [CanBeNull] @@ -170,7 +175,7 @@ namespace osu.Game.Screens.Play } [BackgroundDependencyLoader(true)] - private void load(AudioManager audio, OsuConfigManager config, OsuGame game) + private void load(AudioManager audio, OsuConfigManager config, OsuGame game, OsuGameBase gameBase) { Mods.Value = base.Mods.Value.Select(m => m.CreateCopy()).ToArray(); @@ -186,6 +191,8 @@ namespace osu.Game.Screens.Play mouseWheelDisabled = config.GetBindable(OsuSetting.MouseDisableWheel); + gameActive.BindTo(gameBase.IsActive); + if (game != null) LocalUserPlaying.BindTo(game.LocalUserPlaying); @@ -420,10 +427,14 @@ namespace osu.Game.Screens.Play samplePlaybackDisabled.Value = DrawableRuleset.FrameStableClock.IsCatchingUp.Value || GameplayClockContainer.GameplayClock.IsPaused.Value; } - private void updatePauseOnFocusLostState() => - HUDOverlay.HoldToQuit.PauseOnFocusLost = PauseOnFocusLost - && !DrawableRuleset.HasReplayLoaded.Value - && !breakTracker.IsBreakTime.Value; + private void updatePauseOnFocusLostState() + { + if (!IsLoaded || !PauseOnFocusLost || DrawableRuleset.HasReplayLoaded.Value || breakTracker.IsBreakTime.Value) + return; + + if (gameActive.Value == false) + performUserRequestedExit(); + } private IBeatmap loadPlayableBeatmap() { From e1789c29b1a1e01f235b8e6bb9bdcfd131e5d715 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 5 Feb 2021 10:28:13 +0300 Subject: [PATCH 20/30] Use `Pause()` instead of `performUserRequestedExit()` to avoid unexpected operations --- osu.Game/Screens/Play/Player.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 556964bca4..542839f11d 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -433,7 +433,7 @@ namespace osu.Game.Screens.Play return; if (gameActive.Value == false) - performUserRequestedExit(); + Pause(); } private IBeatmap loadPlayableBeatmap() From 8d18c7e9299cc9c8e4a8b55563c3a9a22d1a99bd Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 5 Feb 2021 10:28:33 +0300 Subject: [PATCH 21/30] Fix `BreakTracker.IsBreakTime` not updated properly on breaks set Causes a pause from focus lose when playing a beatmap that has a break section at the beginning, due to `IsBreakTime` incorrectly set to `false` --- osu.Game/Screens/Play/BreakTracker.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Play/BreakTracker.cs b/osu.Game/Screens/Play/BreakTracker.cs index 51e21656e1..793b6b0ebe 100644 --- a/osu.Game/Screens/Play/BreakTracker.cs +++ b/osu.Game/Screens/Play/BreakTracker.cs @@ -23,16 +23,16 @@ namespace osu.Game.Screens.Play /// public IBindable IsBreakTime => isBreakTime; - private readonly BindableBool isBreakTime = new BindableBool(); + private readonly BindableBool isBreakTime = new BindableBool(true); public IReadOnlyList Breaks { set { - isBreakTime.Value = false; - breaks = new PeriodTracker(value.Where(b => b.HasEffect) .Select(b => new Period(b.StartTime, b.EndTime - BreakOverlay.BREAK_FADE_DURATION))); + + updateBreakTime(); } } @@ -45,8 +45,12 @@ namespace osu.Game.Screens.Play protected override void Update() { base.Update(); + updateBreakTime(); + } - var time = Clock.CurrentTime; + private void updateBreakTime() + { + var time = Clock?.CurrentTime ?? 0; isBreakTime.Value = breaks?.IsInAny(time) == true || time < gameplayStartTime From 5061231e599a2d04a40a2526872383987c9acddb Mon Sep 17 00:00:00 2001 From: vmaggioli Date: Fri, 5 Feb 2021 09:39:14 -0500 Subject: [PATCH 22/30] Switch to beat length --- .../Compose/Components/Timeline/TimelineHitObjectBlueprint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs index 301543b3c1..d24614299c 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineHitObjectBlueprint.cs @@ -388,7 +388,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline case IHasDuration endTimeHitObject: var snappedTime = Math.Max(hitObject.StartTime, beatSnapProvider.SnapTime(time)); - if (endTimeHitObject.EndTime == snappedTime || Precision.AlmostEquals(snappedTime, hitObject.StartTime, 1)) + if (endTimeHitObject.EndTime == snappedTime || Precision.AlmostEquals(snappedTime, hitObject.StartTime, beatmap.GetBeatLengthAtTime(snappedTime))) return; endTimeHitObject.Duration = snappedTime - hitObject.StartTime; From f29938e15d62bad02c0ff8d0f586bc2764f423a9 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 5 Feb 2021 20:39:57 +0300 Subject: [PATCH 23/30] Make last binding game activity more sensible --- osu.Game/Screens/Play/Player.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 542839f11d..f38eba3f27 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -80,6 +80,9 @@ namespace osu.Game.Screens.Play public int RestartCount; + [Resolved] + private OsuGameBase gameBase { get; set; } + [Resolved] private ScoreManager scoreManager { get; set; } @@ -157,7 +160,8 @@ namespace osu.Game.Screens.Play if (!Mods.Value.Any(m => m is ModAutoplay)) PrepareReplay(); - // needs to be bound here as the last binding, otherwise starting a replay while not focused causes player to exit. + // needs to be bound here as the last binding, otherwise cases like starting a replay while not focused causes player to exit, if activity is bound before checks. + gameActive.BindTo(gameBase.IsActive); gameActive.BindValueChanged(_ => updatePauseOnFocusLostState(), true); } @@ -191,8 +195,6 @@ namespace osu.Game.Screens.Play mouseWheelDisabled = config.GetBindable(OsuSetting.MouseDisableWheel); - gameActive.BindTo(gameBase.IsActive); - if (game != null) LocalUserPlaying.BindTo(game.LocalUserPlaying); @@ -429,7 +431,7 @@ namespace osu.Game.Screens.Play private void updatePauseOnFocusLostState() { - if (!IsLoaded || !PauseOnFocusLost || DrawableRuleset.HasReplayLoaded.Value || breakTracker.IsBreakTime.Value) + if (!PauseOnFocusLost || DrawableRuleset.HasReplayLoaded.Value || breakTracker.IsBreakTime.Value) return; if (gameActive.Value == false) From c9db0bf88651affc9bdd3a165984ad7577770149 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 6 Feb 2021 20:54:13 +0300 Subject: [PATCH 24/30] Call break time update when loaded --- osu.Game/Screens/Play/BreakTracker.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/BreakTracker.cs b/osu.Game/Screens/Play/BreakTracker.cs index 793b6b0ebe..2f3673e91f 100644 --- a/osu.Game/Screens/Play/BreakTracker.cs +++ b/osu.Game/Screens/Play/BreakTracker.cs @@ -32,7 +32,8 @@ namespace osu.Game.Screens.Play breaks = new PeriodTracker(value.Where(b => b.HasEffect) .Select(b => new Period(b.StartTime, b.EndTime - BreakOverlay.BREAK_FADE_DURATION))); - updateBreakTime(); + if (IsLoaded) + updateBreakTime(); } } @@ -50,7 +51,7 @@ namespace osu.Game.Screens.Play private void updateBreakTime() { - var time = Clock?.CurrentTime ?? 0; + var time = Clock.CurrentTime; isBreakTime.Value = breaks?.IsInAny(time) == true || time < gameplayStartTime From 40ddccf0c73904af580d3023b3d79d45a14868f3 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 6 Feb 2021 20:55:55 +0300 Subject: [PATCH 25/30] Do not consider replays for "pause on focus lost" Replays are not pausable as can be seen in the `canPause` check. --- osu.Game/Screens/Play/Player.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index f38eba3f27..81401b08e8 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -160,7 +160,6 @@ namespace osu.Game.Screens.Play if (!Mods.Value.Any(m => m is ModAutoplay)) PrepareReplay(); - // needs to be bound here as the last binding, otherwise cases like starting a replay while not focused causes player to exit, if activity is bound before checks. gameActive.BindTo(gameBase.IsActive); gameActive.BindValueChanged(_ => updatePauseOnFocusLostState(), true); } @@ -267,8 +266,6 @@ namespace osu.Game.Screens.Play DrawableRuleset.HasReplayLoaded.BindValueChanged(_ => updateGameplayState()); - DrawableRuleset.HasReplayLoaded.BindValueChanged(_ => updatePauseOnFocusLostState(), true); - // bind clock into components that require it DrawableRuleset.IsPaused.BindTo(GameplayClockContainer.IsPaused); @@ -431,7 +428,7 @@ namespace osu.Game.Screens.Play private void updatePauseOnFocusLostState() { - if (!PauseOnFocusLost || DrawableRuleset.HasReplayLoaded.Value || breakTracker.IsBreakTime.Value) + if (!PauseOnFocusLost || breakTracker.IsBreakTime.Value) return; if (gameActive.Value == false) From d0ca2b99a850f9903eec2f7ac1956e15a73089a2 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 6 Feb 2021 20:57:01 +0300 Subject: [PATCH 26/30] Remove unnecessary injected dependency --- osu.Game/Screens/Play/Player.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 81401b08e8..bd67d3f06a 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -178,7 +178,7 @@ namespace osu.Game.Screens.Play } [BackgroundDependencyLoader(true)] - private void load(AudioManager audio, OsuConfigManager config, OsuGame game, OsuGameBase gameBase) + private void load(AudioManager audio, OsuConfigManager config, OsuGame game) { Mods.Value = base.Mods.Value.Select(m => m.CreateCopy()).ToArray(); From d74a1437beddf07f471254588a4609130a361cd2 Mon Sep 17 00:00:00 2001 From: Joehu Date: Sun, 7 Feb 2021 15:14:08 -0800 Subject: [PATCH 27/30] Fix player loader metadata not being centred --- .../Screens/Play/BeatmapMetadataDisplay.cs | 78 +++++++++++-------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs b/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs index b53141e8fb..eff06e26ee 100644 --- a/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs +++ b/osu.Game/Screens/Play/BeatmapMetadataDisplay.cs @@ -23,32 +23,6 @@ namespace osu.Game.Screens.Play /// public class BeatmapMetadataDisplay : Container { - private class MetadataLine : Container - { - public MetadataLine(string left, string right) - { - AutoSizeAxes = Axes.Both; - Children = new Drawable[] - { - new OsuSpriteText - { - Anchor = Anchor.TopCentre, - Origin = Anchor.TopRight, - Margin = new MarginPadding { Right = 5 }, - Colour = OsuColour.Gray(0.8f), - Text = left, - }, - new OsuSpriteText - { - Anchor = Anchor.TopCentre, - Origin = Anchor.TopLeft, - Margin = new MarginPadding { Left = 5 }, - Text = string.IsNullOrEmpty(right) ? @"-" : right, - } - }; - } - } - private readonly WorkingBeatmap beatmap; private readonly Bindable> mods; private readonly Drawable facade; @@ -144,15 +118,34 @@ namespace osu.Game.Screens.Play Bottom = 40 }, }, - new MetadataLine("Source", metadata.Source) + new GridContainer { - Origin = Anchor.TopCentre, Anchor = Anchor.TopCentre, - }, - new MetadataLine("Mapper", metadata.AuthorString) - { Origin = Anchor.TopCentre, - Anchor = Anchor.TopCentre, + AutoSizeAxes = Axes.Both, + RowDimensions = new[] + { + new Dimension(GridSizeMode.AutoSize), + new Dimension(GridSizeMode.AutoSize), + }, + ColumnDimensions = new[] + { + new Dimension(GridSizeMode.AutoSize), + new Dimension(GridSizeMode.AutoSize), + }, + Content = new[] + { + new Drawable[] + { + new MetadataLineLabel("Source"), + new MetadataLineInfo(metadata.Source) + }, + new Drawable[] + { + new MetadataLineLabel("Mapper"), + new MetadataLineInfo(metadata.AuthorString) + } + } }, new ModDisplay { @@ -168,5 +161,26 @@ namespace osu.Game.Screens.Play Loading = true; } + + private class MetadataLineLabel : OsuSpriteText + { + public MetadataLineLabel(string text) + { + Anchor = Anchor.TopRight; + Origin = Anchor.TopRight; + Margin = new MarginPadding { Right = 5 }; + Colour = OsuColour.Gray(0.8f); + Text = text; + } + } + + private class MetadataLineInfo : OsuSpriteText + { + public MetadataLineInfo(string text) + { + Margin = new MarginPadding { Left = 5 }; + Text = string.IsNullOrEmpty(text) ? @"-" : text; + } + } } } From 9e0724b138fd4c251dc61e5daa3e702dd2a77cee Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 8 Feb 2021 15:58:41 +0900 Subject: [PATCH 28/30] Remove unnecessary double resolution of OsuGame --- osu.Game/Screens/Play/Player.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index bd67d3f06a..669fa93298 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -80,9 +80,6 @@ namespace osu.Game.Screens.Play public int RestartCount; - [Resolved] - private OsuGameBase gameBase { get; set; } - [Resolved] private ScoreManager scoreManager { get; set; } @@ -160,7 +157,6 @@ namespace osu.Game.Screens.Play if (!Mods.Value.Any(m => m is ModAutoplay)) PrepareReplay(); - gameActive.BindTo(gameBase.IsActive); gameActive.BindValueChanged(_ => updatePauseOnFocusLostState(), true); } @@ -195,7 +191,10 @@ namespace osu.Game.Screens.Play mouseWheelDisabled = config.GetBindable(OsuSetting.MouseDisableWheel); if (game != null) + { LocalUserPlaying.BindTo(game.LocalUserPlaying); + gameActive.BindTo(game.IsActive); + } DrawableRuleset = ruleset.CreateDrawableRulesetWith(playableBeatmap, Mods.Value); From 10142a44716882a4671d4cae2391a96348bd90ba Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 8 Feb 2021 16:59:21 +0900 Subject: [PATCH 29/30] Disable failing test temporarily pending resolution --- .../Visual/Multiplayer/TestScenePlaylistsSongSelect.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs b/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs index 2f7e59f800..1d13c6229c 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs @@ -143,6 +143,7 @@ namespace osu.Game.Tests.Visual.Multiplayer /// Tests that the same instances are not shared between two playlist items. /// [Test] + [Ignore("Temporarily disabled due to a non-trivial test failure")] public void TestNewItemHasNewModInstances() { AddStep("set dt mod", () => SelectedMods.Value = new[] { new OsuModDoubleTime() }); From fb8e31a30385636856e20ebdeb67d76ac6d815c8 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 8 Feb 2021 17:51:57 +0900 Subject: [PATCH 30/30] Fix incorrect connection building due to bad merges --- .../Online/Multiplayer/MultiplayerClient.cs | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 1b966ae1dc..6908795510 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -71,20 +71,6 @@ namespace osu.Game.Online.Multiplayer if (!await connectionLock.WaitAsync(10000)) throw new TimeoutException("Could not obtain a lock to connect. A previous attempt is likely stuck."); - var builder = new HubConnectionBuilder() - .WithUrl(endpoint, options => { options.Headers.Add("Authorization", $"Bearer {api.AccessToken}"); }); - - if (RuntimeInfo.SupportsJIT) - builder.AddMessagePackProtocol(); - else - { - // eventually we will precompile resolvers for messagepack, but this isn't working currently - // see https://github.com/neuecc/MessagePack-CSharp/issues/780#issuecomment-768794308. - builder.AddNewtonsoftJsonProtocol(options => { options.PayloadSerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; }); - } - - connection = builder.Build(); - try { while (api.State.Value == APIState.Online) @@ -235,10 +221,19 @@ namespace osu.Game.Online.Multiplayer private HubConnection createConnection(CancellationToken cancellationToken) { - var newConnection = new HubConnectionBuilder() - .WithUrl(endpoint, options => { options.Headers.Add("Authorization", $"Bearer {api.AccessToken}"); }) - .AddNewtonsoftJsonProtocol(options => { options.PayloadSerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; }) - .Build(); + var builder = new HubConnectionBuilder() + .WithUrl(endpoint, options => { options.Headers.Add("Authorization", $"Bearer {api.AccessToken}"); }); + + if (RuntimeInfo.SupportsJIT) + builder.AddMessagePackProtocol(); + else + { + // eventually we will precompile resolvers for messagepack, but this isn't working currently + // see https://github.com/neuecc/MessagePack-CSharp/issues/780#issuecomment-768794308. + builder.AddNewtonsoftJsonProtocol(options => { options.PayloadSerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; }); + } + + var newConnection = builder.Build(); // this is kind of SILLY // https://github.com/dotnet/aspnetcore/issues/15198