Closed zepumph closed 5 months ago
I recommend this patch to proceed. It takes PhET-iO instrumentation out of the hands of the instances entirely, moves the regionAndCulturePortrayals tandem to a constant in Tandem, and ensures that the query parameter value matches the tandem name across sims.
@marlitas. How would you like to proceed? I don't think I should commit this without talking it through with you first.
@marlitas and I went through the above patch and we would like to proceed with this.
We also saw that perhaps an assertion here would make sense.
We also saw that perhaps an assertion here would make sense.
The above is basically exactly the same as the patch and what @marlitas and I went over (except a lint error and CAV migration support). So I feel good about closing this. Thanks @marlitas.
From https://github.com/phetsims/phet-io-wrappers/issues/612, I saw that it isn't going to be plug and play to support standard wrapper integration because there is no consistency about what the values of the regionAndCultureProperty could be. In center and variability right now, it can be these three options:
centerAndVariability.global.model.preferences.regionAndCulturePortrayals.kickerPortrayalUSA
centerAndVariability.global.model.preferences.regionAndCulturePortrayals.kickerPortrayalAfrica
centerAndVariability.global.model.preferences.regionAndCulturePortrayals.kickerPortrayalAfricaModest
I do not agree with this. I think the tandem names should match the allowed values of the query parameters. I don't believe any other portrayals have been created in phet-io sims, but marking high priority until I confirm there aren't other publications that are about to go out with this discrepancy before we talk about this more.