Image localization and regionAndCulture

[jonathanolson]

Issue for tracking commits and review. Implementing @pixelzoom's vision for regionAndCulture.

[jonathanolson]

Initial implementation above (on main).

I've added a stub that will hook into any regionAndCulturePortrayalProperty (so that it is kept in-sync with the regionAndCultureProperty that was added). Presumably after things are ported over, we won't need that other Property.

I made the decision to only instrument the main regionAndCultureProperty if it has multiple valid values (will be defined/featured similarly to regionAndCulturePortrayalProperty).

Right now it is a fairly independent system from the current portrayal system. Demo in wilder.

[jonathanolson]

I'll review and refine soon with @pixelzoom. @marlitas I'd be curious to get your opinion on the above commits as well (and whether you have any concerns or misgivings about them).

[marlitas]

I've added a stub that will hook into any regionAndCulturePortrayalProperty (so that it is kept in-sync with the regionAndCultureProperty that was added). Presumably after things are ported over, we won't need that other Property.

I'm glad I re-read this because the bidirectional link between regionAndCulturePortrayalProperty and regionAndCultureProperty was very concerning. I am adding a TODO referencing this issue (can be moved elsewhere if needed) so that we don't forget to adjust.

I was also curious about the change set in initialize-globals:

- validValues: packageSimFeatures.supportedRegionsAndCultures || [ null ],
- defaultValue: packageSimFeatures.supportedRegionsAndCultures?.[ 0 ] ?
                    packageSimFeatures.supportedRegionsAndCultures[ 0 ] : null
+ defaultValue: 'usa'

• Is there a reason validValues was removed?
• The default value is not always 'usa'. For example, in Number Line Integers the default values is 'multi'. 'usa' should not be hard coded in.

[pixelzoom]

@jonathanolson and I paired on this for 1.75 hours this morning, commits above. I'll review joist/js/i18n/ and start transitioning graphing-lines.

[pixelzoom]

In phetsims/graphing-lines#143, I moved graphing-lines and graphing-slope-intercept to the new common code. Here's a list of the things that I ran into.

• (1) The output file from grunt modulify was not in pascal case. It should be GraphingLinesImages.ts (like GraphingLinesStrings.ts) but was instead graphingLinesImages.ts. I fixed that in phetsims/chipper@14bb553.

• (2) The entries in GraphingLinesImages.ts did not have "ImageProperty" suffixes on their names. For example, "level1" instead of "level1ImageProperty". This is analagous to strings, were we have slopeStringProperty etc. I fixed that in phetsims/chipper@14bb553.

• (3) The Localization tab does not appear in Preferences unless you run with &locales=*. The Localization tab, and Region and Culture combo box, need to appear in all version of the sim if package.json contains supportedRegionsAndCultures. I did not investigate fixing this.

• (4) Image files must have unique basenames. So most of the work was renaming and reorganizing files. I think this is OK. Let's come up with a standard for naming and propose it to designers. For now, I use the RegionAndCulture values as a prefix, e.g. "africaLevel1_svg.ts". And I used "africaModest" and "latinAmerica", instead of "africa-modest" and "latin-america", to be consistent with RegionAndCulture type values.

• (5) graphing-slope-intercept was expecting to get its "usa" images from graphing-lines, and that's not supported. My solution was to copy the "usa" images to graphing-slope-intercept.

• (6) The term "region and culture" is really problematic in code. We're ending up long, unreadable, inconsistent code names, e.g. supportedRegionsAndCultures (plural "Regions") in package.json and availableRuntimeRegionAndCultures (singular "Region") in regionAndCultureProperty. I wish we could come up with a different name in code. Maybe we could just shorten it to "region" or "culture".

@jonathanolson let's discuss 3, 4, 5, and 6.

[pixelzoom]

(3) The Localization tab does not appear in Preferences unless you run with &locales=*. The Localization tab, and Region and Culture combo box, need to appear in all version of the sim if package.json contains supportedRegionsAndCultures. I did not investigate fixing this.

I addressed this in fcddee0. The Localization tab appears in Preferences if its appropriate to show either regionAndCultureComboBox or localePanel.

[pixelzoom]

Renaming of image files is done, with the exception of soccer-common. The 'usa' images for soccer-common have a problem (see phetsims/soccer-common#13) so I'll need to circle back to rename those files.

I followed convoluted path to get to the final naming convention.

Putting all files in images/localization/ turned out to have some problems:

• odd sorting of 'africa' and 'africaModest' files, where they were interleaved
• lots of files in 1 directory and a huge license.json files

So I ended up leaving files in a directory per region: images/africa/, images/africaModest, images/asia, etc. These seems fine, and serves the same purpose as the proposed localized/ directory - to differentiate between localized and non-localized images.

[pixelzoom]

Here's the bash script that I used to rename image files. I fixed up license.json and RegionAndCulturePortrayal subclasses using global string replacement in WebStorm.

#!/bin/bash

# First command line argument is the prefix to be added to each file.
PREFIX=${1}

# Allow patterns that match no files, so we can iterate over both .png and .svg
shopt -s nullglob

for filename in *.{png,svg}; do

 # Convert the first character of the filename to upper case.
 UPPER=`echo ${filename} | awk '{ print toupper( substr( $0, 1, 1 ) ) substr( $0, 2 ); }'`

 # Add the prefix.
 NEW_FILENAME=${PREFIX}${UPPER}

 # Rename the file, preserving git history.
 git mv ${filename} ${NEW_FILENAME}
done

[pixelzoom]

Remaining work:

• Convert sims to new Region and Culture approach #958

• Delete vestiges of the old implementation. Address remaining TODOs that reference this issue. Delete RegionAndCulturePortrayal.ts.

[pixelzoom]

All sims have been converted to the new approach. So I removed the old implementation from common code in the above commits.

There are references to regionAndCulturePortrayalProperty and preferences.regionAndCulturePortrayals in getOverridingQueryParameters.ts. The code is poorly documented, so it's unclear whether any changes need to be made. I've reopened https://github.com/phetsims/phet-io-wrappers/issues/637 and asked @zepumph for clarification (and documentation).

[pixelzoom]

The last bit of cleanup (getOverridingQueryParameters.ts) will be handled by @zepumph in https://github.com/phetsims/phet-io-wrappers/issues/637. So closing this issue.