phetsims / buoyancy

"Buoyancy" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 2 forks source link

visibleProperty disposal bug #98

Closed zepumph closed 3 months ago

zepumph commented 3 months ago

Reported over in https://github.com/phetsims/buoyancy/issues/76#issuecomment-1944352561.

In the BlockSetModel, we dispose the MassView based on the visibility of the mass. Since https://github.com/phetsims/buoyancy/issues/76 there is a bug due to the fact that we set the visibility of the focusHighlight path based on the visibility of the Mass.

In this weird case, we get a disposal bug because the model visibleProperty fires for a change with two listeners. The first to dispose the view, and the second to update the visibility of an element in the view. During the first listener, the second listener is removed, but since we are in the middle of notifying, we guard listeners, keeping the original list of two intact, so the second listener will fire and update a disposed entity.

zepumph commented 3 months ago

I wanted to try to use some sort of view visibility, but I can't really seem to set that up since the MassView is a THREE/Mass, not a scenery/Node.

AgustinVallejo commented 3 months ago

The ghosts problem was apparently solved by just disposing the focusablePath. And we dont need a visibleProperty since the MassView gets disposed when invisible. Removing TODO in the above commit. Closing.