Fix PP not being calculated correctly for nightcore scores

[bdach]

• Closes osu!lazer Difficulty Calculation Through osu-tools Produces Incorrect Values on ppy/master osu#25175

I guess this would be one way to roll forward with ppy/osu#24640, but there is a chance something somewhere else could have relied on the inheritance and might now break, so not sure whether this should be the solution here...

Check is slightly rewritten since it wasn't making much sense to me previously (why add nightcore to allowed mods if double time is selected?)

[peppy]

Can I get an ELI5 of how this fixes things?

[bdach]

CreateDifficultyModCombinations() will generally only output combinations with double time and ignore nightcore, on the basis that the two are interchangeable. But these two methods don't generally want to drop nightcore, so the logic goes that if a score has nightcore on it and DT is otherwise allowed, then add NC to the set of allowed mods, as it wouldn't generally be there due to the aforementioned method ignoring its existence.

This broke because we removed the ModNightcore : ModDoubleTime inheritance.

[peppy]

To confirm, IsSubclassOf is basically a synonym for what was already there, right?

[smoogipoo]

One thing that could be considered here is to take this implementation (which has tests) and do a conversion of Mod[] -> Ruleset.ConvertToLegacyMods() -> MaskRelevantMods() -> ConvertFromLegacyMods()

The reasoning is that the MaskRelevantMods() method is used for looking up database attributes, so it must match one of the values of DifficultyCalculator.DifficultyAdjustmentMods().

I'd even suggest moving that to the base game project.

[bdach]

To confirm, IsSubclassOf is basically a synonym for what was already there, right?

Not really. That part is checking if double time is in allowedMods at all. It doesn't really correspond to anything in the old code, as the old code just seemed to assume double time will always be in allowedMods and just blindly added nightcore whenever encountering it (...or double time, which is similarly weird).

One thing that could be considered here is to take this implementation (...)

Sure, I'm not opposed. Especially so that it will maybe help us spot this sort of thing earlier in future due to avoiding cross project dependencies.

[peppy]

Not really. That part is checking if double time is in allowedMods at all

Right, I missed the variable change at the start...

[smoogipoo]

Silly GitHub... I had a pending review comment, but it's not critical.

[smoogipoo]

I propose we remove this condition altogether.

[bdach]

...by adding nightcore to DifficultyAdjustmentMods game-side, I presume? I'm not sure how you'd preserve behaviour otherwise.

[smoogipoo]

I was thinking it could be added unconditionally to allowedMods here?

[bdach]

Hmmmmmmmm I think I prefer what's already here. You're probably right that unconditionally adding wouldn't probably ever be a problem but it is what it is.