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

Range of the Population graph's x-axis is incorrect in Preview Sim. #315

Closed pixelzoom closed 1 year ago

pixelzoom commented 2 years ago

Similar to #314, for https://github.com/phetsims/qa/issues/818 ... I inspected occurrences of link and lazyLink, looking for cases where restored state might be overwritten. I found this case, which was also present in the previous (1.4) release.

The range of the Population graph's x-axis is overwritten when restoring state. To reproduce:

  1. Go to the Lab screen
  2. Press the "Add a Mate" button
  3. Let the simulation run until the Population graph starts auto-scrolling to the left. Checking "Limited Food" around generation 3 will ensure that bunnies don't take over the world.
  4. Pause the sim.
  5. Scroll the Population graph fully to the left, so the leftmost x-axis tick mark is 0. The left arrow button (below the graph) is disabled, the right arrow button is enabled. Like this:
screenshot_1772
  1. Press "Preview Sim" in Studio.
  2. Note that in the Preview, the Population graph's x-axis is fully scrolled to the right, not fully scrolled to the left. The left arrow button is enabled, the right arrow button is disabled. Like this:
screenshot_1773

Relevant code in PopulationModel.js, line 183:

    timeInGenerationsProperty.link( timeInGeneration => {
      const max = Math.max( options.xAxisLength, timeInGeneration );
      if ( this.xRangeProperty.value.max !== max ) {
        const min = max - options.xAxisLength;
        this.xRangeProperty.value = new Range( min, max );
      }
    } );

In general, if a listener for aProperty sets the value of bProperty, then that code typically needs to be short-circuited when restoring state. The general pattern is:

aProperty.link( a => {
  if ( !phet.joist.sim.isSettingPhetioStateProperty.value ) {
    bProperty.value = ...;
  }
} );
pixelzoom commented 2 years ago

Fixed in the above commit. @Nancy-Salpepi since you verified #314, would you mind verifying this one in master? If it looks OK, please label as status:fixed-awaiting-deploy.

Nancy-Salpepi commented 2 years ago

Looks good on master!

pixelzoom commented 1 year ago

To verify this issue in https://github.com/phetsims/qa/issues/967, follow the steps in https://github.com/phetsims/natural-selection/issues/315#issue-1302321172.

Please close this issue if it looks OK.

Nancy-Salpepi commented 1 year ago

Looks good in 1.5.0-dev.5. Closing.