search

phetsims / rosetta

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

Shared strings don't show up in certain scenarios #342

Closed liammulh closed 1 year ago

liammulh commented 1 year ago

In https://github.com/phetsims/qa/issues/869, we discovered that a sim with shared strings won't have a shared strings table. I looked at the code, and I think we need to check if there's a translated file, and if there isn't we should set the shared string to be an empty string.

liammulh commented 1 year ago

As of https://github.com/phetsims/rosetta/commit/85f046cb279884f1693907d03239b59b8c92806c the translation utility seems to show shared strings for Concentration, but not for Geometric Optics.

liammulh commented 1 year ago

The shared strings object in the translation form data was empty on the Geometric Optics translation form page. I need to check and see if categorizedStringKeys.shared is empty for Geometric Optics.

liammulh commented 1 year ago

Okay, I've verified that categorizedStringKeys.shared and categorizedStringKeys.sharedSims are both empty for Geometric Optics in Abkhazian (ab) using the master phetsims/babel branch.

Why? Is the logic for adding them incorrect? I will compare the logic of the old translation utility and the new one. It might be that the old one is determining whether it has shared strings based on sim key in the sim metadata. If so, we could just key into the sim metadata for the given sim, I think.

liammulh commented 1 year ago

I think this might actually be fixed. Geometric Optics: Basics is the sim that has the shared strings.

I need to go through a handful of sims and ensure this is working properly. For the next round of testing, QA should go through the sims on the old translation util, and for each sim with shared strings, they should verify that the new translation util also shows shared strings.

liammulh commented 1 year ago

I believe this is fixed. We will need to test this before publication.

liammulh commented 1 year ago

For future reference, the fix for this issue was in https://github.com/phetsims/rosetta/commit/85f046cb279884f1693907d03239b59b8c92806c. The issue was that if there wasn't a translated file for a sim, the shared English value and the shared translated value weren't being set. Of course, when there is no translated file, the translated value needs to be an empty string.

KatieWoe commented 1 year ago

Bumper and Under Pressure both have shared strings on the main website but not on ox-dev

liammulh commented 1 year ago

Bumper and Under Pressure both have shared strings on the main website but not on ox-dev

Bumper's strings ``` { "en": { "SCENERY_PHET/ResetAllButton.name": "‪Reset All‬", "CHAINS/htmlString": "‪Turning left on red is a bold move.‬", "CHAINS/multilineString": "‪Give me a\nbreak.‬", "CHAINS/namedPlaceholdersString": "‪{{name}} is traveling at {{speed}} km/hr‬", "CHAINS/patternString": "‪{0}: {1} {2}‬", "CHAINS/plainString": "‪It's not easy being green.‬", "CHAINS/size": "‪Size‬", "SCENERY_PHET/units_nm": "‪nm‬", "JOIST/termsPrivacyAndLicensing": "‪Terms, Privacy & Licensing‬", "JOIST/translation.credits.link": "‪Translation Credits‬", "JOIST/thirdParty.credits.link": "‪Third-party Credits‬", "SCENERY_PHET/key.pageDown": "‪Pg Down‬", "SCENERY_PHET/key.pageUp": "‪Pg Up‬", "SCENERY_PHET/key.shift": "‪Shift‬", "SCENERY_PHET/keyboardHelpDialog.or": "‪or‬", "SCENERY_PHET/key.tab": "‪Tab‬", "JOIST/keyboardShortcuts.title": "‪Keyboard Shortcuts‬", "JOIST/keyboardShortcuts.toGetStarted": "‪to get started‬", "JOIST/credits.contributors": "‪Contributors: {0}‬", "JOIST/credits.graphicArts": "‪Graphic Arts: {0}‬", "JOIST/credits.leadDesign": "‪Lead Design: {0}‬", "JOIST/credits.qualityAssurance": "‪Quality Assurance: {0}‬", "JOIST/credits.softwareDevelopment": "‪Software Development: {0}‬", "JOIST/credits.soundDesign": "‪Sound Design: {0}‬", "JOIST/credits.team": "‪Team: {0}‬", "JOIST/credits.thanks": "‪Thanks‬", "JOIST/credits.title": "‪Credits‬", "JOIST/credits.translation": "‪Translation‬", "JOIST/updates.checking": "‪Checking for updates…‬", "JOIST/updates.getUpdate": "‪Get Update…‬", "JOIST/updates.newVersionAvailable": "‪There is a new version available: {0}.‬", "JOIST/updates.noThanks": "‪No Thanks‬", "JOIST/updates.offline": "‪Unable to check for updates.‬", "JOIST/updates.outOfDate": "‪New version available‬", "JOIST/updates.upToDate": "‪This simulation is up to date.‬", "JOIST/updates.yourCurrentVersion": "‪Your current version is: {0}.‬", "JOIST/versionPattern": "‪version {0}‬", "JOIST/options.title": "‪Options‬", "JOIST/menuItem.about": "‪About…‬", "JOIST/menuItem.enhancedSound": "‪Enhanced Sound‬", "JOIST/menuItem.fullscreen": "‪Full Screen‬", "JOIST/menuItem.getUpdate": "‪Check for Updates…‬", "JOIST/menuItem.mailInputEventsLog": "‪Mail Input Events Log‬", "JOIST/menuItem.options": "‪Options…‬", "JOIST/menuItem.outputInputEventsLog": "‪Output Input Events Log‬", "JOIST/menuItem.phetWebsite": "‪PhET Website…‬", "JOIST/menuItem.reportAProblem": "‪Report a Problem…‬", "JOIST/menuItem.screenshot": "‪Screenshot‬", "JOIST/menuItem.submitInputEventsLog": "‪Submit Input Events Log‬", "JOIST/simTitleWithScreenNamePattern": "‪{{simName}} — {{screenName}}‬", "BUMPER/bumper.title": "‪Bumper‬" } } ```

