Fold Options into Preferences.

[pixelzoom]

Related to #835, and summary of Zoom discussion with @jessegreenberg ...

• Any sim that has an Options dialog (uses SimOptions createOptionsDialogContent) will need to be folded into Preferences.
• This will probably involve using SimOptions preferencesConfiguration instead of createOptionsDialogContent.
• If the Options dialog uses ProjectorModeCheckbox, it should be removed, and instead added to the Preferences "Visual" tab.

These sims use createOptionsDialogContent but only have a ProjectorModeCheckbox:

• [x] atomic-interactions
• [x] blackbody-spectrum
• [x] charges-and-fields
• [x] coulombs-law
• [x] diffusion
• [x] models-of-the-hydrogen-atom
• [x] rutherford-scattering
• [x] states-of-matter
• [x] states-of-matter-basics

These sims use createOptionsDialogContent and have controls that should appear in Preferences "Visual" tab:

• [x] build-an-atom
• [x] center-and-variability
• [x] circuit-construction-kit-common
• [x] gas-properties
• [x] gases-intro
• [x] geometric-optics
• [x] molecule-polarity
• [x] molecule-shapes
• [x] molecule-shapes-basics
• [x] tambo

[zepumph]

These sims use createOptionsDialogContent and have controls that should appear in Preferences "Visual" tab:

In that case, it will probably want to hold off until #835 has been worked out. If options are totally sim-specific, then we can proceed using PreferencesConfiguration.createSimControls

[zepumph]

Definitely on hold until we finish https://github.com/phetsims/joist/issues/835, there is a new API for creating custom preferences, even in the general tab.

[zepumph]

Now unblocked! Yay

[zepumph]

I converted the ProjectorMode-checkbox-only sims over to preferences above. It lost PhET-iO instrumentation on some, but consistency is good. If we want the checkbox instrumented in the future, we will add it then.

[zepumph]

The second half of the list has been done above. A couple more things to do:

• [x] new ProjectorModeCheckbox should not be used, we should be moving towards the preferencesModel setting.
• [x] OptionsDialog should be deleted. Right now the main challenge is that OptionsDialog has constants used elsewhere.
• [x] GreenhouseEffectQueryParameters.optionsDialog should probably be renamed
• [x] OptionsDialogContent should probably be renamed to PreferencesContent

That's all I have time for tonight!

JG Edits:

• [x] Custom content Nodes should use PreferencesDialog fonts so they match other dialog contents.
• [x] Should "High Contrast Particls" be moved to "Visual" tab in build-an-atom?

[jessegreenberg]

Excellent! I am seeing a couple of phet-io problems on CT in diffucion/gas-properties/geometric-optics that I don't know how to fix

Assertion failed: phetioID already registered: diffusion.general.view.navigationBar.preferencesButton.preferencesDialogCapsule.preferencesDialog.preferencesPanels.generalPreferencesPane

[pixelzoom]

A couple of other things that need to be done:

• [x] Rename "Options" classes/variables/methods/etc. like GOOptionsNode.js. Should we use "Preferences", e.g. GOPreferencesNode?

• [x] Update documentation like this that appears in GOOptionsNode.ts: // GOOptionsNode is the user interface for global options, accessed via PhET > Options.

[zepumph]

Excellent! I am seeing a couple of phet-io problems on CT in diffucion/gas-properties/geometric-optics that I don't know how to fix

Fixed above. We changed the tandem API so now you are given a parent tandem, instead of the tandem you should use.

[pixelzoom]

• [x] GasPropertiesGlobalOptionsNode is still creating a ProjectorModeCheckbox.

[zepumph]

See above:

new ProjectorModeCheckbox should not be used, we should be moving towards the preferencesModel setting

[pixelzoom]

I see, I misunderstood. I thought that the point of addressing the first checklist in https://github.com/phetsims/joist/issues/837#issue-1346723654 was to delete ProjectorModeCheckBox and use supportsProjectorMode: true. And since you checked all of those boxes...

[zepumph]

Righto!

I deleted ProjectorModeCheckbox above. The only weird thing is that diffusion still creates a GasProperties preferences Node even though it has no content for it. Since it is in a suite. I thought that was better, in case a new option is added to that sime that should apply to all three sims (gas-properties, diffusion, and gases-intro).

