phetsims / tandem

Simulation-side code for PhET-iO
MIT License
0 stars 5 forks source link

Autoselect target follows linked elements recursively (is that good?) #300

Closed samreid closed 1 year ago

samreid commented 1 year ago

While working on https://github.com/phetsims/center-and-variability/commit/f2941dc88774d5274cb83bc9ef8722dae0748184 in https://github.com/phetsims/center-and-variability/issues/204 I saw there were 2 levels of linked elements. The cardNode => cardModel => soccerBallModel. The value is changed on the soccer ball model. I incorrectly thought autoselecting linked element on the cardNode would autoselect to the cardModel, but it ended up all the way at soccerBallModel. @zepumph is this all good?

UPDATE: In this case it's probably OK since all of the important data is over in soccerBallModel. But some other sim may have important data in the cardModel layer itself.

zepumph commented 1 year ago

I agree. How does this sound?

zepumph commented 1 year ago

I am considering this a bug fix for the original implementation of sim-side autoselect over in https://github.com/phetsims/studio/issues/304

samreid commented 1 year ago

I reviewed the commit and confirmed it works well in center and variability. Some questions:

zepumph commented 1 year ago

Nice catch. To understand better, I was able to create a failing unit test that showed how we could still recurse into a Text.stringProperty if in linking selection mode. Anything else here?

I can't think of a better name than fromLinking want to think about something else?

samreid commented 1 year ago

Excellent, thanks! Looks good, closing.