diff --git a/Assets/Tests/Editor/ExternalSceneChangeResolverTests.cs b/Assets/Tests/Editor/ExternalSceneChangeResolverTests.cs index 49045dd962..a480fb7d65 100644 --- a/Assets/Tests/Editor/ExternalSceneChangeResolverTests.cs +++ b/Assets/Tests/Editor/ExternalSceneChangeResolverTests.cs @@ -192,6 +192,53 @@ public void ResolveExternalSceneChanges_WhenSceneUnchanged_DoesNotSaveOrReload() Assert.That(reloadWasCalled, Is.False); } + [Test] + public void FocusReturnService_WhenHoldSucceeds_EmitsHoldArmedVibeLog() + { + // Verifies successful Disallow arms held and emits the observability vibe event once. + bool autoRefreshHeld = false; + List vibeOperations = new List(); + ExternalAssetFocusReturnService service = new ExternalAssetFocusReturnService( + () => autoRefreshHeld, + isHeld => autoRefreshHeld = isHeld, + () => false, + () => { }, + () => { }, + () => { }, + logWarning: null, + logVibeInfo: (operation, message, context) => vibeOperations.Add(operation), + logVibeWarning: null); + + service.HoldAutoRefreshIfNeeded(); + service.HoldAutoRefreshIfNeeded(); + + Assert.That(autoRefreshHeld, Is.True); + Assert.That(vibeOperations, Is.EqualTo(new[] { "external_scene_hold_armed" })); + } + + [Test] + public void FocusReturnService_WhenDisallowThrows_EmitsHoldFailedVibeLog() + { + // Verifies Disallow failures leave SessionState unheld and emit hold_failed vibe warning. + bool autoRefreshHeld = false; + List vibeOperations = new List(); + ExternalAssetFocusReturnService service = new ExternalAssetFocusReturnService( + () => autoRefreshHeld, + isHeld => autoRefreshHeld = isHeld, + () => false, + () => throw new InvalidOperationException("kCodeReload"), + () => { }, + () => { }, + logWarning: _ => { }, + logVibeInfo: null, + logVibeWarning: (operation, message, context) => vibeOperations.Add(operation)); + + service.HoldAutoRefreshIfNeeded(); + + Assert.That(autoRefreshHeld, Is.False); + Assert.That(vibeOperations, Is.EqualTo(new[] { "external_scene_hold_failed" })); + } + [Test] public void FocusReturnService_WhenFocusIsLost_HoldsAutoRefreshOnce() { @@ -215,6 +262,125 @@ public void FocusReturnService_WhenFocusIsLost_HoldsAutoRefreshOnce() Assert.That(allowCallCount, Is.EqualTo(0)); } + [Test] + public void FocusReturnService_WhenHoldIfCurrentlyUnfocusedTwice_HoldsAutoRefreshOnce() + { + // Verifies Initialize-style unfocused Hold is idempotent (disallow once). + bool autoRefreshHeld = false; + int disallowCallCount = 0; + ExternalAssetFocusReturnService service = CreateFocusReturnService( + () => autoRefreshHeld, + isHeld => autoRefreshHeld = isHeld, + () => false, + () => disallowCallCount++, + () => { }, + () => { }); + + service.HoldIfCurrentlyUnfocused(); + service.HoldIfCurrentlyUnfocused(); + + Assert.That(autoRefreshHeld, Is.True); + Assert.That(disallowCallCount, Is.EqualTo(1)); + } + + [Test] + public void FocusReturnService_WhenDisallowThrows_DoesNotSetHeldFlag() + { + // Verifies kCodeReload Disallow failures leave SessionState unheld for later reconcile. + bool autoRefreshHeld = false; + List warnings = new List(); + ExternalAssetFocusReturnService service = CreateFocusReturnService( + () => autoRefreshHeld, + isHeld => autoRefreshHeld = isHeld, + () => false, + () => throw new InvalidOperationException("kCodeReload"), + () => { }, + () => { }, + warning => warnings.Add(warning)); + + service.HoldAutoRefreshIfNeeded(); + + Assert.That(autoRefreshHeld, Is.False); + Assert.That(warnings.Count, Is.EqualTo(1)); + Assert.That(warnings[0], Does.Contain("DisallowAutoRefresh")); + Assert.That(warnings[0], Does.Contain("InvalidOperationException")); + } + + [Test] + public void FocusReturnService_WhenDisallowStopsFailing_ReconcileHoldsAutoRefresh() + { + // Verifies update reconcile arms Hold after transient Disallow failures without delayCall chains. + bool autoRefreshHeld = false; + bool disallowShouldThrow = true; + int disallowCallCount = 0; + ExternalAssetFocusReturnService service = CreateFocusReturnService( + () => autoRefreshHeld, + isHeld => autoRefreshHeld = isHeld, + () => false, + () => + { + disallowCallCount++; + if (disallowShouldThrow) + { + throw new InvalidOperationException("kCodeReload"); + } + }, + () => { }, + () => { }, + _ => { }); + + service.ReconcileAutoRefreshHoldWithFocus(); + Assert.That(autoRefreshHeld, Is.False); + + disallowShouldThrow = false; + service.ReconcileAutoRefreshHoldWithFocus(); + + Assert.That(autoRefreshHeld, Is.True); + Assert.That(disallowCallCount, Is.EqualTo(2)); + } + + [Test] + public void FocusReturnService_WhenFocusedAndHeld_ReconcileReleasesAfterPreflight() + { + // Verifies focused reconcile resolves external changes then releases a surviving Hold. + bool autoRefreshHeld = true; + List events = new List(); + ExternalAssetFocusReturnService service = CreateFocusReturnService( + () => autoRefreshHeld, + isHeld => autoRefreshHeld = isHeld, + () => true, + () => events.Add("disallow"), + () => events.Add("allow"), + () => events.Add("preflight")); + + service.ReconcileAutoRefreshHoldWithFocus(); + + Assert.That(autoRefreshHeld, Is.False); + Assert.That(events, Is.EqualTo(new[] { "preflight", "allow" })); + } + + [Test] + public void FocusReturnService_WhenAllowThrows_KeepsHeldFlag() + { + // Verifies failed Allow leaves SessionState held so reconcile can retry without counter desync. + bool autoRefreshHeld = true; + List warnings = new List(); + ExternalAssetFocusReturnService service = CreateFocusReturnService( + () => autoRefreshHeld, + isHeld => autoRefreshHeld = isHeld, + () => true, + () => { }, + () => throw new InvalidOperationException("kCodeReload"), + () => { }, + warning => warnings.Add(warning)); + + service.HandleFocusChanged(true); + + Assert.That(autoRefreshHeld, Is.True); + Assert.That(warnings.Count, Is.EqualTo(1)); + Assert.That(warnings[0], Does.Contain("AllowAutoRefresh")); + } + [Test] public void FocusReturnService_WhenFocusReturns_RunsPreflightBeforeReleasingAutoRefresh() { @@ -404,7 +570,8 @@ private static ExternalAssetFocusReturnService CreateFocusReturnService( Func isEditorFocused, Action disallowAutoRefresh, Action allowAutoRefresh, - Action resolveFocusReturnChanges) + Action resolveFocusReturnChanges, + Action logWarning = null) { return new ExternalAssetFocusReturnService( getAutoRefreshHeld, @@ -412,7 +579,8 @@ private static ExternalAssetFocusReturnService CreateFocusReturnService( isEditorFocused, disallowAutoRefresh, allowAutoRefresh, - resolveFocusReturnChanges); + resolveFocusReturnChanges, + logWarning); } } } diff --git a/Packages/src/Editor/FirstPartyTools/Compile/ExternalAssetFocusReturnService.cs b/Packages/src/Editor/FirstPartyTools/Compile/ExternalAssetFocusReturnService.cs index 94d283709d..4561b08636 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/ExternalAssetFocusReturnService.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/ExternalAssetFocusReturnService.cs @@ -5,6 +5,8 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// /// Coordinates Auto Refresh suspension while Unity is unfocused. + /// Why not rely on focusChanged alone: background launch never fires focus-lost, so + /// DisallowAutoRefresh must also be armed from Initialize and periodic reconcile. /// internal sealed class ExternalAssetFocusReturnService { @@ -14,6 +16,9 @@ internal sealed class ExternalAssetFocusReturnService private readonly Action _disallowAutoRefresh; private readonly Action _allowAutoRefresh; private readonly Action _resolveFocusReturnChanges; + private readonly Action _logWarning; + private readonly Action _logVibeInfo; + private readonly Action _logVibeWarning; internal ExternalAssetFocusReturnService( Func getAutoRefreshHeld, @@ -21,7 +26,10 @@ internal ExternalAssetFocusReturnService( Func isEditorFocused, Action disallowAutoRefresh, Action allowAutoRefresh, - Action resolveFocusReturnChanges) + Action resolveFocusReturnChanges, + Action logWarning = null, + Action logVibeInfo = null, + Action logVibeWarning = null) { Debug.Assert(getAutoRefreshHeld != null, "getAutoRefreshHeld must not be null"); Debug.Assert(setAutoRefreshHeld != null, "setAutoRefreshHeld must not be null"); @@ -37,6 +45,10 @@ internal ExternalAssetFocusReturnService( _allowAutoRefresh = allowAutoRefresh ?? throw new ArgumentNullException(nameof(allowAutoRefresh)); _resolveFocusReturnChanges = resolveFocusReturnChanges ?? throw new ArgumentNullException(nameof(resolveFocusReturnChanges)); + _logWarning = logWarning ?? (message => Debug.LogWarning(message)); + // Why inject: pure C# unit tests stay free of VibeLogger; production wires VibeLogger. + _logVibeInfo = logVibeInfo ?? ((operation, message, context) => { }); + _logVibeWarning = logVibeWarning ?? ((operation, message, context) => { }); } internal bool RestoreAutoRefreshIfHeld() @@ -55,6 +67,52 @@ internal bool RestoreAutoRefreshIfHeld() return true; } + /// + /// Arms DisallowAutoRefresh when the Editor starts unfocused (no focus-lost event yet). + /// + internal void HoldIfCurrentlyUnfocused() + { + if (_isEditorFocused()) + { + return; + } + + HoldAutoRefreshIfNeeded(); + } + + /// + /// Aligns held flag with focus without depending on focusChanged delivery. + /// Idempotent: only calls Disallow/Allow when state must change. + /// Why not delayCall retry chains: kCodeReload failures stay unheld and this reconcile retries later. + /// + internal void ReconcileAutoRefreshHoldWithFocus() + { + if (!_isEditorFocused()) + { + if (HoldAutoRefreshIfNeeded()) + { + // Why only on actual repair: reconcile ticks every 0.5s; spam would drown the gate timeline. + _logVibeInfo( + "external_scene_reconcile_repair", + "Reconcile armed Auto Refresh hold while Editor is unfocused", + new { held = true, isFocused = false }); + } + + return; + } + + if (!_getAutoRefreshHeld()) + { + return; + } + + HandleFocusChanged(true); + _logVibeInfo( + "external_scene_reconcile_repair", + "Reconcile released Auto Refresh hold while Editor is focused", + new { held = _getAutoRefreshHeld(), isFocused = true }); + } + internal void HandleFocusChanged(bool isFocused) { if (!isFocused) @@ -73,15 +131,28 @@ internal void HandleFocusChanged(bool isFocused) } } - private void HoldAutoRefreshIfNeeded() + /// + /// Attempts to arm DisallowAutoRefresh. Returns true only when this call newly armed the hold. + /// + internal bool HoldAutoRefreshIfNeeded() { if (_getAutoRefreshHeld()) { - return; + return false; } - _disallowAutoRefresh(); + if (!TryDisallowAutoRefresh()) + { + return false; + } + + // Why only after success: setting SessionState on failure desyncs the Unity counter (ยง10). _setAutoRefreshHeld(true); + _logVibeInfo( + "external_scene_hold_armed", + "Auto Refresh hold armed", + new { held = true, isFocused = _isEditorFocused() }); + return true; } private void ReleaseAutoRefreshIfHeld() @@ -91,8 +162,69 @@ private void ReleaseAutoRefreshIfHeld() return; } - _allowAutoRefresh(); + if (!TryAllowAutoRefresh()) + { + return; + } + _setAutoRefreshHeld(false); + _logVibeInfo( + "external_scene_hold_released", + "Auto Refresh hold released", + new { held = false, isFocused = _isEditorFocused() }); + } + + private bool TryDisallowAutoRefresh() + { + // Why try-catch (hatayama-approved, Disallow/Allow boundary only): Unity throws during kCodeReload. + try + { + _disallowAutoRefresh(); + return true; + } + catch (Exception exception) + { + _logWarning( + "Unity CLI Loop could not DisallowAutoRefresh (often during domain reload). " + + "Will retry via focus reconcile. " + exception.GetType().Name + ": " + exception.Message); + _logVibeWarning( + "external_scene_hold_failed", + "DisallowAutoRefresh failed", + new + { + exceptionType = exception.GetType().FullName, + exceptionMessage = exception.Message, + held = _getAutoRefreshHeld(), + isFocused = _isEditorFocused() + }); + return false; + } + } + + private bool TryAllowAutoRefresh() + { + try + { + _allowAutoRefresh(); + return true; + } + catch (Exception exception) + { + _logWarning( + "Unity CLI Loop could not AllowAutoRefresh (often during domain reload). " + + "Will retry via focus reconcile. " + exception.GetType().Name + ": " + exception.Message); + _logVibeWarning( + "external_scene_release_failed", + "AllowAutoRefresh failed", + new + { + exceptionType = exception.GetType().FullName, + exceptionMessage = exception.Message, + held = _getAutoRefreshHeld(), + isFocused = _isEditorFocused() + }); + return false; + } } } } diff --git a/Packages/src/Editor/FirstPartyTools/Compile/ExternalSceneChangeTracker.cs b/Packages/src/Editor/FirstPartyTools/Compile/ExternalSceneChangeTracker.cs index 1e9ddf9346..db7a18d240 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/ExternalSceneChangeTracker.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/ExternalSceneChangeTracker.cs @@ -32,8 +32,20 @@ internal static class ExternalSceneChangeTracker () => EditorApplication.isFocused, AssetDatabase.DisallowAutoRefresh, AssetDatabase.AllowAutoRefresh, - ResolveForFocusReturn); + ResolveForFocusReturn, + logWarning: null, + logVibeInfo: (operation, message, context) => + { + VibeLogger.LogInfo(operation, message, context, includeStackTrace: false); + }, + logVibeWarning: (operation, message, context) => + { + VibeLogger.LogWarning(operation, message, context); + }); + // Why throttle: reconcile must not call Disallow/Allow every frame when already aligned. + private const double AutoRefreshReconcileIntervalSeconds = 0.5d; private static bool _initialized; + private static double _nextAutoRefreshReconcileTime; public static void Initialize() { @@ -65,11 +77,30 @@ public static void Initialize() PrefabStage.prefabSaved += HandlePrefabSaved; EditorApplication.focusChanged -= HandleFocusChanged; EditorApplication.focusChanged += HandleFocusChanged; + EditorApplication.update -= ReconcileAutoRefreshHoldOnUpdate; + EditorApplication.update += ReconcileAutoRefreshHoldOnUpdate; + // Why record before Hold: fingerprints must exist for focus-return resolve after startup Hold. if (!restoredHeldAutoRefresh && !IsAutoRefreshHeld()) { RecordOpenSceneSnapshots(); RecordCurrentPrefabStageSnapshot(); } + + // Why immediate Hold: background launch never fires focusChanged(false), so Auto Refresh + // would stay enabled until the first focus and show a native external-change dialog. + FocusReturnService.HoldIfCurrentlyUnfocused(); + } + + private static void ReconcileAutoRefreshHoldOnUpdate() + { + double now = EditorApplication.timeSinceStartup; + if (now < _nextAutoRefreshReconcileTime) + { + return; + } + + _nextAutoRefreshReconcileTime = now + AutoRefreshReconcileIntervalSeconds; + FocusReturnService.ReconcileAutoRefreshHoldWithFocus(); } public static (bool CanProceed, string Message, string[] ScenePaths) ResolveForCompile( @@ -85,6 +116,11 @@ public static (bool CanProceed, string Message, string[] ScenePaths) ResolveForC private static void HandleFocusChanged(bool isFocused) { + VibeLogger.LogInfo( + "external_scene_focus_changed", + "Editor focus changed", + new { isFocused, held = IsAutoRefreshHeld() }, + includeStackTrace: false); FocusReturnService.HandleFocusChanged(isFocused); } @@ -134,6 +170,20 @@ private static void ResolveForFocusReturn() { // Focus return treats Unity's in-memory editor state as authoritative because source-control // operations can replace files while Unity is unfocused and would otherwise trigger reload dialogs. + (string AssetPath, bool IsDirty)[] openScenesBefore = GetOpenSceneStates(); + object[] fingerprintDiffsBefore = BuildFingerprintDiffContexts(openScenesBefore); + VibeLogger.LogInfo( + "external_scene_resolve_focus_return", + "ResolveForFocusReturn started", + new + { + phase = "start", + scenes = openScenesBefore, + fingerprintDiffs = fingerprintDiffsBefore, + held = IsAutoRefreshHeld() + }, + includeStackTrace: false); + string[] dirtySceneSaveFailures = SaveDirtyOpenScenesBeforeReload(); LogFocusReturnFailures("save dirty Scene files", dirtySceneSaveFailures); @@ -153,10 +203,70 @@ private static void ResolveForFocusReturn() { Debug.LogWarning( "Unity CLI Loop skipped Prefab Stage external-change reload because the current Prefab Stage is still dirty or could not be saved."); + VibeLogger.LogInfo( + "external_scene_resolve_focus_return", + "ResolveForFocusReturn finished early (Prefab Stage still dirty or unsaved)", + new + { + phase = "end", + skippedPrefabReload = true, + dirtySceneSaveFailures, + missingSceneSaveFailures, + dirtyPrefabSaveFailures, + missingPrefabSaveFailures, + held = IsAutoRefreshHeld() + }, + includeStackTrace: false); return; } ResolveCurrentPrefabStageExternalChangeForFocusReturn(); + + (string AssetPath, bool IsDirty)[] openScenesAfter = GetOpenSceneStates(); + VibeLogger.LogInfo( + "external_scene_resolve_focus_return", + "ResolveForFocusReturn finished", + new + { + phase = "end", + scenes = openScenesAfter, + fingerprintDiffs = BuildFingerprintDiffContexts(openScenesAfter), + dirtySceneSaveFailures, + missingSceneSaveFailures, + held = IsAutoRefreshHeld() + }, + includeStackTrace: false); + } + + private static object[] BuildFingerprintDiffContexts((string AssetPath, bool IsDirty)[] scenes) + { + Debug.Assert(scenes != null, "scenes must not be null"); + + List diffs = new List(scenes.Length); + for (int i = 0; i < scenes.Length; i++) + { + string assetPath = scenes[i].AssetPath; + (bool Exists, DateTime LastWriteTimeUtc, long Length) current = + ReadAssetFileFingerprint(assetPath); + bool hasSnapshot = SceneSnapshots.TryGetValue( + assetPath, + out (bool Exists, DateTime LastWriteTimeUtc, long Length) snapshot); + bool changed = !hasSnapshot || + !ExternalAssetFileStateComparer.HasSameFileState(snapshot, current); + diffs.Add(new + { + assetPath, + isDirty = scenes[i].IsDirty, + changed, + hasSnapshot, + snapshotExists = hasSnapshot && snapshot.Exists, + currentExists = current.Exists, + snapshotLength = hasSnapshot ? snapshot.Length : 0L, + currentLength = current.Length + }); + } + + return diffs.ToArray(); } private static void RecordOpenSceneSnapshots()