various memory leaks related to LocalizedStringProperty and ProfileCo…

…lorProperty, #153 (cherry picked from commit 6a59f6f)
phetsims · Apr 9, 2024 · 5ce4bfb · 5ce4bfb
1 parent 5c97e27
commit 5ce4bfb
Show file tree

Hide file tree

Showing 15 changed files with 81 additions and 31 deletions.
diff --git a/js/common/view/EquationAccordionBox.ts b/js/common/view/EquationAccordionBox.ts
@@ -29,6 +29,7 @@ export default class EquationAccordionBox extends AccordionBox {
                          savedLines: ObservableArray<Line>, expandedProperty: Property<boolean>, tandem: Tandem ) {
 
     const options: AccordionBoxOptions = {
+      isDisposable: false,
       titleNode: titleNode,
       expandedProperty: expandedProperty,
       fill: GLColors.controlPanelFillProperty,

diff --git a/js/common/view/GraphControlPanel.ts b/js/common/view/GraphControlPanel.ts
@@ -58,6 +58,7 @@ export default class GraphControlPanel extends Panel {
       includeStandardLines: true,
 
       // PanelOptions
+      isDisposable: false,
       fill: GLColors.controlPanelFillProperty,
       stroke: 'black',
       lineWidth: 1,

diff --git a/js/common/view/GraphNode.ts b/js/common/view/GraphNode.ts
@@ -52,6 +52,7 @@ const MINUS_SIGN_WIDTH = new Text( '-', { font: MAJOR_TICK_FONT } ).width;
 export default class GraphNode extends Node {
 
   private readonly gridNode: GridNode;
+  private readonly disposeGraphNode: () => void;
 
   public constructor( graph: Graph,
                       modelViewTransform: ModelViewTransform2,
@@ -62,16 +63,24 @@ export default class GraphNode extends Node {
     assert && assert( graph.contains( new Vector2( 0, 0 ) ) && graph.contains( new Vector2( 1, 1 ) ) );
 
     const gridNode = new GridNode( graph, modelViewTransform );
+    const xAxisNode = new XAxisNode( graph, modelViewTransform, xAxisLabelStringProperty );
+    const yAxisNode = new YAxisNode( graph, modelViewTransform, yAxisLabelStringProperty );
 
     super( {
-      children: [
-        gridNode,
-        new XAxisNode( graph, modelViewTransform, xAxisLabelStringProperty ),
-        new YAxisNode( graph, modelViewTransform, yAxisLabelStringProperty )
-      ]
+      children: [ gridNode, xAxisNode, yAxisNode ]
     } );
 
     this.gridNode = gridNode;
+
+    this.disposeGraphNode = () => {
+      xAxisNode.dispose();
+      yAxisNode.dispose();
+    };
+  }
+
+  public override dispose(): void {
+    this.disposeGraphNode();
+    super.dispose();
   }
 
   // Sets the visibility of the grid
@@ -176,6 +185,7 @@ class XAxisNode extends Node {
       labelText.left = lineNode.right + AXIS_LABEL_SPACING;
       labelText.centerY = lineNode.centerY;
     } );
+    this.disposeEmitter.addListener( () => labelText.dispose() );
 
     // ticks
     const numberOfTicks = graph.getWidth() + 1;
@@ -220,12 +230,13 @@ class YAxisNode extends Node {
     this.addChild( lineNode );
 
     // label at positive (top) end
-    const labelNode = new RichText( yAxisLabelStringProperty, { font: AXIS_LABEL_FONT, maxWidth: 30 } );
-    this.addChild( labelNode );
-    labelNode.boundsProperty.link( () => {
-      labelNode.centerX = lineNode.centerX;
-      labelNode.bottom = lineNode.top - AXIS_LABEL_SPACING;
+    const labelText = new RichText( yAxisLabelStringProperty, { font: AXIS_LABEL_FONT, maxWidth: 30 } );
+    this.addChild( labelText );
+    labelText.boundsProperty.link( () => {
+      labelText.centerX = lineNode.centerX;
+      labelText.bottom = lineNode.top - AXIS_LABEL_SPACING;
     } );
+    this.disposeEmitter.addListener( () => labelText.dispose() );
 
     // ticks
     const numberOfTicks = graph.getHeight() + 1;

diff --git a/js/common/view/PointToolBodyNode.ts b/js/common/view/PointToolBodyNode.ts
@@ -171,6 +171,7 @@ export default class PointToolBodyNode extends Node {
     } );
 
     this.disposePointToolBodyNode = () => {
+      bodyRectangle.dispose();
       coordinatesText.dispose();
     };
 

diff --git a/js/common/view/PointToolNode.ts b/js/common/view/PointToolNode.ts
@@ -125,6 +125,7 @@ export default class PointToolNode extends Node {
     this.disposePointToolNode = () => {
       updateMultilink.dispose();
       bodyNode.dispose();
+      probeNode.dispose();
       coordinatesProperty.dispose();
     };
   }

diff --git a/js/common/view/SlopeToolNode.ts b/js/common/view/SlopeToolNode.ts
@@ -97,6 +97,8 @@ export default class SlopeToolNode extends Node {
     this.disposeSlopeToolNode = () => {
       this.riseValueNode.dispose();
       this.runValueNode.dispose();
+      this.riseArrowNode.dispose();
+      this.runArrowNode.dispose();
       lineProperty.unlink( lineObserver );
     };
   }

diff --git a/js/linegame/view/ChallengeNode.ts b/js/linegame/view/ChallengeNode.ts
@@ -195,39 +195,45 @@ export default class ChallengeNode extends Node {
 
     // playStateProperty listener
     const playStateObserver = ( state: PlayState ) => {
-
-      // visibility of face
-      this.faceNode.visible = ( state === PlayState.TRY_AGAIN ||
-                                state === PlayState.SHOW_ANSWER ||
-                                ( state === PlayState.NEXT && challenge.isCorrect() ) );
-
-      // visibility of buttons
-      checkButton.visible = ( state === PlayState.FIRST_CHECK || state === PlayState.SECOND_CHECK );
-      tryAgainButton.visible = ( state === PlayState.TRY_AGAIN );
-      showAnswerButton.visible = ( state === PlayState.SHOW_ANSWER );
-      nextButton.visible = ( state === PlayState.NEXT );
-
-      // dev buttons
-      if ( replayButton && skipButton ) {
-        replayButton.visible = ( state === PlayState.NEXT );
-        skipButton.visible = !replayButton.visible;
+      if ( !this.isDisposed ) {
+
+        // visibility of face
+        this.faceNode.visible = ( state === PlayState.TRY_AGAIN ||
+                                  state === PlayState.SHOW_ANSWER ||
+                                  ( state === PlayState.NEXT && challenge.isCorrect() ) );
+
+        // visibility of buttons
+        checkButton.visible = ( state === PlayState.FIRST_CHECK || state === PlayState.SECOND_CHECK );
+        tryAgainButton.visible = ( state === PlayState.TRY_AGAIN );
+        showAnswerButton.visible = ( state === PlayState.SHOW_ANSWER );
+        nextButton.visible = ( state === PlayState.NEXT );
+
+        // dev buttons
+        if ( replayButton && skipButton ) {
+          replayButton.visible = ( state === PlayState.NEXT );
+          skipButton.visible = !replayButton.visible;
+        }
       }
     };
     model.playStateProperty.link( playStateObserver ); // unlink in dispose
 
     // Move from "Try Again" to "Check" state when the user changes their guess, see graphing-lines#47.
     const guessObserver = ( guess: Line | NotALine ) => {
-      if ( model.playStateProperty.value === PlayState.TRY_AGAIN ) {
+      if ( !this.isDisposed && model.playStateProperty.value === PlayState.TRY_AGAIN ) {
         model.playStateProperty.value = PlayState.SECOND_CHECK;
       }
     };
     challenge.guessProperty.link( guessObserver ); // unlink in dispose
 
     this.disposeChallengeNode = () => {
-      pointToolNode1.dispose();
-      pointToolNode2.dispose();
       model.playStateProperty.unlink( playStateObserver );
       challenge.guessProperty.unlink( guessObserver );
+      checkButton.dispose();
+      tryAgainButton.dispose();
+      showAnswerButton.dispose();
+      nextButton.dispose();
+      pointToolNode1.dispose();
+      pointToolNode2.dispose();
     };
   }
 

diff --git a/js/linegame/view/EquationBoxNode.ts b/js/linegame/view/EquationBoxNode.ts
@@ -27,6 +27,8 @@ export default class EquationBoxNode extends Node {
   private readonly correctIconNode: Path;
   private readonly incorrectIconNode: Path;
 
+  private readonly disposeEquationBoxNode: () => void;
+
   public constructor( titleStringProperty: TReadOnlyProperty<string>, titleColor: TColor, boxSize: Dimension2, equationNode: Node ) {
 
     super();
@@ -78,6 +80,15 @@ export default class EquationBoxNode extends Node {
     // icons are initially hidden
     this.correctIconNode.visible = false;
     this.incorrectIconNode.visible = false;
+
+    this.disposeEquationBoxNode = () => {
+      titleText.dispose();
+    };
+  }
+
+  public override dispose(): void {
+    this.disposeEquationBoxNode();
+    super.dispose();
   }
 
   // Sets the visibility of the correct icon (green check mark).

diff --git a/js/linegame/view/GraphTheLineNode.ts b/js/linegame/view/GraphTheLineNode.ts
@@ -164,6 +164,10 @@ export default class GraphTheLineNode extends ChallengeNode {
     model.playStateProperty.link( playStateObserver ); // unlink in dispose
 
     this.disposeGraphTheLineNode = () => {
+      titleText.dispose();
+      answerBoxNode.dispose();
+      guessBoxNode.dispose();
+      this.notALineText.dispose();
       challenge.guessProperty.unlink( guessObserver );
       model.playStateProperty.unlink( playStateObserver );
       guessEquationNode.dispose();

diff --git a/js/linegame/view/LineGameLevelSelectionButtonGroup.ts b/js/linegame/view/LineGameLevelSelectionButtonGroup.ts
@@ -65,6 +65,7 @@ export default class LineGameLevelSelectionButtonGroup extends LevelSelectionBut
     super( levelSelectionButtonItems, {
 
       // LevelSelectionButtonGroupOptions
+      isDisposable: false,
       levelSelectionButtonOptions: {
         baseColor: 'rgb( 180, 205, 255 )',
         iconToScoreDisplayYSpace: 5
@@ -107,6 +108,7 @@ function createLevelSelectionButtonIcon( level: number, imageProperty: Localized
 
   // 'Level N' centered above image
   return new VBox( {
+    isDisposable: false,
     spacing: 5,
     children: [ text, alignBox ]
   } );

diff --git a/js/linegame/view/MakeTheEquationNode.ts b/js/linegame/view/MakeTheEquationNode.ts
@@ -144,6 +144,9 @@ export default class MakeTheEquationNode extends ChallengeNode {
     model.playStateProperty.link( playStateObserver ); // unlink in dispose
 
     this.disposeMakeTheEquationNode = () => {
+      titleText.dispose();
+      answerBoxNode.dispose();
+      guessBoxNode.dispose();
       challenge.guessProperty.unlink( guessObserver );
       model.playStateProperty.unlink( playStateObserver );
       guessEquationNode.dispose();

diff --git a/js/linegame/view/PlayNode.ts b/js/linegame/view/PlayNode.ts
@@ -30,7 +30,9 @@ export default class PlayNode extends Node {
   public constructor( model: LineGameModel, layoutBounds: Bounds2, visibleBoundsProperty: TReadOnlyProperty<Bounds2>,
                       audioPlayer: GameAudioPlayer ) {
 
-    super();
+    super( {
+      isDisposable: false
+    } );
 
     const statusBar = new FiniteStatusBar( layoutBounds, visibleBoundsProperty, model.scoreProperty, {
       createScoreDisplay: scoreProperty => new ScoreDisplayLabeledNumber( scoreProperty, {

diff --git a/js/linegame/view/ResultsNode.ts b/js/linegame/view/ResultsNode.ts
@@ -26,7 +26,9 @@ export default class ResultsNode extends Node {
 
   public constructor( model: LineGameModel, layoutBounds: Bounds2, audioPlayer: GameAudioPlayer, rewardNodeFunctions: RewardNodeFunction[] ) {
 
-    super();
+    super( {
+      isDisposable: false
+    } );
 
     this.rewardNode = null;
 

diff --git a/js/linegame/view/SettingsNode.ts b/js/linegame/view/SettingsNode.ts
@@ -71,6 +71,7 @@ export default class SettingsNode extends Node {
     } );
 
     super( {
+      isDisposable: false,
       children: [ vBox, timerToggleButton, resetAllButton ],
       tandem: tandem
     } );

diff --git a/js/pointslope/view/PointSlopeEquationNode.ts b/js/pointslope/view/PointSlopeEquationNode.ts
@@ -525,6 +525,8 @@ export default class PointSlopeEquationNode extends EquationNode {
     this.mutate( options );
 
     this.disposePointSlopeEquationNode = () => {
+      xText.dispose();
+      yText.dispose();
       x1Node.dispose();
       y1Node.dispose();
       slopeUndefinedText.dispose();