phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
MIT License
8 stars 6 forks source link

Voicing: Don't hear "Sim Voicing Off" bug #846

Closed zepumph closed 1 year ago

zepumph commented 1 year ago

To reproduce:

Expected Behavior: details button is interrupted then you hear "sim voicing off"

Current Behavior: details button is interrupted

jessegreenberg commented 1 year ago

Confirmed. I am not hearing any content when interrupting a "Quick Info" button (for example, pressing Reset All).

jessegreenberg commented 1 year ago

Caused by this line: https://github.com/phetsims/joist/blob/7c651545b65a38302e8814ee800aa7cf9b80637a/js/toolbar/VoicingToolbarItem.ts#L188-L189

jessegreenberg commented 1 year ago

We added this line because we don't want to hear stale Utterances that are sitting in the queue waiting for the toolbar button responses to finish. I think that is correct. But if we interrupt the the toolbar button responses we do want to hear the next response.

I pushed a proposed fix in the above commit and added a line of doc about where it falls short. But I think it should be acceptable. @zepumph can you please review and verify that this is fixed?

zepumph commented 1 year ago

I am worried about the comment you said. What about in friction, the temperature continues to get lower, and alerts pile up without any interaction. I also think this is quite possible in Greenhouse/MAL because of background sim state being described. I think next steps here are to discuss with @jessegreenberg.

jessegreenberg commented 1 year ago

Thinking about this more I think it is less of a shortcoming and more the correct behavior. I changed my doc language to describe why. I thought it was correct because 1) The queue is still cleared when toolbar button content stops speaking. 2) If there is some user input, we won't have a build-up of alerts. As soon as the user taps on the screen, the toolbar button content becomes lowest priority so anything waiting in the queue should be spoken immediately and interrupt toolbar button content.

I think generally the sim cases you mentioned should still work well. Still happy to discuss but back to @zepumph.

zepumph commented 1 year ago

@jessegreenberg and I discussed this, we are in agreement that the current implementation isn't ideal because if you interrupt a readme button by grabbing the book, it may say 10 "ambient" utterances waiting in the queue before saying "grabbed".

We discussed new priorities, and separating a priority for "AMBIENT" voicing vs "INTERACTIVE":

AMBIENT_HIGH AMBIENT_LOW README_AFTER INTERACTIVE_HIGH INTERACTIVE_LOW README_START

But didn't love it. We will keep thinking about it.

jessegreenberg commented 1 year ago

Ideas that are moderately to extremely yucky.

zepumph commented 1 year ago

Please note this issue is blocking the publication of Friction. I don't think @jessegreenberg and I have a direction forward just yet.

zepumph commented 1 year ago

@jessegreenberg and I realized that more central/powerful fixes were overkill, so we just made a workaround in the voicing toolbar code. The bug is fixed, and life is good. Closing!