Use of timer in scenery

[zepumph]

A11y folks are using PHET_CORE's timer in scenery. This doesn't seem to be supported outside of phet though. @jonathanolson is there a way that we could have that timer support not in phet sims? Alternatively we would abandon this use. @jonathanolson what do you think. Current usages are in PressListener, KeyboardFuzzer, and KeyStateTracker (actively worked on as part of #850).

[zepumph]

Pinging @jonathanolson for comment again, because this issue informs how we proceed with implementing a keystate tracker in #850. It also may inform our ability to move KeyboardDragListener into scenery.

[jonathanolson]

Could things just be tied in to when Display.updateDisplay() is potentially called?

[zepumph]

We thought about that. Could updateDisplay take an optional dt value?

[jonathanolson]

We thought about that. Could updateDisplay take an optional dt value?

Definitely.

[jessegreenberg]

Part of this was done in #909 so we could step the Display's KeyStateTracker. updateDisplay now takes an optional dt for this.

The next step is to review other usages of timer added for a11y. KeyStateTracker global could be stepped in updateDisplay but I am not sure how timers used in individual listeners will change yet. This applies to KeyboardDragListener (which we hope to move to scenery) and PressListener.

[jessegreenberg]

In support of #931, @zepumph and I looked into this again with help from @samreid. Since we last looked into this timer was moved to axon for other reasons (phetsims/phet-core#51). This makes it more accessible in scenery.

In the above commits we removed the dt arg from updateDisplay and chose to make it the responsibility of the scenery client to call timer.emit( dt ) prior to calling updateDisplay. We added this to Display.updateOnRequestAnimationFrame.

Remaining tasks are to add documentation somewhere that describes this requirement, and to do a final review of Display.updateDisplay() usages to see if we need to add any more timer.emit calls.

[jessegreenberg]

There is a usage of updateDisplay in PhetioStateEngine. @zepumph or @samreid does this require timer.emit? Or maybe not because timer extends Emitter and therefore state remains up to date due to its own PhET-iO instrumentation?

[jessegreenberg]

All the other usages are in tests and I don't believe they require this. Scenery unit tests are passing.

[jessegreenberg]

The best place I could think of for this documentation is scenery/doc/user-input, but maybe there is a better spot. Added in the above commit.

@jonathanolson can you please take a look at the change to Display.updateDisplay and Display.updateOnRequestAnimationFrame in 4f0bb50. And can you think of a better place for the documentation in 013b758?

[samreid]

There is a usage of updateDisplay in PhetioStateEngine. @zepumph or @samreid does this require timer.emit?

timer.emit should not be called from PhetioStateEngine

[zepumph]

I think all that is left for this issue is for @jonathanolson to review per #884 (comment). Unassigning.

[jonathanolson]

This seems like a good approach, documentation and implementation look good. Thanks!