Enforce real time difficulty for mod combinations which are not stored in the database

[tsunyoku]

Old discussion about the idea available on Discord: https://discord.com/channels/188630481301012481/380598781432823815/1245654906761777267.

Talked with smoogi about the idea and seems like a good way to get the ball rolling. This will allow for awarding pp on rates for double time other than the default (1.5x) and/or awarding pp for non-legacy mods which have difficulty changes. There's technically some edge cases here when a mod has settings which won't effect difficulty changes and will still go through realtime, but I don't think this is a major issue (and doesn't have a pretty solution). Classic is excluded from the "non-legacy mods" because default CL settings is treated the same as non-CL for now - this might become messy once difficulty calculation changes that effect non-CL specifically are deployed.

I've renamed the REALTIME_DIFFICULTY env var to ALWAYS_REALTIME_DIFFICULTY to try and reduce confusion over when realtime calculations will be used. When merging this'll need to be accounted for to avoid accidentally enabling realtime difficulty always.

To help smoogi/the team with profiling and the implications of realtime calculations on production, I've added a calculate-realtime-difficulty-attributes datadog timer with tags for the mods and ruleset. It's worth noting that the time will include the .osu file download time. This should help figuring out if realtime is too bad to use always, since long-term that would be the better option.

isRankedLegacyMod exists as an easy solution to figure out if the mod is going to be cached in the database. I don't really like this method in general but I don't know of a better way to do it...

There's an added test using OsuModMuted and OsuModTraceable as these are the only 2 mods which are non-legacy, so would be expected to go down the realtime path.

[bdach]

Talked with smoogi about the idea

Was this in DMs or something? I'd rather not proliferate that form of communication.

I don't disagree with the decision to do it, but I'd rather not play telephone game over such matters.

It's worth noting that the time will include the .osu file download time

I would hope that production would not download beatmaps. Or at least hope that it caches them if it has to download.

I wanted to write tests for these cases, however with this diff alone it's not really possible as in all cases mustUseRealtimeDifficulty would be true the mod will have Ranked as false so it'll never reach this block. Tests can be added in the future when this is merged.

I don't understand. How does merging this help write tests? Either tests can be written for this now and after merge, or neither now or after merge until something else happens?

[smoogipoo]

Was this in DMs or something? I'd rather not proliferate that form of communication.

Only briefly. It was not discussed in depth but I said it's a good path to go down.

[tsunyoku]

I don't understand. How does merging this help write tests? Either tests can be written for this now and after merge, or neither now or after merge until something else happens?

It would allow for setting Ranked on a mod that relies on the realtime path. Merging won't immediately make tests writable, I'm just highlighting that reaching this code with the current set of ranked mods is impossible.

I would hope that production would not download beatmaps. Or at least hope that it caches them if it has to download.

This is existing logic, the download path is settable with an env var so I'm not sure if there's some internal service communication that has some caching or something but if not then I'm open to replacement suggestions but I don't know how the infra works at that level.

[bdach]

It would allow for setting Ranked on a mod that relies on the realtime path. Merging won't immediately make tests writable, I'm just highlighting that reaching this code with the current set of ranked mods is impossible.

This makes much more sense as an explanation.

This is existing logic, the download path is settable with an env var so I'm not sure if there's some internal service communication that has some caching or something but if not then I'm open to replacement suggestions but I don't know how the infra works at that level.

Sure, it is, it has also never been ran on production, which means that figuring out whether or not the live version should download beatmaps or not is a valid review concern in my eyes.

[tsunyoku]

figuring out whether or not the live version should download beatmaps or not is a valid review concern in my eyes.

Yeah I 100% agree, just not sure if it's something I can personally weigh in on. I don't know enough about the infrastructure to comment.

[tsunyoku]

It would allow for setting Ranked on a mod that relies on the realtime path. Merging won't immediately make tests writable, I'm just highlighting that reaching this code with the current set of ranked mods is impossible.

