From aa4bd7cd2361dc0b18f4fa52bf5ec3fe7480dcdf Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 09:17:27 +0900 Subject: [PATCH 1/8] Add IsValueType to resolver DTOs The Harmony transpiler patcher (PR 4) needs to know whether each captured parameter/local is a value type before boxing it into the object[] array passed to Capture; boxing a reference-typed value would be incorrect. Mono.Cecil's TypeReference.IsValueType is already resolved from the ELEMENT_TYPE encoding without needing TypeReference.Resolve() (which throws for external assemblies in the Unity Editor, per the PR 2 generic ref struct fix), so it is available at zero extra cost. --- .../Fixtures/ReferenceTypeLocalFixture.cs | 13 +++++++++++++ .../Fixtures/ReferenceTypeLocalFixture.cs.meta | 11 +++++++++++ .../SourcePausePointResolverTests.cs | 17 +++++++++++++++++ .../PausePoint/SourcePausePointLocalVariable.cs | 4 +++- .../PausePoint/SourcePausePointParameter.cs | 4 +++- .../PausePoint/SourcePausePointResolver.cs | 5 +++-- 6 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 Assets/Tests/Editor/SourcePausePointResolver/Fixtures/ReferenceTypeLocalFixture.cs create mode 100644 Assets/Tests/Editor/SourcePausePointResolver/Fixtures/ReferenceTypeLocalFixture.cs.meta diff --git a/Assets/Tests/Editor/SourcePausePointResolver/Fixtures/ReferenceTypeLocalFixture.cs b/Assets/Tests/Editor/SourcePausePointResolver/Fixtures/ReferenceTypeLocalFixture.cs new file mode 100644 index 000000000..2ce1bd4ec --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointResolver/Fixtures/ReferenceTypeLocalFixture.cs @@ -0,0 +1,13 @@ +// FROZEN FIXTURE: content and line numbers are asserted by SourcePausePointResolverTests. +// Do not reformat or edit this file; add a new fixture file instead. +namespace io.github.hatayama.UnityCliLoop.Tests.SourcePausePointResolverFixtures +{ + internal sealed class ReferenceTypeLocalFixture + { + public string Describe(string label) + { + string message = label + "!"; + return message; + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointResolver/Fixtures/ReferenceTypeLocalFixture.cs.meta b/Assets/Tests/Editor/SourcePausePointResolver/Fixtures/ReferenceTypeLocalFixture.cs.meta new file mode 100644 index 000000000..9f5198126 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointResolver/Fixtures/ReferenceTypeLocalFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 044c759db9b84495bcf8eec0fc50a31b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointResolver/SourcePausePointResolverTests.cs b/Assets/Tests/Editor/SourcePausePointResolver/SourcePausePointResolverTests.cs index 40c0eefce..dc09f3fbe 100644 --- a/Assets/Tests/Editor/SourcePausePointResolver/SourcePausePointResolverTests.cs +++ b/Assets/Tests/Editor/SourcePausePointResolver/SourcePausePointResolverTests.cs @@ -28,8 +28,25 @@ public void Resolve_NormalMethod_ResolvesLineWithLocalsAndParameters() Assert.That(result.Resolution.IsStatic, Is.False); Assert.That(result.Resolution.Locals.Select(l => l.Name), Is.EquivalentTo(new[] { "sum" })); Assert.That(result.Resolution.Locals.Single().TypeName, Is.EqualTo("System.Int32")); + Assert.That(result.Resolution.Locals.Single().IsValueType, Is.True); Assert.That(result.Resolution.Parameters.Select(p => p.Name), Is.EqualTo(new[] { "left", "right" })); Assert.That(result.Resolution.Parameters.Select(p => p.TypeName), Is.EqualTo(new[] { "System.Int32", "System.Int32" })); + Assert.That(result.Resolution.Parameters.Select(p => p.IsValueType), Is.EqualTo(new[] { true, true })); + } + + [Test] + public void Resolve_ReferenceTypeLocalMethod_ReportsIsValueTypeFalse() + { + // Verifies a string (reference type) local/parameter is reported with IsValueType=false, + // the counterpart to the int (value type) assertions in Resolve_NormalMethod above. + SourcePausePointResolveResult result = SourcePausePointResolver.Resolve( + FixturesDirectory + "ReferenceTypeLocalFixture.cs", 9); + + Assert.That(result.Success, Is.True); + Assert.That(result.Resolution.Locals.Select(l => l.Name), Is.EquivalentTo(new[] { "message" })); + Assert.That(result.Resolution.Locals.Single().IsValueType, Is.False); + Assert.That(result.Resolution.Parameters.Select(p => p.Name), Is.EqualTo(new[] { "label" })); + Assert.That(result.Resolution.Parameters.Single().IsValueType, Is.False); } [Test] diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointLocalVariable.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointLocalVariable.cs index 342a19f0f..8baa2abd5 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointLocalVariable.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointLocalVariable.cs @@ -8,12 +8,14 @@ internal sealed class SourcePausePointLocalVariable public string Name { get; } public int SlotIndex { get; } public string TypeName { get; } + public bool IsValueType { get; } - public SourcePausePointLocalVariable(string name, int slotIndex, string typeName) + public SourcePausePointLocalVariable(string name, int slotIndex, string typeName, bool isValueType) { Name = name; SlotIndex = slotIndex; TypeName = typeName; + IsValueType = isValueType; } } } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointParameter.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointParameter.cs index b2f155413..8643bbb03 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointParameter.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointParameter.cs @@ -8,12 +8,14 @@ internal sealed class SourcePausePointParameter public string Name { get; } public int Index { get; } public string TypeName { get; } + public bool IsValueType { get; } - public SourcePausePointParameter(string name, int index, string typeName) + public SourcePausePointParameter(string name, int index, string typeName, bool isValueType) { Name = name; Index = index; TypeName = typeName; + IsValueType = isValueType; } } } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs index eb3238547..adc9de0d2 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs @@ -267,7 +267,7 @@ private static void AppendLocalIfCapturable( return; } - results.Add(new SourcePausePointLocalVariable(name, slotIndex, variableType.FullName)); + results.Add(new SourcePausePointLocalVariable(name, slotIndex, variableType.FullName, variableType.IsValueType)); } private static bool IsCaptureExcluded(TypeReference type) @@ -329,7 +329,8 @@ private static List CollectParameters(MethodDefinitio continue; } - parameters.Add(new SourcePausePointParameter(parameter.Name, parameter.Index, parameter.ParameterType.FullName)); + parameters.Add(new SourcePausePointParameter( + parameter.Name, parameter.Index, parameter.ParameterType.FullName, parameter.ParameterType.IsValueType)); } return parameters; From b1a313708a340e2f858c8dff337e3c5648da040d Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 09:35:28 +0900 Subject: [PATCH 2/8] Add Harmony transpiler patcher for source pause points Implements SourcePausePointPatcher, injecting a call to SourcePausePointCapture.Capture at a resolved instruction index via a Harmony transpiler and tracking patches per method so multiple pause points can share one method and be added/removed independently. The call site relies on Harmony/MonoMod's skip-visibility DynamicMethod to reach the internal Capture method across assembly boundaries, so no InternalsVisibleTo widening was needed beyond the three test assemblies. Value-type `this` is dereferenced and boxed explicitly (ldobj+box) since ldarg.0 yields a managed pointer for struct instance methods. Displaced branch-target labels and exception-region blocks are moved onto the new first instruction of an injected sequence so loops and try/finally regions keep executing correctly after patching. Covers five method shapes end-to-end (instance, static, struct instance, loop, try/finally) proving the resolved IL argument/local indices line up against the real compiled assemblies. --- .../Tests/Editor/SourcePausePointPatcher.meta | 8 + .../SourcePausePointPatcher/Fixtures.meta | 8 + .../Fixtures/PatcherLoopMethodFixture.cs | 17 + .../Fixtures/PatcherLoopMethodFixture.cs.meta | 11 + .../Fixtures/PatcherNormalMethodFixture.cs | 15 + .../PatcherNormalMethodFixture.cs.meta | 11 + .../Fixtures/PatcherStaticMethodFixture.cs | 13 + .../PatcherStaticMethodFixture.cs.meta | 11 + .../PatcherStructInstanceMethodFixture.cs | 15 + ...PatcherStructInstanceMethodFixture.cs.meta | 11 + .../PatcherTryFinallyMethodFixture.cs | 24 ++ .../PatcherTryFinallyMethodFixture.cs.meta | 11 + .../SourcePausePointPatcherTests.cs | 176 +++++++++++ .../SourcePausePointPatcherTests.cs.meta | 11 + ...ests.Editor.SourcePausePointPatcher.asmdef | 21 ++ ...Editor.SourcePausePointPatcher.asmdef.meta | 7 + .../PausePoint/AssemblyInfo.cs | 1 + .../PausePoint/SourcePausePointConstants.cs | 9 + .../SourcePausePointPatchFailureReason.cs | 15 + ...SourcePausePointPatchFailureReason.cs.meta | 11 + .../SourcePausePointPatchInjection.cs | 34 ++ .../SourcePausePointPatchInjection.cs.meta | 11 + .../PausePoint/SourcePausePointPatchResult.cs | 32 ++ .../SourcePausePointPatchResult.cs.meta | 11 + .../PausePoint/SourcePausePointPatcher.cs | 290 ++++++++++++++++++ .../SourcePausePointPatcher.cs.meta | 11 + .../src/Runtime/PausePoints/AssemblyInfo.cs | 1 + 27 files changed, 796 insertions(+) create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher.meta create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures.meta create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLoopMethodFixture.cs create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLoopMethodFixture.cs.meta create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherNormalMethodFixture.cs create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherNormalMethodFixture.cs.meta create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStaticMethodFixture.cs create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStaticMethodFixture.cs.meta create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStructInstanceMethodFixture.cs create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStructInstanceMethodFixture.cs.meta create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherTryFinallyMethodFixture.cs create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherTryFinallyMethodFixture.cs.meta create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs.meta create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/UnityCLILoop.Tests.Editor.SourcePausePointPatcher.asmdef create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/UnityCLILoop.Tests.Editor.SourcePausePointPatcher.asmdef.meta create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchInjection.cs create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchInjection.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs.meta diff --git a/Assets/Tests/Editor/SourcePausePointPatcher.meta b/Assets/Tests/Editor/SourcePausePointPatcher.meta new file mode 100644 index 000000000..576a89151 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: d00ee049258040938dd8016d55ebf134 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures.meta b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures.meta new file mode 100644 index 000000000..caae881bd --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: 991529f89d0c4383b309968d96439bbb +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLoopMethodFixture.cs b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLoopMethodFixture.cs new file mode 100644 index 000000000..b32f32d2c --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLoopMethodFixture.cs @@ -0,0 +1,17 @@ +// 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 PatcherLoopMethodFixture + { + public static int SumUpTo(int count) + { + int total = 0; + for (int i = 0; i < count; i++) + { + total += i; + } + return total; + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLoopMethodFixture.cs.meta b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLoopMethodFixture.cs.meta new file mode 100644 index 000000000..7fd443716 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherLoopMethodFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 73af9b49824e4930aa6307b67be70308 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherNormalMethodFixture.cs b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherNormalMethodFixture.cs new file mode 100644 index 000000000..db33e7699 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherNormalMethodFixture.cs @@ -0,0 +1,15 @@ +// 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 sealed class PatcherNormalMethodFixture + { + public string Tag = "fixture-instance"; + + public int Add(int left, int right) + { + int sum = left + right; + return sum; + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherNormalMethodFixture.cs.meta b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherNormalMethodFixture.cs.meta new file mode 100644 index 000000000..6c45a6e85 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherNormalMethodFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 54454a144d994ed6ad1a90e5c7f34862 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStaticMethodFixture.cs b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStaticMethodFixture.cs new file mode 100644 index 000000000..a2aaca65a --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStaticMethodFixture.cs @@ -0,0 +1,13 @@ +// 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 PatcherStaticMethodFixture + { + public static int Add(int left, int right) + { + int sum = left + right; + return sum; + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStaticMethodFixture.cs.meta b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStaticMethodFixture.cs.meta new file mode 100644 index 000000000..52c6c607b --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStaticMethodFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 1c696cf18d9f49278a82e5bf3feb0438 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStructInstanceMethodFixture.cs b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStructInstanceMethodFixture.cs new file mode 100644 index 000000000..1116dc292 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStructInstanceMethodFixture.cs @@ -0,0 +1,15 @@ +// 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 struct PatcherStructInstanceMethodFixture + { + public int Value; + + public int Double() + { + int doubled = Value * 2; + return doubled; + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStructInstanceMethodFixture.cs.meta b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStructInstanceMethodFixture.cs.meta new file mode 100644 index 000000000..b7312d946 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherStructInstanceMethodFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: d64b676bfa954416a7bf07586036073e +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherTryFinallyMethodFixture.cs b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherTryFinallyMethodFixture.cs new file mode 100644 index 000000000..4ad48798b --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherTryFinallyMethodFixture.cs @@ -0,0 +1,24 @@ +// 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; + +namespace io.github.hatayama.UnityCliLoop.Tests.SourcePausePointPatcherFixtures +{ + internal static class PatcherTryFinallyMethodFixture + { + public static int Divide(int numerator, int denominator) + { + int result; + try + { + result = numerator / denominator; + } + finally + { + GC.KeepAlive(denominator); + } + + return result; + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherTryFinallyMethodFixture.cs.meta b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherTryFinallyMethodFixture.cs.meta new file mode 100644 index 000000000..71c852840 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherTryFinallyMethodFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 61863cc5216547cf820dd2690907a888 +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 new file mode 100644 index 000000000..5c9883dbb --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs @@ -0,0 +1,176 @@ +using System; +using System.Linq; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; +using io.github.hatayama.UnityCliLoop.Runtime; +using io.github.hatayama.UnityCliLoop.Tests.SourcePausePointPatcherFixtures; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies the Resolver -> Patcher -> Capture -> Registry pipeline end-to-end against real + /// compiled fixture methods, proving the resolved instruction index and IL argument/local + /// indexing line up correctly across instance, static, value-type, loop, and try/finally shapes. + /// + [TestFixture] + public sealed class SourcePausePointPatcherTests + { + private const string FixturesDirectory = "Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/"; + + private FakePausePointPauseController _pauseController; + + [SetUp] + public void SetUp() + { + _pauseController = new FakePausePointPauseController(); + UloopPausePointRegistry.ConfigureForTests(_pauseController, () => DateTime.UtcNow); + } + + [TearDown] + public void TearDown() + { + SourcePausePointPatcher.UnpatchAll(); + UloopPausePointRegistry.ResetForTests(); + } + + [Test] + public void Patch_InstanceMethod_CapturesLocalsParametersAndInstanceFieldOnHit() + { + // Verifies the base case: an instance method's parameters, its as-yet-unassigned local, + // and its declaring instance's own field are all captured at the resolved statement. + const string id = "patcher-normal-method"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherNormalMethodFixture.cs", 11); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + Assert.That(patchResult.Success, Is.True); + + PatcherNormalMethodFixture fixture = new(); + int sum = fixture.Add(2, 3); + + Assert.That(sum, Is.EqualTo(5)); + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(id); + Assert.That(snapshot.IsHit, Is.True); + Assert.That(snapshot.CapturedVariables.Select(v => v.Name), Is.EquivalentTo(new[] { "left", "right", "sum", "Tag" })); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "left").Value, Is.EqualTo("2")); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "right").Value, Is.EqualTo("3")); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "sum").Value, Is.EqualTo("0")); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "Tag").Value, Is.EqualTo("fixture-instance")); + } + + [Test] + public void Patch_StaticMethod_PassesNullInstanceAndUsesUnshiftedArgumentIndices() + { + // Verifies static-method IL argument indexing has no `this`-slot offset, unlike the instance case above. + const string id = "patcher-static-method"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherStaticMethodFixture.cs", 9); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + Assert.That(patchResult.Success, Is.True); + + int sum = PatcherStaticMethodFixture.Add(10, 20); + + Assert.That(sum, Is.EqualTo(30)); + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(id); + Assert.That(snapshot.IsHit, Is.True); + Assert.That(snapshot.CapturedVariables.Select(v => v.Name), Is.EquivalentTo(new[] { "left", "right", "sum" })); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "left").Value, Is.EqualTo("10")); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "right").Value, Is.EqualTo("20")); + } + + [Test] + public void Patch_StructInstanceMethod_BoxesValueTypeThisAndCapturesInstanceField() + { + // Verifies the ldobj+box path used when `this` is a value type: ldarg.0 yields a managed + // pointer for a struct instance method, which must be dereferenced and boxed before Capture. + const string id = "patcher-struct-instance-method"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherStructInstanceMethodFixture.cs", 11); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + Assert.That(patchResult.Success, Is.True); + + PatcherStructInstanceMethodFixture fixture = new() { Value = 7 }; + int doubled = fixture.Double(); + + Assert.That(doubled, Is.EqualTo(14)); + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(id); + Assert.That(snapshot.IsHit, Is.True); + Assert.That(snapshot.CapturedVariables.Select(v => v.Name), Is.EquivalentTo(new[] { "doubled", "Value" })); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "Value").Value, Is.EqualTo("7")); + } + + [Test] + public void Patch_LoopMethod_PreservesBackEdgeBranchTargetAndCapturesFirstIterationState() + { + // Verifies CodeInstruction.labels are moved to the injected sequence's first instruction + // when the insertion point is a loop's back-edge branch target, so the loop still runs + // correctly; the pause point auto-disarms after its first hit, so only i=0/total=0 is seen. + const string id = "patcher-loop-method"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherLoopMethodFixture.cs", 12); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + Assert.That(patchResult.Success, Is.True); + + int total = PatcherLoopMethodFixture.SumUpTo(4); + + Assert.That(total, Is.EqualTo(6)); + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(id); + Assert.That(snapshot.IsHit, Is.True); + Assert.That(snapshot.HitCount, Is.EqualTo(1)); + Assert.That(snapshot.CapturedVariables.Select(v => v.Name), Is.EquivalentTo(new[] { "i", "total", "count" })); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "i").Value, Is.EqualTo("0")); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "total").Value, Is.EqualTo("0")); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "count").Value, Is.EqualTo("4")); + } + + [Test] + public void Patch_TryFinallyMethod_PreservesExceptionRegionBoundaryAndExecutesNormally() + { + // Verifies CodeInstruction.blocks are moved to the injected sequence's first instruction + // when the insertion point is the first instruction inside a try block, so the exception + // region still starts in the right place and the patched method still executes correctly. + const string id = "patcher-try-finally-method"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherTryFinallyMethodFixture.cs", 14); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + Assert.That(patchResult.Success, Is.True); + + int result = PatcherTryFinallyMethodFixture.Divide(10, 2); + + Assert.That(result, Is.EqualTo(5)); + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(id); + Assert.That(snapshot.IsHit, Is.True); + Assert.That(snapshot.CapturedVariables.Select(v => v.Name), Is.EquivalentTo(new[] { "numerator", "denominator", "result" })); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "numerator").Value, Is.EqualTo("10")); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "denominator").Value, Is.EqualTo("2")); + } + + private sealed class FakePausePointPauseController : IUloopPausePointPauseController + { + public int PauseCount { get; private set; } + public bool IsPlaying => true; + public bool IsPaused => PauseCount > 0; + + public void Pause() + { + PauseCount++; + } + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs.meta b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs.meta new file mode 100644 index 000000000..852856b6e --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ca70e0b7c5d7f4b1cb6c87e3b1f03cde +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/UnityCLILoop.Tests.Editor.SourcePausePointPatcher.asmdef b/Assets/Tests/Editor/SourcePausePointPatcher/UnityCLILoop.Tests.Editor.SourcePausePointPatcher.asmdef new file mode 100644 index 000000000..7f048fec9 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/UnityCLILoop.Tests.Editor.SourcePausePointPatcher.asmdef @@ -0,0 +1,21 @@ +{ + "name": "UnityCLILoop.Tests.Editor.SourcePausePointPatcher", + "rootNamespace": "io.github.hatayama.UnityCliLoop.Tests.Editor", + "references": [ + "GUID:0acc523941302664db1f4e527237feb3", + "GUID:27619889b8ba8c24980f49ee34dbb44a", + "GUID:94d8abc693f543a691a4645a5ff42e5c", + "GUID:527f26a36b5043c2bd4d4036d04cd76d" + ], + "includePlatforms": [ + "Editor" + ], + "excludePlatforms": [], + "allowUnsafeCode": false, + "overrideReferences": false, + "precompiledReferences": [], + "autoReferenced": false, + "defineConstraints": [], + "versionDefines": [], + "noEngineReferences": false +} diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/UnityCLILoop.Tests.Editor.SourcePausePointPatcher.asmdef.meta b/Assets/Tests/Editor/SourcePausePointPatcher/UnityCLILoop.Tests.Editor.SourcePausePointPatcher.asmdef.meta new file mode 100644 index 000000000..ed7c7f3b4 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/UnityCLILoop.Tests.Editor.SourcePausePointPatcher.asmdef.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: 54aa3034a82b43219a8eb1b1e3023e74 +AssemblyDefinitionImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs index fbf602cab..7c0a946fb 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/AssemblyInfo.cs @@ -2,3 +2,4 @@ [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointResolver")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointCapture")] +[assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointPatcher")] diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index a9c5400a2..4e93bfeec 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -14,5 +14,14 @@ internal static class SourcePausePointConstants // pause-point evidence to stay skimmable, mirroring the truncation-by-cap pattern MatchingLogs uses. public const int MaxCapturedVariableCount = 50; public const int MaxCapturedVariableValueLength = 256; + + public const string HarmonyId = "io.github.hatayama.uloop.source-pause-point"; + public const string BurstCompileAttributeFullName = "Unity.Burst.BurstCompileAttribute"; + + // 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 = + "This method cannot be safely patched by file:line. Add UloopPausePoint.Pause(\"id\") " + + "directly in the source instead, then arm it with enable-pause-point --id \"id\"."; } } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs new file mode 100644 index 000000000..e523398ae --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs @@ -0,0 +1,15 @@ +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Why a source pause point could not be patched into a resolved method. + /// + internal enum SourcePausePointPatchFailureReason + { + None, + AssemblyNotLoaded, + UnpatchableAbstract, + UnpatchableExtern, + UnpatchableOpenGeneric, + UnpatchableBurstCompiled, + } +} diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs.meta b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs.meta new file mode 100644 index 000000000..8b2da85af --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7a9ecd826a8f4bde8b1c160483e4a9df +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchInjection.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchInjection.cs new file mode 100644 index 000000000..ede77bf74 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchInjection.cs @@ -0,0 +1,34 @@ +using System.Collections.Generic; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// One armed pause point's worth of context needed to emit its Capture call site, keyed + /// by the method it was patched into (a method may hold several of these at once). + /// + internal sealed class SourcePausePointPatchInjection + { + public string Id { get; } + public int InstructionIndex { get; } + public bool IsStatic { get; } + public bool IsDeclaringTypeValueType { get; } + public IReadOnlyList Parameters { get; } + public IReadOnlyList Locals { get; } + + public SourcePausePointPatchInjection( + string id, + int instructionIndex, + bool isStatic, + bool isDeclaringTypeValueType, + IReadOnlyList parameters, + IReadOnlyList locals) + { + Id = id; + InstructionIndex = instructionIndex; + IsStatic = isStatic; + IsDeclaringTypeValueType = isDeclaringTypeValueType; + Parameters = parameters; + Locals = locals; + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchInjection.cs.meta b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchInjection.cs.meta new file mode 100644 index 000000000..f5e73f47e --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchInjection.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 463f32ed34ee4c5b8a6ea5321f2ad1c9 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs new file mode 100644 index 000000000..8fff0c343 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs @@ -0,0 +1,32 @@ +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Outcome of a patch attempt. + /// + internal sealed class SourcePausePointPatchResult + { + public bool Success { get; } + public SourcePausePointPatchFailureReason FailureReason { get; } + public string ErrorMessage { get; } + public string Hint { get; } + + private SourcePausePointPatchResult( + bool success, SourcePausePointPatchFailureReason failureReason, string errorMessage, string hint) + { + Success = success; + FailureReason = failureReason; + ErrorMessage = errorMessage; + Hint = hint; + } + + public static SourcePausePointPatchResult SuccessResult() + { + return new SourcePausePointPatchResult(true, SourcePausePointPatchFailureReason.None, string.Empty, string.Empty); + } + + public static SourcePausePointPatchResult Failure(SourcePausePointPatchFailureReason reason, string errorMessage, string hint) + { + return new SourcePausePointPatchResult(false, reason, errorMessage, hint); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs.meta b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs.meta new file mode 100644 index 000000000..efb287952 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4e14d55bde11469f812cded5ffa14c99 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs new file mode 100644 index 000000000..bd4a838d5 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs @@ -0,0 +1,290 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Reflection; +using System.Reflection.Emit; + +using HarmonyLib; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Injects a call to at a resolved instruction + /// index via a Harmony transpiler, and tracks the resulting patches so they can be removed. + /// The injected IL calls an internal method across assembly boundaries; this works because + /// Harmony/MonoMod builds the replacement method as a skip-visibility DynamicMethod, so the + /// CLR's normal accessibility check for the call site never runs (verified in PR 4's design + /// discussion against the vendored Harmony's own private-nested-class smoke test). + /// + internal static class SourcePausePointPatcher + { + private static readonly Harmony HarmonyInstance = new(SourcePausePointConstants.HarmonyId); + private static readonly MethodInfo TranspilerMethodInfo = + typeof(SourcePausePointPatcher).GetMethod(nameof(Transpiler), BindingFlags.NonPublic | BindingFlags.Static); + private static readonly MethodInfo CaptureMethodInfo = + typeof(SourcePausePointCapture).GetMethod(nameof(SourcePausePointCapture.Capture)); + + private static readonly Dictionary> InjectionsByMethod = new(); + private static readonly Dictionary MethodById = new(); + + public static SourcePausePointPatchResult Patch(string id, SourcePausePointResolution resolution) + { + Debug.Assert(!string.IsNullOrEmpty(id), "id must not be null or empty."); + Debug.Assert(resolution != null, "resolution must not be null."); + + if (MethodById.ContainsKey(id)) + { + // Re-enabling an id that is already patched is a no-op here: the call site is + // already in the IL and always fires, gated only by the registry's armed state. + return SourcePausePointPatchResult.SuccessResult(); + } + + SourcePausePointPatchResult resolveResult = TryResolveMethod(resolution, out MethodBase method); + if (!resolveResult.Success) + { + return resolveResult; + } + + SourcePausePointPatchResult patchabilityResult = CheckPatchable(method); + if (!patchabilityResult.Success) + { + return patchabilityResult; + } + + SourcePausePointPatchInjection injection = new( + id, + resolution.InstructionIndex, + resolution.IsStatic, + resolution.IsDeclaringTypeValueType, + resolution.Parameters, + resolution.Locals); + + bool methodAlreadyPatched = InjectionsByMethod.TryGetValue(method, out List injections); + if (!methodAlreadyPatched) + { + injections = new List(); + InjectionsByMethod[method] = injections; + } + + injections.Add(injection); + MethodById[id] = method; + + if (methodAlreadyPatched) + { + // Harmony only regenerates a method's replacement when Patch/Unpatch is called; + // re-declaring the same transpiler risks double-registration, so drop and redo it + // to force a clean rebuild from the (untouched) original IL against the new injection set. + HarmonyInstance.Unpatch(method, HarmonyPatchType.Transpiler, SourcePausePointConstants.HarmonyId); + } + HarmonyInstance.Patch(method, transpiler: new HarmonyMethod(TranspilerMethodInfo)); + + return SourcePausePointPatchResult.SuccessResult(); + } + + public static void Unpatch(string id) + { + Debug.Assert(!string.IsNullOrEmpty(id), "id must not be null or empty."); + + if (!MethodById.TryGetValue(id, out MethodBase method)) + { + return; + } + MethodById.Remove(id); + + List injections = InjectionsByMethod[method]; + injections.RemoveAll(injection => injection.Id == id); + + HarmonyInstance.Unpatch(method, HarmonyPatchType.Transpiler, SourcePausePointConstants.HarmonyId); + if (injections.Count == 0) + { + InjectionsByMethod.Remove(method); + } + else + { + HarmonyInstance.Patch(method, transpiler: new HarmonyMethod(TranspilerMethodInfo)); + } + } + + public static void UnpatchAll() + { + HarmonyInstance.UnpatchAll(SourcePausePointConstants.HarmonyId); + InjectionsByMethod.Clear(); + MethodById.Clear(); + } + + private static SourcePausePointPatchResult TryResolveMethod(SourcePausePointResolution resolution, out MethodBase method) + { + method = null; + foreach (Assembly assembly in AppDomain.CurrentDomain.GetAssemblies()) + { + if (assembly.GetName().Name != resolution.AssemblyName) + { + continue; + } + + method = assembly.ManifestModule.ResolveMethod(resolution.MetadataToken); + return SourcePausePointPatchResult.SuccessResult(); + } + + return SourcePausePointPatchResult.Failure( + SourcePausePointPatchFailureReason.AssemblyNotLoaded, + $"Assembly '{resolution.AssemblyName}' is not currently loaded in the AppDomain.", + SourcePausePointConstants.ManualMarkerFallbackHint); + } + + private static SourcePausePointPatchResult CheckPatchable(MethodBase method) + { + if (method.IsAbstract) + { + return SourcePausePointPatchResult.Failure( + SourcePausePointPatchFailureReason.UnpatchableAbstract, + $"'{method}' is abstract and has no method body to patch.", + SourcePausePointConstants.ManualMarkerFallbackHint); + } + + if (method.GetMethodBody() == null) + { + return SourcePausePointPatchResult.Failure( + SourcePausePointPatchFailureReason.UnpatchableExtern, + $"'{method}' has no IL method body (extern or an internal call) and cannot be patched.", + SourcePausePointConstants.ManualMarkerFallbackHint); + } + + if (method.ContainsGenericParameters) + { + return SourcePausePointPatchResult.Failure( + SourcePausePointPatchFailureReason.UnpatchableOpenGeneric, + $"'{method}' is declared with unresolved generic type parameters and cannot be safely patched.", + SourcePausePointConstants.ManualMarkerFallbackHint); + } + + if (HasBurstCompileAttribute(method) || HasBurstCompileAttribute(method.DeclaringType)) + { + return SourcePausePointPatchResult.Failure( + SourcePausePointPatchFailureReason.UnpatchableBurstCompiled, + $"'{method}' (or its declaring type) is marked [BurstCompile] and cannot be patched.", + SourcePausePointConstants.ManualMarkerFallbackHint); + } + + return SourcePausePointPatchResult.SuccessResult(); + } + + private static bool HasBurstCompileAttribute(MemberInfo member) + { + foreach (object attribute in member.GetCustomAttributes(inherit: false)) + { + if (attribute.GetType().FullName == SourcePausePointConstants.BurstCompileAttributeFullName) + { + return true; + } + } + + return false; + } + + private static IEnumerable Transpiler(IEnumerable instructions, MethodBase original) + { + List list = new(instructions); + if (!InjectionsByMethod.TryGetValue(original, out List injections)) + { + return list; + } + + foreach (SourcePausePointPatchInjection injection in injections.OrderByDescending(i => i.InstructionIndex)) + { + List emitted = BuildInjection(injection, original); + + // The instruction we insert before may be a branch target or a try/catch/finally + // boundary marker; both must move to the new first instruction so control flow and + // exception regions still land in the same place, now with our prefix folded in. + CodeInstruction displaced = list[injection.InstructionIndex]; + emitted[0].labels.AddRange(displaced.labels); + displaced.labels.Clear(); + emitted[0].blocks.AddRange(displaced.blocks); + displaced.blocks.Clear(); + + list.InsertRange(injection.InstructionIndex, emitted); + } + + return list; + } + + private static List BuildInjection(SourcePausePointPatchInjection injection, MethodBase method) + { + List emitted = new() { new CodeInstruction(OpCodes.Ldstr, injection.Id) }; + + AppendInstanceLoad(emitted, injection, method); + + ParameterInfo[] runtimeParameters = method.GetParameters(); + int argOffset = injection.IsStatic ? 0 : 1; + AppendNameValueArray( + emitted, + injection.Parameters, + p => CodeInstruction.LoadArgument(p.Index + argOffset, false), + p => p.IsValueType ? runtimeParameters[p.Index].ParameterType : null, + p => p.Name); + + IList runtimeLocals = method.GetMethodBody().LocalVariables; + AppendNameValueArray( + emitted, + injection.Locals, + l => CodeInstruction.LoadLocal(l.SlotIndex, false), + l => l.IsValueType ? runtimeLocals[l.SlotIndex].LocalType : null, + l => l.Name); + + emitted.Add(new CodeInstruction(OpCodes.Call, CaptureMethodInfo)); + return emitted; + } + + private static void AppendInstanceLoad(List emitted, SourcePausePointPatchInjection injection, MethodBase method) + { + if (injection.IsStatic) + { + emitted.Add(new CodeInstruction(OpCodes.Ldnull)); + return; + } + + emitted.Add(CodeInstruction.LoadArgument(0, false)); + if (injection.IsDeclaringTypeValueType) + { + // ldarg.0 on a value-type instance method yields a managed pointer (this is how + // the CLR always passes "this" for struct instance methods); Capture takes + // `object`, so it must be dereferenced to the value and boxed explicitly. + emitted.Add(new CodeInstruction(OpCodes.Ldobj, method.DeclaringType)); + emitted.Add(new CodeInstruction(OpCodes.Box, method.DeclaringType)); + } + } + + private static void AppendNameValueArray( + List emitted, + IReadOnlyList items, + Func loadValue, + Func boxTypeOrNull, + Func nameOf) + { + emitted.Add(new CodeInstruction(OpCodes.Ldc_I4, items.Count * 2)); + emitted.Add(new CodeInstruction(OpCodes.Newarr, typeof(object))); + + for (int i = 0; i < items.Count; i++) + { + T item = items[i]; + + emitted.Add(new CodeInstruction(OpCodes.Dup)); + emitted.Add(new CodeInstruction(OpCodes.Ldc_I4, i * 2)); + emitted.Add(new CodeInstruction(OpCodes.Ldstr, nameOf(item))); + emitted.Add(new CodeInstruction(OpCodes.Stelem_Ref)); + + emitted.Add(new CodeInstruction(OpCodes.Dup)); + emitted.Add(new CodeInstruction(OpCodes.Ldc_I4, i * 2 + 1)); + emitted.Add(loadValue(item)); + Type boxType = boxTypeOrNull(item); + if (boxType != null) + { + emitted.Add(new CodeInstruction(OpCodes.Box, boxType)); + } + emitted.Add(new CodeInstruction(OpCodes.Stelem_Ref)); + } + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs.meta b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs.meta new file mode 100644 index 000000000..c6116efac --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 1f0b0954c14e4097aeaf5a55998f09cd +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Runtime/PausePoints/AssemblyInfo.cs b/Packages/src/Runtime/PausePoints/AssemblyInfo.cs index 08b99d4ab..b97c1c72a 100644 --- a/Packages/src/Runtime/PausePoints/AssemblyInfo.cs +++ b/Packages/src/Runtime/PausePoints/AssemblyInfo.cs @@ -7,3 +7,4 @@ [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.PlayMode")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointCapture")] +[assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor.SourcePausePointPatcher")] From 9c34f1d4e8ebbab96b5daa20f7fee378549f6515 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 09:38:22 +0900 Subject: [PATCH 3/8] Test the patch ledger's multi-injection, idempotency, and unpatch paths Adds coverage for the parts of SourcePausePointPatcher's ledger that weren't exercised by the single-injection acceptance tests: two pause points landing in the same method independently, re-patching an already-patched id as a documented no-op, and Unpatch restoring a method's original (uninstrumented) behavior before a later re-patch re-injects the call site. --- .../SourcePausePointPatcherTests.cs | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs index 5c9883dbb..a5b2a5d35 100644 --- a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs +++ b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs @@ -161,6 +161,84 @@ public void Patch_TryFinallyMethod_PreservesExceptionRegionBoundaryAndExecutesNo Assert.That(snapshot.CapturedVariables.First(v => v.Name == "denominator").Value, Is.EqualTo("2")); } + [Test] + public void Patch_TwoPausePointsInSameMethod_BothHitIndependentlyWithCorrectState() + { + // Verifies multiple injections into the same method insert correctly regardless of + // instruction order, each capturing the local's value at its own point in execution. + const string idBeforeAssignment = "patcher-multi-before-assignment"; + const string idBeforeReturn = "patcher-multi-before-return"; + + SourcePausePointResolveResult beforeAssignment = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherNormalMethodFixture.cs", 11); + SourcePausePointResolveResult beforeReturn = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherNormalMethodFixture.cs", 12); + Assert.That(beforeAssignment.Success, Is.True); + Assert.That(beforeReturn.Success, Is.True); + + UloopPausePointRegistry.Enable(idBeforeAssignment, 30); + UloopPausePointRegistry.Enable(idBeforeReturn, 30); + Assert.That(SourcePausePointPatcher.Patch(idBeforeAssignment, beforeAssignment.Resolution).Success, Is.True); + Assert.That(SourcePausePointPatcher.Patch(idBeforeReturn, beforeReturn.Resolution).Success, Is.True); + + PatcherNormalMethodFixture fixture = new(); + int sum = fixture.Add(2, 3); + + Assert.That(sum, Is.EqualTo(5)); + UloopPausePointSnapshot beforeAssignmentSnapshot = UloopPausePointRegistry.GetStatus(idBeforeAssignment); + UloopPausePointSnapshot beforeReturnSnapshot = UloopPausePointRegistry.GetStatus(idBeforeReturn); + Assert.That(beforeAssignmentSnapshot.IsHit, Is.True); + Assert.That(beforeAssignmentSnapshot.CapturedVariables.First(v => v.Name == "sum").Value, Is.EqualTo("0")); + Assert.That(beforeReturnSnapshot.IsHit, Is.True); + Assert.That(beforeReturnSnapshot.CapturedVariables.First(v => v.Name == "sum").Value, Is.EqualTo("5")); + } + + [Test] + public void Patch_SameIdPatchedTwice_IsIdempotentAndStillHits() + { + // Verifies re-patching the same already-patched id is a no-op per Patch's documented + // contract, and the already-injected call site still fires correctly. + const string id = "patcher-idempotent"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherStaticMethodFixture.cs", 9); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + Assert.That(SourcePausePointPatcher.Patch(id, resolveResult.Resolution).Success, Is.True); + Assert.That(SourcePausePointPatcher.Patch(id, resolveResult.Resolution).Success, Is.True); + + int sum = PatcherStaticMethodFixture.Add(1, 1); + + Assert.That(sum, Is.EqualTo(2)); + Assert.That(UloopPausePointRegistry.GetStatus(id).IsHit, Is.True); + } + + [Test] + public void Unpatch_ThenRepatch_RestoresOriginalBehaviorThenCapturesAgain() + { + // Verifies Unpatch removes the injected call site (no more capture/hit) and a + // subsequent Patch with the same id re-injects it correctly. + const string id = "patcher-unpatch-repatch"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherStaticMethodFixture.cs", 9); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + Assert.That(SourcePausePointPatcher.Patch(id, resolveResult.Resolution).Success, Is.True); + SourcePausePointPatcher.Unpatch(id); + + int sumWhileUnpatched = PatcherStaticMethodFixture.Add(4, 5); + Assert.That(sumWhileUnpatched, Is.EqualTo(9)); + Assert.That(UloopPausePointRegistry.GetStatus(id).IsHit, Is.False); + + UloopPausePointRegistry.Enable(id, 30); + Assert.That(SourcePausePointPatcher.Patch(id, resolveResult.Resolution).Success, Is.True); + int sumAfterRepatch = PatcherStaticMethodFixture.Add(6, 7); + + Assert.That(sumAfterRepatch, Is.EqualTo(13)); + Assert.That(UloopPausePointRegistry.GetStatus(id).IsHit, Is.True); + } + private sealed class FakePausePointPauseController : IUloopPausePointPauseController { public int PauseCount { get; private set; } From 31386e6a2b11c8ce267f94a2e87298f4c124d1dd Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 09:41:55 +0900 Subject: [PATCH 4/8] Reject unpatchable methods with a manual marker fallback hint Adds coverage for SourcePausePointPatcher.CheckPatchable's four gates: abstract methods, extern/internal-call methods (no IL body), methods with an unbound generic parameter of their own or declared inside an open generic type, and methods marked (or declared inside a type marked) [BurstCompile]. Each case is confirmed to fail before Harmony is ever invoked and to carry the manual-marker fallback hint. A test-only shadow of Unity.Burst.BurstCompileAttribute is defined locally since this project does not depend on com.unity.burst; SourcePausePointPatcher only ever compares the attribute's FullName. --- .../SourcePausePointPatcherTests.cs | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs index a5b2a5d35..a6d4b66b9 100644 --- a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs +++ b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Reflection; using NUnit.Framework; @@ -239,6 +240,134 @@ public void Unpatch_ThenRepatch_RestoresOriginalBehaviorThenCapturesAgain() Assert.That(UloopPausePointRegistry.GetStatus(id).IsHit, Is.True); } + [Test] + public void Patch_AbstractMethod_ReturnsUnpatchableAbstractFailure() + { + // Verifies an abstract method (no method body to patch) is rejected before ever calling Harmony.Patch. + MethodBase method = typeof(AbstractMethodFixture).GetMethod(nameof(AbstractMethodFixture.DoWork)); + SourcePausePointPatchResult result = SourcePausePointPatcher.Patch( + "patcher-abstract-method", BuildSyntheticResolution(method)); + + Assert.That(result.Success, Is.False); + Assert.That(result.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.UnpatchableAbstract)); + Assert.That(result.Hint, Is.Not.Empty); + } + + [Test] + public void Patch_ExternMethod_ReturnsUnpatchableExternFailure() + { + // Verifies a method with no IL body (an internal call, the same shape a DllImport extern + // method has) is rejected. + MethodBase method = typeof(object).GetMethod("MemberwiseClone", BindingFlags.NonPublic | BindingFlags.Instance); + SourcePausePointPatchResult result = SourcePausePointPatcher.Patch( + "patcher-extern-method", BuildSyntheticResolution(method)); + + Assert.That(result.Success, Is.False); + Assert.That(result.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.UnpatchableExtern)); + } + + [Test] + public void Patch_OpenGenericMethod_ReturnsUnpatchableOpenGenericFailure() + { + // Verifies a method with an unbound generic type parameter of its own is rejected. + MethodBase method = typeof(GenericMethodFixture).GetMethod(nameof(GenericMethodFixture.DoWork)); + SourcePausePointPatchResult result = SourcePausePointPatcher.Patch( + "patcher-open-generic-method", BuildSyntheticResolution(method)); + + Assert.That(result.Success, Is.False); + Assert.That(result.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.UnpatchableOpenGeneric)); + } + + [Test] + public void Patch_NonGenericMethodInsideOpenGenericType_ReturnsUnpatchableOpenGenericFailure() + { + // Verifies a plain (non-generic) method declared inside an open generic type is also + // rejected, since its declaring type's unbound T makes it just as unsafe to patch. + MethodBase method = typeof(GenericTypeFixture<>).GetMethod(nameof(GenericTypeFixture.PlainMethod)); + SourcePausePointPatchResult result = SourcePausePointPatcher.Patch( + "patcher-open-generic-type", BuildSyntheticResolution(method)); + + Assert.That(result.Success, Is.False); + Assert.That(result.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.UnpatchableOpenGeneric)); + } + + [Test] + public void Patch_BurstCompiledMethod_ReturnsUnpatchableBurstCompiledFailure() + { + // Verifies a method itself marked [BurstCompile] is rejected. + MethodBase method = typeof(BurstMethodFixture).GetMethod(nameof(BurstMethodFixture.DoWork)); + SourcePausePointPatchResult result = SourcePausePointPatcher.Patch( + "patcher-burst-method", BuildSyntheticResolution(method)); + + Assert.That(result.Success, Is.False); + Assert.That(result.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.UnpatchableBurstCompiled)); + } + + [Test] + public void Patch_MethodInsideBurstCompiledType_ReturnsUnpatchableBurstCompiledFailure() + { + // Verifies a plain method whose declaring type is marked [BurstCompile] is also rejected, + // mirroring how Unity's Burst-compiled job structs place the attribute on the struct, not the method. + MethodBase method = typeof(BurstTypeFixture).GetMethod(nameof(BurstTypeFixture.Execute)); + SourcePausePointPatchResult result = SourcePausePointPatcher.Patch( + "patcher-burst-type", BuildSyntheticResolution(method)); + + Assert.That(result.Success, Is.False); + Assert.That(result.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.UnpatchableBurstCompiled)); + } + + // Builds a resolution good enough to reach SourcePausePointPatcher's patchability gate; the + // instruction index and locals/parameters are never read because every case here fails before that. + private static SourcePausePointResolution BuildSyntheticResolution(MethodBase method) + { + return new SourcePausePointResolution( + method.Module.Assembly.GetName().Name, + method.MetadataToken, + method.ToString(), + method.IsStatic, + method.DeclaringType.IsValueType, + 0, + 0, + 1, + Array.Empty(), + Array.Empty()); + } + + private abstract class AbstractMethodFixture + { + public abstract void DoWork(); + } + + private static class GenericMethodFixture + { + public static void DoWork(T value) + { + } + } + + private static class GenericTypeFixture + { + public static void PlainMethod() + { + } + } + + private static class BurstMethodFixture + { + [Unity.Burst.BurstCompile] + public static void DoWork() + { + } + } + + [Unity.Burst.BurstCompile] + private struct BurstTypeFixture + { + public static void Execute() + { + } + } + private sealed class FakePausePointPauseController : IUloopPausePointPauseController { public int PauseCount { get; private set; } @@ -252,3 +381,12 @@ public void Pause() } } } + +namespace Unity.Burst +{ + // Test-only shadow of Unity.Burst.BurstCompileAttribute's FullName: this project does not + // depend on com.unity.burst, and SourcePausePointPatcher only ever compares the FullName string. + internal sealed class BurstCompileAttribute : Attribute + { + } +} From 8c45ae3ec3283708f93044d3a23ddb1c04972125 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 10:11:05 +0900 Subject: [PATCH 5/8] Guard injected pause-point capture behind an IsArmed check in IL Fable 5's PR review flagged that the injected transpiler prologue built and boxed the parameter/local array unconditionally on every call, only relying on SourcePausePointCapture's own IsArmed check afterward. Since that check ran after the allocation-heavy IL had already executed, an inactive (not-yet-armed) pause point still paid the cost on every hit. Move the IsArmed check into the injected IL itself, ahead of the array build, with a new branch label that skips straight to the original instruction when not armed. --- .../SourcePausePointPatcherTests.cs | 21 +++++++++++ .../PausePoint/SourcePausePointPatcher.cs | 35 ++++++++++++++++--- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs index a6d4b66b9..6462dee7d 100644 --- a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs +++ b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs @@ -162,6 +162,27 @@ public void Patch_TryFinallyMethod_PreservesExceptionRegionBoundaryAndExecutesNo Assert.That(snapshot.CapturedVariables.First(v => v.Name == "denominator").Value, Is.EqualTo("2")); } + [Test] + public void Patch_NeverArmedId_SkipsCaptureArrayBuildAndExecutesNormally() + { + // Verifies the not-armed guard: when Patch is called without ever Enable-ing the id, + // the injected IsArmed check short-circuits straight to the original instruction + // (skipping the parameter/local array build and the Capture call entirely) and the + // method's own result is unaffected. + const string id = "patcher-never-armed"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherStaticMethodFixture.cs", 9); + Assert.That(resolveResult.Success, Is.True); + + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + Assert.That(patchResult.Success, Is.True); + + int sum = PatcherStaticMethodFixture.Add(4, 5); + + Assert.That(sum, Is.EqualTo(9)); + Assert.That(UloopPausePointRegistry.GetStatus(id).IsHit, Is.False); + } + [Test] public void Patch_TwoPausePointsInSameMethod_BothHitIndependentlyWithCorrectState() { diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs index bd4a838d5..6f802804a 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs @@ -1,11 +1,13 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Reflection; using System.Reflection.Emit; using HarmonyLib; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Runtime; namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { @@ -24,6 +26,8 @@ internal static class SourcePausePointPatcher typeof(SourcePausePointPatcher).GetMethod(nameof(Transpiler), BindingFlags.NonPublic | BindingFlags.Static); private static readonly MethodInfo CaptureMethodInfo = typeof(SourcePausePointCapture).GetMethod(nameof(SourcePausePointCapture.Capture)); + private static readonly MethodInfo IsArmedMethodInfo = + typeof(UloopPausePointRegistry).GetMethod(nameof(UloopPausePointRegistry.IsArmed)); private static readonly Dictionary> InjectionsByMethod = new(); private static readonly Dictionary MethodById = new(); @@ -183,7 +187,8 @@ private static bool HasBurstCompileAttribute(MemberInfo member) return false; } - private static IEnumerable Transpiler(IEnumerable instructions, MethodBase original) + private static IEnumerable Transpiler( + IEnumerable instructions, ILGenerator generator, MethodBase original) { List list = new(instructions); if (!InjectionsByMethod.TryGetValue(original, out List injections)) @@ -193,7 +198,8 @@ private static IEnumerable Transpiler(IEnumerable i.InstructionIndex)) { - List emitted = BuildInjection(injection, original); + Label skip = generator.DefineLabel(); + List emitted = BuildInjection(injection, original, skip); // The instruction we insert before may be a branch target or a try/catch/finally // boundary marker; both must move to the new first instruction so control flow and @@ -204,15 +210,28 @@ private static IEnumerable Transpiler(IEnumerable BuildInjection(SourcePausePointPatchInjection injection, MethodBase method) + private static List BuildInjection(SourcePausePointPatchInjection injection, MethodBase method, Label skip) { - List emitted = new() { new CodeInstruction(OpCodes.Ldstr, injection.Id) }; + // Checking IsArmed before building the parameter/local object array (rather than + // relying on Capture's own check) keeps the overwhelmingly common not-armed hit + // allocation-free: the array-build and boxing instructions below never execute unless armed. + List emitted = new() + { + new CodeInstruction(OpCodes.Ldstr, injection.Id), + new CodeInstruction(OpCodes.Call, IsArmedMethodInfo), + new CodeInstruction(OpCodes.Brfalse, skip), + new CodeInstruction(OpCodes.Ldstr, injection.Id), + }; AppendInstanceLoad(emitted, injection, method); @@ -226,6 +245,12 @@ private static List BuildInjection(SourcePausePointPatchInjecti p => p.Name); IList runtimeLocals = method.GetMethodBody().LocalVariables; + foreach (SourcePausePointLocalVariable local in injection.Locals) + { + Debug.Assert( + runtimeLocals[local.SlotIndex].LocalIndex == local.SlotIndex, + "LocalVariableInfo order must match the resolved slot index."); + } AppendNameValueArray( emitted, injection.Locals, From 1806b26617190b3a56de24972b4917f4d37e63d9 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 10:11:40 +0900 Subject: [PATCH 6/8] Reject stale pause-point resolutions via MVID mismatch check Fable 5's PR review flagged that TryResolveMethod trusted a resolved metadata token against whatever assembly currently matched by name, with no check that it was still the same compiled assembly the resolution was taken from. After a recompile/domain reload, a stale token could resolve to a different method than intended. Add an Mvid field to SourcePausePointResolution (populated from Cecil's ModuleDefinition.Mvid) and compare it against the loaded assembly's ModuleVersionId before resolving, failing with a new StaleAssembly reason when they diverge. Also addresses the review's should-items: switch SourcePausePointPatcher's asserts to UnityEngine.Debug.Assert to match the rest of the PausePoint files, add a contract check that local slot order lines up with the resolved slot index, give AssemblyNotLoaded its own hint text instead of reusing the manual-marker fallback message, and cover the try/finally exception-region end boundary with a new test. --- .../SourcePausePointPatcherTests.cs | 49 +++++++++++++++++++ .../PausePoint/SourcePausePointConstants.cs | 14 ++++++ .../SourcePausePointPatchFailureReason.cs | 1 + .../PausePoint/SourcePausePointPatcher.cs | 11 ++++- .../PausePoint/SourcePausePointResolution.cs | 3 ++ .../PausePoint/SourcePausePointResolver.cs | 1 + 6 files changed, 78 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs index 6462dee7d..69c02f2e6 100644 --- a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs +++ b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs @@ -183,6 +183,31 @@ public void Patch_NeverArmedId_SkipsCaptureArrayBuildAndExecutesNormally() Assert.That(UloopPausePointRegistry.GetStatus(id).IsHit, Is.False); } + [Test] + public void Patch_TryFinallyMethod_AtExceptionRegionEndBoundary_ExecutesNormally() + { + // Verifies the displaced instruction case at the opposite exception-region boundary + // from the existing try/finally test above: line 21 is the first instruction after + // the whole try/finally construct, so it carries an end-of-region block marker + // rather than a begin marker, and must move to the injected sequence the same way. + const string id = "patcher-try-finally-end-boundary"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherTryFinallyMethodFixture.cs", 21); + Assert.That(resolveResult.Success, Is.True); + + UloopPausePointRegistry.Enable(id, 30); + SourcePausePointPatchResult patchResult = SourcePausePointPatcher.Patch(id, resolveResult.Resolution); + Assert.That(patchResult.Success, Is.True); + + int result = PatcherTryFinallyMethodFixture.Divide(10, 2); + + Assert.That(result, Is.EqualTo(5)); + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(id); + Assert.That(snapshot.IsHit, Is.True); + Assert.That(snapshot.CapturedVariables.Select(v => v.Name), Is.EquivalentTo(new[] { "numerator", "denominator", "result" })); + Assert.That(snapshot.CapturedVariables.First(v => v.Name == "result").Value, Is.EqualTo("5")); + } + [Test] public void Patch_TwoPausePointsInSameMethod_BothHitIndependentlyWithCorrectState() { @@ -337,12 +362,36 @@ public void Patch_MethodInsideBurstCompiledType_ReturnsUnpatchableBurstCompiledF Assert.That(result.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.UnpatchableBurstCompiled)); } + [Test] + public void Patch_ResolutionWithStaleMvid_ReturnsStaleAssemblyFailure() + { + // Verifies a resolution taken from a since-recompiled assembly (simulated here by a + // deliberately wrong Mvid) is rejected before ResolveMethod ever runs against a + // metadata token that may no longer mean the same thing in the now-loaded assembly. + MethodBase method = typeof(PatcherStaticMethodFixture).GetMethod(nameof(PatcherStaticMethodFixture.Add)); + SourcePausePointResolution staleResolution = BuildSyntheticResolutionWithMvid(method, Guid.NewGuid().ToString()); + + SourcePausePointPatchResult result = SourcePausePointPatcher.Patch("patcher-stale-assembly", staleResolution); + + Assert.That(result.Success, Is.False); + Assert.That(result.FailureReason, Is.EqualTo(SourcePausePointPatchFailureReason.StaleAssembly)); + Assert.That(result.Hint, Is.Not.Empty); + } + // Builds a resolution good enough to reach SourcePausePointPatcher's patchability gate; the // instruction index and locals/parameters are never read because every case here fails before that. private static SourcePausePointResolution BuildSyntheticResolution(MethodBase method) + { + return BuildSyntheticResolutionWithMvid(method, method.Module.ModuleVersionId.ToString()); + } + + // Same as BuildSyntheticResolution but lets a test supply a deliberately wrong Mvid, to + // exercise the stale-assembly gate without needing a second compiled assembly. + private static SourcePausePointResolution BuildSyntheticResolutionWithMvid(MethodBase method, string mvid) { return new SourcePausePointResolution( method.Module.Assembly.GetName().Name, + mvid, method.MetadataToken, method.ToString(), method.IsStatic, diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index 4e93bfeec..1a0f194f5 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -23,5 +23,19 @@ internal static class SourcePausePointConstants public const string ManualMarkerFallbackHint = "This method cannot be safely patched by file:line. Add UloopPausePoint.Pause(\"id\") " + "directly in the source instead, then arm it with enable-pause-point --id \"id\"."; + + // The manual-marker fallback would not run here either: it lives in a source file that + // belongs to the very assembly that is not loaded yet. + public const string AssemblyNotLoadedHint = + "The assembly this pause point resolves to is not currently loaded in this AppDomain. " + + "Ensure the code path that loads it (e.g. entering Play Mode) has run, then retry."; + + // A stale resolution means the assembly was recompiled after Resolve ran; re-resolving + // against the current compiled output (rather than falling back to a manual marker) is + // the correct next step here. + public const string StaleAssemblyHint = + "The loaded assembly no longer matches the compiled assembly this pause point was " + + "resolved from (a script compile or domain reload may have happened since). Wait for " + + "compilation/domain reload to finish, then resolve and patch again."; } } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs index e523398ae..b34f6c3da 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchFailureReason.cs @@ -7,6 +7,7 @@ internal enum SourcePausePointPatchFailureReason { None, AssemblyNotLoaded, + StaleAssembly, UnpatchableAbstract, UnpatchableExtern, UnpatchableOpenGeneric, diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs index 6f802804a..e4d026312 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs @@ -127,6 +127,15 @@ private static SourcePausePointPatchResult TryResolveMethod(SourcePausePointReso continue; } + if (assembly.ManifestModule.ModuleVersionId.ToString() != resolution.Mvid) + { + return SourcePausePointPatchResult.Failure( + SourcePausePointPatchFailureReason.StaleAssembly, + $"The loaded assembly '{resolution.AssemblyName}' no longer matches the " + + "compiled assembly this resolution was taken from.", + SourcePausePointConstants.StaleAssemblyHint); + } + method = assembly.ManifestModule.ResolveMethod(resolution.MetadataToken); return SourcePausePointPatchResult.SuccessResult(); } @@ -134,7 +143,7 @@ private static SourcePausePointPatchResult TryResolveMethod(SourcePausePointReso return SourcePausePointPatchResult.Failure( SourcePausePointPatchFailureReason.AssemblyNotLoaded, $"Assembly '{resolution.AssemblyName}' is not currently loaded in the AppDomain.", - SourcePausePointConstants.ManualMarkerFallbackHint); + SourcePausePointConstants.AssemblyNotLoadedHint); } private static SourcePausePointPatchResult CheckPatchable(MethodBase method) diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolution.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolution.cs index 843219d9f..60656af10 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolution.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolution.cs @@ -9,6 +9,7 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools internal sealed class SourcePausePointResolution { public string AssemblyName { get; } + public string Mvid { get; } public int MetadataToken { get; } public string MethodDisplayName { get; } public bool IsStatic { get; } @@ -21,6 +22,7 @@ internal sealed class SourcePausePointResolution public SourcePausePointResolution( string assemblyName, + string mvid, int metadataToken, string methodDisplayName, bool isStatic, @@ -32,6 +34,7 @@ public SourcePausePointResolution( IReadOnlyList parameters) { AssemblyName = assemblyName; + Mvid = mvid; MetadataToken = metadataToken; MethodDisplayName = methodDisplayName; IsStatic = isStatic; diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs index adc9de0d2..271a6d9ad 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointResolver.cs @@ -98,6 +98,7 @@ private static SourcePausePointResolveResult ResolveFromCompiledAssembly( SourcePausePointResolution resolution = new SourcePausePointResolution( assemblyName, + assemblyDefinition.MainModule.Mvid.ToString(), method.MetadataToken.ToInt32(), method.FullName, method.IsStatic, From ef0f1ce7075800e92a83ddc1edddf3d97b0f923f Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 10:34:24 +0900 Subject: [PATCH 7/8] Add try-finally ledger compensation to Patch/Unpatch failure paths If Harmony's Patch/Unpatch call throws mid-call (e.g. building invalid IL for a byref-like `this`), the in-memory ledger (InjectionsByMethod, MethodById) was already updated to claim success before the Harmony call ran. A later caller would then see a false "already patched" state and silently miss the pause point, or Unpatch would leave other ids on a method with no transpiler actually attached. Wrap both call sites in try-finally (not try-catch, per this repo's Fail Fast policy) so the ledger only commits when Harmony's call actually completes; on failure the finally block rolls the ledger back to its honest pre-call shape while letting the original exception propagate unchanged. --- .../PausePoint/SourcePausePointPatcher.cs | 57 ++++++++++++++++--- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs index e4d026312..a8d1a2f66 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs @@ -74,14 +74,38 @@ public static SourcePausePointPatchResult Patch(string id, SourcePausePointResol injections.Add(injection); MethodById[id] = method; - if (methodAlreadyPatched) + // The ledger above is written before Harmony actually rebuilds the method. If + // Unpatch/Patch throws (e.g. a byref-like `this` produces invalid IL at JIT time), a + // plain rethrow would leave this id's ledger entry claiming success while Harmony + // never attached the transpiler; a later caller would then see a false "already + // patched" and silently miss the pause point. try-finally (not catch) restores the + // ledger to its honest pre-call shape while letting the original exception propagate + // unchanged, keeping this Fail Fast rather than swallowing the failure. + bool committed = false; + try { - // Harmony only regenerates a method's replacement when Patch/Unpatch is called; - // re-declaring the same transpiler risks double-registration, so drop and redo it - // to force a clean rebuild from the (untouched) original IL against the new injection set. - HarmonyInstance.Unpatch(method, HarmonyPatchType.Transpiler, SourcePausePointConstants.HarmonyId); + if (methodAlreadyPatched) + { + // Harmony only regenerates a method's replacement when Patch/Unpatch is called; + // re-declaring the same transpiler risks double-registration, so drop and redo it + // to force a clean rebuild from the (untouched) original IL against the new injection set. + HarmonyInstance.Unpatch(method, HarmonyPatchType.Transpiler, SourcePausePointConstants.HarmonyId); + } + HarmonyInstance.Patch(method, transpiler: new HarmonyMethod(TranspilerMethodInfo)); + committed = true; + } + finally + { + if (!committed) + { + injections.Remove(injection); + if (injections.Count == 0) + { + InjectionsByMethod.Remove(method); + } + MethodById.Remove(id); + } } - HarmonyInstance.Patch(method, transpiler: new HarmonyMethod(TranspilerMethodInfo)); return SourcePausePointPatchResult.SuccessResult(); } @@ -103,10 +127,29 @@ public static void Unpatch(string id) if (injections.Count == 0) { InjectionsByMethod.Remove(method); + return; } - else + + // If re-patching the remaining injections throws, Harmony is left with no transpiler + // attached at all, yet the ledger would still claim the other ids are patched. Wipe + // this method's ledger entries entirely on failure so a later call sees the honest + // "not patched" state instead of a silently stale success (see the same reasoning in Patch). + bool committed = false; + try { HarmonyInstance.Patch(method, transpiler: new HarmonyMethod(TranspilerMethodInfo)); + committed = true; + } + finally + { + if (!committed) + { + foreach (string remainingId in injections.Select(remaining => remaining.Id).ToList()) + { + MethodById.Remove(remainingId); + } + InjectionsByMethod.Remove(method); + } } } From 25faa22ca53dfae5b5f9c229b5a18a901bf4f818 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 11 Jul 2026 10:35:27 +0900 Subject: [PATCH 8/8] Degrade byref-like this-instance capture instead of rejecting the patch Boxing a ref struct's `this` is illegal IL, so patching an instance method declared on a ref struct would otherwise crash at Harmony's JIT time. Detect byref-like declaring types (same reflection-based IsByRefLikeAttribute check style as the existing Burst check) and emit a null instance load instead of ldarg.0/ldobj/box, so locals and parameters are still captured even though the instance's own fields cannot be. Add a Warning field to SourcePausePointPatchResult (via a default parameter on the existing SuccessResult, keeping this repo's no- overloads rule) so callers can surface that the instance was not captured. Add a ref-struct fixture and test covering the degraded capture path. --- .../PatcherRefStructInstanceMethodFixture.cs | 15 +++++++++++ ...cherRefStructInstanceMethodFixture.cs.meta | 11 ++++++++ .../SourcePausePointPatcherTests.cs | 27 +++++++++++++++++++ .../PausePoint/SourcePausePointConstants.cs | 6 +++++ .../PausePoint/SourcePausePointPatchResult.cs | 10 ++++--- .../PausePoint/SourcePausePointPatcher.cs | 27 ++++++++++++++++++- 6 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherRefStructInstanceMethodFixture.cs create mode 100644 Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherRefStructInstanceMethodFixture.cs.meta diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherRefStructInstanceMethodFixture.cs b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherRefStructInstanceMethodFixture.cs new file mode 100644 index 000000000..36550aadf --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherRefStructInstanceMethodFixture.cs @@ -0,0 +1,15 @@ +// 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 ref struct PatcherRefStructInstanceMethodFixture + { + public int Value; + + public int Double() + { + int doubled = Value * 2; + return doubled; + } + } +} diff --git a/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherRefStructInstanceMethodFixture.cs.meta b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherRefStructInstanceMethodFixture.cs.meta new file mode 100644 index 000000000..5daa501d6 --- /dev/null +++ b/Assets/Tests/Editor/SourcePausePointPatcher/Fixtures/PatcherRefStructInstanceMethodFixture.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 8655a205d5ab240068f65ec1c31cbe2b +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 69c02f2e6..299c99a74 100644 --- a/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs +++ b/Assets/Tests/Editor/SourcePausePointPatcher/SourcePausePointPatcherTests.cs @@ -110,6 +110,33 @@ public void Patch_StructInstanceMethod_BoxesValueTypeThisAndCapturesInstanceFiel Assert.That(snapshot.CapturedVariables.First(v => v.Name == "Value").Value, Is.EqualTo("7")); } + [Test] + public void Patch_RefStructInstanceMethod_DegradesToNullInstanceAndCapturesLocalsWithWarning() + { + // Verifies the byref-like (ref struct) declaring-type degradation: boxing a ref + // struct's `this` is illegal IL, so the injected instance load must fall back to a + // null instance instead. Locals are still captured normally, the instance field + // ("Value") is absent since there is no boxed instance to read it from, and Patch + // reports a non-empty Warning explaining the degradation. + const string id = "patcher-ref-struct-instance-method"; + SourcePausePointResolveResult resolveResult = SourcePausePointResolver.Resolve( + FixturesDirectory + "PatcherRefStructInstanceMethodFixture.cs", 11); + 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.Not.Empty); + + PatcherRefStructInstanceMethodFixture fixture = new() { Value = 5 }; + int doubled = fixture.Double(); + + Assert.That(doubled, Is.EqualTo(10)); + UloopPausePointSnapshot snapshot = UloopPausePointRegistry.GetStatus(id); + Assert.That(snapshot.IsHit, Is.True); + Assert.That(snapshot.CapturedVariables.Select(v => v.Name), Is.EquivalentTo(new[] { "doubled" })); + } + [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 1a0f194f5..3ba7f752f 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -37,5 +37,11 @@ internal static class SourcePausePointConstants "The loaded assembly no longer matches the compiled assembly this pause point was " + "resolved from (a script compile or domain reload may have happened since). Wait for " + "compilation/domain reload to finish, then resolve and patch again."; + + // A byref-like `this` cannot be boxed, so the patcher degrades to a null instance rather + // than rejecting the patch outright; locals and parameters are still captured normally. + public const string RefStructInstanceNotCapturedWarning = + "The declaring type is a ref struct; this-instance fields are not captured " + + "(locals and parameters are still captured normally)."; } } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs index 8fff0c343..d1472faac 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatchResult.cs @@ -9,24 +9,26 @@ internal sealed class SourcePausePointPatchResult public SourcePausePointPatchFailureReason FailureReason { get; } public string ErrorMessage { get; } public string Hint { get; } + public string Warning { get; } private SourcePausePointPatchResult( - bool success, SourcePausePointPatchFailureReason failureReason, string errorMessage, string hint) + bool success, SourcePausePointPatchFailureReason failureReason, string errorMessage, string hint, string warning) { Success = success; FailureReason = failureReason; ErrorMessage = errorMessage; Hint = hint; + Warning = warning; } - public static SourcePausePointPatchResult SuccessResult() + public static SourcePausePointPatchResult SuccessResult(string warning = "") { - return new SourcePausePointPatchResult(true, SourcePausePointPatchFailureReason.None, string.Empty, string.Empty); + return new SourcePausePointPatchResult(true, SourcePausePointPatchFailureReason.None, string.Empty, string.Empty, warning); } public static SourcePausePointPatchResult Failure(SourcePausePointPatchFailureReason reason, string errorMessage, string hint) { - return new SourcePausePointPatchResult(false, reason, errorMessage, hint); + return new SourcePausePointPatchResult(false, reason, errorMessage, hint, string.Empty); } } } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs index a8d1a2f66..6676f52bc 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointPatcher.cs @@ -107,7 +107,10 @@ public static SourcePausePointPatchResult Patch(string id, SourcePausePointResol } } - return SourcePausePointPatchResult.SuccessResult(); + string warning = !method.IsStatic && IsByRefLikeType(method.DeclaringType) + ? SourcePausePointConstants.RefStructInstanceNotCapturedWarning + : string.Empty; + return SourcePausePointPatchResult.SuccessResult(warning); } public static void Unpatch(string id) @@ -239,6 +242,19 @@ private static bool HasBurstCompileAttribute(MemberInfo member) return false; } + private static bool IsByRefLikeType(Type type) + { + foreach (object attribute in type.GetCustomAttributes(inherit: false)) + { + if (attribute.GetType().FullName == SourcePausePointConstants.IsByRefLikeAttributeFullName) + { + return true; + } + } + + return false; + } + private static IEnumerable Transpiler( IEnumerable instructions, ILGenerator generator, MethodBase original) { @@ -322,6 +338,15 @@ private static void AppendInstanceLoad(List emitted, SourcePaus return; } + if (injection.IsDeclaringTypeValueType && IsByRefLikeType(method.DeclaringType)) + { + // A byref-like (ref struct) `this` cannot be boxed at all (illegal IL); degrade to + // a null instance so locals and parameters can still be captured, rather than + // rejecting the whole patch over the instance fields alone. + emitted.Add(new CodeInstruction(OpCodes.Ldnull)); + return; + } + emitted.Add(CodeInstruction.LoadArgument(0, false)); if (injection.IsDeclaringTypeValueType) {