Update MCPForUnityEditorWindow.cs#917
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts when the update check is queued so it runs once during GUI creation instead of every time data is refreshed, reducing unnecessary network calls when the editor window gains focus. Sequence diagram for updated update check timing in MCPForUnityEditorWindowsequenceDiagram
actor User
participant UnityEditor
participant MCPForUnityEditorWindow
rect rgb(230,230,255)
alt Before_PR_OnFocus_triggers_update_check
User ->> UnityEditor: Focus_editor_window
UnityEditor ->> MCPForUnityEditorWindow: OnFocus
MCPForUnityEditorWindow ->> MCPForUnityEditorWindow: RefreshAllData
MCPForUnityEditorWindow ->> MCPForUnityEditorWindow: QueueUpdateCheck
else After_PR_CreateGUI_triggers_update_check
User ->> UnityEditor: Open_or_reload_editor_window
UnityEditor ->> MCPForUnityEditorWindow: CreateGUI
MCPForUnityEditorWindow ->> MCPForUnityEditorWindow: RefreshAllData
MCPForUnityEditorWindow ->> MCPForUnityEditorWindow: QueueUpdateCheck
User ->> UnityEditor: Focus_editor_window
UnityEditor ->> MCPForUnityEditorWindow: OnFocus
MCPForUnityEditorWindow ->> MCPForUnityEditorWindow: RefreshAllData
end
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Now that
QueueUpdateCheck()runs only inCreateGUI(), please double-check whether there are other code paths that callRefreshAllData()(e.g., manual refresh actions) where an update check is still desired, and if so consider either a separate explicit update trigger or a parameter to control this behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that `QueueUpdateCheck()` runs only in `CreateGUI()`, please double-check whether there are other code paths that call `RefreshAllData()` (e.g., manual refresh actions) where an update check is still desired, and if so consider either a separate explicit update trigger or a parameter to control this behavior.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA call to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Pull request overview
This PR reduces how often the Unity Editor window triggers package update checks by moving the update-check scheduling out of the frequent refresh path and into initial UI creation, addressing Issue #830’s report of repeated stalls.
Changes:
- Schedule
QueueUpdateCheck()duringCreateGUI()(initial window build) instead of duringRefreshAllData(). - Stop triggering update checks from focus-driven refreshes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| // Initial updates | ||
| RefreshAllData(); | ||
| QueueUpdateCheck(); |
There was a problem hiding this comment.
QueueUpdateCheck() ultimately calls PackageUpdateService.CheckForUpdate(), which performs synchronous WebClient.DownloadString network requests on the editor main thread. Even though it’s now only queued once per CreateGUI, users with poor connectivity can still experience long UI stalls when the delayCall runs. Consider running the update check off the main thread (or via UnityWebRequest/HttpClient with a short timeout + cancellation) and then marshaling the UI update back onto the main thread, so the editor stays responsive even when offline.
| QueueUpdateCheck(); | |
| _ = Task.Run(() => QueueUpdateCheck()); |
Problem #830. Previously once the GUI is on focus, RefreshAllData will be called and QueueUpdateCheck() is called, for some users with internet issues (from susception), this will cause issue, and check for update is not necessary to run that often. I moved it to the CreateGUI part, so every time the package is updated, restored, reopened, this function will still run.
Summary by Sourcery
Enhancements:
Summary by CodeRabbit