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

Add beat-synced animation to break overlay #29616

Merged
merged 7 commits into from
Sep 4, 2024

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Aug 27, 2024

Add beat-synced animation to break overlay

I've been meaning to make the progress bar synchronise with the beat rather than a continuous countdown, just to give the overlay a bit more of a rhythmic feel.

Not completely happy with how this feels but I think it's a start?

I had to refactor how the break overlay works in the process. It no longer creates transforms for all breaks ahead-of-time, which could be argued as a better way of doing things. It's more dynamically able to handle breaks now (maybe useful for the future, who knows).

osu.2024-08-27.at.07.39.17.mp4

I tried to make the arrows also animate but couldn't get them looking good. Open to suggestion:

diff --git a/osu.Game/Screens/Play/Break/BreakArrows.cs b/osu.Game/Screens/Play/Break/BreakArrows.cs
index 40474a7137..0c596825d9 100644
--- a/osu.Game/Screens/Play/Break/BreakArrows.cs
+++ b/osu.Game/Screens/Play/Break/BreakArrows.cs
@@ -21,11 +21,11 @@ public partial class BreakArrows : CompositeDrawable
         private const float blurred_icon_final_offset = 0.38f;
         private const float blurred_icon_offscreen_offset = 0.7f;
 
-        private readonly GlowIcon leftGlowIcon;
-        private readonly GlowIcon rightGlowIcon;
+        public readonly GlowIcon LeftInnerArrow;
+        public readonly GlowIcon RightInnerArrow;
 
