Avoid PhetioGroup #160
Assignees
Labels
Comments
samreid
added a commit
that referenced
this issue
Apr 26, 2023
|
We converted soccer player and its node to static. This cut down around 70 lines of code, addressed several todos and fixed a bug in the state wrapper. Nice work everyone! There are about 5 more PhetioGroup to go. |
|
@marlitas and @matthew-blackman and I collaborated on the first part, and I'll continue. I'll probably have questions or need help on review afterwards. |
|
I would also like to see how difficult it is to have studio gray out tree element that are |
|
@zepumph and I realized that since only one SoccerPlayer is shown at once now, we may not need arrays or groups at all for that. |
samreid
added a commit
that referenced
this issue
Apr 26, 2023
samreid
added a commit
that referenced
this issue
Apr 27, 2023
samreid
added a commit
that referenced
this issue
Apr 27, 2023
samreid
added a commit
that referenced
this issue
Apr 27, 2023
|
I'll probably need help on this one. Here's a patch attempt at moving the soccer balls to a static pool: Subject: [PATCH] Rename cavObject => soccerBall, see https://github.com/phetsims/center-and-variability/issues/160
---
Index: js/mean-and-median/view/MedianPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/mean-and-median/view/MedianPlotNode.ts b/js/mean-and-median/view/MedianPlotNode.ts
--- a/js/mean-and-median/view/MedianPlotNode.ts (revision 5de536b76ae07233abfba139c0d50ae0e1995b92)
+++ b/js/mean-and-median/view/MedianPlotNode.ts (date 1682596876344)
@@ -40,7 +40,7 @@
const updateMedianBarNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
Index: js/common/model/CAVObject.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CAVObject.ts b/js/common/model/CAVObject.ts
--- a/js/common/model/CAVObject.ts (revision 5de536b76ae07233abfba139c0d50ae0e1995b92)
+++ b/js/common/model/CAVObject.ts (date 1682596268642)
@@ -78,6 +78,7 @@
} );
public readonly dragStartedEmitter: TEmitter = new Emitter();
public animation: Animation | null = null;
+ public readonly isActiveProperty: BooleanProperty;
public constructor( objectType: CAVObjectType, providedOptions: CAVObjectOptions ) {
@@ -118,6 +119,9 @@
this.isShowingAnimationHighlightProperty = new BooleanProperty( false, {
tandem: options.tandem.createTandem( 'isShowingAnimationHighlightProperty' )
} );
+ this.isActiveProperty = new BooleanProperty( false, {
+ tandem: options.tandem.createTandem( 'isActiveProperty' )
+ } );
}
public step( dt: number ): void {
@@ -151,17 +155,28 @@
}
}
- public override dispose(): void {
- super.dispose();
- this.positionProperty.dispose();
- this.velocityProperty.dispose();
- this.animationModeProperty.dispose();
- this.valueProperty.dispose();
- this.dragPositionProperty.dispose();
- this.isMedianObjectProperty.dispose();
- this.isShowingAnimationHighlightProperty.dispose();
- this.disposedEmitter.emit();
- this.disposedEmitter.dispose();
+ //
+ // public override dispose(): void {
+ // super.dispose();
+ // this.positionProperty.dispose();
+ // this.velocityProperty.dispose();
+ // this.animationModeProperty.dispose();
+ // this.valueProperty.dispose();
+ // this.dragPositionProperty.dispose();
+ // this.isMedianObjectProperty.dispose();
+ // this.isShowingAnimationHighlightProperty.dispose();
+ // this.disposedEmitter.emit();
+ // this.disposedEmitter.dispose();
+ // }
+
+ public reset(): void {
+ this.positionProperty.reset();
+ this.velocityProperty.reset();
+ this.animationModeProperty.reset();
+ this.valueProperty.reset();
+ this.dragPositionProperty.reset();
+ this.isMedianObjectProperty.reset();
+ this.isShowingAnimationHighlightProperty.reset();
}
public toStateObject(): CAVObjectStateType {
Index: js/variability/view/RangeNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/RangeNode.ts b/js/variability/view/RangeNode.ts
--- a/js/variability/view/RangeNode.ts (revision 5de536b76ae07233abfba139c0d50ae0e1995b92)
+++ b/js/variability/view/RangeNode.ts (date 1682596928571)
@@ -65,7 +65,7 @@
const updateRangeNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
const rightmostDot = sortedDots[ sortedDots.length - 1 ];
Index: js/variability/view/MADNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/MADNode.ts b/js/variability/view/MADNode.ts
--- a/js/variability/view/MADNode.ts (revision 5de536b76ae07233abfba139c0d50ae0e1995b92)
+++ b/js/variability/view/MADNode.ts (date 1682596912575)
@@ -76,7 +76,7 @@
const children: Node[] = [];
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
if ( sortedDots.length > 0 ) {
Index: js/common/view/CAVPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVPlotNode.ts b/js/common/view/CAVPlotNode.ts
--- a/js/common/view/CAVPlotNode.ts (revision 5de536b76ae07233abfba139c0d50ae0e1995b92)
+++ b/js/common/view/CAVPlotNode.ts (date 1682596707114)
@@ -93,7 +93,7 @@
tandem: tandem,
fill: options.dataPointFill
} );
- }, () => [ model.soccerBallGroup.archetype ], {
+ }, () => [ model.soccerBallGroup[ 0 ] ], {
phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ),
tandem: options.tandem.createTandem( 'dotNodeGroup' ),
supportsDynamicState: false
@@ -113,9 +113,9 @@
map.set( cavObject, dotNode );
};
model.soccerBallGroup.forEach( createDotNode );
- model.soccerBallGroup.elementCreatedEmitter.addListener( createDotNode );
+ model.soccerBallGroupElementCreatedEmitter.addListener( createDotNode );
- model.soccerBallGroup.elementDisposedEmitter.addListener( cavObject => {
+ model.soccerBallGroupElementDisposedEmitter.addListener( cavObject => {
const viewNode = map.get( cavObject )!;
dotNodeGroup.disposeElement( viewNode );
map.delete( cavObject );
Index: js/common/view/CAVScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts
--- a/js/common/view/CAVScreenView.ts (revision 5de536b76ae07233abfba139c0d50ae0e1995b92)
+++ b/js/common/view/CAVScreenView.ts (date 1682596977140)
@@ -97,7 +97,7 @@
fill: null, // Only depict as a soccer ball
tandem: tandem
} );
- }, () => [ model.soccerBallGroup.archetype ], {
+ }, () => [ model.soccerBallGroup[ 0 ] ], {
phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ),
tandem: objectNodeGroupTandem,
supportsDynamicState: false
@@ -128,9 +128,9 @@
// add the dragIndicatorArrowNode above the last object when it is added to the play area. if an object was
// moved before this happens, don't show the dragIndicatorArrowNode
- if ( model.soccerBallGroup.countProperty.value === this.model.physicalRange.max &&
+ if ( model.soccerBallGroupCountProperty.value === this.model.physicalRange.max &&
objectNodesInputEnabledProperty.value &&
- _.every( model.soccerBallGroup.getArray(), cavObject => cavObject.valueProperty.value !== null ) &&
+ _.every( model.soccerBallGroup, cavObject => cavObject.valueProperty.value !== null ) &&
!objectHasBeenDragged ) {
dragIndicatorArrowNode.centerX = this.modelViewTransform.modelToViewX( value );
@@ -155,10 +155,10 @@
map.set( cavObject, cavObjectNode );
};
- model.soccerBallGroup.forEach( createObjectNode );
- model.soccerBallGroup.elementCreatedEmitter.addListener( createObjectNode );
+ model.soccerBallGroup.filter( soccerBall => soccerBall.isActiveProperty.value ).forEach( createObjectNode );
+ model.soccerBallGroupElementCreatedEmitter.addListener( createObjectNode );
- model.soccerBallGroup.elementDisposedEmitter.addListener( cavObject => {
+ model.soccerBallGroupElementDisposedEmitter.addListener( cavObject => {
const viewNode = map.get( cavObject )!;
objectNodeGroup.disposeElement( viewNode );
map.delete( cavObject );
Index: js/median/model/MedianModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/model/MedianModel.ts b/js/median/model/MedianModel.ts
--- a/js/median/model/MedianModel.ts (revision 5de536b76ae07233abfba139c0d50ae0e1995b92)
+++ b/js/median/model/MedianModel.ts (date 1682596885904)
@@ -29,7 +29,7 @@
return new CardModel( cavObject, {
tandem: tandem
} );
- }, () => [ this.soccerBallGroup.archetype ], {
+ }, () => [ this.soccerBallGroup[ 0 ] ], {
phetioType: PhetioGroup.PhetioGroupIO( CardModel.CardModelIO ),
tandem: options.tandem.createTandem( 'cardModelGroup' )
} );
Index: js/common/model/CAVModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CAVModel.ts b/js/common/model/CAVModel.ts
--- a/js/common/model/CAVModel.ts (revision 5de536b76ae07233abfba139c0d50ae0e1995b92)
+++ b/js/common/model/CAVModel.ts (date 1682597170513)
@@ -8,11 +8,8 @@
*/
import centerAndVariability from '../../centerAndVariability.js';
-import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
-import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js';
-import PhetioGroup from '../../../../tandem/js/PhetioGroup.js';
import Tandem from '../../../../tandem/js/Tandem.js';
-import CAVObject, { CAVObjectOptions } from './CAVObject.js';
+import CAVObject from './CAVObject.js';
import CAVObjectType from './CAVObjectType.js';
import Vector2 from '../../../../dot/js/Vector2.js';
import Range from '../../../../dot/js/Range.js';
@@ -47,7 +44,14 @@
const TIME_BETWEEN_RAPID_KICKS = 0.5; // in seconds
export default class CAVModel implements TModel {
- public readonly soccerBallGroup: PhetioGroup<CAVObject, [ StrictOmit<CAVObjectOptions, 'tandem'> ]>;
+ public readonly soccerBallGroup: CAVObject[];
+ public readonly soccerBallGroupElementCreatedEmitter = new Emitter<[ CAVObject ]>( {
+ parameters: [ { valueType: CAVObject } ]
+ } );
+ public readonly soccerBallGroupElementDisposedEmitter = new Emitter<[ CAVObject ]>(
+ { parameters: [ { valueType: CAVObject } ] }
+ );
+ public readonly soccerBallGroupCountProperty: NumberProperty;
public readonly isShowingTopMeanProperty: BooleanProperty;
public readonly isShowingTopMedianProperty: BooleanProperty;
@@ -101,16 +105,20 @@
public constructor( options: CAVModelOptions ) {
- this.soccerBallGroup = new PhetioGroup( ( tandem, providedOptions: StrictOmit<CAVObjectOptions, 'tandem'> ) => {
+ this.soccerBallGroupCountProperty = new NumberProperty( 0, {
+ range: new Range( 0, this.maxNumberOfObjects )
+ } );
- const options = optionize<StrictOmit<CAVObjectOptions, 'tandem'>, EmptySelfOptions, CAVObjectOptions>()( {
- // If it's the first element in the group, mark as isFirstObject. For creating archetype, the objectGroup does
- // not yet exist, so just mark it as first
- isFirstObject: this.soccerBallGroup ? this.soccerBallGroup.count === 0 : true,
- tandem: tandem
- }, providedOptions );
+ this.soccerBallGroup = _.range( 0, this.maxNumberOfObjects ).map( index => {
- const soccerBall = new CAVObject( CAVObjectType.SOCCER_BALL, options );
+ const y0 = CAVObjectType.SOCCER_BALL.radius;
+ const position = new Vector2( 0, y0 );
+
+ const soccerBall = new CAVObject( CAVObjectType.SOCCER_BALL, {
+ isFirstObject: index === 0,
+ tandem: options.tandem.createTandem( `soccerBall${index}` ),
+ position: position
+ } );
// TODO: Should some or all of this move into CAVObject or CAVObjectNode?
const dragPositionListener = ( dragPosition: Vector2 ) => {
@@ -119,13 +127,43 @@
this.moveToTop( soccerBall );
};
soccerBall.dragPositionProperty.lazyLink( dragPositionListener );
- soccerBall.disposedEmitter.addListener( () => soccerBall.dragPositionProperty.unlink( dragPositionListener ) );
+
+ soccerBall.valueProperty.link( ( value, oldValue ) => {
+ if ( value !== null && oldValue === null ) {
+ if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
+ this.soccerBallLandedListener( soccerBall, value );
+ }
+ }
+ } );
+
+ soccerBall.isActiveProperty.link( isActive => {
+ if ( isActive ) {
+ this.soccerBallGroupElementCreatedEmitter.emit( soccerBall );
+ }
+ else {
+ this.soccerBallGroupElementDisposedEmitter.emit( soccerBall );
+ }
+ } );
return soccerBall;
- }, [ {} ], {
- phetioType: PhetioGroup.PhetioGroupIO( CAVObject.CAVObjectIO ),
- tandem: options.tandem.createTandem( 'soccerBallGroup' )
} );
+
+ this.soccerBallGroup.forEach( soccerBall => {
+ soccerBall.isActiveProperty.link( isActive => {
+ this.soccerBallGroupCountProperty.value = this.soccerBallGroup.filter( soccerBall => soccerBall.isActiveProperty.value ).length;
+ } );
+ } );
+
+ // this.soccerBallGroup.elementCreatedEmitter = new Emitter();
+
+ // this.soccerBallGroup.elementRemovedEmitter = new Emitter();
+ // this.soccerBallGroup = new PhetioGroup( ( tandem, providedOptions: StrictOmit<CAVObjectOptions, 'tandem'> ) => {
+ //
+ //
+ // }, [ {} ], {
+ // phetioType: PhetioGroup.PhetioGroupIO( CAVObject.CAVObjectIO ),
+ // tandem: options.tandem.createTandem( 'soccerBallGroup' )
+ // } );
this.isShowingTopMeanProperty = new BooleanProperty( false, {
tandem: options.instrumentMeanPredictionProperty ? options.tandem.createTandem( 'isShowingTopMeanProperty' ) : Tandem.OPT_OUT
@@ -230,9 +268,10 @@
soccerBall.positionProperty.link( () => this.objectChangedEmitter.emit( soccerBall ) );
};
this.soccerBallGroup.forEach( objectCreatedListener );
- this.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
+ // this.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
- this.numberOfRemainingObjectsProperty = new DerivedProperty( [ this.soccerBallGroup.countProperty ], count => {
+ this.numberOfRemainingObjectsProperty = DerivedProperty.deriveAny( this.soccerBallGroup.map( ball => ball.isActiveProperty ), () => {
+ const count = this.soccerBallGroup.filter( ball => ball.isActiveProperty.value ).length;
return this.maxNumberOfObjects - count;
} );
@@ -250,7 +289,7 @@
this.soccerPlayers = _.range( 0, this.maxNumberOfObjects ).map( placeInLine => new SoccerPlayer( placeInLine ) );
// Create an initial ball to show on startup
- this.nextBallToKickProperty = new Property<CAVObject | null>( this.createBall(), {
+ this.nextBallToKickProperty = new Property<CAVObject | null>( this.soccerBallGroup[ 0 ], {
tandem: options.tandem.createTandem( 'nextBallToKickProperty' ),
phetioValueType: NullableIO( ReferenceIO( CAVObject.CAVObjectIO ) )
} );
@@ -283,18 +322,8 @@
this.advanceLine();
if ( this.numberOfRemainingObjectsProperty.value > 0 && this.nextBallToKickProperty.value === null ) {
- this.nextBallToKickProperty.value = this.createBall();
+ this.nextBallToKickProperty.value = this.getFirstInactiveBall();
}
- } );
-
- this.soccerBallGroup.elementCreatedEmitter.addListener( soccerBall => {
- soccerBall.valueProperty.link( ( value, oldValue ) => {
- if ( value !== null && oldValue === null ) {
- if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
- this.soccerBallLandedListener( soccerBall, value );
- }
- }
- } );
} );
this.activeKickerIndexProperty = new NumberProperty( 0, {
@@ -391,15 +420,17 @@
* Clears out the data and the cards
*/
public clearData(): void {
- this.soccerBallGroup.clear();
+ // this.soccerBallGroup.clear();
this.numberOfScheduledSoccerBallsToKickProperty.reset();
this.timeProperty.reset();
this.timeWhenLastBallWasKickedProperty.reset();
- this.nextBallToKickProperty.value = this.createBall();
this.activeKickerIndexProperty.reset();
this.soccerPlayers.forEach( soccerPlayer => soccerPlayer.reset() );
+ this.soccerBallGroup.forEach( soccerBall => soccerBall.reset() );
+
+ this.nextBallToKickProperty.value = this.soccerBallGroup[ 0 ];
}
/**
@@ -455,7 +486,7 @@
// Create the next ball.
// TODO-UX-HIGH: A ball is being created too soon, when using the multikick button
- this.nextBallToKickProperty.value = this.createBall();
+ this.nextBallToKickProperty.value = this.getFirstInactiveBall();
}
assert && assert( this.nextBallToKickProperty.value !== null, 'there was no ball to kick' );
@@ -514,19 +545,6 @@
return dotRandom.nextBoolean() ? CAVConstants.LEFT_SKEWED_DATA : CAVConstants.RIGHT_SKEWED_DATA;
}
- /**
- * Creates a ball at the starting kick position.
- */
- private createBall(): CAVObject {
-
- const y0 = CAVObjectType.SOCCER_BALL.radius;
- const position = new Vector2( 0, y0 );
-
- return this.soccerBallGroup.createNextElement( {
- position: position
- } );
- }
-
/**
* When a ball lands on the ground, animate all other balls that were at this location above the landed ball.
*/
@@ -616,6 +634,10 @@
// New ball will be created later in step
this.nextBallToKickProperty.value = null;
}
+
+ private getFirstInactiveBall(): CAVObject | null {
+ return this.soccerBallGroup.find( ball => !ball.isActiveProperty.value ) || null;
+ }
}
centerAndVariability.register( 'CAVModel', CAVModel );
\ No newline at end of file
Index: js/common/view/CardNodeContainer.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CardNodeContainer.ts b/js/common/view/CardNodeContainer.ts
--- a/js/common/view/CardNodeContainer.ts (revision 5de536b76ae07233abfba139c0d50ae0e1995b92)
+++ b/js/common/view/CardNodeContainer.ts (date 1682596631153)
@@ -122,8 +122,8 @@
} );
};
- model.soccerBallGroup.forEach( objectCreatedListener );
- model.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
+ model.soccerBallGroup.filter( soccerBall => soccerBall.isActiveProperty.value ).forEach( objectCreatedListener );
+ model.soccerBallGroupElementCreatedEmitter.addListener( objectCreatedListener );
model.cardModelGroup.elementDisposedEmitter.addListener( cardModel => {
const cardNode = this.getCardNode( cardModel.cavObject )!;
Index: js/variability/view/IQRNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/IQRNode.ts b/js/variability/view/IQRNode.ts
--- a/js/variability/view/IQRNode.ts (revision 5de536b76ae07233abfba139c0d50ae0e1995b92)
+++ b/js/variability/view/IQRNode.ts (date 1682596895294)
@@ -100,7 +100,7 @@
const updateIQRNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( object => object.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
const rightmostDot = sortedDots[ sortedDots.length - 1 ];
|
|
Also, I want to be really sure that we favor static pools to PhetioGroup, since this will be a lot of work and a lot more work if we change our minds later. |
|
More progress in this patch (no cards yet though, so only test this in screens 2-3). Subject: [PATCH] Update TODOs, see https://github.com/phetsims/center-and-variability/issues/45
---
Index: js/mean-and-median/view/MedianPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/mean-and-median/view/MedianPlotNode.ts b/js/mean-and-median/view/MedianPlotNode.ts
--- a/js/mean-and-median/view/MedianPlotNode.ts (revision 32162e745fe214576b7c1f962c255b73507fd169)
+++ b/js/mean-and-median/view/MedianPlotNode.ts (date 1682608170153)
@@ -40,7 +40,7 @@
const updateMedianBarNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
Index: js/common/model/CAVObject.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CAVObject.ts b/js/common/model/CAVObject.ts
--- a/js/common/model/CAVObject.ts (revision 32162e745fe214576b7c1f962c255b73507fd169)
+++ b/js/common/model/CAVObject.ts (date 1682608170142)
@@ -78,6 +78,7 @@
} );
public readonly dragStartedEmitter: TEmitter = new Emitter();
public animation: Animation | null = null;
+ public readonly isActiveProperty: BooleanProperty;
public constructor( objectType: CAVObjectType, providedOptions: CAVObjectOptions ) {
@@ -118,6 +119,9 @@
this.isShowingAnimationHighlightProperty = new BooleanProperty( false, {
tandem: options.tandem.createTandem( 'isShowingAnimationHighlightProperty' )
} );
+ this.isActiveProperty = new BooleanProperty( false, {
+ tandem: options.tandem.createTandem( 'isActiveProperty' )
+ } );
}
public step( dt: number ): void {
@@ -151,17 +155,28 @@
}
}
- public override dispose(): void {
- super.dispose();
- this.positionProperty.dispose();
- this.velocityProperty.dispose();
- this.animationModeProperty.dispose();
- this.valueProperty.dispose();
- this.dragPositionProperty.dispose();
- this.isMedianObjectProperty.dispose();
- this.isShowingAnimationHighlightProperty.dispose();
- this.disposedEmitter.emit();
- this.disposedEmitter.dispose();
+ //
+ // public override dispose(): void {
+ // super.dispose();
+ // this.positionProperty.dispose();
+ // this.velocityProperty.dispose();
+ // this.animationModeProperty.dispose();
+ // this.valueProperty.dispose();
+ // this.dragPositionProperty.dispose();
+ // this.isMedianObjectProperty.dispose();
+ // this.isShowingAnimationHighlightProperty.dispose();
+ // this.disposedEmitter.emit();
+ // this.disposedEmitter.dispose();
+ // }
+
+ public reset(): void {
+ this.positionProperty.reset();
+ this.velocityProperty.reset();
+ this.animationModeProperty.reset();
+ this.valueProperty.reset();
+ this.dragPositionProperty.reset();
+ this.isMedianObjectProperty.reset();
+ this.isShowingAnimationHighlightProperty.reset();
}
public toStateObject(): CAVObjectStateType {
Index: js/variability/view/RangeNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/RangeNode.ts b/js/variability/view/RangeNode.ts
--- a/js/variability/view/RangeNode.ts (revision 32162e745fe214576b7c1f962c255b73507fd169)
+++ b/js/variability/view/RangeNode.ts (date 1682608170160)
@@ -65,7 +65,7 @@
const updateRangeNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
const rightmostDot = sortedDots[ sortedDots.length - 1 ];
Index: js/variability/view/MADNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/MADNode.ts b/js/variability/view/MADNode.ts
--- a/js/variability/view/MADNode.ts (revision 32162e745fe214576b7c1f962c255b73507fd169)
+++ b/js/variability/view/MADNode.ts (date 1682608170159)
@@ -76,7 +76,7 @@
const children: Node[] = [];
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
if ( sortedDots.length > 0 ) {
Index: js/common/view/CAVPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVPlotNode.ts b/js/common/view/CAVPlotNode.ts
--- a/js/common/view/CAVPlotNode.ts (revision 32162e745fe214576b7c1f962c255b73507fd169)
+++ b/js/common/view/CAVPlotNode.ts (date 1682608391942)
@@ -83,26 +83,18 @@
backgroundNode.addChild( this.dotLayer );
+ const map = new Map<CAVObject, CAVObjectNode>();
+
// TODO: This overlaps with draggingEnabled
const dotPlotObjectNodesDraggableProperty = new BooleanProperty( false );
+ const dotNodeGroup = model.soccerBallGroup.map( cavObject => {
- const dotNodeGroup = new PhetioGroup<CAVObjectNode, [ CAVObject ]>( ( tandem, cavObject ) => {
- return new CAVObjectNode( cavObject, model.isShowingTopMedianProperty, modelViewTransform, dotPlotObjectNodesDraggableProperty, {
+ const dotNode = new CAVObjectNode( cavObject, model.isShowingTopMedianProperty, modelViewTransform, dotPlotObjectNodesDraggableProperty, {
objectViewType: CAVObjectType.DATA_POINT,
draggingEnabled: false,
- tandem: tandem,
+ tandem: options.tandem.createTandem( 'dotNodeGroup' ).createTandem( cavObject.tandem.name ),
fill: options.dataPointFill
} );
- }, () => [ model.soccerBallGroup.archetype ], {
- phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ),
- tandem: options.tandem.createTandem( 'dotNodeGroup' ),
- supportsDynamicState: false
- } );
-
- const map = new Map<CAVObject, CAVObjectNode>();
-
- const createDotNode = ( cavObject: CAVObject ) => {
- const dotNode = dotNodeGroup.createCorrespondingGroupElement( cavObject.tandem.name, cavObject );
cavObject.valueProperty.link( value => {
dotNode.setVisible( value !== null );
@@ -111,14 +103,7 @@
}
} );
map.set( cavObject, dotNode );
- };
- model.soccerBallGroup.forEach( createDotNode );
- model.soccerBallGroup.elementCreatedEmitter.addListener( createDotNode );
-
- model.soccerBallGroup.elementDisposedEmitter.addListener( cavObject => {
- const viewNode = map.get( cavObject )!;
- dotNodeGroup.disposeElement( viewNode );
- map.delete( cavObject );
+ return dotNode;
} );
}
Index: js/common/view/CAVScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts
--- a/js/common/view/CAVScreenView.ts (revision 32162e745fe214576b7c1f962c255b73507fd169)
+++ b/js/common/view/CAVScreenView.ts (date 1682608492311)
@@ -14,7 +14,6 @@
import CAVConstants from '../CAVConstants.js';
import ResetAllButton from '../../../../scenery-phet/js/buttons/ResetAllButton.js';
import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js';
-import PhetioGroup from '../../../../tandem/js/PhetioGroup.js';
import CAVObjectNode from './CAVObjectNode.js';
import { AlignBox, ManualConstraint, Node } from '../../../../scenery/js/imports.js';
import CAVObjectType from '../model/CAVObjectType.js';
@@ -92,45 +91,28 @@
tandem: objectNodeGroupTandem.createTandem( 'inputEnabledProperty' )
} );
- const objectNodeGroup = new PhetioGroup<CAVObjectNode, [ CAVObject ]>( ( tandem, cavObject ) => {
- return new CAVObjectNode( cavObject, model.isShowingPlayAreaMedianProperty, modelViewTransform, objectNodesInputEnabledProperty, {
- fill: null, // Only depict as a soccer ball
- tandem: tandem
- } );
- }, () => [ model.soccerBallGroup.archetype ], {
- phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ),
- tandem: objectNodeGroupTandem,
- supportsDynamicState: false
- } );
-
- this.addChild( this.contentLayer );
- this.addChild( this.frontObjectLayer );
-
const map = new Map<CAVObject, CAVObjectNode>();
- let objectHasBeenDragged = false;
- const dragIndicatorArrowNode = new DragIndicatorArrowNode( {
- tandem: options.tandem.createTandem( 'dragIndicatorArrowNode' ),
- visible: false
- } );
- this.backObjectLayer.addChild( dragIndicatorArrowNode );
+ const soccerBallNodes = model.soccerBallGroup.map( ( soccerBall, index ) => {
+ const soccerBallNode = new CAVObjectNode( soccerBall, model.isShowingPlayAreaMedianProperty, modelViewTransform, objectNodesInputEnabledProperty, {
+ fill: null, // Only depict as a soccer ball
+ tandem: options.tandem.createTandem( 'soccerBallGroup' ).createTandem( 'soccerBallNode' + index )
+ } );
- const createObjectNode = ( cavObject: CAVObject ) => {
- const cavObjectNode = objectNodeGroup.createCorrespondingGroupElement( cavObject.tandem.name, cavObject );
- this.frontObjectLayer.addChild( cavObjectNode );
+ this.frontObjectLayer.addChild( soccerBallNode );
- cavObject.valueProperty.lazyLink( ( value, oldValue ) => {
+ soccerBall.valueProperty.lazyLink( ( value, oldValue ) => {
if ( value !== null ) {
if ( oldValue === null ) {
- assert && assert( this.frontObjectLayer.hasChild( cavObjectNode ) );
- this.frontObjectLayer.removeChild( cavObjectNode );
- this.backObjectLayer.addChild( cavObjectNode );
+ assert && assert( this.frontObjectLayer.hasChild( soccerBallNode ) );
+ this.frontObjectLayer.removeChild( soccerBallNode );
+ this.backObjectLayer.addChild( soccerBallNode );
// add the dragIndicatorArrowNode above the last object when it is added to the play area. if an object was
// moved before this happens, don't show the dragIndicatorArrowNode
- if ( model.soccerBallGroup.countProperty.value === this.model.physicalRange.max &&
+ if ( model.soccerBallGroupCountProperty.value === this.model.physicalRange.max &&
objectNodesInputEnabledProperty.value &&
- _.every( model.soccerBallGroup.getArray(), cavObject => cavObject.valueProperty.value !== null ) &&
+ _.every( model.soccerBallGroup, cavObject => cavObject.valueProperty.value !== null ) &&
!objectHasBeenDragged ) {
dragIndicatorArrowNode.centerX = this.modelViewTransform.modelToViewX( value );
@@ -138,7 +120,7 @@
// calculate where the top object is
const topObjectPositionY = this.modelViewTransform.modelToViewY( 0 ) -
- ( model.getOtherObjectsAtTarget( cavObject ).length + 1 ) *
+ ( model.getOtherObjectsAtTarget( soccerBall ).length + 1 ) *
Math.abs( this.modelViewTransform.modelToViewDeltaY( CAVObjectType.SOCCER_BALL.radius ) ) * 2 -
dragIndicatorArrowNodeMargin;
@@ -153,16 +135,20 @@
}
} );
- map.set( cavObject, cavObjectNode );
- };
- model.soccerBallGroup.forEach( createObjectNode );
- model.soccerBallGroup.elementCreatedEmitter.addListener( createObjectNode );
+ map.set( soccerBall, soccerBallNode );
- model.soccerBallGroup.elementDisposedEmitter.addListener( cavObject => {
- const viewNode = map.get( cavObject )!;
- objectNodeGroup.disposeElement( viewNode );
- map.delete( cavObject );
+ return soccerBallNode;
} );
+
+ this.addChild( this.contentLayer );
+ this.addChild( this.frontObjectLayer );
+
+ let objectHasBeenDragged = false;
+ const dragIndicatorArrowNode = new DragIndicatorArrowNode( {
+ tandem: options.tandem.createTandem( 'dragIndicatorArrowNode' ),
+ visible: false
+ } );
+ this.backObjectLayer.addChild( dragIndicatorArrowNode );
this.playAreaMedianIndicatorNode = new PlayAreaMedianIndicatorNode();
this.addChild( this.playAreaMedianIndicatorNode );
Index: js/median/model/MedianModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/model/MedianModel.ts b/js/median/model/MedianModel.ts
--- a/js/median/model/MedianModel.ts (revision 32162e745fe214576b7c1f962c255b73507fd169)
+++ b/js/median/model/MedianModel.ts (date 1682608170155)
@@ -29,7 +29,7 @@
return new CardModel( cavObject, {
tandem: tandem
} );
- }, () => [ this.soccerBallGroup.archetype ], {
+ }, () => [ this.soccerBallGroup[ 0 ] ], {
phetioType: PhetioGroup.PhetioGroupIO( CardModel.CardModelIO ),
tandem: options.tandem.createTandem( 'cardModelGroup' )
} );
Index: js/common/model/CAVModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CAVModel.ts b/js/common/model/CAVModel.ts
--- a/js/common/model/CAVModel.ts (revision 32162e745fe214576b7c1f962c255b73507fd169)
+++ b/js/common/model/CAVModel.ts (date 1682608212923)
@@ -8,11 +8,8 @@
*/
import centerAndVariability from '../../centerAndVariability.js';
-import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
-import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js';
-import PhetioGroup from '../../../../tandem/js/PhetioGroup.js';
import Tandem from '../../../../tandem/js/Tandem.js';
-import CAVObject, { CAVObjectOptions } from './CAVObject.js';
+import CAVObject from './CAVObject.js';
import CAVObjectType from './CAVObjectType.js';
import Vector2 from '../../../../dot/js/Vector2.js';
import Range from '../../../../dot/js/Range.js';
@@ -47,7 +44,14 @@
const TIME_BETWEEN_RAPID_KICKS = 0.5; // in seconds
export default class CAVModel implements TModel {
- public readonly soccerBallGroup: PhetioGroup<CAVObject, [ StrictOmit<CAVObjectOptions, 'tandem'> ]>;
+ public readonly soccerBallGroup: CAVObject[];
+ public readonly soccerBallGroupElementCreatedEmitter = new Emitter<[ CAVObject ]>( {
+ parameters: [ { valueType: CAVObject } ]
+ } );
+ public readonly soccerBallGroupElementDisposedEmitter = new Emitter<[ CAVObject ]>(
+ { parameters: [ { valueType: CAVObject } ] }
+ );
+ public readonly soccerBallGroupCountProperty: NumberProperty;
public readonly isShowingTopMeanProperty: BooleanProperty;
public readonly isShowingTopMedianProperty: BooleanProperty;
@@ -101,16 +105,20 @@
public constructor( options: CAVModelOptions ) {
- this.soccerBallGroup = new PhetioGroup( ( tandem, providedOptions: StrictOmit<CAVObjectOptions, 'tandem'> ) => {
+ this.soccerBallGroupCountProperty = new NumberProperty( 0, {
+ range: new Range( 0, this.maxNumberOfObjects )
+ } );
- const options = optionize<StrictOmit<CAVObjectOptions, 'tandem'>, EmptySelfOptions, CAVObjectOptions>()( {
- // If it's the first element in the group, mark as isFirstObject. For creating archetype, the objectGroup does
- // not yet exist, so just mark it as first
- isFirstObject: this.soccerBallGroup ? this.soccerBallGroup.count === 0 : true,
- tandem: tandem
- }, providedOptions );
+ this.soccerBallGroup = _.range( 0, this.maxNumberOfObjects ).map( index => {
- const soccerBall = new CAVObject( CAVObjectType.SOCCER_BALL, options );
+ const y0 = CAVObjectType.SOCCER_BALL.radius;
+ const position = new Vector2( 0, y0 );
+
+ const soccerBall = new CAVObject( CAVObjectType.SOCCER_BALL, {
+ isFirstObject: index === 0,
+ tandem: options.tandem.createTandem( `soccerBall${index}` ),
+ position: position
+ } );
// TODO: Should some or all of this move into CAVObject or CAVObjectNode?
const dragPositionListener = ( dragPosition: Vector2 ) => {
@@ -119,12 +127,31 @@
this.moveToTop( soccerBall );
};
soccerBall.dragPositionProperty.lazyLink( dragPositionListener );
- soccerBall.disposedEmitter.addListener( () => soccerBall.dragPositionProperty.unlink( dragPositionListener ) );
+
+ soccerBall.valueProperty.link( ( value, oldValue ) => {
+ if ( value !== null && oldValue === null ) {
+ if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
+ this.soccerBallLandedListener( soccerBall, value );
+ }
+ }
+ } );
+
+ soccerBall.isActiveProperty.link( isActive => {
+ if ( isActive ) {
+ this.soccerBallGroupElementCreatedEmitter.emit( soccerBall );
+ }
+ else {
+ this.soccerBallGroupElementDisposedEmitter.emit( soccerBall );
+ }
+ } );
return soccerBall;
- }, [ {} ], {
- phetioType: PhetioGroup.PhetioGroupIO( CAVObject.CAVObjectIO ),
- tandem: options.tandem.createTandem( 'soccerBallGroup' )
+ } );
+
+ this.soccerBallGroup.forEach( soccerBall => {
+ soccerBall.isActiveProperty.link( isActive => {
+ this.soccerBallGroupCountProperty.value = this.soccerBallGroup.filter( soccerBall => soccerBall.isActiveProperty.value ).length;
+ } );
} );
this.isShowingTopMeanProperty = new BooleanProperty( false, {
@@ -230,9 +257,10 @@
soccerBall.positionProperty.link( () => this.objectChangedEmitter.emit( soccerBall ) );
};
this.soccerBallGroup.forEach( objectCreatedListener );
- this.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
+ // this.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
- this.numberOfRemainingObjectsProperty = new DerivedProperty( [ this.soccerBallGroup.countProperty ], count => {
+ this.numberOfRemainingObjectsProperty = DerivedProperty.deriveAny( this.soccerBallGroup.map( ball => ball.isActiveProperty ), () => {
+ const count = this.soccerBallGroup.filter( ball => ball.isActiveProperty.value ).length;
return this.maxNumberOfObjects - count;
} );
@@ -250,7 +278,7 @@
this.soccerPlayers = _.range( 0, this.maxNumberOfObjects ).map( placeInLine => new SoccerPlayer( placeInLine ) );
// Create an initial ball to show on startup
- this.nextBallToKickProperty = new Property<CAVObject | null>( this.createBall(), {
+ this.nextBallToKickProperty = new Property<CAVObject | null>( this.soccerBallGroup[ 0 ], {
tandem: options.tandem.createTandem( 'nextBallToKickProperty' ),
phetioValueType: NullableIO( ReferenceIO( CAVObject.CAVObjectIO ) )
} );
@@ -283,18 +311,8 @@
this.advanceLine();
if ( this.numberOfRemainingObjectsProperty.value > 0 && this.nextBallToKickProperty.value === null ) {
- this.nextBallToKickProperty.value = this.createBall();
+ this.nextBallToKickProperty.value = this.getFirstInactiveBall();
}
- } );
-
- this.soccerBallGroup.elementCreatedEmitter.addListener( soccerBall => {
- soccerBall.valueProperty.link( ( value, oldValue ) => {
- if ( value !== null && oldValue === null ) {
- if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
- this.soccerBallLandedListener( soccerBall, value );
- }
- }
- } );
} );
this.activeKickerIndexProperty = new NumberProperty( 0, {
@@ -391,15 +409,17 @@
* Clears out the data and the cards
*/
public clearData(): void {
- this.soccerBallGroup.clear();
+ // this.soccerBallGroup.clear();
this.numberOfScheduledSoccerBallsToKickProperty.reset();
this.timeProperty.reset();
this.timeWhenLastBallWasKickedProperty.reset();
- this.nextBallToKickProperty.value = this.createBall();
this.activeKickerIndexProperty.reset();
this.soccerPlayers.forEach( soccerPlayer => soccerPlayer.reset() );
+ this.soccerBallGroup.forEach( soccerBall => soccerBall.reset() );
+
+ this.nextBallToKickProperty.value = this.soccerBallGroup[ 0 ];
}
/**
@@ -455,7 +475,7 @@
// Create the next ball.
// TODO-UX-HIGH: A ball is being created too soon, when using the multikick button
- this.nextBallToKickProperty.value = this.createBall();
+ this.nextBallToKickProperty.value = this.getFirstInactiveBall();
}
assert && assert( this.nextBallToKickProperty.value !== null, 'there was no ball to kick' );
@@ -514,19 +534,6 @@
return dotRandom.nextBoolean() ? CAVConstants.LEFT_SKEWED_DATA : CAVConstants.RIGHT_SKEWED_DATA;
}
- /**
- * Creates a ball at the starting kick position.
- */
- private createBall(): CAVObject {
-
- const y0 = CAVObjectType.SOCCER_BALL.radius;
- const position = new Vector2( 0, y0 );
-
- return this.soccerBallGroup.createNextElement( {
- position: position
- } );
- }
-
/**
* When a ball lands on the ground, animate all other balls that were at this location above the landed ball.
*/
@@ -616,6 +623,10 @@
// New ball will be created later in step
this.nextBallToKickProperty.value = null;
}
+
+ private getFirstInactiveBall(): CAVObject | null {
+ return this.soccerBallGroup.find( ball => !ball.isActiveProperty.value ) || null;
+ }
}
centerAndVariability.register( 'CAVModel', CAVModel );
\ No newline at end of file
Index: js/common/view/CardNodeContainer.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CardNodeContainer.ts b/js/common/view/CardNodeContainer.ts
--- a/js/common/view/CardNodeContainer.ts (revision 32162e745fe214576b7c1f962c255b73507fd169)
+++ b/js/common/view/CardNodeContainer.ts (date 1682608212919)
@@ -49,6 +49,7 @@
type SelfOptions = EmptySelfOptions;
export type CardNodeContainerOptions = SelfOptions & NodeOptions & PickRequired<NodeOptions, 'tandem'>;
+// TODO: Move to median/
export default class CardNodeContainer extends Node {
// Each card is associated with one "cell", no two cards can be associated with the same cell. The leftmost cell is 0.
@@ -122,8 +123,8 @@
} );
};
- model.soccerBallGroup.forEach( objectCreatedListener );
- model.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
+ model.soccerBallGroup.filter( soccerBall => soccerBall.isActiveProperty.value ).forEach( objectCreatedListener );
+ model.soccerBallGroupElementCreatedEmitter.addListener( objectCreatedListener );
model.cardModelGroup.elementDisposedEmitter.addListener( cardModel => {
const cardNode = this.getCardNode( cardModel.cavObject )!;
Index: js/variability/view/IQRNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/IQRNode.ts b/js/variability/view/IQRNode.ts
--- a/js/variability/view/IQRNode.ts (revision 32162e745fe214576b7c1f962c255b73507fd169)
+++ b/js/variability/view/IQRNode.ts (date 1682608170157)
@@ -100,7 +100,7 @@
const updateIQRNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( object => object.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
const rightmostDot = sortedDots[ sortedDots.length - 1 ];
|
|
This patch is even better: Subject: [PATCH] Update TODOs, see https://github.com/phetsims/center-and-variability/issues/45
---
Index: js/mean-and-median/view/MedianPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/mean-and-median/view/MedianPlotNode.ts b/js/mean-and-median/view/MedianPlotNode.ts
--- a/js/mean-and-median/view/MedianPlotNode.ts (revision 33b2eb0618046788176b74e20ca697c2ee5a91f9)
+++ b/js/mean-and-median/view/MedianPlotNode.ts (date 1682608170153)
@@ -40,7 +40,7 @@
const updateMedianBarNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
Index: js/common/model/CAVObject.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CAVObject.ts b/js/common/model/CAVObject.ts
--- a/js/common/model/CAVObject.ts (revision 33b2eb0618046788176b74e20ca697c2ee5a91f9)
+++ b/js/common/model/CAVObject.ts (date 1682608735516)
@@ -78,6 +78,7 @@
} );
public readonly dragStartedEmitter: TEmitter = new Emitter();
public animation: Animation | null = null;
+ public readonly isActiveProperty: BooleanProperty;
public constructor( objectType: CAVObjectType, providedOptions: CAVObjectOptions ) {
@@ -118,6 +119,9 @@
this.isShowingAnimationHighlightProperty = new BooleanProperty( false, {
tandem: options.tandem.createTandem( 'isShowingAnimationHighlightProperty' )
} );
+ this.isActiveProperty = new BooleanProperty( false, {
+ tandem: options.tandem.createTandem( 'isActiveProperty' )
+ } );
}
public step( dt: number ): void {
@@ -151,17 +155,14 @@
}
}
- public override dispose(): void {
- super.dispose();
- this.positionProperty.dispose();
- this.velocityProperty.dispose();
- this.animationModeProperty.dispose();
- this.valueProperty.dispose();
- this.dragPositionProperty.dispose();
- this.isMedianObjectProperty.dispose();
- this.isShowingAnimationHighlightProperty.dispose();
- this.disposedEmitter.emit();
- this.disposedEmitter.dispose();
+ public reset(): void {
+ this.positionProperty.reset();
+ this.velocityProperty.reset();
+ this.animationModeProperty.reset();
+ this.valueProperty.reset();
+ this.dragPositionProperty.reset();
+ this.isMedianObjectProperty.reset();
+ this.isShowingAnimationHighlightProperty.reset();
}
public toStateObject(): CAVObjectStateType {
Index: js/variability/view/RangeNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/RangeNode.ts b/js/variability/view/RangeNode.ts
--- a/js/variability/view/RangeNode.ts (revision 33b2eb0618046788176b74e20ca697c2ee5a91f9)
+++ b/js/variability/view/RangeNode.ts (date 1682608170160)
@@ -65,7 +65,7 @@
const updateRangeNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
const rightmostDot = sortedDots[ sortedDots.length - 1 ];
Index: js/variability/view/MADNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/MADNode.ts b/js/variability/view/MADNode.ts
--- a/js/variability/view/MADNode.ts (revision 33b2eb0618046788176b74e20ca697c2ee5a91f9)
+++ b/js/variability/view/MADNode.ts (date 1682608170159)
@@ -76,7 +76,7 @@
const children: Node[] = [];
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
if ( sortedDots.length > 0 ) {
Index: js/common/view/CAVPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVPlotNode.ts b/js/common/view/CAVPlotNode.ts
--- a/js/common/view/CAVPlotNode.ts (revision 33b2eb0618046788176b74e20ca697c2ee5a91f9)
+++ b/js/common/view/CAVPlotNode.ts (date 1682608391942)
@@ -83,26 +83,18 @@
backgroundNode.addChild( this.dotLayer );
+ const map = new Map<CAVObject, CAVObjectNode>();
+
// TODO: This overlaps with draggingEnabled
const dotPlotObjectNodesDraggableProperty = new BooleanProperty( false );
+ const dotNodeGroup = model.soccerBallGroup.map( cavObject => {
- const dotNodeGroup = new PhetioGroup<CAVObjectNode, [ CAVObject ]>( ( tandem, cavObject ) => {
- return new CAVObjectNode( cavObject, model.isShowingTopMedianProperty, modelViewTransform, dotPlotObjectNodesDraggableProperty, {
+ const dotNode = new CAVObjectNode( cavObject, model.isShowingTopMedianProperty, modelViewTransform, dotPlotObjectNodesDraggableProperty, {
objectViewType: CAVObjectType.DATA_POINT,
draggingEnabled: false,
- tandem: tandem,
+ tandem: options.tandem.createTandem( 'dotNodeGroup' ).createTandem( cavObject.tandem.name ),
fill: options.dataPointFill
} );
- }, () => [ model.soccerBallGroup.archetype ], {
- phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ),
- tandem: options.tandem.createTandem( 'dotNodeGroup' ),
- supportsDynamicState: false
- } );
-
- const map = new Map<CAVObject, CAVObjectNode>();
-
- const createDotNode = ( cavObject: CAVObject ) => {
- const dotNode = dotNodeGroup.createCorrespondingGroupElement( cavObject.tandem.name, cavObject );
cavObject.valueProperty.link( value => {
dotNode.setVisible( value !== null );
@@ -111,14 +103,7 @@
}
} );
map.set( cavObject, dotNode );
- };
- model.soccerBallGroup.forEach( createDotNode );
- model.soccerBallGroup.elementCreatedEmitter.addListener( createDotNode );
-
- model.soccerBallGroup.elementDisposedEmitter.addListener( cavObject => {
- const viewNode = map.get( cavObject )!;
- dotNodeGroup.disposeElement( viewNode );
- map.delete( cavObject );
+ return dotNode;
} );
}
Index: js/common/view/CAVScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts
--- a/js/common/view/CAVScreenView.ts (revision 33b2eb0618046788176b74e20ca697c2ee5a91f9)
+++ b/js/common/view/CAVScreenView.ts (date 1682608492311)
@@ -14,7 +14,6 @@
import CAVConstants from '../CAVConstants.js';
import ResetAllButton from '../../../../scenery-phet/js/buttons/ResetAllButton.js';
import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js';
-import PhetioGroup from '../../../../tandem/js/PhetioGroup.js';
import CAVObjectNode from './CAVObjectNode.js';
import { AlignBox, ManualConstraint, Node } from '../../../../scenery/js/imports.js';
import CAVObjectType from '../model/CAVObjectType.js';
@@ -92,45 +91,28 @@
tandem: objectNodeGroupTandem.createTandem( 'inputEnabledProperty' )
} );
- const objectNodeGroup = new PhetioGroup<CAVObjectNode, [ CAVObject ]>( ( tandem, cavObject ) => {
- return new CAVObjectNode( cavObject, model.isShowingPlayAreaMedianProperty, modelViewTransform, objectNodesInputEnabledProperty, {
- fill: null, // Only depict as a soccer ball
- tandem: tandem
- } );
- }, () => [ model.soccerBallGroup.archetype ], {
- phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ),
- tandem: objectNodeGroupTandem,
- supportsDynamicState: false
- } );
-
- this.addChild( this.contentLayer );
- this.addChild( this.frontObjectLayer );
-
const map = new Map<CAVObject, CAVObjectNode>();
- let objectHasBeenDragged = false;
- const dragIndicatorArrowNode = new DragIndicatorArrowNode( {
- tandem: options.tandem.createTandem( 'dragIndicatorArrowNode' ),
- visible: false
- } );
- this.backObjectLayer.addChild( dragIndicatorArrowNode );
+ const soccerBallNodes = model.soccerBallGroup.map( ( soccerBall, index ) => {
+ const soccerBallNode = new CAVObjectNode( soccerBall, model.isShowingPlayAreaMedianProperty, modelViewTransform, objectNodesInputEnabledProperty, {
+ fill: null, // Only depict as a soccer ball
+ tandem: options.tandem.createTandem( 'soccerBallGroup' ).createTandem( 'soccerBallNode' + index )
+ } );
- const createObjectNode = ( cavObject: CAVObject ) => {
- const cavObjectNode = objectNodeGroup.createCorrespondingGroupElement( cavObject.tandem.name, cavObject );
- this.frontObjectLayer.addChild( cavObjectNode );
+ this.frontObjectLayer.addChild( soccerBallNode );
- cavObject.valueProperty.lazyLink( ( value, oldValue ) => {
+ soccerBall.valueProperty.lazyLink( ( value, oldValue ) => {
if ( value !== null ) {
if ( oldValue === null ) {
- assert && assert( this.frontObjectLayer.hasChild( cavObjectNode ) );
- this.frontObjectLayer.removeChild( cavObjectNode );
- this.backObjectLayer.addChild( cavObjectNode );
+ assert && assert( this.frontObjectLayer.hasChild( soccerBallNode ) );
+ this.frontObjectLayer.removeChild( soccerBallNode );
+ this.backObjectLayer.addChild( soccerBallNode );
// add the dragIndicatorArrowNode above the last object when it is added to the play area. if an object was
// moved before this happens, don't show the dragIndicatorArrowNode
- if ( model.soccerBallGroup.countProperty.value === this.model.physicalRange.max &&
+ if ( model.soccerBallGroupCountProperty.value === this.model.physicalRange.max &&
objectNodesInputEnabledProperty.value &&
- _.every( model.soccerBallGroup.getArray(), cavObject => cavObject.valueProperty.value !== null ) &&
+ _.every( model.soccerBallGroup, cavObject => cavObject.valueProperty.value !== null ) &&
!objectHasBeenDragged ) {
dragIndicatorArrowNode.centerX = this.modelViewTransform.modelToViewX( value );
@@ -138,7 +120,7 @@
// calculate where the top object is
const topObjectPositionY = this.modelViewTransform.modelToViewY( 0 ) -
- ( model.getOtherObjectsAtTarget( cavObject ).length + 1 ) *
+ ( model.getOtherObjectsAtTarget( soccerBall ).length + 1 ) *
Math.abs( this.modelViewTransform.modelToViewDeltaY( CAVObjectType.SOCCER_BALL.radius ) ) * 2 -
dragIndicatorArrowNodeMargin;
@@ -153,16 +135,20 @@
}
} );
- map.set( cavObject, cavObjectNode );
- };
- model.soccerBallGroup.forEach( createObjectNode );
- model.soccerBallGroup.elementCreatedEmitter.addListener( createObjectNode );
+ map.set( soccerBall, soccerBallNode );
- model.soccerBallGroup.elementDisposedEmitter.addListener( cavObject => {
- const viewNode = map.get( cavObject )!;
- objectNodeGroup.disposeElement( viewNode );
- map.delete( cavObject );
+ return soccerBallNode;
} );
+
+ this.addChild( this.contentLayer );
+ this.addChild( this.frontObjectLayer );
+
+ let objectHasBeenDragged = false;
+ const dragIndicatorArrowNode = new DragIndicatorArrowNode( {
+ tandem: options.tandem.createTandem( 'dragIndicatorArrowNode' ),
+ visible: false
+ } );
+ this.backObjectLayer.addChild( dragIndicatorArrowNode );
this.playAreaMedianIndicatorNode = new PlayAreaMedianIndicatorNode();
this.addChild( this.playAreaMedianIndicatorNode );
Index: js/median/model/MedianModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/model/MedianModel.ts b/js/median/model/MedianModel.ts
--- a/js/median/model/MedianModel.ts (revision 33b2eb0618046788176b74e20ca697c2ee5a91f9)
+++ b/js/median/model/MedianModel.ts (date 1682608170155)
@@ -29,7 +29,7 @@
return new CardModel( cavObject, {
tandem: tandem
} );
- }, () => [ this.soccerBallGroup.archetype ], {
+ }, () => [ this.soccerBallGroup[ 0 ] ], {
phetioType: PhetioGroup.PhetioGroupIO( CardModel.CardModelIO ),
tandem: options.tandem.createTandem( 'cardModelGroup' )
} );
Index: js/common/model/CAVModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CAVModel.ts b/js/common/model/CAVModel.ts
--- a/js/common/model/CAVModel.ts (revision 33b2eb0618046788176b74e20ca697c2ee5a91f9)
+++ b/js/common/model/CAVModel.ts (date 1682610980375)
@@ -8,11 +8,8 @@
*/
import centerAndVariability from '../../centerAndVariability.js';
-import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
-import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js';
-import PhetioGroup from '../../../../tandem/js/PhetioGroup.js';
import Tandem from '../../../../tandem/js/Tandem.js';
-import CAVObject, { CAVObjectOptions } from './CAVObject.js';
+import CAVObject from './CAVObject.js';
import CAVObjectType from './CAVObjectType.js';
import Vector2 from '../../../../dot/js/Vector2.js';
import Range from '../../../../dot/js/Range.js';
@@ -47,7 +44,8 @@
const TIME_BETWEEN_RAPID_KICKS = 0.5; // in seconds
export default class CAVModel implements TModel {
- public readonly soccerBallGroup: PhetioGroup<CAVObject, [ StrictOmit<CAVObjectOptions, 'tandem'> ]>;
+ public readonly soccerBallGroup: CAVObject[];
+ public readonly soccerBallGroupCountProperty: NumberProperty;
public readonly isShowingTopMeanProperty: BooleanProperty;
public readonly isShowingTopMedianProperty: BooleanProperty;
@@ -101,16 +99,21 @@
public constructor( options: CAVModelOptions ) {
- this.soccerBallGroup = new PhetioGroup( ( tandem, providedOptions: StrictOmit<CAVObjectOptions, 'tandem'> ) => {
+ this.soccerBallGroupCountProperty = new NumberProperty( 0, {
+ range: new Range( 0, this.maxNumberOfObjects )
+ } );
- const options = optionize<StrictOmit<CAVObjectOptions, 'tandem'>, EmptySelfOptions, CAVObjectOptions>()( {
- // If it's the first element in the group, mark as isFirstObject. For creating archetype, the objectGroup does
- // not yet exist, so just mark it as first
- isFirstObject: this.soccerBallGroup ? this.soccerBallGroup.count === 0 : true,
- tandem: tandem
- }, providedOptions );
+ this.soccerBallGroup = _.range( 0, this.maxNumberOfObjects ).map( index => {
- const soccerBall = new CAVObject( CAVObjectType.SOCCER_BALL, options );
+ const y0 = CAVObjectType.SOCCER_BALL.radius;
+ const position = new Vector2( 0, y0 );
+
+ const soccerBall = new CAVObject( CAVObjectType.SOCCER_BALL, {
+ isFirstObject: index === 0,
+ tandem: options.tandem.createTandem( `soccerBall${index}` ),
+ position: position
+ } );
+ soccerBall.isActiveProperty.value = index === 0;
// TODO: Should some or all of this move into CAVObject or CAVObjectNode?
const dragPositionListener = ( dragPosition: Vector2 ) => {
@@ -119,12 +122,22 @@
this.moveToTop( soccerBall );
};
soccerBall.dragPositionProperty.lazyLink( dragPositionListener );
- soccerBall.disposedEmitter.addListener( () => soccerBall.dragPositionProperty.unlink( dragPositionListener ) );
+
+ soccerBall.valueProperty.link( ( value, oldValue ) => {
+ if ( value !== null && oldValue === null ) {
+ if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
+ this.soccerBallLandedListener( soccerBall, value );
+ }
+ }
+ } );
return soccerBall;
- }, [ {} ], {
- phetioType: PhetioGroup.PhetioGroupIO( CAVObject.CAVObjectIO ),
- tandem: options.tandem.createTandem( 'soccerBallGroup' )
+ } );
+
+ this.soccerBallGroup.forEach( soccerBall => {
+ soccerBall.isActiveProperty.link( isActive => {
+ this.soccerBallGroupCountProperty.value = this.soccerBallGroup.filter( soccerBall => soccerBall.isActiveProperty.value ).length;
+ } );
} );
this.isShowingTopMeanProperty = new BooleanProperty( false, {
@@ -230,9 +243,10 @@
soccerBall.positionProperty.link( () => this.objectChangedEmitter.emit( soccerBall ) );
};
this.soccerBallGroup.forEach( objectCreatedListener );
- this.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
+ // this.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
- this.numberOfRemainingObjectsProperty = new DerivedProperty( [ this.soccerBallGroup.countProperty ], count => {
+ this.numberOfRemainingObjectsProperty = DerivedProperty.deriveAny( this.soccerBallGroup.map( ball => ball.isActiveProperty ), () => {
+ const count = this.soccerBallGroup.filter( ball => ball.isActiveProperty.value ).length;
return this.maxNumberOfObjects - count;
} );
@@ -250,11 +264,12 @@
this.soccerPlayers = _.range( 0, this.maxNumberOfObjects ).map( placeInLine => new SoccerPlayer( placeInLine ) );
// Create an initial ball to show on startup
- this.nextBallToKickProperty = new Property<CAVObject | null>( this.createBall(), {
+ this.nextBallToKickProperty = new Property<CAVObject | null>( this.soccerBallGroup[ 0 ], {
tandem: options.tandem.createTandem( 'nextBallToKickProperty' ),
phetioValueType: NullableIO( ReferenceIO( CAVObject.CAVObjectIO ) )
} );
+ // TODO: Simplify now that we have pools
this.numberOfRemainingKickableSoccerBallsProperty = new DerivedProperty( [
this.numberOfRemainingObjectsProperty,
this.numberOfScheduledSoccerBallsToKickProperty,
@@ -283,18 +298,10 @@
this.advanceLine();
if ( this.numberOfRemainingObjectsProperty.value > 0 && this.nextBallToKickProperty.value === null ) {
- this.nextBallToKickProperty.value = this.createBall();
+ const nextBall = this.getNextBallFromPool();
+ this.nextBallToKickProperty.value = nextBall;
+ nextBall!.isActiveProperty.value = true;
}
- } );
-
- this.soccerBallGroup.elementCreatedEmitter.addListener( soccerBall => {
- soccerBall.valueProperty.link( ( value, oldValue ) => {
- if ( value !== null && oldValue === null ) {
- if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
- this.soccerBallLandedListener( soccerBall, value );
- }
- }
- } );
} );
this.activeKickerIndexProperty = new NumberProperty( 0, {
@@ -391,15 +398,17 @@
* Clears out the data and the cards
*/
public clearData(): void {
- this.soccerBallGroup.clear();
+ // this.soccerBallGroup.clear();
this.numberOfScheduledSoccerBallsToKickProperty.reset();
this.timeProperty.reset();
this.timeWhenLastBallWasKickedProperty.reset();
- this.nextBallToKickProperty.value = this.createBall();
this.activeKickerIndexProperty.reset();
this.soccerPlayers.forEach( soccerPlayer => soccerPlayer.reset() );
+ this.soccerBallGroup.forEach( soccerBall => soccerBall.reset() );
+
+ this.nextBallToKickProperty.value = this.soccerBallGroup[ 0 ];
}
/**
@@ -455,7 +464,7 @@
// Create the next ball.
// TODO-UX-HIGH: A ball is being created too soon, when using the multikick button
- this.nextBallToKickProperty.value = this.createBall();
+ this.nextBallToKickProperty.value = this.getNextBallFromPool();
}
assert && assert( this.nextBallToKickProperty.value !== null, 'there was no ball to kick' );
@@ -472,8 +481,8 @@
const elapsedTime = this.timeProperty.value - frontPlayer.timestampWhenPoisedBegan!;
if ( elapsedTime > 0.075 ) {
- const cavObject = this.nextBallToKickProperty.value!; // TODO: Probably? See https://github.com/phetsims/center-and-variability/issues/59
- this.kickBall( frontPlayer, cavObject );
+ const soccerBall = this.nextBallToKickProperty.value!; // TODO: Probably? See https://github.com/phetsims/center-and-variability/issues/59
+ this.kickBall( frontPlayer, soccerBall );
this.numberOfScheduledSoccerBallsToKickProperty.value--;
}
}
@@ -514,24 +523,15 @@
return dotRandom.nextBoolean() ? CAVConstants.LEFT_SKEWED_DATA : CAVConstants.RIGHT_SKEWED_DATA;
}
- /**
- * Creates a ball at the starting kick position.
- */
- private createBall(): CAVObject {
-
- const y0 = CAVObjectType.SOCCER_BALL.radius;
- const position = new Vector2( 0, y0 );
-
- return this.soccerBallGroup.createNextElement( {
- position: position
- } );
+ public getActiveSoccerBalls(): CAVObject[] {
+ return this.soccerBallGroup.filter( soccerBall => soccerBall.isActiveProperty.value );
}
/**
* When a ball lands on the ground, animate all other balls that were at this location above the landed ball.
*/
private soccerBallLandedListener( cavObject: CAVObject, value: number ): void {
- const otherObjectsInStack = this.soccerBallGroup.filter( x => x.valueProperty.value === value && x !== cavObject );
+ const otherObjectsInStack = this.getActiveSoccerBalls().filter( x => x.valueProperty.value === value && x !== cavObject );
const sortedOthers = _.sortBy( otherObjectsInStack, object => object.positionProperty.value.y );
sortedOthers.forEach( ( cavObject, index ) => {
@@ -616,6 +616,10 @@
// New ball will be created later in step
this.nextBallToKickProperty.value = null;
}
+
+ private getNextBallFromPool(): CAVObject | null {
+ return this.soccerBallGroup.find( ball => !ball.isActiveProperty.value ) || null;
+ }
}
centerAndVariability.register( 'CAVModel', CAVModel );
\ No newline at end of file
Index: js/common/view/CardNodeContainer.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CardNodeContainer.ts b/js/common/view/CardNodeContainer.ts
--- a/js/common/view/CardNodeContainer.ts (revision 33b2eb0618046788176b74e20ca697c2ee5a91f9)
+++ b/js/common/view/CardNodeContainer.ts (date 1682608735511)
@@ -49,6 +49,7 @@
type SelfOptions = EmptySelfOptions;
export type CardNodeContainerOptions = SelfOptions & NodeOptions & PickRequired<NodeOptions, 'tandem'>;
+// TODO: Move to median/
export default class CardNodeContainer extends Node {
// Each card is associated with one "cell", no two cards can be associated with the same cell. The leftmost cell is 0.
@@ -122,8 +123,7 @@
} );
};
- model.soccerBallGroup.forEach( objectCreatedListener );
- model.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
+ model.soccerBallGroup.filter( soccerBall => soccerBall.isActiveProperty.value ).forEach( objectCreatedListener );
model.cardModelGroup.elementDisposedEmitter.addListener( cardModel => {
const cardNode = this.getCardNode( cardModel.cavObject )!;
Index: js/variability/view/IQRNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/IQRNode.ts b/js/variability/view/IQRNode.ts
--- a/js/variability/view/IQRNode.ts (revision 33b2eb0618046788176b74e20ca697c2ee5a91f9)
+++ b/js/variability/view/IQRNode.ts (date 1682608170157)
@@ -100,7 +100,7 @@
const updateIQRNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( object => object.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
const rightmostDot = sortedDots[ sortedDots.length - 1 ];
|
|
TODO: Also move some of those "enhancements" ideas to new issues. |
|
Making more progress. Subject: [PATCH] Update TODOs, see https://github.com/phetsims/center-and-variability/issues/45
---
Index: js/mean-and-median/view/MedianPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/mean-and-median/view/MedianPlotNode.ts b/js/mean-and-median/view/MedianPlotNode.ts
--- a/js/mean-and-median/view/MedianPlotNode.ts (revision ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/mean-and-median/view/MedianPlotNode.ts (date 1682622783991)
@@ -51,7 +51,7 @@
const updateMedianBarNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.getActiveSoccerBalls().filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
Index: js/common/model/CAVObject.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CAVObject.ts b/js/common/model/CAVObject.ts
--- a/js/common/model/CAVObject.ts (revision ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/common/model/CAVObject.ts (date 1682622783989)
@@ -78,6 +78,7 @@
} );
public readonly dragStartedEmitter: TEmitter = new Emitter();
public animation: Animation | null = null;
+ public readonly isActiveProperty: BooleanProperty;
public constructor( objectType: CAVObjectType, providedOptions: CAVObjectOptions ) {
@@ -118,6 +119,9 @@
this.isShowingAnimationHighlightProperty = new BooleanProperty( false, {
tandem: options.tandem.createTandem( 'isShowingAnimationHighlightProperty' )
} );
+ this.isActiveProperty = new BooleanProperty( false, {
+ tandem: options.tandem.createTandem( 'isActiveProperty' )
+ } );
}
public step( dt: number ): void {
@@ -151,17 +155,14 @@
}
}
- public override dispose(): void {
- super.dispose();
- this.positionProperty.dispose();
- this.velocityProperty.dispose();
- this.animationModeProperty.dispose();
- this.valueProperty.dispose();
- this.dragPositionProperty.dispose();
- this.isMedianObjectProperty.dispose();
- this.isShowingAnimationHighlightProperty.dispose();
- this.disposedEmitter.emit();
- this.disposedEmitter.dispose();
+ public reset(): void {
+ this.positionProperty.reset();
+ this.velocityProperty.reset();
+ this.animationModeProperty.reset();
+ this.valueProperty.reset();
+ this.dragPositionProperty.reset();
+ this.isMedianObjectProperty.reset();
+ this.isShowingAnimationHighlightProperty.reset();
}
public toStateObject(): CAVObjectStateType {
Index: js/variability/view/RangeNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/RangeNode.ts b/js/variability/view/RangeNode.ts
--- a/js/variability/view/RangeNode.ts (revision ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/variability/view/RangeNode.ts (date 1682622783992)
@@ -92,7 +92,7 @@
const updateRangeNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
const rightmostDot = sortedDots[ sortedDots.length - 1 ];
Index: js/variability/view/MADNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/MADNode.ts b/js/variability/view/MADNode.ts
--- a/js/variability/view/MADNode.ts (revision ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/variability/view/MADNode.ts (date 1682622783991)
@@ -82,7 +82,7 @@
const children: Node[] = [];
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( soccerBall => soccerBall.valueProperty.value !== null ),
object => object.valueProperty.value );
if ( sortedDots.length > 0 ) {
Index: js/common/view/CAVPlotNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVPlotNode.ts b/js/common/view/CAVPlotNode.ts
--- a/js/common/view/CAVPlotNode.ts (revision ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/common/view/CAVPlotNode.ts (date 1682622783990)
@@ -83,26 +83,18 @@
backgroundNode.addChild( this.dotLayer );
+ const map = new Map<CAVObject, CAVObjectNode>();
+
// TODO: This overlaps with draggingEnabled
const dotPlotObjectNodesDraggableProperty = new BooleanProperty( false );
+ const dotNodeGroup = model.soccerBallGroup.map( cavObject => {
- const dotNodeGroup = new PhetioGroup<CAVObjectNode, [ CAVObject ]>( ( tandem, cavObject ) => {
- return new CAVObjectNode( cavObject, model.isShowingTopMedianProperty, modelViewTransform, dotPlotObjectNodesDraggableProperty, {
+ const dotNode = new CAVObjectNode( cavObject, model.isShowingTopMedianProperty, modelViewTransform, dotPlotObjectNodesDraggableProperty, {
objectViewType: CAVObjectType.DATA_POINT,
draggingEnabled: false,
- tandem: tandem,
+ tandem: options.tandem.createTandem( 'dotNodeGroup' ).createTandem( cavObject.tandem.name ),
fill: options.dataPointFill
} );
- }, () => [ model.soccerBallGroup.archetype ], {
- phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ),
- tandem: options.tandem.createTandem( 'dotNodeGroup' ),
- supportsDynamicState: false
- } );
-
- const map = new Map<CAVObject, CAVObjectNode>();
-
- const createDotNode = ( cavObject: CAVObject ) => {
- const dotNode = dotNodeGroup.createCorrespondingGroupElement( cavObject.tandem.name, cavObject );
cavObject.valueProperty.link( value => {
dotNode.setVisible( value !== null );
@@ -111,14 +103,7 @@
}
} );
map.set( cavObject, dotNode );
- };
- model.soccerBallGroup.forEach( createDotNode );
- model.soccerBallGroup.elementCreatedEmitter.addListener( createDotNode );
-
- model.soccerBallGroup.elementDisposedEmitter.addListener( cavObject => {
- const viewNode = map.get( cavObject )!;
- dotNodeGroup.disposeElement( viewNode );
- map.delete( cavObject );
+ return dotNode;
} );
}
Index: js/common/view/CAVScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts
--- a/js/common/view/CAVScreenView.ts (revision ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/common/view/CAVScreenView.ts (date 1682624159717)
@@ -14,7 +14,6 @@
import CAVConstants from '../CAVConstants.js';
import ResetAllButton from '../../../../scenery-phet/js/buttons/ResetAllButton.js';
import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js';
-import PhetioGroup from '../../../../tandem/js/PhetioGroup.js';
import CAVObjectNode from './CAVObjectNode.js';
import { AlignBox, ManualConstraint, Node } from '../../../../scenery/js/imports.js';
import CAVObjectType from '../model/CAVObjectType.js';
@@ -37,6 +36,7 @@
import PlayAreaMedianIndicatorNode from './PlayAreaMedianIndicatorNode.js';
import CAVAccordionBox from './CAVAccordionBox.js';
import VerticalCheckboxGroup, { VerticalCheckboxGroupItem } from '../../../../sun/js/VerticalCheckboxGroup.js';
+import { AnimationMode } from '../model/AnimationMode.js';
type SelfOptions = {
questionBarOptions: QuestionBarOptions;
@@ -92,45 +92,37 @@
tandem: objectNodeGroupTandem.createTandem( 'inputEnabledProperty' )
} );
- const objectNodeGroup = new PhetioGroup<CAVObjectNode, [ CAVObject ]>( ( tandem, cavObject ) => {
- return new CAVObjectNode( cavObject, model.isShowingPlayAreaMedianProperty, modelViewTransform, objectNodesInputEnabledProperty, {
- fill: null, // Only depict as a soccer ball
- tandem: tandem
- } );
- }, () => [ model.soccerBallGroup.archetype ], {
- phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ),
- tandem: objectNodeGroupTandem,
- supportsDynamicState: false
- } );
-
- this.addChild( this.contentLayer );
- this.addChild( this.frontObjectLayer );
-
const map = new Map<CAVObject, CAVObjectNode>();
- let objectHasBeenDragged = false;
- const dragIndicatorArrowNode = new DragIndicatorArrowNode( {
- tandem: options.tandem.createTandem( 'dragIndicatorArrowNode' ),
- visible: false
- } );
- this.backObjectLayer.addChild( dragIndicatorArrowNode );
+ const soccerBallNodes = model.soccerBallGroup.map( ( soccerBall, index ) => {
+ const soccerBallNode = new CAVObjectNode( soccerBall, model.isShowingPlayAreaMedianProperty, modelViewTransform, objectNodesInputEnabledProperty, {
+ fill: null, // Only depict as a soccer ball
+ tandem: options.tandem.createTandem( 'soccerBallGroup' ).createTandem( 'soccerBallNode' + index )
+ } );
+
+ this.backObjectLayer.addChild( soccerBallNode );
- const createObjectNode = ( cavObject: CAVObject ) => {
- const cavObjectNode = objectNodeGroup.createCorrespondingGroupElement( cavObject.tandem.name, cavObject );
- this.frontObjectLayer.addChild( cavObjectNode );
+ // While flying, it should be in front in z-order, to be in front of the accordion box
+ soccerBall.animationModeProperty.lazyLink( ( animationMode, oldAnimationModel ) => {
+ if ( animationMode === AnimationMode.FLYING ) {
+ this.backObjectLayer.removeChild( soccerBallNode );
+ this.frontObjectLayer.addChild( soccerBallNode );
+ }
+ else if ( oldAnimationModel ) {
+ this.frontObjectLayer.removeChild( soccerBallNode );
+ this.backObjectLayer.addChild( soccerBallNode );
+ }
+ } );
- cavObject.valueProperty.lazyLink( ( value, oldValue ) => {
+ soccerBall.valueProperty.lazyLink( ( value, oldValue ) => {
if ( value !== null ) {
if ( oldValue === null ) {
- assert && assert( this.frontObjectLayer.hasChild( cavObjectNode ) );
- this.frontObjectLayer.removeChild( cavObjectNode );
- this.backObjectLayer.addChild( cavObjectNode );
// add the dragIndicatorArrowNode above the last object when it is added to the play area. if an object was
// moved before this happens, don't show the dragIndicatorArrowNode
- if ( model.soccerBallGroup.countProperty.value === this.model.physicalRange.max &&
+ if ( model.soccerBallGroupCountProperty.value === this.model.physicalRange.max &&
objectNodesInputEnabledProperty.value &&
- _.every( model.soccerBallGroup.getArray(), cavObject => cavObject.valueProperty.value !== null ) &&
+ _.every( model.soccerBallGroup, cavObject => cavObject.valueProperty.value !== null ) &&
!objectHasBeenDragged ) {
dragIndicatorArrowNode.centerX = this.modelViewTransform.modelToViewX( value );
@@ -138,7 +130,7 @@
// calculate where the top object is
const topObjectPositionY = this.modelViewTransform.modelToViewY( 0 ) -
- ( model.getOtherObjectsAtTarget( cavObject ).length + 1 ) *
+ ( model.getOtherObjectsAtTarget( soccerBall ).length + 1 ) *
Math.abs( this.modelViewTransform.modelToViewDeltaY( CAVObjectType.SOCCER_BALL.radius ) ) * 2 -
dragIndicatorArrowNodeMargin;
@@ -153,16 +145,20 @@
}
} );
- map.set( cavObject, cavObjectNode );
- };
- model.soccerBallGroup.forEach( createObjectNode );
- model.soccerBallGroup.elementCreatedEmitter.addListener( createObjectNode );
+ map.set( soccerBall, soccerBallNode );
- model.soccerBallGroup.elementDisposedEmitter.addListener( cavObject => {
- const viewNode = map.get( cavObject )!;
- objectNodeGroup.disposeElement( viewNode );
- map.delete( cavObject );
+ return soccerBallNode;
} );
+
+ this.addChild( this.contentLayer );
+ this.addChild( this.frontObjectLayer );
+
+ let objectHasBeenDragged = false;
+ const dragIndicatorArrowNode = new DragIndicatorArrowNode( {
+ tandem: options.tandem.createTandem( 'dragIndicatorArrowNode' ),
+ visible: false
+ } );
+ this.backObjectLayer.addChild( dragIndicatorArrowNode );
this.playAreaMedianIndicatorNode = new PlayAreaMedianIndicatorNode();
this.addChild( this.playAreaMedianIndicatorNode );
Index: js/median/model/MedianModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/model/MedianModel.ts b/js/median/model/MedianModel.ts
--- a/js/median/model/MedianModel.ts (revision ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/median/model/MedianModel.ts (date 1682622783991)
@@ -29,7 +29,7 @@
return new CardModel( cavObject, {
tandem: tandem
} );
- }, () => [ this.soccerBallGroup.archetype ], {
+ }, () => [ this.soccerBallGroup[ 0 ] ], {
phetioType: PhetioGroup.PhetioGroupIO( CardModel.CardModelIO ),
tandem: options.tandem.createTandem( 'cardModelGroup' )
} );
Index: js/common/model/CAVModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CAVModel.ts b/js/common/model/CAVModel.ts
--- a/js/common/model/CAVModel.ts (revision ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/common/model/CAVModel.ts (date 1682624536220)
@@ -8,11 +8,8 @@
*/
import centerAndVariability from '../../centerAndVariability.js';
-import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
-import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js';
-import PhetioGroup from '../../../../tandem/js/PhetioGroup.js';
import Tandem from '../../../../tandem/js/Tandem.js';
-import CAVObject, { CAVObjectOptions } from './CAVObject.js';
+import CAVObject from './CAVObject.js';
import CAVObjectType from './CAVObjectType.js';
import Vector2 from '../../../../dot/js/Vector2.js';
import Range from '../../../../dot/js/Range.js';
@@ -47,7 +44,8 @@
const TIME_BETWEEN_RAPID_KICKS = 0.5; // in seconds
export default class CAVModel implements TModel {
- public readonly soccerBallGroup: PhetioGroup<CAVObject, [ StrictOmit<CAVObjectOptions, 'tandem'> ]>;
+ public readonly soccerBallGroup: CAVObject[];
+ public readonly soccerBallGroupCountProperty: NumberProperty;
public readonly isShowingTopMeanProperty: BooleanProperty;
public readonly isShowingTopMedianProperty: BooleanProperty;
@@ -59,7 +57,6 @@
protected readonly maxNumberOfObjects = CAVConstants.NUMBER_OF_OBJECTS;
public readonly physicalRange = new Range( 1, 15 );
- // This is the number that we can still add to the PhetioGroup
protected readonly numberOfRemainingObjectsProperty: TReadOnlyProperty<number>;
public readonly medianValueProperty: Property<number | null>;
public readonly meanValueProperty: Property<number | null>;
@@ -101,16 +98,24 @@
public constructor( options: CAVModelOptions ) {
- this.soccerBallGroup = new PhetioGroup( ( tandem, providedOptions: StrictOmit<CAVObjectOptions, 'tandem'> ) => {
+ const updateDataMeasures = () => this.updateDataMeasures();
+
+ // TODO: Rename to soccerBalls
+ this.soccerBallGroupCountProperty = new NumberProperty( 0, {
+ range: new Range( 0, this.maxNumberOfObjects )
+ } );
- const options = optionize<StrictOmit<CAVObjectOptions, 'tandem'>, EmptySelfOptions, CAVObjectOptions>()( {
- // If it's the first element in the group, mark as isFirstObject. For creating archetype, the objectGroup does
- // not yet exist, so just mark it as first
- isFirstObject: this.soccerBallGroup ? this.soccerBallGroup.count === 0 : true,
- tandem: tandem
- }, providedOptions );
+ this.soccerBallGroup = _.range( 0, this.maxNumberOfObjects ).map( index => {
- const soccerBall = new CAVObject( CAVObjectType.SOCCER_BALL, options );
+ const y0 = CAVObjectType.SOCCER_BALL.radius;
+ const position = new Vector2( 0, y0 );
+
+ const soccerBall = new CAVObject( CAVObjectType.SOCCER_BALL, {
+ isFirstObject: index === 0,
+ tandem: options.tandem.createTandem( `soccerBall${index}` ),
+ position: position
+ } );
+ soccerBall.isActiveProperty.value = index === 0;
// TODO: Should some or all of this move into CAVObject or CAVObjectNode?
const dragPositionListener = ( dragPosition: Vector2 ) => {
@@ -119,12 +124,38 @@
this.moveToTop( soccerBall );
};
soccerBall.dragPositionProperty.lazyLink( dragPositionListener );
- soccerBall.disposedEmitter.addListener( () => soccerBall.dragPositionProperty.unlink( dragPositionListener ) );
+
+ soccerBall.valueProperty.link( ( value, oldValue ) => {
+ if ( value !== null && oldValue === null ) {
+ if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
+ this.soccerBallLandedListener( soccerBall, value );
+ }
+ }
+ } );
+
+ const listener = ( value: number | null ) => {
+ if ( value !== null ) {
+ if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
+ this.objectCreated( soccerBall );
+ this.objectValueBecameNonNullEmitter.emit( soccerBall );
+ }
+ soccerBall.valueProperty.unlink( listener ); // Only create the card once, then no need to listen further
+ }
+ };
+ soccerBall.valueProperty.link( listener );
+
+ // Signal to listeners that a value changed
+ // TODO: Maybe should combine with temporary listener for one permanent one
+ soccerBall.valueProperty.link( () => this.objectChangedEmitter.emit( soccerBall ) );
+ soccerBall.positionProperty.link( () => this.objectChangedEmitter.emit( soccerBall ) );
return soccerBall;
- }, [ {} ], {
- phetioType: PhetioGroup.PhetioGroupIO( CAVObject.CAVObjectIO ),
- tandem: options.tandem.createTandem( 'soccerBallGroup' )
+ } );
+
+ this.soccerBallGroup.forEach( soccerBall => {
+ soccerBall.isActiveProperty.link( isActive => {
+ this.soccerBallGroupCountProperty.value = this.getActiveSoccerBalls().length;
+ } );
} );
this.isShowingTopMeanProperty = new BooleanProperty( false, {
@@ -206,33 +237,10 @@
tandem: options.tandem.createTandem( 'timeProperty' )
} );
- const updateDataMeasures = () => this.updateDataMeasures();
this.isShowingPlayAreaMedianProperty.link( updateDataMeasures );
- // Trigger CardModel creation when a ball lands.
- const objectCreatedListener = ( soccerBall: CAVObject ) => {
- const listener = ( value: number | null ) => {
- if ( value !== null ) {
- if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
- this.objectCreated( soccerBall );
- this.objectValueBecameNonNullEmitter.emit( soccerBall );
- }
- soccerBall.valueProperty.unlink( listener ); // Only create the card once, then no need to listen further
- }
- };
- soccerBall.valueProperty.link( listener );
- soccerBall.valueProperty.link( updateDataMeasures );
- soccerBall.positionProperty.link( updateDataMeasures );
-
- // Signal to listeners that a value changed
- // TODO: Maybe should combine with temporary listener for one permanent one
- soccerBall.valueProperty.link( () => this.objectChangedEmitter.emit( soccerBall ) );
- soccerBall.positionProperty.link( () => this.objectChangedEmitter.emit( soccerBall ) );
- };
- this.soccerBallGroup.forEach( objectCreatedListener );
- this.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
-
- this.numberOfRemainingObjectsProperty = new DerivedProperty( [ this.soccerBallGroup.countProperty ], count => {
+ this.numberOfRemainingObjectsProperty = DerivedProperty.deriveAny( this.soccerBallGroup.map( ball => ball.isActiveProperty ), () => {
+ const count = this.getActiveSoccerBalls().length;
return this.maxNumberOfObjects - count;
} );
@@ -250,11 +258,12 @@
this.soccerPlayers = _.range( 0, this.maxNumberOfObjects ).map( placeInLine => new SoccerPlayer( placeInLine ) );
// Create an initial ball to show on startup
- this.nextBallToKickProperty = new Property<CAVObject | null>( this.createBall(), {
+ this.nextBallToKickProperty = new Property<CAVObject | null>( this.soccerBallGroup[ 0 ], {
tandem: options.tandem.createTandem( 'nextBallToKickProperty' ),
phetioValueType: NullableIO( ReferenceIO( CAVObject.CAVObjectIO ) )
} );
+ // TODO: Simplify now that we have pools
this.numberOfRemainingKickableSoccerBallsProperty = new DerivedProperty( [
this.numberOfRemainingObjectsProperty,
this.numberOfScheduledSoccerBallsToKickProperty,
@@ -283,18 +292,10 @@
this.advanceLine();
if ( this.numberOfRemainingObjectsProperty.value > 0 && this.nextBallToKickProperty.value === null ) {
- this.nextBallToKickProperty.value = this.createBall();
+ const nextBall = this.getNextBallFromPool();
+ this.nextBallToKickProperty.value = nextBall;
+ nextBall!.isActiveProperty.value = true;
}
- } );
-
- this.soccerBallGroup.elementCreatedEmitter.addListener( soccerBall => {
- soccerBall.valueProperty.link( ( value, oldValue ) => {
- if ( value !== null && oldValue === null ) {
- if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
- this.soccerBallLandedListener( soccerBall, value );
- }
- }
- } );
} );
this.activeKickerIndexProperty = new NumberProperty( 0, {
@@ -306,6 +307,11 @@
soccerPlayer.isActiveProperty.value = index === activeKickerIndex;
} );
} );
+
+ this.soccerBallGroup.forEach( soccerBall => {
+ soccerBall.valueProperty.link( updateDataMeasures );
+ soccerBall.positionProperty.link( updateDataMeasures );
+ } );
}
protected objectCreated( soccerBall: CAVObject ): void {
@@ -391,15 +397,14 @@
* Clears out the data and the cards
*/
public clearData(): void {
- this.soccerBallGroup.clear();
-
this.numberOfScheduledSoccerBallsToKickProperty.reset();
this.timeProperty.reset();
this.timeWhenLastBallWasKickedProperty.reset();
- this.nextBallToKickProperty.value = this.createBall();
this.activeKickerIndexProperty.reset();
this.soccerPlayers.forEach( soccerPlayer => soccerPlayer.reset() );
+ this.soccerBallGroup.forEach( soccerBall => soccerBall.reset() );
+ this.nextBallToKickProperty.value = this.soccerBallGroup[ 0 ];
}
/**
@@ -421,7 +426,7 @@
}
public getSortedLandedObjects(): CAVObject[] {
- return _.sortBy( this.soccerBallGroup.filter( cavObject => cavObject.valueProperty.value !== null ),
+ return _.sortBy( this.getActiveSoccerBalls().filter( cavObject => cavObject.valueProperty.value !== null ),
// The numerical value takes predence for sorting
cavObject => cavObject.valueProperty.value,
@@ -438,16 +443,14 @@
*/
public step( dt: number ): void {
this.timeProperty.value += dt;
- this.soccerBallGroup.forEach( cavObject => cavObject.step( dt ) );
+ this.getActiveSoccerBalls().forEach( cavObject => cavObject.step( dt ) );
const frontPlayer = this.soccerPlayers[ this.activeKickerIndexProperty.value ];
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;
if ( this.numberOfScheduledSoccerBallsToKickProperty.value > 0 &&
- this.numberOfRemainingObjectsProperty.value + numberBallsThatExistButHaventBeenKicked > 0 &&
+ this.numberOfRemainingObjectsProperty.value &&
this.timeProperty.value >= this.timeWhenLastBallWasKickedProperty.value + TIME_BETWEEN_RAPID_KICKS ) {
if ( this.nextBallToKickProperty.value === null ) {
@@ -455,7 +458,7 @@
// Create the next ball.
// TODO-UX-HIGH: A ball is being created too soon, when using the multikick button
- this.nextBallToKickProperty.value = this.createBall();
+ this.nextBallToKickProperty.value = this.getNextBallFromPool();
}
assert && assert( this.nextBallToKickProperty.value !== null, 'there was no ball to kick' );
@@ -472,8 +475,8 @@
const elapsedTime = this.timeProperty.value - frontPlayer.timestampWhenPoisedBegan!;
if ( elapsedTime > 0.075 ) {
- const cavObject = this.nextBallToKickProperty.value!; // TODO: Probably? See https://github.com/phetsims/center-and-variability/issues/59
- this.kickBall( frontPlayer, cavObject );
+ const soccerBall = this.nextBallToKickProperty.value!; // TODO: Probably? See https://github.com/phetsims/center-and-variability/issues/59
+ this.kickBall( frontPlayer, soccerBall );
this.numberOfScheduledSoccerBallsToKickProperty.value--;
}
}
@@ -514,24 +517,15 @@
return dotRandom.nextBoolean() ? CAVConstants.LEFT_SKEWED_DATA : CAVConstants.RIGHT_SKEWED_DATA;
}
- /**
- * Creates a ball at the starting kick position.
- */
- private createBall(): CAVObject {
-
- const y0 = CAVObjectType.SOCCER_BALL.radius;
- const position = new Vector2( 0, y0 );
-
- return this.soccerBallGroup.createNextElement( {
- position: position
- } );
+ public getActiveSoccerBalls(): CAVObject[] {
+ return this.soccerBallGroup.filter( soccerBall => soccerBall.isActiveProperty.value );
}
/**
* When a ball lands on the ground, animate all other balls that were at this location above the landed ball.
*/
private soccerBallLandedListener( cavObject: CAVObject, value: number ): void {
- const otherObjectsInStack = this.soccerBallGroup.filter( x => x.valueProperty.value === value && x !== cavObject );
+ const otherObjectsInStack = this.getActiveSoccerBalls().filter( x => x.valueProperty.value === value && x !== cavObject );
const sortedOthers = _.sortBy( otherObjectsInStack, object => object.positionProperty.value.y );
sortedOthers.forEach( ( cavObject, index ) => {
@@ -568,15 +562,15 @@
* Adds the provided number of balls to the scheduled balls to kick
*/
public scheduleKicks( numberOfBallsToKick: number ): void {
- this.numberOfScheduledSoccerBallsToKickProperty.value +=
- Math.min( numberOfBallsToKick, this.numberOfRemainingKickableSoccerBallsProperty.value );
+ this.numberOfScheduledSoccerBallsToKickProperty.value += Math.min( numberOfBallsToKick, this.numberOfRemainingObjectsProperty.value );
}
/**
* Select a target location for the nextBallToKick, set its velocity and mark it for animation.
*/
- private kickBall( soccerPlayer: SoccerPlayer, cavObject: CAVObject ): void {
+ private kickBall( soccerPlayer: SoccerPlayer, soccerBall: CAVObject ): void {
soccerPlayer.poseProperty.value = Pose.KICKING;
+ soccerBall.isActiveProperty.value = true;
// Test that the sampling engine is working properly
// TODO: Where should these tests live? Should it be in the unit tests? Or in dot?
@@ -606,16 +600,20 @@
const v0 = Math.sqrt( Math.abs( x1 * Math.abs( CAVConstants.GRAVITY ) / Math.sin( 2 * angle ) ) );
const velocity = Vector2.createPolar( v0, angle );
- cavObject.velocityProperty.value = velocity;
+ soccerBall.velocityProperty.value = velocity;
- cavObject.targetX = x1;
+ soccerBall.targetX = x1;
- cavObject.animationModeProperty.value = AnimationMode.FLYING;
+ soccerBall.animationModeProperty.value = AnimationMode.FLYING;
this.timeWhenLastBallWasKickedProperty.value = this.timeProperty.value;
// New ball will be created later in step
this.nextBallToKickProperty.value = null;
}
+
+ private getNextBallFromPool(): CAVObject | null {
+ return this.soccerBallGroup.find( ball => !ball.isActiveProperty.value ) || null;
+ }
}
centerAndVariability.register( 'CAVModel', CAVModel );
\ No newline at end of file
Index: js/common/view/CardNodeContainer.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CardNodeContainer.ts b/js/common/view/CardNodeContainer.ts
--- a/js/common/view/CardNodeContainer.ts (revision ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/common/view/CardNodeContainer.ts (date 1682622783990)
@@ -49,6 +49,7 @@
type SelfOptions = EmptySelfOptions;
export type CardNodeContainerOptions = SelfOptions & NodeOptions & PickRequired<NodeOptions, 'tandem'>;
+// TODO: Move to median/
export default class CardNodeContainer extends Node {
// Each card is associated with one "cell", no two cards can be associated with the same cell. The leftmost cell is 0.
@@ -122,8 +123,7 @@
} );
};
- model.soccerBallGroup.forEach( objectCreatedListener );
- model.soccerBallGroup.elementCreatedEmitter.addListener( objectCreatedListener );
+ model.getActiveSoccerBalls().forEach( objectCreatedListener );
model.cardModelGroup.elementDisposedEmitter.addListener( cardModel => {
const cardNode = this.getCardNode( cardModel.cavObject )!;
Index: js/variability/view/IQRNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/variability/view/IQRNode.ts b/js/variability/view/IQRNode.ts
--- a/js/variability/view/IQRNode.ts (revision ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/variability/view/IQRNode.ts (date 1682622783991)
@@ -113,7 +113,7 @@
const updateIQRNode = () => {
- const sortedDots = _.sortBy( model.soccerBallGroup.getArrayCopy().filter( object => object.valueProperty.value !== null ),
+ const sortedDots = _.sortBy( model.soccerBallGroup.filter( object => object.valueProperty.value !== null ),
object => object.valueProperty.value );
const leftmostDot = sortedDots[ 0 ];
const rightmostDot = sortedDots[ sortedDots.length - 1 ];
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 ea13da4ca2a3c09cc6ad6fe7ed4c76e893bd6e0b)
+++ b/js/common/view/CAVObjectNode.ts (date 1682622783989)
@@ -130,6 +130,10 @@
this.translation = modelViewTransform.modelToViewPosition( position );
} );
+ cavObject.isActiveProperty.link( isActive => {
+ this.visible = isActive;
+ } );
+
// only setup input-related things if dragging is enabled
if ( options.draggingEnabled ) {
this.dragListener = new DragListener( {
@@ -217,16 +221,6 @@
} ) );
}
}
-
- public override dispose(): void {
- Multilink.unmultilink( this.opacityMultilink );
- Multilink.unmultilink( this.medianHighlightVisibleMultilink );
- this.inputEnabledMultilink && Multilink.unmultilink( this.inputEnabledMultilink );
- this.selfInputEnabledProperty && this.selfInputEnabledProperty.dispose();
- this.dragListener && this.hasInputListener( this.dragListener ) && this.removeInputListener( this.dragListener );
- this.dragListener && this.dragListener.dispose();
- super.dispose();
- }
}
centerAndVariability.register( 'CAVObjectNode', CAVObjectNode );
\ No newline at end of file
Also, from phet-io meeting today: |
|
I have continued the patches from above, and it's better but not fully working. In discussion with @marlitas and @matthew-blackman today, we agreed it is OK to commit not-fully-working code, and I would like to commit what I have even though it needs a lot of help. I also notified our subteam on slack:
After I commit, here are the main deficiencies or next steps I am aware of:
Perhaps @marlitas and @matthew-blackman and I can improve this at our meeting Friday morning. |
samreid
added a commit
that referenced
this issue
Apr 27, 2023
samreid
added a commit
that referenced
this issue
Apr 27, 2023
samreid
added a commit
that referenced
this issue
Apr 28, 2023
samreid
added a commit
that referenced
this issue
Apr 28, 2023
|
Just brainstorming this, but what if we don't create 20 soccer balls per each scene in the third screen? What if we have the different scenes saved as states that each ball can switch to as we switch between? May need to talk through this more. Not sure if I'm explaining it well right now, but I'm imagining a similar set up to one that was used in Number Play. |
samreid
added a commit
that referenced
this issue
Apr 28, 2023
samreid
added a commit
that referenced
this issue
Apr 28, 2023
marlitas
added a commit
that referenced
this issue
Apr 28, 2023
matthew-blackman
added a commit
that referenced
this issue
Apr 28, 2023
matthew-blackman
added a commit
that referenced
this issue
Apr 28, 2023
samreid
added a commit
that referenced
this issue
Apr 28, 2023
samreid
added a commit
that referenced
this issue
Apr 28, 2023
samreid
added a commit
that referenced
this issue
Apr 28, 2023
samreid
added a commit
that referenced
this issue
Apr 28, 2023
samreid
added a commit
that referenced
this issue
Apr 28, 2023
matthew-blackman
added a commit
that referenced
this issue
May 1, 2023
matthew-blackman
added a commit
that referenced
this issue
May 2, 2023
matthew-blackman
added a commit
to phetsims/babel
that referenced
this issue
May 2, 2023
matthew-blackman
added a commit
that referenced
this issue
May 2, 2023
matthew-blackman
added a commit
that referenced
this issue
May 3, 2023
matthew-blackman
added a commit
that referenced
this issue
May 3, 2023
matthew-blackman
added a commit
that referenced
this issue
May 3, 2023
matthew-blackman
added a commit
that referenced
this issue
May 4, 2023
matthew-blackman
added a commit
that referenced
this issue
May 4, 2023
matthew-blackman
added a commit
that referenced
this issue
May 4, 2023
|
Completed, we will continue in the fallout bug issues like: #171 etc. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
From #130 (comment), we think it would be best for the sim if we use static allocation with
isActiveProperty. See also remarks about moving past the 15-element cap in #148The text was updated successfully, but these errors were encountered: