-
Notifications
You must be signed in to change notification settings - Fork 693
Guard refresh wait nudge during compile #558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the Unity readiness wait helper to optionally skip the EditorApplication.QueuePlayerLoopUpdate 'nudge' when a compile has been requested, while preserving the existing polling behavior otherwise. Sequence diagram for conditional Unity nudge during refresh waitsequenceDiagram
actor Developer
participant RefreshUnity
participant WaitHelper as WaitForUnityReadyAsync
participant EditorApplication
Developer->>RefreshUnity: HandleCommand(params)
activate RefreshUnity
note right of RefreshUnity: compileRequested is computed
RefreshUnity->>WaitHelper: WaitForUnityReadyAsync(timeout, allowNudge = !compileRequested)
activate WaitHelper
WaitHelper->>EditorApplication: subscribe Tick to update
activate EditorApplication
alt allowNudge is true (no compile requested)
WaitHelper->>EditorApplication: QueuePlayerLoopUpdate()
else allowNudge is false (compile requested)
note right of WaitHelper: Skip QueuePlayerLoopUpdate
end
loop until Unity ready or timeout
EditorApplication-->>WaitHelper: Tick invoked on update
WaitHelper-->>WaitHelper: Check readiness and timeout
end
WaitHelper-->>RefreshUnity: Task completes or throws TimeoutException
deactivate WaitHelper
RefreshUnity-->>Developer: HandleCommand result
deactivate RefreshUnity
deactivate EditorApplication
Updated class diagram for RefreshUnity wait helper behaviorclassDiagram
class RefreshUnity {
+Task~object~ HandleCommand(JObject params)
-static Task WaitForUnityReadyAsync(TimeSpan timeout, bool allowNudge)
}
class WaitForUnityReadyAsyncWorker {
-TaskCompletionSource~bool~ tcs
-DateTime start
-void Tick()
}
class EditorApplication {
+static event Action update
+static void QueuePlayerLoopUpdate()
}
RefreshUnity --> WaitForUnityReadyAsyncWorker : uses
WaitForUnityReadyAsyncWorker --> EditorApplication : subscribes update
WaitForUnityReadyAsyncWorker --> EditorApplication : calls QueuePlayerLoopUpdate when allowNudge == true
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughA boolean parameter Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-29T15:23:11.613ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The allowNudge parameter was attempting to fix Unity 6 compilation loops by skipping QueuePlayerLoopUpdate, but this was not the root cause. The actual issue (fixed by CoplayDev#559's Unity 6+ preprocessor guard) is that EditorApplication.update callbacks don't survive domain reloads properly in Unity 6+. Since CoplayDev#559 skips WaitForUnityReadyAsync entirely on Unity 6+ when compile is requested, the allowNudge guard is redundant and can be removed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The allowNudge parameter was attempting to fix Unity 6 compilation loops by skipping QueuePlayerLoopUpdate, but this was not the root cause. The actual issue (fixed by #559's Unity 6+ preprocessor guard) is that EditorApplication.update callbacks don't survive domain reloads properly in Unity 6+. Since #559 skips WaitForUnityReadyAsync entirely on Unity 6+ when compile is requested, the allowNudge guard is redundant and can be removed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary
Testing
Summary by Sourcery
Enhancements:
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.