Added REVIEW comments regarding listeners and emitters #29

phetsims · Dec 21, 2018 · 7fd0f47 · 7fd0f47
1 parent 2117249
commit 7fd0f47
Show file tree

Hide file tree

Showing 2 changed files with 3 additions and 0 deletions.
diff --git a/js/game/view/BuildingGameScreenView.js b/js/game/view/BuildingGameScreenView.js
@@ -207,6 +207,7 @@ define( require => {
           transition = this.mainTransitionNode.slideRightTo( this.levelSelectionLayer, QUADRATIC_TRANSITION_OPTIONS );
         }
         if ( oldChallengeNode ) {
+          // REVIEW: Do we not have to remove emitters from transitionNodes?
           transition.endedEmitter.addListener( () => {
             oldChallengeNode.wrapper.dispose();
             oldChallengeNode.dispose();
@@ -218,6 +219,7 @@ define( require => {
 
       const gameAudioPlayer = new GameAudioPlayer( model.soundEnabledProperty );
 
+      // REVIEW: Should a dispose call remove these listeners?
       model.allLevelsCompleteEmitter.addListener( () => gameAudioPlayer.gameOverPerfectScore() );
       model.singleLevelCompleteEmitter.addListener( () => gameAudioPlayer.challengeComplete() );
       model.collectedGroupEmitter.addListener( () => gameAudioPlayer.correctAnswer() );

diff --git a/js/intro/view/numberline/NumberLineNode.js b/js/intro/view/numberline/NumberLineNode.js
@@ -264,6 +264,7 @@ define( require => {
      * @override
      */
     dispose() {
+      // REVIEW: Would it be safer to dispose this.containerCountProperty?
       this.containerCountProperty.unlink( this.containerCountListener );
       this.minorTickMultilink.dispose();
       this.multipliedTickMultilink.dispose();