fix TypeError: Cannot read property enableMicrophone of undefined error (fixes #394)
#395
Conversation
There was a problem hiding this comment.
This looks good but I would also fix toggleFreeze while you're at it, since it occurs for the same reason.
https://github.com/mozilla/hubs/blob/master/src/components/freeze-controller.js#L20
…rror in `src/components/mute-mic.js` (fixes #394)
|
@brianpeiris 👍 thanks, good idea. updated. let me know! |
| @@ -273,16 +273,22 @@ const onReady = async () => { | |||
| mediaStream.removeTrack(track); | |||
| } | |||
| } | |||
| NAF.connection.adapter.setLocalMediaStream(mediaStream); | |||
| if (NAF.connection.adapter && NAF.connection.adapter.setLocalMediaStream) { | |||
There was a problem hiding this comment.
can this be simplified to check only NAF.connection.adapter?
There was a problem hiding this comment.
I think we wanted to check for setLocalMediaStream specifically, in case we were ever using anything other than naf-janus-adapter. setLocalMediaStream is special to our adapter.
There was a problem hiding this comment.
okay, yeah, that's along what I thought
src/components/block-button.js
Outdated
| }, | ||
|
|
||
| // Currently unused | ||
| unblock(clientId) { | ||
| NAF.connection.adapter.unblock(clientId); | ||
| NAF.connection.entities.completeSync(clientId); | ||
| if (NAF.connection) { |
There was a problem hiding this comment.
do you know if we can expect NAF.connection to always be a truthy property?
There was a problem hiding this comment.
Yes, NAF.connection is always a NetworkConnection object, it's defined immediately when you reference networked-aframe
src/components/block-button.js
Outdated
| } | ||
| if (NAF.connection.entities) { | ||
| NAF.connection.entities.completeSync(clientId); | ||
| } |
There was a problem hiding this comment.
Arguably, guarding this block/unblock code is a bit too overzealous, since you can (currently) only ever run this code if there are avatars in the room, and that's only possible if an adapter has been defined.
I would remove the NAF.connection guard though, since that is always defined, as I mentioned above.
There was a problem hiding this comment.
gotcha, thanks! adjusted
src/hub.js
Outdated
| screenEntity.setAttribute("visible", sharingScreen); | ||
| }); | ||
|
|
||
| document.body.addEventListener("blocked", ev => { | ||
| NAF.connection.entities.removeEntitiesOfClient(ev.detail.clientId); | ||
| if (NAF.connection.entities) { |
There was a problem hiding this comment.
so it should be safe to remove this check?
There was a problem hiding this comment.
Yup, entities is also always defined.
| @@ -37,6 +37,10 @@ AFRAME.registerComponent("networked-video-player", { | |||
| document.body.appendChild(container); | |||
| } | |||
|
|
|||
| if (!NAF.connection.adapter || !NAF.connection.adapter.getMediaStream) { | |||
| return; | |||
| } | |||
There was a problem hiding this comment.
This seems unnecessary. connection.adapter should definitely be defined at this point, since we wait for nafConnected above.
Also, getMediaStream should definitely be defined if adapter is.
Also not sure how I feel about returning silently if this condition occurred, I'd rather it fail loudly, if it does ever fail.
|
Gonna push these commits in #804 |
No description provided.