Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Tracked by #967
pixelzoom opened this issue Jul 12, 2022 · 4 comments
Closed
Tracked by #967

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jul 12, 2022

Similar to #314, for phetsims/qa#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 pixelzoom added type:bug Something isn't working dev:phet-io labels Jul 12, 2022
@pixelzoom pixelzoom self-assigned this Jul 12, 2022
pixelzoom added a commit that referenced this issue Jul 12, 2022
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jul 12, 2022

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
Copy link

Looks good on master!

@pixelzoom
Copy link
Contributor Author

To verify this issue in phetsims/qa#967, follow the steps in #315 (comment).

Please close this issue if it looks OK.

@Nancy-Salpepi
Copy link

Looks good in 1.5.0-dev.5.
Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants