assertion failure: tried to removeListener on something that wasn't a listener

[pixelzoom]

Related to #2 (code review).

1. Start the sim with ?ea
2. Go to "One Dimension" screen
3. Try to drag one of the masses

This assertion failure occurs:

assert.js?bust=1581376312284:22 Uncaught Error: Assertion failed: tried to removeListener on something that wasn't a listener
    at window.assertions.assertFunction (assert.js?bust=1581376312284:22)
    at TinyEmitter.removeListener (TinyEmitter.js?bust=1581376312372:104)
    at BooleanProperty.unlink (Property.js?bust=1581376312372:380)
    at MassNode1D.js?bust=1581376312372:100
    at TinyEmitter.emit (TinyEmitter.js?bust=1581376312372:68)
    at BooleanProperty._notifyListeners (Property.js?bust=1581376312372:279)
    at BooleanProperty.set (Property.js?bust=1581376312372:178)
    at DragListener.MassNode1D.dragCallback [as _dragListener] (MassNode1D.js?bust=1581376312372:52)
    at DragListener.drag (PressListener.js?bust=1581376312372:434)
    at DragListener._dragAction.Action.parameters.name (DragListener.js?bust=1581376312372:250)

[pixelzoom]

A similar failure occurs in the "Two Dimensions" screen.

[pixelzoom]

Fixed, ready for review. Lesson is: don't unlink something that's not a listener.

[tmildemberger]

For some reason, before 8751534, even when though we tried to unlink something that was not a listener, the listener that we meant to unlink was unlinked after the (incorrect) call to unlink and the arrows were correctly hidden. After that commit, as we were always trying to unlink something that was not a listener, unlink was not called at all and even after holding and releasing a mass, the arrows kept showing up.

The real problem was that the callback variable was being created inside the function that was linking and unlinking the listener, so we were using different functions to link and unlink, while the reason to the line const callback = this.overUpCallback.bind( this ); to exist was exactly to be able to link callback to dragListener.isOverProperty and later use the same callback to unlink. Moving the line out of the function yields the expected behaviour.

[pixelzoom]

Thanks for explaining the problem with context of the callback variable, that makes sense and I hadn't noticed. I do think it's still worth checking hasListener before calling unlink. arrowsVisibilityProperty currently has an initial value of false. But if arrowsVisibilityProperty had an initial value of false, then unlink would be called before link, and would trigger the "tried to removeListener on something that wasn't a listener" assertion.

[tmildemberger]

Yes, that makes sense. The check is still there, and I'll remember to call hasListener before calling unlink the next time.