Skip to content

Prevent String::num_scientific from giving different precision levels depending on compiler#86951

Closed
TheSofox wants to merge 1 commit into
godotengine:masterfrom
TheSofox:num-scientific-fix
Closed

Prevent String::num_scientific from giving different precision levels depending on compiler#86951
TheSofox wants to merge 1 commit into
godotengine:masterfrom
TheSofox:num-scientific-fix

Conversation

@TheSofox
Copy link
Copy Markdown
Contributor

@TheSofox TheSofox commented Jan 8, 2024

Fixes #78204.

String::num_scientific has different codepaths depending on compiler, however there was a fundamental difference in precision of the conversion from float to String depending compiler/platform. Essentially one used %.16lg while the other used %lg. This led to surprisingly low precision (not even able to print out a timestamp properly). I've tweaked it so that this low precision codepath uses the same high precision as the other codepath.

@TheSofox TheSofox requested a review from a team as a code owner January 8, 2024 04:55
@TheSofox
Copy link
Copy Markdown
Contributor Author

TheSofox commented Jan 8, 2024

Okay, the tests are failing largely because the Unit Tests were made with num_scientific's low precision in mind (eg. num_scientific(30000000) == "3e+07")). It also confuses me that since it implies all the tests use the first codepath, so when is the second codepath ever run?

@AThousandShips
Copy link
Copy Markdown
Member

AThousandShips commented Jan 8, 2024

Unsure what compilers would be non GNUC, but there's no tests run on android, iOS, or web, so might be there

Note that this also breaks existing documentation, so all that has to be updated with the new precision, I ran into these issues when I was trying to solve this myself last year or so

My suggestion for the documentation is to change the specifics of the code generation there to avoid overly specific strings in the documentation

@KoBeWi KoBeWi modified the milestones: 4.3, 4.4 Aug 3, 2024
@AThousandShips AThousandShips changed the title Prevented String::num_scientific from giving different precision levels depending on compiler Prevent String::num_scientific from giving different precision levels depending on compiler Aug 3, 2024
@aaronfranke
Copy link
Copy Markdown
Member

aaronfranke commented Nov 2, 2024

Closing as superseded by PR #98750. Thank you for getting us started fixing this bug :)

@aaronfranke aaronfranke closed this Nov 2, 2024
@aaronfranke aaronfranke removed this from the 4.4 milestone Nov 2, 2024
@TheSofox TheSofox deleted the num-scientific-fix branch May 6, 2025 19:25
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.

var_to_str rounds floats, losing massive precision in the process

4 participants