Remove everGotGamepadEvent flag and gamepadconnected / gamepaddisconnected (#3189)

handlers (fix #2769)

We moved away from using gamepadconnected / gamepaddisconnected events
from the rest of the controls. We forgot to remove the handlers from
daydream-controls. This was causing problems where sometimes gamepadconnected was
fired before tracked-controls had a chance to update controllers and emit controllersupdated. The
flag everGotGamepadEvent was set to true leaving onControllersUpdate
without effect after tracked-controls had updated the controllers list.
The timing of the gamepadconnected event seems to have changed in Chrome
Daydream and this problem had not manifested before.

Author: dmarcos

src/components/daydream-controls.js

@@ -45,7 +45,6 @@ module.exports.Component = registerComponent('daydream-controls', {
     this.checkIfControllerPresent = bind(this.checkIfControllerPresent, this);
     this.removeControllersUpdateListener = bind(this.removeControllersUpdateListener, this);
     this.onAxisMoved = bind(this.onAxisMoved, this);
-    this.onGamepadConnectionEvent = bind(this.onGamepadConnectionEvent, this);
   },
 
   init: function () {
@@ -58,7 +57,6 @@ module.exports.Component = registerComponent('daydream-controls', {
     this.onButtonTouchEnd = function (evt) { onButtonEvent(evt.detail.id, 'touchend', self); };
     this.onAxisMoved = bind(this.onAxisMoved, this);
     this.controllerPresent = false;
-    this.everGotGamepadEvent = false;
     this.lastControllerCheck = 0;
     this.bindMethods();
     this.checkControllerPresentAndSetup = checkControllerPresentAndSetup;  // To allow mock.
@@ -93,27 +91,14 @@ module.exports.Component = registerComponent('daydream-controls', {
     this.checkControllerPresentAndSetup(this, GAMEPAD_ID_PREFIX, {hand: this.data.hand});
   },
 
-  onGamepadConnectionEvent: function (evt) {
-    this.everGotGamepadEvent = true;
-    // Due to an apparent bug in FF Nightly
-    // where only one gamepadconnected / disconnected event is fired,
-    // which makes it difficult to handle in individual controller entities,
-    // we no longer remove the controllersupdate listener as a result.
-    this.checkIfControllerPresent();
-  },
-
   play: function () {
     this.checkIfControllerPresent();
     this.addControllersUpdateListener();
-    window.addEventListener('gamepadconnected', this.onGamepadConnectionEvent, false);
-    window.addEventListener('gamepaddisconnected', this.onGamepadConnectionEvent, false);
   },
 
   pause: function () {
     this.removeEventListeners();
     this.removeControllersUpdateListener();
-    window.removeEventListener('gamepadconnected', this.onGamepadConnectionEvent, false);
-    window.removeEventListener('gamepaddisconnected', this.onGamepadConnectionEvent, false);
   },
 
   injectTrackedControls: function () {
@@ -141,7 +126,7 @@ module.exports.Component = registerComponent('daydream-controls', {
   },
 
   onControllersUpdate: function () {
-    if (!this.everGotGamepadEvent) { this.checkIfControllerPresent(); }
+    this.checkIfControllerPresent();
   },
 
   onModelLoaded: function (evt) {

src/components/gearvr-controls.js

@@ -56,7 +56,6 @@ module.exports.Component = registerComponent('gearvr-controls', {
     this.onButtonTouchEnd = function (evt) { onButtonEvent(evt.detail.id, 'touchend', self); };
     this.onAxisMoved = bind(this.onAxisMoved, this);
     this.controllerPresent = false;
-    this.everGotGamepadEvent = false;
     this.lastControllerCheck = 0;
     this.bindMethods();
     this.checkControllerPresentAndSetup = checkControllerPresentAndSetup;  // To allow mock.

tests/components/daydream-controls.test.js

@@ -1,4 +1,4 @@
-/* global assert, process, setup, suite, test, Event */
+/* global assert, process, setup, suite, test */
 var entityFactory = require('../helpers').entityFactory;
 
 suite('daydream-controls', function () {
@@ -177,34 +177,6 @@ suite('daydream-controls', function () {
     });
   });
 
-  suite('gamepaddisconnected', function () {
-    /**
-     * Due to an apparent bug in FF Nightly
-     * where only one gamepadconnected / disconnected event is fired,
-     * which makes it difficult to handle in individual controller entities,
-     * we no longer remove the controllersupdate listener as a result.
-     */
-    test('checks present on gamepaddisconnected', function () {
-      var el = this.el;
-      var component = el.components['daydream-controls'];
-      var checkIfControllerPresentSpy = this.sinon.spy(component, 'checkIfControllerPresent');
-      // Because checkIfControllerPresent may be used in bound form, bind and reinstall.
-      component.checkIfControllerPresent = component.checkIfControllerPresent.bind(component);
-      component.pause();
-      component.play();
-
-      el.sceneEl.systems['tracked-controls'].controllers = [];
-
-      // Reset everGotGamepadEvent so we don't think we've looked before.
-      delete component.everGotGamepadEvent;
-
-      // Fire emulated gamepaddisconnected event.
-      window.dispatchEvent(new Event('gamepaddisconnected'));
-
-      assert.ok(checkIfControllerPresentSpy.called);
-    });
-  });
-
   suite('armModel', function () {
     function makePresent (el) {
       var component = el.components['daydream-controls'];

tests/components/gearvr-controls.test.js

@@ -1,4 +1,4 @@
-/* global assert, process, setup, suite, test, Event */
+/* global assert, process, setup, suite, test */
 var entityFactory = require('../helpers').entityFactory;
 
 suite('gearvr-controls', function () {
@@ -191,31 +191,6 @@ suite('gearvr-controls', function () {
     });
   });
 
-  suite('gamepaddisconnected', function () {
-    /**
-     * Due to an apparent bug in FF Nightly
-     * where only one gamepadconnected / disconnected event is fired,
-     * which makes it difficult to handle in individual controller entities,
-     * we no longer remove the controllersupdate listener as a result.
-     */
-    test('check present on gamepaddisconnected', function () {
-      var el = this.el;
-      var component = el.components['gearvr-controls'];
-      var checkIfControllerPresentSpy = this.sinon.spy(component, 'checkIfControllerPresent');
-      // Because checkIfControllerPresent may be used in bound form, bind and reinstall.
-      component.checkIfControllerPresent = component.checkIfControllerPresent.bind(component);
-      component.pause();
-      component.play();
-
-      el.sceneEl.systems['tracked-controls'].controllers = [];
-      // Reset everGotGamepadEvent so we don't think we've looked before.
-      delete component.everGotGamepadEvent;
-      // Fire emulated gamepaddisconnected event.
-      window.dispatchEvent(new Event('gamepaddisconnected'));
-      assert.ok(checkIfControllerPresentSpy.called);
-    });
-  });
-
   suite('armModel', function () {
     function makePresent (el) {
       var component = el.components['gearvr-controls'];

tests/components/oculus-touch-controls.test.js

@@ -1,4 +1,4 @@
-/* global assert, process, setup, suite, test, Event */
+/* global assert, process, setup, suite, test */
 var entityFactory = require('../helpers').entityFactory;
 
 suite('oculus-touch-controls', function () {
@@ -149,26 +149,4 @@ suite('oculus-touch-controls', function () {
       el.emit('buttonchanged', {id: 1, state: {value: 0.5, pressed: true, touched: true}});
     });
   });
-
-  suite('gamepaddisconnected', function () {
-    /**
-     * In FF Nightly, only one gamepadconnected/disconnected event is fired,
-     * which makes it difficult to handle in individual controller entities.
-     * We no longer remove the controllersupdate listener as a result.
-     */
-    test('checks if present on gamepaddisconnected event', function () {
-      var checkIfControllerPresentSpy = this.sinon.spy(component, 'checkIfControllerPresent');
-      // Because checkIfControllerPresent may be used in bound form, bind and reinstall.
-      component.checkIfControllerPresent = component.checkIfControllerPresent.bind(component);
-      component.pause();
-      component.play();
-      // Mock isControllerPresent to return false.
-      // Reset everGotGamepadEvent so we don't think we've looked before.
-      delete component.everGotGamepadEvent;
-      // Fire emulated gamepaddisconnected event.
-      window.dispatchEvent(new Event('gamepaddisconnected'));
-
-      assert.ok(checkIfControllerPresentSpy.called);
-    });
-  });
 });

tests/components/vive-controls.test.js

@@ -1,4 +1,4 @@
-/* global assert, Event, process, setup, suite, test, THREE */
+/* global assert, process, setup, suite, test, THREE */
 var entityFactory = require('../helpers').entityFactory;
 
 suite('vive-controls', function () {
@@ -177,28 +177,6 @@ suite('vive-controls', function () {
     });
   });
 
-  suite('gamepaddisconnected', function () {
-    /*
-      Apparent bug in FF Nightly where only one gamepadconnected/disconnected event is
-      fired which makes it difficult to handle in individual controller entities. We no
-      longer remove the controllersupdate listener as a result.
-    */
-    test('checks if controller present on gamepaddisconnected', function () {
-      var checkIfControllerPresentSpy = this.sinon.spy(component, 'checkIfControllerPresent');
-      // Because checkIfControllerPresent may be used in bound form, bind and reinstall.
-      component.checkIfControllerPresent = component.checkIfControllerPresent.bind(component);
-      component.pause();
-      component.play();
-
-      controlsSystem.controllers = [];
-      // Reset everGotGamepadEvent to mock not looked before.
-      delete component.everGotGamepadEvent;
-      // Fire emulated gamepaddisconnected event.
-      window.dispatchEvent(new Event('gamepaddisconnected'));
-      assert.ok(checkIfControllerPresentSpy.called);
-    });
-  });
-
   suite('model', function () {
     test.skip('loads', function (done) {
       component.addEventListeners();

tests/components/windows-motion-controls.test.js

@@ -1,4 +1,4 @@
-/* global assert, process, setup, suite, test, THREE, Event */
+/* global assert, process, setup, suite, test, THREE */
 var entityFactory = require('../helpers').entityFactory;
 
 suite('windows-motion-controls', function () {
@@ -521,26 +521,6 @@ suite('windows-motion-controls', function () {
     }
   });
 
-  suite('gamepaddisconnected', function () {
-    test('checks if present on gamepaddisconnected event', function (done) {
-      var checkIfControllerPresentSpy = this.sinon.spy(component, 'checkIfControllerPresent');
-      // Because checkIfControllerPresent may be used in bound form, using this.bind,
-      // we need to re-create the binding and re-attach the event listeners for our spy to work
-      component.checkIfControllerPresent = component.checkIfControllerPresent.bind(component);
-      // Pause and resume to remove and re-attach the event listeners, with our spy.
-      component.pause();
-      component.play();
-      // Reset everGotGamepadEvent so we don't think we've looked before.
-      delete component.everGotGamepadEvent;
-      // Fire emulated gamepaddisconnected event.
-      window.dispatchEvent(new Event('gamepaddisconnected'));
-
-      assert.ok(checkIfControllerPresentSpy.called, 'checkIfControllerPresent not called in response to gamepaddisconnected');
-
-      setTimeout(() => { done(); });
-    });
-  });
-
   suite('setModelVisibility', function () {
     test('shows model', function () {
       var component = el.components['windows-motion-controls'];