phetsims / circuit-construction-kit-common

"Circuit Construction Kit: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
10 stars 10 forks source link

Batteries not always disposed from tree on reset all #832

Closed arouinfar closed 2 years ago

arouinfar commented 2 years ago

Steps to reproduce:

  1. Open sim in Studio on master.
  2. Drag a battery out of the carousel.
  3. Use ctrl or opt to select the associated element in the tree.
  4. Use the ResetAllButton and the battery will still be selected in the tree, despite it not existing anymore.

This set of errors will appear in the console a few times:

Uncaught Error: Assertion failed: All descendants must be disposed by the next frame: circuitConstructionKitDc.introScreen.model.circuit.batteryGroup.battery_0.powerGeneratedProperty
    at window.assertions.assertFunction (assert.js:25:13)
    at /dev/phettest/chipper/dist/js/tandem/js/PhetioObject.js:539:19
    at Array.forEach (<anonymous>)
    at /dev/phettest/chipper/dist/js/tandem/js/PhetioObject.js:538:19
    at callback (/dev/phettest/chipper/dist/js/axon/js/Timer.js:30:11)
    at Timer.emit (/dev/phettest/chipper/dist/js/axon/js/TinyEmitter.js:69:9)
    at Sim.runAnimationLoop (/dev/phettest/chipper/dist/js/joist/js/Sim.js:796:25)
assert.js:21 Assertion failed: All descendants must be disposed by the next frame: circuitConstructionKitDc.introScreen.model.circuit.batteryGroup.battery_0.powerGeneratedProperty
arouinfar commented 2 years ago

Note that this also happens when disposing of a battery in another way, such as returning to the carousel.

samreid commented 2 years ago

There was a missing dispose call. It seems to be working on my side after that change. Can you please test after that commit?

arouinfar commented 2 years ago

Looks good, thanks!