-        private readonly BlurredIcon leftBlurredIcon;
-        private readonly BlurredIcon rightBlurredIcon;
+        public readonly BlurredIcon LeftOuterArrow;
+        public readonly BlurredIcon RightOuterArrow;
 
         public BreakArrows()
         {
@@ -37,7 +37,7 @@ public BreakArrows()
                     ParallaxAmount = -0.01f,
                     Children = new Drawable[]
                     {
-                        leftGlowIcon = new GlowIcon
+                        LeftInnerArrow = new GlowIcon
                         {
                             Anchor = Anchor.Centre,
                             Origin = Anchor.CentreRight,
@@ -46,7 +46,7 @@ public BreakArrows()
                             BlurSigma = new Vector2(glow_icon_blur_sigma),
                             Size = new Vector2(glow_icon_size),
                         },
-                        rightGlowIcon = new GlowIcon
+                        RightInnerArrow = new GlowIcon
                         {
                             Anchor = Anchor.Centre,
                             Origin = Anchor.CentreLeft,
@@ -62,7 +62,7 @@ public BreakArrows()
                     ParallaxAmount = -0.02f,
                     Children = new Drawable[]
                     {
-                        leftBlurredIcon = new BlurredIcon
+                        LeftOuterArrow = new BlurredIcon
                         {
                             Anchor = Anchor.Centre,
                             Origin = Anchor.CentreRight,
@@ -72,7 +72,7 @@ public BreakArrows()
                             BlurSigma = new Vector2(blurred_icon_blur_sigma),
                             Size = new Vector2(blurred_icon_size),
                         },
-                        rightBlurredIcon = new BlurredIcon
+                        RightOuterArrow = new BlurredIcon
                         {
                             Anchor = Anchor.Centre,
                             Origin = Anchor.CentreLeft,
@@ -89,20 +89,20 @@ public BreakArrows()
 
         public void Show(double duration)
         {
-            leftGlowIcon.MoveToX(-glow_icon_final_offset, duration, Easing.OutQuint);
-            rightGlowIcon.MoveToX(glow_icon_final_offset, duration, Easing.OutQuint);
+            LeftInnerArrow.MoveToX(-glow_icon_final_offset, duration, Easing.OutQuint);
+            RightInnerArrow.MoveToX(glow_icon_final_offset, duration, Easing.OutQuint);
 
-            leftBlurredIcon.MoveToX(-blurred_icon_final_offset, duration, Easing.OutQuint);
-            rightBlurredIcon.MoveToX(blurred_icon_final_offset, duration, Easing.OutQuint);
+            LeftOuterArrow.MoveToX(-blurred_icon_final_offset, duration, Easing.OutQuint);
+            RightOuterArrow.MoveToX(blurred_icon_final_offset, duration, Easing.OutQuint);
         }
 
         public void Hide(double duration)
         {
-            leftGlowIcon.MoveToX(-glow_icon_offscreen_offset, duration, Easing.OutQuint);
-            rightGlowIcon.MoveToX(glow_icon_offscreen_offset, duration, Easing.OutQuint);
+            LeftInnerArrow.MoveToX(-glow_icon_offscreen_offset, duration, Easing.OutQuint);
+            RightInnerArrow.MoveToX(glow_icon_offscreen_offset, duration, Easing.OutQuint);
 
-            leftBlurredIcon.MoveToX(-blurred_icon_offscreen_offset, duration, Easing.OutQuint);
-            rightBlurredIcon.MoveToX(blurred_icon_offscreen_offset, duration, Easing.OutQuint);
+            LeftOuterArrow.MoveToX(-blurred_icon_offscreen_offset, duration, Easing.OutQuint);
+            RightOuterArrow.MoveToX(blurred_icon_offscreen_offset, duration, Easing.OutQuint);
         }
     }
 }
diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs
index 7fc3dba3eb..0994bdd361 100644
--- a/osu.Game/Screens/Play/BreakOverlay.cs
+++ b/osu.Game/Screens/Play/BreakOverlay.cs
@@ -160,7 +160,33 @@ protected override void OnNewBeat(int beatIndex, TimingControlPoint timingPoint,
                 return;
 
             float timeBoxTargetWidth = (float)Math.Max(0, (remainingTimeForCurrentPeriod - timingPoint.BeatLength / currentPeriod.Value.Value.Duration));
-            remainingTimeBox.ResizeWidthTo(timeBoxTargetWidth, timingPoint.BeatLength * 2, Easing.OutQuint);
+            remainingTimeBox.ResizeWidthTo(timeBoxTargetWidth, timingPoint.BeatLength * 2, Easing.OutSine);
+
+            double arrowDuration = timingPoint.BeatLength / 2f;
+            const float arrow_move_amount = 5;
+
+            if (beatIndex % 2 == 0)
+            {
+                breakArrows.LeftInnerArrow
+                           .MoveToY(-arrow_move_amount, arrowDuration, Easing.OutSine)
+                           .Then()
+                           .MoveToY(arrow_move_amount, arrowDuration, Easing.InSine);
+                breakArrows.RightInnerArrow
+                           .MoveToY(-arrow_move_amount, arrowDuration, Easing.OutSine)
+                           .Then()
+                           .MoveToY(arrow_move_amount, arrowDuration, Easing.InSine);
+            }
+            else
+            {
+                breakArrows.LeftOuterArrow
+                           .MoveToY(-arrow_move_amount, arrowDuration, Easing.OutSine)
+                           .Then()
+                           .MoveToY(arrow_move_amount, arrowDuration, Easing.InSine);
+                breakArrows.RightOuterArrow
+                           .MoveToY(-arrow_move_amount, arrowDuration, Easing.OutSine)
+                           .Then()
+                           .MoveToY(arrow_move_amount, arrowDuration, Easing.InSine);
+            }
         }
 
         private void updateDisplay(ValueChangedEvent<Period?> period)

I've been meaning to make the progress bar synchronise with the beat
rather than a continuous countdown, just to give the overlay a bit more
of a rhythmic feel.

Not completely happy with how this feels but I think it's a start?

I had to refactor how the break overlay works in the process. It no
longer creates transforms for all breaks ahead-of-time, which could be
argued as a better way of doing things. It's more dynamically able to
handle breaks now (maybe useful for the future, who knows).
@peppy
Copy link
Sponsor Member Author

peppy commented Aug 27, 2024

Note that I haven't throughly tested rewind, but I couldn't seem to break it..

@bdach
Copy link
Collaborator

bdach commented Aug 27, 2024

I'm not sure if it's the "used to it" effect but this feels like... too much?

Out of all the three things going on with the animation, in order of preference:

  • I dig the beat-synced progress bar, no objections there
  • Not super sure on the parallax, but I guess it works
  • The thing that screws me up the most here is the lateral movement of the text. It just... looks wrong, like everything is in the wrong places. With the centre-aligned progress bar things moving to the left and right just doesn't seem correct. I'd either scrap it or try vertical movement instead?

@peppy
Copy link
Sponsor Member Author

peppy commented Aug 27, 2024

Maybe just having it as an initial horizontal move when it first appears rather than constant will be better, I'll give that a try.

@smoogipoo
Copy link
Contributor

I really don't like the parallax. I don't think the design really works for parallax in general, but the way master applies it to only the arrows makes a lot more sense in my head. Same for the lateral movement of the text. I think it all looks too float-y.

In-fact I didn't even realise the bar was beat syncing in the video because all I could look at was the various components moving around.

@peppy
Copy link
Sponsor Member Author

peppy commented Aug 28, 2024

Sure, I'll remove the parallax as well.

@bdach bdach self-requested a review August 30, 2024 11:24
@bdach
Copy link
Collaborator

bdach commented Aug 30, 2024

Something here is very wrong with pausing:

2024-08-30.13-33-52.mp4

return;

float timeBoxTargetWidth = (float)Math.Max(0, (remainingTimeForCurrentPeriod - timingPoint.BeatLength / currentPeriod.Value.Value.Duration));
remainingTimeBox.ResizeWidthTo(timeBoxTargetWidth, timingPoint.BeatLength * 2, Easing.OutQuint);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't get why this transform has duration of two beat lengths? Won't that cause overlapping transforms if this fires on every beat?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it's just to make it really smooth (looks weird if it comes to a full stop).

@peppy
Copy link
Sponsor Member Author

peppy commented Sep 2, 2024

Something here is very wrong with pausing

Nothing's really wrong, it's just the RemoveCompletedTransforms => false flag. It fixes itself at next display.

I tried to remove this but had some issues. I guess I can try once more.

@peppy
Copy link
Sponsor Member Author

peppy commented Sep 2, 2024

If you're okay with a local fix, this works for that specific case:

diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs
index 7f9e879b44..f2a2a3ed12 100644
--- a/osu.Game/Screens/Play/BreakOverlay.cs
+++ b/osu.Game/Screens/Play/BreakOverlay.cs
@@ -155,6 +155,7 @@ protected override void OnNewBeat(int beatIndex, TimingControlPoint timingPoint,
                 return;
 
             float timeBoxTargetWidth = (float)Math.Max(0, (remainingTimeForCurrentPeriod - timingPoint.BeatLength / currentPeriod.Value.Value.Duration));
+            remainingTimeBox.ClearTransforms(targetMember: nameof(Width));
             remainingTimeBox.ResizeWidthTo(timeBoxTargetWidth, timingPoint.BeatLength * 2, Easing.OutQuint);
         }

@bdach bdach self-requested a review September 2, 2024 09:45
@bdach
Copy link
Collaborator

bdach commented Sep 2, 2024

That diff fixes nothing for me. A start to fixing the actual issue is this:

diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs
index 7f9e879b44..c161d4982b 100644
--- a/osu.Game/Screens/Play/BreakOverlay.cs
+++ b/osu.Game/Screens/Play/BreakOverlay.cs
@@ -52,6 +52,7 @@ public BreakOverlay(bool letterboxing, ScoreProcessor scoreProcessor)
             RelativeSizeAxes = Axes.Both;
 
             MinimumBeatLength = 200;
+            AllowMistimedEventFiring = false;
 
             Child = fadeContainer = new Container
             {

tl;dr: The code as you have it is relying on transforms being added at the exact beat to have the effect desired. Pausing and unpausing the track causes OnNewBeat() to fire out-of-sync, therefore breaking that constraint and just basically causing undefined stuff to happen because it's basically adding new transforms that are not snapped to a beat and breaking the effect.

That said the above is still pretty bad because it doesn't work properly with rewind. I'm not even sure I know how to write this properly. I tried

diff --git a/osu.Game/Screens/Play/BreakOverlay.cs b/osu.Game/Screens/Play/BreakOverlay.cs
index 7f9e879b44..d53e4a5a96 100644
--- a/osu.Game/Screens/Play/BreakOverlay.cs
+++ b/osu.Game/Screens/Play/BreakOverlay.cs
@@ -10,6 +10,7 @@
 using osu.Framework.Graphics.Effects;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.UserInterface;
+using osu.Framework.Logging;
 using osu.Game.Beatmaps.ControlPoints;
 using osu.Game.Beatmaps.Timing;
 using osu.Game.Graphics;
@@ -137,8 +138,8 @@ protected override void LoadComplete()
             currentPeriod.BindValueChanged(updateDisplay, true);
         }
 
-        private float remainingTimeForCurrentPeriod =>
-            currentPeriod.Value == null ? 0 : (float)Math.Max(0, (currentPeriod.Value.Value.End - Time.Current - BREAK_FADE_DURATION) / currentPeriod.Value.Value.Duration);
+        private float remainingTimeForCurrentPeriod(double time) =>
+            currentPeriod.Value == null ? 0 : (float)Math.Max(0, (currentPeriod.Value.Value.End - time - BREAK_FADE_DURATION) / currentPeriod.Value.Value.Duration);
 
         protected override void Update()
         {
@@ -154,7 +155,9 @@ protected override void OnNewBeat(int beatIndex, TimingControlPoint timingPoint,
             if (currentPeriod.Value == null)
                 return;
 
-            float timeBoxTargetWidth = (float)Math.Max(0, (remainingTimeForCurrentPeriod - timingPoint.BeatLength / currentPeriod.Value.Value.Duration));
+            double beatTime = timingPoint.Time + beatIndex * timingPoint.BeatLength;
+            Logger.Log($"beattime = {beatTime} current = {Time.Current}");
+            float timeBoxTargetWidth = (float)Math.Max(0, (remainingTimeForCurrentPeriod(beatTime) - timingPoint.BeatLength / currentPeriod.Value.Value.Duration));
             remainingTimeBox.ResizeWidthTo(timeBoxTargetWidth, timingPoint.BeatLength * 2, Easing.OutQuint);
         }
 
@@ -178,7 +181,7 @@ private void updateDisplay(ValueChangedEvent<Period?> period)
                     .Delay(b.Duration - BREAK_FADE_DURATION)
                     .ResizeWidthTo(0);
 
-                remainingTimeBox.ResizeWidthTo(remainingTimeForCurrentPeriod);
+                remainingTimeBox.ResizeWidthTo(remainingTimeForCurrentPeriod(Time.Current));
 
                 remainingTimeCounter.CountTo(b.Duration).CountTo(0, b.Duration);
 

and it still bugs out because apparently when unpausing BeatSyncedContainer is allowed to go backwards time-wise even when time is progressing...?

[runtime] 2024-09-02 10:22:22 [verbose]: GameplayClockContainer started via call to StartGameplayClock
[runtime] 2024-09-02 10:22:22 [verbose]: beattime = 93120 current = 93364.46661317158
[runtime] 2024-09-02 10:22:23 [verbose]: GameplayClockContainer stopped via call to StopGameplayClock
[runtime] 2024-09-02 10:22:23 [verbose]: beattime = 93000 current = 93537.90891942769
[runtime] 2024-09-02 10:22:23 [verbose]: GameplayClockContainer started via call to StartGameplayClock
[runtime] 2024-09-02 10:22:23 [verbose]: beattime = 93120 current = 93580.29183673469

@peppy
Copy link
Sponsor Member Author

peppy commented Sep 2, 2024

Can you clarify what you want to fix specifically? I don't see the animation being wrong for one beat a huge deal. The fix I proposed was to stop the transforms from piling up, which I thought is what you had an issue with.

Also without AllowMistimedEventFiring it could skip beats in the worst case scenario, which I'm not sure we want.

@bdach
Copy link
Collaborator

bdach commented Sep 2, 2024

I want to stop the progress bar jittering back and forth in width when pausing and unpausing, in a manner that in no way is consistent with the normal animation when pausing is not involved.

Or, in other words, the progress bar should never become longer on unpause.

@peppy
Copy link
Sponsor Member Author

peppy commented Sep 3, 2024

I've applied a very basic fix which seems to look quite good to me, what do you think?

@peppy peppy added the next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! label Sep 4, 2024
@bdach
Copy link
Collaborator

bdach commented Sep 4, 2024

It's still not fixing what I wanted to fix because it's doing something about rewind and not pause. But at this point I'm not sure how to fix it myself either so I might just ignore and move on and wait for the inevitable issue report.

@peppy
Copy link
Sponsor Member Author

peppy commented Sep 4, 2024

Ah, I missed that it was during pause. I've just disabled the mistimed firing flag for now.

@peppy peppy merged commit ca09028 into ppy:master Sep 4, 2024
10 of 13 checks passed
@peppy peppy deleted the break-overlay-animation branch September 5, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:gameplay next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants