phetsims / mean-share-and-balance

"Mean: Share and Balance" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 1 forks source link

Dashed candyBar functionality #142

Closed marlitas closed 9 months ago

marlitas commented 9 months ago

According to the design document we want to show a dashed outline of a candy bar if the tablePlate stack has more candies than it's notepadPlate stack representation.

image

I initially thought this would be straightforward, but it became more involved and would be good to get some more eyes on the potential changes.

A working patch is here, but it could definitely use some cleanup and a second pair of eyes.

```diff Subject: [PATCH] dashed candyBars --- Index: js/leveling-out/view/NotepadCandyBarNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/leveling-out/view/NotepadCandyBarNode.ts b/js/leveling-out/view/NotepadCandyBarNode.ts --- a/js/leveling-out/view/NotepadCandyBarNode.ts (revision 47a140aa4ff7ac8e2338a7369fe247cb3abc9ce2) +++ b/js/leveling-out/view/NotepadCandyBarNode.ts (date 1705982621113) @@ -22,7 +22,7 @@ import CandyBar from '../model/CandyBar.js'; type SelfOptions = EmptySelfOptions; -type NotepadCandyBarNodeOptions = SelfOptions & StrictOmit, 'children'>; +type NotepadCandyBarNodeOptions = SelfOptions & StrictOmit, 'children' | 'visibleProperty'>; export default class NotepadCandyBarNode extends InteractiveHighlighting( Node ) { @@ -40,6 +40,12 @@ stroke: 'black' } ); + candyBar.displayProperty.link( display => { + candyBarRectangle.fill = display === 'solid' ? MeanShareAndBalanceColors.candyBarColorProperty.value : null; + candyBarRectangle.stroke = display === 'solid' ? 'black' : MeanShareAndBalanceColors.candyBarColorProperty.value; + candyBarRectangle.lineDash = display === 'dashed' ? [ 2, 1.5 ] : []; + } ); + const children: Array = [ candyBarRectangle ]; // In ?dev mode, show the index of the candy bar to help understand how things are organized and how they @@ -50,7 +56,9 @@ const options = optionize()( { children: children, - cursor: 'pointer' + cursor: 'pointer', + visibleProperty: candyBar.visibleProperty, + pickableProperty: candyBar.isActiveProperty // We cannot drag a candyBar if it is not active }, providedOptions ); super( options ); Index: js/leveling-out/view/LevelingOutScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/leveling-out/view/LevelingOutScreenView.ts b/js/leveling-out/view/LevelingOutScreenView.ts --- a/js/leveling-out/view/LevelingOutScreenView.ts (revision 47a140aa4ff7ac8e2338a7369fe247cb3abc9ce2) +++ b/js/leveling-out/view/LevelingOutScreenView.ts (date 1705983122177) @@ -72,7 +72,7 @@ // function for what candy bars should do at the end of their drag const candyBarDropped = ( candyBar: NotepadCandyBarNode ) => { - const platesWithSpace = model.getPlatesWithSpace( model.getActivePlates() ); + const platesWithSpace = model.getPlatesWithSpace( model.getActiveNotepadPlates() ); // find the notepadPlate closest to where the candy bar was dropped. const closestPlate = _.minBy( platesWithSpace, plate => Math.abs( plate.position.x - candyBar.candyBar.positionProperty.value.x ) ); @@ -95,6 +95,17 @@ } candyBar.candyBar.parentPlateProperty.set( closestPlate! ); + + // // Update 'dashed' candy bars for the plate the candy bar was move to. + // const tablePlate = model.platesMap.get( closestPlate! ); + // model.setDashedCandyBars( tablePlate!, closestPlate! ); + // + // // Update 'dashed' candy bars for the plate the candy bar was moved from. + // const oldTablePlate = model.platesMap.get( currentParent ); + // model.setDashedCandyBars( oldTablePlate!, currentParent ); + + model.reorganizeCandyBars( closestPlate! ); + model.reorganizeCandyBars( currentParent ); }; @@ -128,8 +139,7 @@ const candyBarsParentTandem = options.tandem.createTandem( 'notepadCandyBars' ); const notepadCandyBars = model.candyBars.map( ( candyBar, i ) => new NotepadCandyBarNode( model, candyBar, notebookPaperBoundsProperty, candyBarDropped, { - tandem: candyBarsParentTandem.createTandem( `notepadCandyBar${i + 1}` ), - visibleProperty: candyBar.isActiveProperty + tandem: candyBarsParentTandem.createTandem( `notepadCandyBar${i + 1}` ) } ) ); // This contains all the candy bars from the top (paper) representation and the bottom (table) representation. Index: js/leveling-out/model/CandyBar.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/leveling-out/model/CandyBar.ts b/js/leveling-out/model/CandyBar.ts --- a/js/leveling-out/model/CandyBar.ts (revision 47a140aa4ff7ac8e2338a7369fe247cb3abc9ce2) +++ b/js/leveling-out/model/CandyBar.ts (date 1705983549793) @@ -9,7 +9,6 @@ * */ -import BooleanProperty from '../../../../axon/js/BooleanProperty.js'; import Property from '../../../../axon/js/Property.js'; import Vector2 from '../../../../dot/js/Vector2.js'; import IOType from '../../../../tandem/js/types/IOType.js'; @@ -18,36 +17,56 @@ import NotepadPlate from './NotepadPlate.js'; import { PhetioObjectOptions } from '../../../../tandem/js/PhetioObject.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; +import Vector2Property from '../../../../dot/js/Vector2Property.js'; +import StringIO from '../../../../tandem/js/types/StringIO.js'; +import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; +import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; + +type StateType = 'plate' | 'dragging' | 'animating'; +type DisplayType = 'solid' | 'dashed' | 'none'; type CandyBarOptions = { - isActive: boolean; + display: DisplayType; notepadPlate: NotepadPlate; position: Vector2; } & PickRequired; -type StateType = 'plate' | 'dragging' | 'animating'; - // Total number of candy bars allocated, for debugging. let count = 0; export default class CandyBar { - - public readonly isActiveProperty: Property; public readonly parentPlateProperty: Property; public readonly positionProperty: Property; public readonly stateProperty: Property; + // If the displayProperty value is set to 'solid', then this candy bar is actively participating in the model and + // contributing towards calculations including the mean. + public readonly displayProperty: Property; + public readonly isActiveProperty: TReadOnlyProperty; + public readonly visibleProperty: TReadOnlyProperty; + // For debugging public readonly index = count++; public constructor( providedOptions: CandyBarOptions ) { - this.isActiveProperty = new BooleanProperty( providedOptions.isActive, { + // The display property determines whether the candy bar is solid, dashed, or not visible. + this.displayProperty = new Property( providedOptions.display, { // phet-io - tandem: providedOptions.tandem.createTandem( 'isActiveProperty' ), + tandem: providedOptions.tandem.createTandem( 'displayProperty' ), phetioReadOnly: true, - phetioState: false + phetioValueType: StringIO, + validValues: [ 'solid', 'dashed', 'none' ] + } ); + + this.isActiveProperty = DerivedProperty.valueEqualsConstant( this.displayProperty, 'solid' ); + // this.isActiveProperty.debug( `isActiveProperty for candy bar ${this.index}` ); + + // A candy bar may be visible if it is not actively participating in the model if it is displaying a dashed + // representation of a candy bar + this.visibleProperty = new DerivedProperty( [ this.displayProperty ], display => { + return display !== 'none'; } ); this.parentPlateProperty = new Property( providedOptions.notepadPlate, { @@ -59,7 +78,12 @@ } ); // REVIEW: These may need phetioState: true - this.positionProperty = new Property( providedOptions.position ); + this.positionProperty = new Vector2Property( providedOptions.position, { + + // phet-io + tandem: providedOptions.tandem.createTandem( 'positionProperty' ), + phetioReadOnly: true + } ); this.stateProperty = new Property( 'plate' ); } @@ -67,7 +91,7 @@ this.positionProperty.reset(); this.stateProperty.reset(); this.parentPlateProperty.reset(); - this.isActiveProperty.reset(); + this.displayProperty.reset(); } } Index: js/leveling-out/model/LevelingOutModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/leveling-out/model/LevelingOutModel.ts b/js/leveling-out/model/LevelingOutModel.ts --- a/js/leveling-out/model/LevelingOutModel.ts (revision 47a140aa4ff7ac8e2338a7369fe247cb3abc9ce2) +++ b/js/leveling-out/model/LevelingOutModel.ts (date 1705983325940) @@ -37,6 +37,7 @@ public readonly notepadPlates: Array; public readonly tablePlates: Array; + public readonly platesMap = new Map(); public readonly candyBars: Array; public readonly meanProperty: TReadOnlyProperty; @@ -90,8 +91,8 @@ // In Mean Share and Balance, we decided arrays start counting at 1 let totalCandyBarCount = 1; - // Statically allocate plates, people, and candyBars. Whether they participate in the model is controlled by the - // isActiveProperty on each one. + // Statically allocate plates and candyBars. Whether they participate in the model is controlled by the + // isActiveProperty on each. for ( let tablePlateIndex = 0; tablePlateIndex < MAX_PEOPLE; tablePlateIndex++ ) { const x = tablePlateIndex * MeanShareAndBalanceConstants.TABLE_PLATE_WIDTH; const notepadPlate = new NotepadPlate( { @@ -120,10 +121,10 @@ const x = notepadPlate.position.x; const y = notepadPlate.position.y - ( ( MeanShareAndBalanceConstants.CANDY_BAR_HEIGHT + 2 ) * ( candyBarIndex + 1 ) ); - const isActive = notepadPlate.isActiveProperty.value && candyBarIndex < tablePlate.candyBarNumberProperty.value; + const display = notepadPlate.isActiveProperty.value && candyBarIndex < tablePlate.candyBarNumberProperty.value ? 'solid' : 'none'; const candyBar = new CandyBar( { - isActive: isActive, + display: display, notepadPlate: notepadPlate, position: new Vector2( x, y ), @@ -150,7 +151,9 @@ } } candyBars.forEach( ( candyBar, i ) => { - candyBar.isActiveProperty.value = isActive && i < tablePlate.candyBarNumberProperty.value; + + // We shouldn't need to worry about 'dashed' candyBars when plates are being added and removed. + candyBar.displayProperty.value = isActive && i < tablePlate.candyBarNumberProperty.value ? 'solid' : 'none'; this.reorganizeCandyBars( notepadPlate ); } ); } ); @@ -164,8 +167,12 @@ else if ( candyBarNumber < oldCandyBarNumber ) { this.tablePlateCandyBarAmountDecrease( notepadPlate, oldCandyBarNumber - candyBarNumber ); } + this.setDashedCandyBars( tablePlate, notepadPlate ); } } ); + + // Connect tablePlates and notepadPlates in map. + this.platesMap.set( notepadPlate, tablePlate ); meanPropertyDependencies.push( tablePlate.candyBarNumberProperty ); meanPropertyDependencies.push( tablePlate.isActiveProperty ); @@ -174,7 +181,7 @@ // Calculates the mean based on the "ground-truth" candyBars on the table. // Must be deriveAny because .map() does not preserve .length() this.meanProperty = DerivedProperty.deriveAny( meanPropertyDependencies, () => { - const candyBarAmounts = this.getActivePeople().map( tablePlate => tablePlate.candyBarNumberProperty.value ); + const candyBarAmounts = this.getActiveTablePlates().map( tablePlate => tablePlate.candyBarNumberProperty.value ); const totalCandyBars = _.sum( candyBarAmounts ); return totalCandyBars / candyBarAmounts.length; }, { @@ -192,11 +199,27 @@ } ); } - public getActivePeople(): Array { + public setDashedCandyBars( tablePlate: TablePlate, notepadPlate: NotepadPlate ): void { + const notepadCandyBars = this.getActiveCandyBarsOnPlate( notepadPlate ); + const numberOfCandyBarsOnNotepadPlate = notepadCandyBars.length; + const numberOfCandyBarsOnTablePlate = tablePlate.candyBarNumberProperty.value; + const candyBars = this.getCandyBarsOnPlate( notepadPlate ); + for ( let i = 0; i < MeanShareAndBalanceConstants.MAX_NUMBER_OF_CANDY_BARS_PER_PERSON; i++ ) { + if ( numberOfCandyBarsOnNotepadPlate < numberOfCandyBarsOnTablePlate ) { + candyBars[ i ].displayProperty.value = i < numberOfCandyBarsOnNotepadPlate ? 'solid' : + i < numberOfCandyBarsOnTablePlate ? 'dashed' : 'none'; + } + else { + candyBars[ i ].displayProperty.value = i < numberOfCandyBarsOnNotepadPlate ? 'solid' : 'none'; + } + } + } + + public getActiveTablePlates(): Array { return this.tablePlates.filter( tablePlate => tablePlate.isActiveProperty.value ); } - public getActivePlates(): Array { + public getActiveNotepadPlates(): Array { return this.notepadPlates.filter( plate => plate.isActiveProperty.value ); } @@ -205,7 +228,8 @@ } public getCandyBarsOnPlate( plate: NotepadPlate ): Array { - return this.candyBars.filter( candyBar => candyBar.parentPlateProperty.value === plate ); + return this.candyBars.filter( candyBar => candyBar.parentPlateProperty.value === plate ) + .sort( ( a, b ) => b.positionProperty.value.y - a.positionProperty.value.y ); } public getInactiveCandyBarsOnPlate( plate: NotepadPlate ): Array { @@ -246,13 +270,15 @@ * When candyBars are added to a notepadPlate they may appear in random positions or be overlapping. Re-stack them. */ public reorganizeCandyBars( plate: NotepadPlate ): void { - const plateStateCandyBars = this.getActivePlateStateCandyBars( plate ); + const plateStateCandyBars = this.getCandyBarsOnPlate( plate ); plateStateCandyBars.forEach( ( candyBar, i ) => { const Y_MARGIN = 2; // Distance between adjacent candyBars, and notepadPlate. const newPosition = new Vector2( plate.position.x, plate.position.y - ( ( MeanShareAndBalanceConstants.CANDY_BAR_HEIGHT + Y_MARGIN ) * ( i + 1 ) ) ); candyBar.positionProperty.set( newPosition ); } ); + + this.setDashedCandyBars( this.platesMap.get( plate )!, plate ); } /** @@ -262,7 +288,7 @@ private shareExtraCandyBars( numberOfExtraCandyBars: number ): void { for ( let i = 0; i < numberOfExtraCandyBars; i++ ) { const minPlate = this.getPlateWithLeastCandyBars(); - this.getBottomInactiveCandyBarOnPlate( minPlate ).isActiveProperty.set( true ); + this.getBottomInactiveCandyBarOnPlate( minPlate ).displayProperty.set( 'solid' ); this.reorganizeCandyBars( minPlate ); } } @@ -274,14 +300,14 @@ private borrowMissingCandyBars( numberOfMissingCandyBars: number ): void { for ( let i = 0; i < numberOfMissingCandyBars; i++ ) { const maxPlate = this.getPlateWithMostActiveCandyBars(); - this.getTopActiveCandyBarOnPlate( maxPlate ).isActiveProperty.set( false ); + this.getTopActiveCandyBarOnPlate( maxPlate ).displayProperty.set( 'none' ); this.reorganizeCandyBars( maxPlate ); } } /** - * When an active tablePlate adds candy bar to their notepadPlate and the paper notepadPlate has no more space on it, - * a piece of candy bar will be added onto the paper notepadPlate with the least candy bar. + * When an active tablePlate adds a candy bar to their notepadPlate and the notepadPlate has no more space on it, + * a candy bar will be added onto the notepadPlate with the least amount of candy bars. */ private tablePlateCandyBarAmountIncrease( plate: NotepadPlate, numberOfCandyBarsAdded: number ): void { for ( let i = 0; i < numberOfCandyBarsAdded; i++ ) { @@ -292,11 +318,11 @@ minPlate !== plate, `minPlate ${minPlate.linePlacement} should not be the same as affected plate: ${plate.linePlacement}` ); - this.getBottomInactiveCandyBarOnPlate( minPlate ).isActiveProperty.set( true ); + this.getBottomInactiveCandyBarOnPlate( minPlate ).displayProperty.set( 'solid' ); this.reorganizeCandyBars( minPlate ); } else { - this.getBottomInactiveCandyBarOnPlate( plate ).isActiveProperty.set( true ); + this.getBottomInactiveCandyBarOnPlate( plate ).displayProperty.set( 'solid' ); } this.reorganizeCandyBars( plate ); } @@ -311,18 +337,18 @@ const numberOfCandyBarsOnPlate = this.getActivePlateStateCandyBars( plate ).length; if ( numberOfCandyBarsOnPlate === 0 ) { const maxPlate = this.getPlateWithMostActiveCandyBars(); - this.getTopActiveCandyBarOnPlate( maxPlate ).isActiveProperty.set( false ); + this.getTopActiveCandyBarOnPlate( maxPlate ).displayProperty.set( 'none' ); this.reorganizeCandyBars( maxPlate ); } else { - this.getTopActiveCandyBarOnPlate( plate ).isActiveProperty.set( false ); + this.getTopActiveCandyBarOnPlate( plate ).displayProperty.set( 'none' ); } this.reorganizeCandyBars( plate ); } } public getPlateWithMostActiveCandyBars(): NotepadPlate { - const maxPlate = _.maxBy( this.getActivePlates(), ( plate => this.getActiveCandyBarsOnPlate( plate ).length ) ); + const maxPlate = _.maxBy( this.getActiveNotepadPlates(), ( plate => this.getActiveCandyBarsOnPlate( plate ).length ) ); // _.maxBy can return undefined if all the elements in the array are null, undefined, or NAN. // candyBarsNumberProperty will always be a number. @@ -330,7 +356,7 @@ } public getPlateWithLeastCandyBars(): NotepadPlate { - const minPlate = _.minBy( this.getActivePlates(), ( plate => this.getActiveCandyBarsOnPlate( plate ).length ) ); + const minPlate = _.minBy( this.getActiveNotepadPlates(), ( plate => this.getActiveCandyBarsOnPlate( plate ).length ) ); // _.minBy can return undefined if all the elements in the array are null, undefined, or NAN. // candyBarsNumberProperty will always be a number. @@ -357,7 +383,9 @@ this.tablePlates.forEach( ( tablePlate, index ) => { this.getCandyBarsOnPlate( this.notepadPlates[ index ] ).forEach( ( candyBar, i ) => { - candyBar.isActiveProperty.value = i < tablePlate.candyBarNumberProperty.value; + + // There will be no 'dashed' candyBars when syncing data. + candyBar.displayProperty.value = tablePlate.isActiveProperty.value && i < tablePlate.candyBarNumberProperty.value ? 'solid' : 'none'; } ); } ); } ```
jbphet commented 9 months ago

Implementation complete, closing.