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

Prefer stateSetEmitter when doing logic after PhET-iO state set #20

Open
zepumph opened this issue Sep 3, 2024 · 1 comment
Open

Prefer stateSetEmitter when doing logic after PhET-iO state set #20

zepumph opened this issue Sep 3, 2024 · 1 comment

Comments

@zepumph
Copy link
Member

zepumph commented Sep 3, 2024

Adding a TODO to a usage of isSettingPhetioStateProperty, from working on phetsims/tandem#314.

It is possible that you could have awkward order dependencies by listening to the Property, since that is the event for "we are done setting state". For example the soundManager uses this to re-enabled audio. Adding a TODO to listen to the stateSetEmitter instead makes sense here, but I don't know the logic well enough to be able to test things. This isn't high priority or anything, just a minor code smell.

https://github.com/phetsims/center-and-variability/blob/8dc7c64447c6b0a67999ca9fb120728f85f028d3/js/common/model/CAVSoccerSceneModel.ts#L60-L64

isSettingPhetioStateProperty.link( isSettingPhetioState => {
if ( !isSettingPhetioState ) {
this.objectChangedEmitter.emit();
}
} );

zepumph added a commit that referenced this issue Sep 3, 2024
Signed-off-by: Michael Kauzmann <michael.kauzmann@colorado.edu>
zepumph added a commit to phetsims/center-and-variability that referenced this issue Sep 3, 2024
Signed-off-by: Michael Kauzmann <michael.kauzmann@colorado.edu>
@marlitas
Copy link
Contributor

marlitas commented Sep 4, 2024

This repo is not a priority right now. Un-assigning until that changes.

@marlitas marlitas removed their assignment Sep 4, 2024
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