phetsims / center-and-variability

"Center and Variability" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 2 forks source link

Kick animation frozen if launched before kick is finished #581

Closed KatieWoe closed 1 year ago

KatieWoe commented 1 year ago

Test device Samsung Operating System Win 11 Browser Chrome Problem description For https://github.com/phetsims/qa/issues/993 In studio, if you launch the sim before a kicking animation is finished, the resulting sim will have the kicker frozen in place until another kick is made. Steps to reproduce

  1. In studio, press the kick 1 button
  2. Quickly, before the ball lands, press Test
  3. Observe the launched sim

Visuals launchinkick

marlitas commented 1 year ago

This was happening because we were not tracking a ball's Kicker as a Property. This patch changes that, but introduces some phet-io API changes so I would like to double check with another developer before committing for cherry pick.

```diff Subject: [PATCH] Create kickerProperty --- Index: js/model/SoccerBall.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/SoccerBall.ts b/js/model/SoccerBall.ts --- a/js/model/SoccerBall.ts (revision db6984b5c0089834b6ca38dd377514cc231b5691) +++ b/js/model/SoccerBall.ts (date 1697587961279) @@ -25,6 +25,8 @@ import SoccerBallValueProperty from './SoccerBallValueProperty.js'; import BooleanProperty from '../../../axon/js/BooleanProperty.js'; import Tandem from '../../../tandem/js/Tandem.js'; +import ReferenceIO from '../../../tandem/js/types/ReferenceIO.js'; +import IOType from '../../../tandem/js/types/IOType.js'; // Global counter for debugging let count = 0; @@ -57,7 +59,7 @@ public readonly resetEmitter: TEmitter = new Emitter(); public animation: Animation | null = null; - public kicker: Kicker | null = null; + public kickerProperty: Property; // Global index for debugging public readonly index = count++; @@ -117,6 +119,13 @@ phetioValueType: NullableIO( NumberIO ), phetioReadOnly: true } ); + + this.kickerProperty = new Property( null, { + phetioFeatured: false, + phetioReadOnly: true, + phetioValueType: NullableIO( ReferenceIO( IOType.ObjectIO ) ), + tandem: tandem.createTandem( 'kickerProperty' ) + } ); } // this doesn't change the soccerBallPhaseProperty of the soccerBall - that is done by the ball animationEnded callback @@ -174,7 +183,7 @@ this.isDraggingProperty.reset(); this.targetXProperty.value = null; - this.kicker = null; + this.kickerProperty.reset(); this.resetEmitter.emit(); } Index: js/model/SoccerSceneModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/SoccerSceneModel.ts b/js/model/SoccerSceneModel.ts --- a/js/model/SoccerSceneModel.ts (revision db6984b5c0089834b6ca38dd377514cc231b5691) +++ b/js/model/SoccerSceneModel.ts (date 1697587961286) @@ -195,7 +195,7 @@ else { // If the soccer player that kicked that ball was still in line when the ball lands, they can leave the line now. - if ( soccerBall.kicker === this.getFrontKicker() ) { + if ( soccerBall.kickerProperty.value === this.getFrontKicker() ) { this.advanceLine(); } } @@ -659,7 +659,7 @@ soccerBall.soccerBallPhaseProperty.value = SoccerBallPhase.FLYING; this.timeWhenLastBallWasKickedProperty.value = this.timeProperty.value; - soccerBall.kicker = kicker; + soccerBall.kickerProperty.value = kicker; playAudio && kickSound.play(); } Index: js/model/Kicker.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/Kicker.ts b/js/model/Kicker.ts --- a/js/model/Kicker.ts (revision db6984b5c0089834b6ca38dd377514cc231b5691) +++ b/js/model/Kicker.ts (date 1697588262547) @@ -18,8 +18,9 @@ import DerivedProperty from '../../../axon/js/DerivedProperty.js'; import TReadOnlyProperty from '../../../axon/js/TReadOnlyProperty.js'; import RegionAndCulturePortrayal from '../../../joist/js/preferences/RegionAndCulturePortrayal.js'; +import PhetioObject from '../../../tandem/js/PhetioObject.js'; -export default class Kicker { +export default class Kicker extends PhetioObject { // Which portion of the kicking animation the person is currently in. Determines which pose to set. public readonly kickerPhaseProperty: Property; @@ -35,6 +36,12 @@ public constructor( placeInLine: number, public readonly characterSetProperty: Property, tandem: Tandem ) { + super( { + phetioState: false, + isDisposable: false, + tandem: tandem + } ); + this.kickerPhaseProperty = new EnumerationProperty( placeInLine === 0 ? KickerPhase.READY : KickerPhase.INACTIVE, { tandem: tandem.createTandem( 'kickerPhaseProperty' ), phetioReadOnly: true ```
KatieWoe commented 1 year ago

This looks fixed in rc.2