I realise this isn't technically true because mods like muted and traceable are ranked which do not affect difficulty calculations, but fit under the conditions for going through realtime calculations. Should I add tests for those cases? Is it a concern that these mods are going through realtime calculations when they technically have no effect on difficulty (yet)?

[bdach]

Should I add tests for those cases?

I'll always take tests if it is possible to write them sanely. Not sure it is going to be in this case, but you could try.

[bdach]

There are two places to check (that I won't link to, for hopefully obvious reasons). So at least I know that now.

[peppy]

I think we can push this out for testing if @bdach is on board with the applied changes.

[bdach]

Not sure how that happened but the envvar had a space in name. Removed in 8b57913.

[bdach]

Well I've done some testing and this appears to do what it says I guess. That said, @peppy If you're going to try this:

• VERY IMPORTANT: Make sure to update the envvar from REALTIME_DIFFICULTY to ALWAYS_USE_REALTIME_DIFFICULTY. It defaults to 1 (and did previously). On production it must currently be set to 0. If you don't update the envvar name (or probably, more likely, specify both in the interim), this is going to start using realtime difficulty for ALL scores which WILL die of death.
• The realtime difficulty process does a web request for EVERY score (to fetch the .osu file). There is NO caching instituted. This is not news, but since we're wanting to switch things on, this is a consideration that this PR introduces. Specifying file:// protocol to bypass that WILL NOT work:

[network] 2024-08-07 09:33:59 [verbose]: Request to file:///home/dachb/Documents/opensource/diffcalc-sheet-generator/beatmaps/869310.osu failed with System.NotSupportedException: The 'file' scheme is not supported.
[network] 2024-08-07 09:33:59 [verbose]: at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
[network] 2024-08-07 09:33:59 [verbose]: at osu.Framework.IO.Network.WebRequest.internalPerform(CancellationToken cancellationToken).

• Realtime difficulty WILL immediately be used after this is deployed, for scores that use a few mods which did not exist in stable, and thus far worked because we basically went "this mod does not affect difficulty attributes, so we can pretend it does not exist for the sake of difficulty attribute lookup / treat it as nomod". As far as I know, this will be the case for mods such as Muted and Traceable. Possibly others that I'm forgetting.

[bdach]

Under the premises above

[developomp]

If it's not a huge hassle, there should probably be a partial pp recalc for old lazer scores so they are retroactively rewarded pp.

[bdach]

If it's not a huge hassle, there should probably be a partial pp recalc for old lazer scores so they are retroactively rewarded pp.

We haven't done this thus far when switching mods to ranked, so I dunno about that.

[stanriders]

If it's not a huge hassle, there should probably be a partial pp recalc for old lazer scores so they are retroactively rewarded pp.

We haven't done this thus far when switching mods to ranked, so I dunno about that.

It's probably gonna be covered by a pp deploy anyway, so it's not really necessary

[peppy]

Well I've done some testing and this appears to do what it says I guess. That said, @peppy If you're going to try this:

• VERY IMPORTANT: Make sure to update the envvar from REALTIME_DIFFICULTY to ALWAYS_USE_REALTIME_DIFFICULTY. It defaults to 1 (and did previously). On production it must currently be set to 0. If you don't update the envvar name (or probably, more likely, specify both in the interim), this is going to start using realtime difficulty for ALL scores which WILL die of death.
• The realtime difficulty process does a web request for EVERY score (to fetch the .osu file). There is NO caching instituted. This is not news, but since we're wanting to switch things on, this is a consideration that this PR introduces. Specifying file:// protocol to bypass that WILL NOT work:

[network] 2024-08-07 09:33:59 [verbose]: Request to file:///home/dachb/Documents/opensource/diffcalc-sheet-generator/beatmaps/869310.osu failed with System.NotSupportedException: The 'file' scheme is not supported.
[network] 2024-08-07 09:33:59 [verbose]: at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
[network] 2024-08-07 09:33:59 [verbose]: at osu.Framework.IO.Network.WebRequest.internalPerform(CancellationToken cancellationToken).

• Realtime difficulty WILL immediately be used after this is deployed, for scores that use a few mods which did not exist in stable, and thus far worked because we basically went "this mod does not affect difficulty attributes, so we can pretend it does not exist for the sake of difficulty attribute lookup / treat it as nomod". As far as I know, this will be the case for mods such as Muted and Traceable. Possibly others that I'm forgetting.

Thanks for the summary. This all sounds in line with my understanding. I'll look to deploy this tomorrow.

[peppy]

Reasoning for not deploying this yet: if it was to be deployed, scores set with non-precomputed difficulty attributes will get slightly different calculation methods. We are postponing deploy of this until after all pp is handled by osu-queue-score-statistics an we have a clean slate with the same algorithm used everywhere.

[smoogipoo]

Something that's been on my mind, but I've been weary of mentioning it because it'll get pushback ("release this ASAP and see if it breaks first!!!"), is that we already have https://github.com/ppy/osu-beatmap-difficulty-lookup-cache which is duplicating this functionality.

It may be worthwhile to hook that up (it's a web request that's being done anyway so overhead should be about the same) and optimising the caching local to that project.

[tsunyoku]

It may be worthwhile to hook that up (it's a web request that's being done anyway so overhead should be about the same) and optimising the caching local to that project.

I'm content with this, but it'll require adjustments over on that repository as it only caches for non-databased-attributes and is also configured to only compute attributes if USE_DATABASE_LOOKUPS isn't set. That repo however does have the advantage of it's currently caching when things are computed in real time, which might improve throughput for some common mod settings (mostly thinking of rate changes here) however I'm not sure if that would consume too much memory in the long-run as any settings combination would be new attributes to store. It'd also require moving over the logic to decide if a mod should use real time calculations (at least if we want it to function similarly to this PR) and I'm unsure if we want to tangle ranked-related logic (see IsRankedLegacyMod) into that project...

I think there's no harm in keeping things simple and deploying this PR as-is with the hopes of having a more centralised solution in the long-run. I imagine things will change with time anyways as lazer mods being ranked and awarding PP is a new concept in the first place.

[tsunyoku]

Commenting as a means to ask if we can think about deploying this some time soon.

The main reasons I want this in are:

• It's a requirement to give rate changes PP
• It will allow us to make difficulty calculation changes that take into consideration the differences in mechanics between lazer and stable. For example: Don't punish slider hit windows in difficulty calculations when slider head accuracy is in use osu#28267 & https://discord.com/channels/188630481301012481/542532479626772500/1367057400452087870
• Whilst I think we can/should rank Mirror without real time by treating it as no-mod (see my reasoning to why mirror mod should be ranked at 1.0x multiplier osu#33076), real time would remove any need to have that debate
• Would allow other mods such as Add Strict Tracking difficulty calculation support osu#30232

I'm not suggesting the immediate ranking of mods following the deploy of this. However, I'd like to see them ranked in a reasonable timeline and we can't really do that without knowing that this works well in production.

[peppy]

It will allow us to make difficulty calculation changes that take into consideration the differences in mechanics between lazer and stable

This would imply you are intending to use this for either all lazer or all stable scores, which is a much larger scope than the odd mod where it's not in the database. Can you confirm this is the case? If so, there are still some prerequisites to make this work at scale.

[tsunyoku]

Hmm, you are right. It would be for all lazer scores, since we have the database tables for legacy scores.

Somehow that much slipped my mind, and this PR does not cause all lazer scores to go down the realtime path because it was intentionally built that way.

I agree that this would require further work to run on absolutely every lazer score, so I'm fine if that is an improvement which comes later - being able to rank lazer mods which have intentional star rating differences is already a big step forward and this PR in its current state allows for that.