Add a query parameter to set initial character set?

[jessegreenberg]

I heard mention of this in the team meeting today. That is not currently supported, is this a required feature @arouinfar?

[arouinfar]

I think it is an aspirational feature @jessegreenberg. As I understand it, the grant requirements are for character set localization, and we've met that goal.

We can defer this to the next release which will include dynamic locale and unification with/redeployment of the Basics version.

[marlitas]

@amanda-phet would we like to include this type of work when adding new character sets? I don't believe it would be difficult to add query parameter support for choosing a character set in all the sims we are hitting.

[marlitas]

I went ahead and did this. It's now ready to test and review.

[zepumph]

Before we do this. I recommend factoring it out using the same strategy as other features phet has. @marlitas and I talked about first adding it as a package.json phetFeatures. Then a general query parameter that allows for a graceful default that sims can use to opt into for support. I'd also like to move this issue to joist/ if that's ok.

Some thoughts on work to do:

• Add an entry for regionAndCulture in simFeature.
• Values should likely be a list of valid values, where the first one is the default when no query parameter is specified
• RegionAndCulturePortrayal could have some static functionality to validate that the simFeature entry in package.json is correct (consistent set of strings across all sims like "asia, africa, etc".
• In general simFeature defines a default if desired, and the query parameter uses that default, and then all usages use the query parameter from initialize globals .

Questions:

• How exactly is the best way to have the instrumented Property have the right validValues for each sim?
• How many sims should we go back and clean up to utilize this consistency (probably all of them).
• This issue was out for review. What design feedback do we need here, and does it need to be done before this refactor (scheduled to be done on Monday by @marlitas and @zepumph).

Thanks all!

[zepumph]

• [ ] I believe this query parameter should be public:true. Is that right @amanda-phet?

[zepumph]

• [x] I think we will be able to remove options.localizationOptions.queryParameterValue since that was the strategy to support sim-specific query parameters.

[zepumph]

• [x] ~Can we have a singleton property instead of createRegionAndCulturePortrayalProperty? I'm not sure. PhET-iO instrumentation may be too awkward to accomplish that.~ No

[marlitas]

```diff Subject: [PATCH] Update license, see: hhttps://github.com/phetsims/energy-skate-park/issues/374 --- Index: joist/js/preferences/PreferencesModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/joist/js/preferences/PreferencesModel.ts b/joist/js/preferences/PreferencesModel.ts --- a/joist/js/preferences/PreferencesModel.ts (revision 3c8944dc6796e35f21b6d1ab70ae1d57318662c6) +++ b/joist/js/preferences/PreferencesModel.ts (date 1700502979902) @@ -244,7 +244,7 @@ characterSets: [], customPreferences: [], includeLocalePanel: true, - queryParameterValue: null + queryParameterValue: window.phet.chipper.queryParameters.regionAndCulture }, providedOptions.localizationOptions ) }; Index: chipper/js/initialize-globals.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/chipper/js/initialize-globals.js b/chipper/js/initialize-globals.js --- a/chipper/js/initialize-globals.js (revision d2298dd693057dc2dab66e0122b8a69192dbbef9) +++ b/chipper/js/initialize-globals.js (date 1700502572942) @@ -470,6 +470,15 @@ defaultValue: Math.random() // eslint-disable-line bad-sim-text }, + // This query parameter sets the region and culture portrayal for the sim. This changes the artwork for the character + // sets that appear throughout the sim. + regionAndCulture: { + type: 'string', + validValues: packageSimFeatures.supportedRegionsAndCultures || [ null ], + defaultValue: packageSimFeatures.supportedRegionsAndCultures && packageSimFeatures.supportedRegionsAndCultures[ 0 ] ? + packageSimFeatures.supportedRegionsAndCultures[ 0 ] : null + }, + /** * Specify a renderer for the Sim's rootNode to use. */ Index: number-line-integers/package.json IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/package.json b/number-line-integers/package.json --- a/number-line-integers/package.json (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/package.json (date 1700502395539) @@ -27,6 +27,17 @@ "NUMBER_LINE_INTEGERS/screen.explore", "NUMBER_LINE_INTEGERS/screen.generic" ], + "simFeatures": { + "supportedRegionsAndCultures": [ + "multi", + "africa", + "africaModest", + "asia", + "latinAmerica", + "oceania", + "usa" + ] + }, "supportsDynamicLocale": true }, "eslintConfig": { Index: number-line-integers/js/explore/view/ExplorerCharacterSetLatinAmerica.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/js/explore/view/ExplorerCharacterSetLatinAmerica.js b/number-line-integers/js/explore/view/ExplorerCharacterSetLatinAmerica.js --- a/number-line-integers/js/explore/view/ExplorerCharacterSetLatinAmerica.js (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/js/explore/view/ExplorerCharacterSetLatinAmerica.js (date 1700502572908) @@ -12,7 +12,7 @@ import boyInWater_png from '../../../images/latin-america/boyInWater_png.js'; import exploreScreenHome_png from '../../../images/latin-america/exploreScreenHome_png.js'; import exploreScreenNav_png from '../../../images/latin-america/exploreScreenNav_png.js'; -import { LATIN_AMERICA_QUERY_VALUE } from '../../common/NLIQueryParameters.js'; +import { LATIN_AMERICA_QUERY_VALUE } from '../../../../joist/js/preferences/RegionAndCulturePortrayal.js'; import NumberLineIntegersStrings from '../../NumberLineIntegersStrings.js'; import ExplorerCharacterSet from './ExplorerCharacterSet.js'; Index: number-line-integers/js/number-line-integers-main.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/js/number-line-integers-main.js b/number-line-integers/js/number-line-integers-main.js --- a/number-line-integers/js/number-line-integers-main.js (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/js/number-line-integers-main.js (date 1700502979890) @@ -20,8 +20,7 @@ const preferencesModel = new PreferencesModel( { localizationOptions: { - characterSets: ExplorerImages.EXPLORER_CHARACTER_SETS, - queryParameterValue: NLIQueryParameters.regionAndCulture + characterSets: ExplorerImages.EXPLORER_CHARACTER_SETS } } ); Index: number-line-integers/js/explore/view/ExplorerCharacterSetAsia.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/js/explore/view/ExplorerCharacterSetAsia.js b/number-line-integers/js/explore/view/ExplorerCharacterSetAsia.js --- a/number-line-integers/js/explore/view/ExplorerCharacterSetAsia.js (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/js/explore/view/ExplorerCharacterSetAsia.js (date 1700502572921) @@ -12,7 +12,7 @@ import girlHiking_png from '../../../images/asia/girlHiking_png.js'; import girlInAir_png from '../../../images/asia/girlInAir_png.js'; import girlInWater_png from '../../../images/asia/girlInWater_png.js'; -import { ASIA_QUERY_VALUE } from '../../common/NLIQueryParameters.js'; +import { ASIA_QUERY_VALUE } from '../../../../joist/js/preferences/RegionAndCulturePortrayal.js'; import NumberLineIntegersStrings from '../../NumberLineIntegersStrings.js'; import ExplorerCharacterSet from './ExplorerCharacterSet.js'; Index: number-line-integers/js/explore/view/ExplorerCharacterSetAfrica.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/js/explore/view/ExplorerCharacterSetAfrica.js b/number-line-integers/js/explore/view/ExplorerCharacterSetAfrica.js --- a/number-line-integers/js/explore/view/ExplorerCharacterSetAfrica.js (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/js/explore/view/ExplorerCharacterSetAfrica.js (date 1700502572928) @@ -7,9 +7,9 @@ */ +import { AFRICA_QUERY_VALUE } from '../../../../joist/js/preferences/RegionAndCulturePortrayal.js'; import exploreScreenHome_png from '../../../images/africa/exploreScreenHome_png.js'; import exploreScreenNav_png from '../../../images/africa/exploreScreenNav_png.js'; -import { AFRICA_QUERY_VALUE } from '../../common/NLIQueryParameters.js'; import ExplorerCharacterSet from './ExplorerCharacterSet.js'; import NumberLineIntegersStrings from '../../NumberLineIntegersStrings.js'; import girlHiking_png from '../../../images/africa/girlHiking_png.js'; Index: number-line-integers/js/explore/view/ExplorerCharacterSetOceania.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/js/explore/view/ExplorerCharacterSetOceania.js b/number-line-integers/js/explore/view/ExplorerCharacterSetOceania.js --- a/number-line-integers/js/explore/view/ExplorerCharacterSetOceania.js (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/js/explore/view/ExplorerCharacterSetOceania.js (date 1700502572925) @@ -13,7 +13,7 @@ import girlHiking_png from '../../../images/oceania/girlHiking_png.js'; import girlInAir_png from '../../../images/oceania/girlInAir_png.js'; import girlInWater_png from '../../../images/oceania/girlInWater_png.js'; -import { OCEANIA_QUERY_VALUE } from '../../common/NLIQueryParameters.js'; +import { OCEANIA_QUERY_VALUE } from '../../../../joist/js/preferences/RegionAndCulturePortrayal.js'; import NumberLineIntegersStrings from '../../NumberLineIntegersStrings.js'; import ExplorerCharacterSet from './ExplorerCharacterSet.js'; Index: number-line-integers/js/explore/view/ExplorerCharacterSetAfricaModest.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/js/explore/view/ExplorerCharacterSetAfricaModest.js b/number-line-integers/js/explore/view/ExplorerCharacterSetAfricaModest.js --- a/number-line-integers/js/explore/view/ExplorerCharacterSetAfricaModest.js (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/js/explore/view/ExplorerCharacterSetAfricaModest.js (date 1700502572947) @@ -12,7 +12,7 @@ import girlHiking_png from '../../../images/africa-conservative/girlHiking_png.js'; import girlInAir_png from '../../../images/africa-conservative/girlInAir_png.js'; import girlInWater_png from '../../../images/africa-conservative/girlInWater_png.js'; -import { AFRICA_MODEST_QUERY_VALUE } from '../../common/NLIQueryParameters.js'; +import { AFRICA_MODEST_QUERY_VALUE } from '../../../../joist/js/preferences/RegionAndCulturePortrayal.js'; import NumberLineIntegersStrings from '../../NumberLineIntegersStrings.js'; import ExplorerCharacterSet from './ExplorerCharacterSet.js'; Index: number-line-integers/number-line-integers_en.html IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/number-line-integers_en.html b/number-line-integers/number-line-integers_en.html --- a/number-line-integers/number-line-integers_en.html (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/number-line-integers_en.html (date 1700502858159) @@ -50,6 +50,17 @@ "NUMBER_LINE_INTEGERS/screen.explore", "NUMBER_LINE_INTEGERS/screen.generic" ], + "simFeatures": { + "supportedRegionsAndCultures": [ + "multi", + "africa", + "africaModest", + "asia", + "latinAmerica", + "oceania", + "usa" + ] + }, "supportsDynamicLocale": true }, "eslintConfig": { Index: number-line-integers/js/explore/view/ExplorerCharacterSetUSA.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/js/explore/view/ExplorerCharacterSetUSA.js b/number-line-integers/js/explore/view/ExplorerCharacterSetUSA.js --- a/number-line-integers/js/explore/view/ExplorerCharacterSetUSA.js (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/js/explore/view/ExplorerCharacterSetUSA.js (date 1700502572945) @@ -12,7 +12,7 @@ import girlHiking_png from '../../../images/usa/girlHiking_png.js'; import girlInAir_png from '../../../images/usa/girlInAir_png.js'; import girlInWater_png from '../../../images/usa/girlInWater_png.js'; -import { USA_QUERY_VALUE } from '../../common/NLIQueryParameters.js'; +import { USA_QUERY_VALUE } from '../../../../joist/js/preferences/RegionAndCulturePortrayal.js'; import NumberLineIntegersStrings from '../../NumberLineIntegersStrings.js'; import ExplorerCharacterSet from './ExplorerCharacterSet.js'; Index: number-line-integers/js/common/NLIQueryParameters.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/js/common/NLIQueryParameters.js b/number-line-integers/js/common/NLIQueryParameters.js --- a/number-line-integers/js/common/NLIQueryParameters.js (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/js/common/NLIQueryParameters.js (date 1700502572914) @@ -8,14 +8,6 @@ import numberLineIntegers from '../numberLineIntegers.js'; -export const USA_QUERY_VALUE = 'usa'; -export const AFRICA_QUERY_VALUE = 'africa'; -export const AFRICA_MODEST_QUERY_VALUE = 'africaModest'; -export const ASIA_QUERY_VALUE = 'asia'; -export const LATIN_AMERICA_QUERY_VALUE = 'latinAmerica'; -export const OCEANIA_QUERY_VALUE = 'oceania'; -export const MULTICULTURAL_QUERY_VALUE = 'multi'; - const NLIQueryParameters = QueryStringMachine.getAll( { /** @@ -26,14 +18,6 @@ defaultCelsius: { type: 'flag', public: true - }, - - // This query parameter sets the region and culture portrayal for the sim. This changes the artwork for the character - // sets that appear throughout the sim. - regionAndCulture: { - type: 'string', - validValues: [ MULTICULTURAL_QUERY_VALUE, AFRICA_QUERY_VALUE, AFRICA_MODEST_QUERY_VALUE, ASIA_QUERY_VALUE, LATIN_AMERICA_QUERY_VALUE, OCEANIA_QUERY_VALUE, USA_QUERY_VALUE ], - defaultValue: MULTICULTURAL_QUERY_VALUE } } ); Index: joist/js/preferences/RegionAndCulturePortrayal.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/joist/js/preferences/RegionAndCulturePortrayal.ts b/joist/js/preferences/RegionAndCulturePortrayal.ts --- a/joist/js/preferences/RegionAndCulturePortrayal.ts (revision 3c8944dc6796e35f21b6d1ab70ae1d57318662c6) +++ b/joist/js/preferences/RegionAndCulturePortrayal.ts (date 1700503886496) @@ -19,6 +19,24 @@ type SelfOptions = EmptySelfOptions; export type RegionAndCulturePortrayalOptions = SelfOptions & PhetioObjectOptions; +export const USA_QUERY_VALUE = 'usa'; +export const AFRICA_QUERY_VALUE = 'africa'; +export const AFRICA_MODEST_QUERY_VALUE = 'africaModest'; +export const ASIA_QUERY_VALUE = 'asia'; +export const LATIN_AMERICA_QUERY_VALUE = 'latinAmerica'; +export const OCEANIA_QUERY_VALUE = 'oceania'; +export const MULTICULTURAL_QUERY_VALUE = 'multi'; + +const SUPPORTED_REGIONS_AND_CULTURES = [ USA_QUERY_VALUE, AFRICA_QUERY_VALUE, AFRICA_MODEST_QUERY_VALUE, + ASIA_QUERY_VALUE, LATIN_AMERICA_QUERY_VALUE, OCEANIA_QUERY_VALUE, MULTICULTURAL_QUERY_VALUE ]; +const regionAndCultureQueryParameter = window.phet.chipper.queryParameters.regionAndCulture; +const simFeaturesSupportedRegionsAndCultures = window.phet.chipper.packageObject.simFeatures.supportedRegionsAndCultures; + +assert && assert( regionAndCultureQueryParameter === null || SUPPORTED_REGIONS_AND_CULTURES.includes( regionAndCultureQueryParameter ), + 'Query parameter is not included in supported query values for Region and Culture' ); +assert && assert( _.every( simFeaturesSupportedRegionsAndCultures, regionAndCulture => SUPPORTED_REGIONS_AND_CULTURES.includes( regionAndCulture ) ), + 'Invalid value for simFeatures.supportedRegionsAndCultures: ' + simFeaturesSupportedRegionsAndCultures + ' Check RegionAndCulturePortrayal.SUPPORTED_REGIONS_AND_CULTURES.' ); + export default class RegionAndCulturePortrayal extends PhetioObject { Index: number-line-integers/js/explore/view/ExplorerCharacterSetMulti.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/number-line-integers/js/explore/view/ExplorerCharacterSetMulti.js b/number-line-integers/js/explore/view/ExplorerCharacterSetMulti.js --- a/number-line-integers/js/explore/view/ExplorerCharacterSetMulti.js (revision 2d86760bd953b72f8ab61c10d6798f9c893976c7) +++ b/number-line-integers/js/explore/view/ExplorerCharacterSetMulti.js (date 1700502572932) @@ -9,7 +9,7 @@ import dotRandom from '../../../../dot/js/dotRandom.js'; -import { MULTICULTURAL_QUERY_VALUE } from '../../common/NLIQueryParameters.js'; +import { MULTICULTURAL_QUERY_VALUE } from '../../../../joist/js/preferences/RegionAndCulturePortrayal.js'; import NumberLineIntegersStrings from '../../NumberLineIntegersStrings.js'; import ExplorerCharacterSet from './ExplorerCharacterSet.js'; import ExplorerCharacterSetAfrica from './ExplorerCharacterSetAfrica.js'; ```

[zepumph]

After the above commit, ?regionAndCulture is a query parameter available by every sim. The default is null (empty string), so in all sims you can provide http://localhost:8080/friction/friction_en.html?brand=phet&ea&debugger&regionAndCulture=&locales=* and it runs successfully. That is a bit awkward but fine. Valid values are set by simFeatures and validated by a main supported list in RegionAndCulturePortrayal.

There is still some cleanup to do, especially in PreferencesModel. But we are on our way!

[zepumph]

Up next in my book:

• [x] Get rid of all sim usages of ?regionAndCulture, using simFeatures.supportedRegionsAndCultures
• [x] Get rid of localizationOptions.queryParameterValue because you can just inline the chipper query parameter where needed.

• [x] I wrote 4 TODOs pointing to this issue, mostly as questions and next steps.

  @marlitas let me know if you want to discuss anything more here. I'd be happy to pair again if desired.

[marlitas]

I addressed all of the up next checkboxes. Over to @zepumph to review.

[marlitas]

Oops, back to me real quick to clean up some QueryParameter files.

[marlitas]

That was done in the above commits. Back to @zepumph

[zepumph]

In (4), can you please specify the sim or sims that you should use as an example for your new case? https://github.com/phetsims/joist/commit/1d08d5d88be48a3a92f9b7069ffab5e7e38d76cc.

Looks great from a dev perspective. Thanks so much. I'm ready to close, but I'm unsure how much you want to check in and confirm with a designer before closing. Back to you.

[marlitas]

Added some sim examples to that documentation. I think this is ready to close.