phetsims / forces-and-motion-basics

"Forces and Motion: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/forces-and-motion-basics
GNU General Public License v3.0
7 stars 10 forks source link

Replace PropertySet with Property #226

Closed jessegreenberg closed 7 years ago

jessegreenberg commented 7 years ago

From phetsims/axon#102, PropertySet is deprecated and should be replaced with Property.

jessegreenberg commented 7 years ago

In #235, we will have a new RC for Forces and Motion Basics. If we do this quickly now, the replacement will be tested in the next RC.

jessegreenberg commented 7 years ago

Actually, this conversion could definitely have an impact on phetsims/QA#12, and may make it difficult to cherry-pick changes into the release branch 2.3-phetio. Ill coordinate with @zepumph about this, either waiting until 2.3-phetio is deployed or if we decide 2.3-phetio needs master SHAs.

zepumph commented 7 years ago

Thanks @jessegreenberg. I hope to have #232 figured out before I leave Friday, so that it won't put this on hold for too long.

jessegreenberg commented 7 years ago

Thanks @zepumph. In #235 we decided to take both 2.3 and 2.3-phetio from master so I will proceed with this issue now.

jessegreenberg commented 7 years ago

One down, 5 to go.

jessegreenberg commented 7 years ago

I was concerned about breaking phetio features during this refactor. In addition to sanity checking wrappers, @zepumph recommended to compare ids in the instance proxies wrapper before and after the change. I have a list of id's before changes related to this, and will compare once refactor is done.

EDIT: would paste here, but it is quite long and would make the issue comments difficult to read.

jessegreenberg commented 7 years ago

PropertySet was removed from this sim. Comparing the phetio id's before and after the refactor is showing no changes:

screen shot 2017-08-21 at 4 42 53 pm

This sim has a lot of Properties, and it was using Events and convenience functions of PropertySet, so I want to do some testing to make sure that things didn't break.

jessegreenberg commented 7 years ago

PropertySet has been removed. I have been spot checking the sim, trying to break things after the refactor but features seem stable. Ill remove references to Property.preventGetSet and close.