Open kathy-phet opened 6 months ago
Please note that the tree referred to above is only in the strings, the full tandem is:
projectileDataLab.general.model.strings.joist.preferences.tabs.localization.regionAndCulture.africaModestStringProperty
There are also other irrelevant strings, such as voicing (the sim does not support voicing)
projectileDataLab.general.model.strings.joist.preferences.tabs.audio.voicing.descriptionStringProperty
This problem was previously identified and discussed in https://github.com/phetsims/phet-io/issues/1877.
I also confirmed that moving the string accessors from static to the constructor does not eliminate them from the API:
Also, if those strings appear in another simulation that actually does support R&C, then later undergoes change, whatever migration rules we introduce in that sim can be reused here. I'm not saying it would be 0 overhead, but it could be shared overhead.
why do we have "supportsRegionAndCulture" true in the json
I do not see the string supportsRegionAndCulture in the API json: https://phet-dev.colorado.edu/html/projectile-data-lab/1.0.0-rc.5/phet-io/projectile-data-lab-phet-io-api.json, were you referring to a different JSON? It is also not in the package.json: https://github.com/phetsims/projectile-data-lab/blob/cee42cccd523879c0413aad0649170cba2461b06/package.json#L25-L29
thus bringing in the ?regionAndCulture query parameter behavior?
Since November 2023 all new sims have this commit https://github.com/phetsims/chipper/commit/dd94297775905a8c251da2f5ef6fa68d17512224 which means all sims since then support the region and culture query parameter key. But each sim only support the query parameter values that it lists in the sim package.json, or defaults to 'usa'
if it doesn't mention any.
I hadn't realized RegionAndCulture would be active on non RegionAndCulture related sims until I ran into it with Gas Properties yesterday.
I'm interested to hear more about this, was it limited to strings or about more than strings?
A note for other team members as we look into this: "unbuilt" mode shows more irrelevant strings than "built" mode does, so please be aware of that.
I'm not worried about the unused Strings. I'm considering the case - similar to locale - where a client appends regionAndCulture=africa to query parameters because that is their preferred regionAndCulture for their product. This sim would pop up a dialog, even though it basically is unaffected by regionAndCulture at all and is never planned to be. Where as locale=es will just continue, even if es is not an option.
Is there such a thing as "supportsRegionsAndCultures": false that would be used for sims that don't have a RegionAndCulture need? if you added it to the json package.
Idea for discussion, populate the schema conditionally and dynamically (untested):
Related issue where we most recently discussed locale fallbacks and when error message popups should appear: https://github.com/phetsims/joist/issues/963#issuecomment-2073767742
@kathy-phet @samreid @matthew-blackman I don't think this is a blocking issue for PDL/PSD, as @samreid points out all sims published after November 2023 support the supportsRegionAndCulture
query parameter. I'll also note that I haven't added this query parameter to phet-io-guide.md yet (see https://github.com/phetsims/phet-io-sim-specific/issues/40), so clients are unlikely to know that it is a supported query parameter in PhET-iO.
Any changes that arise from this issue would also apply to a handful of recently published phet-brand and phet-io-brand sims.
Since November 2023 all new sims have this commit https://github.com/phetsims/chipper/commit/dd94297775905a8c251da2f5ef6fa68d17512224 which means all sims since then support the region and culture query parameter key. But each sim only support the query parameter values that it lists in the sim package.json, or defaults to 'usa' if it doesn't mention any.
Thanks @samreid, this was really helpful to know.
A note for other team members as we look into this: "unbuilt" mode shows more irrelevant strings than "built" mode does, so please be aware of that.
A good reminder @samreid! However, note that all Preferences strings will appear in a built PhET-iO sim, even for irrelevant Preferences. Other irrelevant common code strings (e.g. such as those under strings.sceneryPhet
) should be hidden in a built version, though.
Like @kathy-phet I am not concerned with regionAndCulture
strings (and Preferences strings, more generally) appearing in the PhET-iO API of all sims.
I'm considering the case - similar to locale - where a client appends regionAndCulture=africa to query parameters because that is their preferred regionAndCulture for their product. This sim would pop up a dialog, even though it basically is unaffected by regionAndCulture at all and is never planned to be. Where as locale=es will just continue, even if es is not an option.
@kathy-phet this sounds like a client training issue. All sims support locale
but only select sims support regionAndCuluture
. Clients should not be appending regionAndCulture
to all sims. Theoretically, we could treat regionAndCuluture
more like locale
so that regionAndCulture=africa
will fall back to usa
if africa
is not supported in the sim. I'm not sure that it's worth it, though. While locale
and regionAndCulture
are both means of localization, the latter isn't always applicable. All sims have strings to localize, but not all sims have depictions to localize. I think it's important for clients to realize this.
Is there such a thing as "supportsRegionsAndCultures": false that would be used for sims that don't have a RegionAndCulture need? if you added it to the json package.
We do this with lots of other features/Preferences, but you can always set them to true
with a query parameter. For example, running a sim with supportsSound=true
will turn on the default UI sounds and all the related infrastructure (mute button in navbar, Audio tab in Preferences). What happens if you run a sim with supportRegionsAndCultures=true
? Does it add the Region and Culture control to the Localization tab with just the usa
option?
I'm not worried about clients doing this, but I bring it up because I would expect it to behave like the other "supports" options in initialize-globals.js.
@samreid, @matthew-blackman, @brent-phet and I met to talk through some of these questions this morning. We looked through other query parameter examples and how they behave if the sim doesn't specifically support them. For example, we tried setting projectionColor=inverted
on PDL and it also popped up a warning dialog. Since the region and culture behavior is consistent with other query parameters I don't think it needs to change. And if we decide it does, it should be consistent across all query parameters that may or may not be supported by sims.
Either way I don't believe it's blocking as a change would require a maintenance release.
Noting that 2 PhET-iO sims currently have ?regionAndCulture query parameter behavior: My Solar System Kepler's Laws
I do think that we should revisit the use case for regionAndCulture, which is more parallel with locale than with other sim-specific features (even though it is sim specific).
I was mostly concerned with bringing this behavior into PhET-iO sims that don't need have a regionAndCulture aspect, but it's already in 2 PhET-iO sims. I'll put a discussion note on PhET-iO meetings.
And I'll mark this as deferred at the moment. Not blocking publication.
@matthew-blackman @samreid @arouinfar - Since this sim doesn't have any regionAndCulture features, why do we have "supportsRegionAndCulture" true in the json, and thus bringing in the ?regionAndCulture query parameter behavior? Shouldn't that just not be in the json, so we don't complicate this sim with those query parameters at this point?
Since RegionAndCulture is new behavior AND this is a PHET-iO sim for which we don't have RegionAndCulture, we haven't tested it fully and I don't know that this is the sim to adopt this into the API on? I hadn't realized RegionAndCulture would be active on non RegionAndCulture related sims until I ran into it with Gas Properties yesterday.
I want to make sure someone has decided that yes, we want to have this part of the tree in here?? And yes, we want the sim to say it an unsupported value if regionAndCulture=africa is added, because technically there is no special values.
Should
supportsRegionsAndCultures
just not be in the json for this sim?