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 LegacyApproachCircle incorrectly applying scaling factor #27286

Merged
merged 2 commits into from Feb 21, 2024

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Feb 21, 2024

Simple way of explaining this: it was only supposed to be applied to default skins, but was incorrectly applied everywhere. This was likely an oversight due to the implementation existing in LegacyApproachCircle. But it was in the CreateDefault method which is not used because there's always an approachcircle sprite.

// account for the sprite being used for the default approach circle being taken from stable,
// when hitcircles have 5px padding on each size. this should be removed if we update the sprite.
drawable.Scale = new Vector2(128 / 118f);

Here's the longer version, which I've also updated the inline comment to:

In triangles and argon, we expanded hitcircles to take up the full 128 px which are clickable, but still use the old approach circle sprite. To make it feel correct (ie. disappear as it collides with the hitcircle, not when it overlaps the border) we need to expand it slightly.

This is what the scale adjust exists to do.

Note that this only affects classic and legacy skins.

Before After
2024-02-21 13 49 47@2x 2024-02-21 13 50 27@2x
2024-02-21 13 47 25@2x 2024-02-21 13 45 42@2x
2024-02-21 13 46 56@2x 2024-02-21 13 45 28@2x
2024-02-21 13 46 39@2x 2024-02-21 13 45 50@2x

Closes #27283.

@peppy peppy requested a review from a team February 21, 2024 05:52
@smoogipoo smoogipoo merged commit 0c35950 into ppy:master Feb 21, 2024
13 of 17 checks passed
@peppy peppy deleted the fix-approach-circle branch February 24, 2024 02:44
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.

Approach circle doesnt hit the hitcircle on the perfect tap moment on imported skins
2 participants