Adjust a beatmaps offset automatically after each play

[Inconsist]

Implementing suggested idea in discussion #30521

After each play the calculated offset is scaled with regards to the plays UR and applied immediately.

• Should improve quality of life when using auto restart on mods like SD and PF.
• Should generally help to shift every beatmap ever so much closer to its true correct timing for your specific circumstances.
• The scaled approach and incremental changes ensure that we approach an optimum and don't overshoot it unless we have reason to be sure.
• The feature should be self correcting by nature if the map is played enough times.

[Inconsist]

Tried this on 3 maps now, setting turned on. Each time I started with either +50ms or -50ms beatmap offset to simulate a map that is mistimed or a setup with a lot of delay, to see where it would stabilize. It stabilized every time and then had some variance. Obviously I need a global offset ;) since all 3 maps offset stabilized in the +10ms range.

The charts below show how the actual offset that was applied for the next run developed over time. Starting with +-50ms on the left and iterating from there to the right with every playthrough

[Inconsist]

Setting can now also be toggled either from the settings menu or from the overlay menu that appears when the beatmap is started.

[peppy]

I'm sure this makes good sense, but without attempting to understand the consequences, I'm curious if there's a reason not to always use this algorithm even when the user presses the adjust button.

[Inconsist]

I don't think anything speaks against this per se. But it might just lead to situations where the player feels like the offset should have been way bigger, and is confused why it only says something like 0.8ms.
The button press is a conscious choice you make after every play, so I think displaying the true hit error average here is more genuine towards the player.
You can always choose not to apply an offset you know resulted from you playing poorly.
In my opinion when using the automation the safeguard is needed, but the player should choose themself when using the button.

[peppy]

Fair. Might be good to include this kind of explanation inline to give more context.

[Inconsist]

I think I made a comment that leads any reader to the discussion where the calculation is explained in more detail. I didn't want to write such long comments since they say less than the image.
If you mean giving more context to the player, the setting name is already quite long, I couldn't think of a better way to name the setting for now, any ideas are appreciated.

[peppy]

Please don't merge master into your PR. Leave that to us. It's just noise.

[peppy]

I've removed the checkbox from showing at the loader screen, as I think it's taking up too much real estate. In the future I'd like to instead hide these settings in a popover so they can be discovered without taking up huge amounts of space. For now, users can find this setting in the global settings.

[peppy]

When this is applied, the "offset changed" popout seemingly doesn't appear.

This thing:

[peppy]

As mentioned

[Inconsist]

For me this appeared on every restart, with the proper offset. I do not know why it wouldnt show for you. Also I can not reply to your comments, sorry for taking up extra space.

[Inconsist]

Very rarely on the auto restart with pf mod, it felt like the game had dropped some frames and the restart was extremely quickly. Only one out of 30 restarts maybe. But I read somewhere that while I was doing this the fast retry was reworked at the same time, so I don't know if any of my changes caused this, I see no reason why they would. Those times the offset popup would also disappear almost instantly but in general I always saw it behave as it does in the latest release build.

[Inconsist]

I don't think there is any reason to keep this, when removing the setting from the popup?

[Inconsist]

removed this in 8703f59 since the file can remain unaffected by this pr

[Inconsist]

Is there anything left to do, and did the problem with the offset popup persist? I can not reproduce it myself.

[peppy]

No. Please wait and please don't request reviews from random developers.

[Inconsist]

I apologize. I just saw that they requested changes, which I then fixed, and I re-requested the review from them because it still said the changes were still pending. Sorry, I am just trying to help.

But if I understand correctly, you want me to do absolutely nothing from now on, even when the checks fail like right now, unless instructed otherwise from you or other maintainers, correct? That I can do, I just wasn’t sure.

[peppy]

You can fix the failures, sure.

Just

• Don't merge master
• Don't ask for reviews or bump things after only days have passed. You'll notice we're dealing with 50-100 new PR/issue/discussions a day and each gets a priority based on how important it is.

[Inconsist]

When this is applied, the "offset changed" popout seemingly doesn't appear.

This thing:

II found out why the popup is not showing.
My Algorithm is currently in a place where it will only run when the BeatmapOffsetControl.cs is used i.e. you use the quick retry hotkey or RetryOnFail with mods, nothing is computed, so the beatmap actually stays at 0 ms the entire time, for which the popup not showing is the correct behavior.
But as soon as you retry normally(esc -> retry), the AudioSettings pop up again, the calculation is made, and now it looks like the offset was there the whole time and the popup was broken.
If it is not showing even if the offset is != 0ms then I really do not know what causes it. Because manually setting it to some value worked for me and it showed every time.

This is only the case since the new quick retry update, which seems to completely skip the player loader loading the VisualSettings and AudioSettings which renders my code useless at the place where it currently exists.
It would have to be somewhere where it can be executed on every retry, which previously was true for where I put it, but unfortunately I have not yet found a good new place.

It might work in Player.cs or also PlayerLoader.cs so every time a restart is requested it executes, but I don't know if it really belongs there, it probably doesn’t. But I also can't think of another way right now.

And I do not want to debate, I just genuinely do not know what's ok and what isn’t, but in this case, is there any other way to work on this that doesn't require merging master into my feature branch once, since changes in master quite literally break the entire feature? Or still not a good idea?