Skip to content

Commit

Permalink
instantiate one viewSounds per interaction, #290
Browse files Browse the repository at this point in the history
  • Loading branch information
zepumph committed Dec 22, 2020
1 parent 5690f1e commit 04e74e4
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 16 deletions.
14 changes: 9 additions & 5 deletions js/common/view/BothHandsPDOMNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import Utterance from '../../../../utterance-queue/js/Utterance.js';
import ratioAndProportion from '../../ratioAndProportion.js';
import ratioAndProportionStrings from '../../ratioAndProportionStrings.js';
import BothHandsInteractionListener from './BothHandsInteractionListener.js';
import ViewSounds from './sound/ViewSounds.js';

class BothHandsPDOMNode extends Node {

Expand All @@ -31,7 +32,7 @@ class BothHandsPDOMNode extends Node {
* @param {Property.<number>} unclampedFitnessProperty
* @param {RatioDescriber} ratioDescriber
* @param {BothHandsDescriber} bothHandsDescriber
* @param {ViewSounds} viewSounds
* @param {BooleanProperty} playTickMarkBumpSoundProperty
* @param {BooleanProperty} ratioLockedProperty
* @param {Property.<number>} targetRatioProperty
* @param {function(RatioTerm):number} getIdealTerm
Expand All @@ -46,7 +47,7 @@ class BothHandsPDOMNode extends Node {
unclampedFitnessProperty,
ratioDescriber,
bothHandsDescriber,
viewSounds,
playTickMarkBumpSoundProperty,
ratioLockedProperty,
targetRatioProperty,
getIdealTerm,
Expand Down Expand Up @@ -81,6 +82,8 @@ class BothHandsPDOMNode extends Node {
this.consequentInteractedWithProperty = new BooleanProperty( false );
this.bothHandsFocusedProperty = new BooleanProperty( false );

this.viewSounds = new ViewSounds( tickMarkRangeProperty, tickMarkViewProperty, playTickMarkBumpSoundProperty );

Property.multilink( [
this.antecedentInteractedWithProperty,
this.consequentInteractedWithProperty
Expand Down Expand Up @@ -115,7 +118,7 @@ class BothHandsPDOMNode extends Node {
// @private
this.bothHandsInteractionListener = new BothHandsInteractionListener( interactiveNode, ratioTupleProperty,
this.antecedentInteractedWithProperty, this.consequentInteractedWithProperty, enabledRatioTermsRangeProperty, tickMarkRangeProperty, keyboardStep,
viewSounds.boundarySoundClip, viewSounds.tickMarkBumpSoundClip, ratioLockedProperty, targetRatioProperty, getIdealTerm, {
this.viewSounds.boundarySoundClip, this.viewSounds.tickMarkBumpSoundClip, ratioLockedProperty, targetRatioProperty, getIdealTerm, {
onInput: () => {
this.alertBothHandsContextResponse();
}
Expand All @@ -125,11 +128,11 @@ class BothHandsPDOMNode extends Node {
interactiveNode.addInputListener( {
focus: () => {
this.alertBothHandsObjectResponse( tickMarkViewProperty.value );
viewSounds.grabSoundClip.play();
this.viewSounds.grabSoundClip.play();
this.bothHandsFocusedProperty.value = true;
},
blur: () => {
viewSounds.releaseSoundClip.play();
this.viewSounds.releaseSoundClip.play();
this.bothHandsFocusedProperty.value = false;

// This only works because the bothHandsInteractionListener needs alt-input control resetting
Expand Down Expand Up @@ -187,6 +190,7 @@ class BothHandsPDOMNode extends Node {
this.bothHandsInteractionListener.reset();
this.objectResponseUtterance.reset();
this.contextResponseUtterance.reset();
this.viewSounds.reset();
}

/**
Expand Down
12 changes: 3 additions & 9 deletions js/common/view/RAPScreenView.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import RatioHalf from './RatioHalf.js';
import InProportionSoundGenerator from './sound/InProportionSoundGenerator.js';
import MovingInProportionSoundGenerator from './sound/MovingInProportionSoundGenerator.js';
import StaccatoFrequencySoundGenerator from './sound/StaccatoFrequencySoundGenerator.js';
import ViewSounds from './sound/ViewSounds.js';
import TickMarkView from './TickMarkView.js';
import TickMarkViewRadioButtonGroup from './TickMarkViewRadioButtonGroup.js';

Expand Down Expand Up @@ -124,10 +123,6 @@ class RAPScreenView extends ScreenView {
const playTickMarkBumpSoundProperty = new DerivedProperty( [ model.ratioFitnessProperty ],
fitness => !model.ratio.lockedProperty.value && fitness === model.fitnessRange.min );

// @private
this.viewSounds = new ViewSounds( this.tickMarkRangeProperty,
this.tickMarkViewProperty, playTickMarkBumpSoundProperty );

// by default, the keyboard step size should be half of one default tick mark width. See https://github.com/phetsims/ratio-and-proportion/issues/85
const keyboardStep = 1 / 2 / this.tickMarkRangeProperty.value;

Expand All @@ -152,7 +147,7 @@ class RAPScreenView extends ScreenView {
keyboardStep,
model.ratio.lockedProperty,
model.ratio.lockedProperty, // not a bug
this.viewSounds,
playTickMarkBumpSoundProperty,
this.inProportionSoundGenerator,
() => model.getIdealValueForTerm( RatioTerm.ANTECEDENT ), {
handColorProperty: options.leftHandColorProperty,
Expand Down Expand Up @@ -183,7 +178,7 @@ class RAPScreenView extends ScreenView {
keyboardStep,
model.ratio.lockedProperty,
model.ratio.lockedProperty, // not a bug
this.viewSounds,
playTickMarkBumpSoundProperty,
this.inProportionSoundGenerator,
() => model.getIdealValueForTerm( RatioTerm.CONSEQUENT ), {
handColorProperty: options.rightHandColorProperty,
Expand All @@ -201,7 +196,7 @@ class RAPScreenView extends ScreenView {
model.unclampedFitnessProperty,
this.ratioDescriber,
bothHandsDescriber,
this.viewSounds,
playTickMarkBumpSoundProperty,
model.ratio.lockedProperty,
model.targetRatioProperty,
model.getIdealValueForTerm.bind( model ), merge( {
Expand Down Expand Up @@ -395,7 +390,6 @@ class RAPScreenView extends ScreenView {
this.staccatoFrequencySoundGenerator.reset();
this.inProportionSoundGenerator.reset();
this.movingInProportionSoundGenerator.reset();
this.viewSounds.reset();
}

/**
Expand Down
8 changes: 6 additions & 2 deletions js/common/view/RatioHalf.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import RAPConstants from '../RAPConstants.js';
import CueDisplay from './CueDisplay.js';
import RatioHalfTickMarksNode from './RatioHalfTickMarksNode.js';
import RatioHandNode from './RatioHandNode.js';
import ViewSounds from './sound/ViewSounds.js';
import TickMarkView from './TickMarkView.js';

// constants
Expand Down Expand Up @@ -72,7 +73,7 @@ class RatioHalf extends Rectangle {
* @param {number} keyboardStep
* @param {BooleanProperty} horizontalMovementAllowedProperty
* @param {BooleanProperty} ratioLockedProperty
* @param {ViewSounds} viewSounds
* @param {BooleanProperty} playTickMarkBumpSoundProperty
* @param {InProportionSoundGenerator} inProportionSoundGenerator
* @param {function():number} getIdealValue - a function that gets the value of this RatioHalf term that would achieve the targetRatio
* @param {Object} [options]
Expand All @@ -91,7 +92,7 @@ class RatioHalf extends Rectangle {
keyboardStep,
horizontalMovementAllowedProperty,
ratioLockedProperty,
viewSounds,
playTickMarkBumpSoundProperty,
inProportionSoundGenerator,
getIdealValue,
options ) {
Expand Down Expand Up @@ -131,6 +132,8 @@ class RatioHalf extends Rectangle {
this.tickMarkViewProperty = tickMarkViewProperty;
this.valueProperty = valueProperty;

const viewSounds = new ViewSounds( tickMarkRangeProperty, tickMarkViewProperty, playTickMarkBumpSoundProperty );

// This follows the spec outlined in https://github.com/phetsims/ratio-and-proportion/issues/81
const cueDisplayStateProperty = new DerivedProperty( [
cueArrowsState.interactedWithKeyboardProperty,
Expand Down Expand Up @@ -352,6 +355,7 @@ class RatioHalf extends Rectangle {
// @private
this.resetRatioHalf = () => {
this.ratioHandNode.reset();
viewSounds.reset();
positionProperty.value.setX( INITIAL_X_VALUE );
positionProperty.notifyListenersStatic();
};
Expand Down

0 comments on commit 04e74e4

Please sign in to comment.