-
-
Notifications
You must be signed in to change notification settings - Fork 192
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
[#643] Fix Actor updates happening more than once per frame #655
Conversation
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.
A few comments, but other than that it looks good!
options: { | ||
stdout: true, | ||
failOnError: true | ||
failOnError: false |
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.
Are these changes to GruntFile a part of this pull request?
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.
You can leave these changes out as master already has this (it doesn't have -q
but that is just quiet mode).
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.
Done
@@ -23,6 +23,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). | |||
- `Actor.debugDraw` now works properly for child actors ([#505](https://github.com/excaliburjs/Excalibur/issues/505), [#645](https://github.com/excaliburjs/Excalibur/issues/645)) | |||
- Sprite culling was double scaling calculations ([#646](https://github.com/excaliburjs/Excalibur/issues/646)) | |||
- Fix negative zoom sprite culling ([#539](https://github.com/excaliburjs/Excalibur/issues/539)) | |||
- Fix Actor updates happening more than once per frame, causing multiple pointer events to trigger ([#643](https://github.com/excaliburjs/Excalibur/issues/643)) | |||
- Fix `Actor.on('pointerup')` capturePointer opt-in |
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.
What specifically is being fixed by the capturePointer change?
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.
Take a look at my newest update to see if this is clear enough
for (i = 0, len = this.children.length; i < len; i++) { | ||
// helps move settle collisions | ||
// todo there is a better way to do this | ||
this.children[i].integrate(collisionDelta * .001); |
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.
Want to document the .001 multiplier?
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.
I've extracted this into the ex.Physics global with some explaination
You snuck in some refactoring in physics--were those behavioral or organizational changes? Do they need to be documented? |
@kamranayub mostly organization changes, I'm not sure they need any more rigor than they have already. |
Closes #643
Pointer events were being triggered multiple times per frame. This was a result of the physics doing multiple passes on each actor by calling the update function multiple times.
Proposed Changes:
This fixes the issue by refactoring the integration step of actor update out of actor update, so that it can be called multiple times for collision passes and not affect other operations that expect update to be called once per frame.
Additionally, this uncovered a bug in Actor.on('pointerup') opt-in which was subsequently fixed.