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

Watch online statistics changes after every play & display them in toolbar #27156

Merged
merged 11 commits into from
Feb 14, 2024

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Feb 13, 2024

2024-02-13.15-07-57.mp4

Code 100% conflicts with #27128. And functionality possibly conflicts with #27147. Dunno. I publically said I was going to do anything with this before all that happened.

@frenzibyte frenzibyte self-requested a review February 14, 2024 04:31
Maybe slightly better readability. Dunno.
@peppy peppy self-requested a review February 14, 2024 15:11
@peppy
Copy link
Member

peppy commented Feb 14, 2024

Here's some changes which I think make things feel better:

diff --git a/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs b/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs
index 9960dd4411..52923ea178 100644
--- a/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs
+++ b/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs
@@ -127,7 +127,7 @@ private void load()
                             ValuePrefix = mainValuePrefix,
                             Anchor = Anchor.CentreRight,
                             Origin = Anchor.CentreRight,
-                            Font = OsuFont.GetFont(),
+                            Font = OsuFont.GetFont().With(fixedWidth: true),
                         },
                         new Container
                         {
@@ -140,7 +140,7 @@ private void load()
                                 {
                                     Anchor = Anchor.CentreRight,
                                     Origin = Anchor.CentreRight,
-                                    Font = OsuFont.GetFont(size: 12, fixedWidth: false, weight: FontWeight.SemiBold),
+                                    Font = OsuFont.GetFont(size: 12, fixedWidth: true, weight: FontWeight.SemiBold),
                                     AlwaysPresent = true,
                                 },
                                 titleText = new OsuSpriteText
@@ -183,11 +183,11 @@ public void Display(T before, T delta, T after)
                 titleText.Alpha = 1;
                 deltaValue.Alpha = 0;
 
-                using (BeginDelayedSequence(1500))
+                using (BeginDelayedSequence(1200))
                 {
-                    titleText.FadeOut(250, Easing.OutQuint);
-                    deltaValue.FadeIn(250, Easing.OutQuint)
-                              .Then().Delay(1500)
+                    titleText.FadeOut(250, Easing.OutQuad);
+                    deltaValue.FadeIn(250, Easing.OutQuad)
+                              .Then().Delay(500)
                               .Then().Schedule(() =>
                               {
                                   mainValue.Current.Value = after;
@@ -200,9 +200,9 @@ public void Display(T before, T delta, T after)
         private partial class Counter<T> : RollingCounter<T>
             where T : struct, IEquatable<T>, IFormattable
         {
-            public const double ROLLING_DURATION = 500;
+            public const double ROLLING_DURATION = 1500;
 
-            public FontUsage Font { get; init; } = OsuFont.Default;
+            public FontUsage Font { get; init; } = OsuFont.Default.With(fixedWidth: true);
 
             public string ValuePrefix
             {
@@ -217,7 +217,13 @@ public string ValuePrefix
             private string valuePrefix = string.Empty;
 
             protected override LocalisableString FormatCount(T count) => LocalisableString.Format(@"{0}{1:N0}", ValuePrefix, count);
-            protected override OsuSpriteText CreateSpriteText() => base.CreateSpriteText().With(t => t.Font = Font);
+
+            protected override OsuSpriteText CreateSpriteText() => base.CreateSpriteText().With(t =>
+            {
+                t.Font = Font;
+                t.Spacing = new Vector2(-1.5f, 0);
+            });
+
             protected override double RollingDuration => ROLLING_DURATION;
         }
     }

In addition, the Schedule used to delay the animation seems to break a bit if clicking the test steps multiple times. I'm not sure this will happen in practice, but maybe we want to cancel the last animation when a new value change event comes in to avoid this.

Copy link
Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

I post patch.

@@ -183,11 +183,11 @@ public void Display(T before, T delta, T after)
titleText.Alpha = 1;
deltaValue.Alpha = 0;

using (BeginDelayedSequence(1000))
using (BeginDelayedSequence(1500))
Copy link
Member

Choose a reason for hiding this comment

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

Feels a touch sluggish. 1200 maybe.

@bdach
Copy link
Collaborator Author

bdach commented Feb 14, 2024

the Schedule used to delay the animation seems to break a bit if clicking the test steps multiple times

Aware of this, which is why I attempted to initally try OnComplete() which was broken. So I don't know how one would even do this correctly at this rate.

@peppy
Copy link
Member

peppy commented Feb 14, 2024

Something like this probably:

diff --git a/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs b/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs
index 4d3c12c8be..91fe27050e 100644
--- a/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs
+++ b/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs
@@ -9,6 +9,7 @@
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Localisation;
+using osu.Framework.Threading;
 using osu.Game.Graphics;
 using osu.Game.Graphics.Sprites;
 using osu.Game.Graphics.UserInterface;
@@ -99,6 +100,8 @@ private partial class Statistic<T> : CompositeDrawable
             private Counter<T> deltaValue = null!;
             private OsuSpriteText titleText = null!;
 
+            private ScheduledDelegate? valueUpdateSchedule;
+
             [Resolved]
             private OsuColour colours { get; set; } = null!;
 
@@ -187,14 +190,17 @@ public void Display(T before, T delta, T after)
 
                 using (BeginDelayedSequence(1200))
                 {
-                    deltaValue.FadeIn(250, Easing.OutQuint)
-                              .Then().Delay(1500)
-                              .Then().Schedule(() =>
-                              {
-                                  mainValue.Current.Value = after;
-                                  deltaValue.Current.SetDefault();
-                              });
                     titleText.FadeOut(250, Easing.OutQuad);
+                    deltaValue.FadeIn(250, Easing.OutQuad);
+
+                    using (BeginDelayedSequence(1250))
+                    {
+                        valueUpdateSchedule = Schedule(() =>
+                        {
+                            mainValue.Current.Value = after;
+                            deltaValue.Current.SetDefault();
+                        });
+                    }
                 }
             }
         }

or just use a Scheduler.AddDelayed.

@bdach
Copy link
Collaborator Author

bdach commented Feb 14, 2024

Have applied, thanks.

@peppy peppy enabled auto-merge February 14, 2024 18:55
@peppy peppy merged commit ebea0d2 into ppy:master Feb 14, 2024
10 of 11 checks passed
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.

Online results changes should be watched (and shown) even when not at the results screen
3 participants