Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SimpleDragHandler disposal #647

Closed
jonathanolson opened this issue Jul 12, 2017 · 2 comments
Closed

SimpleDragHandler disposal #647

jonathanolson opened this issue Jul 12, 2017 · 2 comments

Comments

@jonathanolson
Copy link
Contributor

I'm going to add disposal to SimpleDragHandler itself.

What happened in a few cases (notably Sun's error below) was that a full slider that was being interacted with would get disposed. Since its thumb was being dragged, its SimpleDragHandler had a listener attached to the pointer (which disposal didn't exist at the base level, only in TandemSimpleDragHandler, so it didnt' get removed immediately). This led to the listener being called at a later time after the node it was on was disposed.

The standard way of handling this seems like it should dispose the drag listener when its associated Node is disposed.

I didn't have nodes dispose their input listeners, as there are some usages across sims of listeners that are given to many nodes, and other patterns where disposal wouldn't be appropriate.

Tagging for developer meeting to discuss the need, and if this should become a common pattern.

Sun example error:

sun : fuzz : require.js-canvas : run
Uncaught Error: Tried to get matrix after disposing a Node
Error: Tried to get matrix after disposing a Node
    at HSlider.self.(anonymous function) [as getMatrix] (https://bayes.colorado.edu/continuous-testing/snapshot-1499877867087/scenery/js/nodes/Node.js?bust=1499878711088:5072:19)
    at Trail.getMatrix (https://bayes.colorado.edu/continuous-testing/snapshot-1499877867087/scenery/js/util/Trail.js?bust=1499878711088:189:43)
    at Trail.getTransform (https://bayes.colorado.edu/continuous-testing/snapshot-1499877867087/scenery/js/util/Trail.js?bust=1499878711088:225:35)
    at handleTrackEvent (https://bayes.colorado.edu/continuous-testing/snapshot-1499877867087/sun/js/HSliderTrack.js?bust=1499878711088:87:50)
    at drag (https://bayes.colorado.edu/continuous-testing/snapshot-1499877867087/sun/js/HSliderTrack.js?bust=1499878711088:108:9)
    at Object.move (https://bayes.colorado.edu/continuous-testing/snapshot-1499877867087/scenery/js/input/SimpleDragHandler.js?bust=1499878711088:131:29)
    at Input.dispatchToPointer (https://bayes.colorado.edu/continuous-testing/snapshot-1499877867087/scenery/js/input/Input.js?bust=1499878711088:775:27)
    at Input.dispatchEvent (https://bayes.colorado.edu/continuous-testing/snapshot-1499877867087/scenery/js/input/Input.js?bust=1499878711088:748:12)
    at Input.branchChangeEvents (https://bayes.colorado.edu/continuous-testing/snapshot-1499877867087/scenery/js/input/Input.js?bust=1499878711088:686:14)
    at Input.moveEvent (https://bayes.colorado.edu/continuous-testing/snapshot-1499877867087/scenery/js/input/Input.js?bust=1499878711088:651:26)
Approximately 7/12/2017, 10:44:27 AM
@pixelzoom
Copy link
Contributor

This may also be the cause of phetsims/unit-rates#200.

@pixelzoom
Copy link
Contributor

7/13/17 dev meeting:
No longer an issue since we're not going to scuttle Node, see #649. SimpleDragHandler.dispose will remain available in case someone needs it.

Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants