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

Improve interval tool audio clip sound quality #307

Closed samreid closed 1 year ago

samreid commented 1 year ago

From https://github.com/phetsims/center-and-variability/issues/236

@emily-phet said:

I don't love the specific sound, please ask @Ashton-Morris to play around with altering it...something that sounds good alongside the woodblock sounds being used in the sim would be nice. The current interval tool sound feels like it was pulled from a different sim (which it was!), in a not so good way.

samreid commented 1 year ago

Note there is a TODO in the code that says:

// TODO: https://github.com/phetsims/center-and-variability/issues/307 should this be in common code? It was copied from GFL
import saturatedSineLoopTrimmed_wav from '../../../sounds/saturatedSineLoopTrimmed_wav.js';

Let's determine what audio clip to use first.

samreid commented 1 year ago

@Ashton-Morris said:

I am adding these sounds I created to test as alternative loops. They are in our normal folder and I have included .wav files as well incase we want to hear it loop seamlessly for testing as we work on the .mp3 looping.

cvIntervalToolLoopSoundV1.wav cvIntervalToolLoopSoundV3.mp3 cvIntervalToolLoopSoundV3.wav cvIntervalToolLoopSoundV2.mp3 cvIntervalToolLoopSoundV2.wav cvIntervalToolLoopSoundV1.mp3

Also I added these sounds which we used in My Solar System if anyone thinks they may fit.

Lastly I attached one that we used for a tool in Waves Intro

samreid commented 1 year ago

I would also like to ask: based on #309 and #310 we are interested in the more discrete or pulsing sounds. Therefore I wonder whether we need to use the continuous sound clips at all. For instance, for the mean prediction continuous sound, we just play values when passing certain thresholds (or on any keyboard press, like shift+press). Perhaps we should do the same for the interval tool? If we move toward discrete sound clips like the mean prediction, it can free us up from constraints in the continuous sound clip. @Ashton-Morris @catherinecarter what do you think? One of the tradeoffs is that the continuous sound generator can sound like a "bend" in pitch as it moves, but we didn't feel the need to support that for the mean prediction arrow.

Ashton-Morris commented 1 year ago

Lets decide which direction during out meeting later today. I could go either way.

samreid commented 1 year ago

We tried 2 of the sounds above, and they sound much better, nice work @Ashton-Morris. We think this is a good direction (continuous sound, no thresholding), but want to discuss it with @Ashton-Morris @emily-phet and @kathy-phet before we commit:

```diff Subject: [PATCH] Fix alignment of readouts in the variability accordion box --- Index: js/variability/view/VariabilityScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/variability/view/VariabilityScreenView.ts b/js/variability/view/VariabilityScreenView.ts --- a/js/variability/view/VariabilityScreenView.ts (revision 9022f039d5acc94a288b0c9a493276ee13c2e5c4) +++ b/js/variability/view/VariabilityScreenView.ts (date 1687811039692) @@ -29,7 +29,6 @@ import PredictionSlider, { PredictionSliderOptions } from '../../common/view/PredictionSlider.js'; import Property from '../../../../axon/js/Property.js'; import BooleanProperty from '../../../../axon/js/BooleanProperty.js'; -import NumberProperty from '../../../../axon/js/NumberProperty.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; import Range from '../../../../dot/js/Range.js'; import IntervalToolNode from './IntervalToolNode.js'; @@ -37,12 +36,15 @@ import soundManager from '../../../../tambo/js/soundManager.js'; // TODO: https://github.com/phetsims/center-and-variability/issues/307 should this be in common code? It was copied from GFL -import saturatedSineLoopTrimmed_wav from '../../../sounds/saturatedSineLoopTrimmed_wav.js'; +// import saturatedSineLoopTrimmed_wav from '../../../sounds/saturatedSineLoopTrimmed_wav.js'; +import saturatedSineLoopTrimmed_wav from '../../../sounds/cvWaveMeterSawTone_mp3.js'; +// import saturatedSineLoopTrimmed_wav from '../../../sounds/cvIntervalToolLoopSoundV1_wav.js'; import Utils from '../../../../dot/js/Utils.js'; import CAVQueryParameters from '../../common/CAVQueryParameters.js'; import phetAudioContext from '../../../../tambo/js/phetAudioContext.js'; import CAVSoccerSceneModel from '../../common/model/CAVSoccerSceneModel.js'; import SoccerPlayerGroupNumbered from '../../soccer-common/view/SoccerPlayerGroupNumbered.js'; +import NumberProperty from '../../../../axon/js/NumberProperty.js'; type SelfOptions = EmptySelfOptions; type VariabilityScreenViewOptions = SelfOptions & StrictOmit; @@ -127,32 +129,35 @@ intervalToolNode.inputEnabled = !keyboard1 && !mouse1 && !keyboard2 && !mouse2; } ); - const intervalDistanceProperty = new DerivedProperty( [ model.intervalToolDeltaStableProperty ], interval => { - return Utils.roundToInterval( Utils.linear( 0, 16, 2, 1, interval ), CAVQueryParameters.intervalToolSoundInterval ); + const fakeValueProperty = new NumberProperty( 0 ); + const intervalDistanceProperty = new DerivedProperty( [ model.intervalToolDeltaStableProperty, fakeValueProperty ], ( interval, fakeValue ) => { + return fakeValue + Utils.roundToInterval( Utils.linear( 0, 16, 2, 1, interval ), CAVQueryParameters.intervalToolSoundInterval ); } ); // the minimum distance that the interval must change before a sound is played (to prevent sound playing on tiny movements) - const INTERVAL_WIDTH_CHANGE_THRESHOLD = 0.01; - const INTERVAL_POSITION_CHANGE_THRESHOLD = 0.1; - - let lastIntervalWidthValue = intervalDistanceProperty.value; - let lastIntervalTool1Value = model.intervalTool1ValueProperty.value; + // const INTERVAL_WIDTH_CHANGE_THRESHOLD = 0.01; + // const INTERVAL_POSITION_CHANGE_THRESHOLD = 0.1; - const intervalDistanceWithThresholdProperty = new NumberProperty( lastIntervalWidthValue ); + // let lastIntervalWidthValue = intervalDistanceProperty.value; + // let lastIntervalTool1Value = model.intervalTool1ValueProperty.value; - intervalDistanceProperty.link( newValue => { - if ( Math.abs( newValue - lastIntervalWidthValue ) >= INTERVAL_WIDTH_CHANGE_THRESHOLD ) { - intervalDistanceWithThresholdProperty.value = newValue; - lastIntervalWidthValue = newValue; - } - } ); - - // keeps track of the translation of the entire interval tool + // const intervalDistanceWithThresholdProperty = new NumberProperty( intervalDistanceProperty ); + // + // intervalDistanceProperty.link( newValue => { + // if ( Math.abs( newValue - lastIntervalWidthValue ) >= INTERVAL_WIDTH_CHANGE_THRESHOLD ) { + // intervalDistanceWithThresholdProperty.value = newValue; + // lastIntervalWidthValue = newValue; + // } + // } ); + // + // // keeps track of the translation of the entire interval tool model.intervalTool1ValueProperty.link( newValue => { - if ( Math.abs( newValue - lastIntervalTool1Value ) >= INTERVAL_POSITION_CHANGE_THRESHOLD ) { - intervalDistanceWithThresholdProperty.notifyListenersStatic(); - lastIntervalTool1Value = newValue; - } + // intervalDistanceProperty.value += 1E-12; + // if ( Math.abs( newValue - lastIntervalTool1Value ) >= INTERVAL_POSITION_CHANGE_THRESHOLD ) { + // intervalDistanceWithThresholdProperty.notifyListenersStatic(); + // lastIntervalTool1Value = newValue; + // } + fakeValueProperty.value += 1E-12; } ); const biquadFilterNode = new BiquadFilterNode( phetAudioContext, { @@ -177,13 +182,15 @@ if ( value ) { biquadFilterNode.frequency.setTargetAtTime( 600, phetAudioContext.currentTime, 0 ); } + + intervalDistanceProperty.notifyListenersStatic(); } ); this.continuousPropertySoundGenerator = new ContinuousPropertySoundGenerator( - intervalDistanceWithThresholdProperty, + intervalDistanceProperty, saturatedSineLoopTrimmed_wav, new Range( 1, 2 ), { - initialOutputLevel: 0.3, + initialOutputLevel: 1.2, playbackRateCenterOffset: 0, resetInProgressProperty: model.variabilityModelResetInProgressProperty, ```
samreid commented 1 year ago

Closing as duplicate of https://github.com/phetsims/center-and-variability/issues/307

phet-dev commented 1 year ago

Reopening because there is a TODO marked for this issue.

catherinecarter commented 1 year ago

Sound balancing will need to happen, the sound is pretty loud right now.

@emily-phet isn't hearing the double sound, but we will have to check this to ensure the interactions are working with small movements.

In terms of the length of time before the sound stops, it sounds good. The sound has an appropriate balance between stopping too soon and hanging on before stopping.

samreid commented 1 year ago

I fixed the TODO. Sound balancing happening in #304. I'll open a side issue about the excess sound when letting go of the mouse on the interval tool.

samreid commented 1 year ago

All requests addressed or indicated in other issues. Closing.