phetsims / natural-selection

"Natural Selection" is an educational simulation in HTML5, by PhET Interactive Simulations
GNU General Public License v3.0
3 stars 7 forks source link

Proportion data missing after migrating from 1.4 to 1.5 #354

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

Test device MacBook Air M1 chip

Operating System 13.5

Browser Safari 16.6

Problem description For https://github.com/phetsims/qa/issues/967, and possibly related to https://github.com/phetsims/natural-selection/issues/353: the proportions graph will say "no data" in version 1.5 even though there is data in 1.4. This occurs when migrating in both Play and Pause mode, and when in any generation. This doesn't happen with the Populations graph.

Steps to reproduce In the migration wrapper:

  1. On either screen, select the Proportions radio button
  2. Click "Add a Mate"
  3. Click "Migrate Now"

Visuals

Screenshot 2023-08-01 at 7 04 58 PM Screenshot 2023-08-01 at 7 05 11 PM
pixelzoom commented 1 year ago

Definitely related to https://github.com/phetsims/natural-selection/issues/353, where assertion failure is in ProportionsModel.ts recordEndCounts. I'll probably try to resolve this first, since it will likely fix #353.

pixelzoom commented 1 year ago

This problem does not occur in the State wrapper, so it's probably not related to the order that state is restored.

pixelzoom commented 1 year ago

The 1.5 sim is displaying "No Data" for the same reason that recordEndCounts is failing in #353. The value of currentStartCountsProperty is null.

In ProportionsModel:

    const currentStartCountsProperty = new Property<BunnyCounts | null>( null, {
...

    this.hasDataProperty = new DerivedProperty(
      [ currentStartCountsProperty ], currentStartCounts => !!currentStartCounts, {
        tandem: Tandem.OPT_OUT
      } );

In ProportionsGraphNode:

    const noDataText = new Text( NaturalSelectionStrings.noDataStringProperty, {
...

    proportionsModel.hasDataProperty.link( hasData => {
      content.visible = hasData;
      noDataText.visible = !hasData;
    } );
pixelzoom commented 1 year ago

This was 100% caused by #353, and resolved by the fix over there, which involved deleting this a bogus migration rule (duh):

new TandemFragmentDelete( 'proportionsModel.currentStartCountsProperty' ),

We want to get this sim into RC ASAP. So I'm going to proceed with creating the release branch and have @Nancy-Salpepi verify this issue in 1.5.0-rc.1, rather than in master.

Please close if this issue verifies OK.

Nancy-Salpepi commented 1 year ago

This is fixed in rc.1. 🙂 Closing!