From 445c64bd2b95911945315bb5b7c54eb0e607bf8a Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Jul 2026 01:15:26 +0900 Subject: [PATCH 1/3] fix: keep raw capture holder when re-enabling the same pause point while paused Re-enabling the same pause-point id while Unity is still paused (for example to refresh its timeout during a step session) cleared the raw capture holder too, so UloopPausePoint.TryGetCapturedValue reported Found=false even though Unity never resumed. Enable() shared the same helper Clear() used to drop the hit snapshot, which unconditionally clears UloopPausePointRawCaptureHolder. A re-enable does not resume Unity, so the holder's documented paused-window invariant is not actually violated by keeping the previous hit's live references across a same-id re-enable. Split ClearLatestHitSnapshotIfMatches into ForgetHitSnapshotForId (drops only the hit-history entry / latest-hit pointer, used by Enable) and ClearHitSnapshotAndRawCaptureForId (also clears the raw capture holder, used by Clear). Add a regression test that hits a pause point, re-enables the same id while paused, and asserts the captured value is still retrievable. Update the pause-point skill doc (source and its two generated copies) to document the re-enable behavior. Manually verified against a running Unity Editor (6000.3.15f1): hit -> re-enable -> Step x5 keeps the capture, and a subsequent resume still clears it. --- .agents/skills/uloop-pause-point/SKILL.md | 2 +- .claude/skills/uloop-pause-point/SKILL.md | 2 +- Assets/Tests/Editor/PausePointTests.cs | 19 ++++++++++++ .../CliOnlyTools~/PausePoint/Skill/SKILL.md | 2 +- .../PausePoints/UloopPausePointRegistry.cs | 30 ++++++++++++------- 5 files changed, 42 insertions(+), 13 deletions(-) diff --git a/.agents/skills/uloop-pause-point/SKILL.md b/.agents/skills/uloop-pause-point/SKILL.md index 40442008b..fb2af82c2 100644 --- a/.agents/skills/uloop-pause-point/SKILL.md +++ b/.agents/skills/uloop-pause-point/SKILL.md @@ -73,7 +73,7 @@ While Unity is paused on a hit, `execute-dynamic-code` can read live captured re - `GetCapturedNames()` lists captured variable names from that snapshot. - `GetCapturedPausePointId()` returns the pause-point id for the held snapshot. -The holder clears when Unity resumes (not when you `Step` while still paused), when the matching pause point is cleared, when a new hit replaces the snapshot, or when PlayMode exits. After resume, `TryGetCapturedValue` returns `Found=false`. +The holder clears when Unity resumes (not when you `Step` while still paused), when the matching pause point is cleared, when a new hit replaces the snapshot, or when PlayMode exits. After resume, `TryGetCapturedValue` returns `Found=false`. Re-enabling the same pause point while still paused (for example to refresh its timeout during a step session) keeps the held references, because a re-enable does not resume Unity. ## Watch Expressions diff --git a/.claude/skills/uloop-pause-point/SKILL.md b/.claude/skills/uloop-pause-point/SKILL.md index 40442008b..fb2af82c2 100644 --- a/.claude/skills/uloop-pause-point/SKILL.md +++ b/.claude/skills/uloop-pause-point/SKILL.md @@ -73,7 +73,7 @@ While Unity is paused on a hit, `execute-dynamic-code` can read live captured re - `GetCapturedNames()` lists captured variable names from that snapshot. - `GetCapturedPausePointId()` returns the pause-point id for the held snapshot. -The holder clears when Unity resumes (not when you `Step` while still paused), when the matching pause point is cleared, when a new hit replaces the snapshot, or when PlayMode exits. After resume, `TryGetCapturedValue` returns `Found=false`. +The holder clears when Unity resumes (not when you `Step` while still paused), when the matching pause point is cleared, when a new hit replaces the snapshot, or when PlayMode exits. After resume, `TryGetCapturedValue` returns `Found=false`. Re-enabling the same pause point while still paused (for example to refresh its timeout during a step session) keeps the held references, because a re-enable does not resume Unity. ## Watch Expressions diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index 77e188272..0bc04816b 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -578,6 +578,25 @@ public void TryGetCapturedValue_WhenUnrelatedPausePointIsCleared_KeepsLatestHitR Assert.That(UloopPausePoint.GetCapturedPausePointId(), Is.EqualTo("land")); } + [Test] + public void TryGetCapturedValue_WhenSamePausePointIsReenabledWhilePaused_KeepsLatestHitRawCapture() + { + // Verifies re-enabling the same id while still paused (e.g. to refresh its timeout + // during a step session) keeps the held raw capture instead of clearing it. + UloopPausePointRegistry.Enable("jump", 30); + UloopPausePointCapturedVariableFrame frame = new( + new[] { new UloopPausePointCapturedVariableEntry("speed", UloopCapturedVariableScope.Local, 1) }, + false); + UloopPausePointRegistry.HitWithCapturedFrame("jump", frame, Array.Empty(), false); + + UloopPausePointRegistry.Enable("jump", 30); + + (bool found, object value) = UloopPausePoint.TryGetCapturedValue("speed"); + Assert.That(found, Is.True); + Assert.That(value, Is.EqualTo(1)); + Assert.That(UloopPausePoint.GetCapturedPausePointId(), Is.EqualTo("jump")); + } + [Test] public void Hit_WhenPausePointIsEnabled_ReportsEmptyCapturedVariables() { diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md index 40442008b..fb2af82c2 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md @@ -73,7 +73,7 @@ While Unity is paused on a hit, `execute-dynamic-code` can read live captured re - `GetCapturedNames()` lists captured variable names from that snapshot. - `GetCapturedPausePointId()` returns the pause-point id for the held snapshot. -The holder clears when Unity resumes (not when you `Step` while still paused), when the matching pause point is cleared, when a new hit replaces the snapshot, or when PlayMode exits. After resume, `TryGetCapturedValue` returns `Found=false`. +The holder clears when Unity resumes (not when you `Step` while still paused), when the matching pause point is cleared, when a new hit replaces the snapshot, or when PlayMode exits. After resume, `TryGetCapturedValue` returns `Found=false`. Re-enabling the same pause point while still paused (for example to refresh its timeout during a step session) keeps the held references, because a re-enable does not resume Unity. ## Watch Expressions diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs index 595579eac..3a1b4effb 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs @@ -53,7 +53,10 @@ public static UloopPausePointSnapshot Enable( int generation = ++_nextGeneration; UloopPausePointEntry entry = new(id, timeoutSeconds, mode, maxHistory, now, generation); Entries[id] = entry; - ClearLatestHitSnapshotIfMatches(id); + // Why not clear the raw capture holder here: a re-enable does not resume Unity, so the + // paused-window constraint (see UloopPausePointRawCaptureHolder's class comment) is not + // violated by keeping the previous hit's live references across a same-id re-enable. + ForgetHitSnapshotForId(id); return entry.ToSnapshot(now, _pauseController); } @@ -86,7 +89,7 @@ public static UloopPausePointSnapshot Clear(string id) _ => "Pause point cleared." }; entry.MarkCleared(UloopPausePointClearedReason.ExplicitClear, message); - ClearLatestHitSnapshotIfMatches(id); + ClearHitSnapshotAndRawCaptureForId(id); return entry.ToSnapshot(now, _pauseController); } @@ -244,21 +247,28 @@ public static void ClearLatestHitSnapshot() UloopPausePointRawCaptureHolder.Clear(); } - private static void ClearLatestHitSnapshotIfMatches(string id) + // Drops the id's own hit-history entry and, if it currently owns the latest hit, that + // pointer too - but never touches the raw capture holder. Enable() uses this so a same-id + // re-enable while paused only resets the entry's generation bookkeeping. + private static void ForgetHitSnapshotForId(string id) { _hitSnapshots.RemoveAll(hitSnapshot => hitSnapshot.Id == id); - if (_latestHitSnapshot == null) + if (_latestHitSnapshot != null && _latestHitSnapshot.Id == id) { - return; + _latestHitSnapshot = null; } + } - if (_latestHitSnapshot.Id != id) + // Same as ForgetHitSnapshotForId, plus clears the raw capture holder when the id owned the + // latest hit. Clear(id) uses this because an explicit clear is documented to drop captures. + private static void ClearHitSnapshotAndRawCaptureForId(string id) + { + bool ownedLatestHit = _latestHitSnapshot != null && _latestHitSnapshot.Id == id; + ForgetHitSnapshotForId(id); + if (ownedLatestHit) { - return; + UloopPausePointRawCaptureHolder.Clear(); } - - _latestHitSnapshot = null; - UloopPausePointRawCaptureHolder.Clear(); } public static void ConfigureForTests(IUloopPausePointPauseController pauseController, Func nowProvider) From c9d181477719c12433345f101f48dafc3df99742 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Jul 2026 01:15:58 +0900 Subject: [PATCH 2/3] docs: clarify that watch expressions evaluate only while Play Mode is paused "while Unity is both playing and paused" was ambiguous enough that a real usage report misread it as "watches also evaluate while the game is running," and reported that behavior as an implementation bug. The actual implementation (from PR #1729/#1733) only evaluates a watch once per changed Time.frameCount when Play Mode is running AND the Editor is paused (on each hit pause and each Step); nothing is recorded while the game runs unpaused. Reword the sentence in the pause-point skill doc (source and its two generated copies) so it cannot be misread as "playing" meaning "running unpaused." --- .agents/skills/uloop-pause-point/SKILL.md | 2 +- .claude/skills/uloop-pause-point/SKILL.md | 2 +- Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.agents/skills/uloop-pause-point/SKILL.md b/.agents/skills/uloop-pause-point/SKILL.md index fb2af82c2..d55a8d648 100644 --- a/.agents/skills/uloop-pause-point/SKILL.md +++ b/.agents/skills/uloop-pause-point/SKILL.md @@ -84,7 +84,7 @@ uloop enable-watch --id "speed" --expression "UloopPausePoint.TryGetCapturedValu uloop get-watch-values --id "speed" ``` -`enable-watch` compiles the C# expression once, evaluates it immediately for a baseline, and then evaluates it once per changed `Time.frameCount` while Unity is both playing and paused. Multiple watches run in registration order. `enable-watch` rejects a duplicate id instead of overwriting; clear with `clear-watch --id ` before re-registering a changed expression. `clear-watch --id ` removes one watch; `clear-watch --all` removes all watches. `get-watch-values` without `--id` returns every registered watch. +`enable-watch` compiles the C# expression once, evaluates it immediately for a baseline, and then evaluates it once per changed `Time.frameCount`, but only while Play Mode is running and the Editor is paused (each hit pause and each `Step`); nothing is recorded while the game runs unpaused. Multiple watches run in registration order. `enable-watch` rejects a duplicate id instead of overwriting; clear with `clear-watch --id ` before re-registering a changed expression. `clear-watch --id ` removes one watch; `clear-watch --all` removes all watches. `get-watch-values` without `--id` returns every registered watch. The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect the latest raw pause-point capture while paused. Each history entry includes the frame and either a stringified value or an explicit error type and message. A throwing expression is recorded as an error and does not stop the Editor update loop. `--max-history` accepts 1 through 100 and drops the oldest entries after the limit. diff --git a/.claude/skills/uloop-pause-point/SKILL.md b/.claude/skills/uloop-pause-point/SKILL.md index fb2af82c2..d55a8d648 100644 --- a/.claude/skills/uloop-pause-point/SKILL.md +++ b/.claude/skills/uloop-pause-point/SKILL.md @@ -84,7 +84,7 @@ uloop enable-watch --id "speed" --expression "UloopPausePoint.TryGetCapturedValu uloop get-watch-values --id "speed" ``` -`enable-watch` compiles the C# expression once, evaluates it immediately for a baseline, and then evaluates it once per changed `Time.frameCount` while Unity is both playing and paused. Multiple watches run in registration order. `enable-watch` rejects a duplicate id instead of overwriting; clear with `clear-watch --id ` before re-registering a changed expression. `clear-watch --id ` removes one watch; `clear-watch --all` removes all watches. `get-watch-values` without `--id` returns every registered watch. +`enable-watch` compiles the C# expression once, evaluates it immediately for a baseline, and then evaluates it once per changed `Time.frameCount`, but only while Play Mode is running and the Editor is paused (each hit pause and each `Step`); nothing is recorded while the game runs unpaused. Multiple watches run in registration order. `enable-watch` rejects a duplicate id instead of overwriting; clear with `clear-watch --id ` before re-registering a changed expression. `clear-watch --id ` removes one watch; `clear-watch --all` removes all watches. `get-watch-values` without `--id` returns every registered watch. The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect the latest raw pause-point capture while paused. Each history entry includes the frame and either a stringified value or an explicit error type and message. A throwing expression is recorded as an error and does not stop the Editor update loop. `--max-history` accepts 1 through 100 and drops the oldest entries after the limit. diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md index fb2af82c2..d55a8d648 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md @@ -84,7 +84,7 @@ uloop enable-watch --id "speed" --expression "UloopPausePoint.TryGetCapturedValu uloop get-watch-values --id "speed" ``` -`enable-watch` compiles the C# expression once, evaluates it immediately for a baseline, and then evaluates it once per changed `Time.frameCount` while Unity is both playing and paused. Multiple watches run in registration order. `enable-watch` rejects a duplicate id instead of overwriting; clear with `clear-watch --id ` before re-registering a changed expression. `clear-watch --id ` removes one watch; `clear-watch --all` removes all watches. `get-watch-values` without `--id` returns every registered watch. +`enable-watch` compiles the C# expression once, evaluates it immediately for a baseline, and then evaluates it once per changed `Time.frameCount`, but only while Play Mode is running and the Editor is paused (each hit pause and each `Step`); nothing is recorded while the game runs unpaused. Multiple watches run in registration order. `enable-watch` rejects a duplicate id instead of overwriting; clear with `clear-watch --id ` before re-registering a changed expression. `clear-watch --id ` removes one watch; `clear-watch --all` removes all watches. `get-watch-values` without `--id` returns every registered watch. The expression may use `UloopPausePoint.TryGetCapturedValue("name")` to inspect the latest raw pause-point capture while paused. Each history entry includes the frame and either a stringified value or an explicit error type and message. A throwing expression is recorded as an error and does not stop the Editor update loop. `--max-history` accepts 1 through 100 and drops the oldest entries after the limit. From 6664dcac04d89210e98e9e7ff29796b48a35108a Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 13 Jul 2026 01:36:22 +0900 Subject: [PATCH 3/3] Fix Clear(id) leaking raw capture holder after a same-id re-enable CodeRabbit flagged that ClearHitSnapshotAndRawCaptureForId judged raw capture ownership from _latestHitSnapshot, but Enable() already nulls that field for the same id via ForgetHitSnapshotForId as part of a same-id re-enable. So the sequence hit -> re-enable -> Clear(id) left the raw capture holder alive, contradicting the documented contract that an explicit Clear drops captures. Check ownership against UloopPausePointRawCaptureHolder's own captured id instead: the holder already tracks which pause point last stored into it and is updated in lockstep with every hit, so this is equivalent to the old semantics on all other paths and immune to _latestHitSnapshot being cleared early by a re-enable. Added a regression test verifying Clear(id) after a same-id re-enable still clears the holder. --- Assets/Tests/Editor/PausePointTests.cs | 20 +++++++++++++++++++ .../PausePoints/UloopPausePointRegistry.cs | 11 ++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index 0bc04816b..2e9c81854 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -597,6 +597,26 @@ public void TryGetCapturedValue_WhenSamePausePointIsReenabledWhilePaused_KeepsLa Assert.That(UloopPausePoint.GetCapturedPausePointId(), Is.EqualTo("jump")); } + [Test] + public void TryGetCapturedValue_WhenSamePausePointIsReenabledThenCleared_StillClearsRawCapture() + { + // Verifies Clear(id) after a same-id re-enable still drops the raw capture holder, + // even though the re-enable already reset the hit snapshot bookkeeping. + UloopPausePointRegistry.Enable("jump", 30); + UloopPausePointCapturedVariableFrame frame = new( + new[] { new UloopPausePointCapturedVariableEntry("speed", UloopCapturedVariableScope.Local, 1) }, + false); + UloopPausePointRegistry.HitWithCapturedFrame("jump", frame, Array.Empty(), false); + + UloopPausePointRegistry.Enable("jump", 30); + UloopPausePointRegistry.Clear("jump"); + + (bool found, object value) = UloopPausePoint.TryGetCapturedValue("speed"); + Assert.That(found, Is.False); + Assert.That(value, Is.Null); + Assert.That(UloopPausePoint.GetCapturedPausePointId(), Is.Empty); + } + [Test] public void Hit_WhenPausePointIsEnabled_ReportsEmptyCapturedVariables() { diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs index 3a1b4effb..ab8491c44 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs @@ -259,13 +259,16 @@ private static void ForgetHitSnapshotForId(string id) } } - // Same as ForgetHitSnapshotForId, plus clears the raw capture holder when the id owned the - // latest hit. Clear(id) uses this because an explicit clear is documented to drop captures. + // Same as ForgetHitSnapshotForId, plus clears the raw capture holder when the holder's + // captured snapshot belongs to the cleared id. Clear(id) uses this because an explicit + // clear is documented to drop captures. Ownership is checked against the holder itself + // (not _latestHitSnapshot) because Enable()'s ForgetHitSnapshotForId already nulls out + // _latestHitSnapshot on a same-id re-enable, which would otherwise make a later Clear(id) + // think it no longer owns a holder it actually still does. private static void ClearHitSnapshotAndRawCaptureForId(string id) { - bool ownedLatestHit = _latestHitSnapshot != null && _latestHitSnapshot.Id == id; ForgetHitSnapshotForId(id); - if (ownedLatestHit) + if (UloopPausePointRawCaptureHolder.GetCapturedPausePointId() == id) { UloopPausePointRawCaptureHolder.Clear(); }