From f2c2b7c5a45ca6599aedec8a33bd2a296b393a3b Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 8 Sep 2026 09:23:05 +0900 Subject: [PATCH 1/5] feat: restore watch expressions after a domain reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Watch expressions lived only in the WatchExpressionRegistry static, so every domain reload — a compile, a script recompilation, a Play entry with Domain Reload enabled — wiped them with no trace in any response. The caller kept polling get-watch-values and saw an empty list with a success message. Persist (id, expression, maxHistory) in SessionState and re-register them on the first Editor update tick after the reload. The compiled evaluator is an in-memory assembly that cannot be serialized, so the expression is compiled again; history therefore restarts from a fresh baseline. Restore runs sequentially so the registration order, which is also the evaluation order, is preserved. A watch whose expression no longer compiles is dropped and named in the Warning of the next enable-watch or get-watch-values response, and it is removed from the store so the failure is not retried on every later reload. A watch the user re-registered while restore was still compiling keeps the user's registration, silently. Claude-Session: https://claude.ai/code/session_019kkGy4ujdjChrkMcimjKfY --- .../references/watch-expressions.md | 6 +- .../references/watch-expressions.md | 6 +- .../Tests/Editor/WatchRestoreServiceTests.cs | 242 ++++++++++++++++++ .../Editor/WatchRestoreServiceTests.cs.meta | 11 + .../Editor/WatchSessionStateStoreTests.cs | 115 +++++++++ .../WatchSessionStateStoreTests.cs.meta | 11 + Assets/Tests/Editor/WatchToolTests.cs | 143 ++++++++++- .../Skill/references/watch-expressions.md | 6 +- .../FirstPartyToolsEditorStartup.cs | 1 + .../FirstPartyTools/Watch/AssemblyInfo.cs | 4 + .../Watch/IWatchExpressionCompiler.cs | 14 + .../Watch/IWatchExpressionCompiler.cs.meta | 11 + .../Watch/IWatchPersistenceStore.cs | 14 + .../Watch/IWatchPersistenceStore.cs.meta | 11 + .../Watch/WatchEditorStartup.cs | 25 ++ .../Watch/WatchEditorStartup.cs.meta | 11 + .../Watch/WatchExpressionCompiler.cs | 2 +- .../Watch/WatchPersistedRecord.cs | 27 ++ .../Watch/WatchPersistedRecord.cs.meta | 11 + .../Watch/WatchRestoreReport.cs | 30 +++ .../Watch/WatchRestoreReport.cs.meta | 11 + .../Watch/WatchRestoreService.cs | 119 +++++++++ .../Watch/WatchRestoreService.cs.meta | 11 + .../Watch/WatchSessionStateStore.cs | 88 +++++++ .../Watch/WatchSessionStateStore.cs.meta | 11 + .../FirstPartyTools/Watch/WatchTools.cs | 127 ++++++++- docs/glossary.md | 9 + 27 files changed, 1065 insertions(+), 12 deletions(-) create mode 100644 Assets/Tests/Editor/WatchRestoreServiceTests.cs create mode 100644 Assets/Tests/Editor/WatchRestoreServiceTests.cs.meta create mode 100644 Assets/Tests/Editor/WatchSessionStateStoreTests.cs create mode 100644 Assets/Tests/Editor/WatchSessionStateStoreTests.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/IWatchExpressionCompiler.cs create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/IWatchExpressionCompiler.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/IWatchPersistenceStore.cs create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/IWatchPersistenceStore.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/WatchEditorStartup.cs create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/WatchEditorStartup.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/WatchPersistedRecord.cs create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/WatchPersistedRecord.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreReport.cs create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreReport.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/WatchSessionStateStore.cs create mode 100644 Packages/src/Editor/FirstPartyTools/Watch/WatchSessionStateStore.cs.meta 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..cb08fd02d2 --- /dev/null +++ b/Assets/Tests/Editor/WatchRestoreServiceTests.cs @@ -0,0 +1,242 @@ +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: 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); + + public int CompileCount { get; private set; } + + public void FailFor(string expression, string errorMessage) + { + _failuresByExpression[expression] = errorMessage; + } + + public Task CompileAsync(string expression, CancellationToken ct) + { + CompileCount++; + 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" } })); + } + + return Task.FromResult( + WatchCompilationResult.SuccessResult(new ConstantWatchExpressionEvaluator(1))); + } + } + + 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..e4d364e645 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.ResetStoreForTesting(); } /// @@ -55,6 +63,105 @@ 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: 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 +174,39 @@ 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 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..beb2f22adb --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs @@ -0,0 +1,119 @@ +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; + } + + List warnings = 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 (await TryRestoreAsync(record, warnings, ct)) + { + restoredCount++; + } + } + + 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()); + return new WatchRestoreReport(restoredCount, warnings); + } + + private async Task TryRestoreAsync( + WatchPersistedRecord record, + List warnings, + CancellationToken ct) + { + WatchCompilationResult compiled = await _compiler.CompileAsync(record.Expression, ct); + // 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) + { + warnings.Add( + $"Watch '{record.Id}' was not restored after the domain reload because its " + + $"expression no longer compiles: {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; + } + + 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 entries = _registry.GetEntries(); + List records = new(entries.Count); + foreach (WatchExpressionEntry entry in entries) + { + records.Add(new WatchPersistedRecord + { + Id = entry.Id, + Expression = entry.Expression, + MaxHistory = entry.MaxHistory + }); + } + + return records; + } + } +} 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..617e43e32d 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,23 @@ 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) { + int clearedCount = WatchExpressionServices.Registry.ClearAll(); + WatchExpressionServices.SaveRegistrySnapshot(); return new WatchResponse { - ClearedCount = WatchExpressionServices.Registry.ClearAll(), + ClearedCount = clearedCount, Message = "Watch expressions cleared." }; } @@ -272,9 +288,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) @@ -292,10 +312,26 @@ public static WatchResponse GetValues(GetWatchValuesSchema parameters) 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)) @@ -361,13 +397,90 @@ internal static class WatchExpressionServices private static readonly WatchExpressionRegistry RegistryValue = new(StateProvider); private static readonly WatchExpressionCompiler CompilerValue = new(new DynamicCodeCompiler()); private static readonly WatchExpressionStepMonitor Monitor = new(RegistryValue); + private static readonly IWatchPersistenceStore DefaultStore = new WatchSessionStateStore(); + private static IWatchPersistenceStore _store = DefaultStore; public static WatchExpressionRegistry Registry => RegistryValue; - public static WatchExpressionCompiler Compiler => CompilerValue; + public static IWatchExpressionCompiler Compiler => CompilerValue; + 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() + { + _ = RestoreAfterDomainReloadAsync(); + } + + private static async Task RestoreAfterDomainReloadAsync() + { + try + { + WatchRestoreService restoreService = new( + RegistryValue, + CompilerValue, + _store, + EnsureMonitorStarted); + LastRestoreReport = await restoreService.RestoreAsync(CancellationToken.None); + } + 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 ResetStoreForTesting() + { + _store = DefaultStore; + LastRestoreReport = null; + } + + internal static void SetLastRestoreReportForTesting(WatchRestoreReport report) + { + LastRestoreReport = report; + } } } diff --git a/docs/glossary.md b/docs/glossary.md index 703b8ed1f8..c451b33a93 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 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 From 460e8060ced0c30ec82b567411f598508cc35637 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 8 Sep 2026 09:30:48 +0900 Subject: [PATCH 2/5] fix: keep watch restore from contradicting the user and from stopping early Five review findings on the restore path: A clear-watch --all issued while restore was still compiling was silently undone: ClearAll wrote an empty store, then restore kept registering the remaining records and rewrote them over it. The restore now runs under a cancellation token that --all cancels before clearing, and a cancelled restore returns without saving, because the clear already wrote the authoritative store. A record whose id the user re-registered meanwhile no longer produces a "was not restored" warning when its stored expression fails to compile: the watch is not gone, so saying so would be false. This matches the rule the duplicate-registration path already followed. An exception thrown out of CompileAsync now becomes that record's warning instead of ending the restore, so one bad expression cannot strand the watches behind it. OperationCanceledException stays the exception and ends the restore. get-watch-values --id for a watch the restore dropped returned the plain not-found failure. It now carries the restore warning, which is the one moment the caller most needs it. The persistence tests drove the registry directly, so deleting the SaveRegistrySnapshot call from EnableAsync would not have failed anything. The compiler is now injectable for tests and one case runs EnableAsync end to end against a stub compiler. Claude-Session: https://claude.ai/code/session_019kkGy4ujdjChrkMcimjKfY --- .../Tests/Editor/WatchRestoreServiceTests.cs | 91 +++++++++++++++++++ Assets/Tests/Editor/WatchToolTests.cs | 46 +++++++++- .../Watch/WatchRestoreService.cs | 82 ++++++++++++++++- .../FirstPartyTools/Watch/WatchTools.cs | 47 ++++++++-- 4 files changed, 252 insertions(+), 14 deletions(-) diff --git a/Assets/Tests/Editor/WatchRestoreServiceTests.cs b/Assets/Tests/Editor/WatchRestoreServiceTests.cs index cb08fd02d2..3bf1ebadc4 100644 --- a/Assets/Tests/Editor/WatchRestoreServiceTests.cs +++ b/Assets/Tests/Editor/WatchRestoreServiceTests.cs @@ -102,6 +102,77 @@ public IEnumerator RestoreAsync_WhenTheIdWasRegisteredMeanwhile_KeepsTheExisting Assert.That(store.Saved[0].Expression, Is.EqualTo("999")); } + /// + /// What: a compile that throws becomes that record's warning and the rest still restore. + /// + [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" })); + } + + /// + /// 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: an empty store compiles nothing and leaves the store untouched. /// @@ -194,6 +265,9 @@ private sealed class FakeWatchExpressionCompiler : IWatchExpressionCompiler { private readonly Dictionary _failuresByExpression = new(StringComparer.Ordinal); + private readonly Dictionary _throwsByExpression = new(StringComparer.Ordinal); + private CancellationTokenSource _cancelOnCompile; + public int CompileCount { get; private set; } public void FailFor(string expression, string errorMessage) @@ -201,9 +275,26 @@ 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; + } + 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( diff --git a/Assets/Tests/Editor/WatchToolTests.cs b/Assets/Tests/Editor/WatchToolTests.cs index e4d364e645..5d9be4bbd4 100644 --- a/Assets/Tests/Editor/WatchToolTests.cs +++ b/Assets/Tests/Editor/WatchToolTests.cs @@ -30,7 +30,7 @@ public void SetUp() public void TearDown() { WatchExpressionServices.Registry.ClearAll(); - WatchExpressionServices.ResetStoreForTesting(); + WatchExpressionServices.ResetForTesting(); } /// @@ -77,6 +77,41 @@ public void EnableAsync_WhenValidationFails_DoesNotTouchThePersistedRecords() 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. /// @@ -194,6 +229,15 @@ public void Save(IReadOnlyList 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; diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs index beb2f22adb..40317d4c28 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs @@ -45,7 +45,29 @@ public async Task RestoreAsync(CancellationToken ct) // the restored order has to match the order the user registered them in. foreach (WatchPersistedRecord record in records) { - if (await TryRestoreAsync(record, warnings, ct)) + if (ct.IsCancellationRequested) + { + return CancelledReport(restoredCount); + } + + bool restored; + try + { + restored = await TryRestoreAsync(record, warnings, ct); + } + 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++; } @@ -62,20 +84,43 @@ public async Task RestoreAsync(CancellationToken ct) return new WatchRestoreReport(restoredCount, 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, List warnings, CancellationToken ct) { - WatchCompilationResult compiled = await _compiler.CompileAsync(record.Expression, ct); + WatchCompilationResult compiled; + try + { + compiled = await _compiler.CompileAsync(record.Expression, ct); + } + catch (OperationCanceledException) + { + // Cancellation ends the whole restore, so it must not be turned into a warning here. + throw; + } + catch (Exception exception) + { + // The compiler propagates its own failures; one bad expression must not stop the + // remaining watches from coming back. + AddDroppedWarning(warnings, record, exception.Message); + 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) { - warnings.Add( - $"Watch '{record.Id}' was not restored after the domain reload because its " - + $"expression no longer compiles: {DescribeCompilationFailure(compiled)}"); + AddDroppedWarning(warnings, record, DescribeCompilationFailure(compiled)); return false; } @@ -89,6 +134,33 @@ private async Task TryRestoreAsync( 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) diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs index 617e43e32d..262aa90a77 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs @@ -273,6 +273,9 @@ 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 @@ -305,7 +308,11 @@ 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; } } @@ -395,13 +402,15 @@ 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 IWatchExpressionCompiler Compiler => CompilerValue; + public static IWatchExpressionCompiler Compiler => _compiler; public static IWatchPersistenceStore Store => _store; /// @@ -421,19 +430,34 @@ public static void EnsureMonitorStarted() /// public static void RestoreAfterDomainReload() { - _ = RestoreAfterDomainReloadAsync(); + _restoreCancellation = new CancellationTokenSource(); + _ = RestoreAfterDomainReloadAsync(_restoreCancellation.Token); } - private static async Task RestoreAfterDomainReloadAsync() + /// + /// 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, - CompilerValue, + _compiler, _store, EnsureMonitorStarted); - LastRestoreReport = await restoreService.RestoreAsync(CancellationToken.None); + LastRestoreReport = await restoreService.RestoreAsync(ct); + } + catch (OperationCanceledException) + { + // A cancelled restore is a clear the user asked for, not a failure. } catch (Exception exception) { @@ -472,10 +496,17 @@ internal static void OverrideStoreForTesting(IWatchPersistenceStore store) _store = store ?? throw new ArgumentNullException(nameof(store)); } - internal static void ResetStoreForTesting() + 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) From 015e95422abb7ca9fd34a51053eceec4416d0cee Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 8 Sep 2026 09:45:19 +0900 Subject: [PATCH 3/5] test: cover cancellation that lands after a watch record compiled The restore already refuses to register a record when the clear cancels it between CompileAsync and the registry call: SwitchToMainThread's awaiter throws on the cancellation token in both IsCompleted and GetResult. Nothing proved that, so dropping the token from the switch would have gone unnoticed. Claude-Session: https://claude.ai/code/session_019kkGy4ujdjChrkMcimjKfY --- .../Tests/Editor/WatchRestoreServiceTests.cs | 48 ++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/Editor/WatchRestoreServiceTests.cs b/Assets/Tests/Editor/WatchRestoreServiceTests.cs index 3bf1ebadc4..b1fe6fbb45 100644 --- a/Assets/Tests/Editor/WatchRestoreServiceTests.cs +++ b/Assets/Tests/Editor/WatchRestoreServiceTests.cs @@ -173,6 +173,30 @@ public IEnumerator RestoreAsync_WhenCancelledWhileCompiling_LeavesTheStoreToTheC 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. /// @@ -268,6 +292,8 @@ private sealed class FakeWatchExpressionCompiler : IWatchExpressionCompiler 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) @@ -286,6 +312,13 @@ 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++; @@ -302,8 +335,21 @@ public Task CompileAsync(string expression, Cancellation new List { new() { Line = 1, Column = 1, Message = errorMessage, ErrorCode = "CS0103" } })); } - return Task.FromResult( + 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); } } From eeb355be57beb896773fc9ea52192325b3e0f871 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 8 Sep 2026 09:55:03 +0900 Subject: [PATCH 4/5] fix: keep a watch whose compile threw in the persisted store A compilation result failure means the user's expression is invalid, so dropping it is right. An exception from the compiler is our failure, and erasing the user's watch over it loses work that would come back on the next attempt. Retain those records so the next domain reload retries them. Also states the Domain Reload condition on the Play entry in the glossary. Claude-Session: https://claude.ai/code/session_019kkGy4ujdjChrkMcimjKfY --- .../Tests/Editor/WatchRestoreServiceTests.cs | 9 +++- .../Watch/WatchRestoreService.cs | 50 +++++++++++++++---- docs/glossary.md | 6 +-- 3 files changed, 49 insertions(+), 16 deletions(-) diff --git a/Assets/Tests/Editor/WatchRestoreServiceTests.cs b/Assets/Tests/Editor/WatchRestoreServiceTests.cs index b1fe6fbb45..97dfedb89d 100644 --- a/Assets/Tests/Editor/WatchRestoreServiceTests.cs +++ b/Assets/Tests/Editor/WatchRestoreServiceTests.cs @@ -103,7 +103,9 @@ public IEnumerator RestoreAsync_WhenTheIdWasRegisteredMeanwhile_KeepsTheExisting } /// - /// What: a compile that throws becomes that record's warning and the rest still restore. + /// 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() @@ -125,7 +127,10 @@ public IEnumerator RestoreAsync_WhenTheCompilerThrows_ReportsThatRecordAndKeepsG 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" })); + 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."); } /// diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs index 40317d4c28..4372d4b09a 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs @@ -39,7 +39,7 @@ public async Task RestoreAsync(CancellationToken ct) return WatchRestoreReport.Empty; } - List warnings = new(); + 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. @@ -53,7 +53,7 @@ public async Task RestoreAsync(CancellationToken ct) bool restored; try { - restored = await TryRestoreAsync(record, warnings, ct); + restored = await TryRestoreAsync(record, pass, ct); } catch (OperationCanceledException) { @@ -80,8 +80,8 @@ public async Task RestoreAsync(CancellationToken ct) // 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()); - return new WatchRestoreReport(restoredCount, warnings); + _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, @@ -94,7 +94,7 @@ private static WatchRestoreReport CancelledReport(int restoredCount) private async Task TryRestoreAsync( WatchPersistedRecord record, - List warnings, + RestorePass pass, CancellationToken ct) { WatchCompilationResult compiled; @@ -109,9 +109,11 @@ private async Task TryRestoreAsync( } catch (Exception exception) { - // The compiler propagates its own failures; one bad expression must not stop the - // remaining watches from coming back. - AddDroppedWarning(warnings, record, exception.Message); + // 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; } @@ -120,7 +122,9 @@ private async Task TryRestoreAsync( await MainThreadSwitcher.SwitchToMainThread(ct); if (!compiled.Success) { - AddDroppedWarning(warnings, record, DescribeCompilationFailure(compiled)); + // 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; } @@ -171,10 +175,11 @@ private static string DescribeCompilationFailure(WatchCompilationResult compiled return compiled.ErrorMessage; } - private IReadOnlyList SnapshotRegistry() + private IReadOnlyList SnapshotRegistry( + IReadOnlyList retainForRetry) { IReadOnlyList entries = _registry.GetEntries(); - List records = new(entries.Count); + List records = new(entries.Count + retainForRetry.Count); foreach (WatchExpressionEntry entry in entries) { records.Add(new WatchPersistedRecord @@ -185,7 +190,30 @@ private IReadOnlyList SnapshotRegistry() }); } + 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/docs/glossary.md b/docs/glossary.md index c451b33a93..148bed74a1 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -161,9 +161,9 @@ clipped any evidence. 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 causes; they are lost when the Editor -process exits. Value history does not cross a reload: a restored watch starts from a new -baseline. +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 From b670d63d1033fb6e7b01941d55ef6a129b4e61fb Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 8 Sep 2026 10:00:04 +0900 Subject: [PATCH 5/5] fix: configure the watch restore awaits and return to the main thread The repository ConfigureAwait guard requires every Task await under the first-party tools to opt out of the captured context. Those awaits can then resume off-thread, so the restore hops back to the main thread before touching the monitor and the SessionState-backed store. Claude-Session: https://claude.ai/code/session_019kkGy4ujdjChrkMcimjKfY --- .../Editor/FirstPartyTools/Watch/WatchRestoreService.cs | 7 +++++-- Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs index 4372d4b09a..992f536d4f 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchRestoreService.cs @@ -53,7 +53,7 @@ public async Task RestoreAsync(CancellationToken ct) bool restored; try { - restored = await TryRestoreAsync(record, pass, ct); + restored = await TryRestoreAsync(record, pass, ct).ConfigureAwait(false); } catch (OperationCanceledException) { @@ -73,6 +73,9 @@ public async Task RestoreAsync(CancellationToken ct) } } + // 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(); @@ -100,7 +103,7 @@ private async Task TryRestoreAsync( WatchCompilationResult compiled; try { - compiled = await _compiler.CompileAsync(record.Expression, ct); + compiled = await _compiler.CompileAsync(record.Expression, ct).ConfigureAwait(false); } catch (OperationCanceledException) { diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs index 262aa90a77..476d6b2056 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs @@ -453,7 +453,7 @@ private static async Task RestoreAfterDomainReloadAsync(CancellationToken ct) _compiler, _store, EnsureMonitorStarted); - LastRestoreReport = await restoreService.RestoreAsync(ct); + LastRestoreReport = await restoreService.RestoreAsync(ct).ConfigureAwait(false); } catch (OperationCanceledException) {