diff --git a/js/common/model/CAVModel.ts b/js/common/model/CAVModel.ts index 8bd2fe70..04dba1e0 100644 --- a/js/common/model/CAVModel.ts +++ b/js/common/model/CAVModel.ts @@ -87,16 +87,18 @@ export default class CAVModel implements TModel { public readonly resetEmitter: TEmitter = new Emitter(); public readonly numberOfDataPointsProperty: NumberProperty; - public readonly soccerPlayerGroup: PhetioGroup; + public readonly soccerPlayers: SoccerPlayer[]; + private readonly nextBallToKickProperty: Property; // Null if there is no more ball to kick private readonly numberOfScheduledSoccerBallsToKickProperty: NumberProperty; public readonly numberOfRemainingKickableSoccerBallsProperty: TReadOnlyProperty; public readonly hasKickableSoccerBallsProperty: TReadOnlyProperty; - private readonly timeWhenLastBallWasKickedProperty: NumberProperty; - private readonly ballPlayerMap: Map = new Map(); // TODO: Add to PhET-iO State, see https://github.com/phetsims/center-and-variability/issues/128. Keep in mind we may avoid PhetioGroup protected readonly distributionProperty: Property>; + // Starting at 0, iterate through the index of the kickers. This updates the SoccerPlayer.isActiveProperty to show the current kicker + private readonly activeKickerIndexProperty: NumberProperty; + public constructor( providedOptions: CAVModelOptions ) { const options = optionize()( {}, providedOptions ); @@ -247,17 +249,12 @@ export default class CAVModel implements TModel { tandem: options.tandem.createTandem( 'timeWhenLastBallWasKickedProperty' ) } ); - this.soccerPlayerGroup = new PhetioGroup( ( tandem: Tandem, placeInLine: number ) => { + this.soccerPlayers = _.range( 0, this.maxNumberOfObjects ).map( placeInLine => { return new SoccerPlayer( placeInLine, { - tandem: tandem + tandem: options.tandem.createTandem( 'soccerPlayers' ).createTandem( 'soccerPlayer' + placeInLine ) } ); - }, [ 0 ], { - phetioType: PhetioGroup.PhetioGroupIO( SoccerPlayer.SoccerPlayerIO ), - tandem: options.tandem.createTandem( 'soccerPlayerGroup' ) } ); - this.populateSoccerPlayerGroup(); - // Create an initial ball to show on startup this.nextBallToKickProperty = new Property( this.createBall(), { tandem: options.tandem.createTandem( 'nextBallToKickProperty' ), @@ -289,9 +286,7 @@ export default class CAVModel implements TModel { this.objectValueBecameNonNullEmitter.addListener( cavObject => { // If the soccer player that kicked that ball was still in line when the ball lands, they can leave the line now. - if ( this.soccerPlayerGroup.includes( this.ballPlayerMap.get( cavObject )! ) ) { - this.advanceLine(); - } + this.advanceLine(); if ( this.numberOfRemainingObjectsProperty.value > 0 && this.nextBallToKickProperty.value === null ) { this.nextBallToKickProperty.value = this.createBall(); @@ -307,6 +302,16 @@ export default class CAVModel implements TModel { } } ); } ); + + this.activeKickerIndexProperty = new NumberProperty( 0, { + tandem: options.tandem.createTandem( 'activeKickerIndexProperty' ) + } ); + + this.activeKickerIndexProperty.link( activeKickerIndex => { + this.soccerPlayers.forEach( ( soccerPlayer, index ) => { + soccerPlayer.isActiveProperty.value = index === activeKickerIndex; + } ); + } ); } protected objectCreated( cavObject: CAVObject ): void { @@ -448,10 +453,10 @@ export default class CAVModel implements TModel { this.numberOfScheduledSoccerBallsToKickProperty.reset(); this.timeProperty.reset(); this.timeWhenLastBallWasKickedProperty.reset(); - this.ballPlayerMap.clear(); - this.soccerPlayerGroup.clear(); - this.populateSoccerPlayerGroup(); this.nextBallToKickProperty.value = this.createBall(); + this.activeKickerIndexProperty.reset(); + + this.soccerPlayers.forEach( soccerPlayer => soccerPlayer.reset() ); } /** @@ -492,13 +497,9 @@ export default class CAVModel implements TModel { this.timeProperty.value += dt; this.objectGroup.forEach( cavObject => cavObject.step( dt ) ); + const frontPlayer = this.soccerPlayers[ this.activeKickerIndexProperty.value ]; - const frontPlayerList = this.soccerPlayerGroup.filter( soccerPlayer => soccerPlayer.placeInLineProperty.value === 0 ); - - if ( frontPlayerList.length > 0 ) { - - assert && assert( frontPlayerList.length === 1, 'incorrect number of front soccer players: ' + frontPlayerList.length ); - const frontPlayer = frontPlayerList[ 0 ]; + if ( frontPlayer ) { // TODO: number of balls that exist but haven't been kicked??? See KickButtonGroup. Also this may change if we ditch PhetioGroup const numberBallsThatExistButHaventBeenKicked = this.nextBallToKickProperty.value === null ? 0 : 1; @@ -514,9 +515,6 @@ export default class CAVModel implements TModel { this.nextBallToKickProperty.value = this.createBall(); } - // TODO: Why is this called here? https://github.com/phetsims/center-and-variability/issues/59 - this.advanceLine(); - assert && assert( this.nextBallToKickProperty.value !== null, 'there was no ball to kick' ); if ( frontPlayer.poseProperty.value === Pose.STANDING ) { @@ -542,25 +540,11 @@ export default class CAVModel implements TModel { // When a ball lands, or when the next player is supposed to kick (before the ball lands), move the line forward private advanceLine(): void { - // if the previous ball was still in the air, we need to move the line forward so the next player can kick - const kickingPlayers = this.soccerPlayerGroup.filter( soccerPlayer => soccerPlayer.poseProperty.value === Pose.KICKING ); - assert && assert( kickingPlayers.length === 0 || kickingPlayers.length === 1, 'Too many kickers' ); - if ( kickingPlayers.length === 1 ) { - - const soccerPlayersToDispose: SoccerPlayer[] = []; - - this.soccerPlayerGroup.forEach( soccerPlayer => { - if ( soccerPlayer.placeInLineProperty.value === 0 ) { - soccerPlayersToDispose.push( soccerPlayer ); - } - else { - soccerPlayer.placeInLineProperty.value--; - } - } ); - - assert && assert( soccerPlayersToDispose.length === 1, 'should always dispose the front soccer player only' ); - soccerPlayersToDispose.forEach( soccerPlayer => this.soccerPlayerGroup.disposeElement( soccerPlayer ) ); + let nextIndex = this.activeKickerIndexProperty.value + 1; + if ( nextIndex > this.maxNumberOfObjects ) { + nextIndex = 0; } + this.activeKickerIndexProperty.value = nextIndex; } private static chooseDistribution(): ReadonlyArray { @@ -631,8 +615,6 @@ export default class CAVModel implements TModel { private kickBall( soccerPlayer: SoccerPlayer, cavObject: CAVObject ): void { soccerPlayer.poseProperty.value = Pose.KICKING; - this.ballPlayerMap.set( cavObject, soccerPlayer ); - // Test that the sampling engine is working properly // TODO: Where should these tests live? Should it be in the unit tests? Or in dot? // const array = new Array( weights.length ); @@ -671,10 +653,6 @@ export default class CAVModel implements TModel { // New ball will be created later in step this.nextBallToKickProperty.value = null; } - - private populateSoccerPlayerGroup(): void { - _.times( this.maxNumberOfObjects, index => this.soccerPlayerGroup.createNextElement( index ) ); - } } centerAndVariability.register( 'CAVModel', CAVModel ); \ No newline at end of file diff --git a/js/common/model/SoccerPlayer.ts b/js/common/model/SoccerPlayer.ts index 6de77c14..9ba1497b 100644 --- a/js/common/model/SoccerPlayer.ts +++ b/js/common/model/SoccerPlayer.ts @@ -8,51 +8,28 @@ */ import centerAndVariability from '../../centerAndVariability.js'; -import PhetioObject, { PhetioObjectOptions } from '../../../../tandem/js/PhetioObject.js'; -import NumberProperty from '../../../../axon/js/NumberProperty.js'; -import IOType from '../../../../tandem/js/types/IOType.js'; -import NumberIO from '../../../../tandem/js/types/NumberIO.js'; -import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; +import PhetioObject from '../../../../tandem/js/PhetioObject.js'; import Property from '../../../../axon/js/Property.js'; import Pose from './Pose.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; +import BooleanProperty from '../../../../axon/js/BooleanProperty.js'; -type SelfOptions = EmptySelfOptions; -type SoccerPlayerOptions = - SelfOptions - & PhetioObjectOptions - & PickRequired; - -export default class SoccerPlayer extends PhetioObject { +export default class SoccerPlayer { public readonly poseProperty = new Property( Pose.STANDING ); - public readonly placeInLineProperty: NumberProperty; - public static readonly SoccerPlayerIO = new IOType( 'SoccerPlayerIO', { - valueType: SoccerPlayer, - toStateObject: ( soccerPlayer: SoccerPlayer ) => ( { initialPlaceInLine: soccerPlayer.initialPlaceInLine } ), - stateObjectToCreateElementArguments: ( stateObject: SoccerPlayerState ) => { - return [ stateObject.initialPlaceInLine ]; - }, - stateSchema: { - initialPlaceInLine: NumberIO - } - } ); // Also used to determine the artwork for rendering the SoccerPlayerNode public readonly initialPlaceInLine: number; public timestampWhenPoisedBegan: number | null = null; + public readonly isActiveProperty: BooleanProperty; - public constructor( placeInLine: number, providedOptions: SoccerPlayerOptions ) { - - const options = optionize()( { - phetioType: SoccerPlayer.SoccerPlayerIO, - phetioDynamicElement: true - }, providedOptions ); + public constructor( placeInLine: number, options: PickRequired ) { - super( options ); + this.isActiveProperty = new BooleanProperty( placeInLine === 0, { + tandem: options.tandem.createTandem( 'isActiveProperty' ), - this.placeInLineProperty = new NumberProperty( placeInLine, { - tandem: options.tandem.createTandem( 'placeInLineProperty' ) + // This is updated by the CAVModel.activeKickerIndexProperty + phetioReadOnly: true } ); this.initialPlaceInLine = placeInLine; @@ -61,15 +38,6 @@ export default class SoccerPlayer extends PhetioObject { public reset(): void { this.poseProperty.reset(); } - - public override dispose(): void { - this.placeInLineProperty.dispose(); - super.dispose(); - } } -type SoccerPlayerState = { - initialPlaceInLine: number; -}; - centerAndVariability.register( 'SoccerPlayer', SoccerPlayer ); \ No newline at end of file diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts index ca6a07cb..604d76eb 100644 --- a/js/common/view/CAVScreenView.ts +++ b/js/common/view/CAVScreenView.ts @@ -32,7 +32,6 @@ import NumberLineNode from './NumberLineNode.js'; import Bounds2 from '../../../../dot/js/Bounds2.js'; import BackgroundNode from './BackgroundNode.js'; import SoccerPlayerNode from './SoccerPlayerNode.js'; -import SoccerPlayer from '../model/SoccerPlayer.js'; import merge from '../../../../phet-core/js/merge.js'; import KickButtonGroup from './KickButtonGroup.js'; import PlayAreaMedianIndicatorNode from './PlayAreaMedianIndicatorNode.js'; @@ -266,36 +265,13 @@ export default class CAVScreenView extends ScreenView { } ); this.contentLayer.addChild( this.playAreaNumberLineNode ); - const soccerPlayerNodeGroup = new PhetioGroup( ( tandem, soccerPlayer ) => { + const soccerPlayerNodes = model.soccerPlayers.map( soccerPlayer => { return new SoccerPlayerNode( soccerPlayer, this.modelViewTransform, { - tandem: tandem + tandem: options.tandem.createTandem( 'soccerPlayerNodes' ).createTandem( 'soccerPlayerNode' + soccerPlayer.initialPlaceInLine ) } ); - }, () => [ model.soccerPlayerGroup.archetype ], { - phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ), - tandem: options.tandem.createTandem( 'soccerPlayerNodeGroup' ), - supportsDynamicState: false - } ); - - // A layer for the soccer players, so we can adjust their z-ordering within that layer - const soccerPlayerLayer = new Node(); - const createSoccerPlayerNode = ( soccerPlayer: SoccerPlayer ) => { - const soccerPlayerNode = soccerPlayerNodeGroup.createCorrespondingGroupElement( soccerPlayer.tandem.name, soccerPlayer ); - soccerPlayerLayer.addChild( soccerPlayerNode ); - - // TODO: Document why this is correct (since it seems counterintuitive) - soccerPlayerNode.moveToBack(); - }; - this.contentLayer.addChild( soccerPlayerLayer ); - model.soccerPlayerGroup.forEach( createSoccerPlayerNode ); - model.soccerPlayerGroup.elementCreatedEmitter.addListener( createSoccerPlayerNode ); - - model.soccerPlayerGroup.elementDisposedEmitter.addListener( soccerPlayer => { - const viewNode = soccerPlayerNodeGroup.getArray().find( soccerPlayerNode => soccerPlayerNode.soccerPlayer === soccerPlayer )!; - soccerPlayerNodeGroup.disposeElement( viewNode ); } ); - // 0th soccer player is at the front of the line, and should also be in the front in z-ordering - soccerPlayerNodeGroup.getArrayCopy().reverse().forEach( soccerPlayerNode => soccerPlayerNode.moveToFront() ); + soccerPlayerNodes.forEach( soccerPlayerNode => this.contentLayer.addChild( soccerPlayerNode ) ); this.questionBar = new QuestionBar( this.layoutBounds, this.visibleBoundsProperty, merge( { tandem: options.tandem.createTandem( 'questionBar' ) diff --git a/js/common/view/SoccerPlayerNode.ts b/js/common/view/SoccerPlayerNode.ts index 1cfbfc60..4240c828 100644 --- a/js/common/view/SoccerPlayerNode.ts +++ b/js/common/view/SoccerPlayerNode.ts @@ -165,16 +165,20 @@ export default class SoccerPlayerNode extends Node { this.centerBottom = modelViewTransform.modelToViewPosition( new Vector2( 0, 0 ) ).plusXY( -28, 8.5 ); } ); - soccerPlayer.placeInLineProperty.link( placeInLine => { - this.visible = placeInLine === 0; + soccerPlayer.isActiveProperty.link( isActive => { + this.visible = isActive; } ); const options = optionize()( { - phetioDynamicElement: true, - excludeInvisibleChildrenFromBounds: false + excludeInvisibleChildrenFromBounds: false, + phetioVisiblePropertyInstrumented: false }, providedOptions ); this.mutate( options ); + + this.addLinkedElement( soccerPlayer.isActiveProperty, { + tandem: options.tandem?.createTandem( 'isActiveProperty' ) + } ); } }