Stage 1: Add GetAwaiter to YieldInstruction + StreamYieldInstruction#289
Draft
MaxHeimbrock wants to merge 2 commits into
Draft
Stage 1: Add GetAwaiter to YieldInstruction + StreamYieldInstruction#289MaxHeimbrock wants to merge 2 commits into
MaxHeimbrock wants to merge 2 commits into
Conversation
Contributor
Author
|
I was able to convert So this worked well. |
4 tasks
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>
da5555c to
3f5a9a9
Compare
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>
2 tasks
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
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.
Summary
GetAwaiter()toYieldInstructionandStreamYieldInstructionso callers canawait room.Connect(...)and similar one-shot/streaming FFI ops without taking on a UniTask dependency.IsDone/IsCurrentReadDone/IsEosproperty setters — every existing concrete instruction (Connect, PublishTrack, PerformRpc, SendText/File, stream open/write/close, etc.) becomes awaitable with no change to its completion path.Interlocked.CompareExchangeon a single continuation slot. Reset() clears the slot so each chunk of a stream gets its own awaiter.GetResult()is a no-op so theawaitsurface keeps strict parity withyield return(callers still inspectIsError). 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 observesIsError.Scripts~/run_unity.sh test -m PlayMode -f 'Connect_FailsWithInvalidUrl$'— original coroutine-path test still passes (no regression).await room.Connect(...)in a sample once a livelivekit-server --devis running, to confirm main-thread resumption.🤖 Generated with Claude Code