phetsims / energy-skate-park-basics

"Energy Skate Park: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/energy-skate-park-basics
GNU General Public License v3.0
2 stars 9 forks source link

Track does not get smoothed out after repositioning #419

Closed megan-phet closed 6 years ago

megan-phet commented 6 years ago

Test device:

Operating System:

Browser:

Problem description:

For phetsims/QA#134. After repositioning a smooth track the jagged edges will reappear.

Steps to reproduce:

  1. connect two tracks together
  2. make a curve by dragging any point on the track.
  3. reposition the whole track

Screenshots:

video

Troubleshooting information (do not edit):

Name: ‪Energy Skate Park: Basics‬ URL: https://phet-dev.colorado.edu/html/energy-skate-park-basics/1.4.0-dev.1/phet/energy-skate-park-basics_en_phet.html Version: 1.4.0-dev.1 2018-06-20 00:09:06 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_5) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.1.1 Safari/605.1.15 Language: en-US Window: 1310x736 Pixel Ratio: 2/1 WebGL: WebGL 1.0 (2.1 INTEL-10.34.27) GLSL: WebGL GLSL ES 1.0 (1.20) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 15 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 16) Max viewport: 8192x8192 OES_texture_float: true Dependencies JSON: {"assert":{"sha":"928741cf","branch":"master"},"axon":{"sha":"f0522e7c","branch":"master"},"brand":{"sha":"89d28f63","branch":"master"},"chipper":{"sha":"e8946524","branch":"master"},"dot":{"sha":"6482f8c9","branch":"master"},"energy-skate-park-basics":{"sha":"288fcefd","branch":"master"},"joist":{"sha":"22e437d5","branch":"master"},"kite":{"sha":"a1086efc","branch":"master"},"phet-core":{"sha":"17326041","branch":"master"},"phet-io":{"sha":"3ea0727a","branch":"master"},"phet-io-wrapper-classroom-activity":{"sha":"53708616","branch":"master"},"phet-io-wrapper-hookes-law-energy":{"sha":"8a546a32","branch":"master"},"phet-io-wrapper-lab-book":{"sha":"1527afd6","branch":"master"},"phet-io-wrappers":{"sha":"8d814eab","branch":"master"},"phetcommon":{"sha":"6ec8cd89","branch":"master"},"query-string-machine":{"sha":"4182612f","branch":"master"},"scenery":{"sha":"88cb642e","branch":"master"},"scenery-phet":{"sha":"7bcde0b2","branch":"master"},"sherpa":{"sha":"88c3b828","branch":"master"},"sun":{"sha":"7579e8fa","branch":"master"},"tandem":{"sha":"8461b6f3","branch":"master"}}
jessegreenberg commented 6 years ago

This is not in the published version of the sim, and could have been introduced in changes for #417

jessegreenberg commented 6 years ago

Comparing diffs, I don't see how change for #417 could cause this. I verifed that updateTrackShape is being called on drag end. Comparing against deployed version 1.1, that version used Events, and is updating track shape on 'reset', 'smothed', and 'updated'. I see that TrackNode is now adding listeners to Emitters for these events, perhaps we are missing an emit() call?

jessegreenberg commented 6 years ago

By comparison to 1.1 branch, I am not seeing any missing emit calls. It is working correctly when dragging ControlPointNodes, maybe Ill have better finding what the difference is between track and control point dragging currently.

jessegreenberg commented 6 years ago

Also, it looks like this bit of code in TrackNode should be handling exactly this issue:

    track.draggingProperty.link( function( dragging ) {
      if ( !dragging ) {
        self.updateTrackShape();
      }
    } );
jessegreenberg commented 6 years ago

Oh, draggingProperty is not being updated when the track is dragged.

jessegreenberg commented 6 years ago

I added this to TrackDragListener on end and it seemed to fix the problem.

// notify listeners that the track is no longer being dragged
track.draggingProperty.value = false;

Was something like this in 1.1?

jessegreenberg commented 6 years ago

Sort of, used to be this:

track.dragging = false;

This may have been introduced in the Property->PropertySet refactor? Anyway, I think this should be fixed. @megan-phet could you please check on phettest?

megan-phet commented 6 years ago

@jessegreenberg, on the version in master, there are no tracks.

video

jessegreenberg commented 6 years ago

Uh oh! Thanks for letting me know.

jessegreenberg commented 6 years ago

Sorry about that @megan-phet, master should no longer have that problem. Back to you.

KatieWoe commented 6 years ago

Just took a look at this, and it seems to be fixed in master.

jessegreenberg commented 6 years ago

Thanks for verifying @KatieWoe. Closing this issue.