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

Add sane precision values to all settings #30798

Merged
merged 2 commits into from
Nov 20, 2024
Merged

Conversation

frenzibyte
Copy link
Member

@frenzibyte frenzibyte commented Nov 20, 2024

May help with #30632, as a workaround. I don't think there should be any floating-type config setting existing with no precision value defined.

@peppy
Copy link
Member

peppy commented Nov 20, 2024

Can you explain how this fixes it? smoogi and I were looking into this but agreed that it was likely an edge case bindable issue (potentially cancellation of load causing backwards propagation of precision from a Bindable<float> (where it will be default aka 0) to a BindableNumber<float>. And I'm not sure this is worth merging unless you can give some insight into how it works to fix.

Like, the more probable-to-fix-issue would be using BindableNumebrs everywhere instead of Bindable<float>.

dunno.

Copy link
Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

as ↑

@peppy peppy changed the title Fix "positional hitsounds level" setting (and other settings) not specifying a precision constraint Add sane precision values to all settings Nov 20, 2024
Copy link
Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

As discussed on discord. Updated OP to not imply it fixes an issue that it may not.

@peppy peppy merged commit 2c0140f into ppy:master Nov 20, 2024
7 of 9 checks passed
@frenzibyte frenzibyte deleted the fix-precision branch November 20, 2024 18:07
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.

2 participants