hapticdata / animitter

requestAnimationFrame loop + EventEmitter for browser, VRDisplay, node.js and AMD.
MIT License
28 stars 4 forks source link

Pause/Resume fn? #2

Closed binarykitchen closed 7 years ago

binarykitchen commented 7 years ago

Your work is impressive. Almost want to use it for https://github.com/binarykitchen/videomail-client if there were pausing and resuming functionality.

Any chance these could be implemented? Totally aware that they can complicate things when it comes to fps + delta calculations.

hapticdata commented 7 years ago

The animitter instance has animitter().stop() and animitter().start() which also emit 'stop' and 'start' events. This should suit the needs for pausing and resuming. Is there a difference in behavior you are suggesting?

hapticdata commented 7 years ago

start() and stop() should cover any pausing and resuming needs. If there is additional functionality you are suggesting feel free to open this back up.

binarykitchen commented 7 years ago

they do? that was not clear when reading documentation.

do you think you can adjust that and also add an unit test case which covers this?

hapticdata commented 7 years ago

The test tracking animitter.running counter starts multiple instance, stops them and then starts them again.

It does look like adding a test to verify the progression of deltaTime and elapsedTime could be beneficial. Is there any other aspect you were thinking needed a test?

I think you are right that there could be a clearer example of starting, stopping, and starting again a loop in the readme. The difference between stop() and complete() is that complete() wont let you restart the loop (unless you have called reset() first).

binarykitchen commented 7 years ago

Yeah, documentation/examples could be more clear about the pausing/resuming functionality.

Also what's lacking is to pause the elapsedTime counter upon pausing as well I think.

Anyway, I would cover all that in new unit tests carefully to be sure all works as expected.

You might ask why I am questioning all that. If you look at my other app source code here... https://github.com/binarykitchen/videomail-client/blob/develop/src/wrappers/visuals/recorder.js#L737

...you can see that I am counting the elapsedTime outside of your code for now.

hapticdata commented 7 years ago

I just published 3.0.0 to NPM (since these are breaking changes). With this update elapsedTime and deltaTime don't include time where the loop is stopped. You should no longer need to calculate this outside the loop like in your example. Additionally getFPS() now returns the calculated current frame rate as opposed to the user provided one, which is now getFPSLimit().

I've added a new example to the README as well as added an additional test for getElapsedTime().

Thank you for your help and suggestions!

binarykitchen commented 7 years ago

Looking great - thanks heaps

But regarding function names, I would suggest to have the pause functionality, which is currently the stop fn to be renamed to pause() and have a resume() likewise. And start() is meant only for starting a new loop. And stop() means the same as complete().

hapticdata commented 7 years ago

I've thought about this a bit. In your use case where the loop is heading towards a destination (completing the recording of a video) the function names pause() and resume() do make more sense (as they similarly would for a tweening engine).

animitter itself is intended as a feature-rich wrapper around requestAnimationFrame and not exclusively for loops that have a sense of progress they are tracking towards. Often loops are on-going until the page is exited (such as a canvas / WebGL based site) for the broader applicable uses it makes sense to keep these functions as stop() to halt the loop and start() to activate the loop.