search

phetsims / energy-skate-park

"Energy Skate Park" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
2 stars 11 forks source link

Assertion: model has no saved EnergySkateParkDataSamples to retrieve #278

Closed jessegreenberg closed 4 years ago

jessegreenberg commented 4 years ago

Not on CT, but after quickly dragging the cursor as far left as possibhle on the graphs screen I hit

Assertion failed: model has no saved EnergySkateParkDataSamples to retrieve

jessegreenberg commented 4 years ago

Plot looks like this: image

dataSamples array is empty. cursor value is 60.528619583038434. Triggered from the drag listener on the cursor during

const closestSample = model.getClosestSkaterSample( this.getCursorValue() );
jessegreenberg commented 4 years ago

I am confused because I thought data wasn't cleared until drag end.

jessegreenberg commented 4 years ago

I notice the sim is paused in the assertion case: image

jessegreenberg commented 4 years ago

Hit it again. Seems like I really have to jerk the mouse to get it to happen.

jessegreenberg commented 4 years ago

Caught the trace that is clearing samplese during drag: image

So what is happening is the cursor is made invisible during drag by being dragged all the way to the left. Changing visibility interrupts the listeners, calling end drag and clearing samples before we release. We should not be changing cursor visibility while it is being dragged.

EDIT: Actually, it seems like interrupting when it becomes invisible is correct. What shouldn't be allowed is dragging the cursor so that it leaves the chart. We ahve a precision issue. image So the cursor value is 10^-13 to the left of the edge of the graph.

jessegreenberg commented 4 years ago

XYCursorPlot keeps the cursor within the chart bounds:

    // keep the cursor within the grid bounds
    this.chartCursor.centerX = Utils.clamp( viewPosition, 0, this.plotWidth );
    this.chartCursor.centerY = this.gridNode.centerY;

We should keep the cursor value within bounds as well, instead of just the Node position.

jessegreenberg commented 4 years ago

Should be fixed in the above commit. The cursor value is now constrained to be within plot bounds. I think this is correct and we won't have precision comparison issues like https://github.com/phetsims/energy-skate-park/issues/278#issuecomment-656183427 because calculations for visibility and constraining are the same. Closing.