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

Make DELTA_TIME configurable #5

Closed
johanhelsing opened this issue Mar 12, 2023 · 2 comments · Fixed by #22
Closed

Make DELTA_TIME configurable #5

johanhelsing opened this issue Mar 12, 2023 · 2 comments · Fixed by #22
Labels
enhancement New feature or request

Comments

@johanhelsing
Copy link
Contributor

johanhelsing commented Mar 12, 2023

Okay, so after #4 the algorithm logic got a lot less awkward, and that opens up some possibilities. Currently DELTA_TIME is hard-coded to 60 Hz, but it should probably be configurable at some level.

The obvious thing would be to just make it a resource:

pub struct PhysicsTimeStep(pub f32)

However I'm wondering, would that hurt performance? Does the rust compiler precompute constants, that means we shouldn't do this?

If so, perhaps we should consider if we can somehow use const-generics to still make it configurable at build-time (at least at an internal level).

Second question is: would it make sense to also allow a variable timestep? i.e. consume the remainder of the accumulator with one not-whole physics step.

Would obviously not be deterministic, but could perhaps produce smoother visuals if you don't do smoothing in your renderer.

@Jondolf
Copy link
Owner

Jondolf commented Mar 16, 2023

I think we should probably just make it a resource like NumSubsteps for the sake of ergonomics and consistency. I doubt it would have any meaningful performance impact.

We could also group simulation configuration like this under one resource like XpbdConfig/XpbdContext or PhysicsConfig/PhysicsContext, although I'm not sure if it's more standard in bevy to have many small resources or a few larger ones.

Second question is: would it make sense to also allow a variable timestep? i.e. consume the remainder of the accumulator with one not-whole physics step.

I haven't tried other timesteps before, but it would probably be good to have them available, and it should be quite straightforward to implement it now.

bevy_rapier seems to have a variable timestep here, and it also seems to have an "interpolated timestep".

@Jondolf Jondolf added the enhancement New feature or request label Mar 16, 2023
@Jondolf
Copy link
Owner

Jondolf commented Mar 22, 2023

I'm working on this, PR coming soon

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

Successfully merging a pull request may close this issue.

2 participants