Skip to content

Commit

Permalink
bunny arrays should be stateful (removing listener order dependencies),
Browse files Browse the repository at this point in the history
#361

Signed-off-by: Michael Kauzmann <michael.kauzmann@colorado.edu>
  • Loading branch information
zepumph committed May 8, 2024
1 parent 69edfd7 commit fe0b0e4
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
12 changes: 10 additions & 2 deletions js/common/model/BunnyCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import SelectedBunnyProperty from './SelectedBunnyProperty.js';
import BunnyCounts from './BunnyCounts.js';
import NaturalSelectionUtils from '../NaturalSelectionUtils.js';
import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js';
import isClearingPhetioDynamicElementsProperty from '../../../../tandem/js/isClearingPhetioDynamicElementsProperty.js';

const LITTER_SIZE = 4;
assert && assert( LITTER_SIZE === 4,
Expand Down Expand Up @@ -181,17 +182,18 @@ export default class BunnyCollection {
}
} );

this.liveBunnies.push( bunny );
// liveBunnies (and all arrays) are stateful, so they will have their own state set separately.
!isSettingPhetioStateProperty.value && this.liveBunnies.push( bunny );
}
else {
assert && assert( isSettingPhetioStateProperty.value,
'a dead bunny should only be created when restoring PhET-iO state' );
this.deadBunnies.push( bunny );
}
} );

// When a bunny is disposed, remove it from the appropriate arrays. removeListener is not necessary.
bunnyGroup.elementDisposedEmitter.addListener( bunny => {
assert && assert( !( isSettingPhetioStateProperty.value && !isClearingPhetioDynamicElementsProperty.value ), 'should never dispose a bunny while setting state outside of initial clear' );

const liveIndex = this.liveBunnies.indexOf( bunny );
if ( liveIndex !== -1 ) {
Expand All @@ -216,6 +218,12 @@ export default class BunnyCollection {

this.genePool = genePool;
this.bunnyGroup = bunnyGroup;

if ( assert && Tandem.PHET_IO_ENABLED ) {
phet.phetio.phetioEngine.phetioStateEngine.stateSetEmitter.addListener( () => {
this.assertValidCounts();
} );
}
}

public dispose(): void {
Expand Down
14 changes: 10 additions & 4 deletions js/common/model/createBunnyArray.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import naturalSelection from '../../naturalSelection.js';
import Bunny from './Bunny.js';
import BunnyCounts from './BunnyCounts.js';
import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js';
import Tandem from '../../../../tandem/js/Tandem.js';

// Additional properties that will be added to ObservableArray<Bunny>
type AdditionalProperties = {
Expand All @@ -39,8 +40,7 @@ export default function createBunnyArray( providedOptions: BunnyArrayOptions ):
countsPropertyFeatured: false,

// ObservableArrayOptions
phetioType: createObservableArray.ObservableArrayIO( ReferenceIO( Bunny.BunnyIO ) ),
phetioState: false
phetioType: createObservableArray.ObservableArrayIO( ReferenceIO( Bunny.BunnyIO ) )
}, providedOptions );

// We want to add countsProperty later, so do a little TypeScript hackery here to make that possible.
Expand All @@ -50,8 +50,7 @@ export default function createBunnyArray( providedOptions: BunnyArrayOptions ):
tandem: options.tandem.createTandem( 'countsProperty' ),
phetioValueType: BunnyCounts.BunnyCountsIO,
phetioFeatured: options.countsPropertyFeatured,
phetioReadOnly: true,
phetioState: false // because counts will be restored as Bunny instances are restored to BunnyGroup
phetioReadOnly: true
} );

bunnyArray.countsProperty = countsProperty;
Expand Down Expand Up @@ -80,6 +79,13 @@ export default function createBunnyArray( providedOptions: BunnyArrayOptions ):
}
} );

// State may set in an unexpected order, but by the end, we must have the right counts.
if ( assert && Tandem.PHET_IO_ENABLED ) {
phet.phetio.phetioEngine.phetioStateEngine.stateSetEmitter.addListener( () => {
assert && assert( countsProperty.value.totalCount === bunnyArray.length, 'counts out of sync' );
} );
}

bunnyArray.dispose = () => {
Disposable.assertNotDisposable();
};
Expand Down

0 comments on commit fe0b0e4

Please sign in to comment.