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

Support state for notepadModeProperty changes #204

Closed marlitas closed 2 months ago

marlitas commented 3 months ago

Connected to https://github.com/phetsims/phet-io/issues/1661. Currently ObservableArray listeners are firing unexpectedly before the notepadModeProperty value has been set in state.

zepumph commented 3 months ago

@marlitas and I got around the problem with this patch. @marlitas, please note in this issue if you discover any other problems with the state setting. Perhaps apply this patch, and see if any other issues occur?

```diff Subject: [PATCH] workaround, https://github.com/phetsims/phet-io/issues/1661 --- Index: phet-io/js/PhetioStateEngine.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/phet-io/js/PhetioStateEngine.ts b/phet-io/js/PhetioStateEngine.ts --- a/phet-io/js/PhetioStateEngine.ts (revision eb1ea5100748f91ecaf506ff0fcba95872ce0bf3) +++ b/phet-io/js/PhetioStateEngine.ts (date 1713381623440) @@ -313,6 +313,10 @@ phetioObject = this.create( state, phetioID ); } + if ( phetioObject.phetioID.includes( 'snacksOnNotepadPlate' ) && !completedIDs.includes( 'meanShareAndBalance.fairShareScreen.model.notepadModeProperty' ) ) { + throw new CouldNotYetDeserializeError(); + } + // Could throw CouldNotYetDeserializeError this.setStateForPhetioObject( phetioObject, state ); Index: mean-share-and-balance/js/fair-share/model/FairShareModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/mean-share-and-balance/js/fair-share/model/FairShareModel.ts b/mean-share-and-balance/js/fair-share/model/FairShareModel.ts --- a/mean-share-and-balance/js/fair-share/model/FairShareModel.ts (revision ada739d63bd1620db1bedfb07daa1f2061b7f1bc) +++ b/mean-share-and-balance/js/fair-share/model/FairShareModel.ts (date 1713381773067) @@ -490,8 +490,8 @@ // TODO: The commented out code just below is for debugging observable array listeners in phet-io, see https://github.com/phetsims/phet-io/issues/1661. // assert && assert( this.notepadModeProperty.value !== NotepadMode.COLLECT, // 'apples should not be added to plates in collect mode' ); - // const notepadMode = this.notepadModeProperty.hasDeferredValue ? this.notepadModeProperty.deferredValue : this.notepadModeProperty.value; - if ( this.notepadModeProperty.value === NotepadMode.SHARE ) { + const notepadMode = this.notepadModeProperty.hasDeferredValue ? this.notepadModeProperty.deferredValue : this.notepadModeProperty.value; + if ( notepadMode === NotepadMode.SHARE ) { // Calculate the fractional amount that will be set for the apples being distributed to the plates. const numberOfWholeApples = Math.floor( this.meanProperty.value ); @@ -525,7 +525,7 @@ }, APPLE_FRACTION_DISTRIBUTION_DELAY * 1000 ) ); } } - else if ( this.notepadModeProperty.value === NotepadMode.SYNC ) { + else if ( notepadMode === NotepadMode.SYNC ) { apple.isActiveProperty.value = true; apple.fractionProperty.value = Fraction.ONE; apple.moveTo( plate.getPositionForStackedItem( index ), this.animateAddedSnacks );
zepumph commented 3 months ago

I'll report back here once we have progress on https://github.com/phetsims/phet-io/issues/1661. Just to be clear, for this specific MSAB case, the order dependency is:

meanShareAndBalance.fairShareScreen.model.notepadModeProperty needs to have its new value, before meanShareAndBalance.fairShareScreen.model.plate1.snacksOnNotepadPlate fires its listeners from adding new snacks.

zepumph commented 2 months ago

https://github.com/phetsims/phet-io/issues/1661 has been completed. @marlitas can you please review and ensure that MSAB PhET-iO state is working.

marlitas commented 2 months ago

Yes this is great! Thanks for turning that assertion on and doing this work. Ready to close!