Region and Culture query parameter should be graceful and future proof

[zepumph]

From conversations in https://docs.google.com/document/d/1-nqTlDtLtPB8JrdDrE8KdJB-4Phwe6QvTV_UnsZrteg/edit and
phetsims/projectile-data-lab#324

Design requirements:

• Vital that if you provide a regionAndCulture that we generally accept in the “Main list”, then it doesn’t provide an “invalid dialog”
• PhET-iO standard wrapper must support providing this parameter.

Designers are accepting of two solutions, up to devs what is easiest/best

• No validation beyond alphabetical characters, then graceful fallback if something isn’t available for that sim.
• Validate based on the main superset of all supported region and cultures, and provide the invalid dialog if there is a value that isn’t in that main list, and still be graceful if that sim doesn’t have that region and culture as a supported value.

@jonathanolson @brent-phet and I are going to pursue the "no validation" strategy for this.

open questions:

• Since there won't be a QSM warning dialog in phet brand, are we going to list supported regions in teacher tips or somewhere else?

[zepumph]

Noting that @samreid may be working on a couple more Region and culture sims in the coming months.

[zepumph]

Here is a first pass on this feature.

Subject: [PATCH] allow grace to regionAndCulture, https://github.com/phetsims/joist/issues/974
---
Index: joist/js/i18n/regionAndCultureProperty.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/i18n/regionAndCultureProperty.ts b/joist/js/i18n/regionAndCultureProperty.ts
--- a/joist/js/i18n/regionAndCultureProperty.ts	(revision 81ec3838cb514bc423fbcd740c99d160ae429168)
+++ b/joist/js/i18n/regionAndCultureProperty.ts	(date 1738105445512)
@@ -62,43 +62,48 @@
 export const supportedRegionAndCultureValues: RegionAndCulture[] = _.uniq( [
   DEFAULT_REGION_AND_CULTURE, // Always supported, since it is our fallback.
   ...( packageJSON?.phet?.simFeatures?.supportedRegionsAndCultures || [] )
-].filter( regionAndCulture => RegionAndCultureValues.includes( regionAndCulture ) ) );
+] );
+
+// TODO: do this?
+assert && assert( _.every( supportedRegionAndCultureValues, x => RegionAndCultureValues.includes( x ) ),
+  'packageJSON provided unsupported region and culture' );
 
 // Is the specified regionAndCulture supported at runtime?
 const isSupportedRegionAndCulture = ( regionAndCulture?: RegionAndCulture ): boolean => {
   return !!( regionAndCulture && supportedRegionAndCultureValues.includes( regionAndCulture ) );
 };
 
-const initialRegionAndCulture: RegionAndCulture = window.phet.chipper.queryParameters.regionAndCulture;
-assert && assert( isSupportedRegionAndCulture( initialRegionAndCulture ),
-  `Unsupported value for query parameter ?regionAndCulture: ${initialRegionAndCulture}` );
+const initialRegionAndCulture: RegionAndCulture = isSupportedRegionAndCulture( window.phet.chipper.queryParameters.regionAndCulture ) ?
+                                                  window.phet.chipper.queryParameters.regionAndCulture : DEFAULT_REGION_AND_CULTURE;
 
 // Globally available, similar to phet.chipper.locale, for things that might read this (e.g. from puppeteer in the future).
