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

Fix interval duration bug #477

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix interval duration bug #477

wants to merge 2 commits into from

Conversation

morteako
Copy link
Collaborator

Solves #473

The first interval lasted 1 second too long, since it would need f.ex. 2 elapsed seconds to be larger than a duration of 1 second.
This worked out fine for the rest of the parts, because the would start then be initialized with the extra time (1 second)
from : newElapsed - currentPartDuration * 1000

The fix is in the first commit
Also changed some names and comments.
The naming using prev confused me a bit, since it is actually the current, so i removed that prefix

Gifs

Bug: Here we see that the first part lasts 2 seconds (0:01 and 0:00), even though it should only last 1
This is fixed in pr-gif, (first and all parts are just 0:01. which seems weird, but 1-second intervals are weird)

One consequence of this pr is that we now never reach 0:00, and 0:01 is now the last second displayed for an interval.
Another consequence is that the first second of every interval (not just first as before), will be shown as the full duration.
F.ex. a 5:00 interval will go from (5:00 4:59 4:58...) instead of the old (4:59)

Current (bug)

2025-01-23 21 03 42

PR (fix)

2025-01-23 21 02 21

The first interval lasted 1 second too long, since it would need f.ex. 2 elapsed seconds to be bigger than a duration of 1 second.
This worked out fine for the rest of the parts, because the would start then be initialized with the extra time (1 second)
from : newElapsed - currentPartDuration * 1000
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.

1 participant