ppy
Adjust hitwindows to match stable
This is fixing https://github.com/ppy/osu/issues/11311 and https://github.com/ppy/osu/issues/28744 Unfortunately all already set replays can't be fixed
From my testing - Standard and Taiko work correctly Mania requires additional testing
Just for reference for anyone reading https://github.com/ppy/osu/pull/26452 also exists, probably was tested way better than this is, but is also likely overcomplicated.
As touched on in https://github.com/ppy/osu/issues/11311#issuecomment-2076636500, I'm not making a shot call on this alone without a serious talk within @ppy/team-client on how we wanna proceed with the problem at hand. That said this PR needs to be fully tested (see op comment about mania) before I'd be willing to review it.
I think the title could be changed to better reflect the purpose of this pr. Based on the discussion on discord, I'm assuming the main goal here is to match lazer gameplay with lazer replays, not to match stable hitwindows.
Matching stable hitwindows should be considered a different issue that doesn't need as much urgency imo.
I think the title could be changed to better reflect the purpose of this pr. Based on the discussion on discord, I'm assuming the main goal here is to match lazer gameplay with lazer replays, not to match stable hitwindows.
Matching stable hitwindows should be considered a different issue that doesn't need as much urgency imo.
Title shows exactly what this PR does - changing hitwindows to match stable Replays in lazer are encoded the same way stable replays are encoded And this creates a desync between replays and gameplay By making hitwindows as in stable - you're fixing lazer replays and stable replays being wrong at the same time
Additional note
Right now: ScoreInfo is correct from lazer perspective and incorrect from stable perspective. Replay is incorrect from any perspective.
After this change: ScoreInfo of old lazer replays (pre-change) will be plainly incorrect as they were judged on different hitwindows. Replay will be correct from both lazer and stable perspectives.
This means that rerunned old lazer replays will output correct judgements.
By making hitwindows as in stable - you're fixing lazer replays and stable replays being wrong at the same time
Don't think it's a good idea to bring in the complexity of matching stable hitwindows for a quickfix to lazer replays. Looking at the table in https://github.com/ppy/osu/issues/11311#issuecomment-1407488892, stable hitwindows are very inconsistent between modes. And I don't know if it's a good idea to have lazer replicate this inconsistency. Not to mention the lazer hitwindow for mania Perfect is intentionally changed to be different from stable.
If you still want to match stable hitwindows, here are some test replays that are all edge hits where timeOffset == HitWindow
The plays were set in stable, then played back in lazer.
Beatmap: circles.osz
| Replay | result (stable) | result (master) | result (PR) | hitwindow (stable) | hitwindow (master) | hitwindow (PR) |
|---|---|---|---|---|---|---|
| osu100.osr | 100 | 300 | 100 | < 21 | <= 21.2 | <= 20.5 |
| osu50.osr | 50 | 100 | 100 | < 61 | <= 61.6 | <= 61.5 |
| osumiss.osr | miss | 50 | 50 | < 101 | <= 102 | <= 101.5 |
| taiko100.osr | 100 | 300 | 300 | < 20 | <= 20.6 | <= 20.5 |
| taikomiss.osr | miss | 100 | miss | < 51 | <= 51.2 | <= 50.5 |
| taikomissearly.osr | miss | miss | miss | <= 70 | <= 71 | <= 70.5 |
| mania300.osr | 300 | 300 | 300 | <= 34 | <= 34.6 | <= 34.5 |
| mania200.osr | 200 | 200 | 200 | <= 67 | <= 67.6 | <= 67.5 |
By making hitwindows as in stable - you're fixing lazer replays and stable replays being wrong at the same time
Don't think it's a good idea to bring in the complexity of matching stable hitwindows for a quickfix to lazer replays. Looking at the table in #11311 (comment), stable hitwindows are very inconsistent between modes. And I don't know if it's a good idea to have lazer replicate this inconsistency. Not to mention the lazer hitwindow for mania Perfect is intentionally changed to be different from stable.
If you still want to match stable hitwindows, here are some test replays that are all edge hits where
timeOffset == HitWindowThe plays were set in stable, then played back in lazer.Beatmap: circles.osz
Replay result (stable) result (master) result (PR) hitwindow (stable) hitwindow (master) hitwindow (PR) osu100.osr 100 300 100 < 21 <= 21.2 <= 20.5 osu50.osr 50 100 100 < 61 <= 61.6 <= 61.5 osumiss.osr miss 50 50 < 101 <= 102 <= 101.5 taiko100.osr 100 300 300 < 20 <= 20.6 <= 20.5 taikomiss.osr miss 100 miss < 51 <= 51.2 <= 50.5 taikomissearly.osr miss miss miss <= 70 <= 71 <= 70.5 mania300.osr 300 300 300 <= 34 <= 34.6 <= 34.5 mania200.osr 200 200 200 <= 67 <= 67.6 <= 67.5
Actually any hitwindows that make timings end in .5 will fix the main problem - desync between gameplay and replay. It's just would be convenient to also fix stable replays being wrong. I will look into a formula more precisely.
Mania obviously can't be fixed in this way.
If you still want to match stable hitwindows, here are some test replays that are all edge hits where
timeOffset == HitWindowThe plays were set in stable, then played back in lazer.
I fixed it and now your tests pass.
Rechecked the hitwindows just to make sure and they seem good to me. I did get the hitwindows slightly wrong for master though, since I didn't account for the float to double conversion.
| Replay | hitwindow (stable) | hitwindow (master) | hitwindow (PR) |
|---|---|---|---|
| osu100.osr | < 21 | <= 21.1999988555908 | <= 20.5 |
| osu50.osr | < 61 | <= 61.5999984741211 | <= 60.5 |
| osumiss.osr | < 101 | <= 101.999998092651 | <= 100.5 |
| taiko100.osr | < 20 | <= 20.5999994277954 | <= 19.5 |
| taikomiss.osr | < 51 | <= 51.1999988555908 | <= 50.5 |
| taikomissearly.osr | <= 70 | <= 70.9999990463257 | <= 70.5 |
| mania300.osr | <= 34 | <= 34.5999994277954 | <= 34.5 |
| mania200.osr | <= 67 | <= 67.5999994277954 | <= 67.5 |
Also, BIG warning - this changes pp outputs because pp is converting OD to hitwindows
I would like to comment on newly added tests in #32770 and #32810.
Considering that the TestSceneReplayStability test cases are created for current hitwindows - they're failing, as this PR changes hitwindows. But the TestSceneLegacyReplayPlayback ones for osu and taiko passing as expected.
Mania legacy tests are failing because their overly-complicated behavior was not replicated in this PR. But considering that hitwindows are now rounded - they're consistent with it's own replays. I would like to fix mania to be also 1-to-1, but I would like to hear what approach I should take for problem of converts having different hitwindows without scorev2.
@Givikap120 can you leave the matter of this PR to me and the rest of the team? thanks, appreciated in advance.