Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip the pause cooldown when in intro / break time #31143

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

peppy
Copy link
Member

@peppy peppy commented Dec 16, 2024

Had a quick look at adding test coverage in TestScenePause but the setup to get into either of these states seems a bit annoying..

Closes #31124.

Had a quick look at adding test coverage in `TestScenePause` but the
setup to get into either of these states seems a bit annoying..
@bdach
Copy link
Collaborator

bdach commented Dec 16, 2024

Change seems ok in empirical testing but I would like to have some automated here as well. What would you say to the following?:

diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs
index 6aa2c4e40d..7855c138ab 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs
@@ -12,6 +12,7 @@
 using osu.Framework.Logging;
 using osu.Framework.Screens;
 using osu.Framework.Testing;
+using osu.Game.Beatmaps;
 using osu.Game.Configuration;
 using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.Cursor;
@@ -19,6 +20,7 @@
 using osu.Game.Rulesets.UI;
 using osu.Game.Screens.Play;
 using osu.Game.Skinning;
+using osu.Game.Storyboards;
 using osuTK;
 using osuTK.Input;
 
@@ -28,6 +30,12 @@ public partial class TestScenePause : OsuPlayerTestScene
     {
         protected new PausePlayer Player => (PausePlayer)base.Player;
 
+        protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null)
+        {
+            beatmap.AudioLeadIn = 4000;
+            return base.CreateWorkingBeatmap(beatmap, storyboard);
+        }
+
         private readonly Container content;
 
         protected override Container<Drawable> Content => content;
@@ -202,6 +210,7 @@ public void TestUserPauseWhenPauseNotAllowed()
         [Test]
         public void TestUserPauseDuringCooldownTooSoon()
         {
+            AddStep("seek to gameplay", () => Player.GameplayClockContainer.Seek(0));
             AddStep("move cursor outside", () => InputManager.MoveMouseTo(Player.ScreenSpaceDrawQuad.TopLeft - new Vector2(10)));
 
             pauseAndConfirm();
@@ -213,9 +222,23 @@ public void TestUserPauseDuringCooldownTooSoon()
             confirmNotExited();
         }
 
+        [Test]
+        public void TestUserPauseDuringIntroSkipsCooldown()
+        {
+            AddStep("seek before gameplay", () => Player.GameplayClockContainer.Seek(-5000));
+            AddStep("move cursor outside", () => InputManager.MoveMouseTo(Player.ScreenSpaceDrawQuad.TopLeft - new Vector2(10)));
+
+            pauseAndConfirm();
+
+            resume();
+            pauseViaBackAction();
+            confirmPaused();
+        }
+
         [Test]
         public void TestQuickExitDuringCooldownTooSoon()
         {
+            AddStep("seek to gameplay", () => Player.GameplayClockContainer.Seek(0));
             AddStep("move cursor outside", () => InputManager.MoveMouseTo(Player.ScreenSpaceDrawQuad.TopLeft - new Vector2(10)));
 
             pauseAndConfirm();

(If you happen to run test browser and notice TestQuickExitDuringCooldownTooSoon failing, that happens on master as well, and I'm choosing to ignore it for the time being 🙈)

@peppy
Copy link
Member Author

peppy commented Dec 16, 2024

Sure that works for me.

bdach
bdach previously approved these changes Dec 16, 2024
@bdach
Copy link
Collaborator

bdach commented Dec 16, 2024

Aaaaand the other test randomly starts failing because github runners are made of loose straw and surrounding tree sticks and apparently the audio track can skip 300ms in a frame or something making all of this effort to test this pointless. That said I don't think my test changes specifically would break that.

Stick a [FlakyTest] on it and call it a day or what?

@frenzibyte
Copy link
Member

FlakyTest for now and demote to ignored if failure persists IMO.

@peppy
Copy link
Member Author

peppy commented Dec 20, 2024

time to ignore the test? 😅

@bdach bdach force-pushed the no-pause-cooldown-break-intro branch from 42ebbc6 to 3ec63d0 Compare December 20, 2024 10:22
@bdach bdach merged commit e62b329 into ppy:master Dec 20, 2024
9 of 10 checks passed
@peppy peppy deleted the no-pause-cooldown-break-intro branch December 22, 2024 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

can't pause immediately after unpausing when in intro/break
3 participants