phetsims / quadrilateral

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

incorrect behavior during `?shuffleListeners` #407

Closed samreid closed 1 year ago

samreid commented 1 year ago

Discovered in https://github.com/phetsims/axon/issues/215 and https://github.com/phetsims/quadrilateral/issues/398, the sim has some incorrect behavior during ?shuffleListeners.

image
samreid commented 1 year ago

I also saw this one time while dragging:

image
jessegreenberg commented 1 year ago

Thanks! I am seeing problems like this in master at the moment too without ?shuffleListeners, we may have a regression from recent commits.

jessegreenberg commented 1 year ago

Checking out recent SHAs, I start seeing bad behavior at 3696fa6a0b71858db56642b4d32f2e25f7002ea0, Ill take a look.

jessegreenberg commented 1 year ago

Fixed in the above commit, sim is fuzzing without error again. But there are still some problems with ?shuffleListeners. No errors but the shape looks wrong.

jessegreenberg commented 1 year ago

Here are the vertex positions: image

For a view that looks like this: image

So perhaps it is just a view problem. In this state if I only check one of the visibility checkboxes, the view corrects itself.

nly check one of the visibility checkboxes, the view corrects itself.

Nope, that is just the markersVisibleProperty because the multilink for re-drawing sides has this:

Multilink.multilink( [ side.vertex1.positionProperty, side.vertex2.positionProperty, markersVisibleProperty ], ( vertex1Position, vertex2Position, markersVisible ) => {

The multilink has the right Vertex position values, but it uses a QuadrilateralSide.modelLine to create the shape. So here is the problematic order of events.

This would be fixed if the QuadrilateralSide.modelLine was observable, so that the view can update after the modelLine is set. Or, QuadrilateralSide.modelLine could be removed and we could just use the vertex positions.

jessegreenberg commented 1 year ago

Alright, fixed in the above commit. I made the side model Line Shape observable so we can link to it and guarantee that it will be up to date after vertex positions have been set.

Playing with the sim, I don't see any more odd behavior and no errors when fuzzing with ?shuffleListeners. Closing.