From d0f099089be769d49c6a1958a77d061fa13fb00e Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 00:37:09 +0900 Subject: [PATCH 1/6] feat(pause-point): collect the parameters capture cannot box Capture silently drops byref, pointer, and ref-struct parameters because none of them can be boxed into the snapshot, so a caller who armed a line to read an out argument saw the name simply missing from CapturedVariables with nothing explaining why. Add the collectors that name those parameters together with the reason their type cannot be boxed, on both resolve paths (Cecil for compiled assemblies, reflection for hot-reload shims). The existing exclusion predicates now derive from the same reason mapper, so the captured set and the reported set are guaranteed to stay complements of each other. Locals are deliberately left out: their names come from the PDB and shift with compiler-generated hoisting, so a list built from them would be unstable across builds. Claude-Session: https://claude.ai/code/session_01XbhSMKK4LFud57iAmowDz7 --- ...PausePointNotCapturableParameterFixture.cs | 30 +++ ...PointNotCapturableParameterFixture.cs.meta | 11 + ...ePausePointNotCapturableParametersTests.cs | 237 ++++++++++++++++++ ...ePointNotCapturableParametersTests.cs.meta | 11 + .../SourcePausePointCaptureEligibility.cs | 106 +++++++- .../PausePoint/SourcePausePointConstants.cs | 7 + 6 files changed, 400 insertions(+), 2 deletions(-) create mode 100644 Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs create mode 100644 Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs.meta create mode 100644 Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs create mode 100644 Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs.meta diff --git a/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs b/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs new file mode 100644 index 0000000000..9c3daac423 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs @@ -0,0 +1,30 @@ +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 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..12bd3230bd --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs @@ -0,0 +1,237 @@ +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: 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/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..9a767b8b11 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -182,6 +182,13 @@ 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"; + // 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. From f178c71292a1d5eacec0c552156514511c26670b Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 00:58:26 +0900 Subject: [PATCH 2/6] feat(pause-point): report the not-capturable parameters on enable and status A caller who armed a line to read an `out` argument saw the name simply missing from CapturedVariables. The exclusion was correct (byref, pointer, and ref-struct values cannot be boxed into the snapshot) but nothing on the wire said so, which reads as a capture bug. Carry the collected names, each with the reason its type cannot be boxed, from both resolve paths through the registry entry and snapshot into the enable and pause-point-status responses as NotCapturableVariables, and name them once more in the enable Warnings together with the two workarounds (copy into a local, or arm the consuming line with --snapshot-timing post-line). The field is omitted entirely when every parameter can be captured, so the shared status contract shape is unchanged for a fully capturable method. Hot-reload retarget follows the same rule as ResolvedLine: the two commit-success paths store the newly resolved list, and every path that discards its resolution clears it, because a stale exclusion list left behind by a discarded resolution would be a lie. Adding the registry setter pushed UloopPausePointRegistry.cs over the 500 SLOC gate, so its Editor pause-window group (the window fields, GetActivePausePointId, and every resume/credit/close path) moves to a partial file beside it. Claude-Session: https://claude.ai/code/session_01TxzFzK1W8JCdwWhDs6nSSZ --- .../HotReloadPausePointContractTests.cs | 3 +- ...ePointExpiredRecommendedNextActionTests.cs | 1 + .../PausePointNotCapturableVariablesTests.cs | 232 ++++++++++++++++++ ...sePointNotCapturableVariablesTests.cs.meta | 11 + .../PausePointStatusResponseContractTests.cs | 1 + Assets/Tests/Editor/PausePointTests.cs | 3 +- .../SourcePausePointPatcherTests.cs | 3 +- .../PausePointNotCapturableWarnings.cs | 26 ++ .../PausePointNotCapturableWarnings.cs.meta | 11 + .../PausePoint/PausePointTools.cs | 12 + .../PausePoint/PausePointUseCase.cs | 10 + .../PausePoint/SourcePausePointConstants.cs | 6 + .../SourcePausePointHotReloadRetarget.cs | 13 +- .../PausePoint/SourcePausePointResolution.cs | 7 +- .../PausePoint/SourcePausePointResolver.cs | 5 +- .../SourcePausePointShimResolution.cs | 13 + .../SourcePausePointShimResolver.cs | 10 + .../Api/PausePointStatusBridgeCommand.cs | 12 + .../PausePoints/UloopPausePointEntry.cs | 6 + .../UloopPausePointRegistry.PauseWindow.cs | 153 ++++++++++++ ...loopPausePointRegistry.PauseWindow.cs.meta | 11 + .../PausePoints/UloopPausePointRegistry.cs | 157 ++---------- .../PausePoints/UloopPausePointSnapshot.cs | 5 + ...se_point_status_response_key_order_test.go | 2 + .../projectrunner/pause_point_types.go | 4 + .../pause_point_status_response_contract.json | 3 + 26 files changed, 576 insertions(+), 144 deletions(-) create mode 100644 Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs create mode 100644 Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/PausePointNotCapturableWarnings.cs create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/PausePointNotCapturableWarnings.cs.meta create mode 100644 Packages/src/Runtime/PausePoints/UloopPausePointRegistry.PauseWindow.cs create mode 100644 Packages/src/Runtime/PausePoints/UloopPausePointRegistry.PauseWindow.cs.meta 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..0596b54d68 --- /dev/null +++ b/Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs @@ -0,0 +1,232 @@ +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 to a local, 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/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/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/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index 9a767b8b11..80c273fab2 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -189,6 +189,12 @@ internal static class SourcePausePointConstants 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 to a local," + + " 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..5a5ba8f84f 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, @@ -86,6 +87,7 @@ public UloopPausePointSnapshot( CapturedVariablesTruncated = capturedVariablesTruncated; TruncatedVariableNames = truncatedVariableNames ?? Array.Empty(); TruncatedVariableCount = truncatedVariableCount; + NotCapturableVariables = notCapturableVariables ?? Array.Empty(); ClearedReason = clearedReason ?? string.Empty; StatusBeforeClear = statusBeforeClear ?? string.Empty; LateHitDiscardedAfterClear = lateHitDiscardedAfterClear; @@ -129,6 +131,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 +188,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, From 8b67ea1fafb26375447ce51cb8550e5e210fcc17 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 00:59:06 +0900 Subject: [PATCH 3/6] docs(pause-point): explain which parameters can never be captured The scopes section said which scopes are captured but never that three parameter shapes are excluded outright, so a reader hunting a missing `out` argument had nothing to look up. State the byref / pointer / ref-struct exclusion, point at NotCapturableVariables as the place the names and reasons appear, and name the two workarounds. Claude-Session: https://claude.ai/code/session_01TxzFzK1W8JCdwWhDs6nSSZ --- .../skills/uloop-pause-point/references/captured-variables.md | 1 + .../skills/uloop-pause-point/references/captured-variables.md | 1 + .../PausePoint/Skill/references/captured-variables.md | 1 + 3 files changed, 3 insertions(+) diff --git a/.agents/skills/uloop-pause-point/references/captured-variables.md b/.agents/skills/uloop-pause-point/references/captured-variables.md index 07fbd4b0cd..ef55483a37 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 it into a local, 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..ef55483a37 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 it into a local, 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/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md index 07fbd4b0cd..ef55483a37 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 it into a local, 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. From 8bf2c7d8800cb33c642a49f172cedcd9f87533bb Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 01:10:21 +0900 Subject: [PATCH 4/6] fix(pause-point): assert the not-capturable list instead of coalescing it Review round on #2572. The snapshot constructor's `?? Array.Empty()` for the new argument pushed its cyclomatic complexity to 16, over the repository's limit of 15. Every writer already supplies a list - the entry initializes it, SetNotCapturableVariables asserts non-null, and the NotEnabled factory passes Array.Empty - so the coalesce was defending against a caller bug it should surface instead. Replace it with a precondition assert. The skipFirstParameter test could not fail: the fixture's leading parameter was capturable, so skipping it left the expected list identical and an implementation ignoring the flag would still pass. Add a fixture method whose leading parameter is byref, and assert both directions - skipped, the leading name is gone; not skipped, it is reported. Claude-Session: https://claude.ai/code/session_01TxzFzK1W8JCdwWhDs6nSSZ --- ...PausePointNotCapturableParameterFixture.cs | 9 ++++ ...ePausePointNotCapturableParametersTests.cs | 43 +++++++++++++++++++ .../PausePoints/UloopPausePointSnapshot.cs | 6 ++- 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs b/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs index 9c3daac423..257211ebdf 100644 --- a/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs +++ b/Assets/Tests/Editor/SourcePausePointNotCapturableParameterFixture.cs @@ -20,6 +20,15 @@ public int Combine( 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) diff --git a/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs b/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs index 12bd3230bd..1de2322582 100644 --- a/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs +++ b/Assets/Tests/Editor/SourcePausePointNotCapturableParametersTests.cs @@ -123,6 +123,49 @@ public void CollectNotCapturableParametersFromReflection_SkipFirstParameter_Skip 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. /// diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs b/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs index 5a5ba8f84f..cd98763a39 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs @@ -56,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; @@ -87,7 +91,7 @@ public UloopPausePointSnapshot( CapturedVariablesTruncated = capturedVariablesTruncated; TruncatedVariableNames = truncatedVariableNames ?? Array.Empty(); TruncatedVariableCount = truncatedVariableCount; - NotCapturableVariables = notCapturableVariables ?? Array.Empty(); + NotCapturableVariables = notCapturableVariables; ClearedReason = clearedReason ?? string.Empty; StatusBeforeClear = statusBeforeClear ?? string.Empty; LateHitDiscardedAfterClear = lateHitDiscardedAfterClear; From b700bc44e08ef4da206f63313446178555d75762 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 01:27:02 +0900 Subject: [PATCH 5/6] fix(pause-point): give advice that works for all three not-capturable shapes "Copy the value to a local" only helps a ref/out/in parameter. Copying a pointer or a ref struct produces another pointer or ref struct, which still cannot be boxed, so two thirds of the reported cases were told to do something that does not work. Say to copy the value it refers to into a plain local, with the two shapes that need a specific move named (dereference a pointer, ToArray() a span). The skill reference carries the same advice, so it is reworded to match. Claude-Session: https://claude.ai/code/session_01TxzFzK1W8JCdwWhDs6nSSZ --- .../uloop-pause-point/references/captured-variables.md | 2 +- .../uloop-pause-point/references/captured-variables.md | 2 +- Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs | 3 ++- .../PausePoint/Skill/references/captured-variables.md | 2 +- .../FirstPartyTools/PausePoint/SourcePausePointConstants.cs | 5 +++-- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.agents/skills/uloop-pause-point/references/captured-variables.md b/.agents/skills/uloop-pause-point/references/captured-variables.md index ef55483a37..dfdeed0da2 100644 --- a/.agents/skills/uloop-pause-point/references/captured-variables.md +++ b/.agents/skills/uloop-pause-point/references/captured-variables.md @@ -18,7 +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 it into a local, or arm the line that consumes it with `--snapshot-timing post-line`. +- 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 ef55483a37..dfdeed0da2 100644 --- a/.claude/skills/uloop-pause-point/references/captured-variables.md +++ b/.claude/skills/uloop-pause-point/references/captured-variables.md @@ -18,7 +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 it into a local, or arm the line that consumes it with `--snapshot-timing post-line`. +- 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/PausePointNotCapturableVariablesTests.cs b/Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs index 0596b54d68..42adf0ad8a 100644 --- a/Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs +++ b/Assets/Tests/Editor/PausePointNotCapturableVariablesTests.cs @@ -25,7 +25,8 @@ public sealed class PausePointNotCapturableVariablesTests 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 to a local, or use --snapshot-timing post-line on the line that consumes it."; + + "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. 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 ef55483a37..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,7 +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 it into a local, or arm the line that consumes it with `--snapshot-timing post-line`. +- 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/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index 80c273fab2..c3a19f0709 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -192,8 +192,9 @@ internal static class SourcePausePointConstants // 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 to a local," - + " or use --snapshot-timing post-line on the line that consumes it."; + "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 From 85f7fc0a82c94198f7d4dd6000f803cc332aad8b Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 4 Sep 2026 02:02:16 +0900 Subject: [PATCH 6/6] test(pause-point): pass the not-capturable list from the warnings-channel fixture The warnings-channel tests arrived on main while this branch was open, so their snapshot factory predates the NotCapturableVariables argument and stopped compiling after the rebase. Pass an empty list, matching every other snapshot fixture. Claude-Session: https://claude.ai/code/session_01TxzFzK1W8JCdwWhDs6nSSZ --- Assets/Tests/Editor/PausePointWarningsChannelTests.cs | 1 + 1 file changed, 1 insertion(+) 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,