Conversation
hiroshihorie
left a comment
hiroshihorie
left a comment
There was a problem hiding this comment.
I think JS SDK emits both RemoteParticipant disconnected / connected events (by using the normal participant disconnect flow etc), maybe we should do that also ?
| // Re-add participants | ||
| var participantsToAdd: [Livekit_ParticipantInfo] = [] | ||
| if response.hasParticipant { | ||
| participantsToAdd.append(response.participant) | ||
| } | ||
| participantsToAdd.append(contentsOf: response.otherParticipants) | ||
|
|
||
| for info in participantsToAdd { | ||
| _state.mutate { $0.updateRemoteParticipant(info: info, room: self) } | ||
| } |
There was a problem hiding this comment.
It’s not well documented, but response.participant appears to be a LocalParticipant. We should probably call localParticipant.updateInfo instead. (I’m not sure if any of the info actually changes, though.)
|
@hiroshihorie I don't think that's a hi-priority task tbh, any reliable ways to test it? |
|
@pblazej Yes, probably not super high priority. I was hoping Livekit_SimulateScenario as a case but it looks like it doesn't. |
|
Let's try to prioritize #845 (more severe), then fix this one. |
|
@lukasIO lo-priority question: how was it tested on JS (e2e)? |
|
We should bump protocol ver to v16 |
| } | ||
|
|
||
| // Update local participant | ||
| if response.hasParticipant { |
There was a problem hiding this comment.
Not sure if we wanna validate this to avoid partial states?
| sender._set(subscribedQualities: qualities) | ||
| } | ||
|
|
||
| override func set(info: Livekit_ParticipantInfo, connectionState: ConnectionState) { |
There was a problem hiding this comment.
This is more of an inconsistency with JS.
| case v11 = 11 | ||
| /// Faster room join (delayed ``Room/sid``) | ||
| case v12 = 12 | ||
| /// Regions in leave request, `canReconnect` obsoleted by `action` |
There was a problem hiding this comment.
This would be a nice prerequisite to the changes actually introduced in #845
|
@hiroshihorie I think the basic logic + protocol is there, working with the CLI: Recording.at.2025-12-30.10.49.44.AM.Cropped.in.2025-12-30.10.52.43.AM.mp4 |
pblazej
force-pushed
the
blaze/room-moved
branch
from
0cb8486 to
ab59ad2
Compare
| localParticipant.set(info: response.participant, connectionState: _state.connectionState) | ||
| } | ||
|
|
||
| _republishLocalTracks() |
There was a problem hiding this comment.
@pblazej Can you check if this is required ? 🙏 We might not need to re-publish ?
There was a problem hiding this comment.
yes it is unfortunately, you can check by commenting that out
2 participants
No description provided.