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

Convert Particles.changedCallbacks to use Emitter #216

Closed
samreid opened this issue Nov 4, 2017 · 4 comments
Closed

Convert Particles.changedCallbacks to use Emitter #216

samreid opened this issue Nov 4, 2017 · 4 comments
Assignees

Comments

@samreid
Copy link
Member

samreid commented Nov 4, 2017

Noted during #213.

Particles.js defines:

    this.changedCallbacks = []; // @protected called when the collection of particles changes

/// ...

    /**
     * Registers a callback that will be called when the collection of particles changes in some way
     * (eg, number of particles, particles move, ...)
     * @param {function} callback
     * @public
     */
    registerChangedCallback: function( callback ) {
      this.changedCallbacks.push( callback );
    }

Then Precipitate.js has this code:

    // @private Notify that the precipitate has changed.
    fireChanged: function() {
      var changedCallbacks = this.changedCallbacks.slice( 0 ); // copy to prevent concurrent modification
      for ( var i = 0; i < changedCallbacks.length; i++ ) {
        changedCallbacks[ i ]( this );
      }
    },

And ShakerParticles.js defines this code:

    // @private Notify that at least one particle was added, removed, or moved
    fireParticlesChanged: function() {
      var changedCallbacks = this.changedCallbacks.slice( 0 );
      for ( var i = 0; i < changedCallbacks.length; i++ ) {
        changedCallbacks[ i ]();
      }
    }

This code should be rewritten to use axon Emitter.

@pixelzoom
Copy link
Contributor

This sim implements its own listener mechanism because (1) it was ported directly from the Java version, and (2) the port predates the existence of Emitter. I question the value (and cost) of such as small amount of working/straightforward code. But I guess it's good to be consistent.

@pixelzoom
Copy link
Contributor

Same pattern noted in gravity-and-orbits and scenery, see above issues.

pixelzoom added a commit that referenced this issue Nov 6, 2017
@pixelzoom
Copy link
Contributor

Conversion to Emitter completed. Also required a change to TShakerParticles. Test in requirejs mode and with instance-proxies wrapper. Next savings was 11 lines of code.

@samreid please review.

@samreid
Copy link
Member Author

samreid commented Nov 6, 2017

The change set looks correct and I tested by saturating a solution then diluting it. Closing.

@samreid samreid closed this as completed Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants