Multiple disposal issue

[phet-steele]

From the checks in phetsims/scenery/issues/601:

Uncaught Error: Assertion failed: This Node has already been disposed, and cannot be disposed again
    at window.assertions.assertFunction (assert.js:21)
    at FlagNode.Node.dispose (Node.js?bust=1488307460572:503)
    at NetForceScreenView.js?bust=1488307460572:338
    at Array.listener (Multilink.js?bust=1488307460572:40)
    at Emitter.emit2 (Emitter.js?bust=1488307460572:147)
    at Property._notifyObservers (Property.js?bust=1488307460572:176)
    at Property._setAndNotifyObservers (Property.js?bust=1488307460572:165)
    at Property.set (Property.js?bust=1488307460572:133)
    at NetForceModel.set [as state] (PropertySet.js?bust=1488307460572:124)
    at NetForceModel.returnCart (NetForceModel.js?bust=1488307460572:350)

To reproduce:

1. 1st screen, let a team win.
2. Press "Return".

@samreid @kathy-phet note that this severely affects the classroom activity wrapper for this sim (should go without saying).

[samreid]

@phet-steele is the bug present in the last published version:

https://phet-io.colorado.edu/sims/forces-and-motion-basics/2.2.6-phetio/wrappers/classroom-activity/classroom-activity.html?wrapper=classroom-activity&sim=forces-and-motion-basics&validationRule=crossCheckID&validationPrefix=WE16-&publisher_id=d332cd2f&application_id=8b5d5bf5cbb578c34e71759b2bd3ce4b&widget_id=forces-and-motion-test&phet-io.emitInputEvents=true&learner_id=phettest

[phet-steele]

@phet-steele is the bug present in the last published version:

No, and I would imagine you would need to run with ?ea to see the issue. To clarify it it wasn't already known, this issue was found on master in require js.

[jessegreenberg]

I ran into this while taking a look at https://github.com/phetsims/forces-and-motion-basics/issues/220.

Dispose is registered to events in FlagNode with

    model.once( 'reset-all', this.dispose.bind( this ) );
    model.once( 'cart-returned', this.dispose.bind( this ) );

When one is triggered, the other isn't removed in dispose, so if you press "Return" and then "Reset All", and the flag will be disposed multiple times.

Also, dispose can be called in NetForceScreenView whenever the cart moves, and whenever the cart model stateProperty changes.

    Property.multilink( [ model.stateProperty, model.cart.xProperty ], function( state, x ) {
      flagNode && flagNode.dispose();
      if ( state === 'completed' && Math.abs( x ) > 1E-6 ) {
        showFlagNode();
      }
    } );

After a winning state, "Reset All" and "Return" buttons (which already trigger dispose) are the only way to change these Properties, so I am not sure if this disposal is necessary.

[jessegreenberg]

I can no longer reproduce this issue, but want to wait for verification in the phet-io issue before closing.

[zepumph]

I just got it again today:

SimIFrameAPI Invocation Error: Error: Assertion failed: Cannot register two different instances to the same id: forcesAndMotionBasics.netForceScreen.view.flagNode
    at window.assertions.assertFunction (assert.js:21)
    at Object.addInstance (phetio.js?bust=1498414858945:417)
    at Object.addInstance (phetio.js?bust=1498414858945:57)
    at Tandem.addInstance (Tandem.js?bust=1498414858945:127)
    at FlagNode.setTandem (Node.js?bust=1498414858945:3481)
    at FlagNode.set tandem [as tandem] (Node.js?bust=1498414858945:3486)
    at Node.js?bust=1498414858945:4777
    at u (lodash-4.17.4.min.js?bust=1498414858945:5)
    at Function.ru (lodash-4.17.4.min.js?bust=1498414858945:67)
    at FlagNode.mutate (Node.js?bust=1498414858945:4764)

[samreid]

I can get this problem by running instance proxies and winning tug of war once:

Cannot register two different instances to the same id: 
forcesAndMotionBasics.netForceScreen.view.flagNode

[samreid]

Proposed fix is above, and testing in instance proxies seems OK (no more "cannot register two") errors that I can see, and the flag still seems to be displayed when it should be. @jessegreenberg can you please review and test?

[jessegreenberg]

The above looks great @samreid, I tested in PhET brand and the instance proxies wrapper, its working great. Closing.