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 String.number_format() method for more readable large numbers #89424

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Mar 12, 2024

This is used in the editor to display large numbers such as in the View Information panel.

TextServer.format_number() automatically uses this method with the current language (if no language is specified), which benefits many locations in the editor such as the inspector, performance monitors and the 3D editor View Information panel.

Unit tests have been added for both String.number_format() and TextServer.number_format().

Preview

English

Screenshot_20240311_224042 Screenshot_20240311_235852 Screenshot_20240311_235917

French

Screenshot_20240312_000354

This is used in the editor to display large numbers such as in the
View Information panel.

`TextServer.format_number()` automatically uses this method with the current
language (if no language is specified), which benefits many locations
in the editor such as the inspector, performance monitors and the 3D editor
View Information panel.
@Calinou Calinou added this to the 4.x milestone Mar 12, 2024
@Calinou Calinou requested review from a team as code owners March 12, 2024 15:56
@KoBeWi
Copy link
Member

KoBeWi commented Mar 12, 2024

Does not work well in the inspector:

oVhfcccVSz.mp4

Double click will select parts separately and the commas are treated as decimal separators, so if you add an actual separator, the result is invalid.

In space-separated locales, the decimal part is simply rejected.

F0tKHTQ5DY.mp4

@AThousandShips
Copy link
Member

Somewhat related on the editor side I think:

@Calinou Calinou marked this pull request as draft March 12, 2024 17:11
@Calinou
Copy link
Member Author

Calinou commented Mar 12, 2024

Does not work well in the inspector:

I recall it working correctly in my testing (the displayed number during editing didn't have any separators). I think it should work this way preferably, but we should make it so EditorSpinSlider and SpinBox accept numbers with various separators nonetheless for better usability.

To handle situations where we have both , and . separators in the same string, the decimal separator is the one that comes last in the string.

@bruvzg
Copy link
Member

bruvzg commented Mar 12, 2024

TextServerAdvanced::_format_number isn't doing anything like this (adding separators), it's only converting western Arabic numerals to the language specific numeral system. So it should be either modified to do so or not use it as fallback.

For the reference: ICU have full number formatting methods, but we do not build ICU internationalization part and do not include relevant databases (nor sure if it's possible to include only part relevant to the number formatting, full database is ~36 MB).

Edit: decimal format strings are part of main locale files, so not counting extra code that should be added it will add at least 8MB if data if used directly. So it probably should be a custom implementation, similar to the one used for numeral system conversion.

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.

Add a String method to format numbers with a delimiter separator and custom decimal separator
4 participants