From b0e21c2749d24d58de0b4c92569244bdeed77460 Mon Sep 17 00:00:00 2001 From: nwabear Date: Fri, 25 Oct 2019 14:57:49 -0500 Subject: [PATCH 01/11] Fixed Issue #6442 --- osu.Game/Rulesets/UI/DrawableRuleset.cs | 7 +++++++ osu.Game/Screens/Play/Player.cs | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/osu.Game/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs index d5b3df27df..d37f053486 100644 --- a/osu.Game/Rulesets/UI/DrawableRuleset.cs +++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs @@ -239,6 +239,11 @@ namespace osu.Game.Rulesets.UI continueResume(); } + public override void CancelResume() + { + ResumeOverlay.Hide(); + } + /// /// Creates and adds the visual representation of a to this . /// @@ -453,6 +458,8 @@ namespace osu.Game.Rulesets.UI /// The action to run when resuming is to be completed. public abstract void RequestResume(Action continueResume); + public abstract void CancelResume(); + /// /// Create a for the associated ruleset and link with this /// . diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 0b363eac4d..7eccde4555 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -443,6 +443,11 @@ namespace osu.Game.Screens.Play { if (!canPause) return; + if (IsResuming) + { + DrawableRuleset.CancelResume(); + } + IsResuming = false; GameplayClockContainer.Stop(); PauseOverlay.Show(); From f8354eefc4e058b98fc9d834bc0be2bc441db1e4 Mon Sep 17 00:00:00 2001 From: nwabear Date: Fri, 25 Oct 2019 16:49:18 -0500 Subject: [PATCH 02/11] Added null check in the CancelResume method --- osu.Game/Rulesets/UI/DrawableRuleset.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs index d37f053486..d3e1118625 100644 --- a/osu.Game/Rulesets/UI/DrawableRuleset.cs +++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs @@ -241,7 +241,7 @@ namespace osu.Game.Rulesets.UI public override void CancelResume() { - ResumeOverlay.Hide(); + ResumeOverlay?.Hide(); } /// From 9e2e87c8d13cf147b720825a5ff266c024cabf72 Mon Sep 17 00:00:00 2001 From: nwabear Date: Sat, 26 Oct 2019 14:29:52 -0500 Subject: [PATCH 03/11] added visual tests added small commenting added xmldoc for CancelResume(); --- .../Visual/Gameplay/TestScenePause.cs | 18 ++++++++++++++++++ osu.Game/Rulesets/UI/DrawableRuleset.cs | 5 +++++ 2 files changed, 23 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index 2df22df659..64022b2410 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -69,6 +69,24 @@ namespace osu.Game.Tests.Visual.Gameplay confirmClockRunning(true); } + [Test] + public void TestPauseWithResumeOverlay() + { + AddStep("move cursor to center", () => InputManager.MoveMouseTo(Player.ScreenSpaceDrawQuad.Centre)); + AddUntilStep("wait for hitobjects", () => Player.ScoreProcessor.Health.Value < 1); + + pauseAndConfirm(); + + resume(); + confirmClockRunning(false); + confirmPauseOverlayShown(false); + + pauseAndConfirm(); + + AddUntilStep("resume overlay is not active", () => Player.DrawableRuleset.ResumeOverlay.State.Value == Visibility.Hidden); + confirmPaused(); + } + [Test] public void TestResumeWithResumeOverlaySkipped() { diff --git a/osu.Game/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs index d3e1118625..44e2e60be7 100644 --- a/osu.Game/Rulesets/UI/DrawableRuleset.cs +++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs @@ -239,8 +239,10 @@ namespace osu.Game.Rulesets.UI continueResume(); } + public override void CancelResume() { + // called if the user pauses while the resume overlay is open ResumeOverlay?.Hide(); } @@ -458,6 +460,9 @@ namespace osu.Game.Rulesets.UI /// The action to run when resuming is to be completed. public abstract void RequestResume(Action continueResume); + /// + /// Invoked when the user requests to pause while the resume overlay is active. + /// public abstract void CancelResume(); /// From e35931fdfc7b6c94d401ab0076ce3e80f2edffcd Mon Sep 17 00:00:00 2001 From: nwabear Date: Sat, 26 Oct 2019 14:33:59 -0500 Subject: [PATCH 04/11] removed blank line --- osu.Game/Rulesets/UI/DrawableRuleset.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs index 44e2e60be7..e005eea831 100644 --- a/osu.Game/Rulesets/UI/DrawableRuleset.cs +++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs @@ -239,7 +239,6 @@ namespace osu.Game.Rulesets.UI continueResume(); } - public override void CancelResume() { // called if the user pauses while the resume overlay is open From 32dabf80a6f896cba970220d0e2435051a2c5982 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 31 Oct 2019 13:42:11 +0900 Subject: [PATCH 05/11] Ensure forceful exit completely exits from mutliplayer Previously it may have gotten blocked by being in a sub screen. --- osu.Game/Screens/Multi/Multiplayer.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Multi/Multiplayer.cs b/osu.Game/Screens/Multi/Multiplayer.cs index 5945e9de13..941d7a2478 100644 --- a/osu.Game/Screens/Multi/Multiplayer.cs +++ b/osu.Game/Screens/Multi/Multiplayer.cs @@ -174,7 +174,10 @@ namespace osu.Game.Screens.Multi { // This is temporary since we don't currently have a way to force screens to be exited if (this.IsCurrentScreen()) - this.Exit(); + { + while (this.IsCurrentScreen()) + this.Exit(); + } else { this.MakeCurrent(); From 5b405abc5253480414eb421028d48c1cb019ef0b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 31 Oct 2019 13:43:25 +0900 Subject: [PATCH 06/11] Schedule forcefullyExit call for safety Screen state may have changed at an inopportune moment. Run on local scheduler, not API scheduler to avoid any weirdness. --- osu.Game/Screens/Multi/Multiplayer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Multi/Multiplayer.cs b/osu.Game/Screens/Multi/Multiplayer.cs index 941d7a2478..86d52ff791 100644 --- a/osu.Game/Screens/Multi/Multiplayer.cs +++ b/osu.Game/Screens/Multi/Multiplayer.cs @@ -167,7 +167,7 @@ namespace osu.Game.Screens.Multi public void APIStateChanged(IAPIProvider api, APIState state) { if (state != APIState.Online) - forcefullyExit(); + Schedule(forcefullyExit); } private void forcefullyExit() From 0cd912fcd3ad364b8ade2a7d5e98685f3e3c68d5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 31 Oct 2019 15:04:13 +0900 Subject: [PATCH 07/11] Cover all non-APIAccess APIRequest calls with exception handling --- osu.Game/Beatmaps/BeatmapManager.cs | 11 ++++-- .../DownloadableArchiveModelManager.cs | 35 ++++++++++++------- .../Changelog/ChangelogSingleBuild.cs | 12 ++++++- osu.Game/Overlays/ChangelogOverlay.cs | 19 ++++++++-- 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index dd2044b4bc..6e485f642a 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -392,8 +392,15 @@ namespace osu.Game.Beatmaps req.Failure += e => { LogForModel(set, $"Online retrieval failed for {beatmap} ({e.Message})"); }; - // intentionally blocking to limit web request concurrency - req.Perform(api); + try + { + // intentionally blocking to limit web request concurrency + req.Perform(api); + } + catch (Exception e) + { + LogForModel(set, $"Online retrieval failed for {beatmap} ({e.Message})"); + } } } } diff --git a/osu.Game/Database/DownloadableArchiveModelManager.cs b/osu.Game/Database/DownloadableArchiveModelManager.cs index e3c6ad25e6..a81dff3475 100644 --- a/osu.Game/Database/DownloadableArchiveModelManager.cs +++ b/osu.Game/Database/DownloadableArchiveModelManager.cs @@ -86,16 +86,7 @@ namespace osu.Game.Database }, TaskCreationOptions.LongRunning); }; - request.Failure += error => - { - DownloadFailed?.Invoke(request); - - if (error is OperationCanceledException) return; - - notification.State = ProgressNotificationState.Cancelled; - Logger.Error(error, $"{HumanisedModelName.Titleize()} download failed!"); - currentDownloads.Remove(request); - }; + request.Failure += triggerFailure; notification.CancelRequested += () => { @@ -108,11 +99,31 @@ namespace osu.Game.Database currentDownloads.Add(request); PostNotification?.Invoke(notification); - Task.Factory.StartNew(() => request.Perform(api), TaskCreationOptions.LongRunning); + Task.Factory.StartNew(() => + { + try + { + request.Perform(api); + } + catch (Exception error) + { + triggerFailure(error); + } + }, TaskCreationOptions.LongRunning); DownloadBegan?.Invoke(request); - return true; + + void triggerFailure(Exception error) + { + DownloadFailed?.Invoke(request); + + if (error is OperationCanceledException) return; + + notification.State = ProgressNotificationState.Cancelled; + Logger.Error(error, $"{HumanisedModelName.Titleize()} download failed!"); + currentDownloads.Remove(request); + } } public bool IsAvailableLocally(TModel model) => CheckLocalAvailability(model, modelStore.ConsumableItems.Where(m => !m.DeletePending)); diff --git a/osu.Game/Overlays/Changelog/ChangelogSingleBuild.cs b/osu.Game/Overlays/Changelog/ChangelogSingleBuild.cs index 9c3504f477..adcd33fb48 100644 --- a/osu.Game/Overlays/Changelog/ChangelogSingleBuild.cs +++ b/osu.Game/Overlays/Changelog/ChangelogSingleBuild.cs @@ -44,7 +44,17 @@ namespace osu.Game.Overlays.Changelog req.Failure += _ => complete = true; // This is done on a separate thread to support cancellation below - Task.Run(() => req.Perform(api)); + Task.Run(() => + { + try + { + req.Perform(api); + } + catch + { + complete = true; + } + }); while (!complete) { diff --git a/osu.Game/Overlays/ChangelogOverlay.cs b/osu.Game/Overlays/ChangelogOverlay.cs index dfe3669813..559989af5c 100644 --- a/osu.Game/Overlays/ChangelogOverlay.cs +++ b/osu.Game/Overlays/ChangelogOverlay.cs @@ -170,6 +170,7 @@ namespace osu.Game.Overlays var tcs = new TaskCompletionSource(); var req = new GetChangelogRequest(); + req.Success += res => Schedule(() => { // remap streams to builds to ensure model equality @@ -183,8 +184,22 @@ namespace osu.Game.Overlays tcs.SetResult(true); }); - req.Failure += _ => initialFetchTask = null; - req.Perform(API); + + req.Failure += _ => + { + initialFetchTask = null; + tcs.SetResult(false); + }; + + try + { + req.Perform(API); + } + catch + { + initialFetchTask = null; + tcs.SetResult(false); + } await tcs.Task; }); From 0171b2ae7c0882e2d0ad7e499d4e4369b580af6b Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 1 Nov 2019 12:10:03 +0900 Subject: [PATCH 08/11] Fix scrolling hitobjects expiring too soon --- .../UI/Scrolling/ScrollingHitObjectContainer.cs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs b/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs index e00597dd56..857929ff9e 100644 --- a/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs +++ b/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs @@ -13,12 +13,6 @@ namespace osu.Game.Rulesets.UI.Scrolling { public class ScrollingHitObjectContainer : HitObjectContainer { - /// - /// A multiplier applied to the length of the scrolling area to determine a safe default lifetime end for hitobjects. - /// This is only used to limit the lifetime end within reason, as proper lifetime management should be implemented on hitobjects themselves. - /// - private const float safe_lifetime_end_multiplier = 2; - private readonly IBindable timeRange = new BindableDouble(); private readonly IBindable direction = new Bindable(); @@ -123,28 +117,22 @@ namespace osu.Game.Rulesets.UI.Scrolling if (cached.IsValid) return; - double endTime = hitObject.HitObject.StartTime; - if (hitObject.HitObject is IHasEndTime e) { - endTime = e.EndTime; - switch (direction.Value) { case ScrollingDirection.Up: case ScrollingDirection.Down: - hitObject.Height = scrollingInfo.Algorithm.GetLength(hitObject.HitObject.StartTime, endTime, timeRange.Value, scrollLength); + hitObject.Height = scrollingInfo.Algorithm.GetLength(hitObject.HitObject.StartTime, e.EndTime, timeRange.Value, scrollLength); break; case ScrollingDirection.Left: case ScrollingDirection.Right: - hitObject.Width = scrollingInfo.Algorithm.GetLength(hitObject.HitObject.StartTime, endTime, timeRange.Value, scrollLength); + hitObject.Width = scrollingInfo.Algorithm.GetLength(hitObject.HitObject.StartTime, e.EndTime, timeRange.Value, scrollLength); break; } } - hitObject.LifetimeEnd = scrollingInfo.Algorithm.TimeAt(scrollLength * safe_lifetime_end_multiplier, endTime, timeRange.Value, scrollLength); - foreach (var obj in hitObject.NestedHitObjects) { computeInitialStateRecursive(obj); From dcc8f6e82799b3fe7055068b446780ad5ed76113 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Nov 2019 14:43:52 +0900 Subject: [PATCH 09/11] Better group cancel conditional --- 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 7eccde4555..280ebca651 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -446,9 +446,9 @@ namespace osu.Game.Screens.Play if (IsResuming) { DrawableRuleset.CancelResume(); + IsResuming = false; } - IsResuming = false; GameplayClockContainer.Stop(); PauseOverlay.Show(); lastPauseActionTime = GameplayClockContainer.GameplayClock.CurrentTime; From 05002ea3e8cd308e46e8147f909920ebd56300b3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Nov 2019 14:50:38 +0900 Subject: [PATCH 10/11] Move Show/Hide code to PopIn/PopOut --- osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs index 8347d255fa..aecfac3b70 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs @@ -38,9 +38,10 @@ namespace osu.Game.Rulesets.Osu.UI }); } - public override void Show() + protected override void PopIn() { - base.Show(); + base.PopIn(); + GameplayCursor.ActiveCursor.Hide(); cursorScaleContainer.MoveTo(GameplayCursor.ActiveCursor.Position); clickToResumeCursor.Appear(); @@ -55,13 +56,13 @@ namespace osu.Game.Rulesets.Osu.UI } } - public override void Hide() + protected override void PopOut() { + base.PopOut(); + localCursorContainer?.Expire(); localCursorContainer = null; - GameplayCursor.ActiveCursor.Show(); - - base.Hide(); + GameplayCursor?.ActiveCursor.Show(); } protected override bool OnHover(HoverEvent e) => true; From e23ea94383e293c26764125637ef4627232f436f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Nov 2019 15:33:38 +0900 Subject: [PATCH 11/11] Add one more level of null check --- osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs index aecfac3b70..3b18e41f30 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs @@ -62,7 +62,7 @@ namespace osu.Game.Rulesets.Osu.UI localCursorContainer?.Expire(); localCursorContainer = null; - GameplayCursor?.ActiveCursor.Show(); + GameplayCursor?.ActiveCursor?.Show(); } protected override bool OnHover(HoverEvent e) => true;