+// TODO: should we delete this? It is unused, and we could use phet.joist.regionAndCultureProperty, https://github.com/phetsims/joist/issues/974
 phet.chipper.regionAndCulture = initialRegionAndCulture;
 
 class RegionAndCultureProperty extends Property<RegionAndCulture> {
   protected override unguardedSet( value: RegionAndCulture ): void {
-    if ( supportedRegionAndCultureValues.includes( value ) ) {
+    if ( isSupportedRegionAndCulture( value ) ) {
       super.unguardedSet( value );
     }
     else {
-      assert && assert( false, 'Unsupported region-and-culture: ' + value );
-
-      // Do not try to set if the value was invalid
+      // TODO: Any console logging as "unsupported"? https://github.com/phetsims/joist/issues/974
+      super.unguardedSet( DEFAULT_REGION_AND_CULTURE ); // Be graceful to unsupported values
     }
   }
 }
 
 const isInstrumented = supportedRegionAndCultureValues.length > 1;
 
+const phetioDocumentation = `Describes how a region and culture will be portrayed in the sim. Setting to an unsupported\
+ value will gracefully set back to '${DEFAULT_REGION_AND_CULTURE}'. This simulation supports the following values:\
+ ${supportedRegionAndCultureValues.join( ', ' )}.`;
+
 const regionAndCultureProperty = new RegionAndCultureProperty( initialRegionAndCulture, {
 
-  // Sorted so that changing the order of "supportedRegionsAndCultures" in package.json does not change the PhET-iO API.
-  validValues: supportedRegionAndCultureValues.sort(),
   tandem: isInstrumented ? Tandem.GENERAL_MODEL.createTandem( 'regionAndCultureProperty' ) : Tandem.OPT_OUT,
   phetioFeatured: isInstrumented,
   phetioValueType: StringIO,
-  phetioDocumentation: 'Describes how a region and culture will be portrayed in the sim.'
+  phetioDocumentation: phetioDocumentation
 } );
 
 joist.register( 'regionAndCultureProperty', regionAndCultureProperty );
Index: chipper/js/browser/initialize-globals.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/browser/initialize-globals.js b/chipper/js/browser/initialize-globals.js
--- a/chipper/js/browser/initialize-globals.js	(revision c82a254429cc2cb4b1c5871d05ccbded6addcc44)
+++ b/chipper/js/browser/initialize-globals.js	(date 1738105139014)
@@ -469,8 +469,7 @@
     regionAndCulture: {
       public: true,
       type: 'string',
-      defaultValue: packagePhet?.simFeatures?.defaultRegionAndCulture ?? 'usa',
-      validValues: packagePhet?.simFeatures?.supportedRegionsAndCultures ?? [ 'usa' ] // default value must be in validValues
+      defaultValue: packagePhet?.simFeatures?.defaultRegionAndCulture ?? 'usa'
     },
 
     /**

[zepumph]

I'd next like to discuss this with @marlitas since she was one of the original authors. I don't think it will be too challenging to get to a commit point here. I doubt we will want to maintenance release this, but we may need it for partners.

[zepumph]

I discussed this with @marlitas today.

We believe we want to MR this grace.

Here is @marlitas's list of sims published with regionAndCulture, but she mentioned that another good way to check support is with the simFeatures package flag called supportedRegionsAndCultures.

Number Line: Operations
Number Line: Integers
Number Line: Distance
Energy Skate Park
Area Model Algebra
Area Model Multiplication
Arithmetic
Graphing Lines
Graphing Slope-Intercept
Balancing Act
Forces and Motion: Basics
Color Vision
Center and Variability (doesn't even support ?regionAndCulture, so it doesn't apply to this MR)
Mean Share and Balance

[zepumph]

@marlitas and I went through the changes, and decided that adding graceful handling for the Property values itself is not desirable. We still want validValues for regionAndCultureProperty, and to have an assertion if trying to set the value to an unsupported regionAndCulture.

This logic differentiation is acceptable because phet-io clients have the ability to see what the valid entries are for a given sim through its state:

const state = getThePropertyState();
if( state.validValues.includes( 'asia')){	
  phetioClient.invoke( 'regionAndCultureProperty', 'setValue', ['asia'] )
}

Whereas, the query parameter is more teacher facing, and so needs to be much more user friendly with a fallback to the default instead of a query parameter dialog error.

Side quests (not needed for the maintenance release):

• Delete phet.chipper.regionAndCulture = initialRegionAndCulture;, it is unused and it also is just the initial, and never changes from the Property changing. Bye!
• We want to convert the filter to an assertion when creating the var supportedRegionAndCultureValues. Packagejson values should always be correct and supported.
• Use isSupportedRegionAndCulture( value ) in unguarded set instead of duplicated includes().

Here is the likely final MR changes.

Subject: [PATCH] allow grace to regionAndCulture, https://github.com/phetsims/joist/issues/974
---
Index: joist/js/i18n/regionAndCultureProperty.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/i18n/regionAndCultureProperty.ts b/joist/js/i18n/regionAndCultureProperty.ts
--- a/joist/js/i18n/regionAndCultureProperty.ts	(revision dc5839729a99e2dd02cd26267155b1ac0c5f593e)
+++ b/joist/js/i18n/regionAndCultureProperty.ts	(date 1738171924503)
@@ -69,9 +69,8 @@
   return !!( regionAndCulture && supportedRegionAndCultureValues.includes( regionAndCulture ) );
 };
 
-const initialRegionAndCulture: RegionAndCulture = window.phet.chipper.queryParameters.regionAndCulture;
-assert && assert( isSupportedRegionAndCulture( initialRegionAndCulture ),
-  `Unsupported value for query parameter ?regionAndCulture: ${initialRegionAndCulture}` );
+const initialRegionAndCulture: RegionAndCulture = isSupportedRegionAndCulture( window.phet.chipper.queryParameters.regionAndCulture ) ?
+                                                  window.phet.chipper.queryParameters.regionAndCulture : DEFAULT_REGION_AND_CULTURE;
 
 // Globally available, similar to phet.chipper.locale, for things that might read this (e.g. from puppeteer in the future).
 phet.chipper.regionAndCulture = initialRegionAndCulture;
Index: chipper/js/browser/initialize-globals.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/browser/initialize-globals.js b/chipper/js/browser/initialize-globals.js
--- a/chipper/js/browser/initialize-globals.js	(revision 7fe27b2f766c62c06e7b620e17551aa8c5ad65aa)
+++ b/chipper/js/browser/initialize-globals.js	(date 1738170813670)
@@ -469,8 +469,7 @@
     regionAndCulture: {
       public: true,
       type: 'string',
-      defaultValue: packagePhet?.simFeatures?.defaultRegionAndCulture ?? 'usa',
-      validValues: packagePhet?.simFeatures?.supportedRegionsAndCultures ?? [ 'usa' ] // default value must be in validValues
+      defaultValue: packagePhet?.simFeatures?.defaultRegionAndCulture ?? 'usa'
     },
 
     /**

[zepumph]

allow grace to regionAndCulture QP are the two that need cherry picking.

[zepumph]

I wrote a script to determine what published sim versions had supportedRegionsAndCultures in their package, and came up with this list:

area-model-algebra 1.3
area-model-multiplication 1.3
arithmetic 1.2
balancing-act 1.3
energy-skate-park 1.3
forces-and-motion-basics 2.4
graphing-lines 1.4
graphing-slope-intercept 1.2
mean-share-and-balance 1.1
number-line-distance 1.1
number-line-integers 1.2

The differences between this list and @marlitas' above are that this list doesn't have CAV (expected), and also Number Line Operations). I didn't see NL:Operations published with region and culture on the website, or locally, so I'm going to proceed leaving it out of the list.

Although, I do see an error when using the parameter:

• I should still handle NL:Operations gracefully probably. SEE Region and Culture query parameter should be graceful and future proof #974 (comment)

[zepumph]

For QA to test:

For all sims:

• Query parameter warning dialog should never show for the regionAndCulture parameter.
• all HTML: ?regionAndCulture=fdjksalf does not crash the sim
• all HTML: No query parameter doesn't crash the sim
• debug HTML: ?regionAndCulture=fdjksalf does not crash the sim or throw any assertion

For any sim in this list: #974 (comment)

• Make sure ?regionAndCulture parameter still works, try with asia or africa depending on the sim and what is supported.
• Use of the query parameter sets the right value in the preferences dialog combo box.