Skip to content

Commit

Permalink
Address listener order dependency errors, see: #54
Browse files Browse the repository at this point in the history
  • Loading branch information
marlitas committed Jan 2, 2025
1 parent 664f061 commit 2ca261d
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 11 deletions.
1 change: 1 addition & 0 deletions js/common/model/CountingObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export default class CountingObject extends PhetioObject {

// This Property should not reset. It is managed by the addend array when added or removed.O
this.addendTypeProperty = new EnumerationProperty( AddendType.INACTIVE, {
hasListenerOrderDependencies: true,
tandem: this.tandem.createTandem( 'addendTypeProperty' ),
phetioReadOnly: true,
phetioFeatured: false
Expand Down
2 changes: 2 additions & 0 deletions js/common/model/DecompositionModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export default class DecompositionModel extends NumberPairsModel {
const initialSceneModel = sceneModels.find( sceneModel => sceneModel.total === options.initialTotalValue );
assert && assert( initialSceneModel, `initialSceneModel not found for total: ${options.initialTotalValue}` );
const selectedSceneModelProperty = new Property( initialSceneModel!, {
hasListenerOrderDependencies: true,
phetioValueType: NumberPairsScene.NumberPairsSceneIO,
tandem: options.tandem.createTandem( 'selectedSceneModelProperty' ),
phetioFeatured: true
Expand All @@ -67,6 +68,7 @@ export default class DecompositionModel extends NumberPairsModel {
const rightAddendCountingObjectsProperty = new DerivedProperty( [ selectedSceneModelProperty ],
sceneModel => sceneModel.rightAddendObjects );
const leftAddendProperty = new DynamicProperty<number, number, NumberPairsScene>( selectedSceneModelProperty, {
hasListenerOrderDependencies: true,
derive: 'leftAddendProperty',
bidirectional: true // This Property needs to be bidirectional because it is set by the slider in the NumberLineNode.
} );
Expand Down
23 changes: 14 additions & 9 deletions js/common/model/NumberPairsModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export default class NumberPairsModel implements TModel {
// The range will update after all addends have stabilized their values during construction.
this.numberLineSliderEnabledRangeProperty = new Property(
new Range( NumberPairsConstants.TWENTY_NUMBER_LINE_RANGE.min, numberOfCountingObjects ), {
hasListenerOrderDependencies: true,
phetioValueType: Range.RangeIO,
tandem: options.tandem.createTandem( 'numberLineSliderEnabledRangeProperty' )
} );
Expand All @@ -172,9 +173,11 @@ export default class NumberPairsModel implements TModel {
} );

this.leftAddendCountingObjectsLengthProperty = new DynamicProperty( this.leftAddendCountingObjectsProperty, {
hasListenerOrderDependencies: true,
derive: 'lengthProperty'
} );
this.rightAddendCountingObjectsLengthProperty = new DynamicProperty( this.rightAddendCountingObjectsProperty, {
hasListenerOrderDependencies: true,
derive: 'lengthProperty'
} );
}
Expand Down Expand Up @@ -397,15 +400,15 @@ export default class NumberPairsModel implements TModel {
new Vector2( position.x + xTranslation, position.y ) );
const newLeftLocationPositions = rightLocationPositions.map( position =>
new Vector2( position.x - xTranslation, position.y ) );
this.setLocationPositions( newLeftLocationPositions, newRightLocationPositions );
this.setLocationPositions( leftAddendObjects, rightAddendObjects, newLeftLocationPositions, newRightLocationPositions );

// Bead positions should be a translation across the separator.
const updatedSeparatorPosition = NumberPairsModel.calculateBeadSeparatorXPosition( this.leftAddendProperty.value );
const rightXTranslation = _.max( rightBeadXPositions )! - ( updatedSeparatorPosition - NumberPairsConstants.BEAD_DISTANCE_FROM_SEPARATOR );
const leftXTranslation = updatedSeparatorPosition + NumberPairsConstants.BEAD_DISTANCE_FROM_SEPARATOR - _.min( leftBeadXPositions )!;
const newLeftBeadXPositions = rightBeadXPositions.map( x => x - rightXTranslation );
const newRightBeadXPositions = leftBeadXPositions.map( x => x + leftXTranslation );
this.setBeadXPositions( newLeftBeadXPositions, newRightBeadXPositions );
this.setBeadXPositions( leftAddendObjects, rightAddendObjects, newLeftBeadXPositions, newRightBeadXPositions );

assert && assert( this.leftAddendCountingObjectsProperty.value.length === this.leftAddendProperty.value, 'Addend array length and value should match' );
assert && assert( this.rightAddendCountingObjectsProperty.value.length === this.rightAddendProperty.value, 'Addend array length and value should match' );
Expand All @@ -415,10 +418,10 @@ export default class NumberPairsModel implements TModel {
* Set the location positions of the counting objects based on the provided left and right location positions.
* @param leftLocationPositions
* @param rightLocationPositions
* @param leftAddendObjects - prevent incorrect intermediary values by using the same counting objects as the call site.
* @param rightAddendObjects
*/
public setLocationPositions( leftLocationPositions: Vector2[], rightLocationPositions: Vector2[] ): void {
const leftAddendObjects = this.leftAddendCountingObjectsProperty.value;
const rightAddendObjects = this.rightAddendCountingObjectsProperty.value;
public setLocationPositions( leftAddendObjects: CountingObject[], rightAddendObjects: CountingObject[], leftLocationPositions: Vector2[], rightLocationPositions: Vector2[] ): void {

assert && assert( leftAddendObjects.length === leftLocationPositions.length, 'leftAddendObjects should be the same length as the rightLocationPositions now that they have been swapped.' );
assert && assert( rightAddendObjects.length === rightLocationPositions.length, 'rightAddendObjects should be the same length as the leftLocationPositions now that they have been swapped.' );
Expand Down Expand Up @@ -455,12 +458,14 @@ export default class NumberPairsModel implements TModel {
* Set the bead x positions of the counting objects based on the provided left and right x positions.
* @param leftXPositions
* @param rightXPositions
* @param leftAddendObjects - prevent incorrect intermediary values by using the same counting objects as the call site.
* @param rightAddendObjects
*/
public setBeadXPositions( leftXPositions: number[], rightXPositions: number[] ): void {
this.leftAddendCountingObjectsProperty.value.forEach( ( countingObject, index ) => {
public setBeadXPositions( leftAddendObjects: CountingObject[], rightAddendObjects: CountingObject[], leftXPositions: number[], rightXPositions: number[] ): void {
leftAddendObjects.forEach( ( countingObject, index ) => {
countingObject.beadXPositionProperty.value = leftXPositions[ index ];
} );
this.rightAddendCountingObjectsProperty.value.forEach( ( countingObject, index ) => {
rightAddendObjects.forEach( ( countingObject, index ) => {
countingObject.beadXPositionProperty.value = rightXPositions[ index ];
} );
this.beadXPositionsProperty.value = leftXPositions.concat( rightXPositions );
Expand Down Expand Up @@ -511,7 +516,7 @@ export default class NumberPairsModel implements TModel {
return groupingIndex + i + beadSeparatorXPosition + distanceFromSeparator;
} );

this.setBeadXPositions( leftXPositions, rightXPositions );
this.setBeadXPositions( leftAddendBeads, rightAddendBeads, leftXPositions, rightXPositions );
}

/**
Expand Down
1 change: 1 addition & 0 deletions js/common/model/NumberPairsScene.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export default class NumberPairsScene {
} );
this.leftAddendProperty = new NumberProperty( initialLeftAddendValue, {
range: sceneRange,
hasListenerOrderDependencies: true,
numberType: 'Integer',
tandem: tandem.createTandem( 'leftAddendProperty' )
} );
Expand Down
2 changes: 1 addition & 1 deletion js/common/view/BeadsOnWireNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export default class BeadsOnWireNode extends Node {
rightAddendXPositions = this.shiftXPositions( rightAddendXPositions, 1, separatorXPosition + beadDistanceFromSeparator );
}

this.model.setBeadXPositions( leftAddendXPositions, rightAddendXPositions );
this.model.setBeadXPositions( leftAddendBeads, rightAddendBeads, leftAddendXPositions, rightAddendXPositions );
}

/**
Expand Down
7 changes: 7 additions & 0 deletions js/common/view/NumberLineSlider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ export default class NumberLineSlider extends HSlider {
const numberLineRange = providedOptions.numberLineRange;
const thumbNode = new ThumbNode( providedOptions.tandem );

leftAddendNumberProperty.link( leftAddendNumber => {
console.log( 'leftAddend', leftAddendNumber );
} );
enabledRangeProperty.link( enabledRange => {
console.log( 'enabledRange', enabledRange );
} );

const sliderTickParent = new Node();
const trackNode = new NumberLineSliderTrack(
leftAddendNumberProperty,
Expand Down
4 changes: 3 additions & 1 deletion js/sum/model/SumModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@ export default class SumModel extends NumberPairsModel {
const leftAddendProperty = new NumberProperty( NumberPairsConstants.SUM_INITIAL_LEFT_ADDEND_VALUE, {
numberType: 'Integer',
range: SCENE_RANGE,
hasListenerOrderDependencies: true,
tandem: options.tandem.createTandem( 'leftAddendProperty' )
} );

const totalProperty = new NumberProperty(
NumberPairsConstants.SUM_INITIAL_LEFT_ADDEND_VALUE + NumberPairsConstants.SUM_INITIAL_RIGHT_ADDEND_VALUE, {
tandem: options.tandem.createTandem( 'totalProperty' ),
numberType: 'Integer',
hasListenerOrderDependencies: true,
range: SCENE_RANGE
} );

Expand Down Expand Up @@ -214,7 +216,7 @@ export default class SumModel extends NumberPairsModel {
this.setCountingObjectsToInitialValues();

const initialBeadPositions = NumberPairsModel.getInitialBeadPositions( this.leftAddendProperty.value, this.totalProperty.value );
this.setBeadXPositions( initialBeadPositions.leftAddendXPositions, initialBeadPositions.rightAddendXPositions );
this.setBeadXPositions( this.leftAddendCountingObjectsProperty.value, this.rightAddendCountingObjectsProperty.value, initialBeadPositions.leftAddendXPositions, initialBeadPositions.rightAddendXPositions );
this.beadXPositionsProperty.value = [ ...initialBeadPositions.leftAddendXPositions, ...initialBeadPositions.rightAddendXPositions ];
}
}
Expand Down

0 comments on commit 2ca261d

Please sign in to comment.