Merge pull request #17403 from peppy/editor-exit-harsh-blocking

Disallow exiting the editor without saving (unless explicitly confirming)

osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs

@@ -17,6 +17,13 @@ namespace osu.Game.Tests.Visual.Editing
 {
     public class TestSceneEditorSaving : EditorSavingTestScene
     {
+        [Test]
+        public void TestCantExitWithoutSaving()
+        {
+            AddRepeatStep("Exit", () => InputManager.Key(Key.Escape), 10);
+            AddAssert("Editor is still active screen", () => Game.ScreenStack.CurrentScreen is Editor);
+        }
+
         [Test]
         public void TestMetadata()
         {

osu.Game/Screens/Edit/Editor.cs

@@ -28,7 +28,6 @@ using osu.Game.Graphics.UserInterface;
 using osu.Game.Input.Bindings;
 using osu.Game.Online.API;
 using osu.Game.Overlays;
-using osu.Game.Overlays.Dialog;
 using osu.Game.Overlays.Notifications;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Edit;
@@ -98,7 +97,7 @@ namespace osu.Game.Screens.Edit
 
         private bool canSave;
 
-        private bool exitConfirmed;
+        protected bool ExitConfirmed { get; private set; }
 
         private string lastSavedHash;
 
@@ -587,7 +586,7 @@ namespace osu.Game.Screens.Edit
 
         public override bool OnExiting(IScreen next)
         {
-            if (!exitConfirmed)
+            if (!ExitConfirmed)
             {
                 // dialog overlay may not be available in visual tests.
                 if (dialogOverlay == null)
@@ -596,12 +595,9 @@ namespace osu.Game.Screens.Edit
                     return true;
                 }
 
-                // if the dialog is already displayed, confirm exit with no save.
+                // if the dialog is already displayed, block exiting until the user explicitly makes a decision.
-                if (dialogOverlay.CurrentDialog is PromptForSaveDialog saveDialog)
+                if (dialogOverlay.CurrentDialog is PromptForSaveDialog)
-                {
-                    saveDialog.PerformAction<PopupDialogDangerousButton>();
                     return true;
-                }
 
                 if (isNewBeatmap || HasUnsavedChanges)
                 {
@@ -646,7 +642,7 @@ namespace osu.Game.Screens.Edit
         {
             Save();
 
-            exitConfirmed = true;
+            ExitConfirmed = true;
             this.Exit();
         }
 
@@ -669,7 +665,7 @@ namespace osu.Game.Screens.Edit
                 Beatmap.SetDefault();
             }
 
-            exitConfirmed = true;
+            ExitConfirmed = true;
             this.Exit();
         }
 

osu.Game/Tests/Visual/EditorTestScene.cs

@@ -7,10 +7,13 @@ using osu.Framework.Allocation;
 using osu.Framework.Audio;
 using osu.Framework.IO.Stores;
 using osu.Framework.Platform;
+using osu.Framework.Screens;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
 using osu.Game.Database;
 using osu.Game.Online.API;
+using osu.Game.Overlays;
+using osu.Game.Overlays.Dialog;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Edit;
 using osu.Game.Screens.Edit;
@@ -93,6 +96,10 @@ namespace osu.Game.Tests.Visual
 
         protected class TestEditor : Editor
         {
+            [Resolved(canBeNull: true)]
+            [CanBeNull]
+            private DialogOverlay dialogOverlay { get; set; }
+
             public new void Undo() => base.Undo();
 
             public new void Redo() => base.Redo();
@@ -111,6 +118,18 @@ namespace osu.Game.Tests.Visual
 
             public new bool HasUnsavedChanges => base.HasUnsavedChanges;
 
+            public override bool OnExiting(IScreen next)
+            {
+                // For testing purposes allow the screen to exit without saving on second attempt.
+                if (!ExitConfirmed && dialogOverlay?.CurrentDialog is PromptForSaveDialog saveDialog)
+                {
+                    saveDialog.PerformAction<PopupDialogDangerousButton>();
+                    return true;
+                }
+
+                return base.OnExiting(next);
+            }
+
             public TestEditor(EditorLoader loader = null)
                 : base(loader)
             {