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.

Sign up for GitHub

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

Jump to bottom

move isSettingPhetioStateProperty to this repo #294

Closed
zepumph opened this issue Apr 17, 2023 · 8 comments
Closed

move isSettingPhetioStateProperty to this repo #294

zepumph opened this issue Apr 17, 2023 · 8 comments
Assignees
@zepumph
Labels
dev:phet-io status:ready-for-review

Comments

@zepumph
Copy link
Member

zepumph commented Apr 17, 2023

From phetsims/chipper#1004, this would cleanup a lot of common code that needs to check for the existence of this Property before using it as a global. Thanks @samreid

Also tagging the backlog project board.
@zepumph zepumph self-assigned this Apr 17, 2023
@zepumph zepumph mentioned this issue Apr 17, 2023
Closed
@zepumph
Copy link
Member Author

zepumph commented Apr 17, 2023

There are currently 76 usages of phet.joist.sim.isSettingPhetioStateProperty, this seems like a really nice global to factor out!

@zepumph
Copy link
Member Author

zepumph commented May 31, 2023 •
edited
Loading

  • We probably would want the same pattern for isClearingPhetioDynamicElementsProperty.
  • Check in audioManager for sim.isSettingPhetioStateProperty
  • Check in number-compare/play-main for sim.isSettingPhetioStateProperty

zepumph added a commit to phetsims/capacitor-lab-basics that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
8577e79 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/charges-and-fields that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
66d2cc0 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/john-travoltage that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
7e4946c 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/center-and-variability that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
05e641b 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/calculus-grapher that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
5d81274 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/wave-on-a-string that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
fa79d0b 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/gravity-and-orbits that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
6883ec4 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, #294
1e3cc52 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/gas-properties that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
302c42d 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/graphing-quadratics that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
4bb04f5 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/energy-forms-and-changes that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
bf6df04 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/states-of-matter that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
83ed58b 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/fourier-making-waves that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
c0056df 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/number-compare that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
b2462f0 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/density-buoyancy-common that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
bc77500 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/circuit-construction-kit-common that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
11ef57a 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/energy-skate-park that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
55a1374 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit that referenced this issue May 31, 2023
@zepumph
move isClearingPhetioDynamicElementsProperty to tandem/, #294
c2ce38c 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/greenhouse-effect that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
e158a52 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/number-play that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
0ce8b8e 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/ratio-and-proportion that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
8b2e9fc 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/axon that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
dfa0e61 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/natural-selection that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
83d18d3 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/geometric-optics that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
c004d5a 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/sun that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
1e6593e 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/axon that referenced this issue May 31, 2023
@zepumph
move isClearingPhetioDynamicElementsProperty to tandem/, phetsims/tan…
748541a 
…dem#294

Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/projectile-motion that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
a1f62bf 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/tambo that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
eefccf0 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/ph-scale that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
752ef72 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/scenery-phet that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
e920a98 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/joist that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
94daf6d 
Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/joist that referenced this issue May 31, 2023
@zepumph
move isClearingPhetioDynamicElementsProperty to tandem/, phetsims/tan…
e36da79 
…dem#294

Signed-off-by: Michael Kauzmann <[email protected]>
zepumph added a commit to phetsims/scenery that referenced this issue May 31, 2023
@zepumph
move isSettingPhetioStateProperty to tandem/, phetsims/tandem#294
39e4e1b 
Signed-off-by: Michael Kauzmann <[email protected]>
@zepumph
Copy link
Member Author

zepumph commented May 31, 2023

@samreid, a spot check here would be good. It was pretty straight forward.

My process was to start with the new TinyProperty called newIsSettingPhetioStateProperty, which helped me confirm that all old usages were handled, then I renamed new back to the original name.

@zepumph zepumph assigned samreid and unassigned zepumph May 31, 2023
@samreid
Copy link
Member

samreid commented Jun 8, 2023

I spot checked some of the commits above and this all seems great, thanks! Closing.

@samreid samreid closed this as completed Jun 8, 2023
@phet-dev phet-dev reopened this Jun 8, 2023
@phet-dev
Copy link
Contributor

phet-dev commented Jun 8, 2023

Reopening because there is a TODO marked for this issue.

@zepumph
Copy link
Member Author

zepumph commented Jun 8, 2023

^^^^^^^^^^^^^^^^^^^^

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

So fun!

@zepumph zepumph assigned zepumph and unassigned samreid Jun 8, 2023
zepumph added a commit to phetsims/tambo that referenced this issue Jun 13, 2023
@zepumph
simplify notSettingPhetioStateProperty, phetsims/tandem#294
1c9e296 
Signed-off-by: Michael Kauzmann <[email protected]>
@zepumph
Copy link
Member Author

zepumph commented Jun 13, 2023

@samreid can you give this one more spot check. I believe that CAV has some sounds that you may be able to confirm are still working in the state wrapper.

@zepumph zepumph assigned samreid and unassigned zepumph Jun 13, 2023
@samreid
Copy link
Member

samreid commented Jun 14, 2023

The preceding commit is really great. CAV state wrapper sounds correct to me as far as I can tell. It has 2 sound effects that are triggered in step(dt) (soccer ball landing and continuous sound for the interval tool node) and I do hear those doubled in the downstream sim. I'm not sure how to address that but will open a side issue in CAV. Can this issue be closed?

@samreid samreid assigned zepumph and unassigned samreid Jun 14, 2023
@samreid samreid mentioned this issue Jun 14, 2023
Closed
@zepumph zepumph closed this as completed Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zepumph zepumph

Labels
dev:phet-io status:ready-for-review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@samreid @zepumph @phet-dev