From b82b9663a0bb959734aa1330c7b7f0449b33fd14 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 21 Jul 2026 00:27:31 +0900 Subject: [PATCH 1/4] fix: add JIT-inlining hint to pause-point timeout guidance Mono can inline very small target methods into their callers, making a source pause point never fire even though the target line runs. Extend pausePointTimeoutHint's HitCount=0 branch with a hint pointing users at this cause so they can move the pause point into the calling method. --- .../internal/projectrunner/pause_point_errors.go | 3 ++- .../internal/projectrunner/pause_point_wait_test.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors.go b/cli/project-runner/internal/projectrunner/pause_point_errors.go index 06942097f0..75810fca73 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors.go @@ -79,7 +79,8 @@ func pausePointTimeoutHint(response pausePointStatusResponse) string { } if response.HitCount == 0 && response.Status == pausePointStatusEnabled { return "Marker was enabled but never hit. Confirm the id matches UloopPausePoint.Pause(\"\") and that the code path was executed. In fast-progressing games the state may have already moved past the marker (for example back to Ready or GameOver), so re-trigger the code path and wait again. " + - "If the marker targets a Unity message method such as OnCollisionEnter2D/OnTriggerEnter2D, check whether `enable-pause-point`'s response carried a Warning about cached message dispatch: Unity can resolve a GameObject's message dispatch before the marker patch is installed, so a GameObject that already existed at enable time may never reach the marker even though the method body runs. Recreating the GameObject after enabling, or embedding UloopPausePoint.Pause(\"id\") directly in the method body, avoids this." + "If the marker targets a Unity message method such as OnCollisionEnter2D/OnTriggerEnter2D, check whether `enable-pause-point`'s response carried a Warning about cached message dispatch: Unity can resolve a GameObject's message dispatch before the marker patch is installed, so a GameObject that already existed at enable time may never reach the marker even though the method body runs. Recreating the GameObject after enabling, or embedding UloopPausePoint.Pause(\"id\") directly in the method body, avoids this. " + + "If the target line is inside a very small method, Mono's JIT may have inlined it into callers and the pause point never fires; move the pause point into the calling method." } return "" } diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go index a97b1f5aa2..b166178395 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go @@ -917,7 +917,8 @@ func TestPausePointTimeoutErrorIncludesDiagnosisHint(t *testing.T) { HitCount: 0, }, wantHint: "Marker was enabled but never hit. Confirm the id matches UloopPausePoint.Pause(\"\") and that the code path was executed. In fast-progressing games the state may have already moved past the marker (for example back to Ready or GameOver), so re-trigger the code path and wait again. " + - "If the marker targets a Unity message method such as OnCollisionEnter2D/OnTriggerEnter2D, check whether `enable-pause-point`'s response carried a Warning about cached message dispatch: Unity can resolve a GameObject's message dispatch before the marker patch is installed, so a GameObject that already existed at enable time may never reach the marker even though the method body runs. Recreating the GameObject after enabling, or embedding UloopPausePoint.Pause(\"id\") directly in the method body, avoids this.", + "If the marker targets a Unity message method such as OnCollisionEnter2D/OnTriggerEnter2D, check whether `enable-pause-point`'s response carried a Warning about cached message dispatch: Unity can resolve a GameObject's message dispatch before the marker patch is installed, so a GameObject that already existed at enable time may never reach the marker even though the method body runs. Recreating the GameObject after enabling, or embedding UloopPausePoint.Pause(\"id\") directly in the method body, avoids this. " + + "If the target line is inside a very small method, Mono's JIT may have inlined it into callers and the pause point never fires; move the pause point into the calling method.", }, } From 716af2cf623fbdcb34b2f3758a1b9b6ada863513 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 21 Jul 2026 00:27:43 +0900 Subject: [PATCH 2/4] fix: surface JIT-inlining risk warning at pause-point patch time Small method bodies (or ones marked AggressiveInlining) can be inlined by Mono's JIT into their callers, silently defeating a source pause point even though the target line executes. Detect this at patch time via IL body size (heuristic threshold) and the AggressiveInlining implementation flag, and surface a warning through BuildPatchWarning so the risk is visible before a confusing HitCount=0 timeout. Existing physical-callback-warning tests are loosened from exact-match to Contains/Does-Not-Contain since their fixture methods are also small enough to independently trigger the new inlining warning. New tests cover the below-threshold, above-threshold, and AggressiveInlining cases plus the joined dual-warning ordering. --- .../PatcherAggressiveInliningMethodFixture.cs | 29 +++++ ...herAggressiveInliningMethodFixture.cs.meta | 11 ++ .../Fixtures/PatcherLargeMethodFixture.cs | 26 +++++ .../PatcherLargeMethodFixture.cs.meta | 11 ++ .../SourcePausePointPatcherTests.cs | 107 +++++++++++++++++- .../PausePoint/SourcePausePointConstants.cs | 11 ++ .../PausePoint/SourcePausePointPatcher.cs | 23 ++++ 7 files changed, 214 insertions(+), 4 deletions(-) create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherAggressiveInliningMethodFixture.cs create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherAggressiveInliningMethodFixture.cs.meta create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLargeMethodFixture.cs create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLargeMethodFixture.cs.meta diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherAggressiveInliningMethodFixture.cs b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherAggressiveInliningMethodFixture.cs new file mode 100644 index 0000000000..c98da0a575 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherAggressiveInliningMethodFixture.cs @@ -0,0 +1,29 @@ +// FROZEN FIXTURE: content and line numbers are asserted by SourcePausePointPatcherTests. +// Do not reformat or edit this file; add a new fixture file instead. +using System.Runtime.CompilerServices; + +namespace io.github.hatayama.UnityCliLoop.Tests.SourcePausePointPatcherFixtures +{ + internal static class PatcherAggressiveInliningMethodFixture + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static int Classify(int value) + { + int result; + if (value > 100) + { + result = value * 2; + } + else if (value > 10) + { + result = value + 5; + } + else + { + result = value - 1; + } + + return result; + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherAggressiveInliningMethodFixture.cs.meta b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherAggressiveInliningMethodFixture.cs.meta new file mode 100644 index 0000000000..f03d6e8a7b --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherAggressiveInliningMethodFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 58327c02ce6da452aa68743f64911393 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLargeMethodFixture.cs b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLargeMethodFixture.cs new file mode 100644 index 0000000000..50ed86a58b --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLargeMethodFixture.cs @@ -0,0 +1,26 @@ +// FROZEN FIXTURE: content and line numbers are asserted by SourcePausePointPatcherTests. +// Do not reformat or edit this file; add a new fixture file instead. +namespace io.github.hatayama.UnityCliLoop.Tests.SourcePausePointPatcherFixtures +{ + internal static class PatcherLargeMethodFixture + { + public static int Classify(int value) + { + int result; + if (value > 100) + { + result = value * 2; + } + else if (value > 10) + { + result = value + 5; + } + else + { + result = value - 1; + } + + return result; + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLargeMethodFixture.cs.meta b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLargeMethodFixture.cs.meta new file mode 100644 index 0000000000..60c213e02f --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLargeMethodFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 360c530c3d7184de9ba24bab69769e92 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs index f9e56ed389..266d9ebb1e 100644 --- a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs +++ b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs @@ -145,7 +145,10 @@ public void Patch_PhysicsMessageMethodOnMonoBehaviour_ReturnsCachedDispatchWarni // Verifies OnCollisionEnter2D on a MonoBehaviour-derived type reports the informational // warning about Unity's physics message dispatch caching its call path independently of // this patch (see To-Do 2 investigation: an already-existing GameObject's collision - // callback can miss the pause point even though the method body runs). + // callback can miss the pause point even though the method body runs). This fixture's + // body is also small enough to additionally trigger the inlining-risk warning (see + // Patch_PhysicsMessageMethodOnMonoBehaviour_AlsoTriggersInliningWarning for the exact + // joined string), so this test only pins the physical-callback warning's presence. const string id = "patcher-physical-callback-method"; SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( FixturesDirectory + "PatcherPhysicalCallbackMethodFixture.cs", 13); @@ -155,7 +158,29 @@ public void Patch_PhysicsMessageMethodOnMonoBehaviour_ReturnsCachedDispatchWarni SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); Assert.That(patchResult.Success, Is.True); - Assert.That(patchResult.Warning, Is.EqualTo(SourcePausePointConstants.PhysicalCallbackMayMissExistingInstanceWarning)); + Assert.That(patchResult.Warning, Does.Contain(SourcePausePointConstants.PhysicalCallbackMayMissExistingInstanceWarning)); + } + + [Test] + public void Patch_PhysicsMessageMethodOnMonoBehaviour_AlsoTriggersInliningWarning() + { + // Verifies that when both the physical-callback warning and the small-body + // inlining-risk warning apply to the same method (OnCollisionEnter2D's body is only + // 16 IL bytes, well under SmallMethodInliningRiskThresholdBytes), BuildPatchWarning + // joins them in the same order as the checks appear in its source (physical-callback + // first, then inlining-risk), space-separated. + const string id = "patcher-physical-callback-method-dual-warning"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherPhysicalCallbackMethodFixture.cs", 13); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + + Assert.That(patchResult.Success, Is.True); + Assert.That(patchResult.Warning, Is.EqualTo( + SourcePausePointConstants.PhysicalCallbackMayMissExistingInstanceWarning + " " + + SourcePausePointConstants.SmallMethodInliningRiskWarning)); } [Test] @@ -163,6 +188,8 @@ public void Patch_OrdinaryMessageMethodOnSameMonoBehaviour_DoesNotReturnCachedDi { // Verifies Update() on the same MonoBehaviour fixture does not trigger the // physics-message warning, since only physics message methods carry the caching risk. + // Update()'s body is also small (16 IL bytes), so the inlining-risk warning is still + // present; this test only asserts the physical-callback warning's absence. const string id = "patcher-ordinary-message-method"; SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( FixturesDirectory + "PatcherPhysicalCallbackMethodFixture.cs", 18); @@ -172,7 +199,7 @@ public void Patch_OrdinaryMessageMethodOnSameMonoBehaviour_DoesNotReturnCachedDi SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); Assert.That(patchResult.Success, Is.True); - Assert.That(patchResult.Warning, Is.Empty); + Assert.That(patchResult.Warning, Does.Not.Contain(SourcePausePointConstants.PhysicalCallbackMayMissExistingInstanceWarning)); } [Test] @@ -180,7 +207,9 @@ public void Patch_PhysicsNamedMethodOnNonMonoBehaviourType_DoesNotReturnCachedDi { // Verifies the warning requires both a matching method name AND a MonoBehaviour-derived // declaring type, so an unrelated plain class that happens to name a method - // "OnTriggerEnter2D" does not produce a false positive. + // "OnTriggerEnter2D" does not produce a false positive. This fixture's body is also + // small (16 IL bytes), so the inlining-risk warning is still present; this test only + // asserts the physical-callback warning's absence. const string id = "patcher-physics-named-method-plain-class"; SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( FixturesDirectory + "PatcherPhysicsNamedMethodOnPlainClassFixture.cs", 9); @@ -189,10 +218,80 @@ public void Patch_PhysicsNamedMethodOnNonMonoBehaviourType_DoesNotReturnCachedDi UloopPausePointRegistry.Enable(id, 30); SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + Assert.That(patchResult.Success, Is.True); + Assert.That(patchResult.Warning, Does.Not.Contain(SourcePausePointConstants.PhysicalCallbackMayMissExistingInstanceWarning)); + } + + [Test] + public void Patch_SmallMethodBody_ReturnsInliningRiskWarning() + { + // Verifies a method whose IL body is at or under SmallMethodInliningRiskThresholdBytes + // triggers the inlining-risk warning. The precondition assert guards against C# compiler + // version drift silently moving this fixture's IL size to the wrong side of the threshold. + byte[] ilBytes = typeof(PatcherStaticMethodFixture).GetMethod(nameof(PatcherStaticMethodFixture.Add)) + .GetMethodBody().GetILAsByteArray(); + Assert.That(ilBytes.Length, Is.LessThanOrEqualTo(SourcePausePointConstants.SmallMethodInliningRiskThresholdBytes), + "Fixture precondition failed: PatcherStaticMethodFixture.Add must stay at or under the inlining-risk threshold."); + + const string id = "patcher-small-method-body"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherStaticMethodFixture.cs", 10); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + + Assert.That(patchResult.Success, Is.True); + Assert.That(patchResult.Warning, Is.EqualTo(SourcePausePointConstants.SmallMethodInliningRiskWarning)); + } + + [Test] + public void Patch_LargeMethodBody_DoesNotReturnInliningRiskWarning() + { + // Verifies a method whose IL body clearly exceeds SmallMethodInliningRiskThresholdBytes + // (with a safety margin against compiler version drift, rather than sizing the fixture + // to land just past the boundary) does not trigger the inlining-risk warning. + byte[] ilBytes = typeof(PatcherLargeMethodFixture).GetMethod(nameof(PatcherLargeMethodFixture.Classify)) + .GetMethodBody().GetILAsByteArray(); + Assert.That(ilBytes.Length, Is.GreaterThan(SourcePausePointConstants.SmallMethodInliningRiskThresholdBytes + 8), + "Fixture precondition failed: PatcherLargeMethodFixture.Classify must clearly exceed the inlining-risk threshold."); + + const string id = "patcher-large-method-body"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherLargeMethodFixture.cs", 23); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + Assert.That(patchResult.Success, Is.True); Assert.That(patchResult.Warning, Is.Empty); } + [Test] + public void Patch_AggressiveInliningMethod_ReturnsInliningRiskWarningRegardlessOfSize() + { + // Verifies [MethodImpl(MethodImplOptions.AggressiveInlining)] triggers the inlining-risk + // warning independent of IL body size: this fixture's body is identical in shape to + // PatcherLargeMethodFixture (clearly over the threshold), so the warning here can only + // come from the attribute check, not the size check. + byte[] ilBytes = typeof(PatcherAggressiveInliningMethodFixture).GetMethod(nameof(PatcherAggressiveInliningMethodFixture.Classify)) + .GetMethodBody().GetILAsByteArray(); + Assert.That(ilBytes.Length, Is.GreaterThan(SourcePausePointConstants.SmallMethodInliningRiskThresholdBytes + 8), + "Fixture precondition failed: PatcherAggressiveInliningMethodFixture.Classify must clearly exceed the inlining-risk threshold so this test isolates the attribute check."); + + const string id = "patcher-aggressive-inlining-method"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherAggressiveInliningMethodFixture.cs", 26); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + + Assert.That(patchResult.Success, Is.True); + Assert.That(patchResult.Warning, Is.EqualTo(SourcePausePointConstants.SmallMethodInliningRiskWarning)); + } + [Test] public void Patch_LoopMethod_PreservesBackEdgeBranchTargetAndCapturesFirstIterationState() { diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index 4172bd1eab..2141839124 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -21,6 +21,11 @@ internal static class SourcePausePointConstants public const string HarmonyId = "io.github.hatayama.uloop.source-pause-point"; public const string BurstCompileAttributeFullName = "Unity.Burst.BurstCompileAttribute"; + // A heuristic threshold, not a guarantee: Mono's JIT inlining decision depends on far more + // than IL byte count (call-site count, caller size, tiering), so this only flags methods + // small enough that inlining is plausible, to explain a HitCount=0 symptom after the fact. + public const int SmallMethodInliningRiskThresholdBytes = 32; + // The only escape hatch a caller has when a method cannot be patched by file:line: the // hand-written marker path still works and does not depend on IL patching at all. public const string ManualMarkerFallbackHint = @@ -60,6 +65,12 @@ internal static class SourcePausePointConstants + "after enabling this pause point, or embed UloopPausePoint.Pause(\"id\") directly in the " + "method body and arm it with enable-pause-point --id instead."; + // Surfaces the same JIT-inlining risk documented under Requirements & Safety in the skill, + // but at enable time instead of only after a confusing HitCount=0 timeout. + public const string SmallMethodInliningRiskWarning = + "The target method body is very small and may be inlined by Mono's JIT into its callers; " + + "if HitCount stays 0 while the line demonstrably runs, move the pause point into the calling method."; + // Release code optimization strips most sequence points and hoists/elides locals, so the // Resolver's PDB-driven lookup cannot reliably find a patch location; rejecting up front // avoids patching the wrong instruction instead of failing later in a confusing way. diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs index 1c525ff0fe..ba35050102 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs @@ -266,6 +266,11 @@ private static string BuildPatchWarning(MethodBase method) warnings.Add(SourcePausePointConstants.PhysicalCallbackMayMissExistingInstanceWarning); } + if (IsLikelyJitInlined(method)) + { + warnings.Add(SourcePausePointConstants.SmallMethodInliningRiskWarning); + } + return string.Join(" ", warnings); } @@ -282,6 +287,24 @@ private static bool IsByRefLikeType(Type type) return false; } + // Heuristic, not a guarantee: [AggressiveInlining] is only a hint the JIT may ignore, and + // IL body size alone cannot predict Mono's actual inlining decision (call-site count, caller + // size, and tiering all matter too). Both false positives (flagged but never inlined) and + // false negatives (inlined despite exceeding the threshold) are possible; this exists solely + // to explain a HitCount=0 symptom, not to predict it precisely. + // Harmony detours the JIT-compiled native code and never rewrites the metadata IL, so + // measuring after Patch still reads the original method body size. + internal static bool IsLikelyJitInlined(MethodBase method) + { + if ((method.GetMethodImplementationFlags() & MethodImplAttributes.AggressiveInlining) != 0) + { + return true; + } + + byte[] ilBytes = method.GetMethodBody()?.GetILAsByteArray(); + return ilBytes != null && ilBytes.Length <= SourcePausePointConstants.SmallMethodInliningRiskThresholdBytes; + } + private static IEnumerable Transpiler( IEnumerable instructions, ILGenerator generator, MethodBase original) { From 323871430bfd783ed5e753253bc6d3f14ee4e4f7 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 21 Jul 2026 00:30:16 +0900 Subject: [PATCH 3/4] feat: surface pre-line snapshot timing in enable-pause-point response Users have observed captured values that look like they belong to the line after ResolvedLine, since a source pause point's capture happens before the resolved line executes (like an IDE breakpoint). Add a SnapshotTiming field to PausePointResponse, set for file:line markers only, making this timing explicit in the response instead of leaving it documented only in the skill. Additive-only response field, so no protocol version bump is needed. --- Assets/Tests/Editor/PausePointTests.cs | 3 +++ .../Editor/FirstPartyTools/PausePoint/PausePointTools.cs | 2 ++ .../FirstPartyTools/PausePoint/SourcePausePointConstants.cs | 6 ++++++ 3 files changed, 11 insertions(+) diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index bc974ef497..93e5df79d7 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -594,6 +594,8 @@ public async Task Enable_WhenMarkerCreated_ReturnsStateManagementFields() Assert.That(response.RemainingMilliseconds, Is.EqualTo(30000)); Assert.That(response.Generation, Is.EqualTo(1)); Assert.That(response.Expired, Is.False); + // An id-only marker has no resolved source line, so no pre-line timing note applies. + Assert.That(response.SnapshotTiming, Is.Empty); Assert.That(response.EditorState.CapturedAt, Is.EqualTo(UloopPausePointEditorStateCapturedAt.Current)); Assert.That(response.RecommendedNextAction, Is.Empty); } @@ -987,6 +989,7 @@ public async Task Enable_WhenFileAndLineResolveToRealMethod_PatchesAndCapturesVa Assert.That(response.ResolvedLine, Is.EqualTo(FixtureLine)); Assert.That(response.ResolvedLineText, Is.EqualTo("return sum;")); Assert.That(response.ResolvedMethod, Does.Contain("Add")); + Assert.That(response.SnapshotTiming, Is.EqualTo(SourcePausePointConstants.PreLineSnapshotTimingNote)); EnableBySourceLocationFixture fixture = new(); int sum = fixture.Add(2, 3); diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs index c700dc5e18..8ce6eeb3fd 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs @@ -51,6 +51,7 @@ public class PausePointResponse : UnityCliLoopToolResponse public int ResolvedLine { get; set; } public string ResolvedLineText { get; set; } = string.Empty; public string ResolvedMethod { get; set; } = string.Empty; + public string SnapshotTiming { get; set; } = string.Empty; public string Status { get; set; } = string.Empty; public bool IsEnabled { get; set; } public bool IsHit { get; set; } @@ -358,6 +359,7 @@ private static PausePointResponse EnableBySourceLocation(EnablePausePointSchema response.ResolvedLine = resolveResult.Resolution.ResolvedLine; response.ResolvedLineText = ReadResolvedLineText(parameters.File, resolveResult.Resolution.ResolvedLine); response.ResolvedMethod = resolveResult.Resolution.MethodDisplayName; + response.SnapshotTiming = SourcePausePointConstants.PreLineSnapshotTimingNote; response.Warning = MergeWarnings(CreateEnableWarning(), patchResult.Warning); return response; } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index 2141839124..b4ede83c55 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -71,6 +71,12 @@ internal static class SourcePausePointConstants "The target method body is very small and may be inlined by Mono's JIT into its callers; " + "if HitCount stays 0 while the line demonstrably runs, move the pause point into the calling method."; + // Callers have observed captured values that look like they belong to the line after + // ResolvedLine; this makes the pre-line snapshot timing explicit in the response itself + // instead of leaving it documented only in the skill. + public const string PreLineSnapshotTimingNote = + "pre-line: variables are captured before ResolvedLine executes"; + // Release code optimization strips most sequence points and hoists/elides locals, so the // Resolver's PDB-driven lookup cannot reliably find a patch location; rejecting up front // avoids patching the wrong instruction instead of failing later in a confusing way. From e2c4fc9f4cbb5fe73c3b2605fe5f226c458d08c5 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 21 Jul 2026 00:38:41 +0900 Subject: [PATCH 4/4] fix: narrow IsLikelyJitInlined visibility to private Nothing outside BuildPatchWarning (same class) references this helper and no test calls it directly, so internal was unnecessarily broad. --- .../FirstPartyTools/PausePoint/SourcePausePointPatcher.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs index ba35050102..a245b1f52c 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs @@ -294,7 +294,7 @@ private static bool IsByRefLikeType(Type type) // to explain a HitCount=0 symptom, not to predict it precisely. // Harmony detours the JIT-compiled native code and never rewrites the metadata IL, so // measuring after Patch still reads the original method body size. - internal static bool IsLikelyJitInlined(MethodBase method) + private static bool IsLikelyJitInlined(MethodBase method) { if ((method.GetMethodImplementationFlags() & MethodImplAttributes.AggressiveInlining) != 0) {