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

Problem using the same Announcer and Utterance with two different UtteranceQueues #48

Closed
jessegreenberg opened this issue Jan 21, 2022 · 2 comments

Comments

@jessegreenberg
Copy link
Contributor

Announcer.announcementCompleteEmitter introduces a problem with using the same Announcer with the same Utterance with two different UtteranceQueues.

Consider the following

const testUtterance = new phet.utteranceQueue.Utterance( {
    alert: 'This is a test utterance.',
    alertStableDelay: 0,
    announcerOptions: { cancelSelf: false }
} );
phet.scenery.voicingUtteranceQueue.addToBack( testUtterance )
phet.joist.joistVoicingUtteranceQueue.addToBack( testUtterance )

When the phet.scenery.voicingUtteranceQueue is finished announcing, the announcementCompleteEmitter` will fire. A listener in both UtteranceQueues will try to remove listeners from testUtterance.priorityProperty. Each UtteranceQueue in this case thinks that its Announcer is finished with the testUtterance, even though only the voicingUtteranceQueue finished speaking it.

This is a very rare case, and we will approach this with low priority until we encounter it.

@jessegreenberg
Copy link
Contributor Author

I think we addressed this in the check added in #46

        // It is possible that this.announcer is also used by a different UtteranceQueue so when
        // announcementCompleteEmitter emits, it may not be for this UtteranceWrapper. this.announcingUtteranceWrapper
        // and its announcingUtterancePriorityListener could only have been set by this queue, so this check ensures
        // that we are removing the priorityProperty listener from the correct Utterance.
        if ( this.announcingUtteranceWrapper.utterance.priorityProperty.hasListener( announcingUtterancePriorityListener ) ) {
          this.announcingUtteranceWrapper.utterance.priorityProperty.unlink( announcingUtterancePriorityListener );
          // ......

I think this can be closed, @zepumph do you agree?

@zepumph
Copy link
Member

zepumph commented Jan 24, 2022

Yes, I remember now. Thanks!

@zepumph zepumph closed this as completed Jan 24, 2022
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

2 participants