phetsims / gas-properties

"Gas Properties" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 6 forks source link

Investigate why Properties need `hasListenerOrderDependencies: true` #186

Open zepumph opened 4 years ago

zepumph commented 4 years ago

From phetsims/axon#215, when running with ?shuffleListeners, we get this error: assigning to responsible dev to determine if it is worth fixing:

Uncaught Error: Assertion failed: heavyParticles has not been populated yet
Error: Assertion failed: heavyParticles has not been populated yet
    at window.assertions.assertFunction (http://localhost:8080/assert/js/assert.js:22:13)
    at http://localhost:8080/gas-properties/js/common/model/ParticleSystem.js:109:19
    at listener (http://localhost:8080/axon/js/DerivedProperty.js:73:35)
    at TinyEmitter.emit (http://localhost:8080/axon/js/TinyEmitter.js:69:53)
    at NumberProperty._notifyListeners (http://localhost:8080/axon/js/Property.js:273:25)
    at NumberProperty.set (http://localhost:8080/axon/js/Property.js:171:14)
    at NumberProperty.reset (http://localhost:8080/axon/js/Property.js:336:10)
    at NumberProperty.reset (http://localhost:8080/axon/js/NumberProperty.js:175:11)
    at ParticleSystem.removeAllParticles (http://localhost:8080/gas-properties/js/common/model/ParticleSystem.js:144:41)
    at ParticleSystem.reset (http://localhost:8080/gas-properties/js/common/model/ParticleSystem.js:136:10)
pixelzoom commented 4 years ago

To be considered when a new release branch is created for this sim. And my feeling is that this is incredibly low priority, and would not block a release.

pixelzoom commented 7 months ago

shuffleListeners was replace with listenerOrder, which is now run as a standard CT test. I haven't seen any listenerOrder errors in CT, so closing this issue.

phet-dev commented 7 months ago

Reopening because there is a TODO marked for this issue.

pixelzoom commented 7 months ago

It looks like the reason why this is no longer a problem in CT is that @zepumph added hasListenerOrderDependencies: true for 3 Properties. While that does silence the problem, it doesn't address the problem -- it's not clear to me why those 3 Properties need to have order depenendencies.

So... I'll change the title of this issue and leave it open to investigate whether those 3 Properties really need hasListenerOrderDependencies: true.

pixelzoom commented 3 months ago

The Properties with hasListenerOrderDependencies: true are:

I poked around a little, and I don't see anything obvious/easy that can be changed. So we will skip this for the 1.1 release #191, unless it causes PhET-iO problems.