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

Remove default TinyProperty from Utterance.canAnnounceProperties #79

Closed
jessegreenberg opened this issue Apr 14, 2022 · 3 comments
Closed
Assignees

Comments

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Apr 14, 2022

This Property is always there, when you add a new Property it doesn't ever change or remove this default. Having it there is convenient for the implementation of the canAnnounceProperty because the DerivedProperty needs at least one dependency. But it makes an assertion like this (maybe to be added to Voicing.ts) irrelevant because the length is always greater than zero

  assert && assert( utterance.canAnnounceProperties.length > 0, 'canAnnounceProperties required, this Utterance might not be connected to Node in the scene graph.' );
@jessegreenberg
Copy link
Contributor Author

This assertion was added to Voicing.ts as part of phetsims/scenery#1403 so it would be nice to improve this.

@jessegreenberg
Copy link
Contributor Author

This was done in the above commit. After the change I immediately found an Utterance that was not attached to the scene graph for Voicing which is great.

@jessegreenberg
Copy link
Contributor Author

Sims passed local fuzzing. I am going to go ahead and close this one but feel free to review if you wish as part of phetsims/scenery#1403 @zepumph.

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

1 participant