ppy / osu

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

Consistently failing to calculate beatmap difficulty when switch from STD to CTB #21978

Closed Trung0246 closed 1 year ago

Trung0246 commented 1 year ago

Type

Game behaviour

Bug description

Last diff for this map: https://osu.ppy.sh/beatmapsets/336802#osu/762034

Edit: looks like some maps are affected, not just the mentioned map: https://osu.ppy.sh/beatmapsets/894104#osu/1871283. More testing apparently almost all of the map have this weird issue.

Screenshots or videos

No response

Version

2022.1228.0

Logs

database.log input.log legacy-ipc.log network.log performance.log performance-audio.log performance-draw.log performance-input.log performance-update.log runtime.log updater.log

bdach commented 1 year ago

I don't see it.

osu_2023-01-01_10-58-38

Please attach a screenshot or video of what you're seeing.

Trung0246 commented 1 year ago

Oops thought it was unnecessary, guess I was wrong. Hm this is kinda weird to replicate although I listed steps below:

1) Open osu! 2) Quickly navigate to song select screen 3) Choose mods 4) Switch to CTB

Have to do everything in a short timeframe.

https://www.youtube.com/watch?v=4q2FxSx3U4A

(a little bit blurry since youtube is processing the video)

Trung0246 commented 1 year ago

@bdach can you confirm the reproduction?

bdach commented 1 year ago

I cannot. This is in the logs I guess but I have no idea how to make it happen on my side.

2023-01-01 04:20:06 [error]: Failed to calculate beatmap difficulty
2023-01-01 04:20:06 [error]: System.InvalidCastException: Unable to cast object of type 'osu.Game.Rulesets.Osu.Beatmaps.OsuBeatmapProcessor' to type 'osu.Game.Rulesets.Catch.Beatmaps.CatchBeatmapProcessor'.
2023-01-01 04:20:06 [error]: at osu.Game.Rulesets.Catch.Mods.CatchModDifficultyAdjust.ApplyToBeatmapProcessor(IBeatmapProcessor beatmapProcessor)
2023-01-01 04:20:06 [error]: at osu.Game.Beatmaps.WorkingBeatmap.GetPlayableBeatmap(IRulesetInfo ruleset, IReadOnlyList`1 mods, CancellationToken token)
2023-01-01 04:20:06 [error]: at osu.Game.Rulesets.Difficulty.DifficultyCalculator.preProcess(IEnumerable`1 mods, CancellationToken cancellationToken)
2023-01-01 04:20:06 [error]: at osu.Game.Rulesets.Difficulty.DifficultyCalculator.Calculate(IEnumerable`1 mods, CancellationToken cancellationToken)
2023-01-01 04:20:06 [error]: at osu.Game.Beatmaps.BeatmapDifficultyCache.computeDifficulty(DifficultyCacheLookup& key, CancellationToken cancellationToken)

Looks to be related to ctb difficulty adjust specifically, and also hard rock.

ppy-sentryintegration[bot] commented 1 year ago

Sentry issue: OSU-6Z

frenzibyte commented 1 year ago

Upon a bit of an investigation after reproducing this with debugger on, it seems this was rather a bad regression triggered by https://github.com/ppy/osu/pull/20758.

To put it in simple terms, song select stores a decoupled bindable for ruleset that gets updated 200ms after the ruleset is actually switched, and the AdvancedStats component listens to and uses both the decoupled ruleset bindable and the game-wide mods bindable in its difficulty calculation tasks.

Now, when a user switches from osu! to osu!catch with a common mod selected (DA specifically), the following happens in order:

  1. Game-wide ruleset bindable changes from osu! to osu!catch.
  2. OsuGame notices the change and updates the game-wide mods bindable from OsuDifficultyAdjustment to CatchDifficultyAdjustment
  3. AdvancedStats receives the mods change and issues a difficulty calculation task on the beatmap and the selected catch mod, but on the osu! ruleset (because again, it's using the decoupled bindable)
  4. Later on after 200ms, SongSelect updates the decoupled ruleset bindable from osu! to osu!catch
  5. AdvancedStats receives the ruleset change and issues a difficulty calculation task with the right arguments.

The problem lies on (3), which is when the exception in the logs are coming from (I'm still not sure why it's hard to reproduce, something to do with timing I suppose).

One solution that I'm thinking of right now but not entirely sure of is to change AdvancedStats to use the game-wide ruleset bindable, potentially by using GetBindableDifficulty with a parameter of not accounting for selected mods in the star rating value.

That will however cause the advanced statistics display to update its values before other components do (but to be fair, the star rating display on the wedge also updates prematurely).

bdach commented 1 year ago

I considered this as a plausible scenario but sentry shows that this issue first surfaced 8 months ago, which is prior to that pull. So I'm not sure that it's the only way to reproduce.

frenzibyte commented 1 year ago

The other scenario that's irrelevant from the aforementioned PR happens when a user has osu!catch selected with DA/HR mod on, then proceeds to edit a beatmap that's originally made in osu!, so the following happens in EditorLoader: https://github.com/ppy/osu/blob/7bc8908ca9c026fed1d831eb6e58df7624a8d614/osu.Game/Screens/Edit/EditorLoader.cs#L67-L70

Since EditorLoader is pushed from song select, the ruleset bindable it's changing belongs to song select, so AdvancedStats gets updated as well. And this time, the selected mods bindable is disabled, so the mods remain in an invalid state while AdvancedStats issues the diffcalc task, thus failing with the same failure once again.

The solution proposed above would also fix this case as BeatmapDifficultyCache performs a one-frame schedule on any change to the game-wide bindables bindables, to avoid these kind of situations when updating the tracked bindables.

Trung0246 commented 1 year ago

@frenzibyte I think the bug don't even need to rely on Catch for editor case, since I just tried switching to any modes and also looks like changing mods after entering the editor in different mode would cause the same thing. @bdach you can to reproduce this now I guess since this way is way more reliable.

https://user-images.githubusercontent.com/11626920/211225090-7925d8e1-5b2f-41b8-9c10-72ec4c60a8da.mp4

database.log input.log legacy-ipc.log network.log performance.log performance-audio.log performance-draw.log performance-input.log performance-update.log runtime.log updater.log

frenzibyte commented 1 year ago

osu!catch was only ever brought as an example. The main bug is that one of song select's components is not handling ruleset changes properly, and resulted in faulty difficulty calculation requests.