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

Actor updates are being called more than once per frame #643

Closed
kamranayub opened this issue Sep 1, 2016 · 3 comments · Fixed by #655
Closed

Actor updates are being called more than once per frame #643

kamranayub opened this issue Sep 1, 2016 · 3 comments · Fixed by #655
Assignees
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior
Milestone

Comments

@kamranayub
Copy link
Member

kamranayub commented Sep 1, 2016

Context

I noticed while doing some audio testing (/tests/audio) that when I clicked the red square, the sound actually ended up playing 5 times all at once.

When I stepped through, it's because when we capture pointer events, each browser pointer event is captured in an array during a frame (the array is cleared at the end of a frame).

So, even a single "click" can generate n pointer events during a single frame.

Is this expected? It means people need to have guards in their event handlers to compensate for being called multiple times in a single frame.

Keep in mind: there could be a case where if you are using multi-touch you could trigger pointer events in multiple locations on the same frame--this is where it may make sense to keep an array.

  var i: number = 0, 
        len: number = this._pointerUp.length;

         for (i; i < len; i++) {
            if (actor.contains(this._pointerUp[i].x, this._pointerUp[i].y, !isUIActor)) {
               actor.eventDispatcher.emit('pointerup', this._pointerUp[i]);
            }
         }

         i = 0;
         len = this._pointerDown.length;

         for (i; i < len; i++) {
            if (actor.contains(this._pointerDown[i].x, this._pointerDown[i].y, !isUIActor)) {
               actor.eventDispatcher.emit('pointerdown', this._pointerDown[i]);
            }
         }

Proposal

I propose only emitting a single pointerup, pointercancel and pointerdown event per actor during a frame (first valid pointer event wins). I would leave pointermove to allow multiple per frame.

This means, the logic would go:

  • For each pointer event in array:
    • If actor contains pointer coords trigger event and break out of loop
@kamranayub kamranayub added this to the 0.7.1 Release milestone Sep 1, 2016
@kamranayub
Copy link
Member Author

@excaliburjs/core-contributers thoughts? I will mark as a bug if you think it makes sense.

@eonarheim
Copy link
Member

Seems like a bug, go for it

On Wed, Aug 31, 2016, 20:25 Kamran Ayub notifications@github.com wrote:

@excaliburjs/core-contributers
https://github.com/orgs/excaliburjs/teams/core-contributers thoughts? I
will mark as a bug if you think it makes sense.


You are receiving this because you are on a team that was mentioned.

Reply to this email directly, view it on GitHub
#643 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAlW5_xnBtx6_dFUT6fmz-G8dB4Fm-15ks5qlkWfgaJpZM4JyR16
.

@kamranayub kamranayub added the bug This issue describes undesirable, incorrect, or unexpected behavior label Sep 4, 2016
@kamranayub
Copy link
Member Author

Found it:

while (iter > 0) {
                // Cycle through actors updating actors
                for (i = 0, len = this.children.length; i < len; i++) {
                    this.children[i].update(engine, collisionDelta);
                    this.children[i].collisionArea.recalc();
                }
                // TODO meh I don't like how this works... maybe find a way to make collisions
                // a trait
                // Run collision resolution strategy
                if (this._broadphase && ex.Physics.enabled) {
                    this._broadphase.update(this.children, collisionDelta);
                    this._broadphase.findCollisionContacts(this.children, collisionDelta);
                }
                iter--;
            }

It is running actor update multiple times per frame, thus multiple trait updates.

@eonarheim eonarheim self-assigned this Sep 11, 2016
@kamranayub kamranayub changed the title Multiple pointer events can occur during a single frame Actor updates are being called more than once per frame Sep 11, 2016
eonarheim added a commit that referenced this issue Oct 4, 2016
<!-- If you're closing an issue with this pull request, or contributing a significant change, please include your changes in the appropriate section of CHANGELOG.md as outlined in CONTRIBUTING.md-->

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants