phetsims / rosetta

PhET's Simulation Translation Utility
MIT License
3 stars 1 forks source link

Shared string count is wrong for Masses and Springs: Basics in `zh_TW`, `pl`, possibly other locales #393

Closed liammulh closed 1 year ago

liammulh commented 1 year ago

@oliver-phet re-opened https://github.com/phetsims/rosetta/issues/390:

@liammulh the zh_TW translator is still seeing the same issue on 3 sims: Masses and Springs: Basics Molecule Shapes: Basics Molecule Shapes

All 3 seem to have 2 shared strings that aren't available to translate, but are included in the info statistics (all strings are translated). Screenshot_20230408_081345

The issue for Molecule Shapes and Molecule Shapes: Basics is being handled in https://github.com/phetsims/molecule-shapes/issues/252.

Here is what we see when we click the info button on the translation report page for zh_TW for the stats for Masses and Springs: Basics (hereafter referred to as Basics):

stats

There are two missing shared strings. The mother sim to Basics is Masses and Springs (hereafter referred to as Regular). Basics shares most of its sim-specific strings with Regular.

liammulh commented 1 year ago

I added a print statement that should help me figure out which string keys aren't incrementing the number of translated shared strings.

Code as it is now

https://github.com/phetsims/rosetta/blob/59f9751d80238fe55a1d43536cac594694cce16c/src/server/translationApi/translationReport/getTranslationReportObject.js#L159-L171

