Remove SoccerBall.isActiveProperty (use phase instead), see #188

phetsims · Jun 2, 2023 · 98d1c7a · 98d1c7a
1 parent 3f7b831
commit 98d1c7a
Show file tree

Hide file tree

Showing 10 changed files with 89 additions and 99 deletions.
diff --git a/js/common/model/AnimationMode.ts b/js/common/model/AnimationMode.ts
diff --git a/js/common/model/CAVSceneModel.ts b/js/common/model/CAVSceneModel.ts
@@ -29,7 +29,7 @@ import CAVConstants from '../CAVConstants.js';
 import Animation from '../../../../twixt/js/Animation.js';
 import Easing from '../../../../twixt/js/Easing.js';
 import Pose from './Pose.js';
-import { AnimationMode } from './AnimationMode.js';
+import { SoccerBallPhase } from './SoccerBallPhase.js';
 import BooleanProperty from '../../../../axon/js/BooleanProperty.js';
 import PhetioObject, { PhetioObjectOptions } from '../../../../tandem/js/PhetioObject.js';
 import Multilink from '../../../../axon/js/Multilink.js';
@@ -198,10 +198,10 @@ export default class CAVSceneModel extends PhetioObject implements TModel {
 
     this.soccerBalls.forEach( soccerBall => {
       soccerBall.soccerBallPhaseProperty.link( soccerBallPhase => {
-        if ( soccerBallPhase === AnimationMode.STACKED ) {
+        if ( soccerBallPhase === SoccerBallPhase.STACKED ) {
           this.stackedSoccerBallCountProperty.value =
             this.getActiveSoccerBalls().filter( soccerBall =>
-              soccerBall.soccerBallPhaseProperty.value === AnimationMode.STACKED ).length;
+              soccerBall.soccerBallPhaseProperty.value === SoccerBallPhase.STACKED ).length;
         }
       } );
     } );
@@ -245,9 +245,9 @@ export default class CAVSceneModel extends PhetioObject implements TModel {
       ...this.soccerBalls.map( soccerBall => soccerBall.soccerBallPhaseProperty ) ], () => {
 
       const kickedSoccerBalls = this.getActiveSoccerBalls().filter(
-        soccerBall => soccerBall.soccerBallPhaseProperty.value === AnimationMode.FLYING ||
-                      soccerBall.soccerBallPhaseProperty.value === AnimationMode.STACKING ||
-                      soccerBall.soccerBallPhaseProperty.value === AnimationMode.STACKED
+        soccerBall => soccerBall.soccerBallPhaseProperty.value === SoccerBallPhase.FLYING ||
+                      soccerBall.soccerBallPhaseProperty.value === SoccerBallPhase.STACKING ||
+                      soccerBall.soccerBallPhaseProperty.value === SoccerBallPhase.STACKED
       );
       const value = this.maxKicksProperty.value - kickedSoccerBalls.length - this.numberOfScheduledSoccerBallsToKickProperty.value;
 
@@ -390,7 +390,7 @@ export default class CAVSceneModel extends PhetioObject implements TModel {
 
   public getSortedStackedObjects(): SoccerBall[] {
     return _.sortBy( this.getActiveSoccerBalls().filter( soccerBall =>
-        soccerBall.soccerBallPhaseProperty.value === AnimationMode.STACKED ),
+        soccerBall.soccerBallPhaseProperty.value === SoccerBallPhase.STACKED ),
 
       // The numerical value takes precedence for sorting
       soccerBall => soccerBall.valueProperty.value,
@@ -436,8 +436,7 @@ export default class CAVSceneModel extends PhetioObject implements TModel {
 
           const soccerBall = this.soccerBalls.find( soccerBall =>
             soccerBall.valueProperty.value === null &&
-            soccerBall.isActiveProperty.value &&
-            soccerBall.soccerBallPhaseProperty.value === AnimationMode.READY
+            soccerBall.soccerBallPhaseProperty.value === SoccerBallPhase.READY
           );
 
           // In fuzzing, sometimes there are no soccer balls available
@@ -484,16 +483,15 @@ export default class CAVSceneModel extends PhetioObject implements TModel {
         nextIndex = 0;
       }
       this.activeKickerIndexProperty.value = nextIndex;
-      const nextBallFromPool = this.soccerBalls.find( ball => !ball.isActiveProperty.value ) || null;
+      const nextBallFromPool = this.soccerBalls.find( ball => ball.soccerBallPhaseProperty.value === SoccerBallPhase.INACTIVE ) || null;
       if ( nextBallFromPool && this.soccerBalls.indexOf( nextBallFromPool ) < this.maxKicksProperty.value ) {
-        nextBallFromPool.soccerBallPhaseProperty.value = AnimationMode.READY;
+        nextBallFromPool.soccerBallPhaseProperty.value = SoccerBallPhase.READY;
       }
-
     }
   }
 
   public getActiveSoccerBalls(): SoccerBall[] {
-    return this.soccerBalls.filter( soccerBall => soccerBall.isActiveProperty.value );
+    return this.soccerBalls.filter( soccerBall => soccerBall.soccerBallPhaseProperty.value !== SoccerBallPhase.INACTIVE );
   }
 
   /**
@@ -515,12 +513,12 @@ export default class CAVSceneModel extends PhetioObject implements TModel {
     soccerBall.clearAnimation();
 
     if ( otherObjectsInStack.length === 0 ) {
-      soccerBall.soccerBallPhaseProperty.value = AnimationMode.STACKED;
+      soccerBall.soccerBallPhaseProperty.value = SoccerBallPhase.STACKED;
       this.stackChangedEmitter.emit( [ soccerBall ] );
       this.objectChangedEmitter.emit();
     }
     else {
-      soccerBall.soccerBallPhaseProperty.value = AnimationMode.STACKING;
+      soccerBall.soccerBallPhaseProperty.value = SoccerBallPhase.STACKING;
       soccerBall.animation = new Animation( {
         duration: animationTime,
         targets: [ {
@@ -532,7 +530,7 @@ export default class CAVSceneModel extends PhetioObject implements TModel {
 
       soccerBall.animation.endedEmitter.addListener( () => {
         soccerBall.animation = null;
-        soccerBall.soccerBallPhaseProperty.value = AnimationMode.STACKED;
+        soccerBall.soccerBallPhaseProperty.value = SoccerBallPhase.STACKED;
 
         this.objectChangedEmitter.emit();
 
@@ -541,7 +539,7 @@ export default class CAVSceneModel extends PhetioObject implements TModel {
 
           // Identify the soccer balls in the stack at the time the animation ended
           this.stackChangedEmitter.emit( this.getActiveSoccerBalls().filter( x =>
-            x.valueProperty.value === value && x.soccerBallPhaseProperty.value === AnimationMode.STACKED ) );
+            x.valueProperty.value === value && x.soccerBallPhaseProperty.value === SoccerBallPhase.STACKED ) );
         }
       } );
       soccerBall.animation.start();
@@ -574,7 +572,7 @@ export default class CAVSceneModel extends PhetioObject implements TModel {
 
     soccerBall.targetXProperty.value = x1;
 
-    soccerBall.soccerBallPhaseProperty.value = AnimationMode.FLYING;
+    soccerBall.soccerBallPhaseProperty.value = SoccerBallPhase.FLYING;
     this.timeWhenLastBallWasKickedProperty.value = this.timeProperty.value;
 
     soccerBall.soccerPlayer = soccerPlayer;

diff --git a/js/common/model/SoccerBall.ts b/js/common/model/SoccerBall.ts
@@ -19,14 +19,11 @@ import CAVConstants from '../CAVConstants.js';
 import Property from '../../../../axon/js/Property.js';
 import NullableIO from '../../../../tandem/js/types/NullableIO.js';
 import Emitter from '../../../../axon/js/Emitter.js';
-import { AnimationMode } from './AnimationMode.js';
+import { SoccerBallPhase } from './SoccerBallPhase.js';
 import EnumerationProperty from '../../../../axon/js/EnumerationProperty.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
 import TEmitter from '../../../../axon/js/TEmitter.js';
 import SoccerPlayer from './SoccerPlayer.js';
-import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
-import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
-import BooleanIO from '../../../../tandem/js/types/BooleanIO.js';
 import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js';
 import NumberTone from './NumberTone.js';
 
@@ -46,7 +43,7 @@ export default class SoccerBall extends PhetioObject {
   // Continuous position during animation. After landing, it's discrete.
   public readonly positionProperty: Vector2Property;
   public readonly velocityProperty: Vector2Property;
-  public readonly soccerBallPhaseProperty: Property<AnimationMode>;
+  public readonly soccerBallPhaseProperty: Property<SoccerBallPhase>;
   public readonly isMedianObjectProperty: BooleanProperty;
   public readonly isQ1ObjectProperty: BooleanProperty;
   public readonly isQ3ObjectProperty: BooleanProperty;
@@ -62,7 +59,6 @@ export default class SoccerBall extends PhetioObject {
   public readonly resetEmitter: TEmitter = new Emitter();
 
   public animation: Animation | null = null;
-  public readonly isActiveProperty: TReadOnlyProperty<boolean>;
   public soccerPlayer: SoccerPlayer | null = null;
 
   // Global index for debugging
@@ -87,7 +83,7 @@ export default class SoccerBall extends PhetioObject {
     this.velocityProperty = new Vector2Property( new Vector2( 0, 0 ), {
       tandem: options.tandem.createTandem( 'velocityProperty' )
     } );
-    this.soccerBallPhaseProperty = new EnumerationProperty( isFirstSoccerBall ? AnimationMode.READY : AnimationMode.INACTIVE, {
+    this.soccerBallPhaseProperty = new EnumerationProperty( isFirstSoccerBall ? SoccerBallPhase.READY : SoccerBallPhase.INACTIVE, {
       tandem: options.tandem.createTandem( 'soccerBallPhaseProperty' )
     } );
     this.dragPositionProperty = new Vector2Property( this.positionProperty.value.copy() );
@@ -107,11 +103,6 @@ export default class SoccerBall extends PhetioObject {
     this.isAnimationHighlightVisibleProperty = new BooleanProperty( false, {
       tandem: options.tandem.createTandem( 'isAnimationHighlightVisibleProperty' )
     } );
-    this.isActiveProperty = new DerivedProperty( [ this.soccerBallPhaseProperty ],
-      soccerBallPhase => soccerBallPhase !== AnimationMode.INACTIVE, {
-        tandem: options.tandem.createTandem( 'isActiveProperty' ),
-        phetioValueType: BooleanIO
-      } );
     this.targetXProperty = new Property<number | null>( null, {
       tandem: options.tandem.createTandem( 'targetXProperty' ),
       phetioValueType: NullableIO( NumberIO )
@@ -127,7 +118,7 @@ export default class SoccerBall extends PhetioObject {
   }
 
   public step( dt: number ): void {
-    if ( this.soccerBallPhaseProperty.value === AnimationMode.FLYING ) {
+    if ( this.soccerBallPhaseProperty.value === SoccerBallPhase.FLYING ) {
 
       assert && assert( this.targetXProperty.value !== null, 'targetXProperty.value should be non-null when animating' );
 

diff --git a/js/common/model/SoccerBallPhase.ts b/js/common/model/SoccerBallPhase.ts
@@ -0,0 +1,23 @@
+// Copyright 2022-2023, University of Colorado Boulder
+
+import Enumeration from '../../../../phet-core/js/Enumeration.js';
+import EnumerationValue from '../../../../phet-core/js/EnumerationValue.js';
+import centerAndVariability from '../../centerAndVariability.js';
+
+/**
+ * SoccerBallPhase is used to identify what type of animation a SoccerBall is undergoing.
+ *
+ * @author Chris Klusendorf (PhET Interactive Simulations)
+ * @author Sam Reid (PhET Interactive Simulations)
+ */
+
+export class SoccerBallPhase extends EnumerationValue {
+  public static readonly INACTIVE = new SoccerBallPhase();
+  public static readonly READY = new SoccerBallPhase();
+  public static readonly FLYING = new SoccerBallPhase();
+  public static readonly STACKING = new SoccerBallPhase();
+  public static readonly STACKED = new SoccerBallPhase();
+  private static readonly enumeration = new Enumeration( SoccerBallPhase );
+}
+
+centerAndVariability.register( 'SoccerBallPhase', SoccerBallPhase );
diff --git a/js/common/view/CAVObjectNode.ts b/js/common/view/CAVObjectNode.ts
@@ -12,7 +12,7 @@ import centerAndVariability from '../../centerAndVariability.js';
 import { Node, NodeOptions, Text } from '../../../../scenery/js/imports.js';
 import SoccerBall from '../model/SoccerBall.js';
 import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js';
-import { AnimationMode } from '../model/AnimationMode.js';
+import { SoccerBallPhase } from '../model/SoccerBallPhase.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
 import Multilink from '../../../../axon/js/Multilink.js';
 import Vector2 from '../../../../dot/js/Vector2.js';
@@ -50,8 +50,8 @@ export default class CAVObjectNode extends Node {
     // 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.
     Multilink.multilink( [ soccerBall.valueProperty, soccerBall.soccerBallPhaseProperty ],
-      ( value, animationMode ) => {
-        this.opacity = ( value === null && animationMode === AnimationMode.READY && !soccerBall.isFirstSoccerBall ) ? 0.4 : 1;
+      ( value, soccerBallPhase ) => {
+        this.opacity = ( value === null && soccerBallPhase === SoccerBallPhase.READY && !soccerBall.isFirstSoccerBall ) ? 0.4 : 1;
       } );
   }
 

diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts
@@ -37,7 +37,7 @@ import CAVObjectType from '../model/CAVObjectType.js';
 import ToggleNode from '../../../../sun/js/ToggleNode.js';
 import PlayAreaMedianIndicatorNode from './PlayAreaMedianIndicatorNode.js';
 import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
-import { AnimationMode } from '../model/AnimationMode.js';
+import { SoccerBallPhase } from '../model/SoccerBallPhase.js';
 
 type SelfOptions = {
   questionBarOptions: StrictOmit<QuestionBarOptions, 'tandem'>;
@@ -255,7 +255,7 @@ export default class CAVScreenView extends ScreenView {
   private getTopObjectPositionY( value: number ): number {
     const sceneModel = this.model.selectedSceneModelProperty.value;
     const ballsAtLocation = sceneModel.soccerBalls.filter( soccerBall =>
-      soccerBall.valueProperty.value === value && soccerBall.soccerBallPhaseProperty.value === AnimationMode.STACKED );
+      soccerBall.valueProperty.value === value && soccerBall.soccerBallPhaseProperty.value === SoccerBallPhase.STACKED );
     const modelHeight = ballsAtLocation.length * CAVObjectType.SOCCER_BALL.radius * 2 * ( 1 - CAVConstants.SOCCER_BALL_OVERLAP );
     const viewHeight = this.modelViewTransform.modelToViewDeltaY( modelHeight );
     return this.modelViewTransform.modelToViewY( 0 ) + viewHeight;

diff --git a/js/common/view/DataPointNode.ts b/js/common/view/DataPointNode.ts
@@ -10,10 +10,11 @@ import Vector2 from '../../../../dot/js/Vector2.js';
 import timesSolidShape from '../../../../sherpa/js/fontawesome-5/timesSolidShape.js';
 import CAVConstants, { DATA_POINT_SCALE_PROPERTY } from '../CAVConstants.js';
 import PlotType from '../model/PlotType.js';
-import Multilink from '../../../../axon/js/Multilink.js';
 import CAVColors from '../CAVColors.js';
 import Property from '../../../../axon/js/Property.js';
 import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js';
+import { SoccerBallPhase } from '../model/SoccerBallPhase.js';
+import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
 
 type DataPointNodeOptions = CAVObjectNodeOptions & { fill: TColor };
 
@@ -35,9 +36,15 @@ export default class DataPointNode extends CAVObjectNode {
       translationProperty.value = modelViewTransform.modelToViewPosition( scaledPosition );
     };
 
-    super( soccerBall, modelViewTransform, CAVObjectType.DATA_POINT.radius, optionize<DataPointNodeOptions, EmptySelfOptions, CAVObjectNodeOptions>()( {
-      translationStrategy: translationStrategy
-    }, providedOptions ) );
+    const options = optionize<DataPointNodeOptions, EmptySelfOptions, CAVObjectNodeOptions>()( {
+      translationStrategy: translationStrategy,
+
+      // Data point should be visible if the soccer ball landed
+      visibleProperty: new DerivedProperty( [ soccerBall.soccerBallPhaseProperty ], phase =>
+        phase === SoccerBallPhase.STACKED || phase === SoccerBallPhase.STACKING )
+    }, providedOptions );
+
+    super( soccerBall, modelViewTransform, CAVObjectType.DATA_POINT.radius, options );
 
     DATA_POINT_SCALE_PROPERTY.link( scale => {
       this.setScaleMagnitude( scale );
@@ -83,11 +90,6 @@ export default class DataPointNode extends CAVObjectNode {
 
     this.addChild( node );
 
-    // Data point should be visible if the soccer ball is active AND if the soccer ball took a non-null value.
-    Multilink.multilink( [ soccerBall.isActiveProperty, soccerBall.valueProperty ], ( isActive, value ) => {
-      this.visible = isActive && value !== null;
-    } );
-
     super.addDebugText( soccerBall );
   }
 }

diff --git a/js/common/view/SceneView.ts b/js/common/view/SceneView.ts
@@ -9,7 +9,7 @@
 
 import { Node } from '../../../../scenery/js/imports.js';
 import SoccerBallNode from './SoccerBallNode.js';
-import { AnimationMode } from '../model/AnimationMode.js';
+import { SoccerBallPhase } from '../model/SoccerBallPhase.js';
 import CAVSceneModel from '../model/CAVSceneModel.js';
 import SoccerPlayerNode, { SoccerPlayerImageSet } from './SoccerPlayerNode.js';
 import Tandem from '../../../../tandem/js/Tandem.js';
@@ -54,12 +54,11 @@ export default class SceneView {
     } );
 
     // Keep soccer balls in one layer so we can control the focus order
-    const frontLayer = new Node( );
+    const frontLayer = new Node();
 
     sceneModel.soccerBalls.map( ( soccerBall, index ) => {
       const soccerBallNode = new SoccerBallNode(
         soccerBall,
-        sceneModel.isVisibleProperty,
         modelViewTransform,
         model.objectNodesInputEnabledProperty, {
           tandem: options.tandem.createTandem( 'soccerBallNodes' ).createTandem( `soccerBallNode${index + 1}` ),
@@ -69,14 +68,14 @@ export default class SceneView {
       backLayerSoccerBallLayer.addChild( soccerBallNode );
 
       // While flying, it should be in front in z-order, to be in front of the accordion box
-      soccerBall.soccerBallPhaseProperty.lazyLink( ( animationMode, oldAnimationMode ) => {
+      soccerBall.soccerBallPhaseProperty.lazyLink( ( soccerBallPhase, oldSoccerBallPhase ) => {
 
         //when the ball is kicked
-        if ( animationMode === AnimationMode.FLYING ) {
+        if ( soccerBallPhase === SoccerBallPhase.FLYING ) {
           backLayerSoccerBallLayer.removeChild( soccerBallNode );
           frontLayer.addChild( soccerBallNode );
         }
-        else if ( oldAnimationMode === AnimationMode.FLYING ) {
+        else if ( oldSoccerBallPhase === SoccerBallPhase.FLYING ) {
           frontLayer.removeChild( soccerBallNode );
           backLayerSoccerBallLayer.addChild( soccerBallNode );
         }