Complete UniTask implementation#319
Draft
MaxHeimbrock wants to merge 17 commits into
Draft
Conversation
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>
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>
MaxHeimbrock
force-pushed
the
max/unitask-complete
branch
from
a629011 to
6145795
Compare
Stage 2 of the UniTask migration. The new LiveKit.UniTask asmdef hosts an AsUniTask extension on YieldInstruction and StreamYieldInstruction; the asmdef compiles only when com.cysharp.unitask is installed (the versionDefine auto-activates LIVEKIT_UNITASK). When UniTask is absent, the extension simply does not exist — no compile error, no runtime cost, no impact on Stage 1's awaiter. AsUniTask wraps the existing one-shot completion path in a UniTaskCompletionSource and adds CancellationToken support with "abandon awaiter" semantics: a cancel faults the UniTask with OperationCanceledException, but the underlying FFI request is not aborted. GetResult stays non-throwing for IsError parity with yield return / await; throwing variants can be layered on later. Includes a UniTask migration of Samples~/Meet to demonstrate the new path end-to-end (Connect / PublishLocalCamera / PublishLocalMicrophone all switch to async UniTask with cancellation tied to GetCancellationTokenOnDestroy). Long-running per-frame pumps stay on StartCoroutine since they aren't request/response. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Remove UniTask package
Stage 3 of the UniTask migration. Exposes ByteStreamReader/TextStreamReader incremental reads as IUniTaskAsyncEnumerable<TChunk> so chunks can be consumed with `await foreach`, building on Stage 1's StreamYieldInstruction awaiter and Stage 2's AsUniTask. A single generic extension AsAsyncEnumerable<TChunk>(this ReadIncrementalInstructionBase<TChunk>) covers both byte[] and string readers. The loop mirrors the coroutine consumer's observable behavior: await a chunk, yield it, re-check IsEos AFTER yielding (Reset() is disallowed past EoS), and Reset() for the next chunk. On EoS carrying a StreamError the enumerable throws that error — idiomatic for await foreach, the one place the UniTask surface throws rather than exposing IsError. Cancellation surfaces as OperationCanceledException with abandon-awaiter semantics. To let the separate LiveKit.UniTask assembly drive the loop, two members are widened to public (both already public on the sibling DataTrack.ReadFrameInstruction, behavior-preserving): StreamYieldInstruction.IsCurrentReadDone getter and ReadIncrementalInstructionBase<T>.LatestChunk. The runtime and test UniTask asmdefs gain a UniTask.Linq reference (source of UniTaskAsyncEnumerable.Create / IUniTaskAsyncEnumerable), and InternalsVisibleTo is extended to the PlayModeTests.UniTask assembly so the deterministic tests can construct a synthetic reader (the same FfiHandle-based seam the EditMode tests use). DataTrack frame streaming is intentionally out of scope (its ReadFrameInstruction has no awaiter and no Reset) — a possible follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Stage 4 (capstone) of the UniTask migration. Adds a README section covering the three interchangeable async styles the SDK now supports, and states the policy: coroutines remain the default and fully supported; async/await and UniTask are additive opt-ins; the coroutine API is not deprecated. - async/await with no dependency (instructions are awaitable; inspect IsError, await does not throw — parity with yield return). - UniTask opt-in (com.cysharp.unitask + LIVEKIT_UNITASK): AsUniTask with CancellationToken, UniTask.WhenAll composition, and AsAsyncEnumerable for await foreach over incremental streams (throws StreamError on error EoS). Examples use the verified public signatures (Connect(url, token, RoomOptions), PublishTrack(track, options), ReadIncremental().AsAsyncEnumerable()) and point to the Meet sample (UniTask) and Basic sample (coroutines) as references. Docs-only; no code, no deprecation, no version bump. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MaxHeimbrock
force-pushed
the
max/unitask-complete
branch
from
6145795 to
632297b
Compare
AsUniTask does not throw on failure (parity with the coroutine path), so the parallel-publish example silently ignored failed operations. Keep the instruction references, await WhenAll, then inspect IsError on each. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Previously a failed operation completed the await silently (GetResult was a no-op) and callers had to check IsError — surprising for async/await users (unlike coroutines, the .NET convention is that failures throw), an easy source of silent failures. YieldInstructionAwaiter.GetResult now throws when IsError. The thrown type is the instruction's typed error where one exists (StreamError, RpcError, PublishDataTrackError) and a new LiveKitException otherwise (Connect, PublishTrack, SetMetadata/Name/Attributes, PublishData, GetStats, …). This is wired via an internal virtual YieldInstruction.CreateAwaitException() overridden per instruction; Connect/PublishTrack/FfiInstruction now retain the error message they previously discarded. AsUniTask funnels through GetResult so the UniTask faults identically (TrySetException / FromException). Coroutines are unchanged: yield return never calls GetResult, so it still inspects IsError. Tests and README updated accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The LiveKit.UniTask runtime assembly and its PlayMode tests are gated by LIVEKIT_UNITASK, which only activates when com.cysharp.unitask is present. Without the package in the Meet project, that whole surface (AsUniTask, AsAsyncEnumerable, and their tests) was excluded from compilation, so CI never exercised it. Add the dependency (and lock it) so the opt-in path is built and tested. Confirmed locally: the gated UniTask/stream tests now run (10 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This reverts commit 90fbb8e.
A custom INotifyCompletion awaiter resumes on whatever thread completes the operation. Connect completes on the main thread (dispatchToMainThread:true), but operations registered dispatchToMainThread:false (SetMetadata, stream writes, …) and data-stream chunk events complete on the FFI callback thread — so `await` / `await foreach` resumed off the main thread, and touching a Unity API there threw "can only be called from the main thread". Coroutines never had this problem (they resume on the main thread via the player loop). Route both awaiter continuations through AwaiterScheduler.Resume, which posts to the captured main-thread SynchronizationContext. When already on the main thread (e.g. Connect) it runs inline to avoid an extra frame of latency. This makes the await path's threading identical to the coroutine path — and the README's "continuations resume on Unity's main thread" now holds. Adds GetAwaiter_ResumesOnMainThread_WhenCompletedOffThread, which completes a synthetic instruction from a thread-pool thread and asserts the await resumes on the main thread (red before this change, green after). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add UnsafeOnCompleted to YieldInstructionAwaiter and StreamYieldInstructionAwaiter so the async state machine can resume without capturing ExecutionContext (one fewer allocation per await). Safe because the continuation doesn't rely on the flowed context — AwaiterScheduler marshals it to the main thread regardless. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Previously the CancellationTokenRegistration was disposed only when the instruction completed. A cancelled-but-never-completed awaiter kept its registration (and closure) pinned to a long-lived CancellationTokenSource. Dispose it from the cancel callback too; idempotent with the completion-path dispose, and safe because the pre-cancelled token is handled earlier. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Note that awaiting an instruction (and AsUniTask) never throws on failure — inspect IsError — while the stream enumerable throws StreamError, since await foreach has no post-loop point to check IsError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop comments that referenced the staged rollout and development history (e.g. "Stage 1", an earlier flaky test variant, "pre-existing limitation") and a stale reference to a removed test. Reword to describe current behavior only. No code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
OnEos assigned IsEos before Error, but the IsEos setter fires the awaiter
continuation. When completion runs inline on the main thread the continuation
observed IsError == false and delivered the final chunk as success instead of
throwing the stream error. Assign Error first, matching the sibling
DataTrack.ReadFrameInstruction.SetEos ordering.
Also revert LatestChunk from public back to internal (it throws on get and
exists only for the UniTask adapter) and grant the adapter access via
InternalsVisibleTo("LiveKit.UniTask"), instead of widening public surface for
an optional integration.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both UniTask asmdefs declared a rootNamespace that didn't match the code: the runtime extensions live in namespace LiveKit (not LiveKit.UniTaskExtensions) and the tests in LiveKit.PlayModeTests.UniTaskBridge (not .UniTask). rootNamespace is advisory (it only seeds the namespace for newly created scripts), so this has no compile effect, but it removes a misleading signal and prevents future new files from drifting into the wrong namespace. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The git dependency had no ref, so it resolved to UniTask master HEAD at resolution time — CI reproducibility relied solely on the lockfile hash. Pin the manifest URL to the 2.5.11 release tag and update the lockfile to the matching commit (2e993ff18f28c931602a07292df0b0804eebef99) so the version CI exercises is explicit and deterministic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MaxHeimbrock
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
We currently only support Coroutines for async execution, with this PR we will support async/await and if clients import the UniTask package, we also support UniTasks natively.