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

Prevent timer from lagging behind #41

Merged
merged 1 commit into from
May 18, 2019
Merged

Prevent timer from lagging behind #41

merged 1 commit into from
May 18, 2019

Conversation

twinkelmann
Copy link
Contributor

Implemented the timer with setTimeout instead of setInterval to control the delay between each calls and prevent accumulation of lag.

Instead of accumulating ~2 ms of lag on each interval, this method will keep the lag in check on the long run.
It cannot accumulate as each interval is calculated with the current lag in mind.

This change partially addresses #32 (window in background might still cause issues, so I can't say this alone will fix it).

@Splode
Copy link
Owner

Splode commented May 18, 2019

Great strategy. I think this will definitely help the timer's accuracy.

Thanks for the awesome contribution! 💯

@Splode Splode merged commit ce8d377 into Splode:master May 18, 2019
@Splode Splode mentioned this pull request May 19, 2019
@twinkelmann
Copy link
Contributor Author

Glad to contribute 👍

@NeonArray
Copy link

I'm curious if using performance.now() would be more accurate than Date? I'd be interested to see some performance benchmarks as well as some sort of verifying unit tests. I'm just wondering which method is more accurate for a timer in this context, and without tests it's all just sort of assumption and conjecture.

Not saying it would make a difference here necessarily. I just happened to be reading about timers the other day and thought I'd throw this information at you guys for your consideration.

I'd also still consider using Electrons built in API's for timing as they seem a bit more "bullet proof."

Anyway, happy coding!

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

Successfully merging this pull request may close these issues.

3 participants