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.

Sign up for GitHub

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

[Merged by Bors] - Fixes Timer Precision Error Causing Panic #2362

Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - Fixes Timer Precision Error Causing Panic #2362

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d39d5d5
Bug fix and tests for #2361
jacobgardner Jun 19, 2021
f9eba3f
Run cargo fmt against changed file
jacobgardner Jun 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion crates/bevy_core/src/time/timer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ impl Timer {

if self.finished() {
if self.repeating() {
self.times_finished = self.percent().floor() as u32;
self.times_finished =
(self.elapsed().as_nanos() / self.duration().as_nanos()) as u32;
// Duration does not have a modulo
self.set_elapsed(self.elapsed() - self.duration() * self.times_finished);
} else {
Expand Down Expand Up @@ -464,4 +465,20 @@ mod tests {
t.tick(Duration::from_secs_f32(0.5));
assert_eq!(t.times_finished(), 0);
}

#[test]
fn times_finished_precise() {
let mut t = Timer::from_seconds(0.01, true);
let duration = Duration::from_secs_f64(1.0 / 3.0);

t.tick(duration);
assert_eq!(t.times_finished(), 33);
t.tick(duration);
assert_eq!(t.times_finished(), 33);
t.tick(duration);
assert_eq!(t.times_finished(), 33);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't the extra tick be on this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So my logic was roughly this:

0 + 33.3333 = 33.3333 (Rounds down to 33 times finished, leaving the remainder as .3333)
.3333 + 33.3333 = 33.6666 (Rounds down to 33 again, remainder fo .6666)
.6666 + 33.3333 = 33.9999 (Rounds down last time to 33 with remainder of .9999)
.9999 + 33.3333 = 34.3332 (Rounds down to 34 this time)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will add the extra tick after the fourth iteration, then it will correctly add it every three iterations... I would have liked it to be every three from the start, but that's probably not possible when using those kind of numbers 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotta love floating point numbers 🙃

// It has one additional tick this time to compensate for missing 100th tick
t.tick(duration);
assert_eq!(t.times_finished(), 34);
}
}