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

Sound for FaucetNode #840

Open
pixelzoom opened this issue Jan 31, 2024 · 2 comments
Open

Sound for FaucetNode #840

pixelzoom opened this issue Jan 31, 2024 · 2 comments

Comments

@pixelzoom
Copy link
Contributor

Related to sound design for Faraday's Electromagnetic Lab (FEL), this issue involves both design and development.

From phetsims/faradays-electromagnetic-lab#15 (comment) ...

@arouinfar noted that we will need sound for FaucetNode. Since it's a common code component, we'll need default sounds, and the ability to override the defaults with sim-specific sounds.

Brainstorming aspects that might need sound:

  • Moving the shooter. Default might be the same as Slider default, since FaucetNode extends AccessibleSlider.
  • Tap-to-dispense featured. Default might be the same as PushButton.
  • flowRateProperty. Default might sound like water.

It will be useful for whoever is doing the sound design to consult the scenery-phet demo for FaucetNode, runnable from phetmarks, to see the important behaviors/options.

@pixelzoom
Copy link
Contributor Author

2/13/24 design meeting @arouinfar @Ashton-Morris @emily-phet @pixelzoom

@Ashton-Morris presented a mockup.

Sounds for "flow started" and "flowed ended", to add realism.

@Ashton-Morris wondered if we needed an additional "flow changed" sound.

"flow" sound continues while faucet is open. Should there be an option to fade the "flow" sound out after some period of time?

Possibly use noise generator (tambo NoiseGenerator.ts) for "flow".

"Tap to dispense" feature could use same sequence of "flow started" => "flow" => "flow ended" sound. @pixelzoom pointed out that "tap to dispense" is used to dispense small amounts of fluid, typically over milliseconds, so playing the full sequence might be problematic.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 27, 2024

I did a quick sprint on trying to add default sound to FaucetNode, similar to Slider, since FaucetNode and Slider both extend AccessibleSlider. Unfortunately, none of the sound code is in AccessibleSlider, it's all in Slider. And FaucetNode seems to be missing a KeyboardDragListener, so I could only get sound working for DragListener. Here's an uncommitted patch showing how far I got before I bailed.

patch
Subject: [PATCH] disable sonification, restore default grab/release sounds, https://github.com/phetsims/faradays-electromagnetic-lab/issues/77
---
Index: js/FaucetNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/FaucetNode.ts b/js/FaucetNode.ts
--- a/js/FaucetNode.ts	(revision 00eb337ee15fc3caf5e75ad60336698f897f78dc)
+++ b/js/FaucetNode.ts	(date 1708994808243)
@@ -24,9 +24,10 @@
 import Property from '../../axon/js/Property.js';
 import stepTimer from '../../axon/js/stepTimer.js';
 import Bounds2 from '../../dot/js/Bounds2.js';
+import Range from '../../dot/js/Range.js';
 import LinearFunction from '../../dot/js/LinearFunction.js';
 import InstanceRegistry from '../../phet-core/js/documentation/InstanceRegistry.js';
-import { Circle, DragListener, GroupHighlightPath, HighlightPath, Image, InteractiveHighlighting, KeyboardListener, Node, NodeOptions, Rectangle } from '../../scenery/js/imports.js';
+import { Circle, DragListener, GroupHighlightPath, HighlightPath, Image, InteractiveHighlighting, KeyboardDragListener, KeyboardListener, Node, NodeOptions, Rectangle } from '../../scenery/js/imports.js';
 import EventType from '../../tandem/js/EventType.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import IOType from '../../tandem/js/types/IOType.js';
@@ -49,6 +50,7 @@
 import AccessibleSlider, { AccessibleSliderOptions } from '../../sun/js/accessibility/AccessibleSlider.js';
 import { Shape } from '../../kite/js/imports.js';
 import WithOptional from '../../phet-core/js/types/WithOptional.js';
+import ValueChangeSoundPlayer, { ValueChangeSoundPlayerOptions } from '../../tambo/js/sound-generators/ValueChangeSoundPlayer.js';
 
 // constants
 const DEBUG_ORIGIN = false; // when true, draws a red dot at the origin (bottom-center of the spout)
