Why does Events.off need to return the index?

[zepumph]

@samreid And I couldn't find any usages of the return value from off or offStatic. I think it would be good to double check with @jonathanolson before getting rid of it though.

[zepumph]

I used (return|=){1} (\w*\.){0,3}off(Static){0,1}\( to search for usages.

[samreid]

There is some usage here:

https://github.com/phetsims/scenery/blob/0eaff198894fa9470c47d86dca082aa97f0b70c9/js/nodes/Node.js#L5121-L5130

    offStatic: function offStaticOverride( eventName, listener ) {
      var index = Events.prototype.offStatic.call( this, eventName, listener );

      // Throw an error when removing a non-listener (except when the Node has already been disposed)
      if ( assert && !this.isDisposed ) {
        assert && assert( index >= 0, 'Node.offStatic was called but no listener was removed' );
      }
      this.onEventListenerRemoved( eventName, listener );
      return index;
    },

[samreid]

But in that case it would be equally good to return true or false, or maybe to have Events do its own assertion. @jonathanolson what do you think?

[jonathanolson]

I'm fine with that feature being removed/changed, I don't remember using it. For Scenery, having strict error checking (fail on trying to remove a non-listener) would be nice.

[samreid]

Proposed fix is here. I had to put a band-aid on one call site to prevent erroring. When I moved the error checking directly to Events.js, fuzz testing revealed a problem in many (but not all sims) that involved removing listeners from TransformTracker.js. For now, I adopted the prior behavior of not checking those listeners. But based on #199 (comment) I'm wondering if this is a latent bug, or if it should remain unchecked. @jonathanolson can you take a look?

[samreid]

Events was deleted in phetsims/scenery#490, closing.