Merge pull request #17817 from peppy/fix-popup-dialog-handling-exit-sequence

Fix run-from-menu operations not working correctly when initiated from inside multiplayer

osu.Game.Tests/Visual/Navigation/TestScenePerformFromScreen.cs

@@ -171,6 +171,46 @@ namespace osu.Game.Tests.Visual.Navigation
             }
         }
 
+        [TestCase(true)]
+        [TestCase(false)]
+        public void TestPerformBlockedByDialogSubScreen(bool confirm)
+        {
+            TestScreenWithNestedStack screenWithNestedStack = null;
+
+            PushAndConfirm(() => screenWithNestedStack = new TestScreenWithNestedStack());
+
+            AddAssert("wait for nested screen", () => screenWithNestedStack.SubScreenStack.CurrentScreen == screenWithNestedStack.Blocker);
+
+            AddStep("try to perform", () => Game.PerformFromScreen(_ => actionPerformed = true));
+
+            AddUntilStep("wait for dialog", () => screenWithNestedStack.Blocker.ExitAttempts == 1);
+
+            AddWaitStep("wait a bit", 10);
+
+            AddUntilStep("wait for dialog display", () => Game.Dependencies.Get<DialogOverlay>().IsLoaded);
+
+            AddAssert("screen didn't change", () => Game.ScreenStack.CurrentScreen == screenWithNestedStack);
+            AddAssert("nested screen didn't change", () => screenWithNestedStack.SubScreenStack.CurrentScreen == screenWithNestedStack.Blocker);
+
+            AddAssert("did not perform", () => !actionPerformed);
+
+            AddAssert("only one exit attempt", () => screenWithNestedStack.Blocker.ExitAttempts == 1);
+
+            if (confirm)
+            {
+                AddStep("accept dialog", () => InputManager.Key(Key.Number1));
+                AddAssert("nested screen changed", () => screenWithNestedStack.SubScreenStack.CurrentScreen != screenWithNestedStack.Blocker);
+                AddUntilStep("did perform", () => actionPerformed);
+            }
+            else
+            {
+                AddStep("cancel dialog", () => InputManager.Key(Key.Number2));
+                AddAssert("screen didn't change", () => Game.ScreenStack.CurrentScreen == screenWithNestedStack);
+                AddAssert("nested screen didn't change", () => screenWithNestedStack.SubScreenStack.CurrentScreen == screenWithNestedStack.Blocker);
+                AddAssert("did not perform", () => !actionPerformed);
+            }
+        }
+
         private void importAndWaitForSongSelect()
         {
             AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).WaitSafely());
@@ -200,5 +240,30 @@ namespace osu.Game.Tests.Visual.Navigation
                 return base.OnExiting(next);
             }
         }
+
+        public class TestScreenWithNestedStack : OsuScreen, IHasSubScreenStack
+        {
+            public DialogBlockingScreen Blocker { get; private set; }
+
+            public ScreenStack SubScreenStack { get; } = new ScreenStack();
+
+            public TestScreenWithNestedStack()
+            {
+                AddInternal(SubScreenStack);
+
+                SubScreenStack.Push(Blocker = new DialogBlockingScreen());
+            }
+
+            public override bool OnExiting(IScreen next)
+            {
+                if (SubScreenStack.CurrentScreen != null)
+                {
+                    SubScreenStack.CurrentScreen.Exit();
+                    return true;
+                }
+
+                return base.OnExiting(next);
+            }
+        }
     }
 }

osu.Game/Overlays/Dialog/PopupDialog.cs

@@ -88,9 +88,13 @@ namespace osu.Game.Overlays.Dialog
                         if (actionInvoked) return;
 
                         actionInvoked = true;
-                        action?.Invoke();
 
+                        // Hide the dialog before running the action.
+                        // This is important as the code which is performed may check for a dialog being present (ie. `OsuGame.PerformFromScreen`)
+                        // and we don't want it to see the already dismissed dialog.
                         Hide();
+
+                        action?.Invoke();
                     };
                 }
             }

osu.Game/PerformFromMenuRunner.cs

@@ -97,11 +97,14 @@ namespace osu.Game
                 // if this has a sub stack, recursively check the screens within it.
                 if (current is IHasSubScreenStack currentSubScreen)
                 {
-                    if (findValidTarget(currentSubScreen.SubScreenStack.CurrentScreen))
+                    var nestedCurrent = currentSubScreen.SubScreenStack.CurrentScreen;
+
+                    if (nestedCurrent != null)
                     {
                         // should be correct in theory, but currently untested/unused in existing implementations.
-                        current.MakeCurrent();
+                        // note that calling findValidTarget actually performs the final operation.
-                        return true;
+                        if (findValidTarget(nestedCurrent))
+                            return true;
                     }
                 }
 
@@ -125,6 +128,18 @@ namespace osu.Game
         /// <returns>Whether a dialog blocked interaction.</returns>
         private bool checkForDialog(IScreen current)
         {
+            // An exit process may traverse multiple levels.
+            // When checking for dismissing dialogs, let's also consider sub screens.
+            while (current is IHasSubScreenStack currentWithSubScreenStack)
+            {
+                var nestedCurrent = currentWithSubScreenStack.SubScreenStack.CurrentScreen;
+
+                if (nestedCurrent == null)
+                    break;
+
+                current = nestedCurrent;
+            }
+
             var currentDialog = dialogOverlay.CurrentDialog;
 
             if (lastEncounteredDialog != null)