phetsims / scenery-phet

Reusable components based on Scenery that are specific to PhET simulations.
MIT License
8 stars 6 forks source link

MeasuringTapeNode should not reset Properties that it did not create. #822

Closed pixelzoom closed 11 months ago

pixelzoom commented 11 months ago

MeasuringTapeNode should not be resetting basePositionProperty and tipPositionProperty if it did not create them -- ie, if they were provided via options.

This is causing problems in solar-system-common, where we have a measuring tape model element that is responsible for creating and resetting these Properties. (Until this is fixed, we'll refrain from calling MeasuringTapeNode reset.)

Changing this will require testing reset behavior of all sims that use MeasuringTapeNode.

None of the authors of MeasuringTapeNode are available. So assignig to the responsibile dev for scenery-phet to decide how to proceed.

jessegreenberg commented 11 months ago

Sure, I will take a look this might be quick.

Hits for new MeasuringTapeNode:

extends MeasuringTapeNode:

jessegreenberg commented 11 months ago

https://github.com/phetsims/scenery-phet/commit/6e4cd6049f9592a917dbbaa4cb2d6f78a9735825 makes this change, adding the blocks-publication label until this is tested.

jessegreenberg commented 11 months ago

OK, I tested and updated usages noted above and added a note for each one. @pixelzoom can you please review and any other testing you would like to see?

pixelzoom commented 11 months ago

Looks really great. Thanks @jessegreenberg!