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

Use consistent decimal places in BeatmapAttributeText #30802

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

cloudrac3r
Copy link
Contributor

Currently, BeatmapAttributeText will always display 2 decimal places, possibly with trailing zeroes, for attributes that normally don't have trailing zeroes.

In most of the game, and on web, difficulty attributes are displayed with one decimal place, but can be displayed as whole numbers if there are no decimals e.g. (CS 3.8, HP 5.5, Accuracy 9, AR 9.2) for https://osu.ppy.sh/beatmapsets/1314891#osu/2725039. If mods are applied, difficulty attributes will be displayed with up to 2 decimal places. This can be seen by turning on DT then looking at the top left section of lazer song select. So I have changed the difficulty attribute format to 0.##.

The same logic applies for BPM. Usually it's displayed as a whole number, but it can have up to 2 decimal places if necessary: https://osu.ppy.sh/beatmapsets/2268670#fruits/4831566. So I changed this format to 0.## as well.

I have not changed the display of star rating because it's always 2 decimal places with trailing zeroes: https://osu.ppy.sh/beatmapsets/2253429#osu/4794131

I tested this manually and it seems to work. I'm not sure if there's any automated tests that need to be updated.

smoogipoo
smoogipoo previously approved these changes Nov 21, 2024
Copy link
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

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

Seems ok to me

@smoogipoo
Copy link
Contributor

Tests need adjustments here.

@bdach
Copy link
Collaborator

bdach commented Nov 21, 2024

Have fixed the tests.

Two things to @cloudrac3r:

  • I'll forgive making the commit on github.com this time, but for anything more serious, please don't.

  • Either github tooling has potentially terminal breakage for whatever reason, or something somewhere with your git setup is borked. I initially couldn't push to this branch to fix the tests because of whatever this is:

    remote: Resolving deltas: 100% (14/14), completed with 10 local objects.
    To github.com:OpenSauce04/osu.git
     ! [remote rejected]       patch-1 -> patch-1 (permission denied)
    error: failed to push some refs to 'github.com:OpenSauce04/osu.git'
    

    I cloned the branch using github's own gh, so I dunno what is up with whatever that is. A manual edit in .git/config to point at the correct remote fixed this.

@cloudrac3r
Copy link
Contributor Author

Username OpenSauce04 is not my account, so it's strange that you saw that error. I've taken your advice for next time. Thank you for fixing the tests!

@bdach bdach merged commit 23ef8fd into ppy:master Nov 21, 2024
9 of 10 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.

3 participants