Skip to content

Use correct HitWindows class for osu!taiko hit windows in difficulty calculator#31579

Merged
tsunyoku merged 6 commits intoppy:pp-devfrom
tsunyoku:use-correct-taiko-hit-windows
Jan 20, 2025
Merged

Use correct HitWindows class for osu!taiko hit windows in difficulty calculator#31579
tsunyoku merged 6 commits intoppy:pp-devfrom
tsunyoku:use-correct-taiko-hit-windows

Conversation

@tsunyoku
Copy link
Member

Caught whilst porting over changes to osu!stable. This causes a noticable difference in the hit windows passed to the Rhythm skill as Taiko hit windows are not the same as the default provided in the HitWindows class.

Leaving it up to @ppy/taiko-pp-commitee to figure out what this means for their changes, but this is a non-negotiable fix.

@tsunyoku tsunyoku requested review from a team January 19, 2025 19:18
@tsunyoku tsunyoku requested review from a team and removed request for a team January 19, 2025 19:24
@stanriders
Copy link
Member

I'll start a sheet calculation when we get #31546 and #31556 merged

@tsunyoku
Copy link
Member Author

!diffcalc
RULESET=taiko
OSU_A=https://github.com/ppy/osu/tree/pp-dev
OSU_B=#31579

@github-actions
Copy link

@tsunyoku
Copy link
Member Author

!diffcalc
RULESET=taiko
OSU_A=https://github.com/ppy/osu/tree/pp-dev
OSU_B=#31579

@github-actions
Copy link

Copy link
Member

@Lawtrohux Lawtrohux left a comment

Choose a reason for hiding this comment

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

Good catch. Changes are all reasonable or non-consequential

@tsunyoku tsunyoku enabled auto-merge (squash) January 20, 2025 12:03
Copy link
Member

@stanriders stanriders left a comment

Choose a reason for hiding this comment

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

Good catch

@tsunyoku tsunyoku merged commit a77dfb1 into ppy:pp-dev Jan 20, 2025
8 of 9 checks passed
@tsunyoku tsunyoku moved this from Pending Deploy to Deployed in Difficulty calculation changes Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Deployed

Development

Successfully merging this pull request may close these issues.

3 participants