search

phetsims / gravity-and-orbits

"Gravity And Orbits" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
12 stars 6 forks source link

should projector mode be stateful? #458

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

For https://github.com/phetsims/qa/issues/838, if I turn on projector mode in studio it will be on in the standard wrapper. I'm still not sure what in the preferences menu is stateful.

samreid commented 1 year ago

From slack:

image image
Nancy Salpepi [Sep 30th at 6:01 AM](https://phetsims.slack.com/archives/C6HPE0J91/p1664539289425449) Should projector mode be stateful? 18 replies Sam Reid [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664554982221709?thread_ts=1664539289.425449&cid=C6HPE0J91) If I recall correctly, that is the long-term plan. But I don’t recall what we decided for now for this release. [@arouinfar](https://phetsims.slack.com/team/U5PTARWV7) may recall better. And I noted you opened https://github.com/phetsims/gravity-and-orbits/issues/458, thanks! Chris Malley [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556267528839?thread_ts=1664539289.425449&cid=C6HPE0J91) Projector mode was most definitely stateful in the sims that I worked on. It was formerly up to the sim to create (and PhET-iO instrument) the associated Property, and add a checkbox to the Options dialog. When Options and projector mode were absorbed into the Preferences dialog, that statefulness seems to have been lost. So I would consider that not a “long-term plan” but a regression, worthy of a general issue in joist. (edited) Nancy Salpepi [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556407917409?thread_ts=1664539289.425449&cid=C6HPE0J91) It is stateful right now [@pixelzoom](https://phetsims.slack.com/team/U6EMFARV2) . I was told that most preferences shouldn’t be so I was just making sure that was correct. :+1::skin-tone-2: 1 Sam Reid [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556411182549?thread_ts=1664539289.425449&cid=C6HPE0J91) I’m seeing a note that says: // Audio manager, color profile property and localeProperty are supposed to be stateful. All other preferences // should be phetioState: false so they are not captured in the state assert && assert( phetioObject.phetioState === ( phetioObject.phetioID.endsWith( '.colorProfileProperty' ) || phetioObject.phetioID.endsWith( '.audioEnabledProperty' ) || phetioObject.phetioID.endsWith( '.localeProperty' ) || Sam Reid [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556425734409?thread_ts=1664539289.425449&cid=C6HPE0J91) So being stateful sounds intended for that (edited) Nancy Salpepi [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556653530269?thread_ts=1664539289.425449&cid=C6HPE0J91) Thank you! I will make a note in the QA book that it is only those 3 things in the preferences menu that are stateful. Sam Reid [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556689207999?thread_ts=1664539289.425449&cid=C6HPE0J91) Thanks, it goes on to say: // Sim preferences should also be stateful preferencesKey.includes( '.simulationModel.' ) ), So maybe that should also be mentioned? Nancy Salpepi [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556779055209?thread_ts=1664539289.425449&cid=C6HPE0J91) I don’t know what that means. haha. Can you put that in everyday language for me or give me an example? Chris Malley [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556816941749?thread_ts=1664539289.425449&cid=C6HPE0J91) I think it implies that ALL things in the Preferences dialog should be stateful. Chris Malley [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556852100159?thread_ts=1664539289.425449&cid=C6HPE0J91) Sorry, that’s wrong…. All things that are sim specific in the Preferences dialog should be stateful. :+1: 1 Sam Reid [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556860338859?thread_ts=1664539289.425449&cid=C6HPE0J91) That last line is referring to sim-specific things. Nancy Salpepi [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556908546229?thread_ts=1664539289.425449&cid=C6HPE0J91) Got it. Thank you! Chris Malley [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664556951879699?thread_ts=1664539289.425449&cid=C6HPE0J91) To clarify further… settings under the Simulation tab in the Preferences dialog should be stateful. For example, in Geometric Optics: screenshot_1895.png screenshot_1895.png Chris Malley [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664557002332099?thread_ts=1664539289.425449&cid=C6HPE0J91) preferencesKey.includes( '.simulationModel.' ) ) in the code snippet that [@samreid](https://phetsims.slack.com/team/U6DKKJ2FN) posted above refers to the part of the Preferences model that appears in the Simulation tab. (edited) Nancy Salpepi [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664557031676129?thread_ts=1664539289.425449&cid=C6HPE0J91) Perfect. Thanks. Amy Rouinfar [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664557160631509?thread_ts=1664539289.425449&cid=C6HPE0J91) The subset of the Preferences that should be stateful are: anything in the Simulation tab, projector mode, and audioEnabledProperty (the property hooked up to the mute button in the navbar) Nancy Salpepi [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664558268111989?thread_ts=1664539289.425449&cid=C6HPE0J91) as far as the preferences menu goes, the audioEnabledProperty refers to the audio features toggle? Amy Rouinfar [4 days ago](https://phetsims.slack.com/archives/C6HPE0J91/p1664558471845079?thread_ts=1664539289.425449&cid=C6HPE0J91) Yes, it should be hooked up to both the Audio Features toggle in the Preferences dialog and the sound button in the navbar. :+1::skin-tone-2: 1
samreid commented 1 year ago

Based on the conversation above, it seems like projector mode is supposed to be stateful, and it is stateful in the RC. Therefore I think this issue can be closed.