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

Update mania scroll speed range to match stable #9902

Merged
merged 4 commits into from Aug 18, 2020
Merged

Update mania scroll speed range to match stable #9902

merged 4 commits into from Aug 18, 2020

Conversation

lanpai
Copy link
Contributor

@lanpai lanpai commented Aug 18, 2020

Resolves #9868.

Using DrawableManiaRuleset.MAX_TIME_RANGE to calculate mania's scroll speed resolves to a value unlike osu!stable's.

@smoogipoo
Copy link
Contributor

You've only changed the display here. You need to change the actual constants (that you've removed from the calculation here).

DrawableManiaRuleset.MAX_TIME_RANGE = 13720
DrawableManiaRuleset.MIN_TIME_RANGE = 340

@lanpai
Copy link
Contributor Author

lanpai commented Aug 18, 2020

Thought that it might have unintended consequences. Changed the min and max values for the range as suggested.

EDIT: Having the upper bound for the range that high makes it rather difficult to set the setting in the menu.

@smoogipoo
Copy link
Contributor

The menu should probably not be using millisecond-level values in the first place. It probably should also be using 1..40 ticks.

But that's another issue: #9689

@smoogipoo
Copy link
Contributor

@peppy Think it's fine if we break this? Or do you want a migration?

@peppy
Copy link
Sponsor Member

peppy commented Aug 18, 2020

We already have migration support in OsuConfigManager so let's add one. It should take a few minutes.

@peppy
Copy link
Sponsor Member

peppy commented Aug 18, 2020

@lanpai please do not use force push in the future.

@smoogipoo
Copy link
Contributor

smoogipoo commented Aug 18, 2020

This is ManiaRulesetConfigManager which requires a database migration.

@lanpai
Copy link
Contributor Author

lanpai commented Aug 18, 2020

How about we use the equation 13380 * ((x-340)/13380)^2 + 340 this would create more control in the lower end of the range but less in the higher end. For example, at the half-way point where 6690 would be, it would now be mapped to 3354 while the lower bound and upper bound are kept the same.

Since the lower bound requires more control than the upper bound.

My bad on the force push was typing too fast haha.

@bdach
Copy link
Collaborator

bdach commented Aug 18, 2020

People will complain it's not the same as stable. Let's keep this 100% consistent IMO; finer grained control is possible via the slider in settings.

@peppy
Copy link
Sponsor Member

peppy commented Aug 18, 2020

If it's a ruleset config, you can actually migrate with a manual sql statement, I believe. Should also be pretty easy to do?

If migrating is too hard, maybe resetting to default is better (presuming if this is not done, the speed becomes very wacky? if it's only changing a small amount maybe it's fine).

@lanpai
Copy link
Contributor Author

lanpai commented Aug 18, 2020

I'm referring to the slider in the settings, @bdach . I play at a speed around 24-26 in osu!stable and I found it really difficult to get the slider to the correct position as the slider doesn't allow for very precise movements and the equivalent to the range I play at is less than 5% of the entire slider.

@bdach
Copy link
Collaborator

bdach commented Aug 18, 2020

That would be a separate PR, and has already been noted by users in #9689.

@smoogipoo
Copy link
Contributor

Nevermind, looks like a migration isn't required since the database stores ms values.

@bdach
Copy link
Collaborator

bdach commented Aug 18, 2020

Not 100% sure about that - the new minimum is higher than the old one, would we not need to clip it to be safe?

@smoogipoo
Copy link
Contributor

It's already going to be clipped by the config manager, so it'll be fine. I've also never seen anyone play at speed 40 in stable, so I don't worry about potentially clipping people off either.

@bdach
Copy link
Collaborator

bdach commented Aug 18, 2020

Right, I checked and you're correct. Seems to clip automatically with no intervention needed.

@bdach
Copy link
Collaborator

bdach commented Aug 18, 2020

So I double-checked this in game and this is actually a little buggy at the extremes; if I go from scroll speed 1 to 40 I don't arrive at 340ms, I arrive at 343 (as 13720/40 = 343). Then I can actually increase once more to 340ms, and then if I go back to scroll speed 1, I arrive at 12169ms. Not sure whether that's something we want to address right here and now.

@bdach bdach changed the title Updated calculation of mania scroll speed Update mania scroll speed range to match stable Aug 18, 2020
@smoogipoo
Copy link
Contributor

I think that's fine for the time being? Probably want to change the relative calculations to consider both endpoints but that can be done in a separate PR and isn't suuuuper important imo.

@smoogipoo smoogipoo merged commit a4ba218 into ppy:master Aug 18, 2020
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.

The scroll speed setting for mania on osu!Lazer isn't same as on osu!stable
4 participants