Skip to content

Send ReadyForRoomEvent after Connect to flush queued room events#282

Open
MaxHeimbrock wants to merge 1 commit into
mainfrom
max/ffi-ready-for-room-event
Open

Send ReadyForRoomEvent after Connect to flush queued room events#282
MaxHeimbrock wants to merge 1 commit into
mainfrom
max/ffi-ready-for-room-event

Conversation

@MaxHeimbrock
Copy link
Copy Markdown
Contributor

@MaxHeimbrock MaxHeimbrock commented May 15, 2026

Summary

Depends on: livekit/rust-sdks#1068 - have to check why this even passes one of the two test suites without.

  • Room.OnConnect now sends the new FFI ReadyForRoomEventRequest after subscribing to FfiClient.RoomEventReceived and before firing the public Connected event, completing the new ready-handshake the Rust SDK requires.
  • Adds a PlayMode regression test (LateJoinTrackSubscriptionTests) that publishes 2 audio + 2 video tracks first, then connects a late joiner with TrackSubscribed pre-wired, and asserts every snapshot publication produces exactly one TrackSubscribed event.
  • Regenerates the C# proto bindings (the new request/response, plus mechanical drift from running protoc 34 against the current Rust protos) and patches DataTrack.cs for the DataTrackStreamEOS.error field type change.

Why

Without ReadyForRoomEventRequest, room events emitted between ConnectCallback and the C# listener registration could be silently dropped — late joiners would miss TrackSubscribed / ParticipantConnected for state that already existed at connect time. The new Rust contract (on client-sdk-rust branch ladvoc/ffi-event-flush) makes this deterministic: the connect task parks for 1 s waiting for ReadyForRoomEventRequest and disconnects the room with ConnectionTimeout if it never arrives. This PR ports that contract to the Unity SDK.

Test plan

  • Scripts~/run_unity.sh test -m PlayMode -f LateJoinTrackSubscriptionTests
    • Pre-fix: fails with Rust log timed out waiting for ReadyForRoomEventRequest after ConnectCallback
    • Post-fix: passes; LiveKit server log confirms 4 tracks published + subscribed
  • Full PlayMode suite: 42/56 pass. The 10 failures are all pre-existing data-track issues (DataTrackTests.*, FfiBoundaryTests.Disconnect_CalledTwice_DoesNotThrow) surfacing as Failed to open data track publish transport: connection error: closed — not introduced by this PR.

Notes for reviewers

  • The Rust submodule pointer is not updated in this PR. The matching Rust change is on client-sdk-rust branch ladvoc/ffi-event-flush and must merge to main before this PR's submodule pointer is updated and merged. Until then, this PR can be tested locally by checking out that branch in client-sdk-rust~/ and rebuilding the FFI dylib.
  • The proto-regen diff is large but mechanical: ~8.6k inserts across 6 generated files due to protoc 34 + cumulative drift since the last regen. Behavioural change is limited to the new ReadyForRoomEvent request/response and the consumer fix in Runtime/Scripts/DataTrack.cs:393.

🤖 Generated with Claude Code

The Rust FFI now parks the connect task after sending ConnectCallback and
only spawns event-forwarding tasks once the client sends
ReadyForRoomEventRequest. Without it, room events emitted between Connect
and the C# listener registration were dropped (late-join scenario), and
on the new Rust SDK the room is closed with ConnectionTimeout after 1s.

Room.OnConnect now sends the ready request immediately after subscribing
to FfiClient.RoomEventReceived and before firing the public Connected
event. Adds a PlayMode regression test that publishes 4 tracks before a
subscriber joins late and asserts every TrackSubscribed event arrives
exactly once.

Proto bindings regenerated via Scripts~/generate_proto.sh against the
new ffi.proto / room.proto. DataTrack.cs consumer updated for the
DataTrackStreamEOS.error field type change (string -> message).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant