diff --git a/.agents/skills/uloop-pause-point/references/captured-variables.md b/.agents/skills/uloop-pause-point/references/captured-variables.md index 07fbd4b0cd..dfdeed0da2 100644 --- a/.agents/skills/uloop-pause-point/references/captured-variables.md +++ b/.agents/skills/uloop-pause-point/references/captured-variables.md @@ -18,6 +18,7 @@ Read this before interpreting unexpected, missing, or truncated captured values, ## Scopes and the `this` Entry - `Scope` is `Local`, `Parameter`, `InstanceField`, or `This`. `InstanceField` entries come from a reflection walk of the paused instance's declared type, not from the method's IL usage, so a field the method never reads can still appear — and `MaxCapturedVariableCount` still caps the total entry count across all scopes, so a field-heavy type can push some instance fields out of the snapshot. If a specific field you want is missing, read it directly from the live instance instead of waiting on the capped snapshot: while still paused, `UloopPausePoint.TryGetCapturedValue("this")` returns the live `this` reference, so `execute-dynamic-code` can read any field or property off it regardless of the cap. +- A `ref`, `out`, or `in` parameter, a pointer, and a `ref struct` value (`Span` and any user-defined `ref struct`) can never be captured, because the snapshot boxes every value and none of these shapes can be boxed. The enable and `pause-point-status` responses list each such parameter with its reason in `NotCapturableVariables` (absent when there are none), so a missing name is explained rather than looking like a capture bug; to observe one of those values, copy the value it refers to into a plain local (dereference a pointer, `ToArray()` a span), or arm the line that consumes it with `--snapshot-timing post-line`. - The snapshot also includes a synthetic `this` entry (Scope `This`) for the paused instance itself, so you can tell which instance or GameObject was hit via its `UnityObjectPath` and `UnityObjectInstanceId`. For an async or coroutine method it resolves to the original outer instance, not the compiler-generated state machine, and static methods emit no `this` entry. While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("this")` returns the live instance reference (for example so a watch expression can read `transform.position`). - async and coroutine methods work: hoisted locals and the original `this` fields appear under their normal names. - Auto-implemented properties are captured as instance fields under the property name (the compiler-generated backing field is un-mangled), so you do not need to rewrite them as explicit fields for verification. diff --git a/.claude/skills/uloop-pause-point/references/captured-variables.md b/.claude/skills/uloop-pause-point/references/captured-variables.md index 07fbd4b0cd..dfdeed0da2 100644 --- a/.claude/skills/uloop-pause-point/references/captured-variables.md +++ b/.claude/skills/uloop-pause-point/references/captured-variables.md @@ -18,6 +18,7 @@ Read this before interpreting unexpected, missing, or truncated captured values, ## Scopes and the `this` Entry - `Scope` is `Local`, `Parameter`, `InstanceField`, or `This`. `InstanceField` entries come from a reflection walk of the paused instance's declared type, not from the method's IL usage, so a field the method never reads can still appear — and `MaxCapturedVariableCount` still caps the total entry count across all scopes, so a field-heavy type can push some instance fields out of the snapshot. If a specific field you want is missing, read it directly from the live instance instead of waiting on the capped snapshot: while still paused, `UloopPausePoint.TryGetCapturedValue("this")` returns the live `this` reference, so `execute-dynamic-code` can read any field or property off it regardless of the cap. +- A `ref`, `out`, or `in` parameter, a pointer, and a `ref struct` value (`Span` and any user-defined `ref struct`) can never be captured, because the snapshot boxes every value and none of these shapes can be boxed. The enable and `pause-point-status` responses list each such parameter with its reason in `NotCapturableVariables` (absent when there are none), so a missing name is explained rather than looking like a capture bug; to observe one of those values, copy the value it refers to into a plain local (dereference a pointer, `ToArray()` a span), or arm the line that consumes it with `--snapshot-timing post-line`. - The snapshot also includes a synthetic `this` entry (Scope `This`) for the paused instance itself, so you can tell which instance or GameObject was hit via its `UnityObjectPath` and `UnityObjectInstanceId`. For an async or coroutine method it resolves to the original outer instance, not the compiler-generated state machine, and static methods emit no `this` entry. While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("this")` returns the live instance reference (for example so a watch expression can read `transform.position`). - async and coroutine methods work: hoisted locals and the original `this` fields appear under their normal names. - Auto-implemented properties are captured as instance fields under the property name (the compiler-generated backing field is un-mangled), so you do not need to rewrite them as explicit fields for verification. diff --git a/Assets/Tests/Editor/HotReload/HotReloadPausePointContractTests.cs b/Assets/Tests/Editor/HotReload/HotReloadPausePointContractTests.cs index d85b8fdf48..756ad3f1ad 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadPausePointContractTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadPausePointContractTests.cs @@ -904,7 +904,8 @@ private static SourcePausePointResolution BuildSyntheticResolution( compiledMethodStartLine, compiledMethodEndLine, Array.Empty(), - Array.Empty()); + Array.Empty(), + Array.Empty()); } private static string BuildEditedComputePlusHundred(string onDisk) diff --git a/Assets/Tests/Editor/PausePointExpiredRecommendedNextActionTests.cs b/Assets/Tests/Editor/PausePointExpiredRecommendedNextActionTests.cs index 7a9e665e9e..0106cbcd07 100644 --- a/Assets/Tests/Editor/PausePointExpiredRecommendedNextActionTests.cs +++ b/Assets/Tests/Editor/PausePointExpiredRecommendedNextActionTests.cs @@ -168,6 +168,7 @@ private static UloopPausePointSnapshot CreateExpiredSnapshot( false, Array.Empty(), 0, + Array.Empty(), string.Empty, string.Empty, false, diff --git a/Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs b/Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs new file mode 100644 index 0000000000..42adf0ad8a --- /dev/null +++ b/Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs @@ -0,0 +1,233 @@ +using System; +using System.Collections.Generic; + +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.Runtime; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies the enable-time warning and the registry-to-response path for parameters that + /// capture cannot box. + /// + [TestFixture] + public sealed class PausePointNotCapturableVariablesTests + { + private const string ByRefEntry = "accumulator (ref/out/in parameter cannot be boxed)"; + private const string RefStructEntry = "scratch (ref struct cannot be boxed)"; + + private const string ExpectedWarningForTwoEntries = + "Parameters not captured because they cannot be boxed: " + + "accumulator (ref/out/in parameter cannot be boxed), scratch (ref struct cannot be boxed). " + + "Copy the value it refers to into a plain local (dereference a pointer, ToArray() a span), " + + "or use --snapshot-timing post-line on the line that consumes it."; + + /// + /// What: a non-empty list produces the warning naming every entry with its reason. + /// + [Test] + public void BuildNotCapturableParametersWarningOrEmpty_WithEntries_NamesThemAndTheWorkaround() + { + string warning = PausePointNotCapturableWarnings.BuildNotCapturableParametersWarningOrEmpty( + new[] { ByRefEntry, RefStructEntry }); + + Assert.That(warning, Is.EqualTo(ExpectedWarningForTwoEntries)); + } + + /// + /// What: an empty list produces no warning, so a fully capturable method stays quiet. + /// + [Test] + public void BuildNotCapturableParametersWarningOrEmpty_WithEmptyList_ReturnsEmpty() + { + string warning = PausePointNotCapturableWarnings.BuildNotCapturableParametersWarningOrEmpty( + Array.Empty()); + + Assert.That(warning, Is.Empty); + } + + /// + /// What: a null list produces no warning instead of throwing. + /// + [Test] + public void BuildNotCapturableParametersWarningOrEmpty_WithNull_ReturnsEmpty() + { + string warning = PausePointNotCapturableWarnings.BuildNotCapturableParametersWarningOrEmpty(null); + + Assert.That(warning, Is.Empty); + } + + /// + /// What: SetNotCapturableVariables is visible on the next status snapshot. + /// + [Test] + public void SetNotCapturableVariables_WhenStored_AppearsInStatusSnapshot() + { + UloopPausePointRegistry.ConfigureForTests(new FakeNotCapturablePauseController(), () => DateTime.UtcNow); + try + { + const string id = "Assets/Scripts/Enemy.cs:42"; + UloopPausePointRegistry.Enable(id, 30); + UloopPausePointRegistry.SetNotCapturableVariables(id, new[] { ByRefEntry }); + + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(id); + + Assert.That(snapshot.NotCapturableVariables, Is.EqualTo(new[] { ByRefEntry })); + } + finally + { + UloopPausePointRegistry.ResetForTests(); + } + } + + /// + /// What: clearing with an empty list drops a previously stored exclusion list, so a + /// discarded resolution never leaves a stale list behind. + /// + [Test] + public void SetNotCapturableVariables_WhenClearedWithEmptyList_DropsPreviousEntries() + { + UloopPausePointRegistry.ConfigureForTests(new FakeNotCapturablePauseController(), () => DateTime.UtcNow); + try + { + const string id = "Assets/Scripts/Enemy.cs:42"; + UloopPausePointRegistry.Enable(id, 30); + UloopPausePointRegistry.SetNotCapturableVariables(id, new[] { ByRefEntry }); + UloopPausePointRegistry.SetNotCapturableVariables(id, Array.Empty()); + + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(id); + + Assert.That(snapshot.NotCapturableVariables, Is.Empty); + } + finally + { + UloopPausePointRegistry.ResetForTests(); + } + } + + /// + /// What: the status response carries the stored entries through FromSnapshot. + /// + [Test] + public void StatusResponseFromSnapshot_WithStoredEntries_CarriesThem() + { + UloopPausePointRegistry.ConfigureForTests(new FakeNotCapturablePauseController(), () => DateTime.UtcNow); + try + { + const string id = "Assets/Scripts/Enemy.cs:42"; + UloopPausePointRegistry.Enable(id, 30); + UloopPausePointRegistry.SetNotCapturableVariables(id, new[] { ByRefEntry }); + + PausePointStatusResponse response = + PausePointStatusResponse.FromSnapshot(UloopPausePointRegistry.GetStatus(id)); + + Assert.That(response.NotCapturableVariables, Is.EqualTo(new[] { ByRefEntry })); + } + finally + { + UloopPausePointRegistry.ResetForTests(); + } + } + + /// + /// What: with nothing to report the status response omits the field from its JSON, so the + /// shared contract shape stays unchanged for fully capturable methods. + /// + [Test] + public void StatusResponseFromSnapshot_WithNoEntries_OmitsFieldFromJson() + { + UloopPausePointRegistry.ConfigureForTests(new FakeNotCapturablePauseController(), () => DateTime.UtcNow); + try + { + const string id = "Assets/Scripts/Enemy.cs:42"; + UloopPausePointRegistry.Enable(id, 30); + + PausePointStatusResponse response = + PausePointStatusResponse.FromSnapshot(UloopPausePointRegistry.GetStatus(id)); + string json = JsonConvert.SerializeObject( + response, + Formatting.None, + UnityCliLoopJsonResponseSerializerSettings.Settings); + + Assert.That(JObject.Parse(json).ContainsKey("NotCapturableVariables"), Is.False); + } + finally + { + UloopPausePointRegistry.ResetForTests(); + } + } + + /// + /// What: the enable response carries the stored entries through FromSnapshot. + /// + [Test] + public void EnableResponseFromSnapshot_WithStoredEntries_CarriesThem() + { + UloopPausePointRegistry.ConfigureForTests(new FakeNotCapturablePauseController(), () => DateTime.UtcNow); + try + { + const string id = "Assets/Scripts/Enemy.cs:42"; + UloopPausePointRegistry.Enable(id, 30); + UloopPausePointRegistry.SetNotCapturableVariables(id, new[] { ByRefEntry, RefStructEntry }); + + PausePointResponse response = + PausePointResponse.FromSnapshot(UloopPausePointRegistry.GetStatus(id)); + + Assert.That( + response.NotCapturableVariables, + Is.EqualTo(new[] { ByRefEntry, RefStructEntry })); + } + finally + { + UloopPausePointRegistry.ResetForTests(); + } + } + + /// + /// What: with nothing to report the enable response omits the field from its JSON. + /// + [Test] + public void EnableResponseFromSnapshot_WithNoEntries_OmitsFieldFromJson() + { + UloopPausePointRegistry.ConfigureForTests(new FakeNotCapturablePauseController(), () => DateTime.UtcNow); + try + { + const string id = "Assets/Scripts/Enemy.cs:42"; + UloopPausePointRegistry.Enable(id, 30); + + PausePointResponse response = + PausePointResponse.FromSnapshot(UloopPausePointRegistry.GetStatus(id)); + string json = JsonConvert.SerializeObject( + response, + Formatting.None, + UnityCliLoopJsonResponseSerializerSettings.Settings); + + Assert.That(JObject.Parse(json).ContainsKey("NotCapturableVariables"), Is.False); + } + finally + { + UloopPausePointRegistry.ResetForTests(); + } + } + + private sealed class FakeNotCapturablePauseController : IUloopPausePointPauseController + { + public bool IsPlaying => true; + public bool IsPaused => false; + + public void Pause() + { + } + + public void Resume() + { + } + } + } +} diff --git a/Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs.meta b/Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs.meta new file mode 100644 index 0000000000..947c6e3924 --- /dev/null +++ b/Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4d336eb96c4cb4e81b16a708653fb723 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs b/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs index b57c9e6f7f..c05ed69c55 100644 --- a/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs +++ b/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs @@ -115,6 +115,7 @@ public void PausePointStatusResponse_WhenSerialized_MatchesSharedContractFieldSh CapturedVariablesTruncated = true, TruncatedVariableNames = new[] { "extraField" }, TruncatedVariableCount = 1, + NotCapturableVariables = new[] { "accumulator (ref/out/in parameter cannot be boxed)" }, ClearedReason = "", StatusBeforeClear = "", LateHitDiscardedAfterClear = false, diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index 99e3547588..fc0489036f 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -2222,7 +2222,8 @@ private static SourcePausePointResolution WithStaleMvid(SourcePausePointResoluti resolution.CompiledMethodStartLine, resolution.CompiledMethodEndLine, resolution.Locals, - resolution.Parameters); + resolution.Parameters, + resolution.NotCapturableVariables); } private static async Task EnablePausePointAsync(string id) diff --git a/Assets/Tests/Editor/PausePointWarningsChannelTests.cs b/Assets/Tests/Editor/PausePointWarningsChannelTests.cs index 3c45ff1f8b..c561e8413a 100644 --- a/Assets/Tests/Editor/PausePointWarningsChannelTests.cs +++ b/Assets/Tests/Editor/PausePointWarningsChannelTests.cs @@ -211,6 +211,7 @@ private static UloopPausePointSnapshot CreateSnapshot( false, Array.Empty(), 0, + Array.Empty(), string.Empty, string.Empty, false, diff --git a/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs b/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs new file mode 100644 index 0000000000..257211ebdf --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs @@ -0,0 +1,39 @@ +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + // Read back through Mono.Cecil from Library/ScriptAssemblies/UnityCLILoop.Tests.Editor.dll, + // so this fixture must stay in the UnityCLILoop.Tests.Editor assembly: a nested test asmdef + // compiles into a different dll that the Cecil test does not open. + internal sealed class SourcePausePointNotCapturableParameterFixture + { + // ref/out/in and ref-struct parameters exist here specifically to verify that capture + // reports them as not capturable instead of dropping them silently. + public int Combine( + int value, + ref int accumulator, + out int doubled, + in int multiplier, + System.Span scratch) + { + doubled = value * 2; + accumulator += doubled; + scratch[0] = accumulator; + return scratch[0] * multiplier; + } + + // The leading parameter is byref here specifically so skipFirstParameter is observable: + // skipping it changes the reported list, which a fixture with a capturable first + // parameter could never show. + public int CombineLeadingByRef(ref int accumulator, int value, in int multiplier) + { + accumulator += value; + return accumulator * multiplier; + } + + // The capturable-only counterpart, so the empty not-capturable list is asserted against + // a method that really has nothing to report. + public int Add(int left, int right) + { + return left + right; + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs.meta b/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs.meta new file mode 100644 index 0000000000..a93bd28874 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 6cfb89a2066244c11997400d38f67627 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs b/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs new file mode 100644 index 0000000000..1de2322582 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs @@ -0,0 +1,280 @@ +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Reflection; + +using Mono.Cecil; +using Mono.Cecil.Cil; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies that parameters excluded from capture are reported by name with the reason their + /// type cannot be boxed, against the real Roslyn IL emitted for the editor test assembly. + /// + [TestFixture] + public sealed class SourcePausePointNotCapturableParametersTests + { + private const string AssemblyRelativePath = "Library/ScriptAssemblies/UnityCLILoop.Tests.Editor.dll"; + private const string FixtureTypeName = + "io.github.hatayama.UnityCliLoop.Tests.Editor.SourcePausePointNotCapturableParameterFixture"; + + /// + /// What: the Cecil path names every ref/out/in and ref-struct parameter of the real + /// compiled fixture, in declaration order, with the reason each cannot be boxed. + /// + [Test] + public void CollectNotCapturableParameters_RealAssemblyByRefAndRefStruct_NamesEachWithReason() + { + using AssemblyDefinition assembly = ReadEditorTestAssembly(); + MethodDefinition method = FindFixtureMethod(assembly, "Combine"); + + List notCapturable = + SourcePausePointCaptureEligibility.CollectNotCapturableParameters(method); + + Assert.That(notCapturable, Is.EqualTo(ExpectedCombineNotCapturableNames())); + } + + /// + /// What: the Cecil not-capturable names stay disjoint from the captured parameters, so + /// the same parameter never appears in both lists. + /// + [Test] + public void CollectParameters_RealAssemblyByRefAndRefStruct_KeepsOnlyTheCapturableParameter() + { + using AssemblyDefinition assembly = ReadEditorTestAssembly(); + MethodDefinition method = FindFixtureMethod(assembly, "Combine"); + + List parameters = + SourcePausePointCaptureEligibility.CollectParameters(method); + + Assert.That(parameters.Select(parameter => parameter.Name), Is.EqualTo(new[] { "value" })); + } + + /// + /// What: a Cecil method whose parameters are all capturable reports an empty list. + /// + [Test] + public void CollectNotCapturableParameters_RealAssemblyCapturableParametersOnly_ReturnsEmpty() + { + using AssemblyDefinition assembly = ReadEditorTestAssembly(); + MethodDefinition method = FindFixtureMethod(assembly, "Add"); + + List notCapturable = + SourcePausePointCaptureEligibility.CollectNotCapturableParameters(method); + + Assert.That(notCapturable, Is.Empty); + } + + /// + /// What: the reflection path reports the same names and reasons as the Cecil path for the + /// same method, so shim-resolved markers do not disagree with compiled-resolved ones. + /// + [Test] + public void CollectNotCapturableParametersFromReflection_ByRefAndRefStruct_NamesEachWithReason() + { + MethodBase method = FindReflectionFixtureMethod("Combine"); + + List notCapturable = + SourcePausePointCaptureEligibility.CollectNotCapturableParametersFromReflection( + method, + skipFirstParameter: false); + + Assert.That(notCapturable, Is.EqualTo(ExpectedCombineNotCapturableNames())); + } + + /// + /// What: the reflection not-capturable names stay disjoint from the captured parameters. + /// + [Test] + public void CollectParametersFromReflection_ByRefAndRefStruct_KeepsOnlyTheCapturableParameter() + { + MethodBase method = FindReflectionFixtureMethod("Combine"); + + List parameters = + SourcePausePointCaptureEligibility.CollectParametersFromReflection( + method, + skipFirstParameter: false); + + Assert.That(parameters.Select(parameter => parameter.Name), Is.EqualTo(new[] { "value" })); + } + + /// + /// What: skipFirstParameter drops the leading delegation argument from the reflection + /// not-capturable list the same way it drops it from the captured list. + /// + [Test] + public void CollectNotCapturableParametersFromReflection_SkipFirstParameter_SkipsTheLeadingParameter() + { + MethodBase method = FindReflectionFixtureMethod("Combine"); + + List notCapturable = + SourcePausePointCaptureEligibility.CollectNotCapturableParametersFromReflection( + method, + skipFirstParameter: true); + + // "value" is the skipped leading parameter and was capturable anyway, so the list is + // unchanged; the assertion pins that skipping never shifts or renames the entries. + Assert.That(notCapturable, Is.EqualTo(ExpectedCombineNotCapturableNames())); + } + + /// + /// What: skipFirstParameter drops a leading byref parameter from the reflection + /// not-capturable list, so the skip is observable in the reported names. + /// + [Test] + public void CollectNotCapturableParametersFromReflection_SkipFirstParameter_DropsLeadingByRefParameter() + { + MethodBase method = FindReflectionFixtureMethod("CombineLeadingByRef"); + + List notCapturable = + SourcePausePointCaptureEligibility.CollectNotCapturableParametersFromReflection( + method, + skipFirstParameter: true); + + Assert.That( + notCapturable, + Is.EqualTo(new[] { "multiplier (" + SourcePausePointConstants.NotCapturableByRefParameterReason + ")" })); + } + + /// + /// What: without the skip, the same leading byref parameter is reported, which is what + /// makes the skipping assertion above meaningful. + /// + [Test] + public void CollectNotCapturableParametersFromReflection_WithoutSkip_KeepsLeadingByRefParameter() + { + MethodBase method = FindReflectionFixtureMethod("CombineLeadingByRef"); + + List notCapturable = + SourcePausePointCaptureEligibility.CollectNotCapturableParametersFromReflection( + method, + skipFirstParameter: false); + + Assert.That( + notCapturable, + Is.EqualTo( + new[] + { + "accumulator (" + SourcePausePointConstants.NotCapturableByRefParameterReason + ")", + "multiplier (" + SourcePausePointConstants.NotCapturableByRefParameterReason + ")" + })); + } + + /// + /// What: a reflection method whose parameters are all capturable reports an empty list. + /// + [Test] + public void CollectNotCapturableParametersFromReflection_CapturableParametersOnly_ReturnsEmpty() + { + MethodBase method = FindReflectionFixtureMethod("Add"); + + List notCapturable = + SourcePausePointCaptureEligibility.CollectNotCapturableParametersFromReflection( + method, + skipFirstParameter: false); + + Assert.That(notCapturable, Is.Empty); + } + + /// + /// What: a pointer type reports the pointer reason on the reflection path. Pointer + /// parameters cannot be declared in this assembly (unsafe code is off), so the reason + /// mapper is exercised directly. + /// + [Test] + public void DescribeNotCapturableReasonFromReflection_PointerType_ReturnsPointerReason() + { + string reason = SourcePausePointCaptureEligibility.DescribeNotCapturableReasonFromReflection( + typeof(int).MakePointerType()); + + Assert.That(reason, Is.EqualTo(SourcePausePointConstants.NotCapturablePointerReason)); + } + + /// + /// What: a capturable type reports no reason on the reflection path. + /// + [Test] + public void DescribeNotCapturableReasonFromReflection_CapturableType_ReturnsEmpty() + { + string reason = SourcePausePointCaptureEligibility.DescribeNotCapturableReasonFromReflection(typeof(int)); + + Assert.That(reason, Is.Empty); + } + + /// + /// What: a Cecil pointer type reports the pointer reason, covering the branch the + /// compiled fixture cannot reach without unsafe code. + /// + [Test] + public void DescribeNotCapturableReason_CecilPointerType_ReturnsPointerReason() + { + using AssemblyDefinition assembly = ReadEditorTestAssembly(); + TypeReference pointerType = new PointerType(assembly.MainModule.TypeSystem.Int32); + + string reason = SourcePausePointCaptureEligibility.DescribeNotCapturableReason(pointerType); + + Assert.That(reason, Is.EqualTo(SourcePausePointConstants.NotCapturablePointerReason)); + } + + /// + /// What: a capturable Cecil type reports no reason. + /// + [Test] + public void DescribeNotCapturableReason_CecilCapturableType_ReturnsEmpty() + { + using AssemblyDefinition assembly = ReadEditorTestAssembly(); + + string reason = SourcePausePointCaptureEligibility.DescribeNotCapturableReason( + assembly.MainModule.TypeSystem.Int32); + + Assert.That(reason, Is.Empty); + } + + private static string[] ExpectedCombineNotCapturableNames() + { + return new[] + { + "accumulator (" + SourcePausePointConstants.NotCapturableByRefParameterReason + ")", + "doubled (" + SourcePausePointConstants.NotCapturableByRefParameterReason + ")", + "multiplier (" + SourcePausePointConstants.NotCapturableByRefParameterReason + ")", + "scratch (" + SourcePausePointConstants.NotCapturableRefStructReason + ")" + }; + } + + private static MethodBase FindReflectionFixtureMethod(string methodName) + { + MethodInfo method = typeof(SourcePausePointNotCapturableParameterFixture).GetMethod(methodName); + Debug.Assert(method != null, "The fixture must declare the requested method."); + return method; + } + + private static AssemblyDefinition ReadEditorTestAssembly() + { + // Unity's working directory is not guaranteed to be the project root, so anchor the + // path on Assets instead of the process cwd. + string projectRoot = Path.GetDirectoryName(UnityEngine.Application.dataPath); + string assemblyPath = Path.Combine(projectRoot, AssemblyRelativePath); + ReaderParameters readerParameters = new ReaderParameters + { + InMemory = true, + ReadSymbols = true, + SymbolReaderProvider = new PortablePdbReaderProvider() + }; + + return AssemblyDefinition.ReadAssembly(assemblyPath, readerParameters); + } + + private static MethodDefinition FindFixtureMethod(AssemblyDefinition assembly, string methodName) + { + TypeDefinition fixture = assembly.MainModule.GetType(FixtureTypeName); + Debug.Assert(fixture != null, "The compiled fixture type must exist."); + return fixture.Methods.Single(method => method.Name == methodName); + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs.meta b/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs.meta new file mode 100644 index 0000000000..dd0b51e388 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ef80207b8995442038d44a77a11e8a37 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs index f11e71de65..465a989813 100644 --- a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs +++ b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs @@ -823,7 +823,8 @@ private static SourcePausePointResolution BuildSyntheticResolutionWithMvid( compiledMethodStartLine, compiledMethodEndLine, Array.Empty(), - Array.Empty()); + Array.Empty(), + Array.Empty()); } private abstract class AbstractMethodFixture 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 07fbd4b0cd..dfdeed0da2 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md @@ -18,6 +18,7 @@ Read this before interpreting unexpected, missing, or truncated captured values, ## Scopes and the `this` Entry - `Scope` is `Local`, `Parameter`, `InstanceField`, or `This`. `InstanceField` entries come from a reflection walk of the paused instance's declared type, not from the method's IL usage, so a field the method never reads can still appear — and `MaxCapturedVariableCount` still caps the total entry count across all scopes, so a field-heavy type can push some instance fields out of the snapshot. If a specific field you want is missing, read it directly from the live instance instead of waiting on the capped snapshot: while still paused, `UloopPausePoint.TryGetCapturedValue("this")` returns the live `this` reference, so `execute-dynamic-code` can read any field or property off it regardless of the cap. +- A `ref`, `out`, or `in` parameter, a pointer, and a `ref struct` value (`Span` and any user-defined `ref struct`) can never be captured, because the snapshot boxes every value and none of these shapes can be boxed. The enable and `pause-point-status` responses list each such parameter with its reason in `NotCapturableVariables` (absent when there are none), so a missing name is explained rather than looking like a capture bug; to observe one of those values, copy the value it refers to into a plain local (dereference a pointer, `ToArray()` a span), or arm the line that consumes it with `--snapshot-timing post-line`. - The snapshot also includes a synthetic `this` entry (Scope `This`) for the paused instance itself, so you can tell which instance or GameObject was hit via its `UnityObjectPath` and `UnityObjectInstanceId`. For an async or coroutine method it resolves to the original outer instance, not the compiler-generated state machine, and static methods emit no `this` entry. While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("this")` returns the live instance reference (for example so a watch expression can read `transform.position`). - async and coroutine methods work: hoisted locals and the original `this` fields appear under their normal names. - Auto-implemented properties are captured as instance fields under the property name (the compiler-generated backing field is un-mangled), so you do not need to rewrite them as explicit fields for verification. diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointNotCapturableWarnings.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointNotCapturableWarnings.cs new file mode 100644 index 0000000000..fc3a40ac03 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointNotCapturableWarnings.cs @@ -0,0 +1,26 @@ +using System.Collections.Generic; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Builds the enable-time warning that names the parameters capture cannot box. + /// + internal static class PausePointNotCapturableWarnings + { + /// + /// Warns about parameters left out of CapturedVariables, or empty when there are none. + /// + internal static string BuildNotCapturableParametersWarningOrEmpty( + IReadOnlyList notCapturableVariables) + { + if (notCapturableVariables == null || notCapturableVariables.Count == 0) + { + return string.Empty; + } + + return string.Format( + SourcePausePointConstants.NotCapturableParametersWarningFormat, + string.Join(", ", notCapturableVariables)); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointNotCapturableWarnings.cs.meta b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointNotCapturableWarnings.cs.meta new file mode 100644 index 0000000000..99a5c45edd --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointNotCapturableWarnings.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2f6befb79c9054b759fd102b11202269 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs index c75c0b4182..9aa4493052 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs @@ -75,6 +75,10 @@ public class PausePointResponse : UnityCliLoopToolResponse public int MaxCallerFrames { get; set; } public IReadOnlyList CapturedVariableHistory { get; set; } = Array.Empty(); + // Parameters of the armed method that capture cannot box, each with the reason. + // Null when there are none so the response omits the field (matches Go omitempty). + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + public IReadOnlyList NotCapturableVariables { get; set; } public int HistoryDroppedCount { get; set; } public bool Expired { get; set; } public string EnabledAtUtc { get; set; } = string.Empty; @@ -105,6 +109,13 @@ public class PausePointResponse : UnityCliLoopToolResponse [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public string SuppressedByHotReloadReason { get; set; } + // Why null for empty: the response omits the field entirely when every parameter of the + // armed method can be captured, so a reader never sees an empty list to interpret. + internal static IReadOnlyList NormalizeNotCapturableVariables(IReadOnlyList values) + { + return values == null || values.Count == 0 ? null : values; + } + public bool ShouldSerializeLineBasis() { return !string.IsNullOrEmpty(LineBasis); @@ -137,6 +148,7 @@ internal static PausePointResponse FromSnapshot(UloopPausePointSnapshot snapshot .Select(PausePointCapturedHistoryFrame.FromSnapshot) .ToList(), HistoryDroppedCount = snapshot.HistoryDroppedCount, + NotCapturableVariables = NormalizeNotCapturableVariables(snapshot.NotCapturableVariables), Expired = snapshot.Expired, EnabledAtUtc = snapshot.EnabledAtUtc, ElapsedSinceEnabledMilliseconds = snapshot.ElapsedSinceEnabledMilliseconds, diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs index 4aaa7c6cfd..4fd8e1e7fe 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs @@ -258,6 +258,7 @@ private static PausePointResponse EnableBySourceLocation( "EditedFile", retargetedToHotReloadPatch: true, hasActiveHotReloadPatches: true, + shimResolution.NotCapturableVariables, editedMethodStartLine: shimResolution.SourceStartLine, editedMethodEndLine: shimResolution.SourceEndLine); } @@ -371,6 +372,7 @@ private static PausePointResponse EnableBySourceLocation( "LastCompiledSource", retargetedToHotReloadPatch: false, hasActiveHotReloadPatches: shimLookup != null, + resolveResult.Resolution.NotCapturableVariables, compiledMethodStartLine: resolveResult.Resolution.CompiledMethodStartLine, compiledMethodEndLine: resolveResult.Resolution.CompiledMethodEndLine, patchedMethodPdbUnavailableWarning: patchedMethodPdbUnavailableWarning, @@ -410,6 +412,7 @@ private static PausePointResponse FinishEnableBySourceLocation( string lineBasis, bool retargetedToHotReloadPatch, bool hasActiveHotReloadPatches, + IReadOnlyList notCapturableVariables, int editedMethodStartLine = 0, int editedMethodEndLine = 0, int compiledMethodStartLine = 0, @@ -447,11 +450,14 @@ private static PausePointResponse FinishEnableBySourceLocation( ? SourcePausePointSourceLineReader.ReadLineTextFromSource(compiledSnapshotSource, resolvedLine) : PausePointLineTextReader.ReadResolvedLineText(parameters.File, resolvedLine, resolvedEndLine); UloopPausePointRegistry.SetResolvedLine(id, resolvedLine, resolvedLineText); + UloopPausePointRegistry.SetNotCapturableVariables(id, notCapturableVariables); PausePointResponse response = PausePointResponse.FromSnapshot(snapshot); // Why re-read after SetResolvedLine: FromSnapshot above used the pre-write snapshot. response.ResolvedLine = resolvedLine; response.ResolvedLineText = resolvedLineText; + response.NotCapturableVariables = + PausePointResponse.NormalizeNotCapturableVariables(notCapturableVariables); response.ResolvedMethod = resolvedMethod; response.SnapshotTiming = DescribeSnapshotTiming(ParseSnapshotTiming(parameters.SnapshotTiming)); response.LineBasis = lineBasis; @@ -519,6 +525,10 @@ private static PausePointResponse FinishEnableBySourceLocation( parameters.Mode, resolvedMethod, snapshot.MaxHistory)); + PausePointEnableWarningList.AddIfNotEmpty( + warningEntries, + PausePointNotCapturableWarnings.BuildNotCapturableParametersWarningOrEmpty( + notCapturableVariables)); PausePointEnableWarningList.AddIfNotEmpty(warningEntries, rearmWarning); PausePointEnableWarningList.AddIfNotEmpty( warningEntries, diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCaptureEligibility.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCaptureEligibility.cs index 0a94083e54..8277657729 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCaptureEligibility.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointCaptureEligibility.cs @@ -163,7 +163,9 @@ private static bool IsCaptureExcluded(TypeReference type) { // byref locals/parameters (ref, out, in) and pointers cannot be boxed; ref structs // (Span, and any user-defined "ref struct") cannot be boxed either. - return type.IsByReference || type.IsPointer || IsRefStructType(type); + // Why derived from the reason: the excluded set and the reported set must stay the + // same set, so a parameter is never both captured and named as not capturable. + return DescribeNotCapturableReason(type).Length > 0; } private static bool IsRefStructType(TypeReference type) @@ -208,6 +210,29 @@ private static bool IsKnownFrameworkRefStructType(TypeReference type) return elementType.FullName == "System.Span`1" || elementType.FullName == "System.ReadOnlySpan`1"; } + /// + /// Names the parameters CollectParameters leaves out, each with the reason its type cannot + /// be boxed, so the enable response can say why an expected name is missing. + /// Locals are deliberately not reported: their names come from the PDB and change with + /// compiler-generated hoisting, so a name list built from them would be unstable. + /// + internal static List CollectNotCapturableParameters(MethodDefinition method) + { + List results = new List(); + foreach (ParameterDefinition parameter in method.Parameters) + { + string reason = DescribeNotCapturableReason(parameter.ParameterType); + if (reason.Length == 0) + { + continue; + } + + results.Add(FormatNotCapturableParameter(parameter.Name, reason)); + } + + return results; + } + internal static List CollectParameters(MethodDefinition method) { List parameters = new List(); @@ -261,9 +286,86 @@ internal static List CollectParametersFromReflection( return parameters; } + /// + /// Reflection counterpart of CollectNotCapturableParameters, for the shim path. + /// + internal static List CollectNotCapturableParametersFromReflection( + MethodBase method, + bool skipFirstParameter) + { + Debug.Assert(method != null, "method must not be null."); + + // Fully qualify: ToolContracts also defines ParameterInfo (tool schema DTO). + System.Reflection.ParameterInfo[] runtimeParameters = method.GetParameters(); + List results = new List(); + int startIndex = skipFirstParameter ? 1 : 0; + for (int index = startIndex; index < runtimeParameters.Length; index++) + { + System.Reflection.ParameterInfo parameter = runtimeParameters[index]; + string reason = DescribeNotCapturableReasonFromReflection(parameter.ParameterType); + if (reason.Length == 0) + { + continue; + } + + results.Add(FormatNotCapturableParameter(parameter.Name, reason)); + } + + return results; + } + + /// + /// The reason a Cecil type is excluded from capture, or empty when it is capturable. + /// + internal static string DescribeNotCapturableReason(TypeReference type) + { + // Byref first: a `ref Span<int>` is both byref and a ref struct, and the byref + // shape is the one the caller can act on by copying the value into a local. + if (type.IsByReference) + { + return SourcePausePointConstants.NotCapturableByRefParameterReason; + } + + if (type.IsPointer) + { + return SourcePausePointConstants.NotCapturablePointerReason; + } + + return IsRefStructType(type) + ? SourcePausePointConstants.NotCapturableRefStructReason + : string.Empty; + } + + /// + /// The reason a reflection type is excluded from capture, or empty when it is capturable. + /// + internal static string DescribeNotCapturableReasonFromReflection(Type type) + { + if (type.IsByRef) + { + return SourcePausePointConstants.NotCapturableByRefParameterReason; + } + + if (type.IsPointer) + { + return SourcePausePointConstants.NotCapturablePointerReason; + } + + return IsByRefLikeTypeReflection(type) + ? SourcePausePointConstants.NotCapturableRefStructReason + : string.Empty; + } + + private static string FormatNotCapturableParameter(string name, string reason) + { + return name + " (" + reason + ")"; + } + private static bool IsCaptureExcludedReflection(Type type) { - return type.IsByRef || type.IsPointer || IsByRefLikeTypeReflection(type); + // Why derived from the reason: the excluded set and the reported set must stay the + // same set, so a parameter is never both captured and named as not capturable. + return DescribeNotCapturableReasonFromReflection(type).Length > 0; } private static bool IsByRefLikeTypeReflection(Type type) diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index 2ed83612b0..c3a19f0709 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -182,6 +182,20 @@ internal static class SourcePausePointConstants public const string PostLineSnapshotTimingValue = "post-line"; + // Reasons a parameter is left out of CapturedVariables. Capture boxes every value into + // the snapshot, and these three shapes cannot be boxed at all, so naming the shape tells + // the caller why a name they expected is absent and which workaround applies. + public const string NotCapturableByRefParameterReason = "ref/out/in parameter cannot be boxed"; + public const string NotCapturablePointerReason = "pointer cannot be boxed"; + public const string NotCapturableRefStructReason = "ref struct cannot be boxed"; + + // Named at enable time so the caller learns the parameter is missing before running the + // code path, instead of reading CapturedVariables afterwards and suspecting a capture bug. + public const string NotCapturableParametersWarningFormat = + "Parameters not captured because they cannot be boxed: {0}. Copy the value it refers to" + + " into a plain local (dereference a pointer, ToArray() a span), or use" + + " --snapshot-timing post-line on the line that consumes it."; + // Why a resolve failure rather than a silent pre-line fallback: a statement that // always throws has no "after", and capturing before it would report values the // caller explicitly asked not to see. diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointHotReloadRetarget.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointHotReloadRetarget.cs index c0f6b2a31f..30bdc413c7 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointHotReloadRetarget.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointHotReloadRetarget.cs @@ -229,6 +229,9 @@ private static void RetargetMarkersOntoHotReloadPatch(List markerIds, Me id, shimResolution.ResolvedLine, newLineText); + UloopPausePointRegistry.SetNotCapturableVariables( + id, + shimResolution.NotCapturableVariables); if (!string.IsNullOrEmpty(previousLineText) && !string.Equals(previousLineText, newLineText, StringComparison.Ordinal)) { @@ -259,6 +262,7 @@ private static void RestoreMarkersAfterHotReloadRevert(List markerIds, M if (!SourcePausePointPatcher.RequestById.TryGetValue(id, out SourcePausePointEnableRequest request)) { UloopPausePointRegistry.SetResolvedLine(id, 0, null); + UloopPausePointRegistry.SetNotCapturableVariables(id, Array.Empty()); SuppressMarkerRestoreFailed(id); continue; } @@ -271,6 +275,7 @@ private static void RestoreMarkersAfterHotReloadRevert(List markerIds, M if (!resolveResult.Success) { UloopPausePointRegistry.SetResolvedLine(id, 0, null); + UloopPausePointRegistry.SetNotCapturableVariables(id, Array.Empty()); SuppressMarkerRestoreFailed(id); continue; } @@ -282,6 +287,7 @@ private static void RestoreMarkersAfterHotReloadRevert(List markerIds, M || !IsSameLogicalMethodOrItsStateMachine(restoredMethod, logicalOwner)) { UloopPausePointRegistry.SetResolvedLine(id, 0, null); + UloopPausePointRegistry.SetNotCapturableVariables(id, Array.Empty()); SuppressMarkerRestoreFailed(id); continue; } @@ -314,6 +320,9 @@ private static void RestoreMarkersAfterHotReloadRevert(List markerIds, M id, resolveResult.Resolution.ResolvedLine, restoredLineText); + UloopPausePointRegistry.SetNotCapturableVariables( + id, + resolveResult.Resolution.NotCapturableVariables); UloopPausePointRegistry.SetSuppressedByHotReload(id, false, null); UloopPausePointRegistry.SetRetargetedToHotReloadPatch(id, false); } @@ -322,8 +331,10 @@ private static void RestoreMarkersAfterHotReloadRevert(List markerIds, M { if (!committed) { - // Why clear: restore failed, so previously stored shim line is no longer trustworthy. + // Why clear: restore failed, so previously stored shim line and its + // exclusion list are no longer trustworthy. UloopPausePointRegistry.SetResolvedLine(id, 0, null); + UloopPausePointRegistry.SetNotCapturableVariables(id, Array.Empty()); ReanchorMarkerWithoutInjection(id, logicalOwner, request); SuppressMarkerRestoreFailed(id); } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolution.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolution.cs index 4ab65d0bf2..e1bf932bfd 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolution.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolution.cs @@ -27,6 +27,9 @@ internal sealed class SourcePausePointResolution public int CompiledMethodEndLine { get; } public IReadOnlyList Locals { get; } public IReadOnlyList Parameters { get; } + // Parameters left out of Parameters because their type cannot be boxed, each with the + // reason. Reported so a missing name is explained instead of looking like a capture bug. + public IReadOnlyList NotCapturableVariables { get; } public SourcePausePointResolution( string assemblyName, @@ -43,7 +46,8 @@ public SourcePausePointResolution( int compiledMethodStartLine, int compiledMethodEndLine, IReadOnlyList locals, - IReadOnlyList parameters) + IReadOnlyList parameters, + IReadOnlyList notCapturableVariables) { AssemblyName = assemblyName; Mvid = mvid; @@ -60,6 +64,7 @@ public SourcePausePointResolution( CompiledMethodEndLine = compiledMethodEndLine; Locals = locals; Parameters = parameters; + NotCapturableVariables = notCapturableVariables; } } } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs index f317123846..15325d6a33 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs @@ -130,6 +130,8 @@ private static SourcePausePointResolveResult ResolveFromCompiledAssembly( List locals = SourcePausePointCaptureEligibility.CollectCapturableLocals(method, scopeOffset); List parameters = SourcePausePointCaptureEligibility.CollectParameters(method); + List notCapturableVariables = + SourcePausePointCaptureEligibility.CollectNotCapturableParameters(method); (int compiledMethodStartLine, int compiledMethodEndLine) = CollectCompiledMethodSpan( method, normalizedInputPath); @@ -149,7 +151,8 @@ private static SourcePausePointResolveResult ResolveFromCompiledAssembly( compiledMethodStartLine, compiledMethodEndLine, locals, - parameters); + parameters, + notCapturableVariables); return SourcePausePointResolveResult.SuccessResult(resolution); } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointShimResolution.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointShimResolution.cs index 44fab388a4..919ae076e0 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointShimResolution.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointShimResolution.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Diagnostics; using System.Reflection; @@ -18,6 +19,9 @@ internal sealed class SourcePausePointShimResolution public int ResolvedLine { get; } public IReadOnlyList Locals { get; } public IReadOnlyList Parameters { get; } + // Parameters left out of Parameters because their type cannot be boxed, each with the + // reason. Reported so a missing name is explained instead of looking like a capture bug. + public IReadOnlyList NotCapturableVariables { get; } public bool InstanceFromFirstArgument { get; } public string MethodDisplayName { get; } public string ErrorMessage { get; } @@ -34,6 +38,7 @@ private SourcePausePointShimResolution( int resolvedLine, IReadOnlyList locals, IReadOnlyList parameters, + IReadOnlyList notCapturableVariables, bool instanceFromFirstArgument, string methodDisplayName, string errorMessage, @@ -49,6 +54,7 @@ private SourcePausePointShimResolution( ResolvedLine = resolvedLine; Locals = locals; Parameters = parameters; + NotCapturableVariables = notCapturableVariables; InstanceFromFirstArgument = instanceFromFirstArgument; MethodDisplayName = methodDisplayName; ErrorMessage = errorMessage; @@ -64,6 +70,7 @@ public static SourcePausePointShimResolution TransplantChainJoin( int resolvedLine, IReadOnlyList locals, IReadOnlyList parameters, + IReadOnlyList notCapturableVariables, int sourceStartLine, int sourceEndLine) { @@ -77,6 +84,7 @@ public static SourcePausePointShimResolution TransplantChainJoin( resolvedLine, locals, parameters, + notCapturableVariables, instanceFromFirstArgument: false, originalMethod.ToString(), errorMessage: null, @@ -92,6 +100,7 @@ public static SourcePausePointShimResolution ShimDirect( int resolvedLine, IReadOnlyList locals, IReadOnlyList parameters, + IReadOnlyList notCapturableVariables, bool instanceFromFirstArgument, int sourceStartLine, int sourceEndLine) @@ -106,6 +115,7 @@ public static SourcePausePointShimResolution ShimDirect( resolvedLine, locals, parameters, + notCapturableVariables, instanceFromFirstArgument, logicalOwner.ToString(), errorMessage: null, @@ -125,6 +135,7 @@ public static SourcePausePointShimResolution NotInPatchedMethod() resolvedLine: 0, locals: null, parameters: null, + notCapturableVariables: Array.Empty(), instanceFromFirstArgument: false, methodDisplayName: null, errorMessage: null, @@ -146,6 +157,7 @@ public static SourcePausePointShimResolution PatchedMethodPdbUnavailable(MethodB resolvedLine: 0, locals: null, parameters: null, + notCapturableVariables: Array.Empty(), instanceFromFirstArgument: false, typeName + "." + logicalOwner.Name, errorMessage: null, @@ -167,6 +179,7 @@ public static SourcePausePointShimResolution NoStatementInPatchedMethod( resolvedLine: 0, locals: null, parameters: null, + notCapturableVariables: Array.Empty(), instanceFromFirstArgument: false, logicalOwner.ToString(), errorMessage, diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointShimResolver.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointShimResolver.cs index c51fe78bd2..44822f482e 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointShimResolver.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointShimResolver.cs @@ -112,6 +112,10 @@ public static SourcePausePointShimResolution Resolve( SourcePausePointCaptureEligibility.CollectParametersFromReflection( methodEntry.OriginalMethod, skipFirstParameter: false); + List notCapturableVariables = + SourcePausePointCaptureEligibility.CollectNotCapturableParametersFromReflection( + methodEntry.OriginalMethod, + skipFirstParameter: false); return SourcePausePointShimResolution.TransplantChainJoin( methodEntry.OriginalMethod, methodEntry.ShimMethod, @@ -120,6 +124,7 @@ public static SourcePausePointShimResolution Resolve( sequencePoint.StartLine, locals, parameters, + notCapturableVariables, methodEntry.SourceStartLine, methodEntry.SourceEndLine); } @@ -134,6 +139,10 @@ public static SourcePausePointShimResolution Resolve( SourcePausePointCaptureEligibility.CollectParametersFromReflection( targetMethod, skipFirstParameter: instanceFromFirstArgument); + List shimNotCapturableVariables = + SourcePausePointCaptureEligibility.CollectNotCapturableParametersFromReflection( + targetMethod, + skipFirstParameter: instanceFromFirstArgument); return SourcePausePointShimResolution.ShimDirect( targetMethod, @@ -143,6 +152,7 @@ public static SourcePausePointShimResolution Resolve( sequencePoint.StartLine, locals, shimParameters, + shimNotCapturableVariables, instanceFromFirstArgument, methodEntry.SourceStartLine, methodEntry.SourceEndLine); diff --git a/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs b/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs index 578fba4670..22fac4b3cf 100644 --- a/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs +++ b/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs @@ -188,6 +188,10 @@ public class PausePointStatusResponse : UnityCliLoopToolResponse public bool CapturedVariablesTruncated { get; set; } public IReadOnlyList TruncatedVariableNames { get; set; } = Array.Empty(); public int TruncatedVariableCount { get; set; } + // Parameters of the resolved method that capture cannot box, each with the reason. + // Null when there are none so the status contract omits the field (matches Go omitempty). + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + public IReadOnlyList NotCapturableVariables { get; set; } public string ClearedReason { get; set; } = string.Empty; public string StatusBeforeClear { get; set; } = string.Empty; public bool LateHitDiscardedAfterClear { get; set; } @@ -209,6 +213,13 @@ public class PausePointStatusResponse : UnityCliLoopToolResponse [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public string ResolvedLineText { get; set; } + // Why null for empty: the status contract omits the field entirely when every parameter of + // the resolved method can be captured, so a reader never sees an empty list to interpret. + private static IReadOnlyList NormalizeNotCapturableVariables(IReadOnlyList values) + { + return values == null || values.Count == 0 ? null : values; + } + internal static PausePointStatusResponse FromSnapshot(UloopPausePointSnapshot snapshot) { if (snapshot == null) @@ -260,6 +271,7 @@ internal static PausePointStatusResponse FromSnapshot(UloopPausePointSnapshot sn CapturedVariablesTruncated = snapshot.CapturedVariablesTruncated, TruncatedVariableNames = snapshot.TruncatedVariableNames, TruncatedVariableCount = snapshot.TruncatedVariableCount, + NotCapturableVariables = NormalizeNotCapturableVariables(snapshot.NotCapturableVariables), ClearedReason = snapshot.ClearedReason, StatusBeforeClear = snapshot.StatusBeforeClear, LateHitDiscardedAfterClear = snapshot.LateHitDiscardedAfterClear, diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs b/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs index 91f3e54fe2..109254ddbd 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs @@ -49,6 +49,7 @@ public UloopPausePointEntry( CapturedVariables = Array.Empty(); CallerFrames = Array.Empty(); TruncatedVariableNames = Array.Empty(); + NotCapturableVariables = Array.Empty(); _capturedVariableHistory = new Queue(maxHistory); } @@ -92,6 +93,10 @@ public UloopPausePointEntry( // 0 / null-or-empty means unresolved (not yet written by enable or retarget). public int ResolvedLine { get; set; } public string ResolvedLineText { get; set; } + // Parameters the resolved method has that capture cannot box, each with the reason. + // Written by enable and by hot-reload retarget alongside ResolvedLine, and emptied + // whenever the resolution behind it is discarded. + public IReadOnlyList NotCapturableVariables { get; set; } public void IncrementMethodEntryCount() { @@ -336,6 +341,7 @@ public UloopPausePointSnapshot ToSnapshot(DateTime nowUtc, IUloopPausePointPause CapturedVariablesTruncated, TruncatedVariableNames, TruncatedVariableCount, + NotCapturableVariables, ClearedReason, StatusBeforeClear, LateHitDiscardedAfterClear, diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.PauseWindow.cs b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.PauseWindow.cs new file mode 100644 index 0000000000..c5182221b7 --- /dev/null +++ b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.PauseWindow.cs @@ -0,0 +1,153 @@ +#if UNITY_EDITOR +using System; +using System.Threading; + +namespace io.github.hatayama.UnityCliLoop.Runtime +{ + /// + /// The registry's Editor pause window: which marker's hit is holding the Editor paused, and + /// every path that opens, credits back, or closes that pause. Split from the main registry + /// file so each stays inside the repository file-length limit. + /// + internal static partial class UloopPausePointRegistry + { + // Why Interlocked: disconnect monitor / DisconnectAllClients run on the thread pool. + private static int _pendingClientDisconnectResume; + // Set when a hit pauses the Editor (see HitCore), cleared when ResumeEditorPause runs. + // While set, no entry's capture window is allowed to expire (see TryExpire): a marker's + // own hit freezes every marker's countdown for the duration of the inspection pause, + // and the frozen duration is credited back to each entry's ExpiresAtUtc on resume. + private static DateTime? _pauseWindowStartUtc; + // The id of the marker whose hit is actually holding the Editor paused. Kept separate + // from _latestHitSnapshot, which every hit overwrites (including Trace-mode hits that + // never pause): using _latestHitSnapshot here would let an unrelated Trace hit that + // occurs while already paused (e.g. via execute-dynamic-code or Step) misattribute the + // pause to itself. Overwritten by a later non-Trace hit, since a second marker hitting + // while already paused becomes the new (only) reason the Editor stays paused. + private static string _pauseWindowOwnerId; + + /// + /// Read-only signal for callers outside this registry (e.g. execute-dynamic-code) that + /// need to know whether the Editor is currently paused because of a pause-point hit, and + /// which marker's hit is responsible. Reads _pauseWindowOwnerId rather than + /// _latestHitSnapshot, which every hit overwrites (including Trace-mode hits that never + /// pause) and would otherwise misattribute an open pause window to an unrelated marker. + /// Returns empty when no window is open, even if the Editor happens to be paused for an + /// unrelated reason. + /// + public static string GetActivePausePointId() + { + return _pauseWindowStartUtc.HasValue ? _pauseWindowOwnerId ?? string.Empty : string.Empty; + } + + /// + /// Requests Editor resume when a CLI client drops mid-request or the bridge disconnects all clients. + /// Why flag only: callers run on the thread pool where Unity Editor APIs are unsafe; the main-thread + /// Editor update pump applies the resume via ApplyPendingClientDisconnectResume. + /// Why not on every short-lived command close: that would resume immediately after await-pause-point + /// returns a Hit and break the paused inspection workflow. + /// + public static void ResumeEditorPauseForClientDisconnect() + { + Interlocked.Exchange(ref _pendingClientDisconnectResume, 1); + } + + /// + /// Consumes a pending client-disconnect resume on the Editor main thread. + /// + public static void ApplyPendingClientDisconnectResume() + { + if (Interlocked.Exchange(ref _pendingClientDisconnectResume, 0) == 0) + { + return; + } + + // Why discard when already running: Option B still clears a stale request without + // calling Resume on an Editor that is not paused. + if (!_pauseController.IsPaused) + { + return; + } + + ResumeEditorPause(); + } + + private static void ResumeEditorPause() + { + CreditPauseWindowAndClose(NowUtc()); + _pauseController.Resume(); + } + + // Resumes the Editor only when a pause-point hit currently owns the pause (a pause window + // is open) and reports whether it did. Clear/ClearAll use this so they never resume a + // manual pause that no pause-point hit is responsible for. Disconnect and expiry paths + // deliberately call ResumeEditorPause directly instead, because they must release the + // Editor even when the pause was manual. + private static bool ResumeEditorPauseIfOwnedByPausePoint() + { + // A manual unpause (Editor pause button / control-play-mode) may not have been observed + // yet, because the external-resume sync runs on the Editor update tick. Reconcile first + // so a window left open by that unobserved unpause is credited and closed here instead + // of being misreported as a resume this clear performed (it would be a no-op resume of + // an already-unpaused Editor) and instead of leaving the stale window freezing expiry. + ClosePauseWindowIfEditorResumedExternally(); + if (!_pauseWindowStartUtc.HasValue) + { + return false; + } + + ResumeEditorPause(); + return true; + } + + // Clear(id) must resume only when this id is the pause-window owner. Clearing a different + // marker (including AwaitTimeoutAutoClear of a never-hit wait) must leave the owner's + // inspection pause in place. + private static bool ResumeEditorPauseIfOwnedByMarker(string id) + { + if (_pauseWindowOwnerId != id) + { + return false; + } + + return ResumeEditorPauseIfOwnedByPausePoint(); + } + + /// + /// Closes an open pause window when the Editor was unpaused through a path that never + /// calls back into this registry - control-play-mode's Play/Stop + /// (ControlPlayModeUseCase sets EditorApplication.isPaused directly) or the + /// Editor's own pause button. Without this, a window left open by such an external resume + /// would freeze every marker's countdown forever and later over-credit the elapsed + /// wall-clock time back onto ExpiresAtUtc. Call every main-thread Editor update. + /// + public static void ClosePauseWindowIfEditorResumedExternally() + { + if (!_pauseWindowStartUtc.HasValue || _pauseController.IsPaused) + { + return; + } + + CreditPauseWindowAndClose(NowUtc()); + } + + // Credits the frozen duration (now - max(pauseWindowStart, EnabledAtUtc)) back onto every + // entry's ExpiresAtUtc and closes the window. A no-op when no window is open. + private static void CreditPauseWindowAndClose(DateTime now) + { + if (!_pauseWindowStartUtc.HasValue) + { + return; + } + + DateTime pauseWindowStart = _pauseWindowStartUtc.Value; + _pauseWindowStartUtc = null; + _pauseWindowOwnerId = null; + foreach (UloopPausePointEntry entry in Entries.Values) + { + entry.ExtendExpiryForPause(pauseWindowStart, now); + } + } + } +} +#endif diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.PauseWindow.cs.meta b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.PauseWindow.cs.meta new file mode 100644 index 0000000000..4fba261603 --- /dev/null +++ b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.PauseWindow.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7a664780ad1dd4244a5d9641f99c3bcb +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs index 0c850fdc0d..63f5dd6951 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs @@ -14,7 +14,7 @@ namespace io.github.hatayama.UnityCliLoop.Runtime /// accessors are Harmony entry points, and the disconnect resume path only sets a pending flag /// so thread-pool callers never touch EditorApplication.isPaused. /// - internal static class UloopPausePointRegistry + internal static partial class UloopPausePointRegistry { public const int DefaultTimeoutSeconds = 30; public const int DefaultMaxHistory = 20; @@ -30,21 +30,7 @@ internal static class UloopPausePointRegistry private static Func _nowProvider = () => DateTime.UtcNow; private static int _nextGeneration; private static int _nextHitSequence; - // Why Interlocked: disconnect monitor / DisconnectAllClients run on the thread pool. - private static int _pendingClientDisconnectResume; private static UloopPausePointSnapshot _latestHitSnapshot; - // Set when a hit pauses the Editor (see HitCore), cleared when ResumeEditorPause runs. - // While set, no entry's capture window is allowed to expire (see TryExpire): a marker's - // own hit freezes every marker's countdown for the duration of the inspection pause, - // and the frozen duration is credited back to each entry's ExpiresAtUtc on resume. - private static DateTime? _pauseWindowStartUtc; - // The id of the marker whose hit is actually holding the Editor paused. Kept separate - // from _latestHitSnapshot, which every hit overwrites (including Trace-mode hits that - // never pause): using _latestHitSnapshot here would let an unrelated Trace hit that - // occurs while already paused (e.g. via execute-dynamic-code or Step) misattribute the - // pause to itself. Overwritten by a later non-Trace hit, since a second marker hitting - // while already paused becomes the new (only) reason the Editor stays paused. - private static string _pauseWindowOwnerId; // One input can hit several markers in the same frame; tools need the full list, // not just the latest hit, to report every marker that interrupted them. private static readonly List _hitSnapshots = new(); @@ -281,6 +267,24 @@ public static void SetResolvedLine(string id, int resolvedLine, string resolvedL entry.ResolvedLineText = resolvedLineText; } + /// + /// Stores the parameters capture cannot box for the currently resolved method, so status + /// keeps explaining a missing name after enable and after a hot-reload retarget. + /// Pass an empty list to clear (the resolution behind it was discarded). + /// No-op when the id is unknown. + /// + public static void SetNotCapturableVariables(string id, IReadOnlyList notCapturableVariables) + { + Debug.Assert(notCapturableVariables != null, "notCapturableVariables must not be null"); + + if (!Entries.TryGetValue(id, out UloopPausePointEntry entry)) + { + return; + } + + entry.NotCapturableVariables = notCapturableVariables; + } + /// /// Extends a marker's capture window to at least minimumRemainingSeconds from now, so a /// slow multi-step CLI round trip (enable -> seed state -> await) does not let the marker @@ -516,20 +520,6 @@ public static UloopPausePointSnapshot GetLatestHitSnapshot() return _latestHitSnapshot; } - /// - /// Read-only signal for callers outside this registry (e.g. execute-dynamic-code) that - /// need to know whether the Editor is currently paused because of a pause-point hit, and - /// which marker's hit is responsible. Reads _pauseWindowOwnerId rather than - /// _latestHitSnapshot, which every hit overwrites (including Trace-mode hits that never - /// pause) and would otherwise misattribute an open pause window to an unrelated marker. - /// Returns empty when no window is open, even if the Editor happens to be paused for an - /// unrelated reason. - /// - public static string GetActivePausePointId() - { - return _pauseWindowStartUtc.HasValue ? _pauseWindowOwnerId ?? string.Empty : string.Empty; - } - public static void ClearLatestHitSnapshot() { _latestHitSnapshot = null; @@ -559,115 +549,6 @@ public static void ApplyCaptureWindowExpirations() } } - /// - /// Requests Editor resume when a CLI client drops mid-request or the bridge disconnects all clients. - /// Why flag only: callers run on the thread pool where Unity Editor APIs are unsafe; the main-thread - /// Editor update pump applies the resume via ApplyPendingClientDisconnectResume. - /// Why not on every short-lived command close: that would resume immediately after await-pause-point - /// returns a Hit and break the paused inspection workflow. - /// - public static void ResumeEditorPauseForClientDisconnect() - { - Interlocked.Exchange(ref _pendingClientDisconnectResume, 1); - } - - /// - /// Consumes a pending client-disconnect resume on the Editor main thread. - /// - public static void ApplyPendingClientDisconnectResume() - { - if (Interlocked.Exchange(ref _pendingClientDisconnectResume, 0) == 0) - { - return; - } - - // Why discard when already running: Option B still clears a stale request without - // calling Resume on an Editor that is not paused. - if (!_pauseController.IsPaused) - { - return; - } - - ResumeEditorPause(); - } - - private static void ResumeEditorPause() - { - CreditPauseWindowAndClose(NowUtc()); - _pauseController.Resume(); - } - - // Resumes the Editor only when a pause-point hit currently owns the pause (a pause window - // is open) and reports whether it did. Clear/ClearAll use this so they never resume a - // manual pause that no pause-point hit is responsible for. Disconnect and expiry paths - // deliberately call ResumeEditorPause directly instead, because they must release the - // Editor even when the pause was manual. - private static bool ResumeEditorPauseIfOwnedByPausePoint() - { - // A manual unpause (Editor pause button / control-play-mode) may not have been observed - // yet, because the external-resume sync runs on the Editor update tick. Reconcile first - // so a window left open by that unobserved unpause is credited and closed here instead - // of being misreported as a resume this clear performed (it would be a no-op resume of - // an already-unpaused Editor) and instead of leaving the stale window freezing expiry. - ClosePauseWindowIfEditorResumedExternally(); - if (!_pauseWindowStartUtc.HasValue) - { - return false; - } - - ResumeEditorPause(); - return true; - } - - // Clear(id) must resume only when this id is the pause-window owner. Clearing a different - // marker (including AwaitTimeoutAutoClear of a never-hit wait) must leave the owner's - // inspection pause in place. - private static bool ResumeEditorPauseIfOwnedByMarker(string id) - { - if (_pauseWindowOwnerId != id) - { - return false; - } - - return ResumeEditorPauseIfOwnedByPausePoint(); - } - - /// - /// Closes an open pause window when the Editor was unpaused through a path that never - /// calls back into this registry - control-play-mode's Play/Stop - /// (ControlPlayModeUseCase sets EditorApplication.isPaused directly) or the - /// Editor's own pause button. Without this, a window left open by such an external resume - /// would freeze every marker's countdown forever and later over-credit the elapsed - /// wall-clock time back onto ExpiresAtUtc. Call every main-thread Editor update. - /// - public static void ClosePauseWindowIfEditorResumedExternally() - { - if (!_pauseWindowStartUtc.HasValue || _pauseController.IsPaused) - { - return; - } - - CreditPauseWindowAndClose(NowUtc()); - } - - // Credits the frozen duration (now - max(pauseWindowStart, EnabledAtUtc)) back onto every - // entry's ExpiresAtUtc and closes the window. A no-op when no window is open. - private static void CreditPauseWindowAndClose(DateTime now) - { - if (!_pauseWindowStartUtc.HasValue) - { - return; - } - - DateTime pauseWindowStart = _pauseWindowStartUtc.Value; - _pauseWindowStartUtc = null; - _pauseWindowOwnerId = null; - foreach (UloopPausePointEntry entry in Entries.Values) - { - entry.ExtendExpiryForPause(pauseWindowStart, now); - } - } - // While a hit has the Editor paused, no entry may expire: the countdown is frozen for // everyone until the open window is closed and credits the frozen duration back (see // CreditPauseWindowAndClose). Without this gate, TryExpire callers would still expire a diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs b/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs index 0b3beda755..cd98763a39 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs @@ -42,6 +42,7 @@ public UloopPausePointSnapshot( bool capturedVariablesTruncated, IReadOnlyList truncatedVariableNames, int truncatedVariableCount, + IReadOnlyList notCapturableVariables, string clearedReason, string statusBeforeClear, bool lateHitDiscardedAfterClear, @@ -55,6 +56,10 @@ public UloopPausePointSnapshot( string hitWhenErrorNote) { Debug.Assert(editorState != null, "editorState must not be null"); + // Why a precondition instead of a null coalesce: every writer already supplies a list + // (the entry initializes it, SetNotCapturableVariables asserts non-null, and the + // NotEnabled factory passes Array.Empty), so a null here is a caller bug to surface. + Debug.Assert(notCapturableVariables != null, "notCapturableVariables must not be null"); Id = id ?? string.Empty; Status = status ?? UloopPausePointStatus.NotEnabled; @@ -86,6 +91,7 @@ public UloopPausePointSnapshot( CapturedVariablesTruncated = capturedVariablesTruncated; TruncatedVariableNames = truncatedVariableNames ?? Array.Empty(); TruncatedVariableCount = truncatedVariableCount; + NotCapturableVariables = notCapturableVariables; ClearedReason = clearedReason ?? string.Empty; StatusBeforeClear = statusBeforeClear ?? string.Empty; LateHitDiscardedAfterClear = lateHitDiscardedAfterClear; @@ -129,6 +135,8 @@ public UloopPausePointSnapshot( public bool CapturedVariablesTruncated { get; } public IReadOnlyList TruncatedVariableNames { get; } public int TruncatedVariableCount { get; } + // Parameters of the resolved method that capture cannot box, each with the reason. + public IReadOnlyList NotCapturableVariables { get; } public string ClearedReason { get; } public string StatusBeforeClear { get; } public bool LateHitDiscardedAfterClear { get; } @@ -184,6 +192,7 @@ public static UloopPausePointSnapshot NotEnabled(string id, IUloopPausePointPaus false, Array.Empty(), 0, + Array.Empty(), string.Empty, string.Empty, false, diff --git a/cli/project-runner/internal/projectrunner/pause_point_status_response_key_order_test.go b/cli/project-runner/internal/projectrunner/pause_point_status_response_key_order_test.go index 8fad292429..27c2b9cf54 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_status_response_key_order_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_status_response_key_order_test.go @@ -63,6 +63,7 @@ func TestPausePointStatusResponseMarshalsReadingNotesBeforeCapturedVariables(t * "CapturedVariablesTruncated", "TruncatedVariableNames", "TruncatedVariableCount", + "NotCapturableVariables", "CapturedVariablesTruncatedNote", "CapturedVariablePreviewNote", "ClearedReason", @@ -130,6 +131,7 @@ func fullyPopulatedPausePointStatusResponse() pausePointStatusResponse { CapturedVariablesTruncated: true, TruncatedVariableNames: []string{"n"}, TruncatedVariableCount: 1, + NotCapturableVariables: []string{"accumulator (ref/out/in parameter cannot be boxed)"}, CapturedVariablesTruncatedNote: "note", CapturedVariablePreviewNote: "preview", ClearedReason: "ExplicitClear", diff --git a/cli/project-runner/internal/projectrunner/pause_point_types.go b/cli/project-runner/internal/projectrunner/pause_point_types.go index bea447df7a..361c1cdec6 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_types.go +++ b/cli/project-runner/internal/projectrunner/pause_point_types.go @@ -55,6 +55,10 @@ type pausePointStatusResponse struct { CapturedVariablesTruncated bool `json:"CapturedVariablesTruncated"` TruncatedVariableNames []string `json:"TruncatedVariableNames"` TruncatedVariableCount int `json:"TruncatedVariableCount"` + // NotCapturableVariables names the resolved method's parameters that capture cannot box + // (ref/out/in, pointer, ref struct), each with the reason. Unity omits the field when every + // parameter can be captured, so omitempty keeps the shared contract shape unchanged. + NotCapturableVariables []string `json:"NotCapturableVariables,omitempty"` // CapturedVariablesTruncatedNote is set by the CLI, not Unity, when // --captured-variable-names dropped every truncated variable so the remaining diff --git a/tests/contracts/pause_point_status_response_contract.json b/tests/contracts/pause_point_status_response_contract.json index da54c584c6..a3ac77b0e9 100644 --- a/tests/contracts/pause_point_status_response_contract.json +++ b/tests/contracts/pause_point_status_response_contract.json @@ -79,6 +79,9 @@ "extraField" ], "TruncatedVariableCount": 1, + "NotCapturableVariables": [ + "accumulator (ref/out/in parameter cannot be boxed)" + ], "ClearedReason": "", "StatusBeforeClear": "", "LateHitDiscardedAfterClear": false,