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

Replace Duration::as_secs by Duration::as_secs_f64 #299

Merged
merged 1 commit into from
Dec 30, 2020
Merged

Replace Duration::as_secs by Duration::as_secs_f64 #299

merged 1 commit into from
Dec 30, 2020

Conversation

musikid
Copy link
Contributor

@musikid musikid commented Dec 26, 2020

Hi!
This commit just replace the cast to f64 with the appropriate function.
Closes #296.

@coveralls
Copy link

coveralls commented Dec 26, 2020

Coverage Status

Coverage increased (+3.7%) to 38.196% when pulling d8cda02 on MusiKid:duration into c111eb4 on heim-rs:master.

@svartalf
Copy link
Member

Thank you for your PR!

Unfortunately, it does not really fixes this issue and also breaks current behavior; please compare Duration::as_secs_f64 sources:
https://github.com/rust-lang/rust/blob/e2a2592885539ca97bfb1232669e7519a0c0703b/library/core/src/time.rs#L643-L645

and current master branch HEAD:

// TODO: Can be replaced with a `delta_time.as_secs_f64()`
// as soon as https://github.com/rust-lang/rust/issues/54361 will be stable
const NANOS_PER_SEC: u32 = 1_000_000_000;
let mut delta_time_secs =
(delta_time.as_secs() as f64) + (delta_time.as_nanos() as f64) / (NANOS_PER_SEC as f64);

In our case whole following block must be replaced with Duration::as_secs_f64 call, as it is basically a bundled copy of it:

 const NANOS_PER_SEC: u32 = 1_000_000_000; 
 let mut delta_time_secs = 
     (delta_time.as_secs() as f64) + (delta_time.as_nanos() as f64) / (NANOS_PER_SEC as f64); 

@musikid
Copy link
Contributor Author

musikid commented Dec 29, 2020

Sorry!
I misunderstood what you were excepting to be replaced. I'm going to change this.

Signed-off-by: MusiKid <musikid@outlook.com>
@svartalf svartalf merged commit 21630c4 into heim-rs:master Dec 30, 2020
@svartalf
Copy link
Member

Great, thank you for your help!

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.

Replace bundled Duration::as_secs_f64 implementation with a real one
3 participants