From a9da58b2b37ae2b050d45e79c2deec9c1b3fa12d Mon Sep 17 00:00:00 2001 From: hwabis Date: Sun, 22 Oct 2023 14:41:15 -0400 Subject: [PATCH 1/4] Add and fix no exit blocking test --- .../UserInterface/TestSceneScreenStack.cs | 21 +++++++++++++++++++ osu.Framework/Screens/ScreenStack.cs | 3 ++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs index 7474f18455..b56ddb2395 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs @@ -798,6 +798,27 @@ public void TestPushOnExiting() AddAssert("Stack is not empty", () => stack.CurrentScreen != null); } + + [Test] + public void TestPushOnExitingWithoutBlocking() + { + TestScreen screen1 = null; + + pushAndEnsureCurrent(() => + { + screen1 = new TestScreen(id: 1); + screen1.Exiting = () => + { + screen1.Push(new TestScreen(id: 2)); + return false; + }; + return screen1; + }); + + AddStep("ensure push throws", () => Assert.Throws( + () => screen1.Exit())); + } + [Test] public void TestInvalidPushBlocksNonImmediateSuspend() { diff --git a/osu.Framework/Screens/ScreenStack.cs b/osu.Framework/Screens/ScreenStack.cs index a319ea0c42..e30fe97c40 100644 --- a/osu.Framework/Screens/ScreenStack.cs +++ b/osu.Framework/Screens/ScreenStack.cs @@ -304,7 +304,8 @@ private bool exitFrom([CanBeNull] IScreen source, bool shouldFireExitEvent = tru if ((source == null || toExit.ValidForResume) && blockRequested) return true; - stack.Pop(); + if (toExit != stack.Pop()) + throw new ScreenNotCurrentException("Cannot push to ScreenStack during exit without blocking it."); } // we will probably want to change this logic when we support returning to a screen after exiting. From 0c39dcd674bd8f4014525f3d1e98b053538a6b1d Mon Sep 17 00:00:00 2001 From: hwabis Date: Sun, 22 Oct 2023 14:45:58 -0400 Subject: [PATCH 2/4] Clarify exception message --- osu.Framework/Screens/ScreenStack.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Framework/Screens/ScreenStack.cs b/osu.Framework/Screens/ScreenStack.cs index e30fe97c40..525e3fc7a8 100644 --- a/osu.Framework/Screens/ScreenStack.cs +++ b/osu.Framework/Screens/ScreenStack.cs @@ -305,7 +305,7 @@ private bool exitFrom([CanBeNull] IScreen source, bool shouldFireExitEvent = tru return true; if (toExit != stack.Pop()) - throw new ScreenNotCurrentException("Cannot push to ScreenStack during exit without blocking it."); + throw new ScreenNotCurrentException("Cannot push to ScreenStack during exit without blocking the exit."); } // we will probably want to change this logic when we support returning to a screen after exiting. From 85f91a48c9c39cf63f79909f7286d1cdc6c87612 Mon Sep 17 00:00:00 2001 From: hwabis Date: Sun, 22 Oct 2023 14:48:47 -0400 Subject: [PATCH 3/4] Formatting --- osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs index b56ddb2395..6465b7182f 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs @@ -798,7 +798,6 @@ public void TestPushOnExiting() AddAssert("Stack is not empty", () => stack.CurrentScreen != null); } - [Test] public void TestPushOnExitingWithoutBlocking() { From f3a5205619941a64068c1864eaa298d63e4eb20b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 23 Oct 2023 14:39:22 +0900 Subject: [PATCH 4/4] Use more correct exception type and text --- .../Visual/UserInterface/TestSceneScreenStack.cs | 2 +- osu.Framework/Screens/ScreenStack.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs index 6465b7182f..d8e75ce301 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneScreenStack.cs @@ -814,7 +814,7 @@ public void TestPushOnExitingWithoutBlocking() return screen1; }); - AddStep("ensure push throws", () => Assert.Throws( + AddStep("ensure push throws", () => Assert.Throws( () => screen1.Exit())); } diff --git a/osu.Framework/Screens/ScreenStack.cs b/osu.Framework/Screens/ScreenStack.cs index 525e3fc7a8..865944f41a 100644 --- a/osu.Framework/Screens/ScreenStack.cs +++ b/osu.Framework/Screens/ScreenStack.cs @@ -305,7 +305,7 @@ private bool exitFrom([CanBeNull] IScreen source, bool shouldFireExitEvent = tru return true; if (toExit != stack.Pop()) - throw new ScreenNotCurrentException("Cannot push to ScreenStack during exit without blocking the exit."); + throw new InvalidOperationException($"Cannot push to {nameof(ScreenStack)} during exit without blocking the exit."); } // we will probably want to change this logic when we support returning to a screen after exiting.