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

Fix SpinBox and ParamsInput's rounding causing "discard changes" to appear in deck options #3686

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

iamllama
Copy link
Contributor

@iamllama iamllama commented Jan 4, 2025

Fixes #3592

Was able to reproduce @abdnh's findings, e.g. ParamsInput rounds
[1.2799999713897705, 3.7643001079559326, 10.089300155639648, 49.89339828491211, 7.828400135040283, 0.5245000123977661, 0.8407999873161316, 0.17730000615119934, 0.9509999752044678, 0.3490000069141388, 0.4876999855041504, 2.5562000274658203, 0.0010000000474974513, 0.8695999979972839, 2.9093000888824463, 0.23149999976158142, 2.989799976348877, 0.9776999950408936, 1.131500005722046]
to
[1.28, 3.7643, 10.0893, 49.8934, 7.8284, 0.5245, 0.8408, 0.1773, 0.951, 0.349, 0.4877, 2.5562, 0.001, 0.8696, 2.9093, 0.2315, 2.9898, 0.9777, 1.1315]
and SpinBox rounds 0.8999999761581421 to 0.9

The issue isn't that components round them, it's that the f32s passed from rslib are widened to f64s in typescript, so both need to be narrowed back down to single-precision when checking for equality

This won't result in a false negative (i.e not prompting when there are in fact changes) as long as the smallest allowed input step is greater than f32's epsilon (~0.00000012). Right now the smallest step possible is ParamsInput's 0.0001

N.B if another preset is selected, the new preset values will be rounded when focused on and blurred, but since they weren't taken into account on mount, they'll always be different from the original config and prompt to discard changes. Handled by #3689

@iamllama iamllama marked this pull request as draft January 4, 2025 18:24
@iamllama iamllama force-pushed the deck-options-discard-changes branch from f44fc06 to d999eb3 Compare January 6, 2025 10:19
@iamllama iamllama marked this pull request as ready for review January 6, 2025 10:32
@abdnh abdnh merged commit d01d83e into ankitects:main Jan 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Discard changes?" pops up when selecting desired, historical retention or FSRS parameters
2 participants