fix: quietly save dirty Scenes before CLI Play Mode start#1755
Conversation
Extract the run-tests quiet-save path into a shared EditorUtility helper and call it on Edit→Play so control-play-mode no longer leaves dirty scenes that block later tools or hang on Unity save dialogs. Resume-from-pause skips save because SaveScene does not work while already playing. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughAdds a shared quiet-save service for dirty scenes and prefab stages, blocks PlayMode entry when saving fails or changes remain, and delegates test execution validation to the service. ChangesUnsaved Editor Change Handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ControlPlayModeUseCase
participant IEditorUnsavedChangesQuietSaver
participant UnityEditor
ControlPlayModeUseCase->>IEditorUnsavedChangesQuietSaver: SaveUnsavedEditorChanges()
IEditorUnsavedChangesQuietSaver->>UnityEditor: Save dirty scenes and prefab stage
IEditorUnsavedChangesQuietSaver-->>ControlPlayModeUseCase: Failed changes
ControlPlayModeUseCase->>IEditorUnsavedChangesQuietSaver: DetectUnsavedEditorChanges()
IEditorUnsavedChangesQuietSaver-->>ControlPlayModeUseCase: Remaining changes
ControlPlayModeUseCase-->>UnityEditor: Block PlayMode start
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
LGTM. Verified: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Packages/src/Editor/FirstPartyTools/Common/EditorUtility/EditorUnsavedChangesQuietSaver.cs (1)
31-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate dirty-detection logic between detect/save paths.
AddDirtyLoadedScenes/SaveDirtyLoadedScenesandAddDirtyPrefabStage/SaveDirtyPrefabStageeach repeat the same dirty-state guard clauses. Since this class is now the single shared source of truth for both PlayMode start and test execution gating, keeping detect/save logic in sync via duplicated conditions is a maintenance risk if the dirty criteria changes later.♻️ Suggested consolidation
- private static void AddDirtyLoadedScenes(List<string> unsavedEditorChanges) - { - Debug.Assert(unsavedEditorChanges != null, "unsavedEditorChanges must not be null"); - - for (int i = 0; i < SceneManager.sceneCount; i++) - { - Scene scene = SceneManager.GetSceneAt(i); - if (!scene.IsValid() || !scene.isLoaded || !scene.isDirty) - { - continue; - } - - unsavedEditorChanges.Add("Scene: " + GetSceneDisplayPath(scene)); - } - } - - private static void SaveDirtyLoadedScenes(List<string> failedChanges) - { - Debug.Assert(failedChanges != null, "failedChanges must not be null"); - - for (int i = 0; i < SceneManager.sceneCount; i++) - { - Scene scene = SceneManager.GetSceneAt(i); - if (!scene.IsValid() || !scene.isLoaded || !scene.isDirty) - { - continue; - } - - if (string.IsNullOrEmpty(scene.path) || !EditorSceneManager.SaveScene(scene)) - { - failedChanges.Add("Scene: " + GetSceneDisplayPath(scene)); - } - } - } + private static IEnumerable<Scene> GetDirtyLoadedScenes() + { + for (int i = 0; i < SceneManager.sceneCount; i++) + { + Scene scene = SceneManager.GetSceneAt(i); + if (scene.IsValid() && scene.isLoaded && scene.isDirty) + { + yield return scene; + } + } + } + + private static void AddDirtyLoadedScenes(List<string> unsavedEditorChanges) + { + foreach (Scene scene in GetDirtyLoadedScenes()) + { + unsavedEditorChanges.Add("Scene: " + GetSceneDisplayPath(scene)); + } + } + + private static void SaveDirtyLoadedScenes(List<string> failedChanges) + { + foreach (Scene scene in GetDirtyLoadedScenes()) + { + if (string.IsNullOrEmpty(scene.path) || !EditorSceneManager.SaveScene(scene)) + { + failedChanges.Add("Scene: " + GetSceneDisplayPath(scene)); + } + } + }A similar
TryGetDirtyPrefabStage(out PrefabStage)helper would remove the duplicated guard in the prefab-stage pair.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/src/Editor/FirstPartyTools/Common/EditorUtility/EditorUnsavedChangesQuietSaver.cs` around lines 31 - 93, Consolidate the repeated dirty-state checks by introducing shared helpers for loaded scenes and the current prefab stage, such as a dirty-scene predicate/lookup and TryGetDirtyPrefabStage(out PrefabStage). Update AddDirtyLoadedScenes, SaveDirtyLoadedScenes, AddDirtyPrefabStage, and SaveDirtyPrefabStage to reuse these helpers while preserving their existing detection and save-failure behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@Packages/src/Editor/FirstPartyTools/Common/EditorUtility/EditorUnsavedChangesQuietSaver.cs`:
- Around line 31-93: Consolidate the repeated dirty-state checks by introducing
shared helpers for loaded scenes and the current prefab stage, such as a
dirty-scene predicate/lookup and TryGetDirtyPrefabStage(out PrefabStage). Update
AddDirtyLoadedScenes, SaveDirtyLoadedScenes, AddDirtyPrefabStage, and
SaveDirtyPrefabStage to reuse these helpers while preserving their existing
detection and save-failure behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e4090505-092e-4879-8ade-cb7bc0d09e0a
⛔ Files ignored due to path filters (3)
Packages/src/Editor/FirstPartyTools/Common/EditorUtility/EditorUnsavedChangesQuietSaver.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/Common/EditorUtility/IEditorUnsavedChangesQuietSaver.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/ControlPlayMode/UnityCLILoop.FirstPartyTools.ControlPlayMode.Editor.asmdefis excluded by none and included by none
📒 Files selected for processing (5)
Assets/Tests/Editor/ControlPlayModeUseCaseTests.csPackages/src/Editor/FirstPartyTools/Common/EditorUtility/EditorUnsavedChangesQuietSaver.csPackages/src/Editor/FirstPartyTools/Common/EditorUtility/IEditorUnsavedChangesQuietSaver.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.csPackages/src/Editor/FirstPartyTools/RunTests/TestExecutionStateValidationService.cs
1 participant
Summary
control-play-mode --action Playnow quietly saves dirty loaded Scenes and the current Prefab Stage before Edit→Play, matching the run-tests save pathEditorUnsavedChangesQuietSaverso both tools use the same quiet-save implementationUser Impact
Changes
IEditorUnsavedChangesQuietSaver/EditorUnsavedChangesQuietSaverunder Common/EditorUtilityTestExecutionStateValidationServicedelegates detect/save to the shared helperControlPlayModeUseCaseruns quiet save only on Edit→PlayVerification
uloop compile— 0 errors / 0 warningsuloop run-testsfilterControlPlayModeUseCaseTests|TestExecutionStateValidationServiceTests— 24 passedTest plan
uloop control-play-mode --action Play→ Scene saved, Play startsMade with Cursor