phetsims / projectile-data-lab

"Projectile Data Lab" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 0 forks source link

Add sound when jumping to first/last launched projectile #285

Closed Nancy-Salpepi closed 2 months ago

Nancy-Salpepi commented 3 months ago

For https://github.com/phetsims/qa/issues/1068, @matthew-blackman and I both agree that a sound should be heard when using home and end keys to jump to the first and last projectile on the field sign.

samreid commented 3 months ago

I'll take a look at this one.

samreid commented 3 months ago

Here is a working patch. I left debugging information in for the review. @matthew-blackman let's take a look together:

```diff Subject: [PATCH] Increase memory available to QuickServer node processes, see https://github.com/phetsims/chipper/issues/1431 --- Index: js/common/view/SelectorNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/SelectorNode.ts b/js/common/view/SelectorNode.ts --- a/js/common/view/SelectorNode.ts (revision 346f6024a6d422430030bbc71d8d464e24064114) +++ b/js/common/view/SelectorNode.ts (date 1712849481029) @@ -7,7 +7,7 @@ * @author Sam Reid (PhET Interactive Simulations) */ -import { Color, Node, NodeOptions, Path, SceneryConstants } from '../../../../scenery/js/imports.js'; +import { Color, KeyboardListener, Node, NodeOptions, Path, SceneryConstants } from '../../../../scenery/js/imports.js'; import AccessibleNumberSpinner, { AccessibleNumberSpinnerOptions } from '../../../../sun/js/accessibility/AccessibleNumberSpinner.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; @@ -157,6 +157,28 @@ // support for binder documentation, stripped out in builds and only runs when ?binder is specified assert && phet.chipper.queryParameters.binder && InstanceRegistry.registerDataURL( 'sun', 'SelectorNode', this ); + + let priorValue: number | null = null; + + assert && assert( this.startInput === _.noop, 'start input should be a no-op before we change it' ); + this.startInput = event => { + + // When input begins, record what the value was before it changes so that when we process a home/end event, + // we do not play duplicate sounds if it was already at the home/end + priorValue = numberProperty.value; + }; + + // Add sound effects for pressing the home/end keys. Note this callback is called after the number property was updated. + // Do not use this.onInput since this would double some of the sounds when the buttons are interacted with in a certain way. + this.addInputListener( new KeyboardListener( { + keys: [ 'home', 'end' ] as const, + callback: () => { + console.log( 'home/end' ); + if ( numberProperty.value !== priorValue ) { + options.playSound( numberProperty.value ); + } + } + } ) ); } } Index: js/common/model/ProjectileSound.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/model/ProjectileSound.ts b/js/common/model/ProjectileSound.ts --- a/js/common/model/ProjectileSound.ts (revision 346f6024a6d422430030bbc71d8d464e24064114) +++ b/js/common/model/ProjectileSound.ts (date 1712846026046) @@ -52,6 +52,7 @@ public static play( projectileType: ProjectileType, x: number, isLanding: boolean ): void { + console.log('soundplay'); const config = projectileSoundsConfig.get( projectileType )!; assert && assert( config, `Projectile type configuration not found for ${projectileType.phetioID}` );
matthew-blackman commented 3 months ago

This should be fixed on main. Nice work @samreid!

Nancy-Salpepi commented 3 months ago

Yes! Sounds good 👂🏻on main.

matthew-blackman commented 2 months ago

Please close after verifying.

KatieWoe commented 2 months ago

Sounds good in rc.2