Translate numbers to selected language

[touhidurrr]

resolves #11895

[touhidurrr]

Ok, here is the result for now. Green means translated properly and red means was not translated for some reason. Maybe the regex I wrote was problematic?

[touhidurrr]

@yairm210 how do i fix this delekt error?

[touhidurrr]

Although not sure why the regex is having issues. See: https://regexr.com/82b23

[SomeTroglodyte]

"delekt" is a very nice typo! But detekt is a static code analysis tool, it's about overall code quality. Master currently fails too. So, just wait and merge forward later.

Normally, tweaking stuff like what detekt dislikes until we're back below threshold would be right up my alley, but I don't have the energy today.

[touhidurrr]

tweaking stuff

How to check this locally?

[touhidurrr]

Also, do you have any idea why all numbers inside the screen is not being translated properly?

[SomeTroglodyte]

Maybe fixing the evil wildcard import in ConsoleLauncher is enough

How to check this locally?

Not sure. Search for detekt PR's, go to their repo. Download separately and run. I wouldn't, I got the checks running on my fork too, so I can read the output for local changes as soon as I push.

[SomeTroglodyte]

That regex is over the top. I believe both \b are misguided (well, the comment says what they're for, but I think in our input that's irrelevant), and I'm sure we'll never have fractional values without a leading zero, or values in scientific notation.

Also, I wouldn't stress tr() so. I'd classify it as extremely performance critical, even though it's UI only. Better.... Better rely on any interesting numbers being within placeholders, so only treat strings inside translateIndividualWord (I'd have named that translateIndividualPhrase) that are in their entirety a number:

Index: core/src/com/unciv/models/translations/Translations.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/com/unciv/models/translations/Translations.kt b/core/src/com/unciv/models/translations/Translations.kt
--- a/core/src/com/unciv/models/translations/Translations.kt	(revision b20c89733bc2283bbd8124aad46652298d6822b9)
+++ b/core/src/com/unciv/models/translations/Translations.kt	(date 1719848052584)
@@ -443,6 +443,9 @@
 private fun String.translateIndividualWord(language: String, hideIcons: Boolean): String {
     if (Stats.isStats(this)) return Stats.parse(this).toString()
 
+    val number = toDoubleOrNull()
+    if (number != null) return numberFormatter.format(number)
+
     val translation = UncivGame.Current.translations.getText(this, language, TranslationActiveModsCache.activeMods)
 
     val stat = Stat.safeValueOf(this)

(Or later in the function, not sure)

not translated for some reason

I have a broken color management so I'm colorblind atm, but - any coder passing a string to the UI, if they know it'll always be a number, they would treat it as not needing translation at all. Many of our helpers will auto-translate, but some things won't. Soooo... Look at e.g. WorldScreenTopBarStats: cultureLabel.setText(getCultureText(civInfo, nextTurnStats)) -> aha! setText is direct Gdx, no translate call. Many more of course.

Last comment for today! Bye and have fun.

[yairm210]

Detekt fixed
The reason the main screen contains some untranslated numbers is because they're never .tr()'d

[touhidurrr]

I believe both \b are misguided

I suspected this also, but all of my tests said otherwise.

aha! setText is direct Gdx, no translate call. Many more of course.

This has to be the reason why

translateIndividualWord

So, in the end the best option might be trying to detect and translate numbers during placeholder fill. I tried this but it does not work at all.

--- a/core/src/com/unciv/models/translations/Translations.kt      
+++ b/core/src/com/unciv/models/translations/Translations.kt      
@@ -491,9 +491,16 @@ fun String.fillPlaceholders(vararg strings: String): String {
     if (keys.size > strings.size)
         throw Exception("String $this has a different number of placeholders ${keys.joinToString()} (${keys.size}) than the substitutive strings ${strings.joinToString()} (${strings.size})!")      

+    val numberFormatter = UncivGame.Current.settings.getNumberFormatFromLocale()
+
+    val translatedStrings = strings.map {
+        val num = toDoubleOrNull() ?: return@map it
+        return@map numberFormatter.format(num)
+    }
+
     var filledString = this.replace(squareBraceRegex, "[]")      
     for (i in keys.indices)     
-        filledString = filledString.replaceFirst("[]", "[${strings[i]}]")
+        filledString = filledString.replaceFirst("[]", "[${translatedStrings[i]}]")
     return filledString
 }

[touhidurrr]

any coder passing a string to the UI, if they know it'll always be a number, they would treat it as not needing translation at all. Many of our helpers will auto-translate, but some things won't.

Then maybe new rule: always use toTranslatedString() to convert numbers to strings?

[touhidurrr]

Need guidance, @yairm210. What do you think should be done?

[yairm210]

A. Not during placeholder fill, you'll miss all the non-placeholder labels
B. Yes, you'll need to add like a .tr() for numbers

[touhidurrr]

Although I am using String.translateIndividualWord() currently in my pr, my concern is that many translations might have numbers that we might miss. For example: 100a is a word, but cannot be parsed with toDouble(). So, this will be missed.
And from the looks of it, I am not getting words here after all.

[touhidurrr]

font issues aside, this is the current update:

As expected, stuff like 0% or 2/2 is broken.

[touhidurrr]

Caching NumberFormat classes is also a problem. It is getting saved to GameSettings.kt which I do not want to.

[SomeTroglodyte]

getting saved to

@Transient

Am recovering from a bricked box thanks to one little mis-keytap trying to fix the new graphics driver issues, plus one timeshift mishap. So, might be a few days until I can catch up here.

[touhidurrr]

@Transient

Where do I put it?

[touhidurrr]

B. Yes, you'll need to add like a .tr() for numbers

Done in 5e47a3a

[touhidurrr]

@yairm210 @SomeTroglodyte need review.

[yairm210]

Please add tests for the following to ensure they remain the same text:

• "1"
• "1.0"
• "+1"
• "-1"

[touhidurrr]

Please add tests for the following to ensure they remain the same text:

• "1"
• "1.0"
• "+1"
• "-1"

As we are using toDoubleOrNull(), I don't think this is possible. +1 would be 1, 1.0 would be 1. Should implement this or it is ok as is?

[touhidurrr]

As expected, stuff like 0% or 2/2 is broken.

"+1", "1.0"

One plan to fix both cases would be something like this: playground

Output:

'1' -> '১'
'+1' -> '+১'
'-1' -> '-১'
'1.0' -> '১.০'
'+1.0' -> '+১.০'
'-1.0' -> '-১.০'
'0%' -> '০%'
'2/2' -> '২/২'
'(4/5)' -> '(৪/৫)'

Should I continue with it?

[touhidurrr]

Although this did not fix cases where String.tr() is not called. This this all fix other cases. From my local test, all previously broken numbers are fixed here. (Except main screen ones where .tr() is not called)

[touhidurrr]

Tell me if I should add this to this pr.

[touhidurrr]

Well, it will commit them anyways. (turned out to be less complex than my initial write).
Tell me what you think!

[yairm210]

There should be nowhere that we need to translate "2/2" or "5%" directly, since we can translate each piece by putting them in square brackets (like "{2}/{2}")
For the examples I listed above, it is important - For example "[+20]% Strength" should be displayed - in English as "+20% Strength" and NOT as "20% Strength", so losing the + because of int parsing -> stringifying is not acceptable

[yairm210]

Looks good, I personally can't think of a problem except for maybe performance, which should hopefully not be a problem