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

Fix toolbar PP change showing +0 instead of 0 #29512

Merged
merged 2 commits into from
Aug 22, 2024

Conversation

peppy
Copy link
Member

@peppy peppy commented Aug 19, 2024

Before After
osu Game Tests 2024-08-19 at 11 52 22 osu Game Tests 2024-08-19 at 11 52 03

smoogipoo
smoogipoo previously approved these changes Aug 19, 2024
@bdach bdach self-requested a review August 19, 2024 12:10
@@ -83,7 +83,7 @@ protected override void LoadComplete()
}

if (update.After.PP != null)
pp.Display(update.Before.PP ?? update.After.PP.Value, Math.Abs((update.After.PP - update.Before.PP) ?? 0M), update.After.PP.Value);
pp.Display((int)(update.Before.PP ?? update.After.PP.Value), (int)Math.Abs((update.After.PP - update.Before.PP) ?? 0M), (int)update.After.PP.Value);
Copy link
Collaborator

@bdach bdach Aug 19, 2024

Choose a reason for hiding this comment

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

You can still have green +0 show with this because flooring/truncating thrice separately is not guaranteed to give you consistent results.

Example: User goes from $1357.6$ total pp to $1358.1$ total pp. The value before floored is $1357$, the value after floored is $1358$, but the delta is $1358.1 - 1357.6 = 0.5$ which will get floored to 0. Thus you get a point of pp increase shown but a green +0 on the delta.

Can be crudely tested with

diff --git a/osu.Game.Tests/Visual/Menus/TestSceneToolbarUserButton.cs b/osu.Game.Tests/Visual/Menus/TestSceneToolbarUserButton.cs
index a81c940d82..3d18dc5ebc 100644
--- a/osu.Game.Tests/Visual/Menus/TestSceneToolbarUserButton.cs
+++ b/osu.Game.Tests/Visual/Menus/TestSceneToolbarUserButton.cs
@@ -137,12 +137,12 @@ public void TestTransientUserStatisticsDisplay()
                     new UserStatistics
                     {
                         GlobalRank = 111_111,
-                        PP = 1357
+                        PP = 1357.6m
                     },
                     new UserStatistics
                     {
                         GlobalRank = 111_111,
-                        PP = 1357.1m
+                        PP = 1358.1m
                     });
             });
             AddStep("Was null", () =>

Copy link
Member

Choose a reason for hiding this comment

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

We probably want to do (int)after - (int)before on the second argument.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably fine.

@bdach bdach merged commit a32a817 into ppy:master Aug 22, 2024
11 of 13 checks passed
@peppy peppy deleted the toolbar-pp-change-weird branch August 23, 2024 09:56
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.

4 participants