phetsims / gas-properties

"Gas Properties" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 6 forks source link

Incorrect "Oops" dialog when making container wider while holding Temperature constant. #246

Closed pixelzoom closed 6 months ago

pixelzoom commented 6 months ago

Noticed during 5/202/24 design meeting.

Steps to reproduce:

  1. Start the sim, go to Ideal screen.
  2. Add 50 heavy particles in Particles panel.
  3. Select "Temperature (T)" radio button in Hold Constant panel.
  4. Attempt to make the container wider by dragging the resize handle to the left (with either the mouse or keyboard). The dialog shown below opens. The lid is still on the container, pressure is nowhere near blowing the lid off, and making the container wider should not increase pressure.
screenshot_3312
pixelzoom commented 6 months ago

Relevant code is in IdealLawGasModel.ts. Adding a debugger indicates that this.container.lidIsOpenProperty.value is true.

    this.container.lidIsOpenProperty.link( lidIsOpen => {
      if ( !isSettingPhetioStateProperty.value ) {
        if ( lidIsOpen && this.holdConstantProperty.value === 'temperature' ) {
          phet.log && phet.log( 'Oops! T cannot be held constant when the container is open' );
          this.holdConstantProperty.value = 'nothing';
+         debugger;
          this.oopsEmitters.temperatureLidOpenEmitter.emit();
        }
      }
    } );
pixelzoom commented 6 months ago

lidIsOpenProperty is derived in IdealGasLawContainer.ts:

    this.lidIsOpenProperty = new DerivedProperty(
      // this.widthProperty is used by this.getMaxLidWidth()
      [ this.lidIsOnProperty, this.lidWidthProperty, this.widthProperty ],
      ( lidIsOn, lidWidth, containerWidth ) => !lidIsOn || ( lidWidth < this.getMaxLidWidth() ), {
        tandem: options.tandem.createTandem( 'lidIsOpenProperty' ),
        phetioFeatured: true,
        phetioValueType: BooleanIO,
        phetioDocumentation: 'Whether the container\'s lid is open.'
      } );

Inspecting the dependencies:

lidIsOn => true lidWidth => 8075 getMaxLidWidth() => ~8083

So the lid is on, but the lid's width does not cover the opening in the top of the container.

I'm guessing that this is a problem with listener order. There's probably something that adjusts lidWidthProperty that has not executed yet.

pixelzoom commented 6 months ago

The problem is indicated by the comment here:

    this.lidIsOpenProperty = new DerivedProperty(
      // this.widthProperty is used by this.getMaxLidWidth()
      [ this.lidIsOnProperty, this.lidWidthProperty, this.widthProperty ],

getMaxLidWidth uses widthProperty. So a dependency on widthProperty was added to appease axon's strictAxonDependencies option. But widthProperty is updated before lidWidthProperty, and that results in an internmediate state where the container is slightly open.

To resolve this problem in https://github.com/phetsims/gas-properties/commit/eeb0632b7bda8ff340f7c7fda9f60645d3b9e42e, I removed the dependency on widthProperty and added:

strictAxonDependencies: false, // See https://github.com/phetsims/gas-properties/issues/246

The problem is not longer reproducible, so closing.