Skip to content

Stage 1: Add GetAwaiter to YieldInstruction + StreamYieldInstruction#289

Draft
MaxHeimbrock wants to merge 2 commits into
mainfrom
max/yield-instruction-getawaiter
Draft

Stage 1: Add GetAwaiter to YieldInstruction + StreamYieldInstruction#289
MaxHeimbrock wants to merge 2 commits into
mainfrom
max/yield-instruction-getawaiter

Conversation

@MaxHeimbrock

Copy link
Copy Markdown
Contributor

Summary

  • Adds GetAwaiter() to YieldInstruction and StreamYieldInstruction so callers can await room.Connect(...) and similar one-shot/streaming FFI ops without taking on a UniTask dependency.
  • Continuation is invoked from the existing IsDone / IsCurrentReadDone / IsEos property setters — every existing concrete instruction (Connect, PublishTrack, PerformRpc, SendText/File, stream open/write/close, etc.) becomes awaitable with no change to its completion path.
  • Race between FFI-thread completion and main-thread await registration is resolved with a sentinel-value Interlocked.CompareExchange on a single continuation slot. Reset() clears the slot so each chunk of a stream gets its own awaiter.
  • GetResult() is a no-op so the await surface keeps strict parity with yield return (callers still inspect IsError). A throwing variant can be layered on later if we want it.

This is Stage 1 of the staged UniTask migration described in Logs~/unitask-investigation/unitask-migration-analysis.md. It is non-breaking — no public API changes, no new dependencies, all coroutine call sites untouched. If you like the shape, Stage 2 layers an optional UniTask surface behind a scripting define.

Test plan

  • Scripts~/run_unity.sh build macos — clean compile.
  • Scripts~/run_unity.sh test -m PlayMode -f Connect_FailsWithInvalidUrl_Awaitable — new test passes; awaiter resumes and observes IsError.
  • Scripts~/run_unity.sh test -m PlayMode -f 'Connect_FailsWithInvalidUrl$' — original coroutine-path test still passes (no regression).
  • Reviewer: spot-check await room.Connect(...) in a sample once a live livekit-server --dev is running, to confirm main-thread resumption.

🤖 Generated with Claude Code

@MaxHeimbrock MaxHeimbrock marked this pull request as draft May 21, 2026 15:24
@MaxHeimbrock

Copy link
Copy Markdown
Contributor Author

I was able to convert OnStartCall to async void and then ConnectToRoom() to async Task with awaiter instead of yield.

So this worked well.

Stage 1 of the UniTask migration: enable `await room.Connect(...)` and
similar without taking on a UniTask dependency. The awaiter's continuation
is invoked from the existing IsDone / IsCurrentReadDone / IsEos property
setters, so all nine concrete instructions (Connect, PublishTrack, RPC,
SendText/File, stream open/write/close, etc.) become awaitable with no
change to their completion code paths.

Race between FFI-thread completion and main-thread await registration is
resolved with a sentinel-value Interlocked.CompareExchange on a single
continuation slot. GetResult() is intentionally a no-op so the await
surface keeps strict parity with `yield return` (callers still inspect
IsError); a throwing variant can be layered on later.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@MaxHeimbrock MaxHeimbrock force-pushed the max/yield-instruction-getawaiter branch from da5555c to 3f5a9a9 Compare June 8, 2026 09:38
Connect_FailsWithInvalidUrl_Awaitable failed intermittently in the full
PlayMode suite: awaiting the ConnectInstruction resumes the instant IsDone
is set, but the FFI emits its "error while connecting" log batch a frame or
two later — after the test had already reset LogAssert.ignoreFailingMessages,
so the late error surfaced as an unhandled message and failed the test. It
only passed in isolation because the timing happened to line up.

Replace it with two deterministic tests driven by a synthetic YieldInstruction
subclass: one for the OnCompleted path (await registered while pending, then
completed) and one for the IsCompleted fast path (already done before await).
These exercise the GetAwaiter logic directly with no FFI, no dev server, and
no LogAssert race. The real connect-fail path stays covered by the existing
Connect_FailsWithInvalidUrl coroutine test.

Co-Authored-By: Claude Opus 4.8 (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