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

Non-ASCII thousands and decimal separators not processed correctly #14630

Closed
groenroos opened this issue May 11, 2021 · 10 comments · Fixed by #22064
Closed

Non-ASCII thousands and decimal separators not processed correctly #14630

groenroos opened this issue May 11, 2021 · 10 comments · Fixed by #22064
Labels
localisation Related to translations and localisation

Comments

@groenroos
Copy link

In some languages (such as Finnish), thousands are usually separated by spaces. However, this means that in some cases the number can confusingly wrap onto multiple lines, as the space makes the parts of the single number behave like two separate words;

Screenshot 2021-05-11 at 18 04 13

Numbers being injected into strings should be protected from being wrapped onto multiple lines.

@duncanspumpkin
Copy link
Contributor

That would be very hard to pull off. The layout part of the code that decides where to place line breaks is after the token processing code. You would need to change it so that the space is actually a non breaking space.

@ZxBiohazardZx
Copy link
Contributor

would it be an idea to use the UNICODE non-breaking space ( U+00A0) as opposite of the normal space (U+0020)

not sure you can add that as character in the localisation in an easy way (U00A0 as seperator)

@ShimmerFairy
Copy link

If Unicode is an option, then using NBSP as mentioned would be an easy way to specify spaces that should not be broken across lines. In fact, for reference, Unicode comes with a line breaking algorithm, specified in UAX#14. So even if you had to write your own Unicode-aware line breaker (as opposed to using some library that figures it out for you), you at least wouldn't have to figure out an algorithm by yourself.

@groenroos
Copy link
Author

If the Unicode (or some other) non-breaking space character is supported, then it's a gotcha for translators, but at least the user-facing issue can be resolved. Are non-breaking spaces supported & respected in the game currently?

@Gymnasiast
Copy link
Member

Gymnasiast commented May 13, 2021

Yes, they are. I fixed that to allow for French-style punctuation (which similarly puts a non-breaking space before ! and ?).

If you change it in Localisation, could you also update the other language files that have a space for a thousands separator?

@Gymnasiast Gymnasiast added the localisation Related to translations and localisation label May 13, 2021
@groenroos
Copy link
Author

I just tried giving it a go with a single U+00A0 for STR_5151, but the same behaviour still happens - the two segments of the number are still split onto two lines... 😕

I'm running 0.3.3 (3f65f282d) if that makes a difference.

@groenroos
Copy link
Author

I noticed that my previous attempt was thwarted by my IDE trying to be "helpful" by converting my non-breaking spaces to regular spaces.

I now added a proper U+00A0 for STR_5151 in OpenRCT2/Localisation#2832 - however, I had to close that PR, because in-game this seems to corrupt any parent string that needs a thousands separator, by truncating the rest of the string:

Screenshot 2024-05-19 at 19 42 34

I'm assuming the same character works fine for punctuation in fr-FR (albeit it looks like they also use a regular space for STR_5151?). Is there something extra that needs to be fixed or enabled to make non-breaking spaces work for fi-FI in this context?

@Gymnasiast
Copy link
Member

If I had to hazard a guess: the symbol is represented by two bytes in UTF-8, and our code probably makes an assumption somewhere that the symbol is ASCII.

@Gymnasiast Gymnasiast changed the title Numbers should not wrap in languages that use space for thousands separators Non-ASCII thousands and decimal separators not processed correctly May 19, 2024
Gymnasiast added a commit to Gymnasiast/OpenRCT2 that referenced this issue May 19, 2024
Gymnasiast added a commit to Gymnasiast/OpenRCT2 that referenced this issue May 19, 2024
Gymnasiast added a commit to Gymnasiast/OpenRCT2 that referenced this issue May 19, 2024
@Gymnasiast Gymnasiast linked a pull request May 19, 2024 that will close this issue
@Gymnasiast
Copy link
Member

@groenroos A fix is pending, are you able to check that out?

@groenroos
Copy link
Author

@Gymnasiast Thanks - built it locally, and that does indeed seem to fix the issue! 🎉

As for OpenRCT2/Localisation#2832, given that we probably wouldn't want to distribute those strings unless your patch was also merged, would it be safe to re-open that PR now, or wait for later?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
localisation Related to translations and localisation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants