From a339f1bef8d62632078fd2650aea4021902e0d9c Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 12:11:07 +0900 Subject: [PATCH 01/10] fix: make await-pause-point wait for a new hit on continuous markers Co-authored-by: Cursor --- .../pause_point_await_new_hit_test.go | 340 ++++++++++++++++++ .../projectrunner/pause_point_enable.go | 10 +- .../projectrunner/pause_point_errors.go | 14 +- .../pause_point_resume_play_test.go | 12 +- .../projectrunner/pause_point_trigger_test.go | 6 +- .../projectrunner/pause_point_wait.go | 20 +- .../projectrunner/pause_point_wait_poll.go | 148 ++++++-- .../pause_point_wait_poll_test.go | 12 +- .../projectrunner/pause_point_wait_test.go | 20 +- 9 files changed, 516 insertions(+), 66 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go diff --git a/cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go b/cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go new file mode 100644 index 0000000000..ad3a2fc742 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_await_new_hit_test.go @@ -0,0 +1,340 @@ +package projectrunner + +import ( + "bytes" + "context" + "strings" + "testing" + "time" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// Verifies await on an already-hit continuous marker ignores the baseline snapshot and returns +// only after LastHitSequence advances. +func TestWaitForPausePointWaitsForNewHitOnAlreadyHitContinuousMarker(t *testing.T) { + originalQuery := queryPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + pausePointStatusPoll = originalPoll + }() + + queryCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + queryCount++ + sequence := 5 + if queryCount >= 2 { + sequence = 6 + } + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: sequence, + Mode: pausePointModeContinuous, + LastHitSequence: sequence, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + + response, state, _, _, hasNewHitBaseline, err := waitForPausePoint( + context.Background(), + unityipc.Connection{}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + }, + ) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateHit { + t.Fatalf("state mismatch: %s", state) + } + if !hasNewHitBaseline { + t.Fatal("expected a new-hit baseline for an already-hit continuous marker") + } + if response.LastHitSequence != 6 { + t.Fatalf("expected the advanced hit sequence, got %#v", response) + } + if queryCount < 2 { + t.Fatalf("expected at least two status polls, got %d", queryCount) + } +} + +// Verifies await on an already-hit continuous marker times out with PAUSE_POINT_WAIT_TIMEOUT and +// the already-hit baseline hint when LastHitSequence never advances, without clearing the still-armed +// marker (the hint tells the caller to await again). +func TestWaitForPausePointTimesOutWaitingForNewHitOnContinuousMarker(t *testing.T) { + originalQuery := queryPausePointStatus + originalClear := clearPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + clearPausePointStatus = originalClear + pausePointStatusPoll = originalPoll + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 5, + Mode: pausePointModeContinuous, + LastHitSequence: 5, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + clearCalls := 0 + clearPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + clearCalls++ + t.Fatal("timeout while waiting for a new hit must not clear the still-armed continuous marker") + return pausePointStatusResponse{}, nil + } + + stderr := &bytes.Buffer{} + exitCode := runWaitForPausePoint( + context.Background(), + unityipc.Connection{ProjectRoot: "/tmp/MyProject"}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: 40 * time.Millisecond, + }, + &bytes.Buffer{}, + stderr, + ) + if exitCode != 1 { + t.Fatalf("exit code mismatch: %d", exitCode) + } + if clearCalls != 0 { + t.Fatalf("expected clear not to run, got %d calls", clearCalls) + } + stderrText := stderr.String() + if !strings.Contains(stderrText, clierrors.ErrorCodePausePointWaitTimeout) { + t.Fatalf("expected %s, got stderr: %s", clierrors.ErrorCodePausePointWaitTimeout, stderrText) + } + if !strings.Contains(stderrText, pausePointHintAlreadyHitWaitingForNew) { + t.Fatalf("expected already-hit baseline hint, got stderr: %s", stderrText) + } + if strings.Contains(stderrText, clierrors.ErrorCodePausePointExpired) { + t.Fatalf("timeout must not be reclassified as expired: %s", stderrText) + } +} + +// Verifies a transient arm-query failure does not decide "no baseline", so a later stale continuous +// Hit is not returned as an immediate wait success. +func TestWaitForPausePointBaseliningSurvivesTransientArmQueryFailure(t *testing.T) { + originalQuery := queryPausePointStatus + originalClear := clearPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + clearPausePointStatus = originalClear + pausePointStatusPoll = originalPoll + }() + + queryCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + queryCount++ + if queryCount == 1 { + return pausePointStatusResponse{}, context.DeadlineExceeded + } + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 5, + Mode: pausePointModeContinuous, + LastHitSequence: 5, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + clearPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + t.Fatal("baseline timeout must not clear the still-armed continuous marker") + return pausePointStatusResponse{}, nil + } + + stderr := &bytes.Buffer{} + exitCode := runWaitForPausePoint( + context.Background(), + unityipc.Connection{ProjectRoot: "/tmp/MyProject"}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: 40 * time.Millisecond, + resumePlay: true, + }, + &bytes.Buffer{}, + stderr, + ) + if exitCode != 1 { + t.Fatalf("exit code mismatch: %d", exitCode) + } + stderrText := stderr.String() + if !strings.Contains(stderrText, clierrors.ErrorCodePausePointWaitTimeout) { + t.Fatalf("expected %s after a stale continuous Hit, got stderr: %s", clierrors.ErrorCodePausePointWaitTimeout, stderrText) + } + if !strings.Contains(stderrText, pausePointHintAlreadyHitWaitingForNew) { + t.Fatalf("expected already-hit baseline hint, got stderr: %s", stderrText) + } +} + +// Verifies enable --await never baselining a Hit that raced in before the first status query. +func TestWaitForPausePointAcceptsImmediateHitWhenMarkerJustEnabled(t *testing.T) { + originalQuery := queryPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Hour + defer func() { + queryPausePointStatus = originalQuery + pausePointStatusPoll = originalPoll + }() + + queryCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + queryCount++ + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 1, + Mode: pausePointModeContinuous, + LastHitSequence: 1, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + + response, state, _, _, hasNewHitBaseline, err := waitForPausePoint( + context.Background(), + unityipc.Connection{}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + markerJustEnabled: true, + }, + ) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateHit { + t.Fatalf("state mismatch: %s", state) + } + if hasNewHitBaseline { + t.Fatal("enable --await must not establish a new-hit baseline") + } + if response.LastHitSequence != 1 { + t.Fatalf("expected the raced enable-time hit, got %#v", response) + } + if queryCount != 1 { + t.Fatalf("expected a single status query, got %d", queryCount) + } +} + +// Verifies an already-hit single-shot marker still returns immediately (baseline not applied). +func TestWaitForPausePointReturnsImmediatelyForAlreadyHitSingleShotMarker(t *testing.T) { + originalQuery := queryPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Hour + defer func() { + queryPausePointStatus = originalQuery + pausePointStatusPoll = originalPoll + }() + + queryCount := 0 + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + queryCount++ + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 1, + Mode: "single-shot", + LastHitSequence: 1, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + + response, state, _, _, hasNewHitBaseline, err := waitForPausePoint( + context.Background(), + unityipc.Connection{}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + }, + ) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateHit { + t.Fatalf("state mismatch: %s", state) + } + if hasNewHitBaseline { + t.Fatal("single-shot must not establish a new-hit baseline") + } + if response.LastHitSequence != 1 { + t.Fatalf("response mismatch: %#v", response) + } + if queryCount != 1 { + t.Fatalf("expected a single status query, got %d", queryCount) + } +} + +// Verifies the timeout hint for an already-hit baseline is distinct from the generic paused hint. +func TestPausePointTimeoutHintForNewHitBaseline(t *testing.T) { + response := pausePointStatusResponse{ + Id: "jump", + Status: pausePointStatusHit, + Mode: pausePointModeContinuous, + LastHitSequence: 5, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + HitCount: 5, + } + cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + }, response, pausePointWaitStateTimeout, true) + + if cliErr.ErrorCode != clierrors.ErrorCodePausePointWaitTimeout { + t.Fatalf("error code mismatch: %s", cliErr.ErrorCode) + } + if cliErr.Details["Hint"] != pausePointHintAlreadyHitWaitingForNew { + t.Fatalf("hint mismatch: %#v", cliErr.Details) + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable.go b/cli/project-runner/internal/projectrunner/pause_point_enable.go index 50c78da18d..317a400a2b 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable.go @@ -341,6 +341,7 @@ func runEnablePausePointAndAwait( triggerArgs: triggerArgs, startPath: startPath, resumePlay: resumePlay, + markerJustEnabled: true, } return runPausePointWaitAfterEnable( @@ -382,7 +383,7 @@ func runPausePointWaitAfterEnable( stderr io.Writer, ) int { spinner := clicore.NewToolSpinner(stderr, pausePointEnableCommandName) - response, state, triggerResult, resumeResult, err := waitForPausePoint(ctx, connection, options) + response, state, triggerResult, resumeResult, hasNewHitBaseline, err := waitForPausePoint(ctx, connection, options) spinner.Stop() if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ @@ -432,11 +433,14 @@ func runPausePointWaitAfterEnable( return 0 } - if state == pausePointWaitStateTimeout { + // Why skip clear when hasNewHitBaseline: the continuous/trace marker is still armed, and the + // timeout hint tells the caller to await again (with --resume-play). Clearing here would disarm + // it and discard the raw capture holder, making that recovery path impossible. + if state == pausePointWaitStateTimeout && !hasNewHitBaseline { clearPausePointAfterWaitTimeout(ctx, connection, options.id) } - waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state) + waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state, hasNewHitBaseline) waitErr.Command = pausePointEnableCommandName if enableFields.Warning != "" { waitErr.Details["EnableWarning"] = enableFields.Warning diff --git a/cli/project-runner/internal/projectrunner/pause_point_errors.go b/cli/project-runner/internal/projectrunner/pause_point_errors.go index a1484486ca..0905647c0c 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_errors.go +++ b/cli/project-runner/internal/projectrunner/pause_point_errors.go @@ -14,6 +14,7 @@ func pausePointWaitError( options waitForPausePointOptions, response pausePointStatusResponse, state pausePointWaitState, + hasNewHitBaseline bool, ) clierrors.CLIError { response = normalizePausePointStatusResponse(response) @@ -73,7 +74,7 @@ func pausePointWaitError( options, response, true) - hint := pausePointTimeoutHint(response) + hint := pausePointTimeoutHint(response, hasNewHitBaseline) if hint != "" { timeoutError.Details["Hint"] = hint } @@ -110,6 +111,12 @@ const ( pausePointHintPlayModeNotRunning = "PlayMode is not running. Start PlayMode (or trigger the marker code path in Edit Mode), then wait again." pausePointHintEditorAlreadyPaused = "Unity is already paused, so gameplay cannot reach the marker. Resume PlayMode before waiting again." + // Returned when await timed out while waiting for a new hit on an already-hit continuous/trace + // marker. Why not reuse pausePointHintEditorAlreadyPaused: that hint diagnoses a marker that + // never fired, whereas here the marker already hit and the wait needs Play resumed so a later + // sequence can occur. + pausePointHintAlreadyHitWaitingForNew = "The marker had already hit and Unity may still be paused by that hit; pass --resume-play or resume Play Mode so a new hit can occur." + // Shared by both pausePointTimeoutHint and pausePointExpiredHint: patterns where the method // body genuinely ran (or was invoked) yet the marker never fired — a physics/message callback // missing a pre-existing GameObject, a pre-bound delegate bypassing the patch, or control flow @@ -123,7 +130,10 @@ const ( // pausePointTimeoutHint maps the final probed status to a deterministic diagnosis, // because timeouts are where agents struggle to tell a missed code path from Editor state. -func pausePointTimeoutHint(response pausePointStatusResponse) string { +func pausePointTimeoutHint(response pausePointStatusResponse, hasNewHitBaseline bool) string { + if hasNewHitBaseline { + return pausePointHintAlreadyHitWaitingForNew + } if !response.EditorState.IsPlaying { return pausePointHintPlayModeNotRunning } diff --git a/cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go b/cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go index acc5bd08fd..6b3a8af175 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go @@ -132,7 +132,7 @@ func TestWaitForPausePointResumesPlayBeforeTriggerWhenPaused(t *testing.T) { return 0 } - _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -203,7 +203,7 @@ func TestWaitForPausePointSkipsPlayWhenAlreadyUnpaused(t *testing.T) { return 0 } - _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -276,7 +276,7 @@ func TestWaitForPausePointSkipsTriggerWhenResumePlayFails(t *testing.T) { return 0 } - _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -328,7 +328,7 @@ func TestWaitForPausePointSkipsResumeWhenNotArmed(t *testing.T) { return pausePointResumePlayResult{WasPaused: true, Resumed: true} } - _, state, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "does-not-exist", timeoutSeconds: 1, timeout: time.Second, @@ -395,7 +395,7 @@ func TestWaitForPausePointSkipsResumeAndTriggerWhenNotArmed(t *testing.T) { return 0 } - _, state, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "does-not-exist", timeoutSeconds: 1, timeout: time.Second, @@ -469,7 +469,7 @@ func TestWaitForPausePointResumesWithoutTriggerWhenArmed(t *testing.T) { return 0 } - _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, diff --git a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go index 0a6e48257a..dbcedc62a6 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_trigger_test.go @@ -271,7 +271,7 @@ func TestWaitForPausePointJoinsTriggerResult(t *testing.T) { return 0 } - _, _, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -307,7 +307,7 @@ func TestWaitForPausePointJoinsTriggerResult(t *testing.T) { return 0 } - _, _, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -362,7 +362,7 @@ func TestWaitForPausePointSkipsTriggerWhenNotArmed(t *testing.T) { return 0 } - _, state, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "does-not-exist", timeoutSeconds: 1, timeout: time.Second, diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index e8f29efc75..44c657d6e4 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -26,6 +26,12 @@ const ( pausePointStatusNotEnabled = "NotEnabled" pausePointStatusExpired = "Expired" pausePointStatusCleared = "Cleared" + + // Mode strings mirror UloopPausePointCaptureMode on the Unity side. Await uses an allowlist + // (continuous/trace) for the new-hit baseline — never `Mode != "single-shot"` — so an empty + // Mode from an older package keeps the historical immediate-Hit success path. + pausePointModeContinuous = "continuous" + pausePointModeTrace = "trace" ) var ( @@ -55,6 +61,11 @@ type waitForPausePointOptions struct { // (if paused) before dispatching --trigger so a paused-arm workflow can fire input triggers // in one CLI call. resumePlay bool + + // markerJustEnabled is set only by enable-pause-point --await. Why: enable finishes before the + // first status query, and a real hit can race into that window on continuous/trace markers. + // Baselining that hit would demand a later sequence that never comes (continuous pauses on hit). + markerJustEnabled bool } type pausePointStatusOptions struct { @@ -208,7 +219,7 @@ func runWaitForPausePoint( ) int { startedAt := time.Now() spinner := clicore.NewToolSpinner(stderr, clicore.PausePointAwaitCommandName) - response, state, triggerResult, resumeResult, err := waitForPausePoint(ctx, connection, options) + response, state, triggerResult, resumeResult, hasNewHitBaseline, err := waitForPausePoint(ctx, connection, options) spinner.Stop() if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ @@ -253,11 +264,14 @@ func runWaitForPausePoint( return 0 } - if state == pausePointWaitStateTimeout { + // Why skip clear when hasNewHitBaseline: the continuous/trace marker is still armed, and the + // timeout hint tells the caller to await again (with --resume-play). Clearing here would disarm + // it and discard the raw capture holder, making that recovery path impossible. + if state == pausePointWaitStateTimeout && !hasNewHitBaseline { clearPausePointAfterWaitTimeout(ctx, connection, options.id) } - waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state) + waitErr := pausePointWaitError(connection.ProjectRoot, options, response, state, hasNewHitBaseline) if triggerResult != nil { waitErr.Details["TriggerResult"] = triggerResult } diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go index e9da76523e..72cf2beac6 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go @@ -38,14 +38,19 @@ const ( // on it. Once confirmed armed (or already hit), optional resume runs synchronously, then the // trigger races the status poll loop and is joined once the wait itself settles, so a slow trigger // cannot delay reporting a pause-point hit. +// +// hasNewHitBaseline is true when the wait started against an already-hit continuous/trace marker +// and must observe a later LastHitSequence before succeeding. Callers pass it into timeout error +// construction so the hint can explain why a still-Hit marker did not count as a wait success. func waitForPausePoint( ctx context.Context, connection unityipc.Connection, options waitForPausePointOptions, -) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, *pausePointResumePlayResult, error) { - triggerHandle, skippedTriggerResult, resumeResult := startPausePointWaitSideEffects(ctx, connection, options) +) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, *pausePointResumePlayResult, bool, error) { + triggerHandle, skippedTriggerResult, resumeResult, baselineSequence, hasBaseline, baselineDecided := startPausePointWaitSideEffects(ctx, connection, options) - response, state, polledTriggerResult, err := waitForPausePointStatus(ctx, connection, options, triggerHandle) + response, state, polledTriggerResult, hasBaseline, err := waitForPausePointStatus( + ctx, connection, options, triggerHandle, baselineSequence, hasBaseline, baselineDecided) if state == pausePointWaitStateTriggerFailed && resumeResult != nil && resumeResult.Resumed { repaused := repausePlayModeAfterAbandonedWait(ctx, connection, *resumeResult) @@ -53,32 +58,36 @@ func waitForPausePoint( } if skippedTriggerResult != nil { - return response, state, skippedTriggerResult, resumeResult, err + return response, state, skippedTriggerResult, resumeResult, hasBaseline, err } // The trigger goroutine's buffered channel yields exactly one value, so a result already // received by the poll loop must be reused here instead of joining again — a second receive // would block for the whole grace window and then report Completed:false over a real result. if polledTriggerResult != nil { - return response, state, polledTriggerResult, resumeResult, err + return response, state, polledTriggerResult, resumeResult, hasBaseline, err } if triggerHandle != nil { - return response, state, triggerHandle.join(), resumeResult, err + return response, state, triggerHandle.join(), resumeResult, hasBaseline, err } - return response, state, nil, resumeResult, err + return response, state, nil, resumeResult, hasBaseline, err } // startPausePointWaitSideEffects performs the pre-wait --resume-play / --trigger work, returning // either a live trigger handle or the fixed TriggerResult explaining why the trigger was skipped. +// When it queries status to confirm arming, that response is the wait-start snapshot for the +// new-hit baseline (decided before --resume-play), so a post-resume hit advances LastHitSequence +// past the baseline. baselineDecided is false only when no arming query ran (plain await). func startPausePointWaitSideEffects( ctx context.Context, connection unityipc.Connection, options waitForPausePointOptions, -) (*pausePointTriggerHandle, *pausePointTriggerResult, *pausePointResumePlayResult) { +) (*pausePointTriggerHandle, *pausePointTriggerResult, *pausePointResumePlayResult, int, bool, bool) { if options.triggerCommand == "" && !options.resumePlay { - return nil, nil, nil + return nil, nil, nil, -1, false, false } - if !pausePointIsArmed(ctx, connection, options.id) { + armResponse, armed := queryPausePointArmStatus(ctx, connection, options.id) + if !armed { var resumeResult *pausePointResumePlayResult var skippedTriggerResult *pausePointTriggerResult if options.resumePlay { @@ -99,42 +108,51 @@ func startPausePointWaitSideEffects( Error: "trigger was not dispatched: the marker could not be confirmed armed at wait start", } } - return nil, skippedTriggerResult, resumeResult + // Why baselineDecided=false: a transient arm-query failure is also reported as not armed. + // Leaving baseline undecided lets the first successful poll establish it, so a stale + // continuous Hit cannot slip through as an immediate wait success. + return nil, skippedTriggerResult, resumeResult, -1, false, false } + baselineSequence, hasBaseline, baselineDecided := decidePausePointNewHitBaseline(armResponse, options.markerJustEnabled) + var resumeResult *pausePointResumePlayResult if options.resumePlay { result := resumePlayModeForPausePoint(ctx, connection) resumeResult = &result if result.Error != "" { if options.triggerCommand == "" { - return nil, nil, resumeResult + return nil, nil, resumeResult, baselineSequence, hasBaseline, baselineDecided } return nil, &pausePointTriggerResult{ Command: pausePointTriggerCommandString(options.triggerCommand, options.triggerArgs), Error: "trigger was not dispatched: --resume-play failed to resume play mode", - }, resumeResult + }, resumeResult, baselineSequence, hasBaseline, baselineDecided } } if options.triggerCommand == "" { - return nil, nil, resumeResult + return nil, nil, resumeResult, baselineSequence, hasBaseline, baselineDecided } handle := startPausePointTrigger(ctx, connection, options.startPath, options.triggerCommand, options.triggerArgs) - return handle, nil, resumeResult + return handle, nil, resumeResult, baselineSequence, hasBaseline, baselineDecided } -// pausePointIsArmed reports whether the marker is enabled or already hit. A query failure is +// queryPausePointArmStatus reports whether the marker is enabled or already hit. A query failure is // treated as not armed: dispatching a --trigger command against a marker this CLI cannot even // confirm exists would inject the trigger's action into the game with no corresponding wait. -func pausePointIsArmed(ctx context.Context, connection unityipc.Connection, id string) bool { +func queryPausePointArmStatus( + ctx context.Context, + connection unityipc.Connection, + id string, +) (pausePointStatusResponse, bool) { response, err := queryPausePointStatus(ctx, connection, id) if err != nil { - return false + return pausePointStatusResponse{}, false } state := pausePointWaitStateForStatus(response.Status) - return state == "" || state == pausePointWaitStateHit + return response, state == "" || state == pausePointWaitStateHit } func waitForPausePointStatus( @@ -142,7 +160,10 @@ func waitForPausePointStatus( connection unityipc.Connection, options waitForPausePointOptions, triggerHandle *pausePointTriggerHandle, -) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, error) { + baselineSequence int, + hasBaseline bool, + baselineDecided bool, +) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, bool, error) { waitContext, cancel := context.WithTimeout(ctx, options.timeout) defer cancel() @@ -158,16 +179,23 @@ func waitForPausePointStatus( if err == nil { lastResponse = response hasResponse = true - state := pausePointWaitStateForStatus(response.Status) + // Why only once: a later Enabled→Hit transition is the await success itself + // (enable --await). Re-baselining on that first mid-wait Hit would demand a second + // sequence bump and never return. + if !baselineDecided { + baselineSequence, hasBaseline, baselineDecided = decidePausePointNewHitBaseline( + response, options.markerJustEnabled) + } + state := pausePointWaitStateForPolledStatus(response, baselineSequence, hasBaseline) if state != "" { - return response, state, triggerResult, nil + return response, state, triggerResult, hasBaseline, nil } } else { // Why abort: every poll dials again, so a connect the operating system refused // permanently keeps failing for the whole --timeout and the refusal is reported only // after that wait is spent. if clierrors.IsPermanentConnectError(err) { - return lastResponse, "", triggerResult, err + return lastResponse, "", triggerResult, hasBaseline, err } lastErr = err } @@ -175,25 +203,31 @@ func waitForPausePointStatus( select { case <-waitContext.Done(): if ctx.Err() != nil { - return lastResponse, "", triggerResult, ctx.Err() + return lastResponse, "", triggerResult, hasBaseline, ctx.Err() } - finalResponse, finalState, hasFinalResponse, finalErr := queryPausePointStatusAtTimeout(ctx, connection, options.id) + finalResponse, finalState, hasFinalResponse, finalErr := queryPausePointStatusAtTimeout( + ctx, connection, options.id, baselineSequence, hasBaseline) if hasFinalResponse { lastResponse = finalResponse hasResponse = true + if !baselineDecided { + baselineSequence, hasBaseline, _ = decidePausePointNewHitBaseline( + finalResponse, options.markerJustEnabled) + finalState = pausePointWaitStateForPolledStatus(finalResponse, baselineSequence, hasBaseline) + } if finalState != "" { - return finalResponse, finalState, triggerResult, nil + return finalResponse, finalState, triggerResult, hasBaseline, nil } } else if lastErr == nil { lastErr = finalErr } if hasResponse { - return lastResponse, pausePointWaitStateTimeout, triggerResult, nil + return lastResponse, pausePointWaitStateTimeout, triggerResult, hasBaseline, nil } if lastErr != nil { - return lastResponse, "", triggerResult, fmt.Errorf("timed out waiting for pause point status: %w", lastErr) + return lastResponse, "", triggerResult, hasBaseline, fmt.Errorf("timed out waiting for pause point status: %w", lastErr) } - return lastResponse, pausePointWaitStateTimeout, triggerResult, nil + return lastResponse, pausePointWaitStateTimeout, triggerResult, hasBaseline, nil case result := <-triggerDone: // Nil the channel so this case can never fire twice: the handle's channel holds a single // buffered value, and the caller reuses the result received here instead of joining. @@ -201,8 +235,8 @@ func waitForPausePointStatus( triggerDone = nil if pausePointTriggerRejectedBeforeExecution(result) { abortResponse, abortState := abortPausePointWaitAfterTriggerRejection( - ctx, connection, options.id, lastResponse) - return abortResponse, abortState, triggerResult, nil + ctx, connection, options.id, lastResponse, baselineSequence, hasBaseline) + return abortResponse, abortState, triggerResult, hasBaseline, nil } case <-ticker.C: } @@ -217,8 +251,11 @@ func abortPausePointWaitAfterTriggerRejection( connection unityipc.Connection, id string, lastResponse pausePointStatusResponse, + baselineSequence int, + hasBaseline bool, ) (pausePointStatusResponse, pausePointWaitState) { - response, state, hasResponse, _ := queryPausePointStatusAtTimeout(ctx, connection, id) + response, state, hasResponse, _ := queryPausePointStatusAtTimeout( + ctx, connection, id, baselineSequence, hasBaseline) if !hasResponse { return lastResponse, pausePointWaitStateTriggerFailed } @@ -232,6 +269,8 @@ func queryPausePointStatusAtTimeout( ctx context.Context, connection unityipc.Connection, id string, + baselineSequence int, + hasBaseline bool, ) (pausePointStatusResponse, pausePointWaitState, bool, error) { finalContext, cancel := context.WithTimeout(ctx, pausePointFinalStatusProbeTimeout) defer cancel() @@ -241,7 +280,50 @@ func queryPausePointStatusAtTimeout( return pausePointStatusResponse{}, "", false, err } - return response, pausePointWaitStateForStatus(response.Status), true, nil + return response, pausePointWaitStateForPolledStatus(response, baselineSequence, hasBaseline), true, nil +} + +// decidePausePointNewHitBaseline returns (sequence, hasBaseline, decided) for the wait-start +// snapshot. markerJustEnabled forces no baseline: any Hit during enable --await is the success +// itself, including a race where the first status query already observes sequence 1. +func decidePausePointNewHitBaseline( + response pausePointStatusResponse, + markerJustEnabled bool, +) (int, bool, bool) { + if markerJustEnabled { + return -1, false, true + } + sequence, hasBaseline := pausePointNewHitBaseline(response) + return sequence, hasBaseline, true +} + +// pausePointNewHitBaseline records LastHitSequence when await starts against an already-hit +// continuous/trace marker. Mode is allowlisted (never `!= "single-shot"`): an empty Mode is the +// old-package skew case and must keep the historical immediate-Hit success path. +func pausePointNewHitBaseline(response pausePointStatusResponse) (int, bool) { + if response.Status != pausePointStatusHit { + return -1, false + } + if response.Mode != pausePointModeContinuous && response.Mode != pausePointModeTrace { + return -1, false + } + return response.LastHitSequence, true +} + +// pausePointWaitStateForPolledStatus maps a polled status to a terminal wait state, treating an +// already-hit continuous/trace marker as non-terminal until LastHitSequence advances past baseline. +func pausePointWaitStateForPolledStatus( + response pausePointStatusResponse, + baselineSequence int, + hasBaseline bool, +) pausePointWaitState { + if response.Status == pausePointStatusHit { + if !hasBaseline || response.LastHitSequence > baselineSequence { + return pausePointWaitStateHit + } + return "" + } + return pausePointWaitStateForStatus(response.Status) } func pausePointWaitStateForStatus(status string) pausePointWaitState { diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go b/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go index 6882040ca0..8c783bd6a2 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_poll_test.go @@ -76,7 +76,7 @@ func TestWaitForPausePointAbortsWhenTheConnectIsRefused(t *testing.T) { } startedAt := time.Now() - _, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, _, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 10 * time.Second, @@ -128,7 +128,7 @@ func TestWaitForPausePointAbortsWhenTriggerRejectsArguments(t *testing.T) { } startedAt := time.Now() - _, state, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 10 * time.Second, @@ -363,7 +363,7 @@ func TestWaitForPausePointDoesNotAbortWhenTriggerSucceeds(t *testing.T) { return 0 } - _, state, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 50 * time.Millisecond, @@ -430,7 +430,7 @@ func TestWaitForPausePointReportsHitRacingATriggerRejection(t *testing.T) { return 1 } - response, state, triggerResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + response, state, triggerResult, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 10 * time.Second, @@ -677,7 +677,7 @@ func TestWaitForPausePointDoesNotRepauseWhenResumeWasANoOp(t *testing.T) { return 1 } - _, state, _, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, _, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 10 * time.Second, @@ -742,7 +742,7 @@ func TestWaitForPausePointDoesNotRepauseWhenItDidNotResume(t *testing.T) { return 1 } - _, state, _, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + _, state, _, resumeResult, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 60, timeout: 10 * time.Second, diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go index 275927d1e3..ea2e2f7e67 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_test.go @@ -172,7 +172,7 @@ func TestWaitForPausePointReturnsHitAfterEnabledStatus(t *testing.T) { return response, nil } - response, state, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + response, state, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -284,7 +284,7 @@ func TestPausePointExpiredErrorReportsRecoveryFields(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, response, pausePointWaitStateExpired) + }, response, pausePointWaitStateExpired, false) if cliErr.Details["Expired"] != true { t.Fatalf("expired detail mismatch: %#v", cliErr.Details) @@ -313,7 +313,7 @@ func TestPausePointExpiredErrorReportsMarkerTimeoutSeconds(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 5, - }, response, pausePointWaitStateExpired) + }, response, pausePointWaitStateExpired, false) if cliErr.Details["TimeoutSeconds"] != 30 { t.Fatalf("timeoutSeconds detail mismatch: %#v", cliErr.Details) @@ -332,7 +332,7 @@ func TestPausePointExpiredErrorDerivesExpiredFromStatus(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, response, pausePointWaitStateExpired) + }, response, pausePointWaitStateExpired, false) if cliErr.Details["Expired"] != true { t.Fatalf("expired detail mismatch: %#v", cliErr.Details) @@ -426,7 +426,7 @@ func TestWaitForPausePointReturnsNotEnabledStateImmediately(t *testing.T) { }, nil } - response, state, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + response, state, _, _, _, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, timeout: time.Second, @@ -962,7 +962,7 @@ func TestPausePointTimeoutErrorIncludesDiagnosisHint(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, testCase.response, pausePointWaitStateTimeout) + }, testCase.response, pausePointWaitStateTimeout, false) if cliErr.Details["Hint"] != testCase.wantHint { t.Fatalf("hint mismatch: %#v", cliErr.Details) @@ -1011,7 +1011,7 @@ func TestPausePointExpiredErrorIncludesDiagnosisHint(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, testCase.response, pausePointWaitStateExpired) + }, testCase.response, pausePointWaitStateExpired, false) if cliErr.Details["Hint"] != testCase.wantHint { t.Fatalf("hint mismatch: %#v", cliErr.Details) @@ -1031,7 +1031,7 @@ func TestPausePointHintIsOmittedOutsideDiagnosableStates(t *testing.T) { timeoutErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, hitResponse, pausePointWaitStateTimeout) + }, hitResponse, pausePointWaitStateTimeout, false) if _, exists := timeoutErr.Details["Hint"]; exists { t.Fatalf("hint should be omitted when no diagnosis applies: %#v", timeoutErr.Details) } @@ -1044,7 +1044,7 @@ func TestPausePointHintIsOmittedOutsideDiagnosableStates(t *testing.T) { clearedErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, clearedResponse, pausePointWaitStateCleared) + }, clearedResponse, pausePointWaitStateCleared, false) if _, exists := clearedErr.Details["Hint"]; exists { t.Fatalf("hint should be omitted for cleared markers: %#v", clearedErr.Details) } @@ -1064,7 +1064,7 @@ func TestPausePointExpiredErrorReportsNoRemainingTime(t *testing.T) { cliErr := pausePointWaitError("/tmp/MyProject", waitForPausePointOptions{ id: "jump", timeoutSeconds: 1, - }, response, pausePointWaitStateExpired) + }, response, pausePointWaitStateExpired, false) if cliErr.ErrorCode != clierrors.ErrorCodePausePointExpired { t.Fatalf("error code mismatch: %#v", cliErr) From 2cb7e44339113a77ef3486c1df056f4168c638ab Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 12:23:32 +0900 Subject: [PATCH 02/10] fix: separate enable-time patch warnings from pause point hit responses Co-authored-by: Cursor --- .../references/captured-variables.md | 2 +- .../references/troubleshooting.md | 2 +- .../references/captured-variables.md | 2 +- .../references/troubleshooting.md | 2 +- .../Skill/references/captured-variables.md | 2 +- .../Skill/references/troubleshooting.md | 2 +- .../projectrunner/pause_point_enable.go | 15 +-- ...pause_point_enable_await_diagnosis_test.go | 26 ++--- .../projectrunner/pause_point_enable_test.go | 7 +- .../pause_point_enable_time_warning_test.go | 100 ++++++++++++++++++ .../projectrunner/pause_point_logs.go | 20 ++-- .../projectrunner/pause_point_types.go | 6 +- 12 files changed, 142 insertions(+), 44 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/pause_point_enable_time_warning_test.go diff --git a/.agents/skills/uloop-pause-point/references/captured-variables.md b/.agents/skills/uloop-pause-point/references/captured-variables.md index 6588496374..401c3e08e7 100644 --- a/.agents/skills/uloop-pause-point/references/captured-variables.md +++ b/.agents/skills/uloop-pause-point/references/captured-variables.md @@ -86,6 +86,6 @@ For a self-progressing game, arranging a scenario through real input alone is a ## Warnings and Marker Freshness -`await-pause-point`'s hit response also carries a top-level `Warning` (omitted when empty): it flags multiple hits, multiple matching logs, or truncated matching logs, so you can tell a single clean hit apart from evidence that needs closer inspection. `MatchingLogs` (log entries whose text contains the marker id) is still embedded, but source-derived ids rarely appear in log text, so treat `CapturedVariables` as the primary variable evidence. +`await-pause-point`'s hit response also carries a top-level `Warning` (omitted when empty): it flags multiple hits, multiple matching logs, or truncated matching logs, so you can tell a single clean hit apart from evidence that needs closer inspection. Enable-time patch diagnostics (for example physics-callback cached dispatch) are not in `Warning`; on `enable-pause-point --await` they appear as `EnableTimeWarning` instead. `MatchingLogs` (log entries whose text contains the marker id) is still embedded, but source-derived ids rarely appear in log text, so treat `CapturedVariables` as the primary variable evidence. Use `Generation`, `EnabledAtUtc`, and the hit sequence fields from the hit or status response to tell a fresh marker from stale evidence with the same id. `RemainingMilliseconds` and `Expired` are returned directly so you do not need to infer marker lifetime from elapsed time. diff --git a/.agents/skills/uloop-pause-point/references/troubleshooting.md b/.agents/skills/uloop-pause-point/references/troubleshooting.md index b828446156..abd5c9a162 100644 --- a/.agents/skills/uloop-pause-point/references/troubleshooting.md +++ b/.agents/skills/uloop-pause-point/references/troubleshooting.md @@ -18,7 +18,7 @@ Mono can inline very small target methods into callers, and the pause point then ## Physics Message Methods and One-Hop Helpers -Physical Unity message methods (`OnCollisionEnter2D`, `OnTriggerEnter2D`, and similar callbacks) can silently miss: a GameObject that already existed at enable time may keep calling the pre-patch code, so `HitCount` stays `0` even though the method body runs. The condition is environment-dependent, and the response `Warning` flags such lines at enable time. The same applies one hop out — a helper called from a physics message method in the same compiled assembly; deeper call chains or callers in other assemblies are not detected by the warning but can fail the same way. +Physical Unity message methods (`OnCollisionEnter2D`, `OnTriggerEnter2D`, and similar callbacks) can silently miss: a GameObject that already existed at enable time may keep calling the pre-patch code, so `HitCount` stays `0` even though the method body runs. The condition is environment-dependent. On `enable-pause-point --await`, that enable-time patch diagnostic appears as top-level `EnableTimeWarning` (omitted when empty) — it is independent of whether the marker later hits, and it is not folded into hit-time `Warning`. On a non-hit failure, the same text is under `Error.Details.EnableWarning`. The same applies one hop out — a helper called from a physics message method in the same compiled assembly; deeper call chains or callers in other assemblies are not detected by the warning but can fail the same way. Recovery order: diff --git a/.claude/skills/uloop-pause-point/references/captured-variables.md b/.claude/skills/uloop-pause-point/references/captured-variables.md index 6588496374..401c3e08e7 100644 --- a/.claude/skills/uloop-pause-point/references/captured-variables.md +++ b/.claude/skills/uloop-pause-point/references/captured-variables.md @@ -86,6 +86,6 @@ For a self-progressing game, arranging a scenario through real input alone is a ## Warnings and Marker Freshness -`await-pause-point`'s hit response also carries a top-level `Warning` (omitted when empty): it flags multiple hits, multiple matching logs, or truncated matching logs, so you can tell a single clean hit apart from evidence that needs closer inspection. `MatchingLogs` (log entries whose text contains the marker id) is still embedded, but source-derived ids rarely appear in log text, so treat `CapturedVariables` as the primary variable evidence. +`await-pause-point`'s hit response also carries a top-level `Warning` (omitted when empty): it flags multiple hits, multiple matching logs, or truncated matching logs, so you can tell a single clean hit apart from evidence that needs closer inspection. Enable-time patch diagnostics (for example physics-callback cached dispatch) are not in `Warning`; on `enable-pause-point --await` they appear as `EnableTimeWarning` instead. `MatchingLogs` (log entries whose text contains the marker id) is still embedded, but source-derived ids rarely appear in log text, so treat `CapturedVariables` as the primary variable evidence. Use `Generation`, `EnabledAtUtc`, and the hit sequence fields from the hit or status response to tell a fresh marker from stale evidence with the same id. `RemainingMilliseconds` and `Expired` are returned directly so you do not need to infer marker lifetime from elapsed time. diff --git a/.claude/skills/uloop-pause-point/references/troubleshooting.md b/.claude/skills/uloop-pause-point/references/troubleshooting.md index b828446156..abd5c9a162 100644 --- a/.claude/skills/uloop-pause-point/references/troubleshooting.md +++ b/.claude/skills/uloop-pause-point/references/troubleshooting.md @@ -18,7 +18,7 @@ Mono can inline very small target methods into callers, and the pause point then ## Physics Message Methods and One-Hop Helpers -Physical Unity message methods (`OnCollisionEnter2D`, `OnTriggerEnter2D`, and similar callbacks) can silently miss: a GameObject that already existed at enable time may keep calling the pre-patch code, so `HitCount` stays `0` even though the method body runs. The condition is environment-dependent, and the response `Warning` flags such lines at enable time. The same applies one hop out — a helper called from a physics message method in the same compiled assembly; deeper call chains or callers in other assemblies are not detected by the warning but can fail the same way. +Physical Unity message methods (`OnCollisionEnter2D`, `OnTriggerEnter2D`, and similar callbacks) can silently miss: a GameObject that already existed at enable time may keep calling the pre-patch code, so `HitCount` stays `0` even though the method body runs. The condition is environment-dependent. On `enable-pause-point --await`, that enable-time patch diagnostic appears as top-level `EnableTimeWarning` (omitted when empty) — it is independent of whether the marker later hits, and it is not folded into hit-time `Warning`. On a non-hit failure, the same text is under `Error.Details.EnableWarning`. The same applies one hop out — a helper called from a physics message method in the same compiled assembly; deeper call chains or callers in other assemblies are not detected by the warning but can fail the same way. Recovery order: diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md index 6588496374..401c3e08e7 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md @@ -86,6 +86,6 @@ For a self-progressing game, arranging a scenario through real input alone is a ## Warnings and Marker Freshness -`await-pause-point`'s hit response also carries a top-level `Warning` (omitted when empty): it flags multiple hits, multiple matching logs, or truncated matching logs, so you can tell a single clean hit apart from evidence that needs closer inspection. `MatchingLogs` (log entries whose text contains the marker id) is still embedded, but source-derived ids rarely appear in log text, so treat `CapturedVariables` as the primary variable evidence. +`await-pause-point`'s hit response also carries a top-level `Warning` (omitted when empty): it flags multiple hits, multiple matching logs, or truncated matching logs, so you can tell a single clean hit apart from evidence that needs closer inspection. Enable-time patch diagnostics (for example physics-callback cached dispatch) are not in `Warning`; on `enable-pause-point --await` they appear as `EnableTimeWarning` instead. `MatchingLogs` (log entries whose text contains the marker id) is still embedded, but source-derived ids rarely appear in log text, so treat `CapturedVariables` as the primary variable evidence. Use `Generation`, `EnabledAtUtc`, and the hit sequence fields from the hit or status response to tell a fresh marker from stale evidence with the same id. `RemainingMilliseconds` and `Expired` are returned directly so you do not need to infer marker lifetime from elapsed time. diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md index b828446156..abd5c9a162 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/troubleshooting.md @@ -18,7 +18,7 @@ Mono can inline very small target methods into callers, and the pause point then ## Physics Message Methods and One-Hop Helpers -Physical Unity message methods (`OnCollisionEnter2D`, `OnTriggerEnter2D`, and similar callbacks) can silently miss: a GameObject that already existed at enable time may keep calling the pre-patch code, so `HitCount` stays `0` even though the method body runs. The condition is environment-dependent, and the response `Warning` flags such lines at enable time. The same applies one hop out — a helper called from a physics message method in the same compiled assembly; deeper call chains or callers in other assemblies are not detected by the warning but can fail the same way. +Physical Unity message methods (`OnCollisionEnter2D`, `OnTriggerEnter2D`, and similar callbacks) can silently miss: a GameObject that already existed at enable time may keep calling the pre-patch code, so `HitCount` stays `0` even though the method body runs. The condition is environment-dependent. On `enable-pause-point --await`, that enable-time patch diagnostic appears as top-level `EnableTimeWarning` (omitted when empty) — it is independent of whether the marker later hits, and it is not folded into hit-time `Warning`. On a non-hit failure, the same text is under `Error.Details.EnableWarning`. The same applies one hop out — a helper called from a physics message method in the same compiled assembly; deeper call chains or callers in other assemblies are not detected by the warning but can fail the same way. Recovery order: diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable.go b/cli/project-runner/internal/projectrunner/pause_point_enable.go index 317a400a2b..e20413243e 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable.go @@ -410,13 +410,15 @@ func runPausePointWaitAfterEnable( response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) logs, logsErr := fetchMatchingLogs(ctx, connection, options.id, options.matchingLogsMaxCount) - // Unity's warning can come from either the enable response or the status poll that observed - // the hit, so both are passed; the join drops the repeat when they carry the same text. + // Why not join enableFields.Warning into Warning: that text is an enable-time patch + // diagnostic (for example "may not hit on pre-existing GameObjects") and contradicts a + // successful hit when folded into the hit-time Warning. It is exposed separately. payload := buildPausePointHitPayload(pausePointHitPayloadInputs{ response: response, logs: logs, logsErr: logsErr, - unityWarning: joinPausePointWarnings(enableFields.Warning, response.Warning), + unityWarning: response.Warning, + enableTimeWarning: enableFields.Warning, triggerResult: triggerResult, awaitedPausePointID: options.id, expectations: expectations, @@ -465,10 +467,9 @@ func runPausePointWaitAfterEnable( return 1 } -// joinPausePointWarnings concatenates the warnings that apply to one response, dropping empty ones -// and repeats. Repeats are possible because the same text can reach a hit payload from two sources — -// the enable response and the status poll that observed the hit — and printing it twice reads as two -// separate problems. +// joinPausePointWarnings concatenates hit-time warnings for one response, dropping empty ones and +// repeats. Inputs are status-poll text (usually empty), matching-logs diagnosis, and trigger-refusal +// text — enable-time patch diagnostics are not joined here; they use EnableTimeWarning. func joinPausePointWarnings(warnings ...string) string { unique := make([]string, 0, len(warnings)) for _, warning := range warnings { diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable_await_diagnosis_test.go b/cli/project-runner/internal/projectrunner/pause_point_enable_await_diagnosis_test.go index ac876ab4aa..e8d6faeeed 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable_await_diagnosis_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable_await_diagnosis_test.go @@ -61,8 +61,8 @@ func TestRunPausePointWaitAfterEnableWarnsWhenTheTriggerWasRefusedByThisMarker(t } } -// Verifies the enable-time warning survives next to the CLI's refusal warning, and that the -// refusal warning also survives a failed matching-log fetch. +// Verifies the enable-time warning is exposed as EnableTimeWarning (not folded into Warning) next +// to the CLI's refusal warning, and that both survive a failed matching-log fetch. func TestRunPausePointWaitAfterEnableKeepsEnableWarningWithTheRefusalWarning(t *testing.T) { stubPausePointHit(t, "") stubPausePointMatchingLogs(t, errors.New("unity busy")) @@ -74,25 +74,13 @@ func TestRunPausePointWaitAfterEnableKeepsEnableWarningWithTheRefusalWarning(t * t.Errorf("a failed fetch must omit MatchingLogs entirely: %s", output) } result := decodePausePointWaitResult(t, output) - if !strings.Contains(result.Warning, "Enable-time warning.") { - t.Errorf("the enable-time warning was dropped: %q", result.Warning) + if result.EnableTimeWarning != "Enable-time warning." { + t.Errorf("enable-time warning mismatch: %q", result.EnableTimeWarning) + } + if strings.Contains(result.Warning, "Enable-time warning.") { + t.Errorf("enable-time warning must not be folded into Warning: %q", result.Warning) } if !strings.Contains(result.Warning, "refused") { t.Errorf("the refusal warning was dropped: %q", result.Warning) } } - -// Verifies a warning reported by both the enable response and the status poll is printed once: -// repeating identical text reads as two separate problems. -func TestRunPausePointWaitAfterEnableReportsARepeatedUnityWarningOnce(t *testing.T) { - stubPausePointHit(t, "Same Unity warning.") - stubPausePointMatchingLogs(t, nil) - stubPausePointTriggerDispatch(t, `{"Success":true}`) - - _, output := runEnableAwaitWithStubbedTrigger(t, "Same Unity warning.") - - result := decodePausePointWaitResult(t, output) - if strings.Count(result.Warning, "Same Unity warning.") != 1 { - t.Errorf("expected the repeated warning exactly once: %q", result.Warning) - } -} diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable_test.go b/cli/project-runner/internal/projectrunner/pause_point_enable_test.go index 6b512addaf..01f39538c5 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable_test.go @@ -203,8 +203,11 @@ func TestRunEnablePausePointCommandAwaitsAfterSuccessfulEnable(t *testing.T) { if response.Status != pausePointStatusHit || response.HitCount != 1 { t.Fatalf("response mismatch: %#v", response) } - if !strings.Contains(response.Warning, "cached message dispatch warning") { - t.Fatalf("expected enable-time warning to be propagated, got: %q", response.Warning) + if !strings.Contains(response.EnableTimeWarning, "cached message dispatch warning") { + t.Fatalf("expected enable-time warning on EnableTimeWarning, got: %q", response.EnableTimeWarning) + } + if strings.Contains(response.Warning, "cached message dispatch warning") { + t.Fatalf("enable-time warning must not be folded into Warning: %q", response.Warning) } if statusCallCount != 2 { t.Fatalf("status call count mismatch: %d", statusCallCount) diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable_time_warning_test.go b/cli/project-runner/internal/projectrunner/pause_point_enable_time_warning_test.go new file mode 100644 index 0000000000..b77a994c2e --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_enable_time_warning_test.go @@ -0,0 +1,100 @@ +package projectrunner + +import ( + "bytes" + "context" + "encoding/json" + "strings" + "testing" + "time" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +const enableTimePatchWarning = "GameObjects that already existed at enable time may never reach the marker." + +// Verifies a successful enable --await hit keeps the enable-time patch warning out of Warning and +// exposes it on EnableTimeWarning instead. +func TestRunPausePointWaitAfterEnableSeparatesEnableTimeWarningOnHit(t *testing.T) { + stubPausePointHit(t, "") + stubPausePointMatchingLogs(t, nil) + stubPausePointTriggerDispatch(t, `{"Success":true}`) + + code, output := runEnableAwaitWithStubbedTrigger(t, enableTimePatchWarning) + if code != 0 { + t.Fatalf("expected hit success, got %d: %s", code, output) + } + result := decodePausePointWaitResult(t, output) + if result.EnableTimeWarning != enableTimePatchWarning { + t.Fatalf("EnableTimeWarning mismatch: %q", result.EnableTimeWarning) + } + if strings.Contains(result.Warning, enableTimePatchWarning) { + t.Fatalf("enable-time warning must not appear in Warning: %q", result.Warning) + } +} + +// Verifies a non-hit enable --await path still surfaces the enable-time warning on the existing +// Details.EnableWarning key (unchanged; not folded into Details.Warning). +func TestRunPausePointWaitAfterEnableKeepsEnableWarningDetailOnExpired(t *testing.T) { + originalQuery := queryPausePointStatus + originalPoll := pausePointStatusPoll + pausePointStatusPoll = time.Millisecond + defer func() { + queryPausePointStatus = originalQuery + pausePointStatusPoll = originalPoll + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusExpired, + Expired: true, + EditorState: pausePointEditorState{IsPlaying: true, CapturedAt: "Current"}, + Message: "Pause point expired before it was hit.", + }, nil + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runPausePointWaitAfterEnable( + context.Background(), + unityipc.Connection{ProjectRoot: "/tmp/MyProject"}, + waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + matchingLogsMaxCount: 5, + markerJustEnabled: true, + }, + enablePausePointPropagatedFields{Warning: enableTimePatchWarning}, + &stdout, + &stderr, + ) + if code != 1 { + t.Fatalf("expected non-hit failure, got %d stdout=%s stderr=%s", code, stdout.String(), stderr.String()) + } + + var envelope struct { + Error clierrors.CLIError `json:"Error"` + } + if err := json.Unmarshal(stderr.Bytes(), &envelope); err != nil { + t.Fatalf("unmarshal stderr: %v (%s)", err, stderr.String()) + } + if envelope.Error.ErrorCode != clierrors.ErrorCodePausePointExpired { + t.Fatalf("error code mismatch: %s", envelope.Error.ErrorCode) + } + enableWarning, ok := envelope.Error.Details["EnableWarning"].(string) + if !ok || enableWarning != enableTimePatchWarning { + t.Fatalf("Details.EnableWarning mismatch: %#v", envelope.Error.Details["EnableWarning"]) + } + if warning, exists := envelope.Error.Details["Warning"]; exists { + if warningText, isString := warning.(string); isString && strings.Contains(warningText, enableTimePatchWarning) { + t.Fatalf("enable-time warning must stay on EnableWarning, not Details.Warning: %#v", warning) + } + } +} diff --git a/cli/project-runner/internal/projectrunner/pause_point_logs.go b/cli/project-runner/internal/projectrunner/pause_point_logs.go index 48f2919ef4..b97c02bd68 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_logs.go +++ b/cli/project-runner/internal/projectrunner/pause_point_logs.go @@ -35,13 +35,13 @@ type pausePointMatchingLogsResult struct { // pausePointWaitResult extends the hit response with marker-matching logs so // agents do not need a separate get-logs call while Unity is paused. Warning -// carries the only actionable signal that used to live inside a now-removed -// EvidenceSummary; everything else in that summary duplicated fields already -// present on pausePointStatusResponse or MatchingLogs. +// carries hit-time diagnosis only; enable-time patch warnings live in +// EnableTimeWarning so a successful hit is not contradicted by "may not hit" text. type pausePointWaitResult struct { pausePointStatusResponse - MatchingLogs []pausePointMatchingLog `json:"MatchingLogs"` - Warning string `json:"Warning,omitempty"` + MatchingLogs []pausePointMatchingLog `json:"MatchingLogs"` + Warning string `json:"Warning,omitempty"` + EnableTimeWarning string `json:"EnableTimeWarning,omitempty"` // Expectations and AllExpectationsPassed are populated only when --expect was passed, so a // caller that never used --expect sees neither field rather than a vacuous @@ -64,10 +64,13 @@ type pausePointHitPayloadInputs struct { logs pausePointMatchingLogsResult logsErr error - // unityWarning is Unity's own warning for this hit: the status response's on the plain await - // path, the enable response's on the enable --await path. + // unityWarning is the status-poll warning for this hit (plain await or enable --await). + // Enable-time patch warnings must not be folded in here — they go to enableTimeWarning. unityWarning string + // enableTimeWarning is the enable-pause-point patch diagnostic. Empty on plain await. + enableTimeWarning string + triggerResult *pausePointTriggerResult awaitedPausePointID string expectations []pausePointExpectationResult @@ -86,11 +89,13 @@ func buildPausePointHitPayload(inputs pausePointHitPayloadInputs) any { return struct { pausePointStatusResponse Warning string `json:"Warning,omitempty"` + EnableTimeWarning string `json:"EnableTimeWarning,omitempty"` Expectations []pausePointExpectationResult `json:"Expectations,omitempty"` AllExpectationsPassed *bool `json:"AllExpectationsPassed,omitempty"` }{ pausePointStatusResponse: response, Warning: joinPausePointWarnings(inputs.unityWarning, triggerWarning), + EnableTimeWarning: inputs.enableTimeWarning, Expectations: inputs.expectations, AllExpectationsPassed: pausePointAllExpectationsPassedPointer(inputs.expectations), } @@ -103,6 +108,7 @@ func buildPausePointHitPayload(inputs pausePointHitPayloadInputs) any { inputs.unityWarning, buildPausePointWarning(inputs.logs, response.HitCount), triggerWarning), + EnableTimeWarning: inputs.enableTimeWarning, Expectations: inputs.expectations, AllExpectationsPassed: pausePointAllExpectationsPassedPointer(inputs.expectations), } diff --git a/cli/project-runner/internal/projectrunner/pause_point_types.go b/cli/project-runner/internal/projectrunner/pause_point_types.go index 4594e41f54..6a96147c63 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_types.go +++ b/cli/project-runner/internal/projectrunner/pause_point_types.go @@ -31,9 +31,9 @@ type pausePointStatusResponse struct { StatusBeforeClear string `json:"StatusBeforeClear"` LateHitDiscardedAfterClear bool `json:"LateHitDiscardedAfterClear"` - // Warning carries the enable-time diagnostic (for example the physics-callback cached - // message dispatch warning) that PausePointResponse always includes on the Unity side, but - // which only the enable-pause-point --await path (pause_point_enable.go) currently reads. + // Warning is set by Unity on enable/clear tool responses when this shared type decodes those + // envelopes. The status bridge never sets it. On enable-pause-point --await hits, that enable + // response text is exposed as EnableTimeWarning on the wait payload, not as hit-time Warning. Warning string `json:"Warning,omitempty"` // ResolvedLine / ResolvedLineText / ResolvedMethod / SnapshotTiming are copied from the From b5e8f83e84d9f75501e1dbf88920846098564712 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 12:53:42 +0900 Subject: [PATCH 03/10] feat: report which captured variables were truncated in pause point responses --- .../Editor/PausePointCaptureModeTests.cs | 3 +- .../PausePointStatusResponseContractTests.cs | 7 +- Assets/Tests/Editor/PausePointTests.cs | 34 +++-- .../SourcePausePointCaptureTests.cs | 43 +++++- .../SourcePausePointVariableFormatterTests.cs | 1 + .../FirstPartyTools/Compile/CompileSchema.cs | 3 +- .../PausePoint/PausePointTools.cs | 4 +- .../PausePoint/SourcePausePointConstants.cs | 3 + .../SourcePausePointVariableCollector.cs | 132 ++++++++++-------- .../SourcePausePointVariableFormatter.cs | 33 +++-- .../Api/PausePointStatusBridgeCommand.cs | 8 +- .../PausePoints/UloopCapturedVariable.cs | 8 +- .../UloopPausePointCapturedVariableFrame.cs | 14 +- .../PausePoints/UloopPausePointEntry.cs | 13 +- .../PausePoints/UloopPausePointRegistry.cs | 9 +- .../PausePoints/UloopPausePointSnapshot.cs | 8 ++ ...se_point_captured_variable_names_filter.go | 27 ++++ ...int_captured_variable_names_filter_test.go | 15 ++ .../projectrunner/pause_point_types.go | 3 + .../pause_point_status_response_contract.json | 7 +- 20 files changed, 287 insertions(+), 88 deletions(-) diff --git a/Assets/Tests/Editor/PausePointCaptureModeTests.cs b/Assets/Tests/Editor/PausePointCaptureModeTests.cs index 55b2c49f86..86ddd8b880 100644 --- a/Assets/Tests/Editor/PausePointCaptureModeTests.cs +++ b/Assets/Tests/Editor/PausePointCaptureModeTests.cs @@ -172,7 +172,8 @@ private static UloopCapturedVariable CreateVariable(string name, string value) value, string.Empty, string.Empty, - 0); + 0, + truncated: false); } private sealed class FakePausePointPauseController : IUloopPausePointPauseController diff --git a/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs b/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs index d65130ff6b..0ad84a117e 100644 --- a/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs +++ b/Assets/Tests/Editor/PausePointStatusResponseContractTests.cs @@ -25,6 +25,7 @@ public void PausePointStatusResponse_WhenSerialized_MatchesSharedContractFieldSh JObject expected = ReadSharedContractFieldShape(); PausePointStatusResponse response = new() { + Success = true, Id = "Assets/Scripts/Enemy.cs:42", Status = "Hit", IsEnabled = true, @@ -33,6 +34,7 @@ public void PausePointStatusResponse_WhenSerialized_MatchesSharedContractFieldSh TimeoutSeconds = 30, Mode = "continuous", MaxHistory = 20, + MaxPreviewElements = 15, CapturedVariableHistory = new List { new() @@ -72,10 +74,13 @@ public void PausePointStatusResponse_WhenSerialized_MatchesSharedContractFieldSh Value = "Enemy", UnityObjectKind = "SceneObject", UnityObjectPath = "MainScene:/Root/Enemy", - UnityObjectInstanceId = -1234 + UnityObjectInstanceId = -1234, + Truncated = false } }, CapturedVariablesTruncated = true, + TruncatedVariableNames = new[] { "extraField" }, + TruncatedVariableCount = 1, ClearedReason = "", StatusBeforeClear = "", LateHitDiscardedAfterClear = false diff --git a/Assets/Tests/Editor/PausePointTests.cs b/Assets/Tests/Editor/PausePointTests.cs index 3c50cac858..8803b612e4 100644 --- a/Assets/Tests/Editor/PausePointTests.cs +++ b/Assets/Tests/Editor/PausePointTests.cs @@ -888,7 +888,7 @@ public void PausePointStatusBridge_WhenPausePointHitWithCapturedVariables_Return UloopPausePointRegistry.Enable("jump", 30); UloopCapturedVariable[] capturedVariables = { - new("speed", UloopCapturedVariableScope.Local, "System.Int32", "5", string.Empty, string.Empty, 0) + new("speed", UloopCapturedVariableScope.Local, "System.Int32", "5", string.Empty, string.Empty, 0, false) }; UloopPausePointRegistry.HitWithCapturedVariables("jump", capturedVariables, true); JObject parameters = new() { ["id"] = "jump" }; @@ -1004,7 +1004,7 @@ public void HitWithCapturedVariables_WhenPausePointIsEnabled_StoresCapturedVaria UloopPausePointRegistry.Enable("jump", 30); UloopCapturedVariable[] capturedVariables = { - new("speed", UloopCapturedVariableScope.Local, "System.Int32", "5", string.Empty, string.Empty, 0) + new("speed", UloopCapturedVariableScope.Local, "System.Int32", "5", string.Empty, string.Empty, 0, false) }; UloopPausePointSnapshot snapshot = UloopPausePointRegistry.HitWithCapturedVariables( @@ -1027,10 +1027,12 @@ public void TryGetCapturedValue_WhenLatestHitStoredRawFrame_ReturnsLiveReference new UloopPausePointCapturedVariableEntry("scores", UloopCapturedVariableScope.Local, scores), new UloopPausePointCapturedVariableEntry("empty", UloopCapturedVariableScope.Local, null) }, - false); + false, + System.Array.Empty(), + 0); UloopCapturedVariable[] capturedVariables = { - new("scores", UloopCapturedVariableScope.Local, "System.Collections.Generic.List`1[System.Int32]", "[10,20,30]", string.Empty, string.Empty, 0) + new("scores", UloopCapturedVariableScope.Local, "System.Collections.Generic.List`1[System.Int32]", "[10,20,30]", string.Empty, string.Empty, 0, false) }; UloopPausePointRegistry.HitWithCapturedFrame("jump", frame, capturedVariables, false); @@ -1056,7 +1058,9 @@ public void TryGetCapturedValue_WhenRegistryClearsLatestHit_ReturnsNotFound() UloopPausePointRegistry.Enable("jump", 30); UloopPausePointCapturedVariableFrame frame = new( new[] { new UloopPausePointCapturedVariableEntry("speed", UloopCapturedVariableScope.Local, 5) }, - false); + false, + System.Array.Empty(), + 0); UloopPausePointRegistry.HitWithCapturedFrame( "jump", frame, Array.Empty(), false); @@ -1076,10 +1080,14 @@ public void TryGetCapturedValue_WhenNewHitReplacesPrevious_ExposesLatestSnapshot UloopPausePointRegistry.Enable("land", 30); UloopPausePointCapturedVariableFrame jumpFrame = new( new[] { new UloopPausePointCapturedVariableEntry("speed", UloopCapturedVariableScope.Local, 1) }, - false); + false, + System.Array.Empty(), + 0); UloopPausePointCapturedVariableFrame landFrame = new( new[] { new UloopPausePointCapturedVariableEntry("speed", UloopCapturedVariableScope.Local, 2) }, - false); + false, + System.Array.Empty(), + 0); UloopPausePointRegistry.HitWithCapturedFrame("jump", jumpFrame, Array.Empty(), false); UloopPausePointRegistry.HitWithCapturedFrame("land", landFrame, Array.Empty(), false); @@ -1098,7 +1106,9 @@ public void TryGetCapturedValue_WhenUnrelatedPausePointIsCleared_KeepsLatestHitR UloopPausePointRegistry.Enable("land", 30); UloopPausePointCapturedVariableFrame landFrame = new( new[] { new UloopPausePointCapturedVariableEntry("speed", UloopCapturedVariableScope.Local, 7) }, - false); + false, + System.Array.Empty(), + 0); UloopPausePointRegistry.HitWithCapturedFrame("land", landFrame, Array.Empty(), false); UloopPausePointRegistry.Clear("jump"); @@ -1117,7 +1127,9 @@ public void TryGetCapturedValue_WhenSamePausePointIsReenabledWhilePaused_KeepsLa UloopPausePointRegistry.Enable("jump", 30); UloopPausePointCapturedVariableFrame frame = new( new[] { new UloopPausePointCapturedVariableEntry("speed", UloopCapturedVariableScope.Local, 1) }, - false); + false, + System.Array.Empty(), + 0); UloopPausePointRegistry.HitWithCapturedFrame("jump", frame, Array.Empty(), false); UloopPausePointRegistry.Enable("jump", 30); @@ -1136,7 +1148,9 @@ public void TryGetCapturedValue_WhenSamePausePointIsReenabledThenCleared_StillCl UloopPausePointRegistry.Enable("jump", 30); UloopPausePointCapturedVariableFrame frame = new( new[] { new UloopPausePointCapturedVariableEntry("speed", UloopCapturedVariableScope.Local, 1) }, - false); + false, + System.Array.Empty(), + 0); UloopPausePointRegistry.HitWithCapturedFrame("jump", frame, Array.Empty(), false); UloopPausePointRegistry.Enable("jump", 30); diff --git a/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCaptureTests.cs b/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCaptureTests.cs index e027e736d8..2198fe0588 100644 --- a/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCaptureTests.cs +++ b/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointCaptureTests.cs @@ -221,8 +221,8 @@ public void Collect_WithNullInstance_AddsNoThisEntry() [Test] public void Collect_WhenCountCapReachedBeforeThis_OmitsThisAndReportsTruncated() { - // Verifies that when locals already fill the count cap, the "this" entry is dropped and - // truncation is reported per the existing TryAppendEntry contract. + // Verifies that when locals already fill the count cap, the "this" entry is dropped from + // Entries but still counted in TruncatedVariableNames / TruncatedVariableCount. int localCount = SourcePausePointConstants.MaxCapturedVariableCount; object[] locals = new object[localCount * 2]; for (int i = 0; i < localCount; i++) @@ -239,6 +239,45 @@ public void Collect_WhenCountCapReachedBeforeThis_OmitsThisAndReportsTruncated() Assert.That(frame.Entries.Count, Is.EqualTo(SourcePausePointConstants.MaxCapturedVariableCount)); Assert.That(frame.Entries.Any(entry => entry.Name == "this"), Is.False); Assert.That(frame.Truncated, Is.True); + Assert.That(frame.TruncatedVariableCount, Is.GreaterThan(0)); + Assert.That(frame.TruncatedVariableNames, Does.Contain("this")); + } + + [Test] + public void Collect_WhenVariableCountExceedsCap_ReportsTruncatedNamesUpToLimitAndExactCount() + { + // Verifies count-cap overflow keeps collecting names (capped at 20) with an exact total. + int discarded = SourcePausePointConstants.MaxTruncatedVariableNamesReported + 5; + int localCount = SourcePausePointConstants.MaxCapturedVariableCount + discarded; + object[] locals = new object[localCount * 2]; + for (int i = 0; i < localCount; i++) + { + locals[i * 2] = $"local{i}"; + locals[i * 2 + 1] = i; + } + + UloopPausePointCapturedVariableFrame frame = SourcePausePointVariableCollector.Collect( + null, Array.Empty(), locals); + + Assert.That(frame.Entries.Count, Is.EqualTo(SourcePausePointConstants.MaxCapturedVariableCount)); + Assert.That(frame.Truncated, Is.True); + Assert.That(frame.TruncatedVariableCount, Is.EqualTo(discarded)); + Assert.That(frame.TruncatedVariableNames.Count, Is.EqualTo(SourcePausePointConstants.MaxTruncatedVariableNamesReported)); + Assert.That(frame.TruncatedVariableNames[0], Is.EqualTo($"local{SourcePausePointConstants.MaxCapturedVariableCount}")); + } + + [Test] + public void Collect_WhenUnderCountCap_ReportsEmptyTruncatedNames() + { + // Verifies no truncation metadata when every variable fits under the count cap. + object[] locals = { "speed", 5, "damage", 3 }; + + UloopPausePointCapturedVariableFrame frame = SourcePausePointVariableCollector.Collect( + null, Array.Empty(), locals); + + Assert.That(frame.Truncated, Is.False); + Assert.That(frame.TruncatedVariableCount, Is.EqualTo(0)); + Assert.That(frame.TruncatedVariableNames, Is.Empty); } [Test] diff --git a/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointVariableFormatterTests.cs b/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointVariableFormatterTests.cs index 197fdf3256..af3c2c7233 100644 --- a/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointVariableFormatterTests.cs +++ b/Assets/Tests/Editor/SourcePausePointCapture/SourcePausePointVariableFormatterTests.cs @@ -100,6 +100,7 @@ public void Format_WhenValueExceedsMaxLength_TruncatesValueAndSetsTruncatedFlag( Assert.That(variables.Single().Value.Length, Is.EqualTo(SourcePausePointConstants.MaxCapturedVariableValueLength)); Assert.That(truncated, Is.True); + Assert.That(variables.Single().Truncated, Is.True); } [Test] diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileSchema.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileSchema.cs index 6f153a042b..7d8338a0c5 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileSchema.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileSchema.cs @@ -28,8 +28,9 @@ public class CompileSchema : UnityCliLoopToolSchema /// /// How long the CLI waits for compilation to complete, in seconds. /// Unity ignores this value; it is consumed by the CLI. + /// Why no [Description]: first-party schema properties must keep long-form agent guidance + /// in skill files (see FirstPartyToolSchemaMetadataTests), not runtime metadata. /// - [Description("How long the CLI waits for compilation to complete, in seconds. Unity ignores this value; it is consumed by the CLI.")] public int CompileWaitTimeoutSeconds { get; set; } = 600; /// diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs index f782e945bf..38cb3a206a 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/PausePointTools.cs @@ -192,6 +192,7 @@ public class PausePointCapturedVariable public string UnityObjectKind { get; set; } = string.Empty; public string UnityObjectPath { get; set; } = string.Empty; public int UnityObjectInstanceId { get; set; } + public bool Truncated { get; set; } internal static PausePointCapturedVariable FromSnapshot(UloopCapturedVariable snapshot) { @@ -208,7 +209,8 @@ internal static PausePointCapturedVariable FromSnapshot(UloopCapturedVariable sn Value = snapshot.Value, UnityObjectKind = snapshot.UnityObjectKind, UnityObjectPath = snapshot.UnityObjectPath, - UnityObjectInstanceId = snapshot.UnityObjectInstanceId + UnityObjectInstanceId = snapshot.UnityObjectInstanceId, + Truncated = snapshot.Truncated }; } } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs index e7f085af4e..f73a767407 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointConstants.cs @@ -22,6 +22,9 @@ internal static class SourcePausePointConstants // Keeps a single hit's payload small enough for the CLI response and for the console-like // pause-point evidence to stay skimmable, mirroring the truncation-by-cap pattern MatchingLogs uses. public const int MaxCapturedVariableCount = 50; + // How many discarded variable names to surface when the count cap drops extras. The exact + // discarded count is still reported in full via TruncatedVariableCount. + public const int MaxTruncatedVariableNamesReported = 20; public const int MaxCapturedVariableValueLength = 256; // Mirrors UloopPausePointRegistry.DefaultMaxPreviewElements (the Runtime-owned per-marker // default enforced at Enable time) instead of a second independent literal, so the two diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableCollector.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableCollector.cs index ca78ad5703..010b863f2e 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableCollector.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableCollector.cs @@ -31,44 +31,56 @@ public static UloopPausePointCapturedVariableFrame Collect( Debug.Assert(localNamesAndValues.Length % 2 == 0, "localNamesAndValues must contain name/value pairs"); List entries = new(); + List truncatedVariableNames = new(); + int truncatedVariableCount = 0; bool truncated = false; HashSet capturedNames = new(); - bool countCapReached = AppendPairs( - entries, capturedNames, ref truncated, localNamesAndValues, UloopCapturedVariableScope.Local); - if (!countCapReached) - { - countCapReached = AppendPairs( - entries, capturedNames, ref truncated, parameterNamesAndValues, UloopCapturedVariableScope.Parameter); - } + // Why keep scanning after the count cap: callers need the discarded names and the exact + // dropped count, not only a Truncated bool. Values past the cap are never retained. + AppendPairs( + entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, ref truncated, + localNamesAndValues, UloopCapturedVariableScope.Local); + AppendPairs( + entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, ref truncated, + parameterNamesAndValues, UloopCapturedVariableScope.Parameter); - if (instance != null && !countCapReached) + if (instance != null) { - CollectInstanceFieldVariables(instance, entries, capturedNames, ref truncated); + CollectInstanceFieldVariables( + instance, entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, + ref truncated); } - return new UloopPausePointCapturedVariableFrame(entries, truncated); + return new UloopPausePointCapturedVariableFrame( + entries, truncated, truncatedVariableNames, truncatedVariableCount); } - private static bool AppendPairs( - List entries, HashSet capturedNames, ref bool truncated, - object[] namesAndValues, string scope) + private static void AppendPairs( + List entries, + HashSet capturedNames, + List truncatedVariableNames, + ref int truncatedVariableCount, + ref bool truncated, + object[] namesAndValues, + string scope) { for (int i = 0; i < namesAndValues.Length; i += 2) { string name = (string)namesAndValues[i]; object value = namesAndValues[i + 1]; - if (!TryAppendEntry(entries, capturedNames, ref truncated, name, scope, value)) - { - return true; - } + TryAppendEntry( + entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, ref truncated, + name, scope, value); } - - return false; } private static void CollectInstanceFieldVariables( - object instance, List entries, HashSet capturedNames, + object instance, + List entries, + HashSet capturedNames, + List truncatedVariableNames, + ref int truncatedVariableCount, ref bool truncated) { bool isCompilerGeneratedStateMachine = @@ -78,34 +90,38 @@ private static void CollectInstanceFieldVariables( // count cap keeps prioritizing locals and parameters over instance state. if (!isCompilerGeneratedStateMachine) { - if (!TryAppendEntry( - entries, capturedNames, ref truncated, ThisEntryName, UloopCapturedVariableScope.This, instance)) - { - return; - } + TryAppendEntry( + entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, ref truncated, + ThisEntryName, UloopCapturedVariableScope.This, instance); } - (object outerThis, bool countCapReached) = CollectDirectFieldVariables( - instance, entries, capturedNames, ref truncated, followOuterThis: true); - if (countCapReached || outerThis == null) + object outerThis = CollectDirectFieldVariables( + instance, entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, + ref truncated, followOuterThis: true); + if (outerThis == null) { return; } // Async/coroutine state machine: the real `this` is the hoisted outer instance, never the // compiler-generated state machine object. Emit it before the outer instance's fields. - if (!TryAppendEntry( - entries, capturedNames, ref truncated, ThisEntryName, UloopCapturedVariableScope.This, outerThis)) - { - return; - } + TryAppendEntry( + entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, ref truncated, + ThisEntryName, UloopCapturedVariableScope.This, outerThis); - CollectDirectFieldVariables(outerThis, entries, capturedNames, ref truncated, followOuterThis: false); + CollectDirectFieldVariables( + outerThis, entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, + ref truncated, followOuterThis: false); } - private static (object OuterThis, bool CountCapReached) CollectDirectFieldVariables( - object source, List entries, HashSet capturedNames, - ref bool truncated, bool followOuterThis) + private static object CollectDirectFieldVariables( + object source, + List entries, + HashSet capturedNames, + List truncatedVariableNames, + ref int truncatedVariableCount, + ref bool truncated, + bool followOuterThis) { object outerThis = null; bool isCompilerGeneratedStateMachine = Attribute.IsDefined(source.GetType(), typeof(CompilerGeneratedAttribute)); @@ -124,13 +140,9 @@ private static (object OuterThis, bool CountCapReached) CollectDirectFieldVariab Match hoistedLocalMatch = HoistedLocalFieldNamePattern.Match(field.Name); if (hoistedLocalMatch.Success) { - if (!TryAppendEntry( - entries, capturedNames, ref truncated, hoistedLocalMatch.Groups[1].Value, - UloopCapturedVariableScope.Local, field.GetValue(source))) - { - return (outerThis, true); - } - + TryAppendEntry( + entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, ref truncated, + hoistedLocalMatch.Groups[1].Value, UloopCapturedVariableScope.Local, field.GetValue(source)); continue; } @@ -142,13 +154,12 @@ private static (object OuterThis, bool CountCapReached) CollectDirectFieldVariab continue; } - if (!TryAppendEntry(entries, capturedNames, ref truncated, fieldName, plainFieldScope, field.GetValue(source))) - { - return (outerThis, true); - } + TryAppendEntry( + entries, capturedNames, truncatedVariableNames, ref truncatedVariableCount, ref truncated, + fieldName, plainFieldScope, field.GetValue(source)); } - return (outerThis, false); + return outerThis; } private static IEnumerable EnumerateInstanceFields(Type type) @@ -167,23 +178,34 @@ private static IEnumerable EnumerateInstanceFields(Type type) } } - private static bool TryAppendEntry( - List entries, HashSet capturedNames, ref bool truncated, - string name, string scope, object rawValue) + private static void TryAppendEntry( + List entries, + HashSet capturedNames, + List truncatedVariableNames, + ref int truncatedVariableCount, + ref bool truncated, + string name, + string scope, + object rawValue) { if (!capturedNames.Add(name)) { - return true; + return; } if (entries.Count >= SourcePausePointConstants.MaxCapturedVariableCount) { truncated = true; - return false; + truncatedVariableCount++; + if (truncatedVariableNames.Count < SourcePausePointConstants.MaxTruncatedVariableNamesReported) + { + truncatedVariableNames.Add(name); + } + + return; } entries.Add(new UloopPausePointCapturedVariableEntry(name, scope, rawValue)); - return true; } } } diff --git a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs index e50bdc0dbe..e336b200de 100644 --- a/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs +++ b/Packages/src/Editor/FirstPartyTools/PausePoint/SourcePausePointVariableFormatter.cs @@ -38,18 +38,22 @@ public static (List Variables, bool Truncated) FormatFram bool truncated = frame.Truncated; foreach (UloopPausePointCapturedVariableEntry entry in frame.Entries) { - results.Add(FormatVariable(entry.Name, entry.Scope, entry.Value, maxCollectionPreviewElementCount, ref truncated)); + UloopCapturedVariable variable = FormatVariable( + entry.Name, entry.Scope, entry.Value, maxCollectionPreviewElementCount); + results.Add(variable); + truncated |= variable.Truncated; } return (results, truncated); } private static UloopCapturedVariable FormatVariable( - string name, string scope, object rawValue, int maxCollectionPreviewElementCount, ref bool truncated) + string name, string scope, object rawValue, int maxCollectionPreviewElementCount) { if (rawValue == null) { - return new UloopCapturedVariable(name, scope, string.Empty, "null", string.Empty, string.Empty, 0); + return new UloopCapturedVariable( + name, scope, string.Empty, "null", string.Empty, string.Empty, 0, truncated: false); } string typeName = rawValue.GetType().FullName; @@ -58,8 +62,11 @@ private static UloopCapturedVariable FormatVariable( return FormatUnityObjectVariable(name, scope, typeName, unityObjectCandidate); } + // Why a per-variable flag: overall CapturedVariablesTruncated alone cannot tell which + // value was clipped after a name filter narrows the list. + bool variableTruncated = false; if (SourcePausePointCollectionPreviewSerializer.TrySerialize( - rawValue, maxCollectionPreviewElementCount, ref truncated, out string collectionPreview)) + rawValue, maxCollectionPreviewElementCount, ref variableTruncated, out string collectionPreview)) { // Why scale: a per-marker element-count override that raises the element cap // without also raising the byte budget would still get clipped by the fixed @@ -69,13 +76,15 @@ private static UloopCapturedVariable FormatVariable( // the default (10 elements, 1024 chars) already implies. int scaledValueLengthCap = SourcePausePointConstants.MaxCollectionPreviewValueLength * maxCollectionPreviewElementCount / UloopPausePointRegistry.DefaultMaxPreviewElements; - string cappedPreview = ApplyValueLengthCap(collectionPreview, scaledValueLengthCap, ref truncated); - return new UloopCapturedVariable(name, scope, typeName, cappedPreview, string.Empty, string.Empty, 0); + string cappedPreview = ApplyValueLengthCap(collectionPreview, scaledValueLengthCap, ref variableTruncated); + return new UloopCapturedVariable( + name, scope, typeName, cappedPreview, string.Empty, string.Empty, 0, variableTruncated); } string value = ApplyValueLengthCap( - SafeToString(rawValue), SourcePausePointConstants.MaxCapturedVariableValueLength, ref truncated); - return new UloopCapturedVariable(name, scope, typeName, value, string.Empty, string.Empty, 0); + SafeToString(rawValue), SourcePausePointConstants.MaxCapturedVariableValueLength, ref variableTruncated); + return new UloopCapturedVariable( + name, scope, typeName, value, string.Empty, string.Empty, 0, variableTruncated); } private static UloopCapturedVariable FormatUnityObjectVariable( @@ -83,21 +92,23 @@ private static UloopCapturedVariable FormatUnityObjectVariable( { if (!MainThreadSwitcher.IsMainThread) { - return new UloopCapturedVariable(name, scope, typeName, OffMainThreadValue, string.Empty, string.Empty, 0); + return new UloopCapturedVariable( + name, scope, typeName, OffMainThreadValue, string.Empty, string.Empty, 0, truncated: false); } if (unityObjectCandidate == null) { return new UloopCapturedVariable( name, scope, typeName, DestroyedValue, - UloopCapturedVariableUnityObjectKind.Destroyed, string.Empty, UnityObjectIdentifier.GetInstanceId(unityObjectCandidate)); + UloopCapturedVariableUnityObjectKind.Destroyed, string.Empty, + UnityObjectIdentifier.GetInstanceId(unityObjectCandidate), truncated: false); } SourcePausePointUnityObjectClassifier.Classification classification = SourcePausePointUnityObjectClassifier.Classify(unityObjectCandidate); return new UloopCapturedVariable( name, scope, typeName, unityObjectCandidate.name, - classification.Kind, classification.Path, classification.InstanceId); + classification.Kind, classification.Path, classification.InstanceId, truncated: false); } private static string ApplyValueLengthCap(string value, int maxLength, ref bool truncated) diff --git a/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs b/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs index 298fff0679..e637ab3a85 100644 --- a/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs +++ b/Packages/src/Editor/Infrastructure/Api/PausePointStatusBridgeCommand.cs @@ -132,6 +132,8 @@ public class PausePointStatusResponse : UnityCliLoopToolResponse public IReadOnlyList CapturedVariables { get; set; } = Array.Empty(); public bool CapturedVariablesTruncated { get; set; } + public IReadOnlyList TruncatedVariableNames { get; set; } = Array.Empty(); + public int TruncatedVariableCount { get; set; } public string ClearedReason { get; set; } = string.Empty; public string StatusBeforeClear { get; set; } = string.Empty; public bool LateHitDiscardedAfterClear { get; set; } @@ -174,6 +176,8 @@ internal static PausePointStatusResponse FromSnapshot(UloopPausePointSnapshot sn .Select(PausePointStatusCapturedVariable.FromCapturedVariable) .ToList(), CapturedVariablesTruncated = snapshot.CapturedVariablesTruncated, + TruncatedVariableNames = snapshot.TruncatedVariableNames, + TruncatedVariableCount = snapshot.TruncatedVariableCount, ClearedReason = snapshot.ClearedReason, StatusBeforeClear = snapshot.StatusBeforeClear, LateHitDiscardedAfterClear = snapshot.LateHitDiscardedAfterClear @@ -252,6 +256,7 @@ public class PausePointStatusCapturedVariable public string UnityObjectKind { get; set; } = string.Empty; public string UnityObjectPath { get; set; } = string.Empty; public int UnityObjectInstanceId { get; set; } + public bool Truncated { get; set; } internal static PausePointStatusCapturedVariable FromCapturedVariable(UloopCapturedVariable capturedVariable) { @@ -268,7 +273,8 @@ internal static PausePointStatusCapturedVariable FromCapturedVariable(UloopCaptu Value = capturedVariable.Value, UnityObjectKind = capturedVariable.UnityObjectKind, UnityObjectPath = capturedVariable.UnityObjectPath, - UnityObjectInstanceId = capturedVariable.UnityObjectInstanceId + UnityObjectInstanceId = capturedVariable.UnityObjectInstanceId, + Truncated = capturedVariable.Truncated }; } } diff --git a/Packages/src/Runtime/PausePoints/UloopCapturedVariable.cs b/Packages/src/Runtime/PausePoints/UloopCapturedVariable.cs index 65df549f34..1e9819c167 100644 --- a/Packages/src/Runtime/PausePoints/UloopCapturedVariable.cs +++ b/Packages/src/Runtime/PausePoints/UloopCapturedVariable.cs @@ -17,7 +17,8 @@ public UloopCapturedVariable( string value, string unityObjectKind, string unityObjectPath, - int unityObjectInstanceId) + int unityObjectInstanceId, + bool truncated) { Debug.Assert(!string.IsNullOrEmpty(name), "name must not be null or empty"); Debug.Assert(!string.IsNullOrEmpty(scope), "scope must not be null or empty"); @@ -29,6 +30,7 @@ public UloopCapturedVariable( UnityObjectKind = unityObjectKind ?? string.Empty; UnityObjectPath = unityObjectPath ?? string.Empty; UnityObjectInstanceId = unityObjectInstanceId; + Truncated = truncated; } public string Name { get; } @@ -38,6 +40,10 @@ public UloopCapturedVariable( public string UnityObjectKind { get; } public string UnityObjectPath { get; } public int UnityObjectInstanceId { get; } + + // True when this variable's value/preview was clipped (length, collection elements, or + // preview depth). Distinct from the response-level CapturedVariablesTruncated OR. + public bool Truncated { get; } } } #endif diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointCapturedVariableFrame.cs b/Packages/src/Runtime/PausePoints/UloopPausePointCapturedVariableFrame.cs index 559d9d80c7..a90333b16c 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointCapturedVariableFrame.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointCapturedVariableFrame.cs @@ -1,4 +1,5 @@ #if UNITY_EDITOR +using System; using System.Collections.Generic; namespace io.github.hatayama.UnityCliLoop.Runtime @@ -9,14 +10,25 @@ namespace io.github.hatayama.UnityCliLoop.Runtime internal sealed class UloopPausePointCapturedVariableFrame { public UloopPausePointCapturedVariableFrame( - IReadOnlyList entries, bool truncated) + IReadOnlyList entries, + bool truncated, + IReadOnlyList truncatedVariableNames, + int truncatedVariableCount) { Entries = entries; Truncated = truncated; + TruncatedVariableNames = truncatedVariableNames ?? Array.Empty(); + TruncatedVariableCount = truncatedVariableCount; } public IReadOnlyList Entries { get; } public bool Truncated { get; } + + // Names dropped by the variable-count cap (at most MaxTruncatedVariableNamesReported). + public IReadOnlyList TruncatedVariableNames { get; } + + // Exact number of variables dropped by the count cap (not capped at the names list length). + public int TruncatedVariableCount { get; } } } #endif diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs b/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs index b9efccf304..2beef43ed8 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointEntry.cs @@ -31,6 +31,7 @@ public UloopPausePointEntry( IsEnabled = true; Message = "Pause point enabled."; CapturedVariables = Array.Empty(); + TruncatedVariableNames = Array.Empty(); _capturedVariableHistory = new Queue(maxHistory); } @@ -54,6 +55,8 @@ public UloopPausePointEntry( public string Message { get; private set; } public IReadOnlyList CapturedVariables { get; private set; } public bool CapturedVariablesTruncated { get; private set; } + public IReadOnlyList TruncatedVariableNames { get; private set; } + public int TruncatedVariableCount { get; private set; } public int HistoryDroppedCount { get; private set; } public string ClearedReason { get; private set; } = string.Empty; public string StatusBeforeClear { get; private set; } = string.Empty; @@ -169,10 +172,14 @@ public void RecordHitWithCapturedVariables( int hitSequence, int frameCount, IReadOnlyList capturedVariables, - bool capturedVariablesTruncated) + bool capturedVariablesTruncated, + IReadOnlyList truncatedVariableNames, + int truncatedVariableCount) { Debug.Assert(hitSequence > 0, "hitSequence must be greater than zero"); Debug.Assert(capturedVariables != null, "capturedVariables must not be null"); + Debug.Assert(truncatedVariableNames != null, "truncatedVariableNames must not be null"); + Debug.Assert(truncatedVariableCount >= 0, "truncatedVariableCount must not be negative"); if (HitCount == 0) { @@ -192,6 +199,8 @@ public void RecordHitWithCapturedVariables( : "Pause point hit; Unity pause was requested."; CapturedVariables = capturedVariables; CapturedVariablesTruncated = capturedVariablesTruncated; + TruncatedVariableNames = truncatedVariableNames; + TruncatedVariableCount = truncatedVariableCount; if (_capturedVariableHistory.Count == MaxHistory) { @@ -254,6 +263,8 @@ public UloopPausePointSnapshot ToSnapshot(DateTime nowUtc, IUloopPausePointPause recommendedNextAction, CapturedVariables, CapturedVariablesTruncated, + TruncatedVariableNames, + TruncatedVariableCount, ClearedReason, StatusBeforeClear, LateHitDiscardedAfterClear); diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs index d822d3af1a..6e9b826c58 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointRegistry.cs @@ -326,9 +326,16 @@ private static UloopPausePointSnapshot HitCore( int hitSequence = ++_nextHitSequence; int frameCount = Time.frameCount; + IReadOnlyList truncatedVariableNames = capturedFrame != null + ? capturedFrame.TruncatedVariableNames + : Array.Empty(); + int truncatedVariableCount = capturedFrame != null + ? capturedFrame.TruncatedVariableCount + : 0; entry.RecordHitWithCapturedVariables( now, _pauseController.IsPlaying, _pauseController.IsPaused, hitSequence, - frameCount, capturedVariables, capturedVariablesTruncated); + frameCount, capturedVariables, capturedVariablesTruncated, + truncatedVariableNames, truncatedVariableCount); UloopPausePointSnapshot snapshot = entry.ToSnapshot(now, _pauseController); _latestHitSnapshot = snapshot; _hitSnapshots.RemoveAll(hitSnapshot => hitSnapshot.Id == id); diff --git a/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs b/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs index e23a4280e9..cee601780e 100644 --- a/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs +++ b/Packages/src/Runtime/PausePoints/UloopPausePointSnapshot.cs @@ -37,6 +37,8 @@ public UloopPausePointSnapshot( string recommendedNextAction, IReadOnlyList capturedVariables, bool capturedVariablesTruncated, + IReadOnlyList truncatedVariableNames, + int truncatedVariableCount, string clearedReason, string statusBeforeClear, bool lateHitDiscardedAfterClear) @@ -68,6 +70,8 @@ public UloopPausePointSnapshot( RecommendedNextAction = recommendedNextAction ?? string.Empty; CapturedVariables = capturedVariables ?? Array.Empty(); CapturedVariablesTruncated = capturedVariablesTruncated; + TruncatedVariableNames = truncatedVariableNames ?? Array.Empty(); + TruncatedVariableCount = truncatedVariableCount; ClearedReason = clearedReason ?? string.Empty; StatusBeforeClear = statusBeforeClear ?? string.Empty; LateHitDiscardedAfterClear = lateHitDiscardedAfterClear; @@ -98,6 +102,8 @@ public UloopPausePointSnapshot( public string RecommendedNextAction { get; } public IReadOnlyList CapturedVariables { get; } public bool CapturedVariablesTruncated { get; } + public IReadOnlyList TruncatedVariableNames { get; } + public int TruncatedVariableCount { get; } public string ClearedReason { get; } public string StatusBeforeClear { get; } public bool LateHitDiscardedAfterClear { get; } @@ -134,6 +140,8 @@ public static UloopPausePointSnapshot NotEnabled(string id, IUloopPausePointPaus string.Empty, Array.Empty(), false, + Array.Empty(), + 0, string.Empty, string.Empty, false); diff --git a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go index 52eaa599aa..a6ee22749c 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go +++ b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter.go @@ -36,6 +36,11 @@ func filterPausePointCapturedVariablesByName( return response } + // Why before filtering: pause-point-status runs this filter without a hit gate. An unhit + // marker has empty CapturedVariables/history, which would otherwise look like a name miss + // and a Warning blaming the requested names would misdiagnose "not hit yet". + hadCapturedVariables := pausePointResponseHasCapturedVariables(response) + nameSet := make(map[string]struct{}, len(names)) for _, name := range names { nameSet[name] = struct{}{} @@ -60,9 +65,31 @@ func filterPausePointCapturedVariablesByName( response.CapturedVariableNameFilterNoMatch = totalMatchCount == 0 response.CapturedVariableNamesNotFound = unmatchedCapturedVariableNames(names, matchedNames) + // Why Warning only when hadCapturedVariables: machine-readable flags still fire on empty + // snapshots (unchanged), but a human Warning must not claim a name miss when the hit has + // not produced any variables yet. + if hadCapturedVariables && response.CapturedVariableNameFilterNoMatch { + response.Warning = joinPausePointWarnings( + response.Warning, + "No captured variable matched the requested names; the hit captured other variables. Check CapturedVariableNamesNotFound for the names that were absent.") + } return response } +// pausePointResponseHasCapturedVariables reports whether the snapshot already holds any +// captured variable (current or history) before a name filter runs. +func pausePointResponseHasCapturedVariables(response pausePointStatusResponse) bool { + if len(response.CapturedVariables) > 0 { + return true + } + for _, frame := range response.CapturedVariableHistory { + if len(frame.CapturedVariables) > 0 { + return true + } + } + return false +} + // unmatchedCapturedVariableNames lists the requested names that matched nothing, keeping the order // they were requested in so the report reads back against the flag value the caller wrote. A name // matched anywhere — current variables or any history frame — counts as found. A name requested diff --git a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go index f1ea9cd846..5bddb89fee 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_captured_variable_names_filter_test.go @@ -63,6 +63,21 @@ func TestFilterPausePointCapturedVariablesByName(t *testing.T) { if !result.CapturedVariableNameFilterNoMatch { t.Fatal("expected CapturedVariableNameFilterNoMatch to be true when nothing matches") } + const wantWarning = "No captured variable matched the requested names; the hit captured other variables. Check CapturedVariableNamesNotFound for the names that were absent." + if result.Warning != wantWarning { + t.Fatalf("expected human-readable Warning for no-match filter: %q", result.Warning) + } + }) + + t.Run("empty pre-filter snapshot keeps no-match flag but skips Warning", func(t *testing.T) { + // Verifies an unhit status (no variables yet) does not blame the requested names. + result := filterPausePointCapturedVariablesByName(pausePointStatusResponse{}, []string{"velocity"}) + if !result.CapturedVariableNameFilterNoMatch { + t.Fatal("expected CapturedVariableNameFilterNoMatch to stay true on an empty snapshot") + } + if result.Warning != "" { + t.Fatalf("expected no Warning when the snapshot had no variables before filtering: %q", result.Warning) + } }) t.Run("composes with captured-variables names mode: filter first, then strip values", func(t *testing.T) { diff --git a/cli/project-runner/internal/projectrunner/pause_point_types.go b/cli/project-runner/internal/projectrunner/pause_point_types.go index 6a96147c63..7ad851b594 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_types.go +++ b/cli/project-runner/internal/projectrunner/pause_point_types.go @@ -27,6 +27,8 @@ type pausePointStatusResponse struct { RecommendedNextAction string `json:"RecommendedNextAction"` CapturedVariables []pausePointCapturedVariable `json:"CapturedVariables"` CapturedVariablesTruncated bool `json:"CapturedVariablesTruncated"` + TruncatedVariableNames []string `json:"TruncatedVariableNames"` + TruncatedVariableCount int `json:"TruncatedVariableCount"` ClearedReason string `json:"ClearedReason"` StatusBeforeClear string `json:"StatusBeforeClear"` LateHitDiscardedAfterClear bool `json:"LateHitDiscardedAfterClear"` @@ -126,6 +128,7 @@ type pausePointCapturedVariable struct { UnityObjectKind string `json:"UnityObjectKind,omitempty"` UnityObjectPath string `json:"UnityObjectPath,omitempty"` UnityObjectInstanceId int `json:"UnityObjectInstanceId,omitempty"` + Truncated bool `json:"Truncated"` } // pausePointVariableValue returns a pointer to value for use in pausePointCapturedVariable diff --git a/tests/contracts/pause_point_status_response_contract.json b/tests/contracts/pause_point_status_response_contract.json index e5b903f286..f0e9d10801 100644 --- a/tests/contracts/pause_point_status_response_contract.json +++ b/tests/contracts/pause_point_status_response_contract.json @@ -43,10 +43,15 @@ "Value": "Enemy", "UnityObjectKind": "SceneObject", "UnityObjectPath": "MainScene:/Root/Enemy", - "UnityObjectInstanceId": -1234 + "UnityObjectInstanceId": -1234, + "Truncated": false } ], "CapturedVariablesTruncated": true, + "TruncatedVariableNames": [ + "extraField" + ], + "TruncatedVariableCount": 1, "ClearedReason": "", "StatusBeforeClear": "", "LateHitDiscardedAfterClear": false From 58c29e1957a5b0003e6eec075e97a5cde93d719e Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 13:10:56 +0900 Subject: [PATCH 04/10] feat: accept GameView as an alias for the rendering screenshot capture mode --- .agents/skills/uloop-screenshot/SKILL.md | 2 +- .claude/skills/uloop-screenshot/SKILL.md | 2 +- .../FirstPartyToolSchemaMetadataTests.cs | 58 ++++++++++++++++--- .../FirstPartyTools/Screenshot/Skill/SKILL.md | 2 +- .../UnityCliLoopScreenshotTypes.cs | 5 +- cli/common/tools/default-tools.json | 5 +- cli/dispatcher/shared-inputs-stamp.json | 2 +- cli/project-runner/shared-inputs-stamp.json | 2 +- 8 files changed, 62 insertions(+), 16 deletions(-) diff --git a/.agents/skills/uloop-screenshot/SKILL.md b/.agents/skills/uloop-screenshot/SKILL.md index 435880c53b..dc676a6e52 100644 --- a/.agents/skills/uloop-screenshot/SKILL.md +++ b/.agents/skills/uloop-screenshot/SKILL.md @@ -21,7 +21,7 @@ uloop screenshot [--window-name ] [--resolution-scale ] [--match-mo | `--window-name` | string | `Game` | Window name to capture (for example `Game`, `Scene`, `Console`, `Inspector`). Ignored when `--capture-mode rendering`. When the Game tab is Device Simulator and the title is Simulator, default Game falls back to Simulator. | | `--resolution-scale` | number | `1.0` | Resolution scale (0.1 to 1.0) | | `--match-mode` | enum | `exact` | Window name matching mode: `exact`, `prefix`, or `contains`. Ignored when `--capture-mode rendering`. | -| `--capture-mode` | enum | `window` | `window` - capture EditorWindow including toolbar, `rendering` - capture game rendering only (PlayMode required). Rendering screenshots return `ScreenshotToInputFormula` for converting raw image pixels before calling simulate-mouse-input or raycast. | +| `--capture-mode` | enum | `window` | `window` - capture EditorWindow including toolbar, `rendering` - capture game rendering only (PlayMode required), `GameView` - alias for `rendering`. Rendering screenshots return `ScreenshotToInputFormula` for converting raw image pixels before calling simulate-mouse-input or raycast. | | `--output-directory` | string | `""` | Output directory path for saving screenshots. When empty, uses default path (.uloop/outputs/Screenshots/). Accepts absolute paths. | | `--annotate-elements` | flag | - | Annotate interactive UI elements with index labels and interaction hints (A / CLICK, B / DRAG, ...). The response includes an `AnnotatedElements` array with element metadata sorted by z-order. Only works with `--capture-mode rendering` in PlayMode. | | `--annotate-raycast-grid` | flag | - | Annotate clustered 3D physics collider candidates as `PhysicsCollider` entries in `AnnotatedElements`. Uses `Camera.main` visibility and the same top-left Game View coordinates as `simulate-mouse-input`. Only works with `--capture-mode rendering` in PlayMode. | diff --git a/.claude/skills/uloop-screenshot/SKILL.md b/.claude/skills/uloop-screenshot/SKILL.md index 435880c53b..dc676a6e52 100644 --- a/.claude/skills/uloop-screenshot/SKILL.md +++ b/.claude/skills/uloop-screenshot/SKILL.md @@ -21,7 +21,7 @@ uloop screenshot [--window-name ] [--resolution-scale ] [--match-mo | `--window-name` | string | `Game` | Window name to capture (for example `Game`, `Scene`, `Console`, `Inspector`). Ignored when `--capture-mode rendering`. When the Game tab is Device Simulator and the title is Simulator, default Game falls back to Simulator. | | `--resolution-scale` | number | `1.0` | Resolution scale (0.1 to 1.0) | | `--match-mode` | enum | `exact` | Window name matching mode: `exact`, `prefix`, or `contains`. Ignored when `--capture-mode rendering`. | -| `--capture-mode` | enum | `window` | `window` - capture EditorWindow including toolbar, `rendering` - capture game rendering only (PlayMode required). Rendering screenshots return `ScreenshotToInputFormula` for converting raw image pixels before calling simulate-mouse-input or raycast. | +| `--capture-mode` | enum | `window` | `window` - capture EditorWindow including toolbar, `rendering` - capture game rendering only (PlayMode required), `GameView` - alias for `rendering`. Rendering screenshots return `ScreenshotToInputFormula` for converting raw image pixels before calling simulate-mouse-input or raycast. | | `--output-directory` | string | `""` | Output directory path for saving screenshots. When empty, uses default path (.uloop/outputs/Screenshots/). Accepts absolute paths. | | `--annotate-elements` | flag | - | Annotate interactive UI elements with index labels and interaction hints (A / CLICK, B / DRAG, ...). The response includes an `AnnotatedElements` array with element metadata sorted by z-order. Only works with `--capture-mode rendering` in PlayMode. | | `--annotate-raycast-grid` | flag | - | Annotate clustered 3D physics collider candidates as `PhysicsCollider` entries in `AnnotatedElements`. Uses `Camera.main` visibility and the same top-left Game View coordinates as `simulate-mouse-input`. Only works with `--capture-mode rendering` in PlayMode. | diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/FirstPartyToolSchemaMetadataTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/FirstPartyToolSchemaMetadataTests.cs index a5a87472b0..682c345d44 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/FirstPartyToolSchemaMetadataTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/FirstPartyToolSchemaMetadataTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Reflection; using NUnit.Framework; @@ -46,9 +47,11 @@ public void FirstPartySchemaEnumProperties_WhenLoaded_ShouldBeZeroBasedAndContig { // Tests that every enum a first-party schema exposes can be resolved by its ordinal. // The schema cache stores an enum default as a number while listing the members by name, - // so the CLI recovers the name shown in `--help` by indexing the name list with that - // number. A member with an explicit value or a [Flags] enum would make the CLI print a - // different member's name as the default. + // so the CLI recovers the name shown in `--help` / `uloop list` by indexing that name + // list (Go enumValueAtIndex). Same-value aliases are allowed only after the canonical + // name: names[ordinal] must be the first declaration of that value (MetadataToken + // order). Gaps, negative values, and a [Flags] enum would make the CLI print the wrong + // default member name. Type[] schemaTypes = FirstPartySchemaTypes(); Assert.That(schemaTypes, Is.Not.Empty); @@ -76,14 +79,53 @@ public void FirstPartySchemaEnumProperties_WhenLoaded_ShouldBeZeroBasedAndContig Is.Null, $"{location} is a [Flags] enum, which cannot be resolved by ordinal"); - Array members = Enum.GetValues(propertyType); - for (int index = 0; index < members.Length; index++) + // Why MetadataToken order: GetFields does not guarantee declaration order, and + // the CLI indexes Enum.GetNames by the numeric default. The first same-value + // member in declaration order is the canonical name that must occupy that index. + FieldInfo[] memberFields = propertyType.GetFields( + BindingFlags.Public | BindingFlags.Static) + .OrderBy(field => field.MetadataToken) + .ToArray(); + + string[] names = Enum.GetNames(propertyType); + Dictionary canonicalNamesByValue = new(); + HashSet distinctValues = new(); + long maxValue = -1; + foreach (FieldInfo memberField in memberFields) { - long value = Convert.ToInt64(members.GetValue(index)); + long value = Convert.ToInt64(memberField.GetRawConstantValue()); Assert.That( value, - Is.EqualTo((long)index), - $"{location} is not zero-based and contiguous at index {index}"); + Is.GreaterThanOrEqualTo(0), + $"{location} has a negative member value {value}"); + distinctValues.Add(value); + if (!canonicalNamesByValue.ContainsKey(value)) + { + canonicalNamesByValue[value] = memberField.Name; + } + + if (value > maxValue) + { + maxValue = value; + } + } + + Assert.That( + distinctValues.Count, + Is.EqualTo(maxValue + 1), + $"{location} has gaps in its ordinal values"); + + for (long value = 0; value <= maxValue; value++) + { + Assert.That( + names.Length, + Is.GreaterThan((int)value), + $"{location} is missing a canonical name at index {value}"); + string canonicalName = canonicalNamesByValue[value]; + Assert.That( + names[value], + Is.EqualTo(canonicalName), + $"{location} names[{value}] is '{names[value]}' but canonical is '{canonicalName}'"); } checkedEnumPropertyCount++; diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/Screenshot/Skill/SKILL.md index 435880c53b..dc676a6e52 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/Skill/SKILL.md @@ -21,7 +21,7 @@ uloop screenshot [--window-name ] [--resolution-scale ] [--match-mo | `--window-name` | string | `Game` | Window name to capture (for example `Game`, `Scene`, `Console`, `Inspector`). Ignored when `--capture-mode rendering`. When the Game tab is Device Simulator and the title is Simulator, default Game falls back to Simulator. | | `--resolution-scale` | number | `1.0` | Resolution scale (0.1 to 1.0) | | `--match-mode` | enum | `exact` | Window name matching mode: `exact`, `prefix`, or `contains`. Ignored when `--capture-mode rendering`. | -| `--capture-mode` | enum | `window` | `window` - capture EditorWindow including toolbar, `rendering` - capture game rendering only (PlayMode required). Rendering screenshots return `ScreenshotToInputFormula` for converting raw image pixels before calling simulate-mouse-input or raycast. | +| `--capture-mode` | enum | `window` | `window` - capture EditorWindow including toolbar, `rendering` - capture game rendering only (PlayMode required), `GameView` - alias for `rendering`. Rendering screenshots return `ScreenshotToInputFormula` for converting raw image pixels before calling simulate-mouse-input or raycast. | | `--output-directory` | string | `""` | Output directory path for saving screenshots. When empty, uses default path (.uloop/outputs/Screenshots/). Accepts absolute paths. | | `--annotate-elements` | flag | - | Annotate interactive UI elements with index labels and interaction hints (A / CLICK, B / DRAG, ...). The response includes an `AnnotatedElements` array with element metadata sorted by z-order. Only works with `--capture-mode rendering` in PlayMode. | | `--annotate-raycast-grid` | flag | - | Annotate clustered 3D physics collider candidates as `PhysicsCollider` entries in `AnnotatedElements`. Uses `Camera.main` visibility and the same top-left Game View coordinates as `simulate-mouse-input`. Only works with `--capture-mode rendering` in PlayMode. | diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopScreenshotTypes.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopScreenshotTypes.cs index 7ab4b4d2d3..cc6ad7de99 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopScreenshotTypes.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopScreenshotTypes.cs @@ -10,6 +10,9 @@ public enum WindowMatchMode public enum CaptureMode { window = 0, - rendering = 1 + rendering = 1, + // Alias for rendering: agents commonly pass GameView when they mean Game View pixels. + // Same underlying value so CaptureMode comparisons against rendering keep working. + GameView = 1 } } diff --git a/cli/common/tools/default-tools.json b/cli/common/tools/default-tools.json index 514c045bbe..25a313dacc 100644 --- a/cli/common/tools/default-tools.json +++ b/cli/common/tools/default-tools.json @@ -262,10 +262,11 @@ }, "CaptureMode": { "type": "string", - "description": "window - capture EditorWindow including toolbar, rendering - capture game rendering only (PlayMode required). Rendering screenshots return ScreenshotToInputFormula for converting raw image pixels before calling simulate-mouse-input or raycast.", + "description": "window - capture EditorWindow including toolbar, rendering - capture game rendering only (PlayMode required), GameView - alias for rendering. Rendering screenshots return ScreenshotToInputFormula for converting raw image pixels before calling simulate-mouse-input or raycast.", "enum": [ "window", - "rendering" + "rendering", + "GameView" ], "default": "window" }, diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 60430f5310..da719f94b3 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "050e25b77113c7d2cb24bd904463acda2e7f0087" + "sharedInputsHash": "635ca21f5641482cb77d389515c18f7823fc1b74" } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index d536489edc..9bbc3bf032 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "620c09181440d0c45707c8507839b5dab37c73cc" + "sharedInputsHash": "d636f60df5104d3eb992f547b94f3c1e69f1bb9f" } From 235a45301e49f34f6b77b1627b593c3edbd45e51 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 14:12:29 +0900 Subject: [PATCH 05/10] fix: hide the input visualization overlay while capturing screenshots Co-authored-by: Cursor --- .../Common/Overlay/AssemblyInfo.cs | 1 + .../Common/Overlay/OverlayCanvasFactory.cs | 77 ++++- .../Screenshot/ScreenshotUseCase.cs | 278 +++++++++++++----- ...p.FirstPartyTools.Screenshot.Editor.asmdef | 3 +- .../InternalAPIBridge/GameViewBridge.cs | 36 +++ 5 files changed, 308 insertions(+), 87 deletions(-) diff --git a/Packages/src/Editor/FirstPartyTools/Common/Overlay/AssemblyInfo.cs b/Packages/src/Editor/FirstPartyTools/Common/Overlay/AssemblyInfo.cs index 3976368411..d0d5930e80 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/Overlay/AssemblyInfo.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/Overlay/AssemblyInfo.cs @@ -7,6 +7,7 @@ [assembly: InternalsVisibleTo("UnityCLILoop.FirstPartyTools.SimulateKeyboard.Editor")] [assembly: InternalsVisibleTo("UnityCLILoop.FirstPartyTools.SimulateMouseInput.Editor")] [assembly: InternalsVisibleTo("UnityCLILoop.FirstPartyTools.SimulateMouseUi.Editor")] +[assembly: InternalsVisibleTo("UnityCLILoop.FirstPartyTools.Screenshot.Editor")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Editor")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.PlayMode")] [assembly: InternalsVisibleTo("UnityCLILoop.Tests.Demo.Editor")] diff --git a/Packages/src/Editor/FirstPartyTools/Common/Overlay/OverlayCanvasFactory.cs b/Packages/src/Editor/FirstPartyTools/Common/Overlay/OverlayCanvasFactory.cs index 486649be26..5757914b97 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/Overlay/OverlayCanvasFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/Overlay/OverlayCanvasFactory.cs @@ -30,19 +30,61 @@ public void Reset() _instance = null; } + // Why no create: screenshot must hide an already-visible overlay without spawning one. + public GameObject TryGetExisting() + { + ReclaimExistingInstance(); + if (_instance == null) + { + return null; + } + + return _instance.gameObject; + } + public void EnsureExists() { if (_instance != null) { + // Why reactivate: screenshot hide leaves the canvas inactive; the next simulate-* + // call must bring it back without instantiating a second DontDestroyOnLoad copy. + EnsureActive(_instance.gameObject); + return; + } + + ReclaimExistingInstance(); + if (_instance != null) + { + EnsureActive(_instance.gameObject); return; } - // Domain Reload resets _instance but DontDestroyOnLoad objects survive; reclaim one and destroy duplicates + GameObject prefab = AssetDatabase.LoadAssetAtPath(CANVAS_PREFAB_PATH); + Debug.Assert(prefab != null, $"InputVisualizationCanvas prefab not found at {CANVAS_PREFAB_PATH}"); + + GameObject go = (GameObject)PrefabUtility.InstantiatePrefab(prefab); + Object.DontDestroyOnLoad(go); + _instance = go.GetComponent(); + Debug.Assert(_instance != null, "InputVisualizationCanvas component not found on prefab"); + } + + // Domain Reload resets _instance but DontDestroyOnLoad objects survive; reclaim one and destroy duplicates. + private void ReclaimExistingInstance() + { + if (_instance != null) + { + return; + } + + // Why include inactive: screenshot SetActive(false) would otherwise hide the only canvas + // from default FindObjectsByType and make EnsureExists spawn a duplicate. #if UNITY_6000_4_OR_NEWER - InputVisualizationCanvas[] existing = Object.FindObjectsByType(); + InputVisualizationCanvas[] existing = Object.FindObjectsByType( + FindObjectsInactive.Include); #else - InputVisualizationCanvas[] existing = - Object.FindObjectsByType(FindObjectsSortMode.None); + InputVisualizationCanvas[] existing = Object.FindObjectsByType( + FindObjectsInactive.Include, + FindObjectsSortMode.None); #endif for (int i = 0; i < existing.Length; i++) { @@ -55,18 +97,22 @@ public void EnsureExists() Object.DestroyImmediate(existing[i].gameObject); } } - if (_instance != null) + } + + private static void EnsureActive(GameObject overlayRoot) + { + // Why Canvas too: screenshot hide disables Canvas.enabled as well as the GameObject. + // Recovering only activeSelf leaves badges permanently invisible while looking active. + Canvas overlayCanvas = overlayRoot.GetComponent(); + if (overlayCanvas != null && !overlayCanvas.enabled) { - return; + overlayCanvas.enabled = true; } - GameObject prefab = AssetDatabase.LoadAssetAtPath(CANVAS_PREFAB_PATH); - Debug.Assert(prefab != null, $"InputVisualizationCanvas prefab not found at {CANVAS_PREFAB_PATH}"); - - GameObject go = (GameObject)PrefabUtility.InstantiatePrefab(prefab); - Object.DontDestroyOnLoad(go); - _instance = go.GetComponent(); - Debug.Assert(_instance != null, "InputVisualizationCanvas component not found on prefab"); + if (!overlayRoot.activeSelf) + { + overlayRoot.SetActive(true); + } } } @@ -89,5 +135,10 @@ public static void EnsureExists() { ServiceValue.EnsureExists(); } + + public static GameObject TryGetExisting() + { + return ServiceValue.TryGetExisting(); + } } } diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs index fccb6626ff..a18618c760 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs @@ -6,6 +6,7 @@ using UnityEngine; using UnityEditor; +using io.github.hatayama.UnityCliLoop.InternalAPIBridge; using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.FirstPartyTools @@ -139,25 +140,27 @@ private async Task CaptureRenderingAsync( Texture2D texture; GameRenderingImageInfo captureRenderingInfo; bool captureTimedOut; + + // Why SwitchTo before hide: SetActive/Canvas/RT clear are main-thread only. Keep this + // await outside try — nothing is hidden yet if cancellation throws here. + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); + (GameObject inputVisualizationOverlay, bool inputVisualizationWasActive) = + HideInputVisualizationOverlay(); + try { - if (request.AnnotateElements || request.AnnotateRaycastGrid) + // Why wait inside try: WaitFramesOrTimeoutAsync / SwitchTo throw OperationCanceledException + // on CLI disconnect; finally must still restore the overlay. + if (inputVisualizationWasActive) { - List overlayElements = new(annotatedElements); - overlayElements.AddRange(physicsColliderElements); - annotationOverlay = UIElementAnnotator.CreateAnnotationOverlay( - overlayElements, - request.ResolutionScale); - Canvas.ForceUpdateCanvases(); - // Chained CLI calls can read the previous GameView RT before overlay rendering catches up. - bool overlayFramesReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( + bool hideFramesReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( ANNOTATION_OVERLAY_RENDER_WAIT_FRAMES, UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, ct).ConfigureAwait(false); - if (!overlayFramesReady) + if (!hideFramesReady) { return CreateTimedOutResult( - "annotation overlay render", + "input visualization overlay hide", correlationId, new List()); } @@ -165,24 +168,55 @@ private async Task CaptureRenderingAsync( await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); } - (texture, captureRenderingInfo, captureTimedOut) = await EditorWindowCaptureUtility.CaptureGameRenderingAsync( - request.ResolutionScale, - raycastGridRenderingInfo, - UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, - ct).ConfigureAwait(false); - if (captureTimedOut) + try { - return CreateTimedOutResult( - "Play Mode view rendering capture", - correlationId, - new List()); - } + if (request.AnnotateElements || request.AnnotateRaycastGrid) + { + List overlayElements = new(annotatedElements); + overlayElements.AddRange(physicsColliderElements); + annotationOverlay = UIElementAnnotator.CreateAnnotationOverlay( + overlayElements, + request.ResolutionScale); + Canvas.ForceUpdateCanvases(); + // Chained CLI calls can read the previous GameView RT before overlay rendering catches up. + bool overlayFramesReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( + ANNOTATION_OVERLAY_RENDER_WAIT_FRAMES, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, + ct).ConfigureAwait(false); + if (!overlayFramesReady) + { + return CreateTimedOutResult( + "annotation overlay render", + correlationId, + new List()); + } + + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); + } - await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); + (texture, captureRenderingInfo, captureTimedOut) = await EditorWindowCaptureUtility.CaptureGameRenderingAsync( + request.ResolutionScale, + raycastGridRenderingInfo, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, + ct).ConfigureAwait(false); + if (captureTimedOut) + { + return CreateTimedOutResult( + "Play Mode view rendering capture", + correlationId, + new List()); + } + + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); + } + finally + { + DestroyAnnotationOverlay(annotationOverlay, editorContext); + } } finally { - DestroyAnnotationOverlay(annotationOverlay, editorContext); + RestoreInputVisualizationOverlay(inputVisualizationOverlay, inputVisualizationWasActive, editorContext); } // Uses the settled capture-time size, not the pre-capture gameViewSize sample, so annotated @@ -317,67 +351,99 @@ private async Task CaptureWindowsAsync( string timestamp = DateTime.Now.ToString("yyyyMMdd_HHmmss_fff"); List screenshots = new(); - for (int i = 0; i < windows.Length; i++) + // Why SwitchTo before hide: SetActive/Canvas/RT clear are main-thread only. Keep this + // await outside try — nothing is hidden yet if cancellation throws here. + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); + (GameObject inputVisualizationOverlay, bool inputVisualizationWasActive) = + HideInputVisualizationOverlay(); + + try { - EditorWindow window = windows[i]; - (Texture2D texture, bool timedOut) = await EditorWindowCaptureUtility.CaptureWindowAsync( - window, - request.ResolutionScale, - UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, - ct).ConfigureAwait(false); - if (timedOut) + // Why wait inside try: WaitFramesOrTimeoutAsync / SwitchTo throw OperationCanceledException + // on CLI disconnect; finally must still restore the overlay. + if (inputVisualizationWasActive) { - return CreateTimedOutResult("EditorWindow capture", correlationId, screenshots); + bool hideFramesReady = await EditorFrameWaiter.WaitFramesOrTimeoutAsync( + ANNOTATION_OVERLAY_RENDER_WAIT_FRAMES, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, + ct).ConfigureAwait(false); + if (!hideFramesReady) + { + return CreateTimedOutResult( + "input visualization overlay hide", + correlationId, + screenshots); + } + + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); } - await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); - if (texture == null) + for (int i = 0; i < windows.Length; i++) { - VibeLogger.LogWarning( - "screenshot_failed", - $"Failed to capture window index {i}", - correlationId: correlationId - ); - continue; - } + EditorWindow window = windows[i]; + (Texture2D texture, bool timedOut) = await EditorWindowCaptureUtility.CaptureWindowAsync( + window, + request.ResolutionScale, + UnityCliLoopConstants.EDITOR_FRAME_WAIT_TIMEOUT_MS, + ct).ConfigureAwait(false); + if (timedOut) + { + return CreateTimedOutResult("EditorWindow capture", correlationId, screenshots); + } - string fileName = windows.Length == 1 - ? $"{safeWindowName}_{timestamp}.png" - : $"{safeWindowName}_{i + 1}_{timestamp}.png"; - string savedPath = Path.Combine(outputDirectory, fileName); + await CapturedEditorSynchronizationContext.SwitchTo(editorContext, ct); + if (texture == null) + { + VibeLogger.LogWarning( + "screenshot_failed", + $"Failed to capture window index {i}", + correlationId: correlationId + ); + continue; + } - int width = texture.width; - int height = texture.height; + string fileName = windows.Length == 1 + ? $"{safeWindowName}_{timestamp}.png" + : $"{safeWindowName}_{i + 1}_{timestamp}.png"; + string savedPath = Path.Combine(outputDirectory, fileName); - try - { - ScreenshotFileWriter.SaveTextureAsPng(texture, savedPath); + int width = texture.width; + int height = texture.height; - FileInfo savedFileInfo = new(savedPath); - ScreenshotInfo info = new() + try { - ImagePath = savedPath, - FileSizeBytes = savedFileInfo.Length, - Width = width, - Height = height, - }; - ApplyWindowCoordinateMetadata(info); - screenshots.Add(info); - } - catch (Exception ex) - { - // File I/O is external resource access; catch to continue processing remaining windows - VibeLogger.LogWarning( - "screenshot_save_exception", - $"Exception saving window index {i}: {ex.Message}", - correlationId: correlationId - ); - } - finally - { - UnityEngine.Object.DestroyImmediate(texture); + ScreenshotFileWriter.SaveTextureAsPng(texture, savedPath); + + FileInfo savedFileInfo = new(savedPath); + ScreenshotInfo info = new() + { + ImagePath = savedPath, + FileSizeBytes = savedFileInfo.Length, + Width = width, + Height = height, + }; + ApplyWindowCoordinateMetadata(info); + screenshots.Add(info); + } + catch (Exception ex) + { + // File I/O is external resource access; catch to continue processing remaining windows + VibeLogger.LogWarning( + "screenshot_save_exception", + $"Exception saving window index {i}: {ex.Message}", + correlationId: correlationId + ); + } + finally + { + UnityEngine.Object.DestroyImmediate(texture); + } } } + finally + { + RestoreInputVisualizationOverlay(inputVisualizationOverlay, inputVisualizationWasActive, editorContext); + } // why: only prune the package default Screenshots folder; never delete files in a user-specified OutputDirectory if (string.IsNullOrEmpty(request.OutputDirectory)) @@ -395,6 +461,72 @@ private async Task CaptureWindowsAsync( return new ScreenshotResponse { Screenshots = screenshots }; } + // Hides the input-visualization canvas synchronously. Caller must already be on the editor + // main thread, and must run the 2-frame settle wait inside a try/finally that restores. + private static (GameObject Overlay, bool WasActive) HideInputVisualizationOverlay() + { + GameObject overlay = OverlayCanvasFactory.TryGetExisting(); + bool wasActive = overlay != null && overlay.activeSelf; + if (!wasActive) + { + return (overlay, false); + } + + overlay.SetActive(false); + // Also disable Canvas: Screen Space Overlay can keep compositing into the Play Mode + // view RT for a frame after the GameObject alone is deactivated. + Canvas overlayCanvas = overlay.GetComponent(); + if (overlayCanvas != null) + { + overlayCanvas.enabled = false; + } + + Canvas.ForceUpdateCanvases(); + // Why clear: with no camera the Play Mode RT never rewrites itself after a Screen Space + // Overlay is hidden, so the last badge composite would stay forever. With cameras, the + // caller's frame wait redraws the scene without the overlay. + GameViewBridge.ClearMainPlayModeViewRenderTexture(); + GameViewBridge.RepaintMainPlayModeView(); + return (overlay, true); + } + + private static void RestoreInputVisualizationOverlay( + GameObject overlay, + bool wasActive, + SynchronizationContext editorContext) + { + if (!wasActive || overlay == null) + { + return; + } + + if (SynchronizationContext.Current == editorContext) + { + RestoreInputVisualizationOverlayOnMainThread(overlay); + return; + } + + // Why post: timeout/error paths may restore from a non-main thread. + editorContext.Post(_ => RestoreInputVisualizationOverlayOnMainThread(overlay), null); + } + + private static void RestoreInputVisualizationOverlayOnMainThread(GameObject overlay) + { + // Why: Post may run after Play Mode teardown destroyed the DontDestroyOnLoad overlay. + if (overlay == null) + { + return; + } + + Canvas overlayCanvas = overlay.GetComponent(); + if (overlayCanvas != null) + { + overlayCanvas.enabled = true; + } + + overlay.SetActive(true); + } + internal static ScreenshotResponse CreateTimedOutResult( string waitName, string correlationId, diff --git a/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCLILoop.FirstPartyTools.Screenshot.Editor.asmdef b/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCLILoop.FirstPartyTools.Screenshot.Editor.asmdef index 7e6908dc53..3bd502a3fe 100644 --- a/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCLILoop.FirstPartyTools.Screenshot.Editor.asmdef +++ b/Packages/src/Editor/FirstPartyTools/Screenshot/UnityCLILoop.FirstPartyTools.Screenshot.Editor.asmdef @@ -7,7 +7,8 @@ "GUID:a2d87883023de4cf59b7d1962d5dd8aa", "GUID:fc3fd32eddbee40e39c2d76dc184957b", "GUID:96b8d4624fb74ea2849fed17e7ad69d3", - "GUID:77087b1bb30a415e87a80cbf24e7430a" + "GUID:77087b1bb30a415e87a80cbf24e7430a", + "GUID:aa7cf56cc5f074e57ba35272b877e116" ], "includePlatforms": [ "Editor" diff --git a/Packages/src/Editor/InternalAPIBridge/GameViewBridge.cs b/Packages/src/Editor/InternalAPIBridge/GameViewBridge.cs index 71bc674e2f..57d5aa3b0b 100644 --- a/Packages/src/Editor/InternalAPIBridge/GameViewBridge.cs +++ b/Packages/src/Editor/InternalAPIBridge/GameViewBridge.cs @@ -38,6 +38,42 @@ public static RenderTexture GetRenderTexture() return _targetTextureField.GetValue(playModeView) as RenderTexture; } + /// + /// Requests a Play Mode view redraw so m_TargetTexture drops Screen Space Overlay canvases + /// that were just deactivated. + /// + public static void RepaintMainPlayModeView() + { + EnsureMembersResolved(); + + EditorWindow playModeView = FindMainPlayModeView(); + if (playModeView == null) + { + return; + } + + playModeView.Repaint(); + } + + /// + /// Clears the Play Mode view RT immediately after overlay hide. + /// Why: scenes with no camera never rewrite m_TargetTexture, so a deactivated Screen Space + /// Overlay leaves its last composite in the RT forever. + /// + public static void ClearMainPlayModeViewRenderTexture() + { + RenderTexture renderTexture = GetRenderTexture(); + if (renderTexture == null) + { + return; + } + + RenderTexture previousActive = RenderTexture.active; + RenderTexture.active = renderTexture; + GL.Clear(true, true, Color.black); + RenderTexture.active = previousActive; + } + /// /// Resolve m_TargetTexture on the PlayModeView declaring type. /// Must not use a derived Type: GetField does not return private fields declared on base types. From 4dc9507c532775f569daa5ee2b993f0f88d3e04a Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 15:08:18 +0900 Subject: [PATCH 06/10] feat: reject simulate input durations above 30 seconds Co-authored-by: Cursor --- .../skills/uloop-simulate-keyboard/SKILL.md | 2 +- .../uloop-simulate-mouse-input/SKILL.md | 2 +- .../skills/uloop-simulate-mouse-ui/SKILL.md | 2 +- .../skills/uloop-simulate-keyboard/SKILL.md | 2 +- .../uloop-simulate-mouse-input/SKILL.md | 2 +- .../skills/uloop-simulate-mouse-ui/SKILL.md | 2 +- .../InputSimulation/SimulateInputConstants.cs | 12 ++++++++++ .../SimulateInputConstants.cs.meta | 11 ++++++++++ .../KeyboardInputActionExecutor.cs | 12 ++++++++++ .../SimulateKeyboard/Skill/SKILL.md | 2 +- .../MouseInputMotionActionExecutor.cs | 11 ++++++++++ .../MouseInputPressActionExecutor.cs | 22 +++++++++++++++++++ .../SimulateMouseInput/Skill/SKILL.md | 2 +- .../MouseUiPressActionExecutor.cs | 11 ++++++++++ .../SimulateMouseUi/Skill/SKILL.md | 2 +- cli/common/tools/default-tools.json | 6 ++--- cli/dispatcher/shared-inputs-stamp.json | 2 +- cli/project-runner/shared-inputs-stamp.json | 2 +- 18 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 Packages/src/Editor/FirstPartyTools/Common/InputSimulation/SimulateInputConstants.cs create mode 100644 Packages/src/Editor/FirstPartyTools/Common/InputSimulation/SimulateInputConstants.cs.meta diff --git a/.agents/skills/uloop-simulate-keyboard/SKILL.md b/.agents/skills/uloop-simulate-keyboard/SKILL.md index fd8b1887ab..c3c43f44b5 100644 --- a/.agents/skills/uloop-simulate-keyboard/SKILL.md +++ b/.agents/skills/uloop-simulate-keyboard/SKILL.md @@ -29,7 +29,7 @@ uloop simulate-keyboard --action ReleaseAll |-----------|------|---------|-------------| | `--action` | enum | `Press` | `Press` - one-shot key tap (Down then Up), `KeyDown` - hold key down, `KeyUp` - release held key, `ReleaseAll` - force-release every tracked and device-pressed key (allowed while PlayMode is paused; use after a pause-point interruption leaves key state inconsistent) | | `--key` | string | (required except `ReleaseAll`) | Key name matching Input System Key enum (e.g. `W`, `Space`, `LeftShift`, `A`, `Enter`). Case-insensitive. Digit keys use `Digit0`-`Digit9` or `Numpad0`-`Numpad9`, not bare `0`-`9`. Not used by `ReleaseAll`. | -| `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp/ReleaseAll. | +| `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap, max 30). Ignored by KeyDown/KeyUp/ReleaseAll. | ### Actions diff --git a/.agents/skills/uloop-simulate-mouse-input/SKILL.md b/.agents/skills/uloop-simulate-mouse-input/SKILL.md index cc6a406c73..3643f3ecba 100644 --- a/.agents/skills/uloop-simulate-mouse-input/SKILL.md +++ b/.agents/skills/uloop-simulate-mouse-input/SKILL.md @@ -36,7 +36,7 @@ uloop simulate-mouse-input --action [options] | `--x` | number | `0` | Target X position in Game View pixels (origin: top-left). Used by Click and LongPress. Use `AnnotatedElements[].SimX`, or raw image pixels converted with `ScreenshotToInputFormula`. | | `--y` | number | `0` | Target Y position in Game View pixels (origin: top-left). Used by Click and LongPress. Use `AnnotatedElements[].SimY`, or raw image pixels converted with `ScreenshotToInputFormula`. | | `--button` | enum | `Left` | Mouse button: `Left`, `Right`, `Middle`. Used by Click and LongPress. | -| `--duration` | number | `0` | Hold duration for LongPress, or interpolation duration for SmoothDelta (seconds). For Click, 0 = one-shot tap. | +| `--duration` | number | `0` | Hold duration for LongPress, or interpolation duration for SmoothDelta (seconds, max 30). For Click, 0 = one-shot tap. | | `--delta-x` | number | `0` | Delta X in pixels for MoveDelta/SmoothDelta. Positive = right. | | `--delta-y` | number | `0` | Delta Y in pixels for MoveDelta/SmoothDelta. Positive = up. | | `--scroll-x` | number | `0` | Horizontal scroll delta for Scroll action. | diff --git a/.agents/skills/uloop-simulate-mouse-ui/SKILL.md b/.agents/skills/uloop-simulate-mouse-ui/SKILL.md index 0a187f0c03..905d8188e3 100644 --- a/.agents/skills/uloop-simulate-mouse-ui/SKILL.md +++ b/.agents/skills/uloop-simulate-mouse-ui/SKILL.md @@ -34,7 +34,7 @@ uloop simulate-mouse-ui --action --x --y [options] | `--from-x` | number | `0` | Start X position for Drag action (origin: top-left). Drag starts here and moves to `--x`,`--y`. | | `--from-y` | number | `0` | Start Y position for Drag action (origin: top-left). Drag starts here and moves to `--x`,`--y`. | | `--drag-speed` | number | `2000` | Drag speed in pixels per second (0 for instant). 2000 is fast (default), 200 is slow enough to watch. Applies to Drag, DragMove, and DragEnd actions. | -| `--duration` | number | `0.5` | Hold duration in seconds for LongPress action. | +| `--duration` | number | `0.5` | Hold duration in seconds for LongPress action (max 30). | | `--button` | enum | `Left` | Mouse button. `Click` and `LongPress` support `Left`, `Right`, and `Middle`. Drag actions support `Left` only; other buttons return an error. | | `--bypass-raycast` | flag | - | For `Click`, `LongPress`, `Drag`, and `DragStart`, bypass EventSystem raycast and dispatch pointer events directly to `--target-path`. Use when a raycast-blocking overlay visually covers the intended target. | | `--target-path` | string | `""` | Hierarchy path of the target GameObject, for example `Canvas/Panel/Button`. Required when `--bypass-raycast` is used with `Click`, `LongPress`, `Drag`, or `DragStart`; prefer `AnnotatedElements[].Path` from screenshot JSON. | diff --git a/.claude/skills/uloop-simulate-keyboard/SKILL.md b/.claude/skills/uloop-simulate-keyboard/SKILL.md index fd8b1887ab..c3c43f44b5 100644 --- a/.claude/skills/uloop-simulate-keyboard/SKILL.md +++ b/.claude/skills/uloop-simulate-keyboard/SKILL.md @@ -29,7 +29,7 @@ uloop simulate-keyboard --action ReleaseAll |-----------|------|---------|-------------| | `--action` | enum | `Press` | `Press` - one-shot key tap (Down then Up), `KeyDown` - hold key down, `KeyUp` - release held key, `ReleaseAll` - force-release every tracked and device-pressed key (allowed while PlayMode is paused; use after a pause-point interruption leaves key state inconsistent) | | `--key` | string | (required except `ReleaseAll`) | Key name matching Input System Key enum (e.g. `W`, `Space`, `LeftShift`, `A`, `Enter`). Case-insensitive. Digit keys use `Digit0`-`Digit9` or `Numpad0`-`Numpad9`, not bare `0`-`9`. Not used by `ReleaseAll`. | -| `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp/ReleaseAll. | +| `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap, max 30). Ignored by KeyDown/KeyUp/ReleaseAll. | ### Actions diff --git a/.claude/skills/uloop-simulate-mouse-input/SKILL.md b/.claude/skills/uloop-simulate-mouse-input/SKILL.md index cc6a406c73..3643f3ecba 100644 --- a/.claude/skills/uloop-simulate-mouse-input/SKILL.md +++ b/.claude/skills/uloop-simulate-mouse-input/SKILL.md @@ -36,7 +36,7 @@ uloop simulate-mouse-input --action [options] | `--x` | number | `0` | Target X position in Game View pixels (origin: top-left). Used by Click and LongPress. Use `AnnotatedElements[].SimX`, or raw image pixels converted with `ScreenshotToInputFormula`. | | `--y` | number | `0` | Target Y position in Game View pixels (origin: top-left). Used by Click and LongPress. Use `AnnotatedElements[].SimY`, or raw image pixels converted with `ScreenshotToInputFormula`. | | `--button` | enum | `Left` | Mouse button: `Left`, `Right`, `Middle`. Used by Click and LongPress. | -| `--duration` | number | `0` | Hold duration for LongPress, or interpolation duration for SmoothDelta (seconds). For Click, 0 = one-shot tap. | +| `--duration` | number | `0` | Hold duration for LongPress, or interpolation duration for SmoothDelta (seconds, max 30). For Click, 0 = one-shot tap. | | `--delta-x` | number | `0` | Delta X in pixels for MoveDelta/SmoothDelta. Positive = right. | | `--delta-y` | number | `0` | Delta Y in pixels for MoveDelta/SmoothDelta. Positive = up. | | `--scroll-x` | number | `0` | Horizontal scroll delta for Scroll action. | diff --git a/.claude/skills/uloop-simulate-mouse-ui/SKILL.md b/.claude/skills/uloop-simulate-mouse-ui/SKILL.md index 0a187f0c03..905d8188e3 100644 --- a/.claude/skills/uloop-simulate-mouse-ui/SKILL.md +++ b/.claude/skills/uloop-simulate-mouse-ui/SKILL.md @@ -34,7 +34,7 @@ uloop simulate-mouse-ui --action --x --y [options] | `--from-x` | number | `0` | Start X position for Drag action (origin: top-left). Drag starts here and moves to `--x`,`--y`. | | `--from-y` | number | `0` | Start Y position for Drag action (origin: top-left). Drag starts here and moves to `--x`,`--y`. | | `--drag-speed` | number | `2000` | Drag speed in pixels per second (0 for instant). 2000 is fast (default), 200 is slow enough to watch. Applies to Drag, DragMove, and DragEnd actions. | -| `--duration` | number | `0.5` | Hold duration in seconds for LongPress action. | +| `--duration` | number | `0.5` | Hold duration in seconds for LongPress action (max 30). | | `--button` | enum | `Left` | Mouse button. `Click` and `LongPress` support `Left`, `Right`, and `Middle`. Drag actions support `Left` only; other buttons return an error. | | `--bypass-raycast` | flag | - | For `Click`, `LongPress`, `Drag`, and `DragStart`, bypass EventSystem raycast and dispatch pointer events directly to `--target-path`. Use when a raycast-blocking overlay visually covers the intended target. | | `--target-path` | string | `""` | Hierarchy path of the target GameObject, for example `Canvas/Panel/Button`. Required when `--bypass-raycast` is used with `Click`, `LongPress`, `Drag`, or `DragStart`; prefer `AnnotatedElements[].Path` from screenshot JSON. | diff --git a/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/SimulateInputConstants.cs b/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/SimulateInputConstants.cs new file mode 100644 index 0000000000..7565852cd7 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/SimulateInputConstants.cs @@ -0,0 +1,12 @@ +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Shared limits for simulate-keyboard / simulate-mouse-input / simulate-mouse-ui durations. + /// + public static class SimulateInputConstants + { + // Why 30s: agents often pass milliseconds (e.g. 600) as seconds, which freezes Unity for + // minutes and blocks every CLI command with server_busy. Cap rejects that class of typo. + public const float MaxDurationSeconds = 30f; + } +} diff --git a/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/SimulateInputConstants.cs.meta b/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/SimulateInputConstants.cs.meta new file mode 100644 index 0000000000..b0e7805f87 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/SimulateInputConstants.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 888e43e0e005e47449d1eff5b4dbfb5d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputActionExecutor.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputActionExecutor.cs index 4dfb94f4d9..b104afc92c 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputActionExecutor.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputActionExecutor.cs @@ -30,6 +30,18 @@ internal static async Task ExecutePress( }; } + if (duration > SimulateInputConstants.MaxDurationSeconds) + { + return new SimulateKeyboardResponse + { + Success = false, + Message = + $"Duration must be {SimulateInputConstants.MaxDurationSeconds} seconds or less, got: {duration}. The unit is seconds, not milliseconds.", + Action = UnityCliLoopKeyboardAction.Press.ToString(), + KeyName = key.ToString() + }; + } + string keyName = key.ToString(); if (KeyboardKeyState.IsKeyHeld(key)) { diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md index fd8b1887ab..c3c43f44b5 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md @@ -29,7 +29,7 @@ uloop simulate-keyboard --action ReleaseAll |-----------|------|---------|-------------| | `--action` | enum | `Press` | `Press` - one-shot key tap (Down then Up), `KeyDown` - hold key down, `KeyUp` - release held key, `ReleaseAll` - force-release every tracked and device-pressed key (allowed while PlayMode is paused; use after a pause-point interruption leaves key state inconsistent) | | `--key` | string | (required except `ReleaseAll`) | Key name matching Input System Key enum (e.g. `W`, `Space`, `LeftShift`, `A`, `Enter`). Case-insensitive. Digit keys use `Digit0`-`Digit9` or `Numpad0`-`Numpad9`, not bare `0`-`9`. Not used by `ReleaseAll`. | -| `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp/ReleaseAll. | +| `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap, max 30). Ignored by KeyDown/KeyUp/ReleaseAll. | ### Actions diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputMotionActionExecutor.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputMotionActionExecutor.cs index 49f3318a40..b6a3551e13 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputMotionActionExecutor.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputMotionActionExecutor.cs @@ -113,6 +113,17 @@ internal static async Task ExecuteSmoothDelta( }; } + if (request.Duration > SimulateInputConstants.MaxDurationSeconds) + { + return new SimulateMouseInputResponse + { + Success = false, + Message = + $"Duration must be {SimulateInputConstants.MaxDurationSeconds} seconds or less, got: {request.Duration}. The unit is seconds, not milliseconds.", + Action = UnityCliLoopMouseInputAction.SmoothDelta.ToString() + }; + } + Vector2 totalDelta = new(request.DeltaX, request.DeltaY); float duration = request.Duration; float startTime = Time.realtimeSinceStartup; diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputPressActionExecutor.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputPressActionExecutor.cs index 01c3e5c8da..c610e37ee9 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputPressActionExecutor.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputPressActionExecutor.cs @@ -37,6 +37,17 @@ internal static async Task ExecuteClick( }; } + if (request.Duration > SimulateInputConstants.MaxDurationSeconds) + { + return new SimulateMouseInputResponse + { + Success = false, + Message = + $"Duration must be {SimulateInputConstants.MaxDurationSeconds} seconds or less, got: {request.Duration}. The unit is seconds, not milliseconds.", + Action = UnityCliLoopMouseInputAction.Click.ToString() + }; + } + Vector2 inputPos = new(request.X, request.Y); GameViewCoordinateConversion conversion = ConvertInputToUnity(inputPos); RuntimeMouseButton button = ToRuntimeMouseButton(request.Button); @@ -153,6 +164,17 @@ internal static async Task ExecuteLongPress( }; } + if (request.Duration > SimulateInputConstants.MaxDurationSeconds) + { + return new SimulateMouseInputResponse + { + Success = false, + Message = + $"Duration must be {SimulateInputConstants.MaxDurationSeconds} seconds or less, got: {request.Duration}. The unit is seconds, not milliseconds.", + Action = UnityCliLoopMouseInputAction.LongPress.ToString() + }; + } + Vector2 inputPos = new(request.X, request.Y); GameViewCoordinateConversion conversion = ConvertInputToUnity(inputPos); RuntimeMouseButton button = ToRuntimeMouseButton(request.Button); diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.md index cc6a406c73..3643f3ecba 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.md @@ -36,7 +36,7 @@ uloop simulate-mouse-input --action [options] | `--x` | number | `0` | Target X position in Game View pixels (origin: top-left). Used by Click and LongPress. Use `AnnotatedElements[].SimX`, or raw image pixels converted with `ScreenshotToInputFormula`. | | `--y` | number | `0` | Target Y position in Game View pixels (origin: top-left). Used by Click and LongPress. Use `AnnotatedElements[].SimY`, or raw image pixels converted with `ScreenshotToInputFormula`. | | `--button` | enum | `Left` | Mouse button: `Left`, `Right`, `Middle`. Used by Click and LongPress. | -| `--duration` | number | `0` | Hold duration for LongPress, or interpolation duration for SmoothDelta (seconds). For Click, 0 = one-shot tap. | +| `--duration` | number | `0` | Hold duration for LongPress, or interpolation duration for SmoothDelta (seconds, max 30). For Click, 0 = one-shot tap. | | `--delta-x` | number | `0` | Delta X in pixels for MoveDelta/SmoothDelta. Positive = right. | | `--delta-y` | number | `0` | Delta Y in pixels for MoveDelta/SmoothDelta. Positive = up. | | `--scroll-x` | number | `0` | Horizontal scroll delta for Scroll action. | diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiPressActionExecutor.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiPressActionExecutor.cs index 66795b2aa4..e5a2a533c4 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiPressActionExecutor.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/MouseUiPressActionExecutor.cs @@ -102,6 +102,17 @@ internal static async Task ExecuteLongPress( }; } + if (parameters.Duration > SimulateInputConstants.MaxDurationSeconds) + { + return new SimulateMouseUiResponse + { + Success = false, + Message = + $"Duration must be {SimulateInputConstants.MaxDurationSeconds} seconds or less, got: {parameters.Duration}. The unit is seconds, not milliseconds.", + Action = MouseAction.LongPress.ToString() + }; + } + Vector2 inputPos = new(parameters.X, parameters.Y); Vector2 screenPos = MouseUiCoordinateConverter.InputToScreen(inputPos); PointerEventData pointerData = MouseUiPointerTargetResolver.CreatePointerPressData(eventSystem, screenPos, parameters.Button); diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.md index 0a187f0c03..905d8188e3 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.md @@ -34,7 +34,7 @@ uloop simulate-mouse-ui --action --x --y [options] | `--from-x` | number | `0` | Start X position for Drag action (origin: top-left). Drag starts here and moves to `--x`,`--y`. | | `--from-y` | number | `0` | Start Y position for Drag action (origin: top-left). Drag starts here and moves to `--x`,`--y`. | | `--drag-speed` | number | `2000` | Drag speed in pixels per second (0 for instant). 2000 is fast (default), 200 is slow enough to watch. Applies to Drag, DragMove, and DragEnd actions. | -| `--duration` | number | `0.5` | Hold duration in seconds for LongPress action. | +| `--duration` | number | `0.5` | Hold duration in seconds for LongPress action (max 30). | | `--button` | enum | `Left` | Mouse button. `Click` and `LongPress` support `Left`, `Right`, and `Middle`. Drag actions support `Left` only; other buttons return an error. | | `--bypass-raycast` | flag | - | For `Click`, `LongPress`, `Drag`, and `DragStart`, bypass EventSystem raycast and dispatch pointer events directly to `--target-path`. Use when a raycast-blocking overlay visually covers the intended target. | | `--target-path` | string | `""` | Hierarchy path of the target GameObject, for example `Canvas/Panel/Button`. Required when `--bypass-raycast` is used with `Click`, `LongPress`, `Drag`, or `DragStart`; prefer `AnnotatedElements[].Path` from screenshot JSON. | diff --git a/cli/common/tools/default-tools.json b/cli/common/tools/default-tools.json index 25a313dacc..936bdc5d9f 100644 --- a/cli/common/tools/default-tools.json +++ b/cli/common/tools/default-tools.json @@ -512,7 +512,7 @@ }, "Duration": { "type": "number", - "description": "Hold duration in seconds for LongPress action.", + "description": "Hold duration in seconds for LongPress action (max 30).", "default": 0.5 }, "Button": { @@ -583,7 +583,7 @@ }, "Duration": { "type": "number", - "description": "Hold duration for LongPress, or interpolation duration for SmoothDelta (seconds). For Click, 0 = one-shot tap.", + "description": "Hold duration for LongPress, or interpolation duration for SmoothDelta (seconds, max 30). For Click, 0 = one-shot tap.", "default": 0 }, "DeltaX": { @@ -632,7 +632,7 @@ }, "Duration": { "type": "number", - "description": "Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp/ReleaseAll.", + "description": "Hold duration in seconds for Press action (0 = one-shot tap, max 30). Ignored by KeyDown/KeyUp/ReleaseAll.", "default": 0 } } diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index da719f94b3..9a09e58f30 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "635ca21f5641482cb77d389515c18f7823fc1b74" + "sharedInputsHash": "a94f640d4d0d70a5379786fadf7c5fb2335ca28d" } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 9bbc3bf032..1ef96ef634 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "d636f60df5104d3eb992f547b94f3c1e69f1bb9f" + "sharedInputsHash": "e2a4495ac68e04f0639d86e638cf672282590684" } From a8121ee03235dedd9d2c803d83341c387086aac0 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 15:18:58 +0900 Subject: [PATCH 07/10] feat: report discarded mouse input edges when a pause point interrupts simulation Co-authored-by: Cursor --- ...ouseInputSimulationResponseFactoryTests.cs | 33 +++++++++++++++++-- .../MouseInputPressActionExecutor.cs | 6 ++-- .../MouseInputSimulationResponseFactory.cs | 26 ++++++++++++--- 3 files changed, 55 insertions(+), 10 deletions(-) diff --git a/Assets/Tests/PlayMode/MouseInputSimulationResponseFactoryTests.cs b/Assets/Tests/PlayMode/MouseInputSimulationResponseFactoryTests.cs index e55f6229fd..613d00c765 100644 --- a/Assets/Tests/PlayMode/MouseInputSimulationResponseFactoryTests.cs +++ b/Assets/Tests/PlayMode/MouseInputSimulationResponseFactoryTests.cs @@ -31,7 +31,7 @@ public void TearDown() } /// - /// Verifies interrupted button responses project every Pause Point hit and preserve button coordinates. + /// Verifies an interrupted button response when the press never applied reports discarded-edge wording and maps pause evidence plus coordinates. /// [Test] public void InterruptedButtonResult_WithMultiplePausePointHits_MapsPauseEvidenceAndButtonPosition() @@ -45,13 +45,14 @@ public void InterruptedButtonResult_WithMultiplePausePointHits_MapsPauseEvidence SimulateMouseInputResponse response = MouseInputSimulationResponseFactory.InterruptedButtonResult( UnityCliLoopMouseInputAction.Click, "Left", - inputPosition); + inputPosition, + pressWasApplied: false); Assert.That(response.Success, Is.True); Assert.That( response.Message, Is.EqualTo( - "Mouse input stopped because Unity paused during Pause Point inspection. Unity CLI Loop released its held input bookkeeping.")); + "Mouse input stopped because Unity paused during Pause Point inspection. Button 'Left' was released from Unity CLI Loop bookkeeping; the queued input edge was discarded.")); Assert.That(response.Action, Is.EqualTo(UnityCliLoopMouseInputAction.Click.ToString())); Assert.That(response.Button, Is.EqualTo("Left")); Assert.That(response.PositionX, Is.EqualTo(inputPosition.x)); @@ -64,6 +65,32 @@ public void InterruptedButtonResult_WithMultiplePausePointHits_MapsPauseEvidence Assert.That(response.PausePointHits[1].Id, Is.EqualTo("latest-hit")); } + /// + /// Verifies an interrupted button response when the press already applied reports delivered-before-pause wording. + /// + [Test] + public void InterruptedButtonResult_WhenPressWasApplied_ReportsDeliveredBeforePauseMessage() + { + Vector2 inputPosition = new(12f, 34f); + + SimulateMouseInputResponse response = MouseInputSimulationResponseFactory.InterruptedButtonResult( + UnityCliLoopMouseInputAction.LongPress, + "Right", + inputPosition, + pressWasApplied: true); + + Assert.That(response.Success, Is.True); + Assert.That( + response.Message, + Is.EqualTo( + "Mouse input stopped because Unity paused during Pause Point inspection. Button 'Right' press was already delivered to the game before the pause; Unity CLI Loop released it from bookkeeping, so the game may have registered the press.")); + Assert.That(response.Action, Is.EqualTo(UnityCliLoopMouseInputAction.LongPress.ToString())); + Assert.That(response.Button, Is.EqualTo("Right")); + Assert.That(response.PositionX, Is.EqualTo(inputPosition.x)); + Assert.That(response.PositionY, Is.EqualTo(inputPosition.y)); + Assert.That(response.InterruptedByPausePoint, Is.True); + } + /// /// Verifies timed-out button responses preserve the action, button, coordinates, and timeout message. /// diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputPressActionExecutor.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputPressActionExecutor.cs index c610e37ee9..23222eb452 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputPressActionExecutor.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputPressActionExecutor.cs @@ -131,7 +131,8 @@ await MouseInputMainThreadCleanup.ReleaseButtonIfPossible( return MouseInputSimulationResponseFactory.InterruptedButtonResult( UnityCliLoopMouseInputAction.Click, buttonName, - inputPos); + inputPos, + pressWasApplied); } if (waitOutcome == InputSimulationWaitOutcome.TimedOut) @@ -261,7 +262,8 @@ await MouseInputMainThreadCleanup.ReleaseButtonIfPossible( return MouseInputSimulationResponseFactory.InterruptedButtonResult( UnityCliLoopMouseInputAction.LongPress, buttonName, - inputPos); + inputPos, + pressWasApplied); } if (waitOutcome == InputSimulationWaitOutcome.TimedOut) diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputSimulationResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputSimulationResponseFactory.cs index 775a0d2e99..112fc6592e 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputSimulationResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputSimulationResponseFactory.cs @@ -42,15 +42,31 @@ internal static SimulateMouseInputResponse SuccessButtonResult( }; } + // Why branch on pressWasApplied: Paused has two sources. (a) TryDiscardForPause before + // apply leaves pressWasApplied=false — the queued edge never reached the game. + // (b) WaitForPressLifetime after a successful apply leaves pressWasApplied=true — the press + // already landed (including when that press itself fired the pause point). Claiming + // "discarded" in (b) inverts the diagnosis this message exists to prevent. internal static SimulateMouseInputResponse InterruptedButtonResult( UnityCliLoopMouseInputAction action, string buttonName, - Vector2 inputPos) + Vector2 inputPos, + bool pressWasApplied) { - SimulateMouseInputResponse result = InterruptedActionResult(action); - result.Button = buttonName; - result.PositionX = inputPos.x; - result.PositionY = inputPos.y; + string message = pressWasApplied + ? $"Mouse input stopped because Unity paused during Pause Point inspection. Button '{buttonName}' press was already delivered to the game before the pause; Unity CLI Loop released it from bookkeeping, so the game may have registered the press." + : $"Mouse input stopped because Unity paused during Pause Point inspection. Button '{buttonName}' was released from Unity CLI Loop bookkeeping; the queued input edge was discarded."; + SimulateMouseInputResponse result = new() + { + Success = true, + Message = message, + Action = action.ToString(), + Button = buttonName, + PositionX = inputPos.x, + PositionY = inputPos.y, + InterruptedByPausePoint = true + }; + AttachPausePointHit(result); return result; } From c17a0470ceff511b204ae633c972f5ec16066d30 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 15:49:39 +0900 Subject: [PATCH 08/10] feat: map dynamic code runtime exception stack frames to user snippet lines Co-authored-by: Cursor --- .../CompiledAssemblyBuildResultTests.cs | 67 ++++++++++ .../CompiledAssemblyBuildResultTests.cs.meta | 11 ++ ...ynamicCodeExecutionResponseFactoryTests.cs | 114 ++++++++++++++++++ .../RoslynCompilerBackendTests.cs | 45 +++++++ .../RoslynCompilerBackendTests.cs.meta | 11 ++ .../SharedRoslynCompilerWorkerHostTests.cs | 11 ++ .../CompiledAssemblyBuildResult.cs | 8 ++ .../Compilation/ICompiledAssemblyLoader.cs | 4 +- .../DynamicCodeExecutionResponseFactory.cs | 61 ++++++++++ .../CompiledAssemblyBuilder.cs | 18 ++- .../CompiledAssemblyLoadService.cs | 4 +- .../CompiledAssemblyLoader.cs | 6 +- .../DynamicCompilation/DynamicCodeCompiler.cs | 4 +- .../RoslynCompilerBackend.cs | 4 +- ...redRoslynCompilerWorkerProgram.cs.template | 9 +- 15 files changed, 367 insertions(+), 10 deletions(-) create mode 100644 Assets/Tests/Editor/DynamicCodeToolTests/CompiledAssemblyBuildResultTests.cs create mode 100644 Assets/Tests/Editor/DynamicCodeToolTests/CompiledAssemblyBuildResultTests.cs.meta create mode 100644 Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerBackendTests.cs create mode 100644 Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerBackendTests.cs.meta diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/CompiledAssemblyBuildResultTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/CompiledAssemblyBuildResultTests.cs new file mode 100644 index 0000000000..9f2e15a8db --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/CompiledAssemblyBuildResultTests.cs @@ -0,0 +1,67 @@ +using System.Collections.Generic; +using NUnit.Framework; +using UnityEditor.Compilation; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.DynamicCodeToolTests +{ + /// + /// Characterizes CompiledAssemblyBuildResult DTO fields used for PDB propagation. + /// + [TestFixture] + public sealed class CompiledAssemblyBuildResultTests + { + /// + /// Verifies build results expose PDB bytes alongside assembly bytes for the loader. + /// + [Test] + public void Constructor_WhenPdbBytesProvided_ExposesPdbBytesOnResult() + { + byte[] assemblyBytes = { 0x4D, 0x5A }; + byte[] pdbBytes = { 0x42, 0x53, 0x4A, 0x42 }; + CompilerDiagnostics diagnostics = CompilerDiagnostics.FromMessages(System.Array.Empty()); + + CompiledAssemblyBuildResult result = new( + updatedSource: "return 1;", + diagnostics: diagnostics, + ambiguousTypeCandidates: new Dictionary>(), + autoInjectedNamespaces: new List(), + assemblyBytes: assemblyBytes, + pdbBytes: pdbBytes, + referenceResolutionMilliseconds: 1d, + buildMilliseconds: 2d, + buildCount: 1, + shouldCacheResult: true, + compilationBackendKind: DynamicCompilationBackendKind.SharedRoslynWorker); + + Assert.That(result.AssemblyBytes, Is.SameAs(assemblyBytes)); + Assert.That(result.PdbBytes, Is.SameAs(pdbBytes)); + Assert.That(result.PdbBytes.Length, Is.EqualTo(4)); + } + + /// + /// Verifies null PDB bytes remain null so AssemblyBuilder fallback can skip symbols. + /// + [Test] + public void Constructor_WhenPdbBytesNull_ExposesNullPdbBytes() + { + CompilerDiagnostics diagnostics = CompilerDiagnostics.FromMessages(System.Array.Empty()); + + CompiledAssemblyBuildResult result = new( + updatedSource: "return 1;", + diagnostics: diagnostics, + ambiguousTypeCandidates: new Dictionary>(), + autoInjectedNamespaces: new List(), + assemblyBytes: new byte[] { 0x4D, 0x5A }, + pdbBytes: null, + referenceResolutionMilliseconds: 0d, + buildMilliseconds: 0d, + buildCount: 1, + shouldCacheResult: false, + compilationBackendKind: DynamicCompilationBackendKind.AssemblyBuilderFallback); + + Assert.That(result.PdbBytes, Is.Null); + } + } +} diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/CompiledAssemblyBuildResultTests.cs.meta b/Assets/Tests/Editor/DynamicCodeToolTests/CompiledAssemblyBuildResultTests.cs.meta new file mode 100644 index 0000000000..6706298905 --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/CompiledAssemblyBuildResultTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 6bb6a34f8d2134a53b6dac6ef81fdc2d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs index 497e91b41c..60e3ba17bc 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.cs @@ -191,6 +191,120 @@ public void ConvertExecutionResultToResponse_WithExceptionAndInjectedNamespaces_ "Performance hint: Auto-resolved 2 missing using directive(s): using System.Linq; using UnityEngine; — Include them in your code to skip auto-resolution and improve compilation speed.")); } + /// + /// Verifies a stack frame naming user-snippet.cs prepends the user-snippet line log. + /// + [Test] + public void ConvertExecutionResultToResponse_WhenExceptionStackHasUserSnippet_PrependsLineLog() + { + DynamicCodeExecutionResponseFactory factory = new(); + Exception exception = ExceptionWithStackTrace( + new NullReferenceException("Object reference not set to an instance of an object."), + " at DynamicCode.GeneratedClass.Execute () [0x00000] in user-snippet.cs:line 3\n" + + " at io.github.hatayama.UnityCliLoop.FirstPartyTools.CommandRunner.Run ()"); + ExecutionResult result = new() + { + Success = false, + ErrorMessage = "Runtime exception", + Exception = exception, + Logs = new List { "prior" } + }; + + ExecuteDynamicCodeResponse response = factory.ConvertExecutionResultToResponse(result); + + Assert.That(response.Logs[0], Is.EqualTo( + "Exception at user snippet line 3: Object reference not set to an instance of an object.")); + Assert.That(response.Logs, Contains.Item("Exception: Object reference not set to an instance of an object.")); + } + + /// + /// Verifies CommandRunner-style Logs (no Exception field) still get a snippet-line header. + /// + [Test] + public void ConvertExecutionResultToResponse_WhenLogsContainUnitySnippetFrame_PrependsLineLog() + { + DynamicCodeExecutionResponseFactory factory = new(); + ExecutionResult result = new() + { + Success = false, + ErrorMessage = "Object reference not set to an instance of an object", + Logs = new List + { + "Execution exception: Object reference not set to an instance of an object", + "Stack trace: at UnityCliLoop.Dynamic.DynamicCommand.ExecuteAsync () " + + "[0x00017] in /tmp/UnityCliLoopCompilation/user-snippet.cs:3 " + } + }; + + ExecuteDynamicCodeResponse response = factory.ConvertExecutionResultToResponse(result); + + Assert.That(response.Logs[0], Is.EqualTo( + "Exception at user snippet line 3: Object reference not set to an instance of an object")); + } + + /// + /// Verifies stacks without user-snippet.cs leave Logs without a snippet-line header. + /// + [Test] + public void TryExtractUserSnippetLineNumber_WhenStackHasNoUserSnippet_ReturnsFalse() + { + bool extracted = DynamicCodeExecutionResponseFactory.TryExtractUserSnippetLineNumber( + " at System.String.ToString ()\n at Some.Other.Type.Method ()", + out int lineNumber); + + Assert.That(extracted, Is.False); + Assert.That(lineNumber, Is.EqualTo(0)); + } + + /// + /// Verifies the first user-snippet.cs:line N frame is extracted from a stack string. + /// + [Test] + public void TryExtractUserSnippetLineNumber_WhenStackContainsUserSnippet_ReturnsLine() + { + bool extracted = DynamicCodeExecutionResponseFactory.TryExtractUserSnippetLineNumber( + " at Foo.Bar () in /tmp/wrapper.cs:line 40\n" + + " at DynamicCode.GeneratedClass.Execute () in user-snippet.cs:line 3\n" + + " at DynamicCode.GeneratedClass.Execute () in user-snippet.cs:line 7", + out int lineNumber); + + Assert.That(extracted, Is.True); + Assert.That(lineNumber, Is.EqualTo(3)); + } + + /// + /// Verifies Unity/Mono frames that omit the "line" keyword still extract the snippet line. + /// + [Test] + public void TryExtractUserSnippetLineNumber_WhenUnityFormatOmitsLineKeyword_ReturnsLine() + { + bool extracted = DynamicCodeExecutionResponseFactory.TryExtractUserSnippetLineNumber( + " at UnityCliLoop.Dynamic.DynamicCommand.ExecuteAsync () " + + "[0x00017] in /tmp/UnityCliLoopCompilation/user-snippet.cs:3 ", + out int lineNumber); + + Assert.That(extracted, Is.True); + Assert.That(lineNumber, Is.EqualTo(3)); + } + + private static Exception ExceptionWithStackTrace(Exception exception, string stackTrace) + { + return new StackTraceOverrideException(exception.Message, stackTrace); + } + + private sealed class StackTraceOverrideException : Exception + { + private readonly string _stackTrace; + + public StackTraceOverrideException(string message, string stackTrace) + : base(message) + { + _stackTrace = stackTrace; + } + + public override string StackTrace => _stackTrace; + } + /// /// Verifies cancelled results are recognized and mapped to the neutral cancellation response. /// diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerBackendTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerBackendTests.cs new file mode 100644 index 0000000000..b121db37d0 --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerBackendTests.cs @@ -0,0 +1,45 @@ +using System.Collections.Generic; +using System.IO; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.DynamicCodeToolTests +{ + /// + /// Guards one-shot csc response-file options that enable portable PDB emission. + /// + [TestFixture] + public sealed class RoslynCompilerBackendTests + { + /// + /// Verifies WriteCompilerResponseFile emits -debug:portable so the one-shot csc path keeps PDBs. + /// + [Test] + public void WriteCompilerResponseFile_IncludesPortableDebugOption() + { + string responseFilePath = Path.Combine(Path.GetTempPath(), "uloop-roslyn-rsp-" + Path.GetRandomFileName()); + try + { + RoslynCompilerBackend.WriteCompilerResponseFile( + responseFilePath, + sourcePath: "snippet.cs", + dllPath: "snippet.dll", + references: new List(), + defineSymbols: new List(), + allowUnsafeCode: false); + + string[] lines = File.ReadAllLines(responseFilePath); + Assert.That(lines, Does.Contain("-debug:portable")); + Assert.That(lines, Does.Not.Contain("-debug-")); + } + finally + { + if (File.Exists(responseFilePath)) + { + File.Delete(responseFilePath); + } + } + } + } +} diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerBackendTests.cs.meta b/Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerBackendTests.cs.meta new file mode 100644 index 0000000000..e50ad29039 --- /dev/null +++ b/Assets/Tests/Editor/DynamicCodeToolTests/RoslynCompilerBackendTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 0c958665d0c3e4957b7156fc4c6b694b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs b/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs index 232dd9588e..3607b0631f 100644 --- a/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs +++ b/Assets/Tests/Editor/DynamicCodeToolTests/SharedRoslynCompilerWorkerHostTests.cs @@ -581,6 +581,17 @@ public void CreateProgramSource_WhenTemplateIsLoaded_ShouldReplaceTokens() Assert.That(programSource, Does.Not.Contain("{{")); } + /// + /// Verifies the shared worker template still emits portable PDB debug information. + /// + [Test] + public void CreateProgramSource_IncludesPortablePdbEmitOptions() + { + string programSource = SharedRoslynCompilerWorkerProtocol.CreateProgramSource(); + + Assert.That(programSource, Does.Contain("DebugInformationFormat.PortablePdb")); + } + [Test] public void CreateProgramSource_WhenRequestPathPrefixHasLeadingGarbage_ShouldDecodeEncodedPath() { diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Compilation/CompiledAssemblyBuildResult.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Compilation/CompiledAssemblyBuildResult.cs index b33635e0fc..901496c7f4 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Compilation/CompiledAssemblyBuildResult.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Compilation/CompiledAssemblyBuildResult.cs @@ -17,6 +17,12 @@ public sealed class CompiledAssemblyBuildResult public byte[] AssemblyBytes { get; } + /// + /// Portable PDB bytes when the compiler emitted them; null for AssemblyBuilder fallback + /// or failed builds. Loaded with AssemblyBytes so runtime stacks can name user-snippet.cs. + /// + public byte[] PdbBytes { get; } + public double ReferenceResolutionMilliseconds { get; } public double BuildMilliseconds { get; } @@ -33,6 +39,7 @@ public CompiledAssemblyBuildResult( Dictionary> ambiguousTypeCandidates, List autoInjectedNamespaces, byte[] assemblyBytes, + byte[] pdbBytes, double referenceResolutionMilliseconds, double buildMilliseconds, int buildCount, @@ -44,6 +51,7 @@ public CompiledAssemblyBuildResult( AmbiguousTypeCandidates = ambiguousTypeCandidates; AutoInjectedNamespaces = autoInjectedNamespaces; AssemblyBytes = assemblyBytes; + PdbBytes = pdbBytes; ReferenceResolutionMilliseconds = referenceResolutionMilliseconds; BuildMilliseconds = buildMilliseconds; BuildCount = buildCount; diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Compilation/ICompiledAssemblyLoader.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Compilation/ICompiledAssemblyLoader.cs index 37f46ed201..8510c72b72 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Compilation/ICompiledAssemblyLoader.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Compilation/ICompiledAssemblyLoader.cs @@ -5,6 +5,8 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// public interface ICompiledAssemblyLoader { - CompiledAssemblyLoadResult Load(byte[] assemblyBytes); + // Why pdbBytes: optional portable PDB from shared-worker / one-shot csc; null keeps the + // AssemblyBuilder fallback path (no line numbers) working without a second Load API. + CompiledAssemblyLoadResult Load(byte[] assemblyBytes, byte[] pdbBytes); } } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs index c9cf55a04b..c9622f1588 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text.RegularExpressions; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -101,6 +102,12 @@ internal ExecuteDynamicCodeResponse ConvertExecutionResultToResponse( { ApplyExceptionResponseDetails(response, result.Exception); } + else + { + // Why also scan Logs: CommandRunner puts runtime stacks into Logs without setting + // ExecutionResult.Exception, so ApplyExceptionResponseDetails alone would miss them. + PrependUserSnippetExceptionLine(response, null); + } if (result.AutoInjectedNamespaces != null && result.AutoInjectedNamespaces.Count > 0) { @@ -158,6 +165,7 @@ private static void ApplyExceptionResponseDetails( Exception exception) { response.Logs ??= new List(); + PrependUserSnippetExceptionLine(response, exception); response.Logs.Add($"Exception: {exception.Message}"); if (!string.IsNullOrEmpty(exception.StackTrace)) { @@ -165,6 +173,59 @@ private static void ApplyExceptionResponseDetails( } } + // Why prepend: agents scan Logs top-down; the raw stack still follows for detail. + private static void PrependUserSnippetExceptionLine( + ExecuteDynamicCodeResponse response, + Exception exception) + { + response.Logs ??= new List(); + string stackHaystack = exception?.StackTrace; + if (string.IsNullOrEmpty(stackHaystack)) + { + stackHaystack = string.Join("\n", response.Logs); + } + + if (!TryExtractUserSnippetLineNumber(stackHaystack, out int userSnippetLine)) + { + return; + } + + string message = exception?.Message; + if (string.IsNullOrEmpty(message)) + { + message = response.ErrorMessage ?? string.Empty; + } + + string header = $"Exception at user snippet line {userSnippetLine}: {message}"; + if (response.Logs.Count > 0 && string.Equals(response.Logs[0], header, StringComparison.Ordinal)) + { + return; + } + + response.Logs.Insert(0, header); + } + + // Why string parse only: wrapper already emits #line 1 "user-snippet.cs", so a portable + // PDB records user lines directly — no wrapper-to-user conversion table. + // Why both formats: .NET uses "user-snippet.cs:line N"; Unity/Mono often uses + // "…/user-snippet.cs:N" without the "line" keyword. + internal static bool TryExtractUserSnippetLineNumber(string stackTrace, out int lineNumber) + { + lineNumber = 0; + if (string.IsNullOrEmpty(stackTrace)) + { + return false; + } + + Match match = Regex.Match(stackTrace, @"user-snippet\.cs:(?:line )?(\d+)"); + if (!match.Success) + { + return false; + } + + return int.TryParse(match.Groups[1].Value, out lineNumber) && lineNumber > 0; + } + private static void AddAutoInjectedNamespaceHint( ExecuteDynamicCodeResponse response, List autoInjectedNamespaces) diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyBuilder.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyBuilder.cs index 1387335251..884cad0704 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyBuilder.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyBuilder.cs @@ -42,18 +42,22 @@ private sealed class BuildAttemptResult public byte[] AssemblyBytes { get; } + public byte[] PdbBytes { get; } + public BuildAttemptResult( string updatedSource, CompilerDiagnostics diagnostics, Dictionary> ambiguousTypeCandidates, List autoInjectedNamespaces, - byte[] assemblyBytes) + byte[] assemblyBytes, + byte[] pdbBytes) { UpdatedSource = updatedSource; Diagnostics = diagnostics; AmbiguousTypeCandidates = ambiguousTypeCandidates; AutoInjectedNamespaces = autoInjectedNamespaces; AssemblyBytes = assemblyBytes; + PdbBytes = pdbBytes; } } @@ -135,6 +139,7 @@ async Task BuildFunc( attemptResult.AmbiguousTypeCandidates, attemptResult.AutoInjectedNamespaces, attemptResult.AssemblyBytes, + attemptResult.PdbBytes, referenceResolutionMilliseconds, buildMilliseconds, buildCount, @@ -210,9 +215,17 @@ async Task BuildPreparedCodeAsync( autoResult); byte[] assemblyBytes = null; + byte[] pdbBytes = null; if (diagnostics.Errors.Count == 0) { assemblyBytes = File.ReadAllBytes(dllPath); + // Why read before delete: portable PDB is required for Assembly.Load to + // attach sequence points; the temp file is deleted with the dll below. + string pdbPath = Path.ChangeExtension(dllPath, ".pdb"); + if (File.Exists(pdbPath)) + { + pdbBytes = File.ReadAllBytes(pdbPath); + } } return new BuildAttemptResult( @@ -220,7 +233,8 @@ async Task BuildPreparedCodeAsync( diagnostics, autoResult.AmbiguousTypeCandidates, autoInjectedNamespaces, - assemblyBytes); + assemblyBytes, + pdbBytes); } } finally diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyLoadService.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyLoadService.cs index 2e46abad45..1118e385e1 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyLoadService.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyLoadService.cs @@ -5,9 +5,9 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// internal sealed class CompiledAssemblyLoadService : ICompiledAssemblyLoader { - public CompiledAssemblyLoadResult Load(byte[] assemblyBytes) + public CompiledAssemblyLoadResult Load(byte[] assemblyBytes, byte[] pdbBytes) { - return CompiledAssemblyLoader.Load(assemblyBytes); + return CompiledAssemblyLoader.Load(assemblyBytes, pdbBytes); } } } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyLoader.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyLoader.cs index 718af43691..ba2ccee803 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyLoader.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyLoader.cs @@ -9,12 +9,14 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// internal static class CompiledAssemblyLoader { - public static CompiledAssemblyLoadResult Load(byte[] assemblyBytes) + public static CompiledAssemblyLoadResult Load(byte[] assemblyBytes, byte[] pdbBytes) { Debug.Assert(assemblyBytes != null, "assemblyBytes must not be null"); Stopwatch stopwatch = Stopwatch.StartNew(); - Assembly compiledAssembly = Assembly.Load(assemblyBytes); + Assembly compiledAssembly = pdbBytes != null && pdbBytes.Length > 0 + ? Assembly.Load(assemblyBytes, pdbBytes) + : Assembly.Load(assemblyBytes); stopwatch.Stop(); return new CompiledAssemblyLoadResult( diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeCompiler.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeCompiler.cs index a1c01f4837..a205b7b52d 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeCompiler.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeCompiler.cs @@ -135,7 +135,9 @@ public async Task CompileAsync(CompilationRequest request, Ca return failureResult; } - CompiledAssemblyLoadResult assemblyLoadResult = _assemblyLoader.Load(buildResult.AssemblyBytes); + CompiledAssemblyLoadResult assemblyLoadResult = _assemblyLoader.Load( + buildResult.AssemblyBytes, + buildResult.PdbBytes); CompilationResult result = new() { Success = true, diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerBackend.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerBackend.cs index 56a32305bd..e0c3d03178 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerBackend.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/RoslynCompilerBackend.cs @@ -230,7 +230,9 @@ internal static void WriteCompilerResponseFile( "-nostdlib+", "-target:library", "-optimize+", - "-debug-", + // Why portable: one-shot csc fallback must emit a PDB so Assembly.Load can map + // runtime exceptions back to user-snippet.cs lines (same as the shared worker). + "-debug:portable", allowUnsafeCode ? "-unsafe+" : "-unsafe-", QuoteResponseFileArgument("-out:", dllPath) }; diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/Templates/SharedRoslynCompilerWorkerProgram.cs.template b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/Templates/SharedRoslynCompilerWorkerProgram.cs.template index 9fd5966ab6..04809f80b9 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/Templates/SharedRoslynCompilerWorkerProgram.cs.template +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/Templates/SharedRoslynCompilerWorkerProgram.cs.template @@ -167,9 +167,16 @@ public static class Program SyntaxTree syntaxTree = CSharpSyntaxTree.ParseText(sourceText, CreateParseOptions(defineSymbols), sourcePath); List references = BuildReferences(requestLines); CSharpCompilation compilation = CSharpCompilation.Create(Path.GetFileNameWithoutExtension(dllPath), new[] { syntaxTree }, references, CreateCompilationOptions(allowUnsafe)); + // Why portable PDB: execute-dynamic-code loads these bytes so runtime exceptions can + // surface user-snippet.cs line numbers from #line directives in the wrapper. + string pdbPath = Path.ChangeExtension(dllPath, ".pdb"); using (FileStream peStream = new FileStream(dllPath, FileMode.Create, FileAccess.Write, FileShare.Read)) + using (FileStream pdbStream = new FileStream(pdbPath, FileMode.Create, FileAccess.Write, FileShare.Read)) { - EmitResult emitResult = compilation.Emit(peStream); + EmitResult emitResult = compilation.Emit( + peStream, + pdbStream, + options: new EmitOptions(debugInformationFormat: DebugInformationFormat.PortablePdb)); int exitCode = 0; List diagnosticLines = new List(); foreach (Diagnostic diagnostic in emitResult.Diagnostics) From 10e7d71e58bf63bb9fb267e370ac3e34015dead4 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Wed, 29 Jul 2026 15:55:03 +0900 Subject: [PATCH 09/10] docs: fill pause point skill gaps found in round 13-14 verification Co-authored-by: Cursor --- .agents/skills/uloop-pause-point/SKILL.md | 12 +++++++++++- .../references/captured-variables.md | 2 ++ .../references/fast-progressing-games.md | 4 ++++ .claude/skills/uloop-pause-point/SKILL.md | 12 +++++++++++- .../references/captured-variables.md | 2 ++ .../references/fast-progressing-games.md | 4 ++++ .../Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md | 12 +++++++++++- .../Skill/references/captured-variables.md | 2 ++ .../Skill/references/fast-progressing-games.md | 4 ++++ 9 files changed, 51 insertions(+), 3 deletions(-) diff --git a/.agents/skills/uloop-pause-point/SKILL.md b/.agents/skills/uloop-pause-point/SKILL.md index 178a352a16..12d557673e 100644 --- a/.agents/skills/uloop-pause-point/SKILL.md +++ b/.agents/skills/uloop-pause-point/SKILL.md @@ -24,7 +24,7 @@ When the game reaches the line on its own, omit `--trigger`. Fall back to split `--timeout-seconds` on enable starts the marker lifetime clock at enable time and is also the deadline `--await` waits against, so size it to cover both the trigger and the wait. -The response returns the derived marker `Id` (`Assets/Scripts/Enemy.cs:42`), the `ResolvedLine` that was actually patched, the `ResolvedMethod`, and `ResolvedLineText` — the actual source text at `ResolvedLine`. When the requested line has no executable statement, the pause point rounds forward to the next executable line — check `ResolvedLine`/`ResolvedLineText` when precision matters, and re-check them after every code edit — a rewritten file shifts line numbers. Use the returned `Id` for every follow-up command. On a hit, this same response already carries `CapturedVariables` and every other field `await-pause-point` would have returned — no separate `await-pause-point` call is needed. +The response returns the derived marker `Id` (`Assets/Scripts/Enemy.cs:42`), the `ResolvedLine` that was actually patched, the `ResolvedMethod`, and `ResolvedLineText` — the actual source text at `ResolvedLine`. When the requested line has no executable statement, the pause point rounds forward to the next executable line — check `ResolvedLine`/`ResolvedLineText` when precision matters, and re-check them after every code edit — a rewritten file shifts line numbers. Use the returned `Id` for every follow-up command. On a hit, this same response already carries `CapturedVariables` and every other field `await-pause-point` would have returned — no separate `await-pause-point` call is needed. `EditorState` on a hit response is a snapshot from the moment of the hit. After `--resume-play`, a successful await response can still show `EditorState.IsPaused: true` from that hit — it is not the Editor's current pause flag. Read the live state with `control-play-mode --action Status`. 3. Read `CapturedVariables` in the hit response first: the locals, parameters, and `this` instance fields at the paused line are already there (see Reading CapturedVariables). 4. While Unity is still paused, capture any additional evidence with `uloop execute-dynamic-code`, `uloop get-hierarchy`, `uloop find-game-objects`, and one screenshot. @@ -96,6 +96,7 @@ Choose the capture mode when enabling a pause point: - `single-shot` is the default. The first hit pauses Unity and disarms the marker. - `continuous` pauses Unity on every hit and remains armed. `CapturedVariables` holds the latest hit; `CapturedVariableHistory` holds only strictly older frames, so with a single hit the history is empty. - `trace` remains armed and records each hit without pausing Unity. +- For an already-hit `continuous` or `trace` marker, `await-pause-point` waits for a **new** hit after the wait starts (`LastHitSequence` advancing). It does not return the stale hit that is already present. Read the current hit with `pause-point-status` instead. If await times out while waiting for that new hit, the error stays `PAUSE_POINT_WAIT_TIMEOUT` and `Details.Hint` tells you to pass `--resume-play` (or resume Play Mode) so another hit can occur. A freshly enabled marker (including `enable-pause-point --await`) has no prior hit, so the first hit satisfies the wait as before. `--max-history` defaults to 20 and accepts values from 1 through 100. When the limit is exceeded, the oldest frames are dropped and `HistoryDroppedCount` reports how many were removed. `pause-point-status` returns the current `Mode`, `MaxHistory`, history frames, and dropped count. @@ -112,6 +113,14 @@ Every hit response embeds `CapturedVariables`: the method's in-scope locals, its - `--captured-variables names` on `await-pause-point`/`pause-point-status` drops every `Value` and keeps `Name`/`Scope`/`TypeName` — use it first on field-heavy classes, then fetch full values with a plain `pause-point-status` call. - When the response would be dominated by variables you do not need, pass `--captured-variable-names velocity,this` (comma-separated, exact match on `Name`) to keep only those entries; it composes with `--captured-variables full|names`. `CapturedVariablesTruncated` in the response reports truncation at Unity-side capture time and is unrelated to this name filter — it can be `true` even when every requested name was found. Requested names that matched nothing are listed in `CapturedVariableNamesNotFound`, so a partial match is visible without comparing the response against the request by hand. - Pass `--expect 'name=value'` (repeatable; on `await-pause-point`, `enable-pause-point --await`, and `pause-point-status`) to have the CLI compare captured variables against expected values; the response includes an `Expectations` array and `AllExpectationsPassed`, so you do not need to eyeball the JSON. Matching is string equality against the serialized value. On `pause-point-status` a marker that has not been hit yet reports each expectation as not found, and the verdict never changes the exit code — a polling loop reads `AllExpectationsPassed`, not the exit status. + Serialized `value` forms that match in practice (string equality against `CapturedVariables[].Value`): + - bool: `True` / `False` (C# form, capital first letter) + - float: `7` when the value is exactly an integer (not `7.0`) + - Vector2/3 and custom structs via `ToString()`: `(2.31, 6.61)` (one space after each comma) + - enum: `Grass` (member name only) + - `List` / arrays: `[19]`, `[0,1,2,3]` (numeric elements unquoted) + - `List` and other element-`ToString()` collections: `["(9, 3)","(9, 2)"]` (elements quoted) + When unsure, hit once and copy the `Value` string from `CapturedVariables` into `--expect` verbatim. - Collection values (arrays, `List`, dictionaries, plain objects) render as a JSON preview capped at 10 elements by default. When the elements you need sit past that cap (a 10x20 grid, a long list), re-enable with `--max-preview-elements ` (1–1000). The value set at enable time also caps the previews in every later `pause-point-status` response for that marker — status has no flag to change it. - While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; the return is a `(bool Found, object Value)` tuple, and the holder clears on resume. (file:line marker hits only — id-only markers store no capture) These are **live objects in their frame-completed state, not snapshots** — use them only to dig further into objects that are still alive, never to reconstruct what a value was at the paused line. @@ -189,6 +198,7 @@ For `ResumePlayResult` semantics, why `Time.timeScale = 0` is not a substitute f - Prefer natural runtime points after input has been consumed, such as after a command is accepted, a state value changes, an evaluation step resolves, or a dependent component is updated. - For frame-specific bugs, target the suspicious state branch or the line right after the mutation you need to freeze (the snapshot is taken before the target line runs). - A line that runs unconditionally every frame hits on the very next frame, before the input or event you actually wanted to observe arrives. If you need to catch a specific moment, choose a line that only executes conditionally (inside an `if` guarding the event you care about) so the pause point does not fire prematurely. The opposite applies to `continuous` mode paired with a watch expression: the watch only re-evaluates on a paused frame where the marker's line executes, so a conditional line that stops being reached leaves the watch value frozen (see Watch Expressions) — pick a line reached every frame when you need continuous per-Step updates. +- To verify held input (WASD and similar) against a line that runs every frame, do **not** use `--trigger`: call `simulate-keyboard --action KeyDown --key W` first so the key is already held, then arm the marker (optionally with `--await`). Reversing that order — arm then trigger KeyDown — races the every-frame hit before the hold is applied. Release with `KeyUp` or `ReleaseAll` when done. - When every reachable line around the state change you want runs unconditionally every frame, with no existing `if` to hang the pause point on, move the moment you want to observe into a conditional block: `if () { ; UnityEngine.Debug.Assert(); }`, then target the pause point at the assert line: it executes only when the event actually happens, states the mutation's postcondition, and can stay in the codebase after the investigation. Use `UnityEngine.Debug.Assert`, not `System.Diagnostics.Debug.Assert`: a failed System.Diagnostics assert never reaches the Unity Console, so `get-logs` cannot observe it. - An empty-body loop such as `while (TryMove(0, 1)) { }` has no statement inside the braces, so a pause point on the line right after the loop hits at the loop's condition re-check, not once the loop has actually finished advancing. If you need the state after the loop completes, target a line that is guaranteed to run exactly once after the loop exits, not the loop line itself. - Enable pause points after PlayMode is running: entering PlayMode with Domain Reload enabled silently removes every source pause point (`enable-pause-point` warns when this applies); with Domain Reload disabled this does not happen. diff --git a/.agents/skills/uloop-pause-point/references/captured-variables.md b/.agents/skills/uloop-pause-point/references/captured-variables.md index 401c3e08e7..a4a794a8bd 100644 --- a/.agents/skills/uloop-pause-point/references/captured-variables.md +++ b/.agents/skills/uloop-pause-point/references/captured-variables.md @@ -63,6 +63,8 @@ Capturing a deep copy at hit time was deliberately not adopted: it would cost ho ## Raw Capture API While Paused +Add `using io.github.hatayama.UnityCliLoop.Runtime;` in `execute-dynamic-code` snippets before calling `UloopPausePoint.TryGetCapturedValue` / `GetCapturedNames` / `GetCapturedPausePointId`. + While Unity is paused on a hit, `execute-dynamic-code` can read live captured references through `UloopPausePoint`: - `TryGetCapturedValue(string name)` returns `(bool Found, object Value)` for the latest hit only. When multiple captured variables share the same name, the last one wins. diff --git a/.agents/skills/uloop-pause-point/references/fast-progressing-games.md b/.agents/skills/uloop-pause-point/references/fast-progressing-games.md index 60017b8600..281998603c 100644 --- a/.agents/skills/uloop-pause-point/references/fast-progressing-games.md +++ b/.agents/skills/uloop-pause-point/references/fast-progressing-games.md @@ -20,6 +20,10 @@ uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seco Digit keys are `Digit0`-`Digit9` or `Numpad0`-`Numpad9` — bare `0`-`9` is rejected. +## Clear Before Scenario Setup + +`clear-pause-point --all` (and clearing the marker that owns the current pause) resumes Play Mode when the pause came from a pause-point hit. Run clear **before** arranging the board or other scenario setup; otherwise the resume lets the game consume your setup mid-flight. If you still need to build state after clear, re-freeze with `control-play-mode --action Pause` first, then arrange, then arm with `--resume-play`. + ## --resume-play Semantics `--resume-play` runs after the marker's arming is confirmed and before `--trigger` is dispatched: it resumes PlayMode only when PlayMode is actually paused, and reports what it did in `ResumePlayResult` (`WasPaused` / `Resumed` / `Error`; an abandoned wait adds `Repaused` / `RepauseError`). If the resume fails, the trigger is not dispatched and `TriggerResult.Error` says so. If the trigger itself is rejected before it runs, the wait is abandoned and the resume is undone: `Repaused: true` (or `RepauseError`) reports PlayMode being paused again, so gameplay cannot consume the preserved marker while the trigger value is being fixed. When the game reaches the line on its own after resuming (gravity, physics), omit `--trigger` and keep `--resume-play`. diff --git a/.claude/skills/uloop-pause-point/SKILL.md b/.claude/skills/uloop-pause-point/SKILL.md index 178a352a16..12d557673e 100644 --- a/.claude/skills/uloop-pause-point/SKILL.md +++ b/.claude/skills/uloop-pause-point/SKILL.md @@ -24,7 +24,7 @@ When the game reaches the line on its own, omit `--trigger`. Fall back to split `--timeout-seconds` on enable starts the marker lifetime clock at enable time and is also the deadline `--await` waits against, so size it to cover both the trigger and the wait. -The response returns the derived marker `Id` (`Assets/Scripts/Enemy.cs:42`), the `ResolvedLine` that was actually patched, the `ResolvedMethod`, and `ResolvedLineText` — the actual source text at `ResolvedLine`. When the requested line has no executable statement, the pause point rounds forward to the next executable line — check `ResolvedLine`/`ResolvedLineText` when precision matters, and re-check them after every code edit — a rewritten file shifts line numbers. Use the returned `Id` for every follow-up command. On a hit, this same response already carries `CapturedVariables` and every other field `await-pause-point` would have returned — no separate `await-pause-point` call is needed. +The response returns the derived marker `Id` (`Assets/Scripts/Enemy.cs:42`), the `ResolvedLine` that was actually patched, the `ResolvedMethod`, and `ResolvedLineText` — the actual source text at `ResolvedLine`. When the requested line has no executable statement, the pause point rounds forward to the next executable line — check `ResolvedLine`/`ResolvedLineText` when precision matters, and re-check them after every code edit — a rewritten file shifts line numbers. Use the returned `Id` for every follow-up command. On a hit, this same response already carries `CapturedVariables` and every other field `await-pause-point` would have returned — no separate `await-pause-point` call is needed. `EditorState` on a hit response is a snapshot from the moment of the hit. After `--resume-play`, a successful await response can still show `EditorState.IsPaused: true` from that hit — it is not the Editor's current pause flag. Read the live state with `control-play-mode --action Status`. 3. Read `CapturedVariables` in the hit response first: the locals, parameters, and `this` instance fields at the paused line are already there (see Reading CapturedVariables). 4. While Unity is still paused, capture any additional evidence with `uloop execute-dynamic-code`, `uloop get-hierarchy`, `uloop find-game-objects`, and one screenshot. @@ -96,6 +96,7 @@ Choose the capture mode when enabling a pause point: - `single-shot` is the default. The first hit pauses Unity and disarms the marker. - `continuous` pauses Unity on every hit and remains armed. `CapturedVariables` holds the latest hit; `CapturedVariableHistory` holds only strictly older frames, so with a single hit the history is empty. - `trace` remains armed and records each hit without pausing Unity. +- For an already-hit `continuous` or `trace` marker, `await-pause-point` waits for a **new** hit after the wait starts (`LastHitSequence` advancing). It does not return the stale hit that is already present. Read the current hit with `pause-point-status` instead. If await times out while waiting for that new hit, the error stays `PAUSE_POINT_WAIT_TIMEOUT` and `Details.Hint` tells you to pass `--resume-play` (or resume Play Mode) so another hit can occur. A freshly enabled marker (including `enable-pause-point --await`) has no prior hit, so the first hit satisfies the wait as before. `--max-history` defaults to 20 and accepts values from 1 through 100. When the limit is exceeded, the oldest frames are dropped and `HistoryDroppedCount` reports how many were removed. `pause-point-status` returns the current `Mode`, `MaxHistory`, history frames, and dropped count. @@ -112,6 +113,14 @@ Every hit response embeds `CapturedVariables`: the method's in-scope locals, its - `--captured-variables names` on `await-pause-point`/`pause-point-status` drops every `Value` and keeps `Name`/`Scope`/`TypeName` — use it first on field-heavy classes, then fetch full values with a plain `pause-point-status` call. - When the response would be dominated by variables you do not need, pass `--captured-variable-names velocity,this` (comma-separated, exact match on `Name`) to keep only those entries; it composes with `--captured-variables full|names`. `CapturedVariablesTruncated` in the response reports truncation at Unity-side capture time and is unrelated to this name filter — it can be `true` even when every requested name was found. Requested names that matched nothing are listed in `CapturedVariableNamesNotFound`, so a partial match is visible without comparing the response against the request by hand. - Pass `--expect 'name=value'` (repeatable; on `await-pause-point`, `enable-pause-point --await`, and `pause-point-status`) to have the CLI compare captured variables against expected values; the response includes an `Expectations` array and `AllExpectationsPassed`, so you do not need to eyeball the JSON. Matching is string equality against the serialized value. On `pause-point-status` a marker that has not been hit yet reports each expectation as not found, and the verdict never changes the exit code — a polling loop reads `AllExpectationsPassed`, not the exit status. + Serialized `value` forms that match in practice (string equality against `CapturedVariables[].Value`): + - bool: `True` / `False` (C# form, capital first letter) + - float: `7` when the value is exactly an integer (not `7.0`) + - Vector2/3 and custom structs via `ToString()`: `(2.31, 6.61)` (one space after each comma) + - enum: `Grass` (member name only) + - `List` / arrays: `[19]`, `[0,1,2,3]` (numeric elements unquoted) + - `List` and other element-`ToString()` collections: `["(9, 3)","(9, 2)"]` (elements quoted) + When unsure, hit once and copy the `Value` string from `CapturedVariables` into `--expect` verbatim. - Collection values (arrays, `List`, dictionaries, plain objects) render as a JSON preview capped at 10 elements by default. When the elements you need sit past that cap (a 10x20 grid, a long list), re-enable with `--max-preview-elements ` (1–1000). The value set at enable time also caps the previews in every later `pause-point-status` response for that marker — status has no flag to change it. - While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; the return is a `(bool Found, object Value)` tuple, and the holder clears on resume. (file:line marker hits only — id-only markers store no capture) These are **live objects in their frame-completed state, not snapshots** — use them only to dig further into objects that are still alive, never to reconstruct what a value was at the paused line. @@ -189,6 +198,7 @@ For `ResumePlayResult` semantics, why `Time.timeScale = 0` is not a substitute f - Prefer natural runtime points after input has been consumed, such as after a command is accepted, a state value changes, an evaluation step resolves, or a dependent component is updated. - For frame-specific bugs, target the suspicious state branch or the line right after the mutation you need to freeze (the snapshot is taken before the target line runs). - A line that runs unconditionally every frame hits on the very next frame, before the input or event you actually wanted to observe arrives. If you need to catch a specific moment, choose a line that only executes conditionally (inside an `if` guarding the event you care about) so the pause point does not fire prematurely. The opposite applies to `continuous` mode paired with a watch expression: the watch only re-evaluates on a paused frame where the marker's line executes, so a conditional line that stops being reached leaves the watch value frozen (see Watch Expressions) — pick a line reached every frame when you need continuous per-Step updates. +- To verify held input (WASD and similar) against a line that runs every frame, do **not** use `--trigger`: call `simulate-keyboard --action KeyDown --key W` first so the key is already held, then arm the marker (optionally with `--await`). Reversing that order — arm then trigger KeyDown — races the every-frame hit before the hold is applied. Release with `KeyUp` or `ReleaseAll` when done. - When every reachable line around the state change you want runs unconditionally every frame, with no existing `if` to hang the pause point on, move the moment you want to observe into a conditional block: `if () { ; UnityEngine.Debug.Assert(); }`, then target the pause point at the assert line: it executes only when the event actually happens, states the mutation's postcondition, and can stay in the codebase after the investigation. Use `UnityEngine.Debug.Assert`, not `System.Diagnostics.Debug.Assert`: a failed System.Diagnostics assert never reaches the Unity Console, so `get-logs` cannot observe it. - An empty-body loop such as `while (TryMove(0, 1)) { }` has no statement inside the braces, so a pause point on the line right after the loop hits at the loop's condition re-check, not once the loop has actually finished advancing. If you need the state after the loop completes, target a line that is guaranteed to run exactly once after the loop exits, not the loop line itself. - Enable pause points after PlayMode is running: entering PlayMode with Domain Reload enabled silently removes every source pause point (`enable-pause-point` warns when this applies); with Domain Reload disabled this does not happen. diff --git a/.claude/skills/uloop-pause-point/references/captured-variables.md b/.claude/skills/uloop-pause-point/references/captured-variables.md index 401c3e08e7..a4a794a8bd 100644 --- a/.claude/skills/uloop-pause-point/references/captured-variables.md +++ b/.claude/skills/uloop-pause-point/references/captured-variables.md @@ -63,6 +63,8 @@ Capturing a deep copy at hit time was deliberately not adopted: it would cost ho ## Raw Capture API While Paused +Add `using io.github.hatayama.UnityCliLoop.Runtime;` in `execute-dynamic-code` snippets before calling `UloopPausePoint.TryGetCapturedValue` / `GetCapturedNames` / `GetCapturedPausePointId`. + While Unity is paused on a hit, `execute-dynamic-code` can read live captured references through `UloopPausePoint`: - `TryGetCapturedValue(string name)` returns `(bool Found, object Value)` for the latest hit only. When multiple captured variables share the same name, the last one wins. diff --git a/.claude/skills/uloop-pause-point/references/fast-progressing-games.md b/.claude/skills/uloop-pause-point/references/fast-progressing-games.md index 60017b8600..281998603c 100644 --- a/.claude/skills/uloop-pause-point/references/fast-progressing-games.md +++ b/.claude/skills/uloop-pause-point/references/fast-progressing-games.md @@ -20,6 +20,10 @@ uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seco Digit keys are `Digit0`-`Digit9` or `Numpad0`-`Numpad9` — bare `0`-`9` is rejected. +## Clear Before Scenario Setup + +`clear-pause-point --all` (and clearing the marker that owns the current pause) resumes Play Mode when the pause came from a pause-point hit. Run clear **before** arranging the board or other scenario setup; otherwise the resume lets the game consume your setup mid-flight. If you still need to build state after clear, re-freeze with `control-play-mode --action Pause` first, then arrange, then arm with `--resume-play`. + ## --resume-play Semantics `--resume-play` runs after the marker's arming is confirmed and before `--trigger` is dispatched: it resumes PlayMode only when PlayMode is actually paused, and reports what it did in `ResumePlayResult` (`WasPaused` / `Resumed` / `Error`; an abandoned wait adds `Repaused` / `RepauseError`). If the resume fails, the trigger is not dispatched and `TriggerResult.Error` says so. If the trigger itself is rejected before it runs, the wait is abandoned and the resume is undone: `Repaused: true` (or `RepauseError`) reports PlayMode being paused again, so gameplay cannot consume the preserved marker while the trigger value is being fixed. When the game reaches the line on its own after resuming (gravity, physics), omit `--trigger` and keep `--resume-play`. diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md index 178a352a16..12d557673e 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md @@ -24,7 +24,7 @@ When the game reaches the line on its own, omit `--trigger`. Fall back to split `--timeout-seconds` on enable starts the marker lifetime clock at enable time and is also the deadline `--await` waits against, so size it to cover both the trigger and the wait. -The response returns the derived marker `Id` (`Assets/Scripts/Enemy.cs:42`), the `ResolvedLine` that was actually patched, the `ResolvedMethod`, and `ResolvedLineText` — the actual source text at `ResolvedLine`. When the requested line has no executable statement, the pause point rounds forward to the next executable line — check `ResolvedLine`/`ResolvedLineText` when precision matters, and re-check them after every code edit — a rewritten file shifts line numbers. Use the returned `Id` for every follow-up command. On a hit, this same response already carries `CapturedVariables` and every other field `await-pause-point` would have returned — no separate `await-pause-point` call is needed. +The response returns the derived marker `Id` (`Assets/Scripts/Enemy.cs:42`), the `ResolvedLine` that was actually patched, the `ResolvedMethod`, and `ResolvedLineText` — the actual source text at `ResolvedLine`. When the requested line has no executable statement, the pause point rounds forward to the next executable line — check `ResolvedLine`/`ResolvedLineText` when precision matters, and re-check them after every code edit — a rewritten file shifts line numbers. Use the returned `Id` for every follow-up command. On a hit, this same response already carries `CapturedVariables` and every other field `await-pause-point` would have returned — no separate `await-pause-point` call is needed. `EditorState` on a hit response is a snapshot from the moment of the hit. After `--resume-play`, a successful await response can still show `EditorState.IsPaused: true` from that hit — it is not the Editor's current pause flag. Read the live state with `control-play-mode --action Status`. 3. Read `CapturedVariables` in the hit response first: the locals, parameters, and `this` instance fields at the paused line are already there (see Reading CapturedVariables). 4. While Unity is still paused, capture any additional evidence with `uloop execute-dynamic-code`, `uloop get-hierarchy`, `uloop find-game-objects`, and one screenshot. @@ -96,6 +96,7 @@ Choose the capture mode when enabling a pause point: - `single-shot` is the default. The first hit pauses Unity and disarms the marker. - `continuous` pauses Unity on every hit and remains armed. `CapturedVariables` holds the latest hit; `CapturedVariableHistory` holds only strictly older frames, so with a single hit the history is empty. - `trace` remains armed and records each hit without pausing Unity. +- For an already-hit `continuous` or `trace` marker, `await-pause-point` waits for a **new** hit after the wait starts (`LastHitSequence` advancing). It does not return the stale hit that is already present. Read the current hit with `pause-point-status` instead. If await times out while waiting for that new hit, the error stays `PAUSE_POINT_WAIT_TIMEOUT` and `Details.Hint` tells you to pass `--resume-play` (or resume Play Mode) so another hit can occur. A freshly enabled marker (including `enable-pause-point --await`) has no prior hit, so the first hit satisfies the wait as before. `--max-history` defaults to 20 and accepts values from 1 through 100. When the limit is exceeded, the oldest frames are dropped and `HistoryDroppedCount` reports how many were removed. `pause-point-status` returns the current `Mode`, `MaxHistory`, history frames, and dropped count. @@ -112,6 +113,14 @@ Every hit response embeds `CapturedVariables`: the method's in-scope locals, its - `--captured-variables names` on `await-pause-point`/`pause-point-status` drops every `Value` and keeps `Name`/`Scope`/`TypeName` — use it first on field-heavy classes, then fetch full values with a plain `pause-point-status` call. - When the response would be dominated by variables you do not need, pass `--captured-variable-names velocity,this` (comma-separated, exact match on `Name`) to keep only those entries; it composes with `--captured-variables full|names`. `CapturedVariablesTruncated` in the response reports truncation at Unity-side capture time and is unrelated to this name filter — it can be `true` even when every requested name was found. Requested names that matched nothing are listed in `CapturedVariableNamesNotFound`, so a partial match is visible without comparing the response against the request by hand. - Pass `--expect 'name=value'` (repeatable; on `await-pause-point`, `enable-pause-point --await`, and `pause-point-status`) to have the CLI compare captured variables against expected values; the response includes an `Expectations` array and `AllExpectationsPassed`, so you do not need to eyeball the JSON. Matching is string equality against the serialized value. On `pause-point-status` a marker that has not been hit yet reports each expectation as not found, and the verdict never changes the exit code — a polling loop reads `AllExpectationsPassed`, not the exit status. + Serialized `value` forms that match in practice (string equality against `CapturedVariables[].Value`): + - bool: `True` / `False` (C# form, capital first letter) + - float: `7` when the value is exactly an integer (not `7.0`) + - Vector2/3 and custom structs via `ToString()`: `(2.31, 6.61)` (one space after each comma) + - enum: `Grass` (member name only) + - `List` / arrays: `[19]`, `[0,1,2,3]` (numeric elements unquoted) + - `List` and other element-`ToString()` collections: `["(9, 3)","(9, 2)"]` (elements quoted) + When unsure, hit once and copy the `Value` string from `CapturedVariables` into `--expect` verbatim. - Collection values (arrays, `List`, dictionaries, plain objects) render as a JSON preview capped at 10 elements by default. When the elements you need sit past that cap (a 10x20 grid, a long list), re-enable with `--max-preview-elements ` (1–1000). The value set at enable time also caps the previews in every later `pause-point-status` response for that marker — status has no flag to change it. - While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; the return is a `(bool Found, object Value)` tuple, and the holder clears on resume. (file:line marker hits only — id-only markers store no capture) These are **live objects in their frame-completed state, not snapshots** — use them only to dig further into objects that are still alive, never to reconstruct what a value was at the paused line. @@ -189,6 +198,7 @@ For `ResumePlayResult` semantics, why `Time.timeScale = 0` is not a substitute f - Prefer natural runtime points after input has been consumed, such as after a command is accepted, a state value changes, an evaluation step resolves, or a dependent component is updated. - For frame-specific bugs, target the suspicious state branch or the line right after the mutation you need to freeze (the snapshot is taken before the target line runs). - A line that runs unconditionally every frame hits on the very next frame, before the input or event you actually wanted to observe arrives. If you need to catch a specific moment, choose a line that only executes conditionally (inside an `if` guarding the event you care about) so the pause point does not fire prematurely. The opposite applies to `continuous` mode paired with a watch expression: the watch only re-evaluates on a paused frame where the marker's line executes, so a conditional line that stops being reached leaves the watch value frozen (see Watch Expressions) — pick a line reached every frame when you need continuous per-Step updates. +- To verify held input (WASD and similar) against a line that runs every frame, do **not** use `--trigger`: call `simulate-keyboard --action KeyDown --key W` first so the key is already held, then arm the marker (optionally with `--await`). Reversing that order — arm then trigger KeyDown — races the every-frame hit before the hold is applied. Release with `KeyUp` or `ReleaseAll` when done. - When every reachable line around the state change you want runs unconditionally every frame, with no existing `if` to hang the pause point on, move the moment you want to observe into a conditional block: `if () { ; UnityEngine.Debug.Assert(); }`, then target the pause point at the assert line: it executes only when the event actually happens, states the mutation's postcondition, and can stay in the codebase after the investigation. Use `UnityEngine.Debug.Assert`, not `System.Diagnostics.Debug.Assert`: a failed System.Diagnostics assert never reaches the Unity Console, so `get-logs` cannot observe it. - An empty-body loop such as `while (TryMove(0, 1)) { }` has no statement inside the braces, so a pause point on the line right after the loop hits at the loop's condition re-check, not once the loop has actually finished advancing. If you need the state after the loop completes, target a line that is guaranteed to run exactly once after the loop exits, not the loop line itself. - Enable pause points after PlayMode is running: entering PlayMode with Domain Reload enabled silently removes every source pause point (`enable-pause-point` warns when this applies); with Domain Reload disabled this does not happen. diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md index 401c3e08e7..a4a794a8bd 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/captured-variables.md @@ -63,6 +63,8 @@ Capturing a deep copy at hit time was deliberately not adopted: it would cost ho ## Raw Capture API While Paused +Add `using io.github.hatayama.UnityCliLoop.Runtime;` in `execute-dynamic-code` snippets before calling `UloopPausePoint.TryGetCapturedValue` / `GetCapturedNames` / `GetCapturedPausePointId`. + While Unity is paused on a hit, `execute-dynamic-code` can read live captured references through `UloopPausePoint`: - `TryGetCapturedValue(string name)` returns `(bool Found, object Value)` for the latest hit only. When multiple captured variables share the same name, the last one wins. diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/fast-progressing-games.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/fast-progressing-games.md index 60017b8600..281998603c 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/fast-progressing-games.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/references/fast-progressing-games.md @@ -20,6 +20,10 @@ uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seco Digit keys are `Digit0`-`Digit9` or `Numpad0`-`Numpad9` — bare `0`-`9` is rejected. +## Clear Before Scenario Setup + +`clear-pause-point --all` (and clearing the marker that owns the current pause) resumes Play Mode when the pause came from a pause-point hit. Run clear **before** arranging the board or other scenario setup; otherwise the resume lets the game consume your setup mid-flight. If you still need to build state after clear, re-freeze with `control-play-mode --action Pause` first, then arrange, then arm with `--resume-play`. + ## --resume-play Semantics `--resume-play` runs after the marker's arming is confirmed and before `--trigger` is dispatched: it resumes PlayMode only when PlayMode is actually paused, and reports what it did in `ResumePlayResult` (`WasPaused` / `Resumed` / `Error`; an abandoned wait adds `Repaused` / `RepauseError`). If the resume fails, the trigger is not dispatched and `TriggerResult.Error` says so. If the trigger itself is rejected before it runs, the wait is abandoned and the resume is undone: `Repaused: true` (or `RepauseError`) reports PlayMode being paused again, so gameplay cannot consume the preserved marker while the trigger value is being fixed. When the game reaches the line on its own after resuming (gravity, physics), omit `--trigger` and keep `--resume-play`. From ccbf6089564bd11070becb6be3938ddf989937ec Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 29 Jul 2026 16:02:29 +0900 Subject: [PATCH 10/10] Raise dead-code PublicCandidate ceiling for CaptureMode.GameView GameView is a JSON/schema alias for rendering resolved via Enum.TryParse, so the scanner correctly reports it as an unre referenced public member. Co-authored-by: Cursor --- .github/workflows/dead-code.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/dead-code.yml b/.github/workflows/dead-code.yml index 723737ce3b..09738ce7a3 100644 --- a/.github/workflows/dead-code.yml +++ b/.github/workflows/dead-code.yml @@ -45,4 +45,6 @@ jobs: --include-kept false --format table --fail-on high-confidence - --max-public-candidates 23 + # 24: CaptureMode.GameView is a JSON/schema alias for rendering (Enum.TryParse); + # Roslyn sees no C# references by design. + --max-public-candidates 24