diff --git a/.agents/skills/uloop-pause-point/references/watch-expressions.md b/.agents/skills/uloop-pause-point/references/watch-expressions.md index babbedf687..e7275d4e07 100644 --- a/.agents/skills/uloop-pause-point/references/watch-expressions.md +++ b/.agents/skills/uloop-pause-point/references/watch-expressions.md @@ -17,4 +17,8 @@ The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect ## Lifetime -Watch expressions are in-memory Editor state. A domain reload clears them, so re-register them after `uloop compile`, script recompilation, or an Editor restart. For reliable per-Step changes, keep the expression attached to a continuous pause point on an `Update` or `FixedUpdate` line and use `control-play-mode --action Step`. +Watch expressions survive a domain reload. They are saved in Editor session state, so `uloop compile`, a script recompilation, and a Play entry with Domain Reload enabled all keep them registered — the expression is recompiled after the reload and evaluation starts from a fresh baseline, so history collected before the reload is gone. + +A watch whose expression no longer compiles after the reload (its type was renamed or deleted) is dropped, and the next `enable-watch` or `get-watch-values` response reports it in `Warning` with the id and the compiler message. + +Watch expressions do not survive an Editor restart — session state ends with the Editor process. For reliable per-Step changes, keep the expression attached to a continuous pause point on an `Update` or `FixedUpdate` line and use `control-play-mode --action Step`. diff --git a/.claude/skills/uloop-pause-point/references/watch-expressions.md b/.claude/skills/uloop-pause-point/references/watch-expressions.md index babbedf687..e7275d4e07 100644 --- a/.claude/skills/uloop-pause-point/references/watch-expressions.md +++ b/.claude/skills/uloop-pause-point/references/watch-expressions.md @@ -17,4 +17,8 @@ The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect ## Lifetime -Watch expressions are in-memory Editor state. A domain reload clears them, so re-register them after `uloop compile`, script recompilation, or an Editor restart. For reliable per-Step changes, keep the expression attached to a continuous pause point on an `Update` or `FixedUpdate` line and use `control-play-mode --action Step`. +Watch expressions survive a domain reload. They are saved in Editor session state, so `uloop compile`, a script recompilation, and a Play entry with Domain Reload enabled all keep them registered — the expression is recompiled after the reload and evaluation starts from a fresh baseline, so history collected before the reload is gone. + +A watch whose expression no longer compiles after the reload (its type was renamed or deleted) is dropped, and the next `enable-watch` or `get-watch-values` response reports it in `Warning` with the id and the compiler message. + +Watch expressions do not survive an Editor restart — session state ends with the Editor process. For reliable per-Step changes, keep the expression attached to a continuous pause point on an `Update` or `FixedUpdate` line and use `control-play-mode --action Step`. diff --git a/Assets/Tests/Editor/WatchRestoreServiceTests.cs b/Assets/Tests/Editor/WatchRestoreServiceTests.cs new file mode 100644 index 0000000000..97dfedb89d --- /dev/null +++ b/Assets/Tests/Editor/WatchRestoreServiceTests.cs @@ -0,0 +1,384 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using UnityEngine.TestTools; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies how the post-domain-reload restore re-registers stored watch expressions: + /// registration order, what happens to expressions that no longer compile, and that the + /// store is left holding exactly what came back. + /// + [TestFixture] + public sealed class WatchRestoreServiceTests + { + private const int MaxWaitFrames = 600; + + /// + /// What: two stored watches are re-registered in the stored order and rewritten to the store. + /// + [UnityTest] + public IEnumerator RestoreAsync_WithTwoCompilableRecords_RestoresThemInOrder() + { + WatchExpressionRegistry registry = CreateRegistry(); + InMemoryWatchPersistenceStore store = new( + Record("first", "1 + 1", 5), + Record("second", "2 + 2", 9)); + FakeWatchExpressionCompiler compiler = new(); + int monitorStartCount = 0; + WatchRestoreService service = new(registry, compiler, store, () => monitorStartCount++); + + Task task = service.RestoreAsync(CancellationToken.None); + yield return WaitFor(task); + + WatchRestoreReport report = task.Result; + Assert.That(report.RestoredCount, Is.EqualTo(2)); + Assert.That(report.Warnings, Is.Empty); + Assert.That(monitorStartCount, Is.EqualTo(1)); + Assert.That(IdsOf(registry), Is.EqualTo(new[] { "first", "second" })); + Assert.That(IdsOf(store.Saved), Is.EqualTo(new[] { "first", "second" })); + Assert.That(store.Saved[0].MaxHistory, Is.EqualTo(5)); + } + + /// + /// What: an expression that no longer compiles is reported and removed, the rest still restore. + /// + [UnityTest] + public IEnumerator RestoreAsync_WhenOneExpressionNoLongerCompiles_ReportsItAndDropsItFromTheStore() + { + WatchExpressionRegistry registry = CreateRegistry(); + InMemoryWatchPersistenceStore store = new( + Record("broken", "MissingType.Value", 20), + Record("kept", "1 + 1", 20)); + FakeWatchExpressionCompiler compiler = new(); + compiler.FailFor("MissingType.Value", "The name 'MissingType' does not exist"); + WatchRestoreService service = new(registry, compiler, store, () => { }); + + Task task = service.RestoreAsync(CancellationToken.None); + yield return WaitFor(task); + + WatchRestoreReport report = task.Result; + Assert.That(report.RestoredCount, Is.EqualTo(1)); + Assert.That(report.Warnings, Has.Count.EqualTo(1)); + Assert.That(report.Warnings[0], Does.Contain("broken")); + Assert.That(report.Warnings[0], Does.Contain("The name 'MissingType' does not exist")); + Assert.That(IdsOf(registry), Is.EqualTo(new[] { "kept" })); + Assert.That( + IdsOf(store.Saved), + Is.EqualTo(new[] { "kept" }), + "A watch that cannot compile must not be retried on every later domain reload."); + } + + /// + /// What: a watch the user re-registered while restore was compiling keeps the user's registration. + /// + [UnityTest] + public IEnumerator RestoreAsync_WhenTheIdWasRegisteredMeanwhile_KeepsTheExistingRegistrationSilently() + { + WatchExpressionRegistry registry = CreateRegistry(); + registry.Register("first", "999", new ConstantWatchExpressionEvaluator(999), 20); + InMemoryWatchPersistenceStore store = new(Record("first", "1 + 1", 5)); + WatchRestoreService service = new( + registry, + new FakeWatchExpressionCompiler(), + store, + () => { }); + + Task task = service.RestoreAsync(CancellationToken.None); + yield return WaitFor(task); + + WatchRestoreReport report = task.Result; + Assert.That(report.RestoredCount, Is.EqualTo(0)); + Assert.That(report.Warnings, Is.Empty, "Losing a duplicate to the user's own registration is not a problem to report."); + Assert.That(registry.GetEntries()[0].Expression, Is.EqualTo("999")); + Assert.That(store.Saved[0].Expression, Is.EqualTo("999")); + } + + /// + /// What: a compile that throws becomes that record's warning and the rest still restore, + /// and the thrown-on record stays in the store because the failure was the compiler's, + /// not the expression's. + /// + [UnityTest] + public IEnumerator RestoreAsync_WhenTheCompilerThrows_ReportsThatRecordAndKeepsGoing() + { + WatchExpressionRegistry registry = CreateRegistry(); + InMemoryWatchPersistenceStore store = new( + Record("throwing", "boom", 20), + Record("kept", "1 + 1", 20)); + FakeWatchExpressionCompiler compiler = new(); + compiler.ThrowFor("boom", "the compiler process died"); + WatchRestoreService service = new(registry, compiler, store, () => { }); + + Task task = service.RestoreAsync(CancellationToken.None); + yield return WaitFor(task); + + WatchRestoreReport report = task.Result; + Assert.That(report.RestoredCount, Is.EqualTo(1)); + Assert.That(report.Warnings, Has.Count.EqualTo(1)); + Assert.That(report.Warnings[0], Does.Contain("throwing")); + Assert.That(report.Warnings[0], Does.Contain("the compiler process died")); + Assert.That(IdsOf(registry), Is.EqualTo(new[] { "kept" })); + Assert.That( + IdsOf(store.Saved), + Is.EqualTo(new[] { "kept", "throwing" }), + "A compiler exception must not erase the user's watch; it is retried after the next reload."); + } + + /// + /// What: a record whose id the user re-registered meanwhile is not reported as dropped, + /// even when its stored expression no longer compiles - the watch is not gone. + /// + [UnityTest] + public IEnumerator RestoreAsync_WhenAFailingRecordsIdWasRegisteredMeanwhile_ReportsNothing() + { + WatchExpressionRegistry registry = CreateRegistry(); + registry.Register("first", "999", new ConstantWatchExpressionEvaluator(999), 20); + InMemoryWatchPersistenceStore store = new(Record("first", "MissingType.Value", 20)); + FakeWatchExpressionCompiler compiler = new(); + compiler.FailFor("MissingType.Value", "The name 'MissingType' does not exist"); + WatchRestoreService service = new(registry, compiler, store, () => { }); + + Task task = service.RestoreAsync(CancellationToken.None); + yield return WaitFor(task); + + Assert.That(task.Result.Warnings, Is.Empty); + Assert.That(store.Saved[0].Expression, Is.EqualTo("999")); + } + + /// + /// What: a restore cancelled mid-flight registers nothing further and must not write the + /// store, because the clear that cancelled it already wrote the authoritative empty store. + /// + [UnityTest] + public IEnumerator RestoreAsync_WhenCancelledWhileCompiling_LeavesTheStoreToTheClearThatCancelledIt() + { + WatchExpressionRegistry registry = CreateRegistry(); + InMemoryWatchPersistenceStore store = new( + Record("first", "1 + 1", 20), + Record("second", "2 + 2", 20)); + CancellationTokenSource cancellation = new(); + FakeWatchExpressionCompiler compiler = new(); + compiler.CancelOnCompile(cancellation); + WatchRestoreService service = new(registry, compiler, store, () => { }); + + Task task = service.RestoreAsync(cancellation.Token); + yield return WaitFor(task); + + Assert.That(registry.GetEntries(), Is.Empty); + Assert.That(store.SaveCount, Is.EqualTo(0)); + Assert.That(compiler.CompileCount, Is.EqualTo(1), "Cancellation must stop the loop, not just the current record."); + } + + /// + /// What: a cancellation that lands after the record compiled but before it is registered + /// still keeps the watch out of the registry the clear just emptied. + /// + [UnityTest] + public IEnumerator RestoreAsync_WhenCancelledAfterCompiling_RegistersNothing() + { + WatchExpressionRegistry registry = CreateRegistry(); + InMemoryWatchPersistenceStore store = new( + Record("first", "1 + 1", 20), + Record("second", "2 + 2", 20)); + CancellationTokenSource cancellation = new(); + FakeWatchExpressionCompiler compiler = new(); + compiler.CancelAfterCompile(cancellation); + WatchRestoreService service = new(registry, compiler, store, () => { }); + + Task task = service.RestoreAsync(cancellation.Token); + yield return WaitFor(task); + + Assert.That(registry.GetEntries(), Is.Empty); + Assert.That(store.SaveCount, Is.EqualTo(0)); + Assert.That(compiler.CompileCount, Is.EqualTo(1), "Cancellation must stop the loop, not just the current record."); + } + + /// + /// What: an empty store compiles nothing and leaves the store untouched. + /// + [UnityTest] + public IEnumerator RestoreAsync_WithEmptyStore_DoesNotCompileAnything() + { + WatchExpressionRegistry registry = CreateRegistry(); + InMemoryWatchPersistenceStore store = new(); + FakeWatchExpressionCompiler compiler = new(); + WatchRestoreService service = new(registry, compiler, store, () => { }); + + Task task = service.RestoreAsync(CancellationToken.None); + yield return WaitFor(task); + + Assert.That(task.Result.RestoredCount, Is.EqualTo(0)); + Assert.That(compiler.CompileCount, Is.EqualTo(0)); + Assert.That(store.SaveCount, Is.EqualTo(0)); + } + + private static IEnumerator WaitFor(Task task) + { + int frames = 0; + while (!task.IsCompleted && frames < MaxWaitFrames) + { + frames++; + yield return null; + } + + Assert.That(task.IsCompleted, Is.True, "Restore did not complete within the frame budget."); + Assert.That(task.Exception, Is.Null); + } + + private static WatchExpressionRegistry CreateRegistry() + { + return new WatchExpressionRegistry(new FakeWatchEditorStateProvider()); + } + + private static WatchPersistedRecord Record(string id, string expression, int maxHistory) + { + return new WatchPersistedRecord { Id = id, Expression = expression, MaxHistory = maxHistory }; + } + + private static IReadOnlyList IdsOf(WatchExpressionRegistry registry) + { + List ids = new(); + foreach (WatchExpressionEntry entry in registry.GetEntries()) + { + ids.Add(entry.Id); + } + + return ids; + } + + private static IReadOnlyList IdsOf(IReadOnlyList records) + { + List ids = new(); + foreach (WatchPersistedRecord record in records) + { + ids.Add(record.Id); + } + + return ids; + } + + private sealed class InMemoryWatchPersistenceStore : IWatchPersistenceStore + { + private IReadOnlyList _records; + + public InMemoryWatchPersistenceStore(params WatchPersistedRecord[] records) + { + _records = records; + } + + public IReadOnlyList Saved => _records; + public int SaveCount { get; private set; } + + public IReadOnlyList Load() + { + return _records; + } + + public void Save(IReadOnlyList records) + { + SaveCount++; + _records = records; + } + } + + private sealed class FakeWatchExpressionCompiler : IWatchExpressionCompiler + { + private readonly Dictionary _failuresByExpression = new(StringComparer.Ordinal); + + private readonly Dictionary _throwsByExpression = new(StringComparer.Ordinal); + private CancellationTokenSource _cancelOnCompile; + + private CancellationTokenSource _cancelAfterCompile; + + public int CompileCount { get; private set; } + + public void FailFor(string expression, string errorMessage) + { + _failuresByExpression[expression] = errorMessage; + } + + public void ThrowFor(string expression, string exceptionMessage) + { + _throwsByExpression[expression] = exceptionMessage; + } + + /// Mimics a clear-watch --all arriving while the first record is compiling. + public void CancelOnCompile(CancellationTokenSource cancellation) + { + _cancelOnCompile = cancellation; + } + + // Cancels once the compile task itself has completed, so the restore observes the + // cancellation between CompileAsync and the registry call rather than before either. + public void CancelAfterCompile(CancellationTokenSource cancellation) + { + _cancelAfterCompile = cancellation; + } + + public Task CompileAsync(string expression, CancellationToken ct) + { + CompileCount++; + _cancelOnCompile?.Cancel(); + if (_throwsByExpression.TryGetValue(expression, out string exceptionMessage)) + { + throw new InvalidOperationException(exceptionMessage); + } + + if (_failuresByExpression.TryGetValue(expression, out string errorMessage)) + { + return Task.FromResult(WatchCompilationResult.FailureResult( + "Watch expression compilation failed.", + new List { new() { Line = 1, Column = 1, Message = errorMessage, ErrorCode = "CS0103" } })); + } + + Task compiled = Task.FromResult( + WatchCompilationResult.SuccessResult(new ConstantWatchExpressionEvaluator(1))); + if (_cancelAfterCompile == null) + { + return compiled; + } + + CancellationTokenSource cancelAfterCompile = _cancelAfterCompile; + return compiled.ContinueWith( + completed => + { + cancelAfterCompile.Cancel(); + return completed.Result; + }, + TaskScheduler.Default); + } + } + + private sealed class ConstantWatchExpressionEvaluator : IWatchExpressionEvaluator + { + private readonly object _value; + + public ConstantWatchExpressionEvaluator(object value) + { + _value = value; + } + + public WatchEvaluationResult Evaluate() + { + return WatchEvaluationResult.SuccessResult(_value); + } + } + + private sealed class FakeWatchEditorStateProvider : IWatchEditorStateProvider + { + public int FrameCount => 1; + public bool IsPlaying => true; + public bool IsPaused => true; + public DateTime UtcNow => new(2026, 9, 8, 0, 0, 0, DateTimeKind.Utc); + } + } +} diff --git a/Assets/Tests/Editor/WatchRestoreServiceTests.cs.meta b/Assets/Tests/Editor/WatchRestoreServiceTests.cs.meta new file mode 100644 index 0000000000..a6387782c4 --- /dev/null +++ b/Assets/Tests/Editor/WatchRestoreServiceTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 16ffc6860dc4f4dfba49859fa7a39f3c +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/WatchSessionStateStoreTests.cs b/Assets/Tests/Editor/WatchSessionStateStoreTests.cs new file mode 100644 index 0000000000..21c2051bed --- /dev/null +++ b/Assets/Tests/Editor/WatchSessionStateStoreTests.cs @@ -0,0 +1,115 @@ +using System; +using System.Collections.Generic; + +using NUnit.Framework; + +using UnityEditor; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies watch expressions round-trip through SessionState and that unusable stored + /// content degrades to "no watches to restore" instead of failing Editor startup. + /// + [TestFixture] + public sealed class WatchSessionStateStoreTests + { + private const string RecordsKey = "io.github.hatayama.uloopmcp.watch.persistedRecords"; + + private string _originalValue; + + [SetUp] + public void SetUp() + { + _originalValue = SessionState.GetString(RecordsKey, string.Empty); + SessionState.EraseString(RecordsKey); + } + + [TearDown] + public void TearDown() + { + SessionState.SetString(RecordsKey, _originalValue); + } + + /// + /// What: expressions containing newlines, quotes, and non-ASCII survive the round trip byte for byte. + /// + [Test] + public void SaveThenLoad_WithAwkwardExpressionText_RoundTripsEveryField() + { + const string awkwardExpression = "\"quoted\" + \n\t'x' + \"\\u00e9\\u3042\""; + WatchSessionStateStore store = new(); + + store.Save(new List + { + new() { Id = "first", Expression = awkwardExpression, MaxHistory = 7 }, + new() { Id = "second", Expression = "1 + 2", MaxHistory = 20 } + }); + IReadOnlyList loaded = store.Load(); + + Assert.That(loaded, Has.Count.EqualTo(2)); + Assert.That(loaded[0].Id, Is.EqualTo("first")); + Assert.That(loaded[0].Expression, Is.EqualTo(awkwardExpression)); + Assert.That(loaded[0].MaxHistory, Is.EqualTo(7)); + Assert.That(loaded[1].Id, Is.EqualTo("second")); + } + + /// + /// What: saving an empty list clears the store rather than leaving the previous records behind. + /// + [Test] + public void Save_WithEmptyList_LeavesNothingToRestore() + { + WatchSessionStateStore store = new(); + store.Save(new List + { + new() { Id = "first", Expression = "1 + 2", MaxHistory = 20 } + }); + + store.Save(Array.Empty()); + + Assert.That(store.Load(), Is.Empty); + } + + /// + /// What: a record without an expression cannot be recompiled, so saving it is rejected. + /// + [Test] + public void Save_WithNullExpression_ThrowsBecauseTheRecordCouldNeverBeRestored() + { + WatchSessionStateStore store = new(); + + Assert.That( + () => store.Save(new List + { + new() { Id = "first", Expression = null, MaxHistory = 20 } + }), + Throws.ArgumentException); + } + + /// + /// What: an unset key means no watches were persisted, not an error. + /// + [Test] + public void Load_WhenKeyWasNeverWritten_ReturnsEmpty() + { + WatchSessionStateStore store = new(); + + Assert.That(store.Load(), Is.Empty); + } + + /// + /// What: content written by another uloop generation degrades to empty instead of throwing. + /// + [Test] + public void Load_WhenStoredJsonIsNotAWatchRecordList_ReturnsEmpty() + { + SessionState.SetString(RecordsKey, "{ this is not json"); + WatchSessionStateStore store = new(); + + Assert.That(store.Load(), Is.Empty); + } + } +} diff --git a/Assets/Tests/Editor/WatchSessionStateStoreTests.cs.meta b/Assets/Tests/Editor/WatchSessionStateStoreTests.cs.meta new file mode 100644 index 0000000000..1691890198 --- /dev/null +++ b/Assets/Tests/Editor/WatchSessionStateStoreTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ea1bddefb3ee344bb98379b17f0d3226 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/WatchToolTests.cs b/Assets/Tests/Editor/WatchToolTests.cs index 982818d7aa..5d9be4bbd4 100644 --- a/Assets/Tests/Editor/WatchToolTests.cs +++ b/Assets/Tests/Editor/WatchToolTests.cs @@ -1,3 +1,5 @@ +using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -8,21 +10,27 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor { /// - /// Verifies external watch tool input validation and empty-state responses. + /// Verifies external watch tool input validation, empty-state responses, persistence of the + /// registry across watch commands, and how a domain-reload restore report reaches the caller. /// [TestFixture] public sealed class WatchToolTests { + private InMemoryWatchPersistenceStore _store; + [SetUp] public void SetUp() { WatchExpressionServices.Registry.ClearAll(); + _store = new InMemoryWatchPersistenceStore(); + WatchExpressionServices.OverrideStoreForTesting(_store); } [TearDown] public void TearDown() { WatchExpressionServices.Registry.ClearAll(); + WatchExpressionServices.ResetForTesting(); } /// @@ -55,6 +63,140 @@ public void EnableAsync_WhenMaxHistoryIsZero_ReturnsValidationFailure() Assert.That(task.Result.Message, Does.Contain("between 1 and 100")); } + /// + /// What: a rejected registration leaves the persisted records untouched. + /// + [Test] + public void EnableAsync_WhenValidationFails_DoesNotTouchThePersistedRecords() + { + Task task = WatchUseCase.EnableAsync( + new EnableWatchSchema { Id = "speed", Expression = string.Empty }, + CancellationToken.None); + + Assert.That(task.Result.Success, Is.False); + Assert.That(_store.SaveCount, Is.EqualTo(0)); + } + + /// + /// What: a successful enable-watch persists the watch, so the next domain reload restores it. + /// + [Test] + public void EnableAsync_WhenRegistrationSucceeds_PersistsTheWatchForTheNextDomainReload() + { + WatchExpressionServices.OverrideCompilerForTesting(new StubWatchExpressionCompiler()); + + Task task = WatchUseCase.EnableAsync( + new EnableWatchSchema { Id = "speed", Expression = "1 + 2", MaxHistory = 9 }, + CancellationToken.None); + + Assert.That(task.IsCompletedSuccessfully, Is.True); + Assert.That(task.Result.Success, Is.True); + Assert.That(_store.Records, Has.Count.EqualTo(1)); + Assert.That(_store.Records[0].Id, Is.EqualTo("speed")); + Assert.That(_store.Records[0].Expression, Is.EqualTo("1 + 2")); + Assert.That(_store.Records[0].MaxHistory, Is.EqualTo(9)); + } + + /// + /// What: asking for exactly the watch the reload dropped explains why it is gone. + /// + [Test] + public void GetValues_WhenTheRequestedWatchWasDroppedByTheRestore_ExplainsItInWarning() + { + WatchExpressionServices.SetLastRestoreReportForTesting( + new WatchRestoreReport(0, new[] { "Watch 'speed' was not restored." })); + + WatchResponse response = WatchUseCase.GetValues(new GetWatchValuesSchema { Id = "speed" }); + + Assert.That(response.Success, Is.False); + Assert.That(response.Warning, Does.Contain("Watch 'speed' was not restored.")); + } + + /// + /// What: the registry snapshot written for the next domain reload carries every restore input. + /// + [Test] + public void SaveRegistrySnapshot_AfterRegistration_PersistsIdExpressionAndHistoryLimit() + { + WatchExpressionServices.Registry.Register( + "speed", + "1 + 2", + new ConstantWatchExpressionEvaluator(3), + 7); + + WatchExpressionServices.SaveRegistrySnapshot(); + + Assert.That(_store.Records, Has.Count.EqualTo(1)); + Assert.That(_store.Records[0].Id, Is.EqualTo("speed")); + Assert.That(_store.Records[0].Expression, Is.EqualTo("1 + 2")); + Assert.That(_store.Records[0].MaxHistory, Is.EqualTo(7)); + } + + /// + /// What: clearing one watch removes it from the records the next domain reload would restore. + /// + [Test] + public void Clear_WhenAWatchIsCleared_RemovesItFromThePersistedRecords() + { + WatchExpressionServices.Registry.Register( + "speed", + "1 + 2", + new ConstantWatchExpressionEvaluator(3), + 20); + WatchExpressionServices.SaveRegistrySnapshot(); + + WatchResponse response = WatchUseCase.Clear(new ClearWatchSchema { Id = "speed" }); + + Assert.That(response.Success, Is.True); + Assert.That(_store.Records, Is.Empty); + } + + /// + /// What: clearing every watch empties the persisted records too. + /// + [Test] + public void Clear_WithAll_EmptiesThePersistedRecords() + { + WatchExpressionServices.Registry.Register( + "speed", + "1 + 2", + new ConstantWatchExpressionEvaluator(3), + 20); + WatchExpressionServices.SaveRegistrySnapshot(); + + WatchResponse response = WatchUseCase.Clear(new ClearWatchSchema { All = true }); + + Assert.That(response.ClearedCount, Is.EqualTo(1)); + Assert.That(_store.Records, Is.Empty); + } + + /// + /// What: watches dropped by the last domain reload are reported on the next value read. + /// + [Test] + public void GetValues_WhenTheLastRestoreDroppedWatches_ReportsThemInWarning() + { + WatchExpressionServices.SetLastRestoreReportForTesting( + new WatchRestoreReport(0, new[] { "Watch 'speed' was not restored.", "Watch 'hp' was not restored." })); + + WatchResponse response = WatchUseCase.GetValues(new GetWatchValuesSchema()); + + Assert.That(response.Warning, Does.Contain("Watch 'speed' was not restored.")); + Assert.That(response.Warning, Does.Contain("Watch 'hp' was not restored.")); + } + + /// + /// What: with no restore report there is no Warning, and the field is omitted from the response JSON. + /// + [Test] + public void GetValues_WhenNoRestoreHasRun_LeavesWarningUnset() + { + WatchResponse response = WatchUseCase.GetValues(new GetWatchValuesSchema()); + + Assert.That(response.Warning, Is.Null); + Assert.That(response.ShouldSerializeWarning(), Is.False); + } + /// /// Verifies get-watch-values returns a successful empty collection before registration. /// @@ -67,5 +209,48 @@ public void GetValues_WhenNoWatchesAreRegistered_ReturnsEmptySuccess() Assert.That(response.Watches, Is.Empty); Assert.That(response.Message, Does.Contain("No watch expressions")); } + + private sealed class InMemoryWatchPersistenceStore : IWatchPersistenceStore + { + public IReadOnlyList Records { get; private set; } = + Array.Empty(); + + public int SaveCount { get; private set; } + + public IReadOnlyList Load() + { + return Records; + } + + public void Save(IReadOnlyList records) + { + SaveCount++; + Records = records; + } + } + + private sealed class StubWatchExpressionCompiler : IWatchExpressionCompiler + { + public Task CompileAsync(string expression, CancellationToken ct) + { + return Task.FromResult( + WatchCompilationResult.SuccessResult(new ConstantWatchExpressionEvaluator(3))); + } + } + + private sealed class ConstantWatchExpressionEvaluator : IWatchExpressionEvaluator + { + private readonly object _value; + + public ConstantWatchExpressionEvaluator(object value) + { + _value = value; + } + + public WatchEvaluationResult Evaluate() + { + return WatchEvaluationResult.SuccessResult(_value); + } + } } } diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/watch-expressions.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/watch-expressions.md index babbedf687..e7275d4e07 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/watch-expressions.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/watch-expressions.md @@ -17,4 +17,8 @@ The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect ## Lifetime -Watch expressions are in-memory Editor state. A domain reload clears them, so re-register them after `uloop compile`, script recompilation, or an Editor restart. For reliable per-Step changes, keep the expression attached to a continuous pause point on an `Update` or `FixedUpdate` line and use `control-play-mode --action Step`. +Watch expressions survive a domain reload. They are saved in Editor session state, so `uloop compile`, a script recompilation, and a Play entry with Domain Reload enabled all keep them registered — the expression is recompiled after the reload and evaluation starts from a fresh baseline, so history collected before the reload is gone. + +A watch whose expression no longer compiles after the reload (its type was renamed or deleted) is dropped, and the next `enable-watch` or `get-watch-values` response reports it in `Warning` with the id and the compiler message. + +Watch expressions do not survive an Editor restart — session state ends with the Editor process. For reliable per-Step changes, keep the expression attached to a continuous pause point on an `Update` or `FixedUpdate` line and use `control-play-mode --action Step`. diff --git a/Packages/src/Editor/FirstPartyTools/FirstPartyToolsEditorStartup.cs b/Packages/src/Editor/FirstPartyTools/FirstPartyToolsEditorStartup.cs index 268da1f2f4..2935d55193 100644 --- a/Packages/src/Editor/FirstPartyTools/FirstPartyToolsEditorStartup.cs +++ b/Packages/src/Editor/FirstPartyTools/FirstPartyToolsEditorStartup.cs @@ -14,6 +14,7 @@ public static void Initialize() PausePointEditorStartup.Initialize(); CompileEditorStartup.Initialize(); HotReloadEditorStartup.Initialize(); + WatchEditorStartup.Initialize(); ExecuteDynamicCodeEditorStartup.Initialize(); GetLogsEditorStartup.Initialize(); ScreenshotEditorStartup.Initialize(); diff --git a/Packages/src/Editor/FirstPartyTools/Watch/AssemblyInfo.cs b/Packages/src/Editor/FirstPartyTools/Watch/AssemblyInfo.cs index dfa2c7f965..8684cb36be 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/AssemblyInfo.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/AssemblyInfo.cs @@ -1,3 +1,7 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor")] + +// Why: CompositionRoot's asmdef does not reference Watch, so the post-domain-reload restore is +// registered through the FirstPartyTools.Editor facade (same pattern as HotReload and PausePoint). +[assembly: InternalsVisibleTo("UnityCLILoop.FirstPartyTools.Editor")] diff --git a/Packages/src/Editor/FirstPartyTools/Watch/IWatchExpressionCompiler.cs b/Packages/src/Editor/FirstPartyTools/Watch/IWatchExpressionCompiler.cs new file mode 100644 index 0000000000..4d3d118300 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/IWatchExpressionCompiler.cs @@ -0,0 +1,14 @@ +using System.Threading; +using System.Threading.Tasks; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Compiles one watch expression into an evaluator. Exists so restore can be driven without + /// the real Roslyn compilation pipeline. + /// + public interface IWatchExpressionCompiler + { + Task CompileAsync(string expression, CancellationToken ct); + } +} diff --git a/Packages/src/Editor/FirstPartyTools/Watch/IWatchExpressionCompiler.cs.meta b/Packages/src/Editor/FirstPartyTools/Watch/IWatchExpressionCompiler.cs.meta new file mode 100644 index 0000000000..a97284fe94 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/IWatchExpressionCompiler.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: df8174ac6b913476295eaccc2b19f9a7 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/Watch/IWatchPersistenceStore.cs b/Packages/src/Editor/FirstPartyTools/Watch/IWatchPersistenceStore.cs new file mode 100644 index 0000000000..9245be20a1 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/IWatchPersistenceStore.cs @@ -0,0 +1,14 @@ +using System.Collections.Generic; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Stores the watch expressions that must be re-registered after a domain reload. + /// + internal interface IWatchPersistenceStore + { + IReadOnlyList Load(); + + void Save(IReadOnlyList records); + } +} diff --git a/Packages/src/Editor/FirstPartyTools/Watch/IWatchPersistenceStore.cs.meta b/Packages/src/Editor/FirstPartyTools/Watch/IWatchPersistenceStore.cs.meta new file mode 100644 index 0000000000..a643968568 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/IWatchPersistenceStore.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 594814ce7cb734f9b82daccccddd8095 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchEditorStartup.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchEditorStartup.cs new file mode 100644 index 0000000000..60814c1e87 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchEditorStartup.cs @@ -0,0 +1,25 @@ +using UnityEditor; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + // Keeps watch startup wiring inside the watch assembly so the composition root only + // depends on the bundled-tool facade. + internal static class WatchEditorStartup + { + public static void Initialize() + { + // Why not EditorApplication.delayCall: a cold-start session that hits Unity's native + // "Scripts have compiler errors" dialog never flushes delayCall again for the rest of + // that process's lifetime, even for later registrations, while EditorApplication.update + // keeps ticking. Restoring on the first update tick also gives the dynamic-code + // compilation pipeline a fully initialized domain to compile against. + void RestoreOnFirstUpdateTick() + { + EditorApplication.update -= RestoreOnFirstUpdateTick; + WatchExpressionServices.RestoreAfterDomainReload(); + } + + EditorApplication.update += RestoreOnFirstUpdateTick; + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchEditorStartup.cs.meta b/Packages/src/Editor/FirstPartyTools/Watch/WatchEditorStartup.cs.meta new file mode 100644 index 0000000000..1a3637ca1a --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchEditorStartup.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2f2bcfb3d3959427db37ea9dd875e01d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchExpressionCompiler.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchExpressionCompiler.cs index 41555926e4..013d2b41b5 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchExpressionCompiler.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchExpressionCompiler.cs @@ -9,7 +9,7 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// /// Wraps one user expression in a compiled evaluator using the execute-dynamic-code compiler. /// - public sealed class WatchExpressionCompiler + public sealed class WatchExpressionCompiler : IWatchExpressionCompiler { private const string WatchNamespace = "io.github.hatayama.UnityCliLoop.DynamicWatch"; private const string WatchClassName = "WatchExpressionEntryPoint"; diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchPersistedRecord.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchPersistedRecord.cs new file mode 100644 index 0000000000..cbc56807ce --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchPersistedRecord.cs @@ -0,0 +1,27 @@ +using System; +using System.Collections.Generic; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// One watch expression reduced to what a domain reload can carry: the compiled evaluator is + /// an in-memory assembly that cannot be serialized, so only the inputs needed to compile it + /// again are stored. + /// + [Serializable] + internal sealed class WatchPersistedRecord + { + public string Id; + public string Expression; + public int MaxHistory; + } + + /// + /// JsonUtility serializes only classes with fields, so the record collection needs this wrapper. + /// + [Serializable] + internal sealed class WatchPersistedRecordList + { + public List Records = new(); + } +} diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchPersistedRecord.cs.meta b/Packages/src/Editor/FirstPartyTools/Watch/WatchPersistedRecord.cs.meta new file mode 100644 index 0000000000..dc037546d3 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchPersistedRecord.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a01737ebb627b41d79684b74f6247f56 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreReport.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreReport.cs new file mode 100644 index 0000000000..8e8dfc7e45 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreReport.cs @@ -0,0 +1,30 @@ +using System; +using System.Collections.Generic; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Result of re-registering the stored watch expressions after a domain reload. + /// + internal sealed class WatchRestoreReport + { + public static readonly WatchRestoreReport Empty = + new(0, Array.Empty()); + + public WatchRestoreReport(int restoredCount, IReadOnlyList warnings) + { + if (warnings == null) + { + throw new ArgumentNullException(nameof(warnings)); + } + + RestoredCount = restoredCount; + Warnings = warnings; + } + + public int RestoredCount { get; } + + /// Watches that were dropped, one human-readable line each. + public IReadOnlyList Warnings { get; } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreReport.cs.meta b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreReport.cs.meta new file mode 100644 index 0000000000..bc938166e4 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreReport.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4fc8cebd3fcc04d1f85a2aa8496f864f +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs new file mode 100644 index 0000000000..992f536d4f --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs @@ -0,0 +1,222 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Re-registers the watch expressions a domain reload dropped, recompiling each stored + /// expression because the evaluator assembly itself cannot survive the reload. + /// + internal sealed class WatchRestoreService + { + private readonly WatchExpressionRegistry _registry; + private readonly IWatchExpressionCompiler _compiler; + private readonly IWatchPersistenceStore _store; + private readonly Action _ensureMonitorStarted; + + public WatchRestoreService( + WatchExpressionRegistry registry, + IWatchExpressionCompiler compiler, + IWatchPersistenceStore store, + Action ensureMonitorStarted) + { + _registry = registry ?? throw new ArgumentNullException(nameof(registry)); + _compiler = compiler ?? throw new ArgumentNullException(nameof(compiler)); + _store = store ?? throw new ArgumentNullException(nameof(store)); + _ensureMonitorStarted = ensureMonitorStarted + ?? throw new ArgumentNullException(nameof(ensureMonitorStarted)); + } + + public async Task RestoreAsync(CancellationToken ct) + { + IReadOnlyList records = _store.Load(); + if (records.Count == 0) + { + return WatchRestoreReport.Empty; + } + + RestorePass pass = new(); + int restoredCount = 0; + // Sequential, not parallel: the registry evaluates watches in registration order and + // the restored order has to match the order the user registered them in. + foreach (WatchPersistedRecord record in records) + { + if (ct.IsCancellationRequested) + { + return CancelledReport(restoredCount); + } + + bool restored; + try + { + restored = await TryRestoreAsync(record, pass, ct).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Cancellation is how clear-watch --all tells restore to stop; it is the + // user's intent, not a failure to report. + return CancelledReport(restoredCount); + } + + if (ct.IsCancellationRequested) + { + return CancelledReport(restoredCount); + } + + if (restored) + { + restoredCount++; + } + } + + // The ConfigureAwait(false) awaits above can resume off-thread, and both the monitor + // and the SessionState-backed store are main-thread only. + await MainThreadSwitcher.SwitchToMainThread(ct); + if (restoredCount > 0) + { + _ensureMonitorStarted(); + } + + // Overwrite with what actually made it back, so a watch that no longer compiles is + // reported once instead of failing again on every later reload. + _store.Save(SnapshotRegistry(pass.RetainForRetry)); + return new WatchRestoreReport(restoredCount, pass.Warnings); + } + + // Why no Save here: the only thing that cancels a restore is a clear the user asked for, + // and that clear already wrote the authoritative store. Saving the registry now would + // re-persist the records the clear just removed and silently undo it. + private static WatchRestoreReport CancelledReport(int restoredCount) + { + return new WatchRestoreReport(restoredCount, Array.Empty()); + } + + private async Task TryRestoreAsync( + WatchPersistedRecord record, + RestorePass pass, + CancellationToken ct) + { + WatchCompilationResult compiled; + try + { + compiled = await _compiler.CompileAsync(record.Expression, ct).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Cancellation ends the whole restore, so it must not be turned into a warning here. + throw; + } + catch (Exception exception) + { + // An exception is the compiler failing, not the expression being invalid, so the + // watch stays in the store and is retried after the next reload. One such failure + // must not stop the remaining watches from coming back either. + AddDroppedWarning(pass.Warnings, record, exception.Message); + pass.RetainForRetry.Add(record); + return false; + } + + // Why switch back: CompileAsync resumes off-thread, but the registry evaluates the + // expression on registration and must run on the Unity main thread. + await MainThreadSwitcher.SwitchToMainThread(ct); + if (!compiled.Success) + { + // A compilation result failure is the user's expression being invalid, so the + // record is dropped rather than retried. + AddDroppedWarning(pass.Warnings, record, DescribeCompilationFailure(compiled)); + return false; + } + + WatchRegistrationResult registered = _registry.Register( + record.Id, + record.Expression, + compiled.Evaluator, + record.MaxHistory); + // A duplicate id means the user re-registered the watch while restore was compiling. + // Their registration is the newer intent, so it wins and the skip stays silent. + return registered.Success; + } + + // Why check the registry first: if the user re-registered this id while restore was + // compiling, the watch is not gone and telling them it was dropped would be false. + private void AddDroppedWarning(List warnings, WatchPersistedRecord record, string reason) + { + if (IsRegistered(record.Id)) + { + return; + } + + warnings.Add( + $"Watch '{record.Id}' was not restored after the domain reload because its " + + $"expression no longer compiles: {reason}"); + } + + private bool IsRegistered(string id) + { + foreach (WatchExpressionEntry entry in _registry.GetEntries()) + { + if (string.Equals(entry.Id, id, StringComparison.Ordinal)) + { + return true; + } + } + + return false; + } + + private static string DescribeCompilationFailure(WatchCompilationResult compiled) + { + if (compiled.CompilationErrors != null && compiled.CompilationErrors.Count > 0) + { + return compiled.CompilationErrors[0].Message; + } + + return compiled.ErrorMessage; + } + + private IReadOnlyList SnapshotRegistry( + IReadOnlyList retainForRetry) + { + IReadOnlyList entries = _registry.GetEntries(); + List records = new(entries.Count + retainForRetry.Count); + foreach (WatchExpressionEntry entry in entries) + { + records.Add(new WatchPersistedRecord + { + Id = entry.Id, + Expression = entry.Expression, + MaxHistory = entry.MaxHistory + }); + } + + foreach (WatchPersistedRecord record in retainForRetry) + { + // The user may have registered this id themselves while restore was compiling; + // their version is already in the snapshot and must not be duplicated. + if (IsRegistered(record.Id)) + { + continue; + } + + records.Add(record); + } + + return records; + } + + /// + /// Carries what one restore pass accumulated across its records: the warnings to report + /// and the records whose failure was ours rather than the expression's. + /// + private sealed class RestorePass + { + public List Warnings { get; } = new(); + + public List RetainForRetry { get; } = new(); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs.meta b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs.meta new file mode 100644 index 0000000000..4c9cc1a57a --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 46ef1392a762b4d849a1e4e3b720b3fd +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchSessionStateStore.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchSessionStateStore.cs new file mode 100644 index 0000000000..af027c8307 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchSessionStateStore.cs @@ -0,0 +1,88 @@ +using System; +using System.Collections.Generic; +using UnityEditor; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Keeps watch expressions in SessionState, the Editor-local store that survives a domain + /// reload and is dropped when the Editor process exits. + /// + internal sealed class WatchSessionStateStore : IWatchPersistenceStore + { + private const string RecordsKey = "io.github.hatayama.uloopmcp.watch.persistedRecords"; + + public IReadOnlyList Load() + { + string json = SessionState.GetString(RecordsKey, string.Empty); + if (string.IsNullOrEmpty(json)) + { + return Array.Empty(); + } + + WatchPersistedRecordList parsed = TryParse(json); + if (parsed?.Records == null) + { + return Array.Empty(); + } + + List records = new(parsed.Records.Count); + foreach (WatchPersistedRecord record in parsed.Records) + { + // A record without an expression cannot be recompiled, so it is not a watch to restore. + if (record == null || string.IsNullOrEmpty(record.Id) || string.IsNullOrEmpty(record.Expression)) + { + continue; + } + + records.Add(record); + } + + return records; + } + + public void Save(IReadOnlyList records) + { + if (records == null) + { + throw new ArgumentNullException(nameof(records)); + } + + WatchPersistedRecordList payload = new(); + foreach (WatchPersistedRecord record in records) + { + if (record == null || string.IsNullOrEmpty(record.Id) || string.IsNullOrEmpty(record.Expression)) + { + throw new ArgumentException( + "Every persisted watch record must carry a non-empty Id and Expression.", + nameof(records)); + } + + payload.Records.Add(record); + } + + SessionState.SetString(RecordsKey, JsonUtility.ToJson(payload)); + } + + // Why swallow: a malformed value can only come from a store written by another uloop + // generation, and losing the restore is better than blocking Editor startup on it. + private static WatchPersistedRecordList TryParse(string json) + { + try + { + return JsonUtility.FromJson(json); + } + catch (ArgumentException exception) + { + VibeLogger.LogWarning( + "watch_persisted_records_parse_failed", + "Stored watch expressions could not be parsed and were dropped.", + new { error = exception.Message }); + return null; + } + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchSessionStateStore.cs.meta b/Packages/src/Editor/FirstPartyTools/Watch/WatchSessionStateStore.cs.meta new file mode 100644 index 0000000000..45b4d2f704 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchSessionStateStore.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 075a718e3427446d28d7f2fd95bf1739 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs index b0c54bccc5..476d6b2056 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs @@ -50,6 +50,17 @@ public sealed class WatchResponse : UnityCliLoopToolResponse Array.Empty(); public IReadOnlyList CompilationErrors { get; set; } = Array.Empty(); + + /// + /// Set when watches were dropped by the last domain reload, so the caller learns about it + /// on the next watch command instead of silently seeing fewer watches. + /// + public string Warning { get; set; } + + public bool ShouldSerializeWarning() + { + return !string.IsNullOrEmpty(Warning); + } } /// @@ -250,18 +261,26 @@ public static async Task EnableAsync(EnableWatchSchema parameters } WatchExpressionServices.EnsureMonitorStarted(); + WatchExpressionServices.SaveRegistrySnapshot(); WatchExpressionEntry entry = WatchExpressionServices.Registry.GetEntries() .Single(candidate => candidate.Id == parameters.Id); - return WatchResponseFromEntry(entry, "Watch expression enabled."); + WatchResponse response = WatchResponseFromEntry(entry, "Watch expression enabled."); + response.Warning = BuildRestoreWarning(); + return response; } public static WatchResponse Clear(ClearWatchSchema parameters) { if (parameters.All) { + // Stop any in-flight restore first: otherwise it would keep re-registering the + // watches this call is clearing and save them back over the empty store. + WatchExpressionServices.CancelPendingRestore(); + int clearedCount = WatchExpressionServices.Registry.ClearAll(); + WatchExpressionServices.SaveRegistrySnapshot(); return new WatchResponse { - ClearedCount = WatchExpressionServices.Registry.ClearAll(), + ClearedCount = clearedCount, Message = "Watch expressions cleared." }; } @@ -272,9 +291,13 @@ public static WatchResponse Clear(ClearWatchSchema parameters) } bool cleared = WatchExpressionServices.Registry.Clear(parameters.Id); - return cleared - ? new WatchResponse { Id = parameters.Id, Message = "Watch expression cleared." } - : CreateFailure($"Watch expression '{parameters.Id}' was not found."); + if (!cleared) + { + return CreateFailure($"Watch expression '{parameters.Id}' was not found."); + } + + WatchExpressionServices.SaveRegistrySnapshot(); + return new WatchResponse { Id = parameters.Id, Message = "Watch expression cleared." }; } public static WatchResponse GetValues(GetWatchValuesSchema parameters) @@ -285,17 +308,37 @@ public static WatchResponse GetValues(GetWatchValuesSchema parameters) entries = entries.Where(entry => entry.Id == parameters.Id).ToList(); if (entries.Count == 0) { - return CreateFailure($"Watch expression '{parameters.Id}' was not found."); + // Asking for exactly the watch the reload dropped is the one moment the caller + // most needs the restore report, so the failure carries it too. + WatchResponse notFound = CreateFailure($"Watch expression '{parameters.Id}' was not found."); + notFound.Warning = BuildRestoreWarning(); + return notFound; } } return new WatchResponse { Watches = entries.Select(WatchEntryResponse.FromEntry).ToList(), - Message = entries.Count == 0 ? "No watch expressions are registered." : "Watch values retrieved." + Message = entries.Count == 0 ? "No watch expressions are registered." : "Watch values retrieved.", + Warning = BuildRestoreWarning() }; } + /// + /// Surfaces the watches the last domain reload dropped. Clearing a watch is not a place + /// the caller needs to hear about it, so only enable and value reads carry it. + /// + private static string BuildRestoreWarning() + { + WatchRestoreReport report = WatchExpressionServices.LastRestoreReport; + if (report == null || report.Warnings.Count == 0) + { + return null; + } + + return string.Join(" ", report.Warnings); + } + private static string ValidateEnable(EnableWatchSchema parameters) { if (string.IsNullOrWhiteSpace(parameters.Id)) @@ -359,15 +402,116 @@ internal static class WatchExpressionServices { private static readonly UnityWatchEditorStateProvider StateProvider = new(); private static readonly WatchExpressionRegistry RegistryValue = new(StateProvider); - private static readonly WatchExpressionCompiler CompilerValue = new(new DynamicCodeCompiler()); + private static readonly WatchExpressionCompiler DefaultCompiler = new(new DynamicCodeCompiler()); + private static IWatchExpressionCompiler _compiler = DefaultCompiler; private static readonly WatchExpressionStepMonitor Monitor = new(RegistryValue); + private static readonly IWatchPersistenceStore DefaultStore = new WatchSessionStateStore(); + private static IWatchPersistenceStore _store = DefaultStore; + private static CancellationTokenSource _restoreCancellation; public static WatchExpressionRegistry Registry => RegistryValue; - public static WatchExpressionCompiler Compiler => CompilerValue; + public static IWatchExpressionCompiler Compiler => _compiler; + public static IWatchPersistenceStore Store => _store; + + /// + /// Result of the restore that ran after the last domain reload, or null when no restore + /// has completed in this domain. + /// + public static WatchRestoreReport LastRestoreReport { get; private set; } public static void EnsureMonitorStarted() { Monitor.Start(); } + + /// + /// Recompiles and re-registers the watches the domain reload dropped. Fire-and-forget + /// because compilation is asynchronous and Editor startup must not block on it. + /// + public static void RestoreAfterDomainReload() + { + _restoreCancellation = new CancellationTokenSource(); + _ = RestoreAfterDomainReloadAsync(_restoreCancellation.Token); + } + + /// + /// Stops a restore that is still compiling. Without this, a clear issued mid-restore would + /// be undone: the restore would keep registering the records the clear just removed and + /// write them back over the empty store. + /// + public static void CancelPendingRestore() + { + _restoreCancellation?.Cancel(); + } + + private static async Task RestoreAfterDomainReloadAsync(CancellationToken ct) + { + try + { + WatchRestoreService restoreService = new( + RegistryValue, + _compiler, + _store, + EnsureMonitorStarted); + LastRestoreReport = await restoreService.RestoreAsync(ct).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // A cancelled restore is a clear the user asked for, not a failure. + } + catch (Exception exception) + { + // Why catch everything: this runs on an Editor update tick with nobody to observe + // the Task, so an escaping exception would only surface as an unobserved-task log. + VibeLogger.LogError( + "watch_restore_after_domain_reload_failed", + "Restoring watch expressions after the domain reload failed.", + new { error = exception.ToString() }); + } + } + + /// + /// Writes the registry's current contents over the persisted records, so the next domain + /// reload restores exactly what is registered now. + /// + public static void SaveRegistrySnapshot() + { + IReadOnlyList entries = RegistryValue.GetEntries(); + List records = new(entries.Count); + foreach (WatchExpressionEntry entry in entries) + { + records.Add(new WatchPersistedRecord + { + Id = entry.Id, + Expression = entry.Expression, + MaxHistory = entry.MaxHistory + }); + } + + _store.Save(records); + } + + internal static void OverrideStoreForTesting(IWatchPersistenceStore store) + { + _store = store ?? throw new ArgumentNullException(nameof(store)); + } + + internal static void OverrideCompilerForTesting(IWatchExpressionCompiler compiler) + { + _compiler = compiler ?? throw new ArgumentNullException(nameof(compiler)); + } + + internal static void ResetForTesting() + { + _store = DefaultStore; + _compiler = DefaultCompiler; + LastRestoreReport = null; + _restoreCancellation = null; + } + + internal static void SetLastRestoreReportForTesting(WatchRestoreReport report) + { + LastRestoreReport = report; + } } } diff --git a/docs/glossary.md b/docs/glossary.md index 703b8ed1f8..148bed74a1 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -156,6 +156,15 @@ a pause point, represented by `UloopCapturedVariable` internally and exposed as line executes, and `CapturedVariablesTruncated` reports whether the length or count cap clipped any evidence. +### Watch expression + +One C# expression registered with `enable-watch` and evaluated once per paused frame, whose +bounded value history is read back with `get-watch-values`. Watch expressions are stored in +Editor session state and are restored — recompiled and re-registered — after a domain reload, +including the reload a `uloop compile` or a Play entry with Domain Reload enabled +causes; they are lost when the Editor process exits. Value history does not cross a +reload: a restored watch starts from a new baseline. + ### Hot reload The first-party tool `hot-reload` that replaces method bodies in a running Unity Editor