Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should projector mode be stateful? #458

Closed
Tracked by #841
Nancy-Salpepi opened this issue Sep 30, 2022 · 2 comments
Closed
Tracked by #841

should projector mode be stateful? #458

Nancy-Salpepi opened this issue Sep 30, 2022 · 2 comments
Assignees

Comments

@Nancy-Salpepi
Copy link

For phetsims/qa#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
Copy link
Member

samreid commented Oct 4, 2022

From slack:

image

image

Nancy Salpepi
Sep 30th at 6:01 AM
Should projector mode be stateful?
18 replies

Sam Reid
4 days ago
If I recall correctly, that is the long-term plan. But I don’t recall what we decided for now for this release.
@arouinfar
may recall better. And I noted you opened #458, thanks!

Chris Malley
4 days ago
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
It is stateful right now
@pixelzoom
. I was told that most preferences shouldn’t be so I was just making sure that was correct.
👍:skin-tone-2:
1

Sam Reid
4 days ago
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
So being stateful sounds intended for that (edited)

Nancy Salpepi
4 days ago
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
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
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
I think it implies that ALL things in the Preferences dialog should be stateful.

Chris Malley
4 days ago
Sorry, that’s wrong…. All things that are sim specific in the Preferences dialog should be stateful.
👍
1

Sam Reid
4 days ago
That last line is referring to sim-specific things.

Nancy Salpepi
4 days ago
Got it. Thank you!

Chris Malley
4 days ago
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
preferencesKey.includes( '.simulationModel.' ) ) in the code snippet that
@samreid
posted above refers to the part of the Preferences model that appears in the Simulation tab. (edited)

Nancy Salpepi
4 days ago
Perfect. Thanks.

Amy Rouinfar
4 days ago
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
as far as the preferences menu goes, the audioEnabledProperty refers to the audio features toggle?

Amy Rouinfar
4 days ago
Yes, it should be hooked up to both the Audio Features toggle in the Preferences dialog and the sound button in the navbar.
👍:skin-tone-2:
1

@samreid
Copy link
Member

samreid commented Oct 4, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants