-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Timing, frame-rate consistency improvements and features #777
Conversation
Added delta property to engine update event Added delta argument to various internal functions Changed timeScale argument to use delta instead on various internal functions Fixed issues when using an engine update delta of 0 Improved time independence for friction, air friction, restitution, sleeping, collisions, constraints Removed optional correction argument from Engine.update Removed correction and timeScale from Body.update and Matter.Runner
…y.translate, Body.rotate Added Body.setSpeed, Body.setAngularSpeed Added Body.getSpeed, Body.getVelocity, Body.getAngularVelocity Changed all velocity functions to be time independent
I just bought a 144 hz monitor so I'll be testing this branch asap. Edit: At first glance, simulations now appear much more consistent between 144hz and 60hz for me. With previous versions, the physics varied quite a bit. Will continue to use this branch to see how it behaves and will post updates here. |
@liabru, any update on how this branch looks to you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is amazing! I received some complaints from users on a 144hz monitor that my game was broken, and this branch fixes every issue.
One thing though, applyForce
doesn't take into account frame rate, so I am multiplying the force I want by (1000/60)*currentDelta
. In a way, it's kinda nice because it allows me to set anything I want as the currentDelta
(I'm using a similar method as you here). But it might not be super intuitive that you have to do that.
I need to do some more testing but I'm seriously considering using this branch on my live game.
src/core/Common.js
Outdated
@@ -10,6 +10,7 @@ module.exports = Common; | |||
|
|||
(function() { | |||
|
|||
Common._timeUnit = 1000 / 60; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great if this property dropped the _
. There are some cases where I use it to make Body.applyForce
framerate insensitive.
Example from my code:
// deltaAdjustment is Common._timeUnit divided by the smallest delta over the last 30 frames
// (I'm using something similar as your Runner.tick here https://github.com/liabru/matter-js/blob/82bb41535b09e02dd76a20d5955fc7b4102487d6/src/core/Runner.js#L132)
const recoilFactor = -(body.mass * 0.09) * deltaAdjustment;
const recoil = Vector.mult(norm, recoilFactor);
// recoil
Body.applyForce(playerControlledBody, playerControlledBody.position, recoil);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason is that it's an internal constant so your code shouldn't rely on it. Body.applyForce
now accounts correctly so I guess you won't need it?
src/body/Body.js
Outdated
body.positionPrev.x = body.position.x - velocity.x * timeScale; | ||
body.positionPrev.y = body.position.y - velocity.y * timeScale; | ||
body.velocity.x = velocity.x * timeScale; | ||
body.velocity.y = velocity.y * timeScale; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes you made to the way velocity work broke something in my game. It's a game where you have a character that can shoot bodies while moving. I want the speed of the bullet to be the addition of the speed of the character and a force at which the bullet is thrown. I was doing this by:
- making a bullet body
- setting the velocity of the bullet to the velocity of the character
- applying a force to "throw" the bullet
Obviously, as you warned here, I shouldn't have done it this way, so it's okay that it broke (I'm doing it properly now 😛 ). But I thought you might want to know. 🙂
*/ | ||
Engine._bodiesUpdate = function(bodies, deltaTime, timeScale, correction, worldBounds) { | ||
Engine._bodiesUpdate = function(bodies, delta) { | ||
for (var i = 0; i < bodies.length; i++) { | ||
var body = bodies[i]; | ||
|
||
if (body.isStatic || body.isSleeping) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deltaTime of the static and sleeping bodies do not get set. So if you are doing getVelocity
on them you'll get NaN
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I think this should be fixed now with the initial value for body.deltaTime
?
@liabru , any update on this branch? Would you like some help resolving some of the above comments? |
I am now updating my physics at a fixed timestep so I am no longer using this but FYI there seems to be something wrong with the frictions. Bodies were sliding much faster on a body with friction set to 0 at 144Hz than at 60Hz. |
Are you also doing the interpolation as described in that article? |
@wmike1987 yes, my implementation is strongly inspired by https://github.com/IceCreamYou/MainLoop.js I wish I could have used MainLoop.js directly but it didn't fit nicely in my ECS-based architecture. |
* master: (32 commits) fix lint update dependencies Revert "Merge branch 'pr/526'" Revert "Merge branch 'pr/527'" changed alpha build configuration add window global, stub require and handle bad values in test tools added overlap metric to test tools fix path to build in test worker implemented threaded comparison testing fixed plugins in compare Added build comparison tools and tests Added config and test files to lint Set loose build version on dev server Added watch content base to dev server added timing to engine snapshot updated readme added tag push to release task updated readme removed yuidocjs dev dependency removed unused gulp release tasks ... # Conflicts: # src/collision/Resolver.js # src/core/Engine.js
These changes seem to finally fix velocity being framerate dependent. Any chance to merge this to master soon? |
* master: increase overlap test updates added readme note about vue watchers increase test timeout remove travis config added ci workflow action
@liabru I see you're still working on this, from time to time - are there any plans to merge it into master? I think a lot of people are waiting for this :) |
@photonstorm that's the plan, a major goal was to get constraint stiffness and friction to be consistent across deltas, which I think is now working nicely. Did you try it out at all? |
@liabru Hi, any chance to merge this to master soon? |
Not yet, but I'm working on the 3.60 release of Phaser right now, so would love to have this merged in! I'll try it out next week with our set of examples - if those all work without issue, that would be a really healthy sign. |
Looking forward to the merge! I have a few games that are crying out for this update. |
@liabru I don't suppose there's any chance of you merging this into master, is there? :) I really think it could be beneficial! |
Look out for these changes in the next release. Thanks again to all those who provided feedback here! |
Features
60hz
as a baselineBody.setAngularVelocity
andBody.setVelocity
functions to be timestep independentBody.setSpeed
,Body.setAngularSpeed
,Body.getSpeed
,Body.getVelocity
,Body.getAngularVelocity
updateVelocity
argument toBody.setPosition
,Body.setAngle
,Body.translate
,Body.rotate
correction
fromEngine.update
as it is now built inNotes
When using a fixed timestep of
60hz
(~16.666ms
engine delta) results should look similar to before, as this was taken as the baseline.If you're using a non-fixed timestep or one other than
60hz
(~16.666ms
) results should now become more similar to the60hz
baseline, therefore you may need to adjust e.g. body and constraint properties.Since
Body.setAngularVelocity
andBody.setVelocity
are now timestep independent, you may need to adjust code you may have been using that factored in the timestep.For timestep independence, the
Matter.Body
speed and velocity related get / set functions now relate to a fixed time unit rather than timestep, currently set as1000/60
for easier backwards compatibility at the baseline60hz
.Note that
Body.applyForce
naturally still remains timestep dependent as before, see the updated Body.applyForce docs for details.While the properties
body.velocity
andbody.speed
(and angular versions) still exist they are not typically recommended for user code, in most cases you should switch to the newBody.getVelocity
andBody.getSpeed
as they are timestep independent.Examples have been updated to be timestep independent, as well as using the new functions.
Requesting reviewers, testers and comments
If anybody has any time to help me test this update out further and feedback any issues, it would be greatly appreciated:
delta
values inEngine.update
(more than33.333ms
won't be stable)engine.timing.timeScale
(as another way of modifying delta)updateVelocity
, body velocity and speed setters / gettersHighlighted changes
body.deltaTime
Engine.update
correction
andtimeScale
fromBody.update
andMatter.Runner
updateVelocity
argument toBody.setPosition
,Body.setAngle
,Body.translate
,Body.rotate
Body.setSpeed
,Body.setAngularSpeed
,Body.getSpeed
,Body.getVelocity
,Body.getAngularVelocity
Body.setAngularVelocity
andBody.setVelocity
functions to be timestep independent