From 7d84d5637353ebe9da0607a3045ed3c31b2acfe4 Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 20 Aug 2026 13:36:30 +0900 Subject: [PATCH 1/3] fix: disclose line snapping and blank edited lines in pause-point drift warnings A blank edited line at the resolved location used to suppress the entire compiled-line drift warning, and a forward snap never named the requested --line. Agents could not tell that enable had armed a different statement. Co-authored-by: Cursor --- .../PausePointCompiledLineMapWarningTests.cs | 283 +++++++++++++++++- ...ausePointCompiledLineComparisonWarnings.cs | 189 ++++++++++++ ...ointCompiledLineComparisonWarnings.cs.meta | 11 + .../PausePoint/PausePointEnableWarnings.cs | 53 +--- .../PausePoint/PausePointUseCase.cs | 26 +- .../PausePoint/SourcePausePointConstants.cs | 20 ++ 6 files changed, 509 insertions(+), 73 deletions(-) create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs create mode 100644 Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs.meta diff --git a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs index 1488fbd1a0..b59236dc6b 100644 --- a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs +++ b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs @@ -140,11 +140,12 @@ public void BuildCompiledLineMapResolveFailureWarningOrEmpty_WhenPatchesAreInact [Test] public void BuildCompiledLineDriftWarningOrEmpty_WhenTextsDiffer_ReturnsFormattedWarning() { - string warning = PausePointEnableWarnings.BuildCompiledLineDriftWarningOrEmpty( + string warning = PausePointCompiledLineComparisonWarnings.BuildCompiledLineDriftWarningOrEmpty( " return 1; ", "return 2;", ForwardSlashFile, - 17); + 17, + true); Assert.That( warning, @@ -163,37 +164,212 @@ public void BuildCompiledLineDriftWarningOrEmpty_WhenTextsDiffer_ReturnsFormatte [Test] public void BuildCompiledLineDriftWarningOrEmpty_WhenTextsMatchAfterTrim_ReturnsEmpty() { - string warning = PausePointEnableWarnings.BuildCompiledLineDriftWarningOrEmpty( + string warning = PausePointCompiledLineComparisonWarnings.BuildCompiledLineDriftWarningOrEmpty( " return 1; ", "return 1;", ForwardSlashFile, - 17); + 17, + true); Assert.That(warning, Is.EqualTo(string.Empty)); } /// - /// What: a missing compiled or edited line skips the comparison instead of warning. + /// What: a missing compiled line, or a failed edited-line read, skips the comparison. /// [Test] - public void BuildCompiledLineDriftWarningOrEmpty_WhenEitherSideIsEmpty_ReturnsEmpty() + public void BuildCompiledLineDriftWarningOrEmpty_WhenCompiledMissingOrEditedReadFails_ReturnsEmpty() { Assert.That( - PausePointEnableWarnings.BuildCompiledLineDriftWarningOrEmpty( + PausePointCompiledLineComparisonWarnings.BuildCompiledLineDriftWarningOrEmpty( string.Empty, "return 1;", ForwardSlashFile, - 17), + 17, + true), Is.EqualTo(string.Empty)); Assert.That( - PausePointEnableWarnings.BuildCompiledLineDriftWarningOrEmpty( + PausePointCompiledLineComparisonWarnings.BuildCompiledLineDriftWarningOrEmpty( "return 1;", string.Empty, ForwardSlashFile, - 17), + 17, + false), Is.EqualTo(string.Empty)); } + /// + /// What: a successfully read blank edited line at the resolved line is drift, not silence. + /// + [Test] + public void BuildCompiledLineDriftWarningOrEmpty_WhenEditedLineIsBlankAndReadSucceeded_ReturnsBlankDriftWarning() + { + string warning = PausePointCompiledLineComparisonWarnings.BuildCompiledLineDriftWarningOrEmpty( + " { ", + " ", + ForwardSlashFile, + 109, + true); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' line 109 is '{' in the last compiled source but blank in the edited file. " + + "The marker is armed on the compiled statement. If that is not the statement you meant, " + + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable.")); + } + + /// + /// What: a forward snap from the requested line names the requested edited text and the armed method. + /// + [Test] + public void BuildLineSnapDisclosureWarningOrEmpty_WhenResolvedLineDiffers_ReturnsSnapDisclosure() + { + string warning = PausePointCompiledLineComparisonWarnings.BuildLineSnapDisclosureWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + true, + " LastRemainingBlocks = remainingBlocks; "); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 is 'LastRemainingBlocks = remainingBlocks;' in the edited file, " + + "but the marker snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'.")); + } + + /// + /// What: snap disclosure stays silent when the marker did not leave the requested line. + /// + [Test] + public void BuildLineSnapDisclosureWarningOrEmpty_WhenResolvedLineEqualsRequestedLine_ReturnsEmpty() + { + string warning = PausePointCompiledLineComparisonWarnings.BuildLineSnapDisclosureWarningOrEmpty( + ForwardSlashFile, + 109, + 109, + "GameDirector.ComputeScoreTarget", + true, + "LastRemainingBlocks = remainingBlocks;"); + + Assert.That(warning, Is.EqualTo(string.Empty)); + } + + /// + /// What: a blank requested line still discloses the snap without quoting empty text. + /// + [Test] + public void BuildLineSnapDisclosureWarningOrEmpty_WhenRequestedLineIsBlank_ReturnsBlankSnapDisclosure() + { + string warning = PausePointCompiledLineComparisonWarnings.BuildLineSnapDisclosureWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + true, + " "); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 is blank in the edited file, " + + "but the marker snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'.")); + } + + /// + /// What: a failed requested-line read discloses the snap without claiming the line was blank. + /// + [Test] + public void BuildLineSnapDisclosureWarningOrEmpty_WhenRequestedLineReadFails_OmitsEditedText() + { + string warning = PausePointCompiledLineComparisonWarnings.BuildLineSnapDisclosureWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + false, + string.Empty); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'.")); + } + + /// + /// What: snap disclosure precedes resolved-line drift when both apply, using the existing + /// non-blank drift sentence unchanged. + /// + [Test] + public void ComposeCompiledLineDriftAndSnapWarningOrEmpty_WhenSnapAndNonBlankDrift_PutsSnapBeforeDrift() + { + string warning = PausePointCompiledLineComparisonWarnings.ComposeCompiledLineDriftAndSnapWarningOrEmpty( + ForwardSlashFile, + 10, + 17, + "Example.Run", + "return 1;", + true, + "return 3;", + true, + "return 2;", + 0, + 0, + Array.Empty()); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 10 is 'return 2;' in the edited file, " + + "but the marker snapped forward to line 17 in 'Example.Run'. " + + "'Assets/Scripts/Example.cs' line 17 is 'return 1;' in the last compiled source but 'return 3;' in the edited file. " + + "The marker is armed on the compiled statement. If that is not the statement you meant, " + + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable.")); + } + + /// + /// What: a forward snap onto a blank edited resolved line keeps a drift warning, discloses + /// the snap, and lists the compiled line that matches the requested edited statement. + /// + [Test] + public void ComposeCompiledLineDriftAndSnapWarningOrEmpty_WhenSnapAndBlankResolvedLine_DisclosesSnapBlankDriftAndRequestedCandidate() + { + string[] compiledSourceLines = new string[104]; + for (int index = 0; index < 103; index++) + { + compiledSourceLines[index] = "class Sample"; + } + + compiledSourceLines[103] = " LastRemainingBlocks = remainingBlocks;"; + + string warning = PausePointCompiledLineComparisonWarnings.ComposeCompiledLineDriftAndSnapWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + "{", + true, + string.Empty, + true, + "LastRemainingBlocks = remainingBlocks;", + 100, + 120, + compiledSourceLines); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 is 'LastRemainingBlocks = remainingBlocks;' in the edited file, " + + "but the marker snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'. " + + "'Assets/Scripts/Example.cs' line 109 is '{' in the last compiled source but blank in the edited file. " + + "The marker is armed on the compiled statement. If that is not the statement you meant, " + + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable. " + + "In the last compiled source, 'GameDirector.ComputeScoreTarget' spans lines 100-120. " + + "Candidate: the edited line's text appears at line 104 in the last compiled source.")); + } + /// /// What: enable on an unpatched method in a hot-reloaded file merges the exact drift /// warning and sets the drift next-action when compiled vs edited text differ. @@ -283,6 +459,93 @@ public void Enable_WhenCompiledLineDriftsFromEditedFile_AddsDriftWarningAndNextA } } + /// + /// What: enable on a comment line that rounds forward discloses the snap even when the + /// armed compiled and edited texts match, and still sets the drift next-action. + /// + [Test] + public void Enable_WhenRequestedLineSnapsForward_DisclosesSnapAndSetsNextAction() + { + Func previousLookup = + HotReloadPausePointCoordination.GetShimLookupForFile; + Func previousSnapshot = + HotReloadPausePointCoordination.GetVerifiedSnapshotSourceForFile; + HotReloadShimFileLookup stubLookup = new HotReloadShimFileLookup( + Array.Empty(), + Array.Empty(), + null, + Array.Empty()); + + string absolutePath = Path.Combine( + UnityCliLoopPathResolver.GetProjectRoot(), + ResolveFailureFile); + string diskSource = File.ReadAllText(absolutePath); + int requestedLine = FindLineNumberContaining( + diskSource, + "compiled-line-drift" + "-probe-unique"); + Assert.That(requestedLine, Is.GreaterThan(0)); + int compiledResolvedLine = requestedLine + 1; + + try + { + HotReloadPausePointCoordination.GetShimLookupForFile = _ => stubLookup; + HotReloadPausePointCoordination.GetVerifiedSnapshotSourceForFile = _ => diskSource; + + PausePointResponse response = new PausePointUseCase().Enable(new EnablePausePointSchema + { + File = ResolveFailureFile, + Line = requestedLine, + TimeoutSeconds = 30, + Mode = UloopPausePointCaptureMode.SingleShot + }); + + Assert.That( + response.Success, + Is.True, + response.ErrorCode + " / " + response.Message + " / " + response.RecommendedNextAction); + Assert.That(response.ResolvedLine, Is.EqualTo(compiledResolvedLine)); + SourcePausePointResolveResult spanResult = SourcePausePointResolver.Resolve( + ResolveFailureFile, + response.ResolvedLine); + Assert.That(spanResult.Success, Is.True, spanResult.ErrorMessage); + string requestedEditedText = "// compiled-line-drift" + "-probe-unique"; + string[] compiledLines = SourcePausePointSourceLineReader.SplitSourceLines(diskSource); + string expectedSnap = + "'" + ResolveFailureFile + "' --line " + requestedLine + + " is '" + requestedEditedText + "' in the edited file, but the marker snapped forward to line " + + compiledResolvedLine + " in '" + response.ResolvedMethod + "'."; + expectedSnap = PausePointEnableWarnings.AppendCompiledMethodSpanToDriftWarningOrUnchanged( + expectedSnap, + response.ResolvedMethod, + spanResult.Resolution.CompiledMethodStartLine, + spanResult.Resolution.CompiledMethodEndLine); + expectedSnap = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( + expectedSnap, + "return 424242;", + compiledLines); + expectedSnap = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( + expectedSnap, + requestedEditedText, + compiledLines); + string expectedWarning = PausePointEnableWarnings.MergeWarnings( + PausePointEnableWarnings.MergeWarnings( + PausePointEnableWarnings.MergeWarnings( + PausePointEnableWarnings.CreateEnableWarning(), + PausePointEnableWarnings.BuildCompiledLineMapWarningOrEmpty(true, ResolveFailureFile)), + expectedSnap), + SourcePausePointConstants.SmallMethodInliningRiskWarning); + Assert.That(response.Warning, Is.EqualTo(expectedWarning)); + Assert.That( + response.RecommendedNextAction, + Is.EqualTo(SourcePausePointConstants.HotReloadCompiledLineMapLineDriftNextAction)); + } + finally + { + HotReloadPausePointCoordination.GetShimLookupForFile = previousLookup; + HotReloadPausePointCoordination.GetVerifiedSnapshotSourceForFile = previousSnapshot; + } + } + /// /// What: a known compiled span is appended to a non-empty drift warning. /// diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs new file mode 100644 index 0000000000..3695db2b0c --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs @@ -0,0 +1,189 @@ +using System; +using System.Collections.Generic; +using System.IO; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Builds compiled-vs-edited line-drift and requested-line snap warnings for pause-point enable. + /// + internal static class PausePointCompiledLineComparisonWarnings + { + // Why success-only: resolve failure leaves ResolvedMethod and ResolvedLineText empty, + // so this wording would point at fields that are not on the response. + // Why same resolvedLine on both sides: the resolver rounds empty/comment lines forward, + // so comparing the requested line to the resolved line is a false drift. + // Why readOk is distinct from empty text: a blank edited line is a real mismatch; + // a failed read is not evidence of drift. + internal static string BuildCompiledLineDriftWarningOrEmpty( + string compiledLineText, + string editedLineText, + string file, + int resolvedLine, + bool editedLineReadOk) + { + if (string.IsNullOrEmpty(compiledLineText) || !editedLineReadOk) + { + return string.Empty; + } + + string compiledTrimmed = compiledLineText.Trim(); + string editedTrimmed = editedLineText == null ? string.Empty : editedLineText.Trim(); + if (editedTrimmed.Length == 0) + { + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineMapBlankEditedLineDriftWarningFormat, + SourcePausePointPathNormalizer.ToForwardSlashes(file), + resolvedLine, + compiledTrimmed); + } + + if (string.Equals(compiledTrimmed, editedTrimmed, StringComparison.Ordinal)) + { + return string.Empty; + } + + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineMapLineDriftWarningFormat, + SourcePausePointPathNormalizer.ToForwardSlashes(file), + resolvedLine, + compiledTrimmed, + editedTrimmed); + } + + // Why resolvedLine != requestedLine only: the compiled resolver rounds empty and comment + // lines forward, so this inequality is the snap and has no false positive on this path. + internal static string BuildLineSnapDisclosureWarningOrEmpty( + string file, + int requestedLine, + int resolvedLine, + string resolvedMethod, + bool requestedLineReadOk, + string requestedLineEditedText) + { + if (requestedLine <= 0 || resolvedLine <= 0 || resolvedLine == requestedLine) + { + return string.Empty; + } + + string normalizedFile = SourcePausePointPathNormalizer.ToForwardSlashes(file); + string methodDisplay = resolvedMethod ?? string.Empty; + if (!requestedLineReadOk) + { + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineSnapDisclosureWithoutEditedTextFormat, + normalizedFile, + requestedLine, + resolvedLine, + methodDisplay); + } + + string requestedTrimmed = requestedLineEditedText == null + ? string.Empty + : requestedLineEditedText.Trim(); + if (requestedTrimmed.Length == 0) + { + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineSnapDisclosureBlankRequestedLineFormat, + normalizedFile, + requestedLine, + resolvedLine, + methodDisplay); + } + + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineSnapDisclosureFormat, + normalizedFile, + requestedLine, + requestedTrimmed, + resolvedLine, + methodDisplay); + } + + // Why snap before resolved-line drift: the requested line is what the agent passed; + // the armed line is what actually paused. + // Why search requested-line text too: after a snap the armed line is often blank or a + // brace, so the intended statement lives on the requested line. + // Why skip the second candidate search when texts match: the suffix would duplicate. + internal static string ComposeCompiledLineDriftAndSnapWarningOrEmpty( + string file, + int requestedLine, + int resolvedLine, + string resolvedMethod, + string compiledResolvedLineText, + bool resolvedEditedLineReadOk, + string resolvedEditedLineText, + bool requestedEditedLineReadOk, + string requestedEditedLineText, + int compiledMethodStartLine, + int compiledMethodEndLine, + IReadOnlyList compiledSourceLines) + { + string snapWarning = BuildLineSnapDisclosureWarningOrEmpty( + file, + requestedLine, + resolvedLine, + resolvedMethod, + requestedEditedLineReadOk, + requestedEditedLineText); + string driftWarning = BuildCompiledLineDriftWarningOrEmpty( + compiledResolvedLineText, + resolvedEditedLineText, + file, + resolvedLine, + resolvedEditedLineReadOk); + string combined = PausePointEnableWarnings.MergeWarnings(snapWarning, driftWarning); + combined = PausePointEnableWarnings.AppendCompiledMethodSpanToDriftWarningOrUnchanged( + combined, + resolvedMethod, + compiledMethodStartLine, + compiledMethodEndLine); + combined = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( + combined, + resolvedEditedLineText, + compiledSourceLines); + string resolvedTrimmed = resolvedEditedLineText == null + ? string.Empty + : resolvedEditedLineText.Trim(); + string requestedTrimmed = requestedEditedLineText == null + ? string.Empty + : requestedEditedLineText.Trim(); + if (!string.Equals(resolvedTrimmed, requestedTrimmed, StringComparison.Ordinal)) + { + combined = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( + combined, + requestedEditedLineText, + compiledSourceLines); + } + + return combined; + } + + // Why not ReadLineTextFromSource: that helper returns empty for both a missing line + // and a blank line, which used to suppress a real blank-vs-compiled mismatch. + internal static (bool readOk, string text) ReadEditedLineText(string requestedFile, int lineNumber) + { + if (string.IsNullOrEmpty(requestedFile) || lineNumber <= 0) + { + return (false, string.Empty); + } + + string normalizedFile = SourcePausePointPathNormalizer.ToForwardSlashes(requestedFile); + string absoluteFilePath = Path.Combine(UnityCliLoopPathResolver.GetProjectRoot(), normalizedFile); + if (!File.Exists(absoluteFilePath)) + { + return (false, string.Empty); + } + + string[] lines = SourcePausePointSourceLineReader.SplitSourceLines(File.ReadAllText(absoluteFilePath)); + if (lineNumber > lines.Length) + { + return (false, string.Empty); + } + + return (true, lines[lineNumber - 1].Trim()); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs.meta b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs.meta new file mode 100644 index 0000000000..4e28426d80 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4832e38c6a4704af9abc041a6f182305 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs index 51b5d24308..befdcce0e7 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.IO; using UnityEditor; using UnityEngine; @@ -139,36 +138,6 @@ private static string FormatTypeMethodDisplay(string resolvedMethod, string simp return typeName + "." + simpleName; } - // Why success-only: resolve failure leaves ResolvedMethod and ResolvedLineText empty, - // so this wording would point at fields that are not on the response. - // Why same resolvedLine on both sides: the resolver rounds empty/comment lines forward, - // so comparing the requested line to the resolved line is a false drift. - internal static string BuildCompiledLineDriftWarningOrEmpty( - string compiledLineText, - string editedLineText, - string file, - int resolvedLine) - { - if (string.IsNullOrEmpty(compiledLineText) || string.IsNullOrEmpty(editedLineText)) - { - return string.Empty; - } - - string compiledTrimmed = compiledLineText.Trim(); - string editedTrimmed = editedLineText.Trim(); - if (string.Equals(compiledTrimmed, editedTrimmed, StringComparison.Ordinal)) - { - return string.Empty; - } - - return string.Format( - SourcePausePointConstants.HotReloadCompiledLineMapLineDriftWarningFormat, - SourcePausePointPathNormalizer.ToForwardSlashes(file), - resolvedLine, - compiledTrimmed, - editedTrimmed); - } - internal static string AppendCompiledMethodSpanToDriftWarningOrUnchanged( string driftWarning, string resolvedMethod, @@ -190,7 +159,8 @@ internal static string AppendCompiledMethodSpanToDriftWarningOrUnchanged( } // Why only after a non-empty drift warning: a candidate list without drift would look - // like a second resolution, and empty edited text never produces drift in the first place. + // like a second resolution. + // Why skip empty edited text: a blank line has no statement to locate in compiled source. internal static string AppendCandidateCompiledLinesToDriftWarningOrUnchanged( string driftWarning, string editedLineText, @@ -323,25 +293,6 @@ internal static string AppendNearbyCompiledMethodsSuffix( + "."; } - internal static string ReadEditedLineTextOrEmpty(string requestedFile, int resolvedLine) - { - if (string.IsNullOrEmpty(requestedFile) || resolvedLine <= 0) - { - return string.Empty; - } - - string normalizedFile = SourcePausePointPathNormalizer.ToForwardSlashes(requestedFile); - string absoluteFilePath = Path.Combine(UnityCliLoopPathResolver.GetProjectRoot(), normalizedFile); - if (!File.Exists(absoluteFilePath)) - { - return string.Empty; - } - - return SourcePausePointSourceLineReader.ReadLineTextFromSource( - File.ReadAllText(absoluteFilePath), - resolvedLine); - } - internal static string BuildPatchedMethodPdbUnavailableWarningOrEmpty( bool patchedMethodPdbUnavailable, string methodDisplayName, diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs index f795bae18c..92d52535b1 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointUseCase.cs @@ -387,21 +387,23 @@ private static PausePointResponse FinishEnableBySourceLocation( enableWarning = PausePointEnableWarnings.MergeWarnings(enableWarning, compiledLineMapWarning); if (compareCompiledLineDrift) { - string editedLineText = PausePointEnableWarnings.ReadEditedLineTextOrEmpty(parameters.File, resolvedLine); - string driftWarning = PausePointEnableWarnings.BuildCompiledLineDriftWarningOrEmpty( - resolvedLineText, - editedLineText, + (bool resolvedEditedReadOk, string resolvedEditedLineText) = + PausePointCompiledLineComparisonWarnings.ReadEditedLineText(parameters.File, resolvedLine); + (bool requestedEditedReadOk, string requestedEditedLineText) = + PausePointCompiledLineComparisonWarnings.ReadEditedLineText(parameters.File, parameters.Line); + string[] compiledSourceLines = SourcePausePointSourceLineReader.SplitSourceLines(compiledSnapshotSource); + string driftWarning = PausePointCompiledLineComparisonWarnings.ComposeCompiledLineDriftAndSnapWarningOrEmpty( parameters.File, - resolvedLine); - driftWarning = PausePointEnableWarnings.AppendCompiledMethodSpanToDriftWarningOrUnchanged( - driftWarning, + parameters.Line, + resolvedLine, resolvedMethod, + resolvedLineText, + resolvedEditedReadOk, + resolvedEditedLineText, + requestedEditedReadOk, + requestedEditedLineText, compiledMethodStartLine, - compiledMethodEndLine); - string[] compiledSourceLines = SourcePausePointSourceLineReader.SplitSourceLines(compiledSnapshotSource); - driftWarning = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( - driftWarning, - editedLineText, + compiledMethodEndLine, compiledSourceLines); enableWarning = PausePointEnableWarnings.MergeWarnings(enableWarning, driftWarning); if (driftWarning.Length > 0) diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index 1086c77dc0..c0fb4e0eef 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -240,6 +240,26 @@ internal static class SourcePausePointConstants + "The marker is armed on the compiled statement. If that is not the statement you meant, " + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable."; + // Format: file, resolved line, compiled line text. + // Why a distinct sentence: quoting an empty edited line as '' looks like a missing field. + public const string HotReloadCompiledLineMapBlankEditedLineDriftWarningFormat = + "'{0}' line {1} is '{2}' in the last compiled source but blank in the edited file. " + + "The marker is armed on the compiled statement. If that is not the statement you meant, " + + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable."; + + // Format: file, requested line, requested edited text, resolved line, resolved method. + public const string HotReloadCompiledLineSnapDisclosureFormat = + "'{0}' --line {1} is '{2}' in the edited file, but the marker snapped forward to line {3} in '{4}'."; + + // Format: file, requested line, resolved line, resolved method. + public const string HotReloadCompiledLineSnapDisclosureBlankRequestedLineFormat = + "'{0}' --line {1} is blank in the edited file, but the marker snapped forward to line {2} in '{3}'."; + + // Format: file, requested line, resolved line, resolved method. + // Why omit edited text: a failed read is not the same as a blank line. + public const string HotReloadCompiledLineSnapDisclosureWithoutEditedTextFormat = + "'{0}' --line {1} snapped forward to line {2} in '{3}'."; + public const string HotReloadCompiledLineMapLineDriftNextAction = "Verify ResolvedLineText is the statement you intended. If it is not, run 'uloop compile' " + "and re-enable the pause point."; From acf3b06e8357435cf7bf17e2d325c5e0e43b4f8d Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 20 Aug 2026 13:53:56 +0900 Subject: [PATCH 2/3] fix: keep snap-only candidates on the requested line and name --line A snap without drift was listing the armed line as a match for its own text. Resolved-line candidates now require a drift sentence, and the requested-line search uses wording that names --line so two hits stay distinguishable. Co-authored-by: Cursor --- .../PausePointCompiledLineMapWarningTests.cs | 211 ++++++++++++++++-- ...ausePointCompiledLineComparisonWarnings.cs | 23 +- .../PausePoint/PausePointEnableWarnings.cs | 65 ++++++ .../PausePoint/SourcePausePointConstants.cs | 9 + 4 files changed, 282 insertions(+), 26 deletions(-) diff --git a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs index b59236dc6b..925ef2320e 100644 --- a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs +++ b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs @@ -26,6 +26,14 @@ public sealed class PausePointCompiledLineMapWarningTests private const string GenericPatchedMethodsUseEditedFileSentence = "Methods currently patched by hot reload resolve against the edited file instead"; + private const string CompiledMethodSpanFixtureFile = + "Assets/Tests/Editor/SourcePausePointResolver/Fixtures/CompiledMethodSpanFixture.cs"; + + private const int CompiledMethodSpanFixtureBlankLine = 12; + + private const string MissingEditedLineFile = + "Assets/Tests/Editor/SourcePausePointResolver/Fixtures/DoesNotExistEditedLineRead.cs"; + [SetUp] public void SetUp() { @@ -220,7 +228,46 @@ public void BuildCompiledLineDriftWarningOrEmpty_WhenEditedLineIsBlankAndReadSuc } /// - /// What: a forward snap from the requested line names the requested edited text and the armed method. + /// What: a blank line that exists in an on-disk fixture is a successful read of empty text. + /// + [Test] + public void ReadEditedLineText_WhenLineIsBlank_ReturnsReadOkWithEmptyText() + { + (bool readOk, string text) = PausePointCompiledLineComparisonWarnings.ReadEditedLineText( + CompiledMethodSpanFixtureFile, + CompiledMethodSpanFixtureBlankLine); + + Assert.That(readOk, Is.True); + Assert.That(text, Is.EqualTo(string.Empty)); + } + + /// + /// What: a line number past the end of an on-disk fixture is a failed read, not a blank line. + /// + [Test] + public void ReadEditedLineText_WhenLineIsPastEndOfFile_ReturnsReadFailed() + { + (bool readOk, string text) = PausePointCompiledLineComparisonWarnings.ReadEditedLineText( + CompiledMethodSpanFixtureFile, + UnresolvableLine); + + Assert.That(readOk, Is.False); + Assert.That(text, Is.EqualTo(string.Empty)); + } + + /// + /// What: a missing file is a failed read, not a blank line. + /// + [Test] + public void ReadEditedLineText_WhenFileDoesNotExist_ReturnsReadFailed() + { + (bool readOk, string text) = PausePointCompiledLineComparisonWarnings.ReadEditedLineText( + MissingEditedLineFile, + 1); + + Assert.That(readOk, Is.False); + Assert.That(text, Is.EqualTo(string.Empty)); + } /// [Test] public void BuildLineSnapDisclosureWarningOrEmpty_WhenResolvedLineDiffers_ReturnsSnapDisclosure() @@ -367,7 +414,85 @@ public void ComposeCompiledLineDriftAndSnapWarningOrEmpty_WhenSnapAndBlankResolv + "The marker is armed on the compiled statement. If that is not the statement you meant, " + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable. " + "In the last compiled source, 'GameDirector.ComputeScoreTarget' spans lines 100-120. " - + "Candidate: the edited line's text appears at line 104 in the last compiled source.")); + + "Candidate: the text at --line 107 in the edited file appears at line 104 in the last compiled source.")); + } + + /// + /// What: a snap-only warning still lists a compiled match for the requested --line text + /// and does not list the armed line as a candidate for its own text. + /// + [Test] + public void ComposeCompiledLineDriftAndSnapWarningOrEmpty_WhenSnapOnly_OmitsResolvedSelfCandidate() + { + string[] compiledSourceLines = + { + "class Sample", + " {", + " LastRemainingBlocks = remainingBlocks;", + " {" + }; + + string warning = PausePointCompiledLineComparisonWarnings.ComposeCompiledLineDriftAndSnapWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + "{", + true, + "{", + true, + "LastRemainingBlocks = remainingBlocks;", + 100, + 120, + compiledSourceLines); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 is 'LastRemainingBlocks = remainingBlocks;' in the edited file, " + + "but the marker snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'. " + + "In the last compiled source, 'GameDirector.ComputeScoreTarget' spans lines 100-120. " + + "Candidate: the text at --line 107 in the edited file appears at line 3 in the last compiled source.")); + } + + /// + /// What: when resolved-line drift and a distinct requested-line match both exist, the + /// two candidate sentences name different searches. + /// + [Test] + public void ComposeCompiledLineDriftAndSnapWarningOrEmpty_WhenDriftAndRequestedLineBothMatch_DistinguishesCandidateSentences() + { + string[] compiledSourceLines = + { + "class Sample", + " return 3;", + " LastRemainingBlocks = remainingBlocks;" + }; + + string warning = PausePointCompiledLineComparisonWarnings.ComposeCompiledLineDriftAndSnapWarningOrEmpty( + ForwardSlashFile, + 107, + 109, + "GameDirector.ComputeScoreTarget", + "{", + true, + "return 3;", + true, + "LastRemainingBlocks = remainingBlocks;", + 0, + 0, + compiledSourceLines); + + Assert.That( + warning, + Is.EqualTo( + "'Assets/Scripts/Example.cs' --line 107 is 'LastRemainingBlocks = remainingBlocks;' in the edited file, " + + "but the marker snapped forward to line 109 in 'GameDirector.ComputeScoreTarget'. " + + "'Assets/Scripts/Example.cs' line 109 is '{' in the last compiled source but 'return 3;' in the edited file. " + + "The marker is armed on the compiled statement. If that is not the statement you meant, " + + "recompute --line against the last compiled source, or run 'uloop compile' and re-enable. " + + "Candidate: the edited line's text appears at line 2 in the last compiled source. " + + "Candidate: the text at --line 107 in the edited file appears at line 3 in the last compiled source.")); } /// @@ -509,24 +634,16 @@ public void Enable_WhenRequestedLineSnapsForward_DisclosesSnapAndSetsNextAction( response.ResolvedLine); Assert.That(spanResult.Success, Is.True, spanResult.ErrorMessage); string requestedEditedText = "// compiled-line-drift" + "-probe-unique"; - string[] compiledLines = SourcePausePointSourceLineReader.SplitSourceLines(diskSource); string expectedSnap = "'" + ResolveFailureFile + "' --line " + requestedLine + " is '" + requestedEditedText + "' in the edited file, but the marker snapped forward to line " - + compiledResolvedLine + " in '" + response.ResolvedMethod + "'."; - expectedSnap = PausePointEnableWarnings.AppendCompiledMethodSpanToDriftWarningOrUnchanged( - expectedSnap, - response.ResolvedMethod, - spanResult.Resolution.CompiledMethodStartLine, - spanResult.Resolution.CompiledMethodEndLine); - expectedSnap = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( - expectedSnap, - "return 424242;", - compiledLines); - expectedSnap = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( - expectedSnap, - requestedEditedText, - compiledLines); + + compiledResolvedLine + " in '" + response.ResolvedMethod + "'." + + " In the last compiled source, '" + response.ResolvedMethod + "' spans lines " + + spanResult.Resolution.CompiledMethodStartLine + "-" + + spanResult.Resolution.CompiledMethodEndLine + "." + + " Candidate: the text at --line " + requestedLine + + " in the edited file appears at line " + requestedLine + + " in the last compiled source."; string expectedWarning = PausePointEnableWarnings.MergeWarnings( PausePointEnableWarnings.MergeWarnings( PausePointEnableWarnings.MergeWarnings( @@ -752,7 +869,65 @@ public void AppendCandidateCompiledLinesToDriftWarningOrUnchanged_WhenDriftIsEmp } /// - /// What: retarget warning interpolates resolved method, requested line, and edited span. + /// What: one compiled-source match for the requested --line text names that --line. + /// + [Test] + public void AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged_WhenOneLineMatches_NamesRequestedLine() + { + string drift = + "'Assets/Scripts/Example.cs' --line 107 is 'return 2;' in the edited file, " + + "but the marker snapped forward to line 109 in 'Example.Run'."; + string[] compiledLines = + { + "class Sample", + " return 2;", + " return 1;" + }; + + string warning = PausePointEnableWarnings.AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged( + drift, + 107, + " return 2; ", + compiledLines); + + Assert.That( + warning, + Is.EqualTo( + drift + + " Candidate: the text at --line 107 in the edited file appears at line 2 in the last compiled source.")); + } + + /// + /// What: more than three compiled-source matches for the requested --line text cap at the + /// first three and name that --line. + /// + [Test] + public void AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged_WhenMoreThanThreeLinesMatch_CapsAtFirstThree() + { + string drift = + "'Assets/Scripts/Example.cs' --line 107 is 'return 2;' in the edited file, " + + "but the marker snapped forward to line 109 in 'Example.Run'."; + string[] compiledLines = + { + " return 2;", + " return 1;", + " return 2;", + " return 2;", + " return 2;" + }; + + string warning = PausePointEnableWarnings.AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged( + drift, + 107, + "return 2;", + compiledLines); + + Assert.That( + warning, + Is.EqualTo( + drift + + " Candidate: the text at --line 107 in the edited file appears at lines 1, 3, 4 (first 3 matches) in the last compiled source.")); + } /// [Test] public void BuildRetargetedToHotReloadPatchWarningOrEmpty_WhenRetargeted_ReturnsFormattedWarning() diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs index 3695db2b0c..3a90a4e9bf 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointCompiledLineComparisonWarnings.cs @@ -104,9 +104,11 @@ internal static string BuildLineSnapDisclosureWarningOrEmpty( // Why snap before resolved-line drift: the requested line is what the agent passed; // the armed line is what actually paused. - // Why search requested-line text too: after a snap the armed line is often blank or a - // brace, so the intended statement lives on the requested line. - // Why skip the second candidate search when texts match: the suffix would duplicate. + // Why resolved-text candidates only with a drift sentence: a snap-only warning already + // named the armed line, so searching that same text finds the armed line itself. + // Why still search requested-line text on a snap-only warning: the intended statement is + // on --line, including when braces at the armed line happen to match. + // Why skip the requested-line search when texts match: the suffix would duplicate. internal static string ComposeCompiledLineDriftAndSnapWarningOrEmpty( string file, int requestedLine, @@ -140,10 +142,14 @@ internal static string ComposeCompiledLineDriftAndSnapWarningOrEmpty( resolvedMethod, compiledMethodStartLine, compiledMethodEndLine); - combined = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( - combined, - resolvedEditedLineText, - compiledSourceLines); + if (driftWarning.Length > 0) + { + combined = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( + combined, + resolvedEditedLineText, + compiledSourceLines); + } + string resolvedTrimmed = resolvedEditedLineText == null ? string.Empty : resolvedEditedLineText.Trim(); @@ -152,8 +158,9 @@ internal static string ComposeCompiledLineDriftAndSnapWarningOrEmpty( : requestedEditedLineText.Trim(); if (!string.Equals(resolvedTrimmed, requestedTrimmed, StringComparison.Ordinal)) { - combined = PausePointEnableWarnings.AppendCandidateCompiledLinesToDriftWarningOrUnchanged( + combined = PausePointEnableWarnings.AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged( combined, + requestedLine, requestedEditedLineText, compiledSourceLines); } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs index befdcce0e7..d93bcd2e62 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointEnableWarnings.cs @@ -193,6 +193,44 @@ internal static string AppendCandidateCompiledLinesToDriftWarningOrUnchanged( return driftWarning + FormatCandidateCompiledLinesSuffix(matches, truncated); } + // Why a distinct sentence: the resolved-line candidate does not name --line, so two + // identical "edited line" suffixes would not say which search produced which hit. + internal static string AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged( + string driftWarning, + int requestedLine, + string requestedLineEditedText, + IReadOnlyList compiledSourceLines) + { + if (string.IsNullOrEmpty(driftWarning) || requestedLine <= 0) + { + return driftWarning ?? string.Empty; + } + + if (string.IsNullOrEmpty(requestedLineEditedText) || compiledSourceLines == null) + { + return driftWarning; + } + + string editedTrimmed = requestedLineEditedText.Trim(); + if (editedTrimmed.Length == 0) + { + return driftWarning; + } + + (List matches, bool truncated) = CollectCandidateCompiledLineNumbers( + editedTrimmed, + compiledSourceLines); + if (matches.Count == 0) + { + return driftWarning; + } + + return driftWarning + FormatRequestedLineCandidateCompiledLinesSuffix( + requestedLine, + matches, + truncated); + } + private static (List matches, bool truncated) CollectCandidateCompiledLineNumbers( string editedTrimmed, IReadOnlyList compiledSourceLines) @@ -247,6 +285,33 @@ private static string FormatCandidateCompiledLinesSuffix(List matches, bool listed); } + private static string FormatRequestedLineCandidateCompiledLinesSuffix( + int requestedLine, + List matches, + bool truncated) + { + if (matches.Count == 1 && !truncated) + { + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineDriftRequestedLineCandidateSingleFormat, + requestedLine, + matches[0]); + } + + string listed = string.Join(", ", matches); + if (truncated) + { + listed += string.Format( + SourcePausePointConstants.HotReloadCompiledLineDriftCandidateTruncatedMatchesSuffixFormat, + SourcePausePointConstants.CompiledLineDriftCandidateMatchLimit); + } + + return string.Format( + SourcePausePointConstants.HotReloadCompiledLineDriftRequestedLineCandidateMultipleFormat, + requestedLine, + listed); + } + internal static string BuildRetargetedToHotReloadPatchWarningOrEmpty( bool retargetedToHotReloadPatch, string resolvedMethod, diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index c0fb4e0eef..4fb3fd9333 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -296,6 +296,15 @@ internal static class SourcePausePointConstants public const string HotReloadCompiledLineDriftCandidateMultipleFormat = " Candidate: the edited line's text appears at lines {0} in the last compiled source."; + // Format: requested --line, 1-based compiled line number. + public const string HotReloadCompiledLineDriftRequestedLineCandidateSingleFormat = + " Candidate: the text at --line {0} in the edited file appears at line {1} in the last compiled source."; + + // Format: requested --line, comma-separated 1-based compiled line numbers, with an + // optional truncation note. + public const string HotReloadCompiledLineDriftRequestedLineCandidateMultipleFormat = + " Candidate: the text at --line {0} in the edited file appears at lines {1} in the last compiled source."; + // Why format from CompiledLineDriftCandidateMatchLimit: a hard-coded "3" would lie // if the cap changed. public const string HotReloadCompiledLineDriftCandidateTruncatedMatchesSuffixFormat = From fa533bd9c2ff8e806e9b854ab8a052382f12761e Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 20 Aug 2026 13:56:25 +0900 Subject: [PATCH 3/3] fix: restore XML summaries dropped when inserting pause-point tests Two existing tests lost their /What lines during nearby inserts, leaving a stray closing tag. Restore the original wording. Co-authored-by: Cursor --- .../Tests/Editor/PausePointCompiledLineMapWarningTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs index 925ef2320e..668da781cd 100644 --- a/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs +++ b/Assets/Tests/Editor/PausePointCompiledLineMapWarningTests.cs @@ -268,6 +268,9 @@ public void ReadEditedLineText_WhenFileDoesNotExist_ReturnsReadFailed() Assert.That(readOk, Is.False); Assert.That(text, Is.EqualTo(string.Empty)); } + + /// + /// What: a forward snap from the requested line names the requested edited text and the armed method. /// [Test] public void BuildLineSnapDisclosureWarningOrEmpty_WhenResolvedLineDiffers_ReturnsSnapDisclosure() @@ -928,6 +931,9 @@ public void AppendRequestedLineCandidateCompiledLinesToDriftWarningOrUnchanged_W drift + " Candidate: the text at --line 107 in the edited file appears at lines 1, 3, 4 (first 3 matches) in the last compiled source.")); } + + /// + /// What: retarget warning interpolates resolved method, requested line, and edited span. /// [Test] public void BuildRetargetedToHotReloadPatchWarningOrEmpty_WhenRetargeted_ReturnsFormattedWarning()