phetsims / friction

"Friction" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/friction
GNU General Public License v3.0
4 stars 6 forks source link

observe responseCollector Properties to change Utterance alerts? #214

Closed zepumph closed 3 years ago

zepumph commented 3 years ago

From the quick fix in https://github.com/phetsims/friction/issues/209, I was thinking of maybe mutating the utterance array based on the current value of hintResponsesEnabledProperty. Is that allowed?

zepumph commented 3 years ago

@jessegreenberg said yes!!!!

Perhaps with something like this patch


Index: utterance-queue/js/Utterance.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/utterance-queue/js/Utterance.js b/utterance-queue/js/Utterance.js
--- a/utterance-queue/js/Utterance.js   (revision 6dad9f8862104303c78b132c68e45037c67c8bfb)
+++ b/utterance-queue/js/Utterance.js   (date 1628804358164)
@@ -142,6 +142,7 @@
    */
   set alert( alert ) {
     validate( alert, ALERT_VALIDATOR );
+    this.numberOfTimesAlerted = 0;

     this._alert = alert;
   }
Index: friction/js/friction/view/describers/TemperatureIncreasingDescriber.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/friction/js/friction/view/describers/TemperatureIncreasingDescriber.js b/friction/js/friction/view/describers/TemperatureIncreasingDescriber.js
--- a/friction/js/friction/view/describers/TemperatureIncreasingDescriber.js    (revision dbe8f0eb76bc9b9af2364e1e6a42f25d039118c8)
+++ b/friction/js/friction/view/describers/TemperatureIncreasingDescriber.js    (date 1628804389633)
@@ -15,6 +15,7 @@

 import stepTimer from '../../../../../axon/js/stepTimer.js';
 import StringUtils from '../../../../../phetcommon/js/util/StringUtils.js';
+import responseCollector from '../../../../../scenery/js/accessibility/voicing/responseCollector.js';
 import voicingUtteranceQueue from '../../../../../scenery/js/accessibility/voicing/voicingUtteranceQueue.js';
 import Utterance from '../../../../../utterance-queue/js/Utterance.js';
 import friction from '../../../friction.js';
@@ -103,6 +104,16 @@
         cancelSelf: false
       }
     } );
+
+    responseCollector.hintResponsesEnabledProperty.link( enabled => {
+      if ( enabled ) {
+        this.maxTempVoicingUtterance.alert.push( resetSimMoreObservationSentenceString );
+      }
+      else {
+        this.maxTempVoicingUtterance.alert.splice( this.maxTempVoicingUtterance.alert.indexOf( resetSimMoreObservationSentenceString ), 1 );
+      }
+    } );
+
     this.maxTempDescriptionUtterance = new Utterance( {
       alert: [ MAX_TEMP_STRING, MAX_TEMP_STRING, resetSimMoreObservationSentenceString ],
       loopAlerts: true
zepumph commented 3 years ago

Index: utterance-queue/js/UtteranceTests.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/utterance-queue/js/UtteranceTests.js b/utterance-queue/js/UtteranceTests.js
--- a/utterance-queue/js/UtteranceTests.js  (revision 96fffeb8f200a780a6a2f37642485132a0bd94ff)
+++ b/utterance-queue/js/UtteranceTests.js  (date 1629738364041)
@@ -109,6 +109,8 @@
   alert.reset();
   await alert4();
   testOrder( ', reset should start over' );
+  utterance.alert = [ '1', '2', '3' ];
+  testOrder( ', resetting alert should start over' );
 } );

Index: utterance-queue/js/Utterance.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/utterance-queue/js/Utterance.js b/utterance-queue/js/Utterance.js
--- a/utterance-queue/js/Utterance.js   (revision 96fffeb8f200a780a6a2f37642485132a0bd94ff)
+++ b/utterance-queue/js/Utterance.js   (date 1629737549122)
@@ -147,6 +147,7 @@
    */
   set alert( alert ) {
     validate( alert, ALERT_VALIDATOR );
+    this.numberOfTimesAlerted = 0;

     this._alert = alert;
   }
Index: friction/js/friction/view/describers/TemperatureIncreasingDescriber.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/friction/js/friction/view/describers/TemperatureIncreasingDescriber.js b/friction/js/friction/view/describers/TemperatureIncreasingDescriber.js
--- a/friction/js/friction/view/describers/TemperatureIncreasingDescriber.js    (revision 17eb3a661b857a2e95c7a9d49a1483da8245794b)
+++ b/friction/js/friction/view/describers/TemperatureIncreasingDescriber.js    (date 1629737549118)
@@ -15,6 +15,7 @@

 import stepTimer from '../../../../../axon/js/stepTimer.js';
 import StringUtils from '../../../../../phetcommon/js/util/StringUtils.js';
+import responseCollector from '../../../../../scenery/js/accessibility/voicing/responseCollector.js';
 import voicingUtteranceQueue from '../../../../../scenery/js/accessibility/voicing/voicingUtteranceQueue.js';
 import Utterance from '../../../../../utterance-queue/js/Utterance.js';
 import friction from '../../../friction.js';
@@ -107,6 +108,16 @@
         cancelSelf: false
       }
     } );
+
+    responseCollector.hintResponsesEnabledProperty.link( enabled => {
+      if ( enabled ) {
+        this.maxTempVoicingUtterance.alert.push( resetSimMoreObservationSentenceString );
+      }
+      else {
+        this.maxTempVoicingUtterance.alert.splice( this.maxTempVoicingUtterance.alert.indexOf( resetSimMoreObservationSentenceString ), 1 );
+      }
+    } );
+
     this.maxTempDescriptionUtterance = new Utterance( {
       alert: [ MAX_TEMP_STRING, MAX_TEMP_STRING, resetSimMoreObservationSentenceString ],
       loopAlerts: true

I want to write a unit test for the Utterance change, but will do https://github.com/phetsims/utterance-queue/issues/29 first.

zepumph commented 3 years ago

This worked very well. We may still want to tweak some timing over in https://github.com/phetsims/friction/issues/209, but I think it is ready for review over there.

zepumph commented 3 years ago

https://github.com/phetsims/utterance-queue/issues/31 has replaced this implementation, in #222, we converted these array-based alerts to be ResponesPackets instead. Closing