@@ -76,6 +78,13 @@
 
   // options for the nested type ShooterNode
   shooterOptions?: ShooterNodeOptions;
+
+  // This is used to generate sounds as the shooter is moved by the user. If not provided, the default sound generator
+  // from Slider is used. If set to null, the shooter will generate no sound.
+  valueChangedSoundPlayer?: ValueChangeSoundPlayer | null;
+
+  // Options for the default sound generator. Ignored if soundGenerator is provided.
+  defaultSoundPlayerOptions?: ValueChangeSoundPlayerOptions | null;
 };
 type ParentOptions = AccessibleSliderOptions & NodeOptions;
 
@@ -88,7 +97,7 @@
   public constructor( maxFlowRate: number, flowRateProperty: Property<number>,
                       enabledProperty: TReadOnlyProperty<boolean>, providedOptions?: FaucetNodeOptions ) {
 
-    const options = optionize<FaucetNodeOptions, StrictOmit<SelfOptions, 'shooterOptions'>, WithOptional<ParentOptions, 'valueProperty'>>()( {
+    const options = optionize<FaucetNodeOptions, StrictOmit<SelfOptions, 'shooterOptions' | 'valueChangedSoundPlayer'>, WithOptional<ParentOptions, 'valueProperty'>>()( {
 
       // SelfOptions
       horizontalPipeLength: SPOUT_OUTPUT_CENTER_X,
@@ -99,9 +108,10 @@
       closeOnRelease: true,
       interactiveProperty: new Property( true ),
       rasterizeHorizontalPipeNode: false,
+      defaultSoundPlayerOptions: null,
 
       // AccessibleSliderOptions
-      enabledRangeProperty: new phet.axon.Property( new phet.dot.Range( 0, maxFlowRate ) ),
+      enabledRangeProperty: new Property( new Range( 0, maxFlowRate ) ),
       valueProperty: flowRateProperty,
 
       // ParentOptions
@@ -235,7 +245,14 @@
       this.phetioEndEvent();
     };
 
+    // If no sound generator was provided, create the default.
+    let valueChangedSoundPlayer: ValueChangeSoundPlayer | null;
+    if ( options.valueChangedSoundPlayer === undefined ) {
+      valueChangedSoundPlayer = new ValueChangeSoundPlayer( new Property( new Range( 0, maxFlowRate ) ), options.defaultSoundPlayerOptions || {} );
+    }
+
     let startXOffset = 0; // where the drag started, relative to the target node's origin, in parent view coordinates
+    let previousFlowRate = flowRateProperty.value;
     const dragListener = new DragListener( {
 
       start: event => {
@@ -264,6 +281,17 @@
           const xOffset = listener.currentTarget.globalToParentPoint( event.pointer.point ).x - startXOffset - bodyNode.left;
           const flowRate = offsetToFlowRate.evaluate( xOffset );
 
+          // Play sound as the value changes
+          if ( valueChangedSoundPlayer ) {
+            if ( event.isFromPDOM() ) {
+              valueChangedSoundPlayer.playSoundForValueChange( flowRateProperty.value, previousFlowRate );
+            }
+            else {
+              valueChangedSoundPlayer.playSoundIfThresholdReached( flowRateProperty.value, previousFlowRate );
+            }
+          }
+
+          previousFlowRate = flowRateProperty.value;
           flowRateProperty.set( flowRate );
         }
       },

Ideally (perhaps), AccessibleSlider should handle sound, so that anything that behaves like a slider would get default sounds. But that looks like a lot of work.

Also noting that Slider's API for sound is sort of confusing. options.soundGenerator and options.valueChangeSoundGeneratorOptions actually have nothing to do with SoundGenerator. Slider only takes a ValueChangeSoundPlayer, which is not a SoundGenerator.

Lastly... The default for the 'tap to dispense' feature should probably be similar to the default push button sound. But I didn't get far enough to investigate.

@Ashton-Morris Ashton-Morris removed their assignment Feb 27, 2024
pixelzoom added a commit to phetsims/faradays-electromagnetic-lab that referenced this issue Mar 5, 2024
@arouinfar arouinfar removed their assignment Mar 6, 2024
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

5 participants