Code with print statement

      // Loop through the shared string keys and see if any of the translated
      // string keys and values objects have a value for that key.
      for ( const sharedKey of categorizedStringKeys.shared ) {
        for ( const sharedTranslatedStringKeysAndValues of sharedTranslatedStringKeysAndValuesArray ) {
          for ( const sharedEnglishStringKeysAndValues of sharedEnglishStringKeysAndValuesArray ) {
            if ( simName === 'masses-and-springs-basics' ) {

              // eslint-disable-next-line no-debugger
              debugger;
              const shouldIncrementSharedTranslated =
                sharedTranslatedStringKeysAndValues[ sharedKey ] &&
                sharedTranslatedStringKeysAndValues[ sharedKey ] !== '' &&
                sharedEnglishStringKeysAndValues[ sharedKey ] &&
                sharedEnglishStringKeysAndValues[ sharedKey ] !== '' &&
                sharedEnglishStringKeysAndValues[ sharedKey ] !== NO_LONGER_USED_FLAG;
              logger.info( `${sharedKey} — incrementing shared translated count: ${shouldIncrementSharedTranslated}` );
liammulh commented 1 year ago

I am seeing:

info: springConstantPattern — incrementing shared translated count: undefined
info: springStrength — incrementing shared translated count: undefined
liammulh commented 1 year ago

I believe the sharedKey variable is coming from categorizedStringKeys.shared, which I believe comes from the simName, which in this case is masses-and-springs-basics:

https://github.com/phetsims/rosetta/blob/59f9751d80238fe55a1d43536cac594694cce16c/src/server/translationApi/translationReport/getTranslationReportObject.js#L55

liammulh commented 1 year ago

In the Node REPL I see that sharedTranslatedStringKeysAndValues[ sharedKey ] is undefined for springConstantPattern and springStrength:

< info: springConstantPattern — incrementing shared translated count: undefined
< 
break in src/server/translationApi/translationReport/getTranslationReportObject.js:167
 165 
 166               // eslint-disable-next-line no-debugger
>167               debugger;
 168               const shouldIncrementSharedTranslated =
 169                 sharedTranslatedStringKeysAndValues[ sharedKey ] &&
debug> repl
Press Ctrl+C to leave debug repl
> sharedKey
'springStrength'
> sharedTranslatedStringKeysAndValues[ sharedKey ]
undefined
liammulh commented 1 year ago

I am very confused. When I print the object sharedTranslatedStringKeysAndValues:

(The below patch is sloppily coded; the simSpecificTranslatedStringKeysAndValues variable is the one being used for sharedTranslatedStringKeysAndValues.)

< 
break in src/server/translationApi/translationReport/getFoo.js:41
 39 
 40     // eslint-disable-next-line no-debugger
>41     debugger;
 42   }
 43   return simSpecificTranslatedStringKeysAndValues;
debug> repl
Press Ctrl+C to leave debug repl
> JSON.stringify(simSpecificTranslatedStringKeysAndValues, null, 4);
'{\n' +
  '    "movableLine": "可移動的參考線",\n' +
  '    "cm": "公分",\n' +
  '    "body.custom": "自訂",\n' +
  '    "body.earth": "地球",\n' +
  '    "body.jupiter": "木星",\n' +
  '    "body.moon": "月球",\n' +
  '    "body.planetX": "X 星球",\n' +
  '    "dampingEqualsZero": "阻尼 {{equalsZero}}",\n' +
  '    "damping": "阻尼",\n' +
  '    "gravity": "重力",\n' +
  '    "gravityValue": "{{gravity}} m/s<sup>2</sup>",\n' +
  '    "lots": "大",\n' +
  '    "none": "無",\n' +
  '    "whatIsTheValueOfGravity": "重力的值是多少?",\n' +
  '    "displacement": "位移",\n' +
  '    "massEquilibrium": "質量平衡",\n' +
  '    "naturalLength": "自然長度",\n' +
  '    "periodTrace": "紀錄移動區間",\n' +
  '    "massValue": "{{mass}} 克",\n' +
  '    "questionMark": "?",\n' +
  '    "springConstantPattern": "彈性係數 {{spring}}",\n' +
  '    "springStrength": "彈簧強度 {{spring}}",\n' +
  '    "large": "大",\n' +
  '    "small": "小",\n' +
  '    "mass": "質量",\n' +
  '    "elasticPotentialEnergy": "彈性位能",\n' +
  '    "energyGraph": "能量圖",\n' +
  '    "energyLegend": "能量圖例",\n' +
  '    "eTherm": "熱能",\n' +
  '    "eTot": "總能量",\n' +
  '    "gravitationalPotentialEnergy": "重力位能",\n' +
  '    "ke": "動能",\n' +
  '    "kineticEnergy": "動能",\n' +
  '    "peElas": "彈性位能",\n' +
  '    "peGrav": "重力位能",\n' +
  '    "thermalEnergy": "熱能",\n' +
  '    "totalEnergy": "總能量",\n' +
  '    "heightEqualsZero": "高度 = 0 m",\n' +
  '    "acceleration": "加速度",\n' +
  '    "forces": "力",\n' +
  '    "netForce": "淨力",\n' +
  '    "spring": "彈簧",\n' +
  '    "velocity": "速度"\n' +
  '}'
> 

Notably:

  '    "springConstantPattern": "彈性係數 {{spring}}",\n' +
  '    "springStrength": "彈簧強度 {{spring}}",\n' +

The two string keys we thought we undefined are defined...

Patch ``` diff --git a/src/server/translationApi/translationReport/getFoo.js b/src/server/translationApi/translationReport/getFoo.js index e69de29..0fb444b 100644 --- a/src/server/translationApi/translationReport/getFoo.js +++ b/src/server/translationApi/translationReport/getFoo.js @@ -0,0 +1,62 @@ +// Copyright 2023, University of Colorado Boulder + +// import getCategorizedStringKeys from '../getCategorizedStringKeys.js'; +// import getSimHtml from '../getSimHtml.js'; +// import getSimUrl from '../getSimUrl.js'; +// import getStringKeysWithRepoName from '../getStringKeysWithRepoName.js'; + +import logger from '../logger.js'; +import { longTermStorage } from '../translationApi.js'; + +const getFoo = async ( + simName, + locale, + sharedKeys +) => { + logger.info( 'getting foo' ); + const simSpecificTranslatedStringKeysAndValues = {}; + const stringKeysAndTranslatedValues = await longTermStorage.get( simName, locale ); + const translatedStringKeys = Object.keys( stringKeysAndTranslatedValues ); + + // For each sim specific string key... + for ( const stringKey of sharedKeys ) { + + // If the translated string key has a value... + if ( translatedStringKeys.includes( stringKey ) ) { + + // Map the string key to its translated value. + simSpecificTranslatedStringKeysAndValues[ stringKey ] = stringKeysAndTranslatedValues[ stringKey ].value; + } + else { + + // Map the string key to an empty string. + simSpecificTranslatedStringKeysAndValues[ stringKey ] = ''; + } + } + logger.info( `got ${simName}'s sim-specific translated string keys and values; returning them` ); + + if ( simName === 'masses-and-springs' ) { + + // eslint-disable-next-line no-debugger + debugger; + } + return simSpecificTranslatedStringKeysAndValues; +}; + +export default getFoo; + +// ( async () => { +// const sharedSim = 'masses-and-springs'; +// const sim = 'masses-and-springs-basics'; +// const simUrl = getSimUrl( sim ); +// const simHtml = await getSimHtml( simUrl ); +// const sharedStringKeysWithRepoName = Object.keys( getStringKeysWithRepoName( simHtml ) ); +// for ( const item of sharedStringKeysWithRepoName ) { +// console.log( `REPO_NAME/key: ${item}` ); +// } +// const categorizedStringKeys = await getCategorizedStringKeys( sim, sharedStringKeysWithRepoName ); +// console.log( `----------> ${JSON.stringify( categorizedStringKeys, null, 4 )}` ); +// const sharedKeys = categorizedStringKeys.shared; +// console.log( sharedKeys ); +// await getFoo( sharedSim, 'zh_TW', sharedKeys ); +// } )(); \ No newline at end of file diff --git a/src/server/translationApi/translationReport/getTranslationReportObject.js b/src/server/translationApi/translationReport/getTranslationReportObject.js index 47c817b..d474824 100644 --- a/src/server/translationApi/translationReport/getTranslationReportObject.js +++ b/src/server/translationApi/translationReport/getTranslationReportObject.js @@ -11,6 +11,7 @@ import getStringKeysWithRepoName from '../getStringKeysWithRepoName.js'; import logger from '../logger.js'; import getCommonEnglishStringKeysAndValues from './getCommonEnglishStringKeysAndValues.js'; import getCommonTranslatedStringKeysAndValues from './getCommonTranslatedStringKeysAndValues.js'; +import getFoo from './getFoo.js'; import getPercent from './getPercent.js'; import getShortReportObject from './getShortReportObject.js'; import getSimSpecificEnglishStringKeysAndValues from './getSimSpecificEnglishStringKeysAndValues.js'; @@ -148,10 +149,10 @@ const getTranslationReportObject = async ( // Get the translated keys and values. logger.info( `getting shared sim-specific translated string keys and values for ${sharedSim}` ); - const sharedTranslatedStringKeysAndValues = await getSimSpecificTranslatedStringKeysAndValues( + const sharedTranslatedStringKeysAndValues = await getFoo( sharedSim, locale, - sharedCategorizedStringKeys + categorizedStringKeys.shared ); sharedTranslatedStringKeysAndValuesArray.push( sharedTranslatedStringKeysAndValues ); } ```
liammulh commented 1 year ago

@jbphet helped me figure this out. The issue was that the strings in Masses and Springs (Regular) are out of sync with the strings in Masses and Springs: Basics.

The way this happened is that Masses and Springs was published on a branch. On master, some changes were made to the strings in Masses and Springs, and then Masses and Springs: Basics was published off of master.

In Rosetta, we were checking if the shared strings in the child sim were also in the published version of the mother sim. There were two strings that were in the child sim that weren't in the published version of the mother sim, hence the issue.

In https://github.com/phetsims/rosetta/commit/a035b5caed7ca9ffc1102a54811e66cebfa5dea2 I fixed Rosetta so that it doesn't check if the shared strings in the child sim are also in the published version of the mother sim.

We need to republish Masses and Springs from master.

liammulh commented 1 year ago

The work we need to do in Rosetta is done and deployed in https://github.com/phetsims/rosetta/commit/6021626a17d44d4a05a0be5cb4a3cbf1a41ccb17 (2.0.7).

We still need to republish MAS, but we are tracking that in https://github.com/phetsims/masses-and-springs/issues/381.