Also, new tandem names across the board will fix CT. I went with simPreferencesContent generally.

[zepumph]

@jessegreenberg, over to you for next steps!

[pixelzoom]

I deleted ProjectorModeCheckbox above. The only weird thing is that diffusion still creates a GasProperties preferences Node even though it has no content for it. Since it is in a suite. I thought that was better, in case a new option is added to that sime that should apply to all three sims (gas-properties, diffusion, and gases-intro).

Not good. To be addressed in https://github.com/phetsims/diffusion/issues/8.

[pixelzoom]

Also, new tandem names across the board will fix CT. I went with simPreferencesContent generally.

There's nothing in that name that indicates that it's content for the General tab. What if the sim is also adding sim-specific options to other tabs? (That's certainly supported by PreferencesModel.). How about {{SIM}}{{TAB}}PreferencesContent as a convention. For example, geometricOpticsGeneralPreferencesContent for General tab content added by Geometric Options.

[zepumph]

I feel like this will continue to be evolving, but for now I like it the way it is. These Node's were designed as monolith options for the whole sim. PreferencesModel supports multiple new ways of doing preferences that could be used in the future, and would result in different tandem names. We can have multiple custom preferences for each tab, and we can also have custom preferences on each tab. In those cases, I think the content should be named according to the content that it is adding. In the future we may choose to add a new customPreferences closure to the general tab for each control, in that case the tandem names would want to be named for the control, and not the tab.

If a sim adds a visual customPreference and a generalCustomPreference, shouldn't it be named something better than simVisualPreference and simGeneralPreference? Why not name to what the content actually has?

[pixelzoom]

If a sim adds a visual customPreference and a generalCustomPreference, shouldn't it be named something better than simVisualPreference and simGeneralPreference? Why not name to what the content actually has?

Sure, that would be fine. But that's not what you proposed above. You proposed convention simPreferencesContent. And I'm pointing out a shortcoming of that convention - it assumes that you're only adding custom content to the General, e.g.:

preferencesModel: new PreferencesModel( {
  generalOptions: {
    customPreferences: [ {
      createContent: ...  // tandem name = 'simPreferencesContent'
    } ]
  }
} )

The Preferences API supports adding custom content to any tab. So if we go with your simPreferencesContent convention for the General tab... What do you propose to use for the tandem names for the custom content in the Visual and Audio tabs in this example?

preferencesModel: new PreferencesModel( {
  generalOptions: {
    customPreferences: [ {
      createContent: ...  // tandem name = 'simPreferencesContent'
    } ]
  },
  visualOptions: {
    customPreferences: [ {
      createContent: ...  // tandem name = ???
    } ]
  },
  audioOptions: {
    customPreferences: [ {
      createContent: ...  // tandem name = ???
    } ]
  }
} )

[pixelzoom]

If a sim adds a visual customPreference and a generalCustomPreference, shouldn't it be named something better than simVisualPreference and simGeneralPreference? Why not name to what the content actually has?

A reason not to "name to what the content actually has" is PhET-iO standardization. I can imagine that PhET-iO designers will want to have consistent tandem names for "custom content added to the General tab", "custom content added to the Audio tab", etc.

[pixelzoom]

I discussed the tandem name issue with @zepumph. We agreed that simPreferences is a better general name, for use in all tabs. The phetioID (tandem hierarchy) will tell you which tab you're in.

For example, instead of this, where "geometricOpticsPreferencesContent" is a bt redundant:

geometricOptics.general.view.navigationBar.preferencesButton.preferencesDialogCapsule.archetype.preferencesPanels.generalPreferencesPanel.geometricOpticsPreferencesContent

... we'll have this:

geometricOptics.general.view.navigationBar.preferencesButton.preferencesDialogCapsule.archetype.preferencesPanels.generalPreferencesPanel.simPreferences

[pixelzoom]

• [x] GOOptions.ts and geometricOptics.global.view.options need to be renamed. Should they be renamed to GOPreferences.ts and geometricOptics.global.view.preferences? Note that geometricOptics.global.view.options.cueingArrowsEnabledProperty will NOT appear in the Preferences dialog, because (by design) it has no presence in the UI and is a PhET-iO-only Property.

[pixelzoom]

Things that need to be renamed in my sims:

• [x] GOOptionsNode, GOOptions, associated tandems
• [x] GasPropertiesGlobalOptionsNode, GasPropertiesGlobalOptions, associated tandems
• [x] MPOptionsNode, MPGlobalOptions, associated tandems

It looks like other sims have similar renaming issues (e.g. build-an-atom GlobalOptionsNode) but I'm not going to attempt to enumerate them.

[jessegreenberg]

I think we are done here except for the items listed in https://github.com/phetsims/joist/issues/837#issuecomment-1224750873 and https://github.com/phetsims/joist/issues/837#issuecomment-1224787364. @pixelzoom can you please assign this back to me when those are done? I reviewed the lists in the original issue and sims are looking OK. I looked for remaining references to 'OptionsDialog' or GlobalOptions or 'options content` and other combinations in class names and documentation and updated them. I noticed in https://github.com/phetsims/molecule-shapes-basics/commit/39077fd3b03aa6e9e797fe9fd78031a37f51e892 we lost projector mode in molecule-shapes-basics. Id like to see if QA would be able to review and look for layout problems, missing or broken preferences, or anything else before closing.

[pixelzoom]

I complete geometric-optics, molecule-polarity, and gas-properties in their respective issues (linked above).

General conventions I used:

• Preferences Properties are under {sim}.global.model.preferences in PhET-iO

• tandem name for the Node returned by SimOptions.preferencesModel.generalOptions.customPreferences.createContent is 'simPreferences', as decided in https://github.com/phetsims/joist/issues/837#issuecomment-1224743329

[pixelzoom]

Looks like there's still work to be done here. We decided to use 'simPreferences' as the standard tandem name. That's being violated in every sim that I've looked at. Examples...

build-an-atom-main.js:

createContent: tandem => new VisualPreferencesNode( BAAGlobalPreferences.highContrastParticlesProperty,
  tandem.createTandem( 'buildAnAtomPreferencesContent' ) )

cck-ac-virtual-lab-main.js

createContent: tandem => new CCKCGeneralPreferencesContentNode( tandem.createTandem( 'cckPreferencesContent' ) )

cck-virtual-lab-main.js:

createContent: tandem => new CCKCGeneralPreferencesContentNode( tandem.createTandem( 'cckPreferencesContent' ) )

cck-dc-main.js:

createContent: tandem => new CCKCGeneralPreferencesContentNode( tandem.createTandem( 'cckPreferencesContent' ) )

energy-skate-park-main.js:

createContent: tandem => new EnergySkateParkPreferencesNode( energySkateParkPreferencesModel, tandem.createTandem( 'energySkateParkPreferencesContent' ) )

Back to @jessegreenberg.

[pixelzoom]

There's also no standard for preferences Properties. In all of my sims, they are under {sim}.global.model.preferences.

I see energySkatePark.global.model.energySkateParkPreferencesModel, which is redunant in 2 ways -- "energySkatePark" and "model", both of which are already in the phetioID. I haven't looked at other sims.

Recommended to choose a convention here, and use it consistently. I'm partial to {sim}.global.model.preferences.

[pixelzoom]

On Slack, we discussed the {sim}.global.model.preferences convention mentioned above.

Instead of defining this everywhere, like I've done in my sims:

const preferencesTandem = Tandem.GLOBAL_MODEL.createTandem( 'preferences' );

In the above commits, I've added this to Tandem.ts:

/**
 * Use this as the parent tandem for Properties that are related to sim-specific preferences.
 */
Tandem.PREFERENCES = Tandem.GLOBAL_MODEL.createTandem( 'preferences' );

So we should use Tandem.PREFERENCES for preferences Properties. And if the designers decide they want preferences Properties somewhere else in the Studio tree, we can just change Tandem.PREFERENCES.

[jessegreenberg]

OK, the PhET-iO changes described in the previous three comments are done. I think everything discussed in this issue is now complete. @zepumph can you think of anything else to review or do here?

[zepumph]

All looks good here. Thanks @pixelzoom and @jessegreenberg!!! Ready to close?

[zepumph]

Please note that we will be adding a PhET-iO migration rule because we removed the OptionsDialog menu item (and whole dialog). See https://github.com/phetsims/gravity-and-orbits/issues/434

[jessegreenberg]

Commits above look good. I believe so, closing.