diff --git a/.agents/skills/uloop-pause-point/references/captured-variables.md b/.agents/skills/uloop-pause-point/references/captured-variables.md index e6ebb1d815..a03c0094b7 100644 --- a/.agents/skills/uloop-pause-point/references/captured-variables.md +++ b/.agents/skills/uloop-pause-point/references/captured-variables.md @@ -7,7 +7,7 @@ Read this before interpreting unexpected, missing, or truncated captured values, - The snapshot is taken **before** the resolved line executes, exactly like an IDE breakpoint on that line. To inspect a value after an assignment, place the pause point on the following line. - The pause itself only takes effect at the next frame boundary: the frame that hit the pause point still runs to completion first, so any event that fires later in that same frame (a chained collision, a cascading destroy) has already happened by the time Unity actually stops. Trust `CapturedVariables` (the pre-line snapshot) as evidence for what was true up to the patched line; do not assume the paused state still matches it for events later in the same frame. - Rigidbody values read inside a physics callback (`OnCollision*`/`OnTrigger*`) can be mid-solver intermediates — `velocity` may capture as `(0.0, 0.0)` at the callback even though the body visibly moves. `CapturedVariables` faithfully records that intermediate value; a later `execute-dynamic-code` read returning something different means the physics solver has since finished the step, not that the capture was wrong. -- `execute-dynamic-code` during the pause sees the interrupted method's **post-interrupt** state, not this pre-line snapshot. Use `CapturedVariables` for pre-line evidence; use the raw capture API below when you need live references while paused. If you suspect a captured value is stale or wrong, cross-check it against the live scene object with `execute-dynamic-code` (for example reading `transform.position` off the instance found via `UnityObjectPath`) rather than trusting either source alone. `execute-dynamic-code` responses also carry `EditorPaused` and `ActivePausePointId` — these fields appear only while the Editor is paused, so a call made while a pause point still has Unity paused is unambiguous instead of looking like a stale or buggy result. +- `execute-dynamic-code` during the pause sees the interrupted method's **post-interrupt** state, not this pre-line snapshot. Use `CapturedVariables` for pre-line evidence; use the raw capture API below when you need live references while paused. If you suspect a captured value is stale or wrong, cross-check it against the live scene object with `execute-dynamic-code` (for example reading `transform.position` off the instance found via `UnityObjectPath`) rather than trusting either source alone. `execute-dynamic-code` responses also carry `EditorPaused` and `ActivePausePointId` — these fields appear only while the Editor is paused, so a call made while a pause point still has Unity paused is unambiguous instead of looking like a stale or buggy result. `EditorPlaying` is always present and reports whether Play Mode is running, so a stopped versus playing Editor is visible even when `EditorPaused` is omitted. ## Scopes and the `this` Entry diff --git a/.claude/skills/uloop-pause-point/references/captured-variables.md b/.claude/skills/uloop-pause-point/references/captured-variables.md index e6ebb1d815..a03c0094b7 100644 --- a/.claude/skills/uloop-pause-point/references/captured-variables.md +++ b/.claude/skills/uloop-pause-point/references/captured-variables.md @@ -7,7 +7,7 @@ Read this before interpreting unexpected, missing, or truncated captured values, - The snapshot is taken **before** the resolved line executes, exactly like an IDE breakpoint on that line. To inspect a value after an assignment, place the pause point on the following line. - The pause itself only takes effect at the next frame boundary: the frame that hit the pause point still runs to completion first, so any event that fires later in that same frame (a chained collision, a cascading destroy) has already happened by the time Unity actually stops. Trust `CapturedVariables` (the pre-line snapshot) as evidence for what was true up to the patched line; do not assume the paused state still matches it for events later in the same frame. - Rigidbody values read inside a physics callback (`OnCollision*`/`OnTrigger*`) can be mid-solver intermediates — `velocity` may capture as `(0.0, 0.0)` at the callback even though the body visibly moves. `CapturedVariables` faithfully records that intermediate value; a later `execute-dynamic-code` read returning something different means the physics solver has since finished the step, not that the capture was wrong. -- `execute-dynamic-code` during the pause sees the interrupted method's **post-interrupt** state, not this pre-line snapshot. Use `CapturedVariables` for pre-line evidence; use the raw capture API below when you need live references while paused. If you suspect a captured value is stale or wrong, cross-check it against the live scene object with `execute-dynamic-code` (for example reading `transform.position` off the instance found via `UnityObjectPath`) rather than trusting either source alone. `execute-dynamic-code` responses also carry `EditorPaused` and `ActivePausePointId` — these fields appear only while the Editor is paused, so a call made while a pause point still has Unity paused is unambiguous instead of looking like a stale or buggy result. +- `execute-dynamic-code` during the pause sees the interrupted method's **post-interrupt** state, not this pre-line snapshot. Use `CapturedVariables` for pre-line evidence; use the raw capture API below when you need live references while paused. If you suspect a captured value is stale or wrong, cross-check it against the live scene object with `execute-dynamic-code` (for example reading `transform.position` off the instance found via `UnityObjectPath`) rather than trusting either source alone. `execute-dynamic-code` responses also carry `EditorPaused` and `ActivePausePointId` — these fields appear only while the Editor is paused, so a call made while a pause point still has Unity paused is unambiguous instead of looking like a stale or buggy result. `EditorPlaying` is always present and reports whether Play Mode is running, so a stopped versus playing Editor is visible even when `EditorPaused` is omitted. ## Scopes and the `this` Entry diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeErrorEditorStateTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeErrorEditorStateTests.cs new file mode 100644 index 0000000000..f2232adaa2 --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeErrorEditorStateTests.cs @@ -0,0 +1,252 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; +using io.github.hatayama.UnityCliLoop.Infrastructure; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.DynamicCodeToolTests +{ + /// + /// Verifies error responses serialize EditorPlaying from the injected editor-state reader. + /// + [TestFixture] + public sealed class ExecuteDynamicCodeErrorEditorStateTests + { + [SetUp] + public void SetUp() + { + DynamicCodeForegroundWarmupState.Reset(); + } + + /// + /// What: a compile-failure UseCase response JSON reports injected EditorPlaying=true. + /// + [Test] + public async Task ExecuteAsync_WhenCompilationFails_SerializesInjectedEditorPlaying() + { + MarkForegroundWarmupCompleted(); + FakeDynamicCodeExecutionRuntime runtime = new( + new ExecutionResult + { + Success = false, + ErrorMessage = "Compilation error occurred", + CompilationErrors = new List + { + new CompilationError + { + ErrorCode = "CS0103", + Message = "The name 'Nope' does not exist in the current context" + } + } + }); + ExecuteDynamicCodeUseCase useCase = CreateUseCase(runtime); + + ExecuteDynamicCodeResponse response = await useCase.ExecuteAsync( + new ExecuteDynamicCodeSchema + { + Code = "return Nope;", + CompileOnly = true + }, + CancellationToken.None); + + Assert.That(response.Success, Is.False); + Assert.That(SerializedEditorState(response), Is.EqualTo("{\"EditorPlaying\":true}")); + } + + /// + /// What: a cancelled-result UseCase response JSON reports injected EditorPlaying=true. + /// + [Test] + public async Task ExecuteAsync_WhenResultIsCancelled_SerializesInjectedEditorPlaying() + { + MarkForegroundWarmupCompleted(); + FakeDynamicCodeExecutionRuntime runtime = new( + new ExecutionResult + { + Success = false, + ErrorMessage = UnityCliLoopConstants.ERROR_MESSAGE_EXECUTION_CANCELLED + }); + ExecuteDynamicCodeUseCase useCase = CreateUseCase(runtime); + + ExecuteDynamicCodeResponse response = await useCase.ExecuteAsync( + new ExecuteDynamicCodeSchema { Code = "return 1;" }, + CancellationToken.None); + + Assert.That(response.Success, Is.False); + Assert.That(SerializedEditorState(response), Is.EqualTo("{\"EditorPlaying\":true}")); + } + + /// + /// What: a runtime-restarting UseCase response JSON reports injected EditorPlaying=true. + /// + [Test] + public async Task ExecuteAsync_WhenResultIsRuntimeRestarting_SerializesInjectedEditorPlaying() + { + MarkForegroundWarmupCompleted(); + FakeDynamicCodeExecutionRuntime runtime = new( + DynamicCodeExecutionResponseFactory.CreateRuntimeRestartingExecutionResult()); + ExecuteDynamicCodeUseCase useCase = CreateUseCase(runtime); + + ExecuteDynamicCodeResponse response = await useCase.ExecuteAsync( + new ExecuteDynamicCodeSchema { Code = "return 1;" }, + CancellationToken.None); + + Assert.That(response.Success, Is.False); + Assert.That(SerializedEditorState(response), Is.EqualTo("{\"EditorPlaying\":true}")); + } + + /// + /// What: the OperationCanceledException catch path JSON reports injected EditorPlaying=true. + /// + [Test] + public async Task ExecuteAsync_WhenRuntimeThrowsOperationCanceled_SerializesInjectedEditorPlaying() + { + MarkForegroundWarmupCompleted(); + ExecuteDynamicCodeUseCase useCase = CreateUseCase(new OperationCanceledDynamicCodeExecutionRuntime()); + + ExecuteDynamicCodeResponse response = await useCase.ExecuteAsync( + new ExecuteDynamicCodeSchema { Code = "return 1;" }, + CancellationToken.None); + + Assert.That(response.Success, Is.False); + Assert.That(SerializedEditorState(response), Is.EqualTo("{\"EditorPlaying\":true}")); + } + + /// + /// What: EditorPlaying=false still appears in JSON instead of being omitted. + /// + [Test] + public void ExecuteDynamicCodeResponse_WhenNotPlaying_StillSerializesEditorPlayingFalse() + { + ExecuteDynamicCodeResponse response = new() + { + Success = false, + EditorPlaying = false + }; + + Assert.That(SerializedEditorState(response), Is.EqualTo("{\"EditorPlaying\":false}")); + } + + /// + /// What: a paused response still serializes EditorPaused and ActivePausePointId next to EditorPlaying. + /// + [Test] + public void ExecuteDynamicCodeResponse_WhenPaused_SerializesPlayingPausedAndActiveId() + { + ExecuteDynamicCodeResponse response = new() + { + Success = false, + EditorPlaying = true, + EditorPaused = true, + ActivePausePointId = "jump" + }; + + Assert.That( + SerializedEditorState(response), + Is.EqualTo("{\"EditorPlaying\":true,\"EditorPaused\":true,\"ActivePausePointId\":\"jump\"}")); + } + + private static ExecuteDynamicCodeUseCase CreateUseCase(IDynamicCodeExecutionRuntime runtime) + { + return new ExecuteDynamicCodeUseCase( + runtime, + new FakeDynamicCodeEditorStateReader(isPlaying: true, isPaused: false)); + } + + private static string SerializedEditorState(ExecuteDynamicCodeResponse response) + { + JObject serialized = JObject.Parse( + JsonConvert.SerializeObject(response, JsonRpcResponseSerializer.Settings)); + JObject state = new JObject(); + if (serialized["EditorPlaying"] != null) + { + state["EditorPlaying"] = serialized["EditorPlaying"]; + } + + if (serialized["EditorPaused"] != null) + { + state["EditorPaused"] = serialized["EditorPaused"]; + } + + if (serialized["ActivePausePointId"] != null) + { + state["ActivePausePointId"] = serialized["ActivePausePointId"]; + } + + return state.ToString(Formatting.None); + } + + private static void MarkForegroundWarmupCompleted() + { + bool started = DynamicCodeForegroundWarmupState.TryBegin(); + if (!started) + { + return; + } + + DynamicCodeForegroundWarmupState.MarkCompleted(); + } + + private sealed class FakeDynamicCodeEditorStateReader : IDynamicCodeEditorStateReader + { + public FakeDynamicCodeEditorStateReader(bool isPlaying, bool isPaused) + { + IsPlaying = isPlaying; + IsPaused = isPaused; + } + + public bool IsPlaying { get; } + + public bool IsPaused { get; } + } + + private sealed class FakeDynamicCodeExecutionRuntime : IDynamicCodeExecutionRuntime + { + private readonly Queue _results; + + public FakeDynamicCodeExecutionRuntime(params ExecutionResult[] results) + { + _results = new Queue(results); + } + + public Task ExecuteAsync( + DynamicCodeExecutionRequest request, + CancellationToken cancellationToken = default) + { + return Task.FromResult(_results.Dequeue()); + } + + public Task<(bool Entered, ExecutionResult Result)> TryExecuteIfIdleAsync( + DynamicCodeExecutionRequest request, + CancellationToken cancellationToken = default) + { + return Task.FromResult<(bool, ExecutionResult)>((true, _results.Dequeue())); + } + } + + private sealed class OperationCanceledDynamicCodeExecutionRuntime : IDynamicCodeExecutionRuntime + { + public Task ExecuteAsync( + DynamicCodeExecutionRequest request, + CancellationToken cancellationToken = default) + { + throw new OperationCanceledException(); + } + + public Task<(bool Entered, ExecutionResult Result)> TryExecuteIfIdleAsync( + DynamicCodeExecutionRequest request, + CancellationToken cancellationToken = default) + { + throw new OperationCanceledException(); + } + } + } +} diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeErrorEditorStateTests.cs.meta b/Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeErrorEditorStateTests.cs.meta new file mode 100644 index 0000000000..fb53c7933d --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/ExecuteDynamicCodeErrorEditorStateTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a8ca3e16960e8489f89afb9d9bcab88e +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md index e6ebb1d815..a03c0094b7 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md @@ -7,7 +7,7 @@ Read this before interpreting unexpected, missing, or truncated captured values, - The snapshot is taken **before** the resolved line executes, exactly like an IDE breakpoint on that line. To inspect a value after an assignment, place the pause point on the following line. - The pause itself only takes effect at the next frame boundary: the frame that hit the pause point still runs to completion first, so any event that fires later in that same frame (a chained collision, a cascading destroy) has already happened by the time Unity actually stops. Trust `CapturedVariables` (the pre-line snapshot) as evidence for what was true up to the patched line; do not assume the paused state still matches it for events later in the same frame. - Rigidbody values read inside a physics callback (`OnCollision*`/`OnTrigger*`) can be mid-solver intermediates — `velocity` may capture as `(0.0, 0.0)` at the callback even though the body visibly moves. `CapturedVariables` faithfully records that intermediate value; a later `execute-dynamic-code` read returning something different means the physics solver has since finished the step, not that the capture was wrong. -- `execute-dynamic-code` during the pause sees the interrupted method's **post-interrupt** state, not this pre-line snapshot. Use `CapturedVariables` for pre-line evidence; use the raw capture API below when you need live references while paused. If you suspect a captured value is stale or wrong, cross-check it against the live scene object with `execute-dynamic-code` (for example reading `transform.position` off the instance found via `UnityObjectPath`) rather than trusting either source alone. `execute-dynamic-code` responses also carry `EditorPaused` and `ActivePausePointId` — these fields appear only while the Editor is paused, so a call made while a pause point still has Unity paused is unambiguous instead of looking like a stale or buggy result. +- `execute-dynamic-code` during the pause sees the interrupted method's **post-interrupt** state, not this pre-line snapshot. Use `CapturedVariables` for pre-line evidence; use the raw capture API below when you need live references while paused. If you suspect a captured value is stale or wrong, cross-check it against the live scene object with `execute-dynamic-code` (for example reading `transform.position` off the instance found via `UnityObjectPath`) rather than trusting either source alone. `execute-dynamic-code` responses also carry `EditorPaused` and `ActivePausePointId` — these fields appear only while the Editor is paused, so a call made while a pause point still has Unity paused is unambiguous instead of looking like a stale or buggy result. `EditorPlaying` is always present and reports whether Play Mode is running, so a stopped versus playing Editor is visible even when `EditorPaused` is omitted. ## Scopes and the `this` Entry diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeEditorStateReader.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeEditorStateReader.cs new file mode 100644 index 0000000000..f24326b9ef --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeEditorStateReader.cs @@ -0,0 +1,23 @@ +using UnityEditor; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Reads Editor Play/Pause flags for execute-dynamic-code pause-state application. + /// + internal interface IDynamicCodeEditorStateReader + { + bool IsPlaying { get; } + bool IsPaused { get; } + } + + /// + /// Forwards Play/Pause reads to EditorApplication. + /// + internal sealed class DynamicCodeEditorStateReader : IDynamicCodeEditorStateReader + { + public bool IsPlaying => EditorApplication.isPlaying; + + public bool IsPaused => EditorApplication.isPaused; + } +} diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeEditorStateReader.cs.meta b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeEditorStateReader.cs.meta new file mode 100644 index 0000000000..03b867c6a6 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeEditorStateReader.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 0d6e4b4676c50403295154f800bd8fa6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeResponse.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeResponse.cs index 78d5e46200..497106d216 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeResponse.cs @@ -62,6 +62,12 @@ public string Error /// public string[] NextActions { get; set; } + /// + /// Whether Play Mode is running as this response is returned. Always serialized so a + /// stopped versus playing Editor is visible even when EditorPaused is omitted. + /// + public bool EditorPlaying { get; set; } + /// /// Whether the Editor is paused as this response is returned. Lets an agent recognize a /// post-interrupt state (e.g. a pause point hit during this execution) instead of mistaking diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs index 44a12287ae..8bd811f9f2 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/ExecuteDynamicCodeUseCase.cs @@ -4,8 +4,6 @@ using System.Threading; using System.Threading.Tasks; -using UnityEditor; - using io.github.hatayama.UnityCliLoop.Runtime; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -19,11 +17,15 @@ internal sealed class ExecuteDynamicCodeUseCase : IExecuteDynamicCodeUseCase { private readonly IDynamicCodeExecutionRuntime _runtime; private readonly DynamicCodeExecutionResponseFactory _responseFactory; + private readonly IDynamicCodeEditorStateReader _editorStateReader; - public ExecuteDynamicCodeUseCase(IDynamicCodeExecutionRuntime runtime) + public ExecuteDynamicCodeUseCase( + IDynamicCodeExecutionRuntime runtime, + IDynamicCodeEditorStateReader editorStateReader = null) { _runtime = runtime ?? throw new ArgumentNullException(nameof(runtime)); _responseFactory = new DynamicCodeExecutionResponseFactory(); + _editorStateReader = editorStateReader ?? new DynamicCodeEditorStateReader(); } public async Task ExecuteAsync( @@ -132,10 +134,12 @@ await WarmForegroundExecutionPathIfNeededAsync(parameters, cancellationToken) // Lets an agent recognize a post-interrupt state (e.g. a pause point hit mid-execution) // instead of mistaking a stale-looking result for a bug. ActivePausePointId stays empty - // when the Editor is paused for a reason unrelated to a pause point. + // when the Editor is paused for a reason unrelated to a pause point. EditorPlaying is + // always written so a stopped versus playing Editor is visible after ShouldSerialize + // omits EditorPaused=false. // Why async: the preceding ConfigureAwait(false) continuations may resume this method on a - // thread-pool thread, and EditorApplication.isPaused throws when read off the main thread. - private static async Task ApplyPauseStateAsync( + // thread-pool thread, and the default reader reads EditorApplication off the main thread. + private async Task ApplyPauseStateAsync( ExecuteDynamicCodeResponse response, CancellationToken cancellationToken) { @@ -144,8 +148,9 @@ private static async Task ApplyPauseStateAsync( // MainThreadSwitcher itself, so the code below always runs there regardless. await MainThreadSwitcher.SwitchToMainThread(cancellationToken); + response.EditorPlaying = _editorStateReader.IsPlaying; (bool editorPaused, string activePausePointId) = ExecuteDynamicCodePauseStateResolver.Resolve( - EditorApplication.isPaused, UloopPausePointRegistry.GetActivePausePointId()); + _editorStateReader.IsPaused, UloopPausePointRegistry.GetActivePausePointId()); response.EditorPaused = editorPaused; response.ActivePausePointId = activePausePointId; }