Mutiple errors in Studio

[chrisklus]

@amanda-phet found several different errors that can occur when changing inputEnabledProperty for the soccerBallNodeGroup, inputEnabledProperty for individual soccer balls, and intermixing this with resetting the sim. I think it's related to the lack of un-multilinking in CAVObjectNode. I will try to find steps to reproduce them reliably and see if un-multilinking solves the problems.

[chrisklus]

I made progress on this tonight. The simple case of kick 5 -> reset -> kick 5 -> toggle inputEnabledProperty for the group (which errors) is fixed. I'm no longer getting any Assertion failed: should not be called if disposed errors. I'm still getting tried to removeListener on something that wasn’t a listener from some extensive combination (haven't figured out the simplest case yet) of kicking 5, resetting, toggling inputEnabledProperty for the group and for individual balls.

I think getting help from @samreid or @zepumph would be the best next step. Or advice/reminder on how to find out what non-listener is trying to be removed.

Here's a patch:

Index: js/common/view/CAVObjectNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVObjectNode.ts b/js/common/view/CAVObjectNode.ts
--- a/js/common/view/CAVObjectNode.ts	(revision b69d035fc8368cb1639e214fb5eefe1f13a10428)
+++ b/js/common/view/CAVObjectNode.ts	(date 1649214839267)
@@ -29,6 +29,7 @@
 import IProperty from '../../../../axon/js/IProperty.js';
 import BooleanProperty from '../../../../axon/js/BooleanProperty.js';
 import ballDark_png from '../../../images/ballDark_png.js';
+import Multilink from '../../../../axon/js/Multilink.js';
 
 type SelfOptions = {
   objectViewType?: CAVObjectType;
@@ -45,8 +46,11 @@
 let index = 0;
 
 class CAVObjectNode extends Node {
-  private dragListener: DragListener | null;
+  private readonly dragListener: DragListener | null;
   private readonly selfInputEnabledProperty: BooleanProperty | null;
+  private readonly inputEnabledMultilink: Multilink<[ AnimationMode, number | null, boolean, boolean ]> | null;
+  private readonly medianHighlightVisibleMultilink: Multilink<[ boolean, boolean, boolean ]>;
+  private readonly opacityMultilink: Multilink<[ number | null, AnimationMode ]>;
 
   constructor( casObject: CAVObject, isShowingPlayAreaMedianProperty: IReadOnlyProperty<boolean>,
                modelViewTransform: ModelViewTransform2, objectNodesInputEnabledProperty: IProperty<boolean>,
@@ -168,7 +172,7 @@
       // Prevent dragging or interaction while the object does not have a value (when it is not in the play area yet),
       // when it is animating, if input for this individual node is disabled, or if input for all of the object nodes
       // ahs been disabled
-      Property.multilink<[ AnimationMode, number | null, boolean, boolean ]>(
+      this.inputEnabledMultilink = Property.multilink<[ AnimationMode, number | null, boolean, boolean ]>(
         [ casObject.animationModeProperty, casObject.valueProperty, this.selfInputEnabledProperty, objectNodesInputEnabledProperty ],
         ( mode, value, selfInputEnabled, objectsInputEnabled ) => {
           const inputEnabled = value !== null && mode === AnimationMode.NONE && selfInputEnabled && objectsInputEnabled;
@@ -187,10 +191,12 @@
     else {
       this.dragListener = null;
       this.selfInputEnabledProperty = null;
+      this.inputEnabledMultilink = null;
     }
 
     // show or hide the median highlight
-    Property.multilink( [ casObject.isMedianObjectProperty, isShowingPlayAreaMedianProperty, casObject.isShowingAnimationHighlightProperty ],
+    this.medianHighlightVisibleMultilink = Property.multilink(
+      [ casObject.isMedianObjectProperty, isShowingPlayAreaMedianProperty, casObject.isShowingAnimationHighlightProperty ],
       ( isMedianObject, isShowingPlayAreaMedian, isShowingAnimationHighlight ) => {
         medianHighlight.visible = options.objectViewType === CAVObjectType.DOT ? isShowingAnimationHighlight :
                                   isShowingPlayAreaMedian && isMedianObject;
@@ -198,7 +204,7 @@
 
     // The initial ready-to-kick ball is full opacity. The rest of the balls waiting to be kicked are lower opacity so
     // they don't look like part of the data set, but still look kickable.
-    Property.multilink( [ casObject.valueProperty, casObject.animationModeProperty ],
+    this.opacityMultilink = Property.multilink( [ casObject.valueProperty, casObject.animationModeProperty ],
       ( value: number | null, animationMode: AnimationMode ) => {
         this.opacity = value === null && animationMode === AnimationMode.NONE && !casObject.isFirstObject ? 0.4 : 1;
       } );
@@ -214,8 +220,12 @@
   }
 
   dispose() {
+    Property.unmultilink( this.opacityMultilink );
+    Property.unmultilink( this.medianHighlightVisibleMultilink );
+    this.inputEnabledMultilink && Property.unmultilink( this.inputEnabledMultilink );
     this.selfInputEnabledProperty && this.selfInputEnabledProperty.dispose();
-    this.dragListener && this.dragListener.dispose();
+    this.dragListener && this.hasInputListener( this.dragListener ) && this.removeInputListener( this.dragListener );
+    this.dragListener && this.dragListener.dispose(); // TODO: is this needed?
     super.dispose();
   }
 }

[chrisklus]

@zepumph saved the day and found a problem in Studio, see https://github.com/phetsims/studio/issues/257. Thanks! We committed a temporary solution (https://github.com/phetsims/studio/commit/9af65d0bde5cbce63446e207ec164577adb48b79) that should be cherry-picked with rc.2. Marking this issue ready to cherry pick.

[samreid]

@chrisklus and I confirmed this is fixed in in #122, closing.