phetsims / projectile-motion

"Projectile Motion" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
15 stars 13 forks source link

Assertion while interacting with keypad on "Lab"screen #255

Closed jbphet closed 3 years ago

jbphet commented 3 years ago

While working on #239 I ran into an issue where an assertion is hit. This was on master. Here's the sequence to reproduce:

window.assertions.assertFunction (assert.js:23)
addListener (TinyEmitter.js:114)
link (Property.js:400)
(anonymous) (PressListener.js:307)
emit (TinyEmitter.js:86)
(anonymous) (Emitter.js:39)
execute (Action.js:225)
emit (Emitter.js:64)
push (createObservableArray.js:274)
enter (PressListener.js:697)
dispatchToListeners (Input.js:1842)
dispatchToTargets (Input.js:1874)
dispatchEvent (Input.js:1795)
enterEvents (Input.js:1728)
branchChangeEvents (Input.js:1698)
Input.validatePointersAction.Action.phetioPlayback (Input.js:281)
execute (Action.js:225)
validatePointers (Input.js:862)
updateDisplay (Display.js:408)
(anonymous) (Sim.js:309)
execute (Action.js:225)
stepSimulation (Sim.js:1060)
stepOneFrame (Sim.js:1050)
runAnimationLoop (Sim.js:1028)
.
.
.

I tested this sequence on the current published version and it works fine.

jbphet commented 3 years ago

@jonathanolson - Can you please take a look at this? The problem seems to be occurring down in the bowels of the input listener system. The listener that is being re-added is invalidateHovering, and I've run several tests and added breakpoints in various places, and it's not at all obvious to me what is being done or not done that is leading to the incorrect behavior.

You can start by executing the sequence above and looking at the stack trace and state and see if it makes sense to you. The listener that brings up the keypad is added in CustomProjectileObjectTypeControl and looks like this:

    numberDisplay.addInputListener( new FireListener( { // no removeInputListener required
      fire: editValue,
      fireOnDown: true,
      tandem: numberDisplayTandem.createTandem( 'fireListener' ),
      phetioDocumentation: 'When fired, calls listener to open UI to edit the value for this NumberDisplay'
    } ) );
jonathanolson commented 3 years ago

It looks like only one code-path (clicking outside of the edit box) was clearing the over-pointers. Clicking on the enter button however was not doing this. I added it in the above commit, @jbphet can you review? See the existing this.clickOutsideFireListener.clearOverPointers();

zepumph commented 3 years ago

So It looks like https://github.com/phetsims/scenery/issues/1021 has determined that clearOverPointers is the best we can do in this case. I'll let https://github.com/phetsims/acid-base-solutions/issues/172 know that it may be the same case.

jbphet commented 3 years ago

Thanks @jonathanolson and @zepumph. This does seem to fix the problem, so I'll close the issue, but I commented in the Scenery issue that this seems a bit tricky to have to remember.