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

Couple of World.step() proposals #392

Closed
wants to merge 3 commits into from
Closed

Couple of World.step() proposals #392

wants to merge 3 commits into from

Conversation

swift502
Copy link

@swift502 swift502 commented Aug 4, 2018

I'd like to propose a couple of changes that make my life easier. I don't know for certain that they're generally good for all use cases.

1. Fixed timestepping happens on timeSinceLastCalled = -1 instead of 0

In my app, I have a variable time scale. When I set the value to zero, I would expect the simulation to stop. But instead it switches to the fixed time stepping mode.

I can easily get around it by checking if I'm about to pass timeSinceLastCalled of zero, but it seems pretty unintuitive and unnecessary, as I think the exception can be made for an impossible time step like "-1", instead of the current "0".

It shouldn't break people's code, unless they deliberately passed 0 to the step() function to get fixed time stepping, which seems very unlikely.

Video:

2. Reset world time accumulator when simulation is limited by maxSubSteps

When using the maxSubSteps limiting variable, the world accumulator keeps growing, and it then causes issues when simulation is limited by the maxSubSteps value for a while and then comes back to valid timeSinceLastCalled times, because the simulation needs to catch up and deplete the accumulator by simulating maxSubSteps of frames every step() call no matter what the timeSinceLastCalled is.

Video probably explains it more clearly. When I try to get back to regular time scale values, cannon simulates 10 subSteps, even though I only want one.

I'm not sure if this is just my use case or if people use this feature in a different way, where this behaviour is fine. I do know that ammo.js (Bullet) works like my proposed solution, as the issue didn't exist when I ran on ammo.

Video:


if(timeSinceLastCalled === 0){ // Fixed, simple stepping
if(timeSinceLastCalled === -1){ // Fixed, simple stepping
Copy link
Author

Choose a reason for hiding this comment

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

Or better yet, let's not define an exception value at all. Let's have:

if(typeof timeSinceLastCalled === 'undefined'){ // Fixed, simple stepping

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