Based on the above, it looks like Bumper should share strings with Chains. I looked at Bumper on Ox-Dev, and it would seem at least some of the shared strings are ending up in the common table. Are all shared strings in the common table?

Under Pressure's strings ``` { "en": { "JOIST/HomeButton.name": "‪Home‬", "JOIST/adaptedFrom": "‪adapted\nfrom‬", "JOIST/termsPrivacyAndLicensing": "‪Terms, Privacy & Licensing‬", "JOIST/translation.credits.link": "‪Translation Credits‬", "JOIST/thirdParty.credits.link": "‪Third-party Credits‬", "JOIST/credits.title": "‪Credits‬", "JOIST/credits.leadDesign": "‪Lead Design: {0}‬", "JOIST/credits.softwareDevelopment": "‪Software Development: {0}‬", "JOIST/credits.team": "‪Team: {0}‬", "JOIST/credits.qualityAssurance": "‪Quality Assurance: {0}‬", "JOIST/credits.graphicArts": "‪Graphic Arts: {0}‬", "JOIST/credits.translation": "‪Translation‬", "JOIST/credits.thanks": "‪Thanks‬", "JOIST/updates.upToDate": "‪This simulation is up to date.‬", "JOIST/updates.outOfDate": "‪New version available‬", "JOIST/updates.checking": "‪Checking for updates…‬", "JOIST/updates.offline": "‪Unable to check for updates.‬", "JOIST/updates.newVersionAvailable": "‪There is a new version available: {0}.‬", "JOIST/updates.yourCurrentVersion": "‪Your current version is: {0}.‬", "JOIST/updates.getUpdate": "‪Get Update…‬", "JOIST/updates.noThanks": "‪No Thanks‬", "JOIST/versionPattern": "‪version {0}‬", "JOIST/options.title": "‪Options‬", "JOIST/showPointers": "‪Show Pointers‬", "JOIST/done": "‪Done‬", "JOIST/title.settings": "‪Settings‬", "JOIST/menuItem.options": "‪Options…‬", "JOIST/menuItem.about": "‪About…‬", "JOIST/menuItem.mailInputEventsLog": "‪Mail Input Events Log‬", "JOIST/menuItem.outputInputEventsLog": "‪Output Input Events Log‬", "JOIST/menuItem.phetWebsite": "‪PhET Website…‬", "JOIST/menuItem.reportAProblem": "‪Report a Problem…‬", "JOIST/menuItem.screenshot": "‪Screenshot‬", "JOIST/menuItem.fullscreen": "‪Full Screen‬", "JOIST/menuItem.settings": "‪Settings…‬", "JOIST/menuItem.getUpdate": "‪Check for Updates…‬", "JOIST/menuItem.submitInputEventsLog": "‪Submit Input Events Log‬", "JOIST/PhetButton.name": "‪PhET Menu Button‬", "JOIST/titlePattern": "‪{0} {1}‬", "FLUID_PRESSURE_AND_FLOW/atm": "‪atm‬", "FLUID_PRESSURE_AND_FLOW/psi": "‪psi‬", "FLUID_PRESSURE_AND_FLOW/kPa": "‪kPa‬", "FLUID_PRESSURE_AND_FLOW/valueWithUnitsPattern": "‪{0} {1}‬", "FLUID_PRESSURE_AND_FLOW/ftPerSPerS": "‪ft/s2‬", "FLUID_PRESSURE_AND_FLOW/mPerSPerS": "‪m/s2‬", "FLUID_PRESSURE_AND_FLOW/densityUnitsEnglish": "‪lb/ft3‬", "FLUID_PRESSURE_AND_FLOW/densityUnitsMetric": "‪kg/m3‬", "FLUID_PRESSURE_AND_FLOW/m": "‪m‬", "FLUID_PRESSURE_AND_FLOW/ft": "‪ft‬", "FLUID_PRESSURE_AND_FLOW/atmosphere": "‪Atmosphere‬", "FLUID_PRESSURE_AND_FLOW/on": "‪On‬", "FLUID_PRESSURE_AND_FLOW/off": "‪Off‬", "FLUID_PRESSURE_AND_FLOW/grid": "‪Grid‬", "FLUID_PRESSURE_AND_FLOW/ruler": "‪Ruler‬", "FLUID_PRESSURE_AND_FLOW/metric": "‪Metric‬", "FLUID_PRESSURE_AND_FLOW/english": "‪English‬", "FLUID_PRESSURE_AND_FLOW/units": "‪Units‬", "FLUID_PRESSURE_AND_FLOW/atmospheres": "‪Atmospheres‬", "SCENERY_PHET/ResetAllButton.name": "‪Reset All‬", "FLUID_PRESSURE_AND_FLOW/pressure": "‪Pressure‬", "FLUID_PRESSURE_AND_FLOW/readoutMeters": "‪{0} m‬", "FLUID_PRESSURE_AND_FLOW/readoutFeet": "‪{0} ft‬", "FLUID_PRESSURE_AND_FLOW/massLabelPattern": "‪{0} kg‬", "FLUID_PRESSURE_AND_FLOW/mysteryFluid": "‪Mystery Fluid‬", "FLUID_PRESSURE_AND_FLOW/mysteryPlanet": "‪Mystery Planet‬", "FLUID_PRESSURE_AND_FLOW/planetA": "‪Planet A‬", "FLUID_PRESSURE_AND_FLOW/planetB": "‪Planet B‬", "FLUID_PRESSURE_AND_FLOW/planetC": "‪Planet C‬", "FLUID_PRESSURE_AND_FLOW/fluidA": "‪Fluid A‬", "FLUID_PRESSURE_AND_FLOW/fluidB": "‪Fluid B‬", "FLUID_PRESSURE_AND_FLOW/fluidC": "‪Fluid C‬", "FLUID_PRESSURE_AND_FLOW/fluidDensity": "‪Fluid Density‬", "FLUID_PRESSURE_AND_FLOW/gravity": "‪Gravity‬", "FLUID_PRESSURE_AND_FLOW/earth": "‪Earth‬", "FLUID_PRESSURE_AND_FLOW/mars": "‪Mars‬", "FLUID_PRESSURE_AND_FLOW/jupiter": "‪Jupiter‬", "FLUID_PRESSURE_AND_FLOW/gasoline": "‪gasoline‬", "FLUID_PRESSURE_AND_FLOW/water": "‪water‬", "FLUID_PRESSURE_AND_FLOW/honey": "‪honey‬", "FLUID_PRESSURE_AND_FLOW/underPressureScreenTitle": "‪Under Pressure‬", "UNDER_PRESSURE/under-pressure.title": "‪Under Pressure‬" } } ```

Based on the above, it looks like Under Pressure should share strings with Fluid Pressure and Flow.

liammulh commented 1 year ago

I need to look at getCategorizedStrings.js.

liammulh commented 1 year ago

Are all shared strings in the common table (for Bumper)?

liammulh commented 1 year ago

Are all shared strings in the common table (for Under Pressure)?

liammulh commented 1 year ago

Based on the above investigation, I think it's reasonable to assume that somehow the strings that should be shared strings are being categorized as common strings. How are these strings being categorized as common rather than shared?

liammulh commented 1 year ago

Here's the relevant code in getCategorizedStringKeys.js:

if ( repoName === simName ) {
  logger.verbose( `categorizing sim-specific string key: ${stringKeyWithRepoName}` );
  categorizedStringKeys.simSpecific.push( stringKey );
}
else if ( simNames.includes( repoName ) ) {
  logger.verbose( `categorizing shared string key: ${stringKeyWithRepoName}` );
  categorizedStringKeys.shared.push( stringKey );
  if ( !categorizedStringKeys.sharedSims.includes( repoName ) ) {
    categorizedStringKeys.sharedSims.push( repoName );
  }
}
else {
  logger.verbose( `categorizing common string key: ${stringKeyWithRepoName}` );
  categorizedStringKeys.common.push( stringKey );
}
liammulh commented 1 year ago

Neither Chains nor Fluid Pressure and Flow are in the list of sims being passed to getCategorizedStringKeys.js:

simNames (list of sims) ``` ----------> acid-base-solutions ----------> area-builder ----------> area-model-algebra ----------> area-model-decimals ----------> area-model-introduction ----------> area-model-multiplication ----------> arithmetic ----------> atomic-interactions ----------> balancing-act ----------> balancing-chemical-equations ----------> balloons-and-static-electricity ----------> beers-law-lab ----------> bending-light ----------> blackbody-spectrum ----------> build-a-fraction ----------> build-a-molecule ----------> build-a-nucleus ----------> build-an-atom ----------> capacitor-lab-basics ----------> center-and-variability ----------> charges-and-fields ----------> circuit-construction-kit-ac ----------> circuit-construction-kit-ac-virtual-lab ----------> circuit-construction-kit-dc ----------> circuit-construction-kit-dc-virtual-lab ----------> collision-lab ----------> color-vision ----------> concentration ----------> coulombs-law ----------> curve-fitting ----------> density ----------> diffusion ----------> energy-forms-and-changes ----------> energy-skate-park ----------> energy-skate-park-basics ----------> equality-explorer ----------> equality-explorer-basics ----------> equality-explorer-two-variables ----------> expression-exchange ----------> faradays-law ----------> forces-and-motion-basics ----------> fourier-making-waves ----------> fraction-matcher ----------> fractions-equality ----------> fractions-intro ----------> fractions-mixed-numbers ----------> friction ----------> function-builder ----------> function-builder-basics ----------> gas-properties ----------> gases-intro ----------> gene-expression-essentials ----------> geometric-optics ----------> geometric-optics-basics ----------> graphing-lines ----------> graphing-quadratics ----------> graphing-slope-intercept ----------> gravity-and-orbits ----------> gravity-force-lab ----------> gravity-force-lab-basics ----------> greenhouse-effect ----------> hookes-law ----------> isotopes-and-atomic-mass ----------> john-travoltage ----------> least-squares-regression ----------> make-a-ten ----------> masses-and-springs ----------> masses-and-springs-basics ----------> mean-share-and-balance ----------> molarity ----------> molecule-polarity ----------> molecule-shapes ----------> molecule-shapes-basics ----------> molecules-and-light ----------> my-solar-system ----------> natural-selection ----------> neuron ----------> normal-modes ----------> number-line-distance ----------> number-line-integers ----------> number-line-operations ----------> number-play ----------> ohms-law ----------> pendulum-lab ----------> ph-scale ----------> ph-scale-basics ----------> plinko-probability ----------> projectile-motion ----------> proportion-playground ----------> ratio-and-proportion ----------> reactants-products-and-leftovers ----------> resistance-in-a-wire ----------> rutherford-scattering ----------> states-of-matter ----------> states-of-matter-basics ----------> trig-tour ----------> under-pressure ----------> unit-rates ----------> vector-addition ----------> vector-addition-equations ----------> wave-interference ----------> wave-on-a-string ----------> waves-intro ```
liammulh commented 1 year ago

Neither Bumper nor Fluid Pressure and Flow are in the list of sims being passed to getCategorizedStringKeys.js.

This is because they are not supposed to be in the list of sims if you aren't a team member.

The way this is happening is:

liammulh commented 1 year ago

It wouldn't be very DRY, but we could sort of replicate the code for getting sim names and titles in getCategorizedStringKeys.js, but instead of checking for whether the person is a team member, just get the full list of sims, even the invisible ones. I don't know if this would cause other problems. I'll have to think about what the best solution to this would be.

liammulh commented 1 year ago

Maybe we do something like:

  const simMetadata = await getSimMetadata();
  const simNames = Object.keys( getSimNamesAndTitles( simMetadata, 'true' ) );
liammulh commented 1 year ago

Done (I think) in https://github.com/phetsims/rosetta/commit/cb3956313c1f2255106673f9c74b907b99911143.

liammulh commented 1 year ago

It's okay for Under Pressure's shared strings to be in the common category.

@jbphet and I reviewed this. The strings for FPAF in Under Pressure will be categorized as common strings since that is the "fallback" category if they can't be categorized as shared strings. To be categorized as a shared string, the string's repo needs to be in the list of all sims, and FPAF is never in that list, thus it falls into the common category.

KatieWoe commented 1 year ago

Things look good in Under Pressure and two random sims. Bumper couldn't be tested since sign ins don't work right now, so leaving open.

stemilymill commented 1 year ago

looks good on bumper as well as various other sims 👍