Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting osu!mania scroll speed to single decimal precision #30832

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

peppy
Copy link
Member

@peppy peppy commented Nov 22, 2024

Probably fine to allow this?

Addresses #30663.

@frenzibyte
Copy link
Member

This change makes it somewhat awkward for players to downgrade from a release that has this change. The following exception is thrown:

[runtime] 2024-11-24 01:41:41 [error]: System.FormatException: The input string '22.0' was not in a correct format.
[runtime] 2024-11-24 01:41:41 [error]: at System.Number.ThrowFormatException[TChar](ReadOnlySpan`1 value)
[runtime] 2024-11-24 01:41:41 [error]: at System.String.System.IConvertible.ToInt32(IFormatProvider provider)
[runtime] 2024-11-24 01:41:41 [error]: at System.Convert.ChangeType(Object value, Type conversionType, IFormatProvider provider)
[runtime] 2024-11-24 01:41:41 [error]: at osu.Framework.Bindables.Bindable`1.Parse(Object input, IFormatProvider provider)

Can be replicated by running osu! on this branch and change the scroll speed to any value, decimal or non-decimal, then run the game again on master.

I can see two options toward this:

  • Add special handling in config decoding for when a decimal value is read while the expected type is integer, by truncating said decimal value.
  • Create another setting temporarily ScrollSpeedDecimal for the decimal type and, optionally, after a while migrate the setting back to its original name.

While the first option adds perhaps too much leniency to config decoding, I can't really imagine it going wrong myself.

Copy link
Member

@frenzibyte frenzibyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be a blocker.

@peppy
Copy link
Member Author

peppy commented Nov 26, 2024

But that's the same as any realm change, no? Are you worried about live users or developers? If the latter, it can be fixed as a separate effort if deemed required?

@frenzibyte
Copy link
Member

I'm worried about live users running a new release that turned out to be broken for them and they have to revert back. All of them will have to manually pop up their config file and remove the decimal part to be able to run an old release.

We're quite fast on rolling out hotfixes so if you think it's fine for this to be breaking then I'm fine.

@peppy
Copy link
Member Author

peppy commented Nov 26, 2024

Only if they change the scroll speed then install an old version, right? It's the same as if we roll out an update with a realm change, equal levels of breakage. In general we don't/can't support rollback to older releases in our current development state.

Copy link
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would've suggested something like Shift+F3/F4 to change by 0.1 increments, but I'm not sure how hard that will be to implement so will get this in as is for now.

@smoogipoo smoogipoo merged commit d3d111d into ppy:master Nov 26, 2024
9 of 10 checks passed
@peppy peppy deleted the mania-precise-scroll-speed branch November 27, 2024 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants