ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.32k stars 2.28k forks source link

Ranked convert map with low SV not converting correctly in taiko and mania #17143

Closed smoogipoo closed 2 years ago

smoogipoo commented 2 years ago

Discussed in https://github.com/ppy/osu/discussions/17120

Originally posted by **zachmanthethird** March 6, 2022 Originally mentioned in #15747 and #15808 > https://osu.ppy.sh/b/1494828 At 02:32:949, there's a 0.01x velocity point with a slider that lasts until 02:35:949. In taiko and mania, this note lasts until approximately 03:03. > ![osu_2021-11-22_12-16-07](https://user-images.githubusercontent.com/74262407/142932680-dca9dfdf-d656-4802-a77c-e532bce2fabd.png) > ![osu_2021-11-22_12-35-31](https://user-images.githubusercontent.com/74262407/142932742-1bd6b8d2-a470-4ed9-8536-9fa6aabfa03b.png) > ![osu_2021-11-22_12-35-55](https://user-images.githubusercontent.com/74262407/142932757-1762c0be-c56e-4986-b143-10035bb3a747.png) While originally discarded as a duplicate of #14835, I don't think that's the case. There will inevitably be maps with lower scroll speeds than the allowed limit, and lazer isn't calculating the duration properly for these modes. If std and catch aren't affected, why should taiko and mania? For clarification, this is NOT about allowing lower scroll speeds. This is about ensuring duration is accurate to allow converts to be played. You can still set the SV to 1 but have the correct duration and have the map be playable. In its current state, the map isn't.
bdach commented 2 years ago

for whatever it's worth i tried to look into this one yesterday, and while part of the issue is related to the slider velocity bindable range being too small, there seems to be something else in the conversion algorithms that seems to yield incorrect results.

smoogipoo commented 2 years ago

Do you know if mania was at least fixed? Taiko is especially flimsy and must rely on bringing about FP error under magnitudes of 1E-6 to work correctly. If mania was still broken then that would be a pretty good target to focus on rather than taiko, because I'm fairly certain the taiko calcs are as accurate as they can get... https://github.com/ppy/osu/blob/b90a5864b1726cd854c6b709e07b8d9a8ba4b432/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs#L146-L190

bdach commented 2 years ago

unfortunately no, sorry. i got hard stuck on the taiko case and put the mania case off for later.

zachmanthethird commented 2 years ago

Do you know if mania was at least fixed?

Version 2022.306.0

https://user-images.githubusercontent.com/74262407/156977979-5ec53548-71da-4c80-8375-34dd02bfa19e.mp4

smoogipoo commented 2 years ago

@zachmanthethird I was asking for @bdach 's own changes, not something already present either on master or in a release build.

zachmanthethird commented 2 years ago

I think I might have finally found our answer; though I'm not sure if it was the original problem. Converts do not seem to be correctly referencing per-hitobject Velocity values, and thus the entire map has a constant scroll speed. This means that the "slider" in question was not complaining about a 0.01 value; instead, it's receiving a value of 1 instead of 0.1. Therefore, the range of this value may not have been the problem at all. I specifically tested with https://osu.ppy.sh/b/2169357 and https://osu.ppy.sh/b/2002882 but after coming to this conclusion, I have yet to find a converted beatmap that disproves this theory.