Refactor GetMaxCombo

[MaxOhn]

Arguably for mania it should stay on zero but I'd say this should be fine.

Notably, if max combo calculation in lazer ever differs from stable for any mode, this will require better handling. As far as I know they currently coincide.

[MaxOhn]

After checking some mania leaderboards it seems that max combo does differ between lazer and stable. If this PR goes through as-is, that means the shown value will be incorrect when intending to calculate stable attributes. The actual calculated attributes won't be affected though considering that their calculation in mania (currently) ignores combo.

Regardless, the shown value will be closer to the actual combo than just 0 😄 An option to distinguish lazer from stable attribute calculation as warranted in #226 could help fixing this entirely.

[minisbett]

In my opinion, the correct implementation here would be re-adding the GetMaxCombo method as a virtual method with a default implementation of calling beatmap.GetMaxCombo(). Then, for Mania it can be overriden to include the differences between stable and lazer.

This was a refactor I wanted to do some days ago but I got caught up in other stuff and haven't found time.

Update: I found out CL does not exist in Mania. Therefore I believe it is best to just have all modes do exactly this. But I believe it might be better to follow what stan said here

[MaxOhn]

I removed the method entirely now 👌

Given that the max combo is in somewhat of a limbo until the stable-lazer difference can be handled properly in osu-tools, maybe it's better to hold off on this?

[minisbett]

I removed the method entirely now 👌

Given that the max combo is in somewhat of a limbo until the stable-lazer difference can be handled properly in osu-tools, maybe it's better to hold off on this?

I don't think it will be handled differently since there isn't really a need to. Mania has that combo difference as (afaik?) the only ruleset, and combo does not matter in PP there so its fine to go with Lazer combo, it's pretty much just display.

[minisbett]

Replacing the direct calculation of maximum combo with the IBeatmap.GetMaxCombo() call seems to match 1:1 (tested) (except for disregardable differences between stable and lazer on mania), therefore this seems like a good change.

Take my meaningless approval.

[stanriders]

I'd say we'd want to rely on lazer's methods as much as possible anyway