fix issue with physics-collider

[InfiniteLee]

fix a crash that can occur if a physics-collider is colliding with an entity (e.g. dynamic-body) that is removed from the scene (see: #8)

[wmurphyrd]

Wow, good catch. Thanks!

I'm concerned with having the check in the loop condition - wouldn't that cause it to bail and not process any collisions after that one? I think we need to move the check inside the loop, ensuring that last line that increments the index is executed regardless.

Also, would you consider adding a test case for this situation to the suite? (looks like I need to address some changes in aframe-physics-system to get the tests back online first, though)

[InfiniteLee]

I've had no success trying to reproduce the issue in the test suite (sans this fix, of course), currently trying to use something like this:

suite('physics-collider-with-body', function () {
  setup(function (done) {
    this.scene = document.createElement('a-scene')
    document.body.appendChild(this.scene)
    this.scene.setAttribute('physics', '')

    this.target1 = document.createElement('a-sphere')
    this.target1.setAttribute('physics-collider', '')
    this.target1.setAttribute('static-body', '')
    this.target1.setAttribute('position', "0 1 0")
    this.scene.appendChild(this.target1)

    this.target2 = document.createElement('a-box')
    this.target2.setAttribute('static-body', '')
    this.target2.setAttribute('position', "0 0 0")
    this.scene.appendChild(this.target2)

    this.scene.addEventListener('loaded', () => {
      done()
    })
  })

  suite('lifecyle', function () {
    test('entity can be removed from scene without errors', function (done) {
      process.nextTick(() => {
        this.scene.removeChild(this.target2)
        process.nextTick(done)
      });
    })
  })
})

However, I can easily reproduce it using (almost) identical code in https://glitch.com/edit/#!/modern-harmonica?path=index.html:18:48

I admit though that I haven't used karma or mocha much before. Any ideas?

[wmurphyrd]

Thanks again, Lee! What was going on in your test approach, I believe, was that the handles for the entities in your testing context (e.g., this.target1) were preventing the objects form being garbage collected after removal from the DOM, so physics-collider was still able to reference them. I went with a unit approach over an integration approach to reproduce the issue.