Why keys for webrtcPeers?
maxkrieger opened this issue · 3 comments
Following the code in this PR's comment #6 (comment) doesn't work. provider.on("peers", (e) =>
sends out an event with no access to the simple-peer
objects (webrtcPeers
is a list of strings). I have to instead do provider.room.webrtcConns.get(peerId).peer
.
It's because of this line:
Line 109 in d9f2b28
also: I believe due to how defaults are handled, to get video streaming, you need to explicitly set streams: []
for peerOpts
if you want to dynamically add the stream later on. Not entirely sure, but worked for me.
Hi @maxkrieger ,
I added these options only for a hackathon that also wanted to do video streaming over y-webrtc.
My reasoning probably was that I don't want to send the whole webrtcConns object all the time. I only want to send the keys that changed.
It is better in this context to access peers like this: provider.room.webrtcConns.get(peerId).peer
, because internally I could change the simple-peer object at any time. So if the event above sends over a Map<PeerID, SimplePeer>
then the SimplePeer object might be outdated when you receive this event.
You are indirectly asking if I could change this event to make your use-case easier to handle. That's fair. But I don't have the time to support this use-case - it is unfortunately out-of-scope at the moment..
In any case, your change-request might break existing projects. So it seems like a very bad work/benefit ratio for me.
Btw, it's completely fair if you want to for y-webrtc to handle this use-case better. At some point I might get inspired by your changes and push them upstream. If you made some significant improvements and if you want to help maintain y-webrtc, I'd be happy to push your changes.
Totally reasonable and I hope I didn't sound too standoffish, I've had a great experience with this library and have been recommending it to others.
To me the behavior seemed like a bug (was inconsistent with the recommended usage in that comment) which is why I raised this — since it's relatively undocumented, the solution in my mind is less about breaking API changes than more documentation of the provider
's public state, which I would be happy to offer help with if I wasn't so bogged down as well. Regardless, this clarification makes total sense!