From b3c2d120bf8e8d6e60e6a618f1df976d82b6d406 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 25 May 2023 20:39:11 +0900 Subject: [PATCH 1/3] Fix `OnResume` / `OnSuspending` potentially getting called before `OnEntering` on a sub screen --- osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs index 3d80248306..cfaae56d0f 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs @@ -132,7 +132,9 @@ namespace osu.Game.Screens.OnlinePlay this.ScaleTo(1, 250, Easing.OutSine); Debug.Assert(screenStack.CurrentScreen != null); - screenStack.CurrentScreen.OnResuming(e); + + if (screenStack.CurrentScreen.IsCurrentScreen()) + screenStack.CurrentScreen.OnResuming(e); base.OnResuming(e); } @@ -143,7 +145,9 @@ namespace osu.Game.Screens.OnlinePlay this.FadeOut(250); Debug.Assert(screenStack.CurrentScreen != null); - screenStack.CurrentScreen.OnSuspending(e); + + if (screenStack.CurrentScreen.IsCurrentScreen()) + screenStack.CurrentScreen.OnSuspending(e); } public override bool OnExiting(ScreenExitEvent e) From 741ee84ed6f14bc8b4e0bf7f073698d4319399c6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 May 2023 18:38:28 +0900 Subject: [PATCH 2/3] Add comment and use full conditional similar to extension method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs index cfaae56d0f..50d1d1addd 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs @@ -133,7 +133,10 @@ namespace osu.Game.Screens.OnlinePlay Debug.Assert(screenStack.CurrentScreen != null); - if (screenStack.CurrentScreen.IsCurrentScreen()) + // if a subscreen was pushed to the nested stack while this screen was not present, this path will proxy `OnResuming()` + // to the subscreen before `OnEntering()` can even be called for the subscreen, breaking ordering expectations. + // to work around this, do not proxy resume to screens that haven't loaded yet. + if ((screenStack.CurrentScreen as Drawable)?.IsLoaded == true) screenStack.CurrentScreen.OnResuming(e); base.OnResuming(e); From 62d177440730e507ff3b080dd8c1c1eca66ea6bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 30 May 2023 21:54:56 +0200 Subject: [PATCH 3/3] Apply same comment & reworded condition to suspend too --- osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs index 50d1d1addd..37b50b4863 100644 --- a/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs +++ b/osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs @@ -133,7 +133,7 @@ namespace osu.Game.Screens.OnlinePlay Debug.Assert(screenStack.CurrentScreen != null); - // if a subscreen was pushed to the nested stack while this screen was not present, this path will proxy `OnResuming()` + // if a subscreen was pushed to the nested stack while the stack was not present, this path will proxy `OnResuming()` // to the subscreen before `OnEntering()` can even be called for the subscreen, breaking ordering expectations. // to work around this, do not proxy resume to screens that haven't loaded yet. if ((screenStack.CurrentScreen as Drawable)?.IsLoaded == true) @@ -149,7 +149,10 @@ namespace osu.Game.Screens.OnlinePlay Debug.Assert(screenStack.CurrentScreen != null); - if (screenStack.CurrentScreen.IsCurrentScreen()) + // if a subscreen was pushed to the nested stack while the stack was not present, this path will proxy `OnSuspending()` + // to the subscreen before `OnEntering()` can even be called for the subscreen, breaking ordering expectations. + // to work around this, do not proxy suspend to screens that haven't loaded yet. + if ((screenStack.CurrentScreen as Drawable)?.IsLoaded == true) screenStack.CurrentScreen.OnSuspending(e); }