solutes.water.nameProperty does not migrate

[zepumph]

@Nancy-Salpepi originally reported this in https://github.com/phetsims/phet-io-sim-specific/issues/17#issuecomment-1382824832:

This still seems to not work for water in phetsims/qa#872

1. In Studio version 1.5, change phScale.global.model.solutes.water.nameProperty to "H2O"
2. Save file
3. Load into Studio version 1.6 --it says "Water"

Also in studio version 1.5, if you make this additional change:

1. Change phScale.macroScreen.view.waterFaucetNode.waterLabelNode.textProperty to "spit"
2. Save file
3. Load into Studio version 1.6--it says "Water"

[zepumph]

Ok, it isn't for all solutes, because I got blood and all others but water working, just for water it seems.

[Nancy-Salpepi]

Right. I think it is because water appears in two places--on the faucet and in the combobox-- and there used to be 2 different strings but now there is only one.

[pixelzoom]

This blocks publication of the next RC.

It requires revision to both ph-scale-migration-rules.ts and ph-scale-basics-migration-rules.ts.

[pixelzoom]

Since this problem is specific to solutes.water.nameProperty, I changed the issue title.

[pixelzoom]

@Nancy-Salpepi said:

Right. I think it is because water appears in two places--on the faucet and in the combobox-- and there used to be 2 different strings but now there is only one.

While "Water" does indeed appear in 2 places in the UI, there is (and always has been) only 1 entry in the strings.json file:

  "choice.water": {
    "value": "Water"
  },

The difference is that phScale.global.model.solutes.water.nameProperty was instrumented in 1.5, while the text for phScale.microScreen.view.waterFaucetNode.waterLabelNode was not.

In 1.6, phScale.global.model.solutes.water.nameProperty and phScale.macroScreen.view.waterFaucetNode.waterText.stringProperty both link to phScale.general.model.strings.phScale.choice.waterStringProperty.

[pixelzoom]

These rules also look suspect:

    // These have been replaced with PHScaleStrings.waterStringProperty.
    // NOTE! These must be done before rename of waterLabelNode below.
    new TandemFragmentRenamed( 'phScale.macroScreen.view.waterFaucetNode.waterLabelNode.stringProperty',
      'phScale.general.model.strings.phScale.choice.waterStringProperty' ),
    new TandemFragmentRenamed( 'phScale.microScreen.view.waterFaucetNode.waterLabelNode.stringProperty',
      'phScale.general.model.strings.phScale.choice.waterStringProperty' ),

waterLabelNode is an instance of scenery.Text and was renamed to waterText. And I don't believe that Text had stringProperty in ph-scale 1.5, that was a relatively-recent rename. Wasn't it something like textProperty?

These TandemFragmentRenamed rules are also likely responsible for overwriting phScale.global.model.solutes.water.nameProperty with "Water". Perhaps they shoud be changed to TandemFragmentDelete.

[zepumph]

Because of a logic bug fix in https://github.com/phetsims/phet-io-wrappers/issues/480, and the reordering in https://github.com/phetsims/phet-io-sim-specific/commit/99215da33b4f4ca571d90c74d1d665a5b2572780, the value for waterStringProperty will now be decided first and foremost by the name from the solutes. Please note that https://github.com/phetsims/phet-io/issues/1903#issuecomment-1386300547 will prohibit testing this on master for a time, so I'll mark this on hold, assigned to myself until we can fix that.

[pixelzoom]

• In https://github.com/phetsims/phet-io-sim-specific/commit/99215da33b4f4ca571d90c74d1d665a5b2572780 by @zepumph, I have no idea what the added comment means. Please clarify.

    // Solutions - prioritize version of water over the faucet node text
    new ConvertToStringsStateForLocale( /phScale\.global\.model\.solutes\.(.*)\.nameProperty/, 'phScale.general.model.strings.phScale.choice.$1StringProperty', stringsStatePhetioID ),

[samreid]

We also need commits from:

• https://github.com/phetsims/phet-io-wrappers/issues/480

[zepumph]

Ready to cherry pick here, don't forget about https://github.com/phetsims/phet-io-wrappers/issues/480

[samreid]

@zepumph and I cherry picked and verified this. Would be good for QA to verify it in RC.2.

To verify:

1. follow the directions to set up the version migration wrapper in Improve the version-migration wrapper #275 (comment)
2. Customize these three phetioIDs in the oldVersion

phScale.microScreen.view.waterFaucetNode.waterLabelNode.textProperty
phScale.macroScreen.view.waterFaucetNode.waterLabelNode.textProperty
phScale.global.model.solutes.water.nameProperty

3. When pressing "Migrate", the phScale.global.model.solutes.water.nameProperty should always take precedence in the migrated sim.

[zepumph]

I confirmed in ph-scale and basics 1.6.0-rc.2 that the value of the water customization comes from the solutes. Ready to close after QA verification.

[Nancy-Salpepi]

This looks good in rc.3 for pH Scale and pH Scale basics using the steps in #267 (comment) and using the old method too (saving in old studio version and loading in new).
Closing.