From 4dcbcdb5772f0fbdd3b1e7c2ea939aa60a99d368 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Fri, 24 Jul 2026 02:37:22 +0900 Subject: [PATCH 1/5] feat: add --resume-play to pause-point await flow (#1964) Co-authored-by: Cursor --- .../internal/projectrunner/list_output.go | 10 + .../projectrunner/list_output_test.go | 2 + .../projectrunner/native_command_help.go | 2 + .../projectrunner/native_command_help_test.go | 2 +- .../projectrunner/pause_point_enable.go | 71 ++- .../projectrunner/pause_point_enable_test.go | 12 +- .../projectrunner/pause_point_resume_play.go | 110 ++++ .../pause_point_resume_play_test.go | 507 ++++++++++++++++++ .../projectrunner/pause_point_trigger_test.go | 10 +- .../projectrunner/pause_point_types.go | 4 + .../projectrunner/pause_point_wait.go | 26 +- .../projectrunner/pause_point_wait_poll.go | 65 ++- .../projectrunner/pause_point_wait_test.go | 4 +- docs/regression-harness.md | 1 + ...gression-harness-resume-play-paused-arm.sh | 102 ++++ 15 files changed, 879 insertions(+), 49 deletions(-) create mode 100644 cli/project-runner/internal/projectrunner/pause_point_resume_play.go create mode 100644 cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go create mode 100755 scripts/regression-harness-resume-play-paused-arm.sh diff --git a/cli/project-runner/internal/projectrunner/list_output.go b/cli/project-runner/internal/projectrunner/list_output.go index 5a0665aea..12525a2fb 100644 --- a/cli/project-runner/internal/projectrunner/list_output.go +++ b/cli/project-runner/internal/projectrunner/list_output.go @@ -183,6 +183,16 @@ func appendPausePointEnableAwaitListOptions(tool clicore.ToolDefinition, options Type: "string", Description: "Requires --await. Same as await-pause-point's --expect (repeatable)", }, + listOption{ + Name: "--" + PausePointTriggerFlagName, + Type: "string", + Description: "Requires --await. Same as await-pause-point's --trigger", + }, + listOption{ + Name: "--" + PausePointResumePlayFlagName, + Type: "boolean", + Description: "Requires --await. After confirming the marker is armed, resume PlayMode if paused (before --trigger), so a paused-arm workflow can fire input in one call", + }, ) } diff --git a/cli/project-runner/internal/projectrunner/list_output_test.go b/cli/project-runner/internal/projectrunner/list_output_test.go index 82d8600a0..372f3fe09 100644 --- a/cli/project-runner/internal/projectrunner/list_output_test.go +++ b/cli/project-runner/internal/projectrunner/list_output_test.go @@ -147,6 +147,8 @@ func TestNewListCatalogIncludesEnablePausePointAwaitOptions(t *testing.T) { findListOption(t, enablePausePoint, "--"+PausePointCapturedVariablesFlagName) findListOption(t, enablePausePoint, "--"+PausePointCapturedVariableNamesFlagName) findListOption(t, enablePausePoint, "--"+PausePointExpectFlagName) + findListOption(t, enablePausePoint, "--"+PausePointTriggerFlagName) + findListOption(t, enablePausePoint, "--"+PausePointResumePlayFlagName) } func decodeListCatalog(t *testing.T, content []byte) listCatalog { diff --git a/cli/project-runner/internal/projectrunner/native_command_help.go b/cli/project-runner/internal/projectrunner/native_command_help.go index 9a3797065..fa058fb94 100644 --- a/cli/project-runner/internal/projectrunner/native_command_help.go +++ b/cli/project-runner/internal/projectrunner/native_command_help.go @@ -19,6 +19,7 @@ const ( PausePointCapturedVariableNamesFlagName = "captured-variable-names" PausePointExpectFlagName = "expect" PausePointTriggerFlagName = "trigger" + PausePointResumePlayFlagName = "resume-play" ) // runnerNativeCommandOptions lists the flags accepted by each runner-owned @@ -34,6 +35,7 @@ var runnerNativeCommandOptions = map[string][]string{ "--" + PausePointCapturedVariableNamesFlagName, "--" + PausePointExpectFlagName, "--" + PausePointTriggerFlagName, + "--" + PausePointResumePlayFlagName, }, clicore.PausePointStatusUserCommandName: { "--" + PausePointIDFlagName, diff --git a/cli/project-runner/internal/projectrunner/native_command_help_test.go b/cli/project-runner/internal/projectrunner/native_command_help_test.go index 85c57ab13..dcbe321e0 100644 --- a/cli/project-runner/internal/projectrunner/native_command_help_test.go +++ b/cli/project-runner/internal/projectrunner/native_command_help_test.go @@ -20,7 +20,7 @@ func TestRunProjectLocalAwaitPausePointHelpListsExpectedFlags(t *testing.T) { if code != 0 { t.Fatalf("await-pause-point --help failed: code=%d stderr=%s", code, stderr.String()) } - for _, flag := range []string{"--id", "--timeout-seconds", "--matching-logs-max-count", "--captured-variables", "--expect"} { + for _, flag := range []string{"--id", "--timeout-seconds", "--matching-logs-max-count", "--captured-variables", "--expect", "--trigger", "--resume-play"} { if !strings.Contains(stdout.String(), flag) { t.Fatalf("await-pause-point --help must list %s: %s", flag, stdout.String()) } diff --git a/cli/project-runner/internal/projectrunner/pause_point_enable.go b/cli/project-runner/internal/projectrunner/pause_point_enable.go index 20cd0ceb2..3ea77d607 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable.go @@ -25,11 +25,12 @@ const pausePointEnableCommandName = "enable-pause-point" const pausePointEnableAwaitFlagName = "await" // extractPausePointEnableAwaitFlags pulls the CLI-only --await/--captured-variables/ -// --captured-variable-names/--expect/--trigger flags out of enable-pause-point args before -// generic schema parsing, because none of them are part of the Unity-side EnablePausePointSchema. +// --captured-variable-names/--expect/--trigger/--resume-play flags out of enable-pause-point args +// before generic schema parsing, because none of them are part of the Unity-side +// EnablePausePointSchema. func extractPausePointEnableAwaitFlags( args []string, -) ([]string, bool, pausePointCapturedVariablesMode, []string, []pausePointExpectation, string, []string, error) { +) ([]string, bool, pausePointCapturedVariablesMode, []string, []pausePointExpectation, string, []string, bool, error) { remaining := make([]string, 0, len(args)) await := false mode := pausePointCapturedVariablesModeFull @@ -40,6 +41,7 @@ func extractPausePointEnableAwaitFlags( var triggerCommand string var triggerArgs []string triggerSet := false + resumePlay := false for index := 0; index < len(args); index++ { arg := args[index] @@ -49,10 +51,37 @@ func extractPausePointEnableAwaitFlags( continue } + if arg == "--"+PausePointResumePlayFlagName { + resumePlay = true + continue + } + + // --resume-play=true|1 must be accepted here too: otherwise the =value form falls through + // to Unity schema parsing and becomes a confusing unrelated error. + if isPausePointFlag(arg, PausePointResumePlayFlagName) { + name, value, consumedNext, err := clicore.ParseFlagValue(arg, args, index) + if err != nil { + return nil, false, mode, nil, nil, "", nil, false, err + } + if name != PausePointResumePlayFlagName { + remaining = append(remaining, arg) + continue + } + if value != "true" && value != "1" { + return nil, false, mode, nil, nil, "", nil, false, clierrors.InvalidValueArgumentError( + "--"+PausePointResumePlayFlagName, value, "boolean flag (pass with no value, or =true)") + } + resumePlay = true + if consumedNext { + index++ + } + continue + } + if isPausePointFlag(arg, PausePointTriggerFlagName) { name, value, consumedNext, err := clicore.ParseFlagValue(arg, args, index) if err != nil { - return nil, false, mode, nil, nil, "", nil, err + return nil, false, mode, nil, nil, "", nil, false, err } if name != PausePointTriggerFlagName { remaining = append(remaining, arg) @@ -60,7 +89,7 @@ func extractPausePointEnableAwaitFlags( } parsedCommand, parsedArgs, parseErr := parsePausePointTriggerCommand(pausePointEnableCommandName, value) if parseErr != nil { - return nil, false, mode, nil, nil, "", nil, parseErr + return nil, false, mode, nil, nil, "", nil, false, parseErr } triggerCommand = parsedCommand triggerArgs = parsedArgs @@ -74,7 +103,7 @@ func extractPausePointEnableAwaitFlags( if isPausePointFlag(arg, PausePointCapturedVariablesFlagName) { name, value, consumedNext, err := clicore.ParseFlagValue(arg, args, index) if err != nil { - return nil, false, mode, nil, nil, "", nil, err + return nil, false, mode, nil, nil, "", nil, false, err } if name != PausePointCapturedVariablesFlagName { remaining = append(remaining, arg) @@ -82,7 +111,7 @@ func extractPausePointEnableAwaitFlags( } parsedMode, err := parsePausePointCapturedVariablesMode(value) if err != nil { - return nil, false, mode, nil, nil, "", nil, err + return nil, false, mode, nil, nil, "", nil, false, err } mode = parsedMode modeSet = true @@ -95,7 +124,7 @@ func extractPausePointEnableAwaitFlags( if isPausePointFlag(arg, PausePointCapturedVariableNamesFlagName) { name, value, consumedNext, err := clicore.ParseFlagValue(arg, args, index) if err != nil { - return nil, false, mode, nil, nil, "", nil, err + return nil, false, mode, nil, nil, "", nil, false, err } if name != PausePointCapturedVariableNamesFlagName { remaining = append(remaining, arg) @@ -112,7 +141,7 @@ func extractPausePointEnableAwaitFlags( if isPausePointFlag(arg, PausePointExpectFlagName) { name, value, consumedNext, err := clicore.ParseFlagValue(arg, args, index) if err != nil { - return nil, false, mode, nil, nil, "", nil, err + return nil, false, mode, nil, nil, "", nil, false, err } if name != PausePointExpectFlagName { remaining = append(remaining, arg) @@ -120,7 +149,7 @@ func extractPausePointEnableAwaitFlags( } expectation, parseErr := parsePausePointExpectFlagValue(value) if parseErr != nil { - return nil, false, mode, nil, nil, "", nil, parseErr + return nil, false, mode, nil, nil, "", nil, false, parseErr } expectations = append(expectations, expectation) if consumedNext { @@ -132,9 +161,11 @@ func extractPausePointEnableAwaitFlags( remaining = append(remaining, arg) } - if !await && (modeSet || namesSet || len(expectations) > 0 || triggerSet) { + if !await && (modeSet || namesSet || len(expectations) > 0 || triggerSet || resumePlay) { option := "--" + PausePointCapturedVariablesFlagName switch { + case resumePlay: + option = "--" + PausePointResumePlayFlagName case triggerSet: option = "--" + PausePointTriggerFlagName case len(expectations) > 0: @@ -142,8 +173,8 @@ func extractPausePointEnableAwaitFlags( case namesSet: option = "--" + PausePointCapturedVariableNamesFlagName } - return nil, false, mode, nil, nil, "", nil, &clierrors.ArgumentError{ - Message: "--captured-variables, --captured-variable-names, --expect, and --trigger require --await", + return nil, false, mode, nil, nil, "", nil, false, &clierrors.ArgumentError{ + Message: "--captured-variables, --captured-variable-names, --expect, --trigger, and --resume-play require --await", Option: option, Command: pausePointEnableCommandName, NextActions: []string{ @@ -152,7 +183,7 @@ func extractPausePointEnableAwaitFlags( } } - return remaining, await, mode, capturedVariableNames, expectations, triggerCommand, triggerArgs, nil + return remaining, await, mode, capturedVariableNames, expectations, triggerCommand, triggerArgs, resumePlay, nil } func isPausePointFlag(arg string, flagName string) bool { @@ -167,7 +198,7 @@ func runEnablePausePointCommand( stdout io.Writer, stderr io.Writer, ) int { - remainingArgs, await, capturedVariablesMode, capturedVariableNames, expectations, triggerCommand, triggerArgs, err := extractPausePointEnableAwaitFlags(args) + remainingArgs, await, capturedVariablesMode, capturedVariableNames, expectations, triggerCommand, triggerArgs, resumePlay, err := extractPausePointEnableAwaitFlags(args) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ ProjectRoot: connection.ProjectRoot, @@ -220,7 +251,7 @@ func runEnablePausePointCommand( return runEnablePausePointAndAwait( ctx, connection, params, capturedVariablesMode, capturedVariableNames, expectations, - triggerCommand, triggerArgs, startPath, stdout, stderr) + triggerCommand, triggerArgs, resumePlay, startPath, stdout, stderr) } // runEnablePausePointAndAwait sends the same single enable-pause-point IPC request the @@ -236,6 +267,7 @@ func runEnablePausePointAndAwait( expectations []pausePointExpectation, triggerCommand string, triggerArgs []string, + resumePlay bool, startPath string, stdout io.Writer, stderr io.Writer, @@ -288,6 +320,7 @@ func runEnablePausePointAndAwait( triggerCommand: triggerCommand, triggerArgs: triggerArgs, startPath: startPath, + resumePlay: resumePlay, } return runPausePointWaitAfterEnable(ctx, connection, waitOptions, enableResponse.Warning, stdout, stderr) @@ -306,7 +339,7 @@ func runPausePointWaitAfterEnable( stderr io.Writer, ) int { spinner := clicore.NewToolSpinner(stderr, pausePointEnableCommandName) - response, state, triggerResult, err := waitForPausePoint(ctx, connection, options) + response, state, triggerResult, resumeResult, err := waitForPausePoint(ctx, connection, options) spinner.Stop() if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ @@ -322,6 +355,7 @@ func runPausePointWaitAfterEnable( expectations := evaluatePausePointExpectations(response.CapturedVariables, options.expectations) response.TriggerResult = triggerResult + response.ResumePlayResult = resumeResult response = filterPausePointCapturedVariableHistory(response) response = filterPausePointCapturedVariablesByName(response, options.capturedVariableNames) response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) @@ -379,6 +413,9 @@ func runPausePointWaitAfterEnable( if triggerResult != nil { waitErr.Details["TriggerResult"] = triggerResult } + if resumeResult != nil { + waitErr.Details["ResumePlayResult"] = resumeResult + } if state == pausePointWaitStateTimeout { logs, logsErr := fetchMatchingLogs(ctx, connection, options.id, options.matchingLogsMaxCount) if logsErr == nil { 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 e91003bbe..637b67882 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_enable_test.go +++ b/cli/project-runner/internal/projectrunner/pause_point_enable_test.go @@ -16,7 +16,7 @@ import ( // Verifies --await is extracted and the remaining args are left untouched for schema parsing. func TestExtractPausePointEnableAwaitFlagsExtractsAwait(t *testing.T) { - remaining, await, mode, names, expectations, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--await"}) + remaining, await, mode, names, expectations, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--await"}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -39,7 +39,7 @@ func TestExtractPausePointEnableAwaitFlagsExtractsAwait(t *testing.T) { // Verifies --captured-variables/--captured-variable-names are extracted alongside --await. func TestExtractPausePointEnableAwaitFlagsExtractsCapturedVariableOptions(t *testing.T) { - remaining, await, mode, names, _, _, _, err := extractPausePointEnableAwaitFlags([]string{ + remaining, await, mode, names, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{ "--id", "jump", "--await", "--captured-variables", "names", "--captured-variable-names", "a,b", }) if err != nil { @@ -61,7 +61,7 @@ func TestExtractPausePointEnableAwaitFlagsExtractsCapturedVariableOptions(t *tes // Verifies --expect is extracted (repeatably) alongside --await, and unrelated args are untouched. func TestExtractPausePointEnableAwaitFlagsExtractsExpect(t *testing.T) { - remaining, await, _, _, expectations, _, _, err := extractPausePointEnableAwaitFlags([]string{ + remaining, await, _, _, expectations, _, _, _, err := extractPausePointEnableAwaitFlags([]string{ "--id", "jump", "--await", "--expect", "Health=100", "--expect", "Name=Enemy", }) if err != nil { @@ -86,7 +86,7 @@ func TestExtractPausePointEnableAwaitFlagsExtractsExpect(t *testing.T) { // Verifies --captured-variables without --await is rejected, since it has no effect otherwise. func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForCapturedVariables(t *testing.T) { - _, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--captured-variables", "names"}) + _, _, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--captured-variables", "names"}) if err == nil { t.Fatalf("expected an error") } @@ -97,7 +97,7 @@ func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForCapturedVariables(t *t // Verifies --expect without --await is rejected, since it has no effect otherwise. func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForExpect(t *testing.T) { - _, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--expect", "Health=100"}) + _, _, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--id", "jump", "--expect", "Health=100"}) if err == nil { t.Fatalf("expected an error") } @@ -108,7 +108,7 @@ func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForExpect(t *testing.T) { // Verifies enable-pause-point without --await leaves File/Line/Id/Mode args untouched. func TestExtractPausePointEnableAwaitFlagsWithoutAwaitLeavesArgsUnchanged(t *testing.T) { - remaining, await, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--file", "Assets/Foo.cs", "--line", "10"}) + remaining, await, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{"--file", "Assets/Foo.cs", "--line", "10"}) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/cli/project-runner/internal/projectrunner/pause_point_resume_play.go b/cli/project-runner/internal/projectrunner/pause_point_resume_play.go new file mode 100644 index 000000000..08c38e88e --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_resume_play.go @@ -0,0 +1,110 @@ +package projectrunner + +import ( + "context" + "encoding/json" + "fmt" + "time" + + "github.com/hatayama/unity-cli-loop/common/clicontract" + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +const ( + pausePointResumePlayCommandName = "control-play-mode" + pausePointResumePlayTimeout = 10 * time.Second +) + +// pausePointResumePlayResult reports what --resume-play did (or why it skipped). Set only when +// --resume-play was passed, mirroring TriggerResult's omit-when-unused contract. +type pausePointResumePlayResult struct { + WasPaused bool `json:"WasPaused"` + Resumed bool `json:"Resumed"` + Error string `json:"Error,omitempty"` +} + +type controlPlayModeToolResponse struct { + Success bool `json:"Success"` + IsPaused bool `json:"IsPaused"` + Message string `json:"Message"` +} + +// resumePlayModeForPausePoint is overridden in tests so waitForPausePoint can assert resume/ +// trigger ordering without a live Unity connection. +var resumePlayModeForPausePoint = resumePlayModeForPausePointFromUnity + +// sendControlPlayModeForPausePointResume is overridden in tests so +// resumePlayModeForPausePointFromUnity's Status/Play branches can be exercised without IPC. +var sendControlPlayModeForPausePointResume = sendControlPlayModeForPausePointResumeFromUnity + +// resumePlayModeForPausePointFromUnity asks control-play-mode for Status, then sends Play only when +// the Editor is currently paused. A Status or Play failure is returned as Error so the wait path +// can skip --trigger without inventing a success. +func resumePlayModeForPausePointFromUnity( + ctx context.Context, + connection unityipc.Connection, +) pausePointResumePlayResult { + resumeContext, cancel := context.WithTimeout(ctx, pausePointResumePlayTimeout) + defer cancel() + + statusResponse, err := sendControlPlayModeForPausePointResume(resumeContext, connection, "Status") + if err != nil { + return pausePointResumePlayResult{ + Error: fmt.Sprintf("control-play-mode Status failed: %v", err), + } + } + if !statusResponse.Success { + message := statusResponse.Message + if message == "" { + message = "control-play-mode Status returned Success=false" + } + return pausePointResumePlayResult{Error: message} + } + + if !statusResponse.IsPaused { + return pausePointResumePlayResult{WasPaused: false, Resumed: false} + } + + playResponse, err := sendControlPlayModeForPausePointResume(resumeContext, connection, "Play") + if err != nil { + return pausePointResumePlayResult{ + WasPaused: true, + Resumed: false, + Error: fmt.Sprintf("control-play-mode Play failed: %v", err), + } + } + if !playResponse.Success { + message := playResponse.Message + if message == "" { + message = "control-play-mode Play returned Success=false" + } + return pausePointResumePlayResult{ + WasPaused: true, + Resumed: false, + Error: message, + } + } + + return pausePointResumePlayResult{WasPaused: true, Resumed: true} +} + +func sendControlPlayModeForPausePointResumeFromUnity( + ctx context.Context, + connection unityipc.Connection, + action string, +) (controlPlayModeToolResponse, error) { + result, err := unityipc.NewClient(connection, clicontract.ProjectRunnerVersion()).Send( + ctx, + pausePointResumePlayCommandName, + map[string]any{"Action": action}, + ) + if err != nil { + return controlPlayModeToolResponse{}, err + } + + response := controlPlayModeToolResponse{} + if err := json.Unmarshal(result, &response); err != nil { + return controlPlayModeToolResponse{}, err + } + return response, nil +} 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 new file mode 100644 index 000000000..26c5faa65 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/pause_point_resume_play_test.go @@ -0,0 +1,507 @@ +package projectrunner + +import ( + "context" + "fmt" + "io" + "strings" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// Verifies --resume-play without --await is rejected with the same require-await family of +// errors as --trigger, so the flag cannot silently do nothing on a plain enable call. +func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForResumePlay(t *testing.T) { + _, _, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{ + "--id", "jump", "--resume-play", + }) + if err == nil { + t.Fatalf("expected an error") + } + if !strings.Contains(err.Error(), "require --await") { + t.Fatalf("error message mismatch: %v", err) + } + if !strings.Contains(err.Error(), "--resume-play") { + t.Fatalf("error message must mention --resume-play: %v", err) + } +} + +// Verifies --await --resume-play extracts resumePlay=true and leaves Unity-side args untouched. +func TestExtractPausePointEnableAwaitFlagsExtractsResumePlay(t *testing.T) { + remaining, await, _, _, _, _, _, resumePlay, err := extractPausePointEnableAwaitFlags([]string{ + "--id", "jump", "--await", "--resume-play", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !await { + t.Fatalf("expected await to be true") + } + if !resumePlay { + t.Fatalf("expected resumePlay to be true") + } + if len(remaining) != 2 || remaining[0] != "--id" || remaining[1] != "jump" { + t.Fatalf("remaining args mismatch: %#v", remaining) + } +} + +// Verifies enable-pause-point accepts --resume-play=true the same way await-pause-point does, +// so the =value form is not leaked into Unity schema parsing. +func TestExtractPausePointEnableAwaitFlagsExtractsResumePlayEqualsTrue(t *testing.T) { + _, await, _, _, _, _, _, resumePlay, err := extractPausePointEnableAwaitFlags([]string{ + "--id", "jump", "--await", "--resume-play=true", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !await || !resumePlay { + t.Fatalf("expected await and resumePlay true, got await=%v resumePlay=%v", await, resumePlay) + } +} + +// Verifies await-pause-point accepts --resume-play as a boolean flag. +func TestParseWaitForPausePointOptionsParsesResumePlayFlag(t *testing.T) { + options, err := parseWaitForPausePointOptions([]string{ + "--id", "jump", "--resume-play", + }) + if err != nil { + t.Fatalf("parse failed: %v", err) + } + if !options.resumePlay { + t.Fatalf("expected resumePlay to be true: %#v", options) + } + + optionsEquals, err := parseWaitForPausePointOptions([]string{ + "--id", "jump", "--resume-play=true", + }) + if err != nil { + t.Fatalf("parse --resume-play=true failed: %v", err) + } + if !optionsEquals.resumePlay { + t.Fatalf("expected resumePlay to be true for =true form: %#v", optionsEquals) + } +} + +// Verifies armed + IsPaused=true resumes Play before dispatching --trigger, and reports +// ResumePlayResult{WasPaused:true, Resumed:true}. +func TestWaitForPausePointResumesPlayBeforeTriggerWhenPaused(t *testing.T) { + originalQuery := queryPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + originalResume := resumePlayModeForPausePoint + defer func() { + queryPausePointStatus = originalQuery + dispatchPausePointTriggerCommand = originalDispatch + resumePlayModeForPausePoint = originalResume + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 1, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + + calls := make([]string, 0, 2) + resumePlayModeForPausePoint = func( + ctx context.Context, + connection unityipc.Connection, + ) pausePointResumePlayResult { + calls = append(calls, "resume") + return pausePointResumePlayResult{WasPaused: true, Resumed: true} + } + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + calls = append(calls, "trigger") + _, _ = stdout.Write([]byte(`{"Success":true}`)) + return 0 + } + + _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press"}, + resumePlay: true, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if len(calls) != 2 || calls[0] != "resume" || calls[1] != "trigger" { + t.Fatalf("expected resume then trigger, got %#v", calls) + } + if resumeResult == nil || !resumeResult.WasPaused || !resumeResult.Resumed || resumeResult.Error != "" { + t.Fatalf("ResumePlayResult mismatch: %#v", resumeResult) + } + if triggerResult == nil || !triggerResult.Completed { + t.Fatalf("expected a completed trigger result, got %#v", triggerResult) + } +} + +// Verifies armed + IsPaused=false skips the Play request, reports WasPaused=false/Resumed=false, +// and still dispatches the trigger. +func TestWaitForPausePointSkipsPlayWhenAlreadyUnpaused(t *testing.T) { + originalQuery := queryPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + originalResume := resumePlayModeForPausePoint + defer func() { + queryPausePointStatus = originalQuery + dispatchPausePointTriggerCommand = originalDispatch + resumePlayModeForPausePoint = originalResume + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 1, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: false, CapturedAt: "PausePointHit"}, + }, nil + } + + triggerDispatched := false + resumePlayModeForPausePoint = func( + ctx context.Context, + connection unityipc.Connection, + ) pausePointResumePlayResult { + // The real implementation only sends Play when Status reports IsPaused=true; this stub + // mirrors that contract so the test asserts the wait path still accepts a no-op resume. + return pausePointResumePlayResult{WasPaused: false, Resumed: false} + } + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + triggerDispatched = true + _, _ = stdout.Write([]byte(`{"Success":true}`)) + return 0 + } + + _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press"}, + resumePlay: true, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if resumeResult == nil || resumeResult.WasPaused || resumeResult.Resumed { + t.Fatalf("expected a no-op ResumePlayResult, got %#v", resumeResult) + } + if !triggerDispatched { + t.Fatal("expected trigger to still be dispatched after a no-op resume") + } + if triggerResult == nil || !triggerResult.Completed { + t.Fatalf("expected a completed trigger result, got %#v", triggerResult) + } +} + +// Verifies a failed Status/Play resume skips --trigger and records the fixed skip Error on +// TriggerResult while still returning ResumePlayResult.Error. +func TestWaitForPausePointSkipsTriggerWhenResumePlayFails(t *testing.T) { + originalQuery := queryPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + originalResume := resumePlayModeForPausePoint + defer func() { + queryPausePointStatus = originalQuery + dispatchPausePointTriggerCommand = originalDispatch + resumePlayModeForPausePoint = originalResume + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 1, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + + dispatchCalled := false + resumePlayModeForPausePoint = func( + ctx context.Context, + connection unityipc.Connection, + ) pausePointResumePlayResult { + return pausePointResumePlayResult{ + WasPaused: true, + Resumed: false, + Error: "control-play-mode Status failed: boom", + } + } + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + dispatchCalled = true + _, _ = stdout.Write([]byte(`{"Success":true}`)) + return 0 + } + + _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press"}, + resumePlay: true, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if dispatchCalled { + t.Fatal("expected trigger not to be dispatched after a resume failure") + } + if resumeResult == nil || resumeResult.Error == "" { + t.Fatalf("expected ResumePlayResult.Error, got %#v", resumeResult) + } + if triggerResult == nil || triggerResult.Completed { + t.Fatalf("expected a skipped TriggerResult, got %#v", triggerResult) + } + if triggerResult.Error != "trigger was not dispatched: --resume-play failed to resume play mode" { + t.Fatalf("TriggerResult.Error mismatch: %#v", triggerResult) + } +} + +// Verifies --resume-play alone still confirms arm before resuming, and skips resume (and has no +// trigger) when the marker is not armed — matching the --trigger not-armed skip path. +func TestWaitForPausePointSkipsResumeWhenNotArmed(t *testing.T) { + originalQuery := queryPausePointStatus + originalResume := resumePlayModeForPausePoint + defer func() { + queryPausePointStatus = originalQuery + resumePlayModeForPausePoint = originalResume + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{Id: id, Status: pausePointStatusNotEnabled}, nil + } + + resumeCalled := false + resumePlayModeForPausePoint = func( + ctx context.Context, + connection unityipc.Connection, + ) pausePointResumePlayResult { + resumeCalled = true + return pausePointResumePlayResult{WasPaused: true, Resumed: true} + } + + _, state, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "does-not-exist", + timeoutSeconds: 1, + timeout: time.Second, + resumePlay: true, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateNotEnabled { + t.Fatalf("expected not_enabled state, got %q", state) + } + if resumeCalled { + t.Fatal("expected resumePlayModeForPausePoint not to be called for a not-armed marker") + } + if triggerResult != nil { + t.Fatalf("expected nil TriggerResult when --trigger was not given, got %#v", triggerResult) + } + if resumeResult == nil || resumeResult.Error == "" { + t.Fatalf("expected ResumePlayResult.Error explaining the skip, got %#v", resumeResult) + } +} + +// Verifies armed + --resume-play alone (no --trigger) still resumes and leaves TriggerResult nil. +func TestWaitForPausePointResumesWithoutTriggerWhenArmed(t *testing.T) { + originalQuery := queryPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + originalResume := resumePlayModeForPausePoint + defer func() { + queryPausePointStatus = originalQuery + dispatchPausePointTriggerCommand = originalDispatch + resumePlayModeForPausePoint = originalResume + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{ + Id: id, + Status: pausePointStatusHit, + IsHit: true, + HitCount: 1, + EditorState: pausePointEditorState{IsPlaying: true, IsPaused: true, CapturedAt: "PausePointHit"}, + }, nil + } + + resumeCalled := false + resumePlayModeForPausePoint = func( + ctx context.Context, + connection unityipc.Connection, + ) pausePointResumePlayResult { + resumeCalled = true + return pausePointResumePlayResult{WasPaused: true, Resumed: true} + } + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + t.Fatal("dispatchPausePointTriggerCommand must not be called without --trigger") + return 0 + } + + _, _, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "jump", + timeoutSeconds: 1, + timeout: time.Second, + resumePlay: true, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if !resumeCalled { + t.Fatal("expected resumePlayModeForPausePoint to be called") + } + if resumeResult == nil || !resumeResult.WasPaused || !resumeResult.Resumed { + t.Fatalf("ResumePlayResult mismatch: %#v", resumeResult) + } + if triggerResult != nil { + t.Fatalf("expected nil TriggerResult when --trigger was not given, got %#v", triggerResult) + } +} + +// Verifies resumePlayModeForPausePointFromUnity's Status/Play branches without a live Unity IPC. +func TestResumePlayModeForPausePointFromUnityBranches(t *testing.T) { + originalSend := sendControlPlayModeForPausePointResume + defer func() { sendControlPlayModeForPausePointResume = originalSend }() + + t.Run("Status transport failure", func(t *testing.T) { + actions := make([]string, 0, 1) + sendControlPlayModeForPausePointResume = func( + ctx context.Context, + connection unityipc.Connection, + action string, + ) (controlPlayModeToolResponse, error) { + actions = append(actions, action) + return controlPlayModeToolResponse{}, fmt.Errorf("boom") + } + + result := resumePlayModeForPausePointFromUnity(context.Background(), unityipc.Connection{}) + if result.WasPaused || result.Resumed || !strings.Contains(result.Error, "control-play-mode Status failed") { + t.Fatalf("result mismatch: %#v", result) + } + if len(actions) != 1 || actions[0] != "Status" { + t.Fatalf("expected only Status, got %#v", actions) + } + }) + + t.Run("Status Success=false", func(t *testing.T) { + actions := make([]string, 0, 1) + sendControlPlayModeForPausePointResume = func( + ctx context.Context, + connection unityipc.Connection, + action string, + ) (controlPlayModeToolResponse, error) { + actions = append(actions, action) + return controlPlayModeToolResponse{Success: false, Message: "status denied"}, nil + } + + result := resumePlayModeForPausePointFromUnity(context.Background(), unityipc.Connection{}) + if result.WasPaused || result.Resumed || result.Error != "status denied" { + t.Fatalf("result mismatch: %#v", result) + } + if len(actions) != 1 || actions[0] != "Status" { + t.Fatalf("expected only Status, got %#v", actions) + } + }) + + t.Run("IsPaused=false skips Play", func(t *testing.T) { + actions := make([]string, 0, 1) + sendControlPlayModeForPausePointResume = func( + ctx context.Context, + connection unityipc.Connection, + action string, + ) (controlPlayModeToolResponse, error) { + actions = append(actions, action) + return controlPlayModeToolResponse{Success: true, IsPaused: false}, nil + } + + result := resumePlayModeForPausePointFromUnity(context.Background(), unityipc.Connection{}) + if result.WasPaused || result.Resumed || result.Error != "" { + t.Fatalf("result mismatch: %#v", result) + } + if len(actions) != 1 || actions[0] != "Status" { + t.Fatalf("expected only Status, got %#v", actions) + } + }) + + t.Run("Play transport failure", func(t *testing.T) { + actions := make([]string, 0, 2) + sendControlPlayModeForPausePointResume = func( + ctx context.Context, + connection unityipc.Connection, + action string, + ) (controlPlayModeToolResponse, error) { + actions = append(actions, action) + if action == "Status" { + return controlPlayModeToolResponse{Success: true, IsPaused: true}, nil + } + return controlPlayModeToolResponse{}, fmt.Errorf("play boom") + } + + result := resumePlayModeForPausePointFromUnity(context.Background(), unityipc.Connection{}) + if !result.WasPaused || result.Resumed || !strings.Contains(result.Error, "control-play-mode Play failed") { + t.Fatalf("result mismatch: %#v", result) + } + if len(actions) != 2 || actions[0] != "Status" || actions[1] != "Play" { + t.Fatalf("expected Status then Play, got %#v", actions) + } + }) +} 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 3c8c3f125..0a6e48257 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, @@ -497,7 +497,7 @@ func TestParseWaitForPausePointOptionsParsesTriggerFlag(t *testing.T) { // Verifies enable-pause-point extracts --trigger alongside --await, and rejects --trigger without // --await since it would otherwise have no effect. func TestExtractPausePointEnableAwaitFlagsExtractsTrigger(t *testing.T) { - remaining, await, _, _, _, triggerCommand, triggerArgs, err := extractPausePointEnableAwaitFlags([]string{ + remaining, await, _, _, _, triggerCommand, triggerArgs, _, err := extractPausePointEnableAwaitFlags([]string{ "--id", "jump", "--await", "--trigger", "simulate-keyboard --action Press --key Space --duration 5", }) if err != nil { @@ -525,7 +525,7 @@ func TestExtractPausePointEnableAwaitFlagsExtractsTrigger(t *testing.T) { // Verifies --trigger without --await is rejected, since it has no effect otherwise. func TestExtractPausePointEnableAwaitFlagsRequiresAwaitForTrigger(t *testing.T) { - _, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{ + _, _, _, _, _, _, _, _, err := extractPausePointEnableAwaitFlags([]string{ "--id", "jump", "--trigger", "simulate-keyboard --action Press", }) if err == nil { diff --git a/cli/project-runner/internal/projectrunner/pause_point_types.go b/cli/project-runner/internal/projectrunner/pause_point_types.go index b6aba9e32..f83953a1c 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_types.go +++ b/cli/project-runner/internal/projectrunner/pause_point_types.go @@ -45,6 +45,10 @@ type pausePointStatusResponse struct { // TriggerResult is set by the CLI, not Unity, only when --trigger was passed. It is omitted // entirely otherwise, so callers that never use --trigger see no schema change at all. TriggerResult *pausePointTriggerResult `json:"TriggerResult,omitempty"` + + // ResumePlayResult is set by the CLI, not Unity, only when --resume-play was passed. It is + // omitted entirely otherwise, matching TriggerResult's omit-when-unused contract. + ResumePlayResult *pausePointResumePlayResult `json:"ResumePlayResult,omitempty"` } type pausePointEditorState struct { diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait.go b/cli/project-runner/internal/projectrunner/pause_point_wait.go index 685f4cd9d..e1f055ec7 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait.go @@ -49,6 +49,11 @@ type waitForPausePointOptions struct { triggerCommand string triggerArgs []string startPath string + + // resumePlay comes from --resume-play: after the marker is confirmed armed, resume PlayMode + // (if paused) before dispatching --trigger so a paused-arm workflow can fire input triggers + // in one CLI call. + resumePlay bool } type pausePointStatusOptions struct { @@ -190,7 +195,7 @@ func runWaitForPausePoint( ) int { startedAt := time.Now() spinner := clicore.NewToolSpinner(stderr, clicore.PausePointAwaitCommandName) - response, state, triggerResult, err := waitForPausePoint(ctx, connection, options) + response, state, triggerResult, resumeResult, err := waitForPausePoint(ctx, connection, options) spinner.Stop() if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ @@ -207,6 +212,7 @@ func runWaitForPausePoint( expectations := evaluatePausePointExpectations(response.CapturedVariables, options.expectations) response.TriggerResult = triggerResult + response.ResumePlayResult = resumeResult response = filterPausePointCapturedVariableHistory(response) response = filterPausePointCapturedVariablesByName(response, options.capturedVariableNames) response = applyPausePointCapturedVariablesMode(response, options.capturedVariablesMode) @@ -261,6 +267,9 @@ func runWaitForPausePoint( if triggerResult != nil { waitErr.Details["TriggerResult"] = triggerResult } + if resumeResult != nil { + waitErr.Details["ResumePlayResult"] = resumeResult + } if state == pausePointWaitStateTimeout { // Best-effort: the timeout diagnosis must not depend on a second Unity round trip succeeding. logs, logsErr := fetchMatchingLogs(ctx, connection, options.id, options.matchingLogsMaxCount) @@ -286,6 +295,13 @@ func parseWaitForPausePointOptions(args []string) (waitForPausePointOptions, err for index := 0; index < len(args); index++ { arg := args[index] + + // --resume-play is a boolean flag (no value). ParseFlagValue would otherwise demand one. + if arg == "--"+PausePointResumePlayFlagName { + options.resumePlay = true + continue + } + name, value, consumedNext, err := clicore.ParseFlagValue(arg, args, index) if err != nil { return waitForPausePointOptions{}, err @@ -329,6 +345,14 @@ func parseWaitForPausePointOptions(args []string) (waitForPausePointOptions, err } options.triggerCommand = triggerCommand options.triggerArgs = triggerArgs + case PausePointResumePlayFlagName: + // --resume-play=true style is accepted; any other value is rejected so a typo cannot + // silently disable the resume step. + if value != "true" && value != "1" { + return waitForPausePointOptions{}, clierrors.InvalidValueArgumentError( + "--"+PausePointResumePlayFlagName, value, "boolean flag (pass with no value)") + } + options.resumePlay = true default: return waitForPausePointOptions{}, pausePointUnknownOptionError(clicore.PausePointAwaitCommandName, name) } 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 288e601d0..f00e8b4ab 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go @@ -23,38 +23,69 @@ const ( ) // waitForPausePoint confirms the marker is actually armed with one status query before starting -// the --trigger command: extendPausePointExpiryBeforeWait (the await-pause-point entry point) is -// best-effort and does not confirm arm, so a typo'd or already-expired --id must not still get the -// trigger dispatched into the running game before the wait immediately fails on it. Once confirmed -// armed (or already hit), 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. +// --resume-play / --trigger: extendPausePointExpiryBeforeWait (the await-pause-point entry point) +// is best-effort and does not confirm arm, so a typo'd or already-expired --id must not still get +// Play resumed or the trigger dispatched into the running game before the wait immediately fails +// 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. func waitForPausePoint( ctx context.Context, connection unityipc.Connection, options waitForPausePointOptions, -) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, error) { +) (pausePointStatusResponse, pausePointWaitState, *pausePointTriggerResult, *pausePointResumePlayResult, error) { var triggerHandle *pausePointTriggerHandle var skippedTriggerResult *pausePointTriggerResult - if options.triggerCommand != "" { + var resumeResult *pausePointResumePlayResult + + if options.triggerCommand != "" || options.resumePlay { if pausePointIsArmed(ctx, connection, options.id) { - triggerHandle = startPausePointTrigger(ctx, connection, options.startPath, options.triggerCommand, options.triggerArgs) + if options.resumePlay { + result := resumePlayModeForPausePoint(ctx, connection) + resumeResult = &result + if result.Error != "" { + if options.triggerCommand != "" { + skippedTriggerResult = &pausePointTriggerResult{ + Command: pausePointTriggerCommandString(options.triggerCommand, options.triggerArgs), + Error: "trigger was not dispatched: --resume-play failed to resume play mode", + } + } + } else if options.triggerCommand != "" { + triggerHandle = startPausePointTrigger(ctx, connection, options.startPath, options.triggerCommand, options.triggerArgs) + } + } else if options.triggerCommand != "" { + triggerHandle = startPausePointTrigger(ctx, connection, options.startPath, options.triggerCommand, options.triggerArgs) + } } else { - // --trigger always yields a TriggerResult when given, even when skipped: a silently - // omitted TriggerResult would be indistinguishable from "the trigger ran but this CLI - // exited before it reported anything," hiding the real reason (marker not confirmed - // armed) behind a plain timeout/not-enabled error with no clue why the trigger never fired. - skippedTriggerResult = &pausePointTriggerResult{ - Command: pausePointTriggerCommandString(options.triggerCommand, options.triggerArgs), - Error: "trigger was not dispatched: the marker could not be confirmed armed at wait start", + if options.resumePlay { + // --resume-play always yields a ResumePlayResult when given, even when skipped: a + // silently omitted result would hide "arm was never confirmed" behind a plain + // timeout/not-enabled error with no clue why Play was never resumed. + resumeResult = &pausePointResumePlayResult{ + Error: "resume was not dispatched: the marker could not be confirmed armed at wait start", + } + } + if options.triggerCommand != "" { + // --trigger always yields a TriggerResult when given, even when skipped: a silently + // omitted TriggerResult would be indistinguishable from "the trigger ran but this CLI + // exited before it reported anything," hiding the real reason (marker not confirmed + // armed) behind a plain timeout/not-enabled error with no clue why the trigger never fired. + skippedTriggerResult = &pausePointTriggerResult{ + Command: pausePointTriggerCommandString(options.triggerCommand, options.triggerArgs), + Error: "trigger was not dispatched: the marker could not be confirmed armed at wait start", + } } } } response, state, err := waitForPausePointStatus(ctx, connection, options) if skippedTriggerResult != nil { - return response, state, skippedTriggerResult, err + return response, state, skippedTriggerResult, resumeResult, err + } + if triggerHandle != nil { + return response, state, triggerHandle.join(), resumeResult, err } - return response, state, triggerHandle.join(), err + return response, state, nil, resumeResult, err } // pausePointIsArmed reports whether the marker is enabled or already hit. A query failure is 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 c95f24cc8..699d87cc6 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, @@ -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, diff --git a/docs/regression-harness.md b/docs/regression-harness.md index a4b504f10..e9f5e8210 100644 --- a/docs/regression-harness.md +++ b/docs/regression-harness.md @@ -31,6 +31,7 @@ scenario covers. |------|-------|--------| | Key state divergence after a pause-point interruption | `Assets/RegressionHarness/KeyStateAfterPauseInterruption/` | `scripts/regression-harness-key-state-after-pause-interruption.sh` | | `await-pause-point --trigger` hits within the marker timeout instead of waiting out the triggered command's full duration | `Assets/RegressionHarness/KeyStateAfterPauseInterruption/` (reused) | `scripts/regression-harness-pause-point-trigger.sh` | +| `enable-pause-point --await --resume-play --trigger` resumes a manually paused PlayMode before firing the input trigger and hits within the marker timeout | `Assets/RegressionHarness/KeyStateAfterPauseInterruption/` (reused) | `scripts/regression-harness-resume-play-paused-arm.sh` | | A pause point armed on a physics message method (or a method called one hop from one) can miss a GameObject that already existed before arming. The miss itself is environment-dependent and does not reproduce deterministically (see below) -- the harness runs three scenarios (direct/OnCollisionEnter2D, indirect callee with priming, and OnTriggerEnter2D), each triggering a fresh contact after arming and classifying the result from the component's own hit counter plus `IsHit` | `Assets/RegressionHarness/PhysicsCallbackExistingInstance/` | `scripts/regression-harness-physics-callback-existing-instance.sh` | ### Physics-callback existing-instance miss: environment-dependent, not deterministic diff --git a/scripts/regression-harness-resume-play-paused-arm.sh b/scripts/regression-harness-resume-play-paused-arm.sh new file mode 100755 index 000000000..529fcdeb9 --- /dev/null +++ b/scripts/regression-harness-resume-play-paused-arm.sh @@ -0,0 +1,102 @@ +#!/bin/sh +set -e +# Regression harness for enable-pause-point --await --resume-play --trigger: +# arming a pause-point while PlayMode is manually paused, then resuming and +# firing the input trigger in one CLI call, must hit the marker and report +# ResumePlayResult.WasPaused=true / Resumed=true. Reuses the +# KeyStateAfterPauseInterruption scene scaffolding (see +# regression-harness-key-state-after-pause-interruption.sh). +# +# Usage: sh scripts/regression-harness-resume-play-paused-arm.sh [--project-path ] +# +# Prerequisites: +# - Assets/RegressionHarness/KeyStateAfterPauseInterruption/KeyStateAfterPauseInterruption.unity +# must be open in a running Unity Editor +# - uloop CLI must be installed +# - jq must be installed + +PROJECT_PATH="" +# Why: reject malformed argv before cleanup can touch the wrong Unity project. +if [ "$#" -eq 0 ]; then + : +elif [ "$#" -eq 2 ] && [ "$1" = "--project-path" ] && [ -n "$2" ]; then + PROJECT_PATH="$2" +else + printf '%s\n' "Usage: $0 [--project-path ]" >&2 + exit 2 +fi + +MARKER_FILE="Assets/RegressionHarness/KeyStateAfterPauseInterruption/SpaceHoldPoller.cs" +MARKER_LINE="19" +PRESS_DURATION="5" +RESULT_FILE="$(mktemp)" + +run_uloop() { + if [ -n "$PROJECT_PATH" ]; then + uloop "$@" --project-path "$PROJECT_PATH" + else + uloop "$@" + fi +} + +log() { + printf "\033[36m[resume-play-paused-arm]\033[0m %s\n" "$1" +} + +cleanup() { + rm -f "$RESULT_FILE" + run_uloop clear-pause-point --all > /dev/null 2>&1 || true + run_uloop control-play-mode --action Stop > /dev/null 2>&1 || true +} +trap cleanup EXIT + +log "Stopping any existing Play Mode session..." +run_uloop control-play-mode --action Stop > /dev/null + +log "Clearing any existing pause-point markers..." +run_uloop clear-pause-point --all > /dev/null + +log "Starting Play Mode..." +run_uloop control-play-mode --action Play > /dev/null + +log "Pausing Play Mode before arming..." +run_uloop control-play-mode --action Pause > /dev/null + +log "Arming + awaiting with --resume-play --trigger on $MARKER_FILE:$MARKER_LINE..." +# Why: TimeoutSeconds is not frozen during a manual Pause, so keep the window +# generous enough for resume + trigger + hit under a slow Editor tick. +run_uloop enable-pause-point \ + --file "$MARKER_FILE" \ + --line "$MARKER_LINE" \ + --timeout-seconds 60 \ + --await \ + --resume-play \ + --trigger "simulate-keyboard --action Press --key Space --duration $PRESS_DURATION" \ + > "$RESULT_FILE" 2>&1 || true + +SUCCESS="$(jq -r '.Success' "$RESULT_FILE")" +STATUS="$(jq -r '.Status' "$RESULT_FILE")" +WAS_PAUSED="$(jq -r '.ResumePlayResult.WasPaused' "$RESULT_FILE")" +RESUMED="$(jq -r '.ResumePlayResult.Resumed' "$RESULT_FILE")" +TRIGGER_INTERRUPTED="$(jq -r '.TriggerResult.Response.InterruptedByPausePoint' "$RESULT_FILE")" + +if [ "$SUCCESS" != "true" ] || [ "$STATUS" != "Hit" ]; then + log "FAIL: expected Success=true Status=Hit, got Success=$SUCCESS Status=$STATUS" + cat "$RESULT_FILE" + exit 1 +fi + +if [ "$WAS_PAUSED" != "true" ] || [ "$RESUMED" != "true" ]; then + log "FAIL: expected ResumePlayResult WasPaused=true Resumed=true, got WasPaused=$WAS_PAUSED Resumed=$RESUMED" + cat "$RESULT_FILE" + exit 1 +fi + +if [ "$TRIGGER_INTERRUPTED" != "true" ]; then + log "FAIL: expected TriggerResult.Response.InterruptedByPausePoint=true, got $TRIGGER_INTERRUPTED" + cat "$RESULT_FILE" + exit 1 +fi + +log "PASS: paused-arm --resume-play --trigger hit with ResumePlayResult.Resumed=true." +exit 0 From 88573002b3a5ee505ad9e0ba44b9cf6c0b2dae9e Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Fri, 24 Jul 2026 03:15:19 +0900 Subject: [PATCH 2/5] fix: recover keyboard key state after pause-point interruption with ReleaseAll (#1965) Co-authored-by: Cursor --- .../SpaceHoldPoller.cs | 9 + .../Editor/KeyboardKeyStateServiceTests.cs | 49 +++++ .../KeyboardKeyStateServiceTests.cs.meta | 11 ++ .../Editor/UnityCLILoop.Tests.Editor.asmdef | 11 +- ...oardInputSimulationResponseFactoryTests.cs | 2 +- .../UnityCliLoopInputSimulationTypes.cs | 3 +- .../InputSystemConfiguredUpdateApplier.cs | 52 +++++ .../Application/KeyboardKeyState.cs | 32 +++ .../KeyboardInputActionExecutor.cs | 29 ++- .../KeyboardInputMainThreadCleanup.cs | 110 ++++++++++- .../KeyboardInputSimulationResponseFactory.cs | 3 +- .../SimulateKeyboardResponse.cs | 5 + .../SimulateKeyboardUseCase.cs | 66 ++++++- cli/common/tools/default-tools.json | 11 +- docs/regression-harness.md | 2 +- ...ness-key-state-after-pause-interruption.sh | 184 ++++++++++++++++-- 16 files changed, 544 insertions(+), 35 deletions(-) create mode 100644 Assets/Tests/Editor/KeyboardKeyStateServiceTests.cs create mode 100644 Assets/Tests/Editor/KeyboardKeyStateServiceTests.cs.meta diff --git a/Assets/RegressionHarness/KeyStateAfterPauseInterruption/SpaceHoldPoller.cs b/Assets/RegressionHarness/KeyStateAfterPauseInterruption/SpaceHoldPoller.cs index 7474f758f..18a22d16f 100644 --- a/Assets/RegressionHarness/KeyStateAfterPauseInterruption/SpaceHoldPoller.cs +++ b/Assets/RegressionHarness/KeyStateAfterPauseInterruption/SpaceHoldPoller.cs @@ -19,5 +19,14 @@ private void Update() IsSpaceHeld = isHeld; } } + + /// + /// Clears the sticky IsSpaceHeld latch so a post-resume probe can detect a fresh press edge. + /// Kept below Update so the harness pause-point line number (19) stays stable. + /// + public void ClearStickyHeld() + { + IsSpaceHeld = false; + } } } diff --git a/Assets/Tests/Editor/KeyboardKeyStateServiceTests.cs b/Assets/Tests/Editor/KeyboardKeyStateServiceTests.cs new file mode 100644 index 000000000..b0969223a --- /dev/null +++ b/Assets/Tests/Editor/KeyboardKeyStateServiceTests.cs @@ -0,0 +1,49 @@ +#if ULOOP_HAS_INPUT_SYSTEM +using System.Collections.Generic; +using NUnit.Framework; +using UnityEngine.InputSystem; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Pure bookkeeping tests for KeyboardKeyStateService — no Input System updates or Editor pause. + /// + public class KeyboardKeyStateServiceTests + { + /// + /// Verifies mixed held + transient bookkeeping is fully drained without touching the device. + /// + [Test] + public void ClearTrackedKeys_WhenHeldAndTransientMixed_ReturnsAllKeysAndClearsHeldState() + { + KeyboardKeyStateService service = new KeyboardKeyStateService(); + service.SetKeyDown(Key.W); + service.SetKeyDown(Key.LeftShift); + service.RegisterTransientKey(Key.Space); + + IReadOnlyList released = service.ClearTrackedKeys(); + + Assert.That(released, Is.EquivalentTo(new[] { Key.W, Key.LeftShift, Key.Space })); + Assert.That(service.IsKeyHeld(Key.W), Is.False); + Assert.That(service.IsKeyHeld(Key.LeftShift), Is.False); + Assert.That(service.IsKeyHeld(Key.Space), Is.False); + Assert.That(service.HeldKeys, Is.Empty); + } + + /// + /// Verifies a no-op drain returns an empty list rather than null. + /// + [Test] + public void ClearTrackedKeys_WhenEmpty_ReturnsEmptyList() + { + KeyboardKeyStateService service = new KeyboardKeyStateService(); + + IReadOnlyList released = service.ClearTrackedKeys(); + + Assert.That(released, Is.Empty); + } + } +} +#endif diff --git a/Assets/Tests/Editor/KeyboardKeyStateServiceTests.cs.meta b/Assets/Tests/Editor/KeyboardKeyStateServiceTests.cs.meta new file mode 100644 index 000000000..981146d5a --- /dev/null +++ b/Assets/Tests/Editor/KeyboardKeyStateServiceTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: aa5bf4b4b372f4516aacee0343e07003 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/UnityCLILoop.Tests.Editor.asmdef b/Assets/Tests/Editor/UnityCLILoop.Tests.Editor.asmdef index c95c78255..0c64bbfdc 100644 --- a/Assets/Tests/Editor/UnityCLILoop.Tests.Editor.asmdef +++ b/Assets/Tests/Editor/UnityCLILoop.Tests.Editor.asmdef @@ -41,7 +41,8 @@ "GUID:33fef506e6744f2982e8c13b1196f696", "GUID:da253b56f9d34380980fbe7bafe64666", "GUID:96b8d4624fb74ea2849fed17e7ad69d3", - "GUID:e3fd8b49afd6f4e3cae09011028922b4" + "GUID:e3fd8b49afd6f4e3cae09011028922b4", + "GUID:75469ad4d38634e559750d17036d5f7c" ], "includePlatforms": [ "Editor" @@ -52,6 +53,12 @@ "precompiledReferences": [], "autoReferenced": false, "defineConstraints": [], - "versionDefines": [], + "versionDefines": [ + { + "name": "com.unity.inputsystem", + "expression": "", + "define": "ULOOP_HAS_INPUT_SYSTEM" + } + ], "noEngineReferences": false } diff --git a/Assets/Tests/PlayMode/KeyboardInputSimulationResponseFactoryTests.cs b/Assets/Tests/PlayMode/KeyboardInputSimulationResponseFactoryTests.cs index 48587def6..bd829ff51 100644 --- a/Assets/Tests/PlayMode/KeyboardInputSimulationResponseFactoryTests.cs +++ b/Assets/Tests/PlayMode/KeyboardInputSimulationResponseFactoryTests.cs @@ -51,7 +51,7 @@ public void InterruptedKeyResult_WithMultiplePausePointHits_MapsPauseEvidenceAnd Assert.That( response.Message, Is.EqualTo( - "Keyboard input stopped because Unity paused during Pause Point inspection. Key 'Space' was released from Unity CLI Loop bookkeeping.")); + "Keyboard input stopped because Unity paused during Pause Point inspection. Key 'Space' was released from Unity CLI Loop bookkeeping; queued input edge was discarded.")); Assert.That(response.Action, Is.EqualTo(UnityCliLoopKeyboardAction.Press.ToString())); Assert.That(response.KeyName, Is.EqualTo("Space")); Assert.That(response.InterruptedByPausePoint, Is.True); diff --git a/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/UnityCliLoopInputSimulationTypes.cs b/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/UnityCliLoopInputSimulationTypes.cs index fd6dcdf06..fd532cf18 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/UnityCliLoopInputSimulationTypes.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/InputSimulation/UnityCliLoopInputSimulationTypes.cs @@ -4,7 +4,8 @@ public enum UnityCliLoopKeyboardAction { Press = 0, KeyDown = 1, - KeyUp = 2 + KeyUp = 2, + ReleaseAll = 3 } public enum UnityCliLoopMouseInputAction diff --git a/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs b/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs index 63edf6f8f..837031757 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs @@ -20,6 +20,7 @@ internal static class InputSystemConfiguredUpdateApplier private const int ApplyWaitStateWaiting = 0; private const int ApplyWaitStateApplying = 1; private const int ApplyWaitStateFinishedWithoutApply = 2; + private const int PausePollMilliseconds = 50; public static async Task ApplyOnNextConfiguredUpdate( Action apply, @@ -44,6 +45,21 @@ public static async Task ApplyOnNextConfiguredUpdate Action? callback = null; InputSystemUpdateSubscription? subscription = null; + void TryDiscardForPause() + { + if (Interlocked.CompareExchange( + ref applyWaitState, + ApplyWaitStateFinishedWithoutApply, + ApplyWaitStateWaiting) != ApplyWaitStateWaiting) + { + return; + } + + // Dispose before cleanup release so a queued edge cannot apply after resume. + subscription?.Dispose(); + tcs.TrySetResult(InputSimulationWaitOutcome.Paused); + } + callback = () => { Debug.Assert(callback != null, "callback must be assigned before subscription"); @@ -57,6 +73,14 @@ public static async Task ApplyOnNextConfiguredUpdate return; } + // Why: if PlayMode paused before this update, discard the edge instead of applying + // it after resume (that delayed apply is the Repro B / round-8 stale press). + if (InputSystemUpdateHelper.IsPaused()) + { + TryDiscardForPause(); + return; + } + InputUpdateType currentUpdateType = InputState.currentUpdateType; if (!InputUpdateTypeResolver.IsMatch(currentUpdateType, targetUpdateType)) { @@ -107,6 +131,9 @@ public static async Task ApplyOnNextConfiguredUpdate Task timeoutTask = TimerDelay.Wait( InputSystemUpdateHelper.ApplyTimeoutMilliseconds, timeoutCts.Token); + // Why: onBeforeUpdate may not run while paused, so poll pause separately and + // dispose the subscription before any later resume can apply the queued edge. + _ = WatchForPauseDiscardAsync(TryDiscardForPause, timeoutCts.Token); Task completedTask = await Task.WhenAny(tcs.Task, timeoutTask).ConfigureAwait(false); if (completedTask == timeoutTask) { @@ -137,6 +164,31 @@ public static async Task ApplyOnNextConfiguredUpdate } } + private static async Task WatchForPauseDiscardAsync(Action tryDiscardForPause, CancellationToken ct) + { + Debug.Assert(tryDiscardForPause != null, "tryDiscardForPause must not be null"); + while (!ct.IsCancellationRequested) + { + if (InputSystemUpdateHelper.IsPaused()) + { + tryDiscardForPause(); + return; + } + + // Why not await TimerDelay with ct directly: cancellation throws + // OperationCanceledException, and this path must exit without try/catch. + Task delayTask = TimerDelay.Wait(PausePollMilliseconds, CancellationToken.None); + Task cancellationTask = Task.Delay(Timeout.Infinite, ct); + Task completedTask = await Task.WhenAny(delayTask, cancellationTask).ConfigureAwait(false); + if (completedTask == cancellationTask || ct.IsCancellationRequested) + { + return; + } + + await delayTask.ConfigureAwait(false); + } + } + private static InputSimulationWaitOutcome ApplyOnExplicitUpdate( Action apply, InputUpdateType targetUpdateType, diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Application/KeyboardKeyState.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Application/KeyboardKeyState.cs index 83ff2453b..976e77b40 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Application/KeyboardKeyState.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Application/KeyboardKeyState.cs @@ -1,5 +1,6 @@ #if ULOOP_HAS_INPUT_SYSTEM #nullable enable +using System; using System.Collections.Generic; using UnityEditor; using UnityEngine.InputSystem; @@ -20,6 +21,32 @@ internal sealed class KeyboardKeyStateService public bool IsKeyHeld(Key key) => _heldKeys.Contains(key); public IReadOnlyCollection HeldKeys => _heldKeys; + /// + /// Drains held and transient bookkeeping without touching the Input System device. + /// Why: ReleaseAll's EditMode tests and the CLI action need a device-free drain so + /// bookkeeping can be verified without running real input updates. + /// + public IReadOnlyList ClearTrackedKeys() + { + List released = new List(_heldKeys.Count + _transientKeys.Count); + foreach (Key key in _heldKeys) + { + released.Add(key); + } + + foreach (Key key in _transientKeys) + { + if (!_heldKeys.Contains(key)) + { + released.Add(key); + } + } + + _heldKeys.Clear(); + _transientKeys.Clear(); + return released; + } + public void RegisterPlayModeCallbacks() { // Guard against duplicate subscriptions on domain reload @@ -158,6 +185,11 @@ public static void Clear() ServiceValue.Clear(); } + public static IReadOnlyList ClearTrackedKeys() + { + return ServiceValue.ClearTrackedKeys(); + } + public static void SetKeyState(Keyboard keyboard, Key key, bool pressed) { ServiceValue.SetKeyState(keyboard, key, pressed); diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputActionExecutor.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputActionExecutor.cs index d3c350da6..4dfb94f4d 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputActionExecutor.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputActionExecutor.cs @@ -93,6 +93,20 @@ await InputSystemUpdateHelper.WaitForPressLifetime( key, pressWasApplied); } + else if (waitOutcome == InputSimulationWaitOutcome.Paused) + { + // Why: ApplyOnNextConfiguredUpdate already disposed any pending press + // subscription when it returned Paused. Release + latch-sync only after + // that dispose — a live queued edge or a stale wasPressedThisFrame latch + // (armed by edge monitoring even when apply never committed) would both + // look like a re-press after resume. + KeyboardInputMainThreadCleanup.ReleaseKeyStateImmediatelyAfterPauseInterruption( + keyboard, + key); + + KeyboardKeyState.UnregisterTransientKey(key); + SimulateKeyboardOverlayState.ClearPress(); + } else if (pressWasApplied) { InputSimulationWaitOutcome releaseOutcome = @@ -105,11 +119,6 @@ await KeyboardInputMainThreadCleanup.ReleaseKeyStateIfPossible( waitOutcome = InputSimulationWaitOutcome.TimedOut; KeyboardInputMainThreadCleanup.ScheduleTimedOutPressCleanup(keyboard, key, false); } - else if (waitOutcome == InputSimulationWaitOutcome.Paused) - { - KeyboardKeyState.UnregisterTransientKey(key); - SimulateKeyboardOverlayState.ClearPress(); - } else { KeyboardKeyState.UnregisterTransientKey(key); @@ -248,6 +257,16 @@ await KeyboardInputMainThreadCleanup.RollbackHeldKey( if (waitOutcome == InputSimulationWaitOutcome.Paused) { + // Apply may never have committed, but edge monitoring can still arm the + // wasPressedThisFrame latch; sync it so resume does not report a stale press. + if (!keyDownApplied) + { + await InputSystemUpdateHelper.SwitchToMainThreadIfNeeded(CancellationToken.None); + KeyboardInputMainThreadCleanup.ReleaseKeyStateImmediatelyAfterPauseInterruption( + keyboard, + key); + } + return KeyboardInputSimulationResponseFactory.InterruptedKeyResult( UnityCliLoopKeyboardAction.KeyDown, keyName, diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputMainThreadCleanup.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputMainThreadCleanup.cs index eab92fb24..46c5dffef 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputMainThreadCleanup.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputMainThreadCleanup.cs @@ -1,10 +1,13 @@ #if ULOOP_HAS_INPUT_SYSTEM #nullable enable +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using UnityEditor; using UnityEngine; using UnityEngine.InputSystem; +using UnityEngine.InputSystem.Controls; +using UnityEngine.InputSystem.LowLevel; using io.github.hatayama.UnityCliLoop.Runtime; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -16,6 +19,69 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// internal static class KeyboardInputMainThreadCleanup { + /// + /// Forces every tracked and device-pressed key up via an explicit update, then clears + /// bookkeeping. Safe while PlayMode is paused (ReleaseAll's paused-tolerant path). + /// + internal static IReadOnlyList ReleaseAllKeysImmediately(Keyboard keyboard) + { + HashSet keysToRelease = new HashSet(); + foreach (Key tracked in KeyboardKeyState.ClearTrackedKeys()) + { + keysToRelease.Add(tracked); + } + + if (keyboard != null) + { + foreach (KeyControl control in keyboard.allKeys) + { + if (control != null && control.isPressed) + { + keysToRelease.Add(control.keyCode); + } + } + } + + List releasedNames = new List(keysToRelease.Count); + foreach (Key key in keysToRelease) + { + releasedNames.Add(key.ToString()); + } + + releasedNames.Sort(System.StringComparer.Ordinal); + + if (keyboard != null && keysToRelease.Count > 0 && CanInjectKeyboardState(keyboard)) + { + using (StateEvent.From(keyboard, out InputEventPtr eventPtr)) + { + foreach (Key key in keysToRelease) + { + keyboard[key].WriteValueIntoEvent(0f, eventPtr); + } + + InputUpdateType updateType = InputUpdateTypeResolver.Resolve(); + InputState.Change(keyboard, eventPtr, updateType); + } + + InputSystemUpdateHelper.RunExplicitUpdate(InputUpdateTypeResolver.Resolve()); + + // Why only when still isPressed: ForceSync injects a real press→release edge. + // Unconditionally doing that on every ReleaseAll key would spuriously fire + // gameplay (e.g. jump) when called outside a pause-interruption recovery. + // isPressed=true after a zero write is the stale wasPressedThisFrame latch signature. + foreach (Key key in keysToRelease) + { + if (keyboard[key].isPressed) + { + ForceSyncButtonPressLatch(keyboard, key); + } + } + } + + SimulateKeyboardOverlayState.ClearPress(); + return releasedNames; + } + internal static async Task FinalizePressOverlay(CancellationToken ct) { await InputSystemUpdateHelper.SwitchToMainThreadIfNeeded(CancellationToken.None); @@ -66,7 +132,7 @@ internal static async Task ReleaseKeyStateIfPossible if (EditorApplication.isPaused) { - ReleaseKeyStateImmediately(keyboard, key); + ReleaseKeyStateImmediatelyAfterPauseInterruption(keyboard, key); return InputSimulationWaitOutcome.Completed; } @@ -95,7 +161,11 @@ private static async Task ReleaseKeyStateImmediatelyOnMainThreadAsync( ReleaseKeyStateImmediately(keyboard, key); } - private static void ReleaseKeyStateImmediately(Keyboard keyboard, Key key) + /// + /// Forces a single key up via an explicit update. Used by pause-interruption cleanup + /// after any pending apply subscription has already been disposed. + /// + internal static void ReleaseKeyStateImmediately(Keyboard keyboard, Key key) { Debug.Assert(CanInjectKeyboardState(keyboard), "keyboard state can only be released while PlayMode has a keyboard"); if (!CanInjectKeyboardState(keyboard)) @@ -107,6 +177,42 @@ private static void ReleaseKeyStateImmediately(Keyboard keyboard, Key key) InputSystemUpdateHelper.RunExplicitUpdate(InputUpdateTypeResolver.Resolve()); } + /// + /// Pause-interruption cleanup: dispose path has already dropped pending applies; release + /// the device key and sync Input System button press latches so resume cannot report a + /// stale isPressed=true. + /// + internal static void ReleaseKeyStateImmediatelyAfterPauseInterruption(Keyboard keyboard, Key key) + { + ReleaseKeyStateImmediately(keyboard, key); + if (!CanInjectKeyboardState(keyboard)) + { + return; + } + + ForceSyncButtonPressLatch(keyboard, key); + } + + /// + /// Forces a press→release transition so ButtonControl's wasPressedThisFrame latch updates. + /// Why: Press edge monitoring calls wasPressedThisFrame, which arms m_LastUpdateWasPress. + /// Writing zero into an already-zero state buffer is a no-op for that latch sync, so after + /// Editor pause/resume isPressed can stay true forever while ReadValue is 0. + /// + private static void ForceSyncButtonPressLatch(Keyboard keyboard, Key key) + { + Debug.Assert(CanInjectKeyboardState(keyboard), "press latch sync requires an injectable keyboard"); + if (!CanInjectKeyboardState(keyboard)) + { + return; + } + + KeyboardKeyState.SetKeyState(keyboard, key, true); + InputSystemUpdateHelper.RunExplicitUpdate(InputUpdateTypeResolver.Resolve()); + KeyboardKeyState.SetKeyState(keyboard, key, false); + InputSystemUpdateHelper.RunExplicitUpdate(InputUpdateTypeResolver.Resolve()); + } + internal static void ScheduleTimedOutPressCleanup(Keyboard keyboard, Key key, bool pressWasApplied) { CleanupTimedOutPressAsync(keyboard, key, pressWasApplied, CancellationToken.None).Forget(); diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputSimulationResponseFactory.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputSimulationResponseFactory.cs index 7ddf81d7f..8fd12fdb8 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputSimulationResponseFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/KeyboardInputSimulationResponseFactory.cs @@ -24,7 +24,8 @@ internal static SimulateKeyboardResponse InterruptedKeyResult( SimulateKeyboardResponse result = new() { Success = true, - Message = $"Keyboard input stopped because Unity paused during Pause Point inspection. Key '{keyName}' was released from Unity CLI Loop bookkeeping.", + Message = + $"Keyboard input stopped because Unity paused during Pause Point inspection. Key '{keyName}' was released from Unity CLI Loop bookkeeping; queued input edge was discarded.", Action = action.ToString(), KeyName = keyName, InterruptedByPausePoint = true, diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs index 6ddc42b5a..52d1151b4 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardResponse.cs @@ -64,6 +64,11 @@ public class SimulateKeyboardResponse : UnityCliLoopToolResponse /// public bool? KeyStateDeviceIsPressed { get; set; } + /// + /// Key names released by the ReleaseAll action (bookkeeping and/or device). Null for other actions. + /// + public List? ReleasedKeys { get; set; } + public SimulateKeyboardResponse() { } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs index f467917a1..780e512a8 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs @@ -47,7 +47,11 @@ public async Task ExecuteAsync( #else string correlationId = UnityCliLoopConstants.GenerateCorrelationId(); - ValidationResult preflight = PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); + // ReleaseAll must work while paused so agents can recover stuck device state after a + // pause-point interruption without first resuming PlayMode. + ValidationResult preflight = parameters.Action == UnityCliLoopKeyboardAction.ReleaseAll + ? PlayModeToolPreflightService.RequireActive() + : PlayModeToolPreflightService.RequireActiveAndNotPaused(PausedActionDescription); if (!preflight.IsValid) { return new SimulateKeyboardResponse @@ -58,6 +62,11 @@ public async Task ExecuteAsync( }; } + if (parameters.Action == UnityCliLoopKeyboardAction.ReleaseAll) + { + return await ExecuteReleaseAllAsync(correlationId, ct).ConfigureAwait(false); + } + if (string.IsNullOrEmpty(parameters.Key)) { return new SimulateKeyboardResponse @@ -161,6 +170,61 @@ public async Task ExecuteAsync( } #if ULOOP_HAS_INPUT_SYSTEM + private static async Task ExecuteReleaseAllAsync( + string correlationId, + CancellationToken ct) + { + // Why not ClearLatestHitSnapshot: ReleaseAll is intended for recovery while a + // pause-point inspection is still active; clearing would drop TryGetCapturedValue + // live references that agents need during that pause. + + VibeLogger.LogInfo( + "simulate_keyboard_start", + "Keyboard simulation started", + new { Action = UnityCliLoopKeyboardAction.ReleaseAll.ToString() }, + correlationId: correlationId + ); + + // Why not `using`: SwitchToMainThreadIfNeeded can resume on a thread-pool thread + // after ConfigureAwait(false), and Application.runInBackground is main-thread-only. + InputSimulationRunInBackgroundScope runInBackgroundScope = InputSimulationRunInBackgroundScope.Enable(); + try + { + EnsureOverlayExists(); + await InputSystemUpdateHelper.SwitchToMainThreadIfNeeded(ct); + Keyboard keyboard = Keyboard.current; + IReadOnlyList releasedKeys = + KeyboardInputMainThreadCleanup.ReleaseAllKeysImmediately(keyboard); + List releasedKeysList = new List(releasedKeys); + + string message = releasedKeysList.Count == 0 + ? "Released all keys (none were held)." + : $"Released {releasedKeysList.Count} key(s): {string.Join(", ", releasedKeysList)}"; + + SimulateKeyboardResponse response = new SimulateKeyboardResponse + { + Success = true, + Message = message, + Action = UnityCliLoopKeyboardAction.ReleaseAll.ToString(), + ReleasedKeys = releasedKeysList + }; + + VibeLogger.LogInfo( + "simulate_keyboard_complete", + $"Keyboard simulation completed: {response.Message}", + new { Action = UnityCliLoopKeyboardAction.ReleaseAll.ToString(), Success = true }, + correlationId: correlationId + ); + + return response; + } + finally + { + await InputSystemUpdateHelper.SwitchToMainThreadIfNeeded(CancellationToken.None); + runInBackgroundScope.Dispose(); + } + } + private static void EnsureOverlayExists() { OverlayCanvasFactory.EnsureExists(); diff --git a/cli/common/tools/default-tools.json b/cli/common/tools/default-tools.json index 66740bf28..fc56eba53 100644 --- a/cli/common/tools/default-tools.json +++ b/cli/common/tools/default-tools.json @@ -604,27 +604,28 @@ }, { "name": "simulate-keyboard", - "description": "Simulate keyboard key input in PlayMode via Input System. Supports one-shot press, key-down hold, and key-up release for game controls (WASD, Space, etc.). Requires the Input System package (com.unity.inputsystem).", + "description": "Simulate keyboard key input in PlayMode via Input System. Supports one-shot press, key-down hold, key-up release, and ReleaseAll recovery for game controls (WASD, Space, etc.). Requires the Input System package (com.unity.inputsystem).", "inputSchema": { "type": "object", "properties": { "Action": { "type": "string", - "description": "Keyboard action: Press - one-shot key tap (Down then Up), KeyDown - hold key down, KeyUp - release held key", + "description": "Keyboard action: 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)", "enum": [ "Press", "KeyDown", - "KeyUp" + "KeyUp", + "ReleaseAll" ], "default": "Press" }, "Key": { "type": "string", - "description": "Key name matching Input System Key enum (e.g. \"W\", \"Space\", \"LeftShift\", \"A\", \"Return\"). Case-insensitive." + "description": "Key name matching Input System Key enum (e.g. \"W\", \"Space\", \"LeftShift\", \"A\", \"Return\"). Case-insensitive. Not required for ReleaseAll." }, "Duration": { "type": "number", - "description": "Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp.", + "description": "Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp/ReleaseAll.", "default": 0 } } diff --git a/docs/regression-harness.md b/docs/regression-harness.md index e9f5e8210..307b319e1 100644 --- a/docs/regression-harness.md +++ b/docs/regression-harness.md @@ -29,7 +29,7 @@ scenario covers. | Trap | Scene | Script | |------|-------|--------| -| Key state divergence after a pause-point interruption | `Assets/RegressionHarness/KeyStateAfterPauseInterruption/` | `scripts/regression-harness-key-state-after-pause-interruption.sh` | +| Key state after a pause-point interruption: (1) Press reports `InterruptedByPausePoint`, (2) resume within the apply-timeout window after interrupt must not re-press, (3) after a >6s pause `ReleaseAll` while still paused restores keys without clearing captures, then a fresh Press succeeds | `Assets/RegressionHarness/KeyStateAfterPauseInterruption/` | `scripts/regression-harness-key-state-after-pause-interruption.sh` | | `await-pause-point --trigger` hits within the marker timeout instead of waiting out the triggered command's full duration | `Assets/RegressionHarness/KeyStateAfterPauseInterruption/` (reused) | `scripts/regression-harness-pause-point-trigger.sh` | | `enable-pause-point --await --resume-play --trigger` resumes a manually paused PlayMode before firing the input trigger and hits within the marker timeout | `Assets/RegressionHarness/KeyStateAfterPauseInterruption/` (reused) | `scripts/regression-harness-resume-play-paused-arm.sh` | | A pause point armed on a physics message method (or a method called one hop from one) can miss a GameObject that already existed before arming. The miss itself is environment-dependent and does not reproduce deterministically (see below) -- the harness runs three scenarios (direct/OnCollisionEnter2D, indirect callee with priming, and OnTriggerEnter2D), each triggering a fresh contact after arming and classifying the result from the component's own hit counter plus `IsHit` | `Assets/RegressionHarness/PhysicsCallbackExistingInstance/` | `scripts/regression-harness-physics-callback-existing-instance.sh` | diff --git a/scripts/regression-harness-key-state-after-pause-interruption.sh b/scripts/regression-harness-key-state-after-pause-interruption.sh index 9d471bfeb..69547b5b4 100755 --- a/scripts/regression-harness-key-state-after-pause-interruption.sh +++ b/scripts/regression-harness-key-state-after-pause-interruption.sh @@ -1,16 +1,17 @@ #!/bin/sh set -e -# Regression harness for the KeyStateAfterPauseInterruption trap: a pause-point -# hit during simulate-keyboard Press must be reported as an interruption -# (InterruptedByPausePoint=true) instead of being silently absorbed into a -# successful Completed result. See docs/regression-harness.md. +# Regression harness for the KeyStateAfterPauseInterruption trap: +# 1) A pause-point hit during simulate-keyboard Press must report +# InterruptedByPausePoint=true (early return). +# 2) Resuming within 3s of the interrupt must NOT re-apply a discarded +# queued press edge (device stays unpressed without a new CLI input). +# 3) After a >6s pause, ReleaseAll while still paused must restore key +# state without clearing pause-point captures, then a fresh Press succeeds. +# See docs/regression-harness.md. # # Contract under test: arm the pause-point on the key-consuming line in # SpaceHoldPoller.cs *before* starting Press, so the hit fires naturally from # game code once Press drives the key down (not from a concurrent CLI call). -# The environment-dependent editor-tick-stall path that can also swallow a -# pause is covered by PressLifetimeIterationResolverTests (unit-level), not by -# this live harness — see that test file for the decisive Red/Green coverage. # # Usage: sh scripts/regression-harness-key-state-after-pause-interruption.sh [--project-path ] # @@ -21,14 +22,24 @@ set -e # - jq must be installed PROJECT_PATH="" -if [ "$1" = "--project-path" ] && [ -n "$2" ]; then +# Why: reject malformed argv before cleanup can touch the wrong Unity project. +if [ "$#" -eq 0 ]; then + : +elif [ "$#" -eq 2 ] && [ "$1" = "--project-path" ] && [ -n "$2" ]; then PROJECT_PATH="$2" +else + printf '%s\n' "Usage: $0 [--project-path ]" >&2 + exit 2 fi MARKER_FILE="Assets/RegressionHarness/KeyStateAfterPauseInterruption/SpaceHoldPoller.cs" MARKER_LINE="19" PRESS_DURATION="5" +# EDITOR_FRAME_WAIT_TIMEOUT_MS is 5000ms; the delayed-apply Red only reproduces when +# resume happens before that timeout from the interrupt moment. +APPLY_TIMEOUT_SECONDS="5" RESULT_FILE="$(mktemp)" +PROBE_FILE="$(mktemp)" run_uloop() { if [ -n "$PROJECT_PATH" ]; then @@ -43,12 +54,29 @@ log() { } cleanup() { - rm -f "$RESULT_FILE" + rm -f "$RESULT_FILE" "$PROBE_FILE" run_uloop clear-pause-point --all > /dev/null 2>&1 || true run_uloop control-play-mode --action Stop > /dev/null 2>&1 || true } trap cleanup EXIT +# Writes Result like: devicePressed=False isSpaceHeld=False +# When clear_sticky=1, clears SpaceHoldPoller's sticky latch before reading. +probe_space_state() { + clear_sticky="$1" + run_uloop execute-dynamic-code --code " +using UnityEngine; +using UnityEngine.InputSystem; +using io.github.hatayama.UnityCliLoop.RegressionHarness; +SpaceHoldPoller poller = Object.FindObjectOfType(); +if (poller == null) { return \"error=no-poller\"; } +if ($clear_sticky == 1) { poller.ClearStickyHeld(); } +bool devicePressed = Keyboard.current != null && Keyboard.current[Key.Space].isPressed; +return \"devicePressed=\" + devicePressed + \" isSpaceHeld=\" + poller.IsSpaceHeld; +" > "$PROBE_FILE" +} + +# --- Scenario 1: interrupt reports InterruptedByPausePoint --- log "Stopping any existing Play Mode session..." run_uloop control-play-mode --action Stop > /dev/null @@ -61,16 +89,17 @@ run_uloop control-play-mode --action Play > /dev/null log "Arming pause-point on $MARKER_FILE:$MARKER_LINE (before Press starts)..." run_uloop enable-pause-point --file "$MARKER_FILE" --line "$MARKER_LINE" --timeout-seconds 30 > /dev/null -log "Pressing Space for ${PRESS_DURATION}s in the background (the key-down should hit the marker)..." +log "Pressing Space for ${PRESS_DURATION}s (the key-down should hit the marker)..." START_SECONDS=$(date +%s) -run_uloop simulate-keyboard --action Press --key Space --duration "$PRESS_DURATION" > "$RESULT_FILE" 2>&1 & -PRESS_PID=$! - -wait "$PRESS_PID" || true -ELAPSED_SECONDS=$(( $(date +%s) - START_SECONDS )) +run_uloop simulate-keyboard --action Press --key Space --duration "$PRESS_DURATION" > "$RESULT_FILE" 2>&1 || true +# Why interrupt clock starts here: delayed-apply Red only fires when resume is within +# EDITOR_FRAME_WAIT_TIMEOUT_MS of the interrupt, not of the Play CLI call. +INTERRUPT_AT=$(date +%s) +ELAPSED_SECONDS=$(( INTERRUPT_AT - START_SECONDS )) INTERRUPTED="$(jq -r '.InterruptedByPausePoint' "$RESULT_FILE")" SUCCESS="$(jq -r '.Success' "$RESULT_FILE")" +MESSAGE="$(jq -r '.Message' "$RESULT_FILE")" if [ "$SUCCESS" != "true" ] || [ "$INTERRUPTED" != "true" ]; then log "FAIL: expected Success=true and InterruptedByPausePoint=true, got Success=$SUCCESS InterruptedByPausePoint=$INTERRUPTED" @@ -78,6 +107,15 @@ if [ "$SUCCESS" != "true" ] || [ "$INTERRUPTED" != "true" ]; then exit 1 fi +case "$MESSAGE" in + *"queued input edge was discarded"*) ;; + *) + log "FAIL: Interrupted message missing 'queued input edge was discarded'" + cat "$RESULT_FILE" + exit 1 + ;; +esac + # Why: a swallowed pause completes only after the full requested duration, so # an early return is what distinguishes a real interruption from a coincidence. if [ "$ELAPSED_SECONDS" -ge "$PRESS_DURATION" ]; then @@ -86,5 +124,119 @@ if [ "$ELAPSED_SECONDS" -ge "$PRESS_DURATION" ]; then exit 1 fi -log "PASS: Press was interrupted by the pause-point hit after ${ELAPSED_SECONDS}s (< ${PRESS_DURATION}s requested)." +log "PASS scenario 1: Press interrupted after ${ELAPSED_SECONDS}s (< ${PRESS_DURATION}s)." + +# --- Scenario 2: resume within 3s of interrupt must not re-press --- +log "Scenario 2: probe device while still paused, clear sticky latch, resume within ${APPLY_TIMEOUT_SECONDS}s of interrupt..." +probe_space_state 1 +PROBE_RESULT="$(jq -r '.Result' "$PROBE_FILE")" +case "$PROBE_RESULT" in + *devicePressed=False*|*devicePressed=false*) ;; + *) + log "FAIL scenario 2: expected devicePressed=false while paused after interrupt cleanup, got: $PROBE_RESULT" + cat "$PROBE_FILE" + exit 1 + ;; +esac + +SECONDS_SINCE_INTERRUPT=$(( $(date +%s) - INTERRUPT_AT )) +if [ "$SECONDS_SINCE_INTERRUPT" -ge "$APPLY_TIMEOUT_SECONDS" ]; then + log "FAIL scenario 2: ${SECONDS_SINCE_INTERRUPT}s already elapsed since interrupt (>=${APPLY_TIMEOUT_SECONDS}s apply timeout); probe was too slow to exercise the delayed-apply path." + exit 1 +fi + +run_uloop control-play-mode --action Play > /dev/null +RESUME_AT=$(date +%s) +INTERRUPT_TO_RESUME=$(( RESUME_AT - INTERRUPT_AT )) +if [ "$INTERRUPT_TO_RESUME" -ge "$APPLY_TIMEOUT_SECONDS" ]; then + log "FAIL scenario 2: interrupt→resume took ${INTERRUPT_TO_RESUME}s (>=${APPLY_TIMEOUT_SECONDS}s); delayed-apply Red would not reproduce." + exit 1 +fi + +# Observe without sending any new keyboard CLI input. +sleep 2 + +probe_space_state 0 +PROBE_AFTER="$(jq -r '.Result' "$PROBE_FILE")" +case "$PROBE_AFTER" in + *devicePressed=True*|*devicePressed=true*|*isSpaceHeld=True*|*isSpaceHeld=true*) + log "FAIL scenario 2: resume re-pressed Space without CLI input: $PROBE_AFTER" + cat "$PROBE_FILE" + exit 1 + ;; +esac +case "$PROBE_AFTER" in + *devicePressed=False*|*devicePressed=false*) ;; + *) + log "FAIL scenario 2: unexpected probe result after resume: $PROBE_AFTER" + cat "$PROBE_FILE" + exit 1 + ;; +esac + +log "PASS scenario 2: no re-press after resume ${INTERRUPT_TO_RESUME}s after interrupt ($PROBE_AFTER)." + +run_uloop clear-pause-point --all > /dev/null +run_uloop control-play-mode --action Stop > /dev/null + +# --- Scenario 3: >6s pause → ReleaseAll while paused → capture intact → resume → Press succeeds --- +log "Scenario 3: interrupt, hold pause >6s, ReleaseAll while paused (captures intact), resume, Press must succeed..." +run_uloop control-play-mode --action Play > /dev/null +run_uloop enable-pause-point --file "$MARKER_FILE" --line "$MARKER_LINE" --timeout-seconds 60 > /dev/null +run_uloop simulate-keyboard --action Press --key Space --duration "$PRESS_DURATION" > "$RESULT_FILE" 2>&1 || true +INTERRUPTED3="$(jq -r '.InterruptedByPausePoint' "$RESULT_FILE")" +PAUSE_POINT_ID="$(jq -r '.PausePointId // empty' "$RESULT_FILE")" +if [ "$INTERRUPTED3" != "true" ]; then + log "FAIL scenario 3 setup: expected InterruptedByPausePoint=true" + cat "$RESULT_FILE" + exit 1 +fi + +sleep 7 + +# Why before Play: ReleaseAll must work while paused; that is the recovery path under test. +run_uloop simulate-keyboard --action ReleaseAll > "$RESULT_FILE" 2>&1 || true +RELEASE_SUCCESS="$(jq -r '.Success' "$RESULT_FILE")" +RELEASE_ACTION="$(jq -r '.Action' "$RESULT_FILE")" +if [ "$RELEASE_SUCCESS" != "true" ] || [ "$RELEASE_ACTION" != "ReleaseAll" ]; then + log "FAIL scenario 3: ReleaseAll while paused failed" + cat "$RESULT_FILE" + exit 1 +fi + +# Why: ReleaseAll must not clear pause-point capture holders used by TryGetCapturedValue. +run_uloop execute-dynamic-code --code " +using io.github.hatayama.UnityCliLoop.Runtime; +(bool found, object value) = UloopPausePoint.TryGetCapturedValue(\"this\"); +return \"captureFound=\" + found; +" > "$PROBE_FILE" +CAPTURE_RESULT="$(jq -r '.Result' "$PROBE_FILE")" +case "$CAPTURE_RESULT" in + *captureFound=True*|*captureFound=true*) ;; + *) + log "FAIL scenario 3: TryGetCapturedValue lost capture after paused ReleaseAll (got: $CAPTURE_RESULT; pausePointId=$PAUSE_POINT_ID)" + cat "$PROBE_FILE" + exit 1 + ;; +esac + +run_uloop control-play-mode --action Play > /dev/null + +run_uloop simulate-keyboard --action Press --key Space --duration 0.2 > "$RESULT_FILE" 2>&1 || true +PRESS_SUCCESS="$(jq -r '.Success' "$RESULT_FILE")" +PRESS_INTERRUPTED="$(jq -r '.InterruptedByPausePoint // false' "$RESULT_FILE")" +ALREADY_DOWN="$(jq -r '.PressEdgeKeyAlreadyPressedBeforeQueue // false' "$RESULT_FILE")" +if [ "$PRESS_SUCCESS" != "true" ] || [ "$PRESS_INTERRUPTED" = "true" ]; then + log "FAIL scenario 3: Press after ReleaseAll did not succeed cleanly" + cat "$RESULT_FILE" + exit 1 +fi +if [ "$ALREADY_DOWN" = "true" ]; then + log "FAIL scenario 3: Press still reported key already down after ReleaseAll" + cat "$RESULT_FILE" + exit 1 +fi + +log "PASS scenario 3: paused ReleaseAll kept captures and recovered key state; Press succeeded." +log "PASS: all key-state-after-pause-interruption scenarios." exit 0 From 2b5f370b729e12b99025e43aab55403724a4f93c Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Fri, 24 Jul 2026 03:23:11 +0900 Subject: [PATCH 3/5] docs: document paused-scenario workflow and TryGetCapturedValue signature in skills (#1966) Co-authored-by: Cursor --- .agents/skills/uloop-pause-point/SKILL.md | 38 +++++++++++++------ .../skills/uloop-simulate-keyboard/SKILL.md | 19 +++++++--- .claude/skills/uloop-pause-point/SKILL.md | 38 +++++++++++++------ .../skills/uloop-simulate-keyboard/SKILL.md | 19 +++++++--- .../CliOnlyTools~/PausePoint/Skill/SKILL.md | 38 +++++++++++++------ .../SimulateKeyboard/Skill/SKILL.md | 19 +++++++--- 6 files changed, 120 insertions(+), 51 deletions(-) diff --git a/.agents/skills/uloop-pause-point/SKILL.md b/.agents/skills/uloop-pause-point/SKILL.md index 23bba524d..667493fc6 100644 --- a/.agents/skills/uloop-pause-point/SKILL.md +++ b/.agents/skills/uloop-pause-point/SKILL.md @@ -15,7 +15,7 @@ Use this small loop for one representative frame you care about. No source edit uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seconds 30 --await --trigger "simulate-keyboard --action Press --key Space" ``` -`--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so there is no arm-vs-input race and nothing needs to run in the background. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. +`--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so there is no arm-vs-input race and nothing needs to run in the background. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. Both commands also accept `--resume-play` (requires `--await`) to resume a manually paused PlayMode after arm confirmation and before the trigger — see Fast-Progressing Games. When the game reaches the line on its own, omit `--trigger`. Fall back to split steps only when the triggering action is not a single uloop command (several inputs in sequence, an external event): run `enable-pause-point` without `--await` in the foreground (its response returning is the arm confirmation), then start `uloop await-pause-point --id ` in the background, then send the inputs. Do not approximate arm-waiting with a fixed sleep after a backgrounded enable. @@ -29,7 +29,7 @@ The response returns the derived marker `Id` (`Assets/Scripts/Enemy.cs:42`), the A hit pauses Unity at the next frame boundary — the patched method and the rest of that frame still run to completion. Only `CapturedVariables` is evidence of the values at the patched line; state read after the pause (for example via `execute-dynamic-code`) may already have advanced past it. Treat every post-hit live read as a supplement for follow-up digging — the primary evidence for what a value was at the paused line is always `CapturedVariables`. -If the game progresses on its own (timers, gravity, spawners), run `control-play-mode` `Pause` before setting up scenario state and resume with `control-play-mode --action Play` only after `enable-pause-point` succeeds — otherwise the scenario can be consumed before your input arrives. See Fast-Progressing Games below. +If the game progresses on its own (timers, gravity, spawners), freeze with `control-play-mode --action Pause` before setting up scenario state, then arm with `enable-pause-point --await --resume-play --trigger "..."` so the resume and the input happen in one call — see Fast-Progressing Games below. ## Capture Modes and History @@ -63,7 +63,15 @@ Every hit response embeds `CapturedVariables`: the method's in-scope locals, its - 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`. - Pass `--expect 'name=value'` (repeatable; on `await-pause-point` and `enable-pause-point --await`, not `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. - 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): it raises the element cap and scales the preview's character budget proportionally, so each element keeps the same ~100-character share it has at the default — plenty for numeric or boolean cells, but elements that are individually long can still be clipped by the scaled budget (`CapturedVariablesTruncated` tells you when that happened). The enable response echoes the effective `MaxPreviewElements`. -- While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; 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**: the hit's method ran to completion before the pause landed, so anything it changed — or destroyed — afterwards is already applied. A captured object that the method later passed to `Destroy()` reads as destroyed/null through this API even though `CapturedVariables` shows its pre-line field values intact. Never use live reads to reconstruct what a value was at the paused line; that is what the `CapturedVariables` snapshot is for. Use live reads only to dig further into objects that are still alive. +- While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; the holder clears on resume. (file:line marker hits only — id-only markers store no capture) The return type is the tuple `(bool Found, object Value)`, not the value itself — deconstruct it before use: + + ```csharp + (bool found, object value) = UloopPausePoint.TryGetCapturedValue("this"); + if (!found) { return "capture missing"; } + return value; + ``` + + These are **live objects in their frame-completed state, not snapshots**: the hit's method ran to completion before the pause landed, so anything it changed — or destroyed — afterwards is already applied. A captured object that the method later passed to `Destroy()` reads as destroyed/null through this API even though `CapturedVariables` shows its pre-line field values intact. Never use live reads to reconstruct what a value was at the paused line; that is what the `CapturedVariables` snapshot is for. Use live reads only to dig further into objects that are still alive. Before interpreting unexpected, missing, or truncated values, nested previews that render as type names, Unity-object `Value` strings, capture-time vs live evidence trade-offs, the hit response's `Warning`/`MatchingLogs` fields, marker freshness (`Generation`, `EnabledAtUtc`), or the raw capture API in detail, read [references/captured-variables.md](references/captured-variables.md). @@ -130,21 +138,29 @@ The `enable-pause-point --timeout-seconds` countdown freezes while a hit holds t ## Fast-Progressing Games -When the game advances on its own (a ball keeps bouncing, blocks keep falling), the state you are arranging can move past the target line before the input command and the wait are even issued. Pause the Editor and walk frames explicitly instead: +When the game advances on its own (timers, gravity, spawners, a ball that keeps bouncing, pieces that keep falling), any state you arrange with PlayMode live can be consumed by the game before your next command arrives — each CLI round-trip costs real seconds. Freeze first, build while paused, then resume and fire the input in one call: ```bash -# Freeze the whole player loop while arranging the scenario +# 1) Freeze the whole player loop before arranging anything uloop control-play-mode --action Pause -# ... enable pause points, inspect/arrange state with execute-dynamic-code, get-hierarchy, get-logs ... -# Advance exactly one frame and stay paused (the Editor's Next Frame button) -uloop control-play-mode --action Step -# Resume right before sending the input you are verifying (input simulation needs an unpaused player) -uloop control-play-mode --action Play + +# 2) While paused, build the exact scenario (production methods preferred; see below) +uloop execute-dynamic-code --code '...' + +# 3) One call: confirm the marker armed, resume PlayMode, fire the input, await the hit +uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seconds 60 \ + --await --resume-play --trigger "simulate-keyboard --action Press --key Space" ``` +`--resume-play` (requires `--await`; `await-pause-point` accepts it too) 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`). If the resume fails, the trigger is not dispatched and `TriggerResult.Error` says so. When the game reaches the line on its own after resuming (gravity, physics), omit `--trigger` and keep `--resume-play`. + +Size `--timeout-seconds` generously when arming while paused: a manual Pause does **not** freeze the marker lifetime countdown — only a pause owned by a pause-point hit does. + +This sequence closes the gap that used to require resuming and sending input as separate commands: post-resume drift from gravity or CharacterController motion no longer accumulates across round-trips, and input triggers no longer fail on a paused player, because the resume happens first. + Do not use `Time.timeScale = 0` for this: projects that read unscaled time keep advancing regardless, and the value silently persists into the next PlayMode session. Editor pause and `Step` freeze the entire player loop independent of `Time.timeScale`. -Pause and Step leave one residual race: input simulation needs an unpaused player, so the game runs freely between the final `--action Play` and your input landing. When a single command round-trip takes longer than the game's natural tick interval (for example a piece that auto-falls every 0.8 seconds), the tick fires before the input arrives no matter how the steps are ordered. Remove the race instead of trying to outrun it: temporarily overwrite the tick-interval field with `execute-dynamic-code` (for example set the fall interval to a very large value), run the verification, then restore the original value and confirm the restore with a re-read. +One residual race remains: after the resume, the game runs freely for the single in-process round-trip until the trigger input lands. When even that is longer than the game's natural tick interval (for example a piece that auto-falls every 0.8 seconds), remove the race instead of trying to outrun it: temporarily overwrite the tick-interval field with `execute-dynamic-code`, run the verification, then restore the original value and confirm the restore with a re-read. A pause point hit leaves Unity in this same paused state, so `Step` also works right after a hit: inspect the paused frame, then step forward to watch the following frames commit one at a time. diff --git a/.agents/skills/uloop-simulate-keyboard/SKILL.md b/.agents/skills/uloop-simulate-keyboard/SKILL.md index 9e6879ee5..8b9faf96d 100644 --- a/.agents/skills/uloop-simulate-keyboard/SKILL.md +++ b/.agents/skills/uloop-simulate-keyboard/SKILL.md @@ -26,9 +26,9 @@ uloop simulate-keyboard --action --key [options] | Parameter | Type | Default | Description | |-----------|------|---------|-------------| -| `--action` | enum | `Press` | `Press`, `KeyDown`, `KeyUp` | -| `--key` | string | (required) | 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`. | -| `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp. | +| `--action` | enum | `Press` | `Press`, `KeyDown`, `KeyUp`, `ReleaseAll` | +| `--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. | ### Actions @@ -37,6 +37,7 @@ uloop simulate-keyboard --action --key [options] | `Press` | KeyDown → wait → KeyUp | One-shot tap (jump, use item) | | `KeyDown` | KeyDown only (held until KeyUp) | Start continuous movement, hold sprint | | `KeyUp` | KeyUp only (release held key) | Stop movement, release sprint | +| `ReleaseAll` | Force-releases every tracked and device-pressed key (bookkeeping and Input System device state) | Recover a clean keyboard state after a pause-point interruption | There is no separate hold action: to hold a key, use `Press --duration ` (fixed-time hold) or `KeyDown` followed later by `KeyUp` (open-ended hold). @@ -44,9 +45,11 @@ Use `Press` for edge-triggered keyboard code such as `Keyboard.current.spaceKey. `KeyDown` emits one initial press edge, then only keeps the key held. It does not keep `wasPressedThisFrame` true while the key remains held. If a successful `Press` or `KeyDown` leaves `Keyboard.current..isPressed` true but runtime state does not change, do not immediately rewrite the user's runtime code to `isPressed`. First verify that the target component is active during the command, that it polls input in the configured Input System update phase, and that a missed `KeyDown` edge is followed by `KeyUp` before retrying. +`ReleaseAll` is a recovery action, not part of normal gameplay simulation: after a pause-point interruption, bookkeeping and the Input System device can disagree (`KeyUp` says the key is not held while a fresh `Press` reports it already down), or a stale press latch can keep `isPressed` true after resume. Run `uloop simulate-keyboard --action ReleaseAll` (no `--key`) to force both back to a clean slate — it also works while Unity is still paused for pause-point inspection, and it does not clear pause-point captures. `ReleasedKeys` in the response lists what was released. For ordinary releases during gameplay simulation, keep using `KeyUp`. + ### Pause Point Inspection (Standard for E2E) -For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused and input bookkeeping was safely released; `PressEdgeObserved` is still reported on pause-point interruptions. Interruption detection covers the whole press lifetime: a pause landing while `Press` is holding the key (during the duration wait) also returns promptly with `InterruptedByPausePoint: true`, and the pause takes precedence even when the requested duration had already elapsed — treat such a response as the pause reporting in, not as a delivery failure. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation. +For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused and input bookkeeping was safely released; `PressEdgeObserved` is still reported on pause-point interruptions. Interruption detection covers the whole press lifetime: a pause landing while `Press` is holding the key (during the duration wait) also returns promptly with `InterruptedByPausePoint: true`, and the pause takes precedence even when the requested duration had already elapsed — treat such a response as the pause reporting in, not as a delivery failure. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation. If a later key action still reports inconsistent state after an interruption, recover with `--action ReleaseAll` instead of retrying `KeyUp`. ### KeyDown/KeyUp Rules @@ -74,6 +77,9 @@ uloop simulate-keyboard --action KeyDown --key W uloop screenshot --capture-mode rendering uloop simulate-keyboard --action KeyUp --key W uloop simulate-keyboard --action KeyUp --key LeftShift + +# Recover a clean keyboard state (works while paused; e.g. after a pause-point interruption) +uloop simulate-keyboard --action ReleaseAll ``` ## Output @@ -82,14 +88,15 @@ Returns JSON with: - `Success` (boolean): Whether the action succeeded (e.g. `KeyDown` on a not-yet-held key, `KeyUp` on a currently-held key, or `Press` round-trip) - `Message` (string): Description of what happened or why it failed -- `Action` (string): The `--action` value that was applied (`Press`, `KeyDown`, or `KeyUp`) +- `Action` (string): The `--action` value that was applied (`Press`, `KeyDown`, `KeyUp`, or `ReleaseAll`) - `KeyName` (string, nullable): The key that was acted on; may be `null` when the action could not resolve a key +- `ReleasedKeys` (string list, nullable): Set only for `ReleaseAll`; the key names that were force-released (empty when nothing was held) - `InterruptedByPausePoint` / `PausePointId` / `PausePointHitCount` / `PausePointHits`: Pause-point interruption info (all nullable except the boolean). `PausePointHits` lists every marker hit during this input in hit order; `PausePointId` only names the latest one. See the Pause Point Inspection section above - `PressEdgeObserved` (boolean, nullable): For `Press` and `KeyDown`, whether the press edge (`wasPressedThisFrame`) was actually visible inside a gameplay input update. `false` means the CLI succeeded but gameplay polling most likely missed the edge (e.g. the press was consumed by an editor-only input update) — retry the input or verify with a focused log instead of trusting `Success` alone. `null` only for `KeyUp` and for timed-out responses; pause-point interruptions still report the observed value. When a single-shot pause point is armed, do not blindly retry on `PressEdgeObserved=false` — the input may still have registered late. Check `pause-point-status` for a hit first, so a retry does not consume a re-enabled marker or double-fire the scenario - `PressEdgeConsumedByUpdateType` / `PressEdgeAnyDynamicUpdateObserved` / `PressEdgeKeyAlreadyPressedBeforeQueue` (nullable): Diagnostics populated only when `PressEdgeObserved` is `false` (all `null` when the edge was observed). Read them before retrying: `PressEdgeKeyAlreadyPressedBeforeQueue=true` means the key was already held so no press transition could occur — release it with `KeyUp` before pressing again; `PressEdgeConsumedByUpdateType` naming a non-`Dynamic` update type (for example `Editor`) means an editor-side update consumed the edge before gameplay polling could see it; `PressEdgeAnyDynamicUpdateObserved=false` means no gameplay input update ran at all during the press window, so check that PlayMode is running and unpaused rather than retrying blindly. `Message` carries the same diagnosis as text ## Prerequisites -- Unity must be in **PlayMode** +- Unity must be in **PlayMode**. `Press`/`KeyDown`/`KeyUp` additionally require PlayMode to be unpaused; `ReleaseAll` is allowed while paused. - **Input System package** (`com.unity.inputsystem`) must be installed; this tool only works with the New Input System. - Game code must read input via Input System API (e.g. `Keyboard.current[Key.W].isPressed`), not legacy `Input.GetKey()` diff --git a/.claude/skills/uloop-pause-point/SKILL.md b/.claude/skills/uloop-pause-point/SKILL.md index 23bba524d..667493fc6 100644 --- a/.claude/skills/uloop-pause-point/SKILL.md +++ b/.claude/skills/uloop-pause-point/SKILL.md @@ -15,7 +15,7 @@ Use this small loop for one representative frame you care about. No source edit uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seconds 30 --await --trigger "simulate-keyboard --action Press --key Space" ``` -`--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so there is no arm-vs-input race and nothing needs to run in the background. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. +`--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so there is no arm-vs-input race and nothing needs to run in the background. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. Both commands also accept `--resume-play` (requires `--await`) to resume a manually paused PlayMode after arm confirmation and before the trigger — see Fast-Progressing Games. When the game reaches the line on its own, omit `--trigger`. Fall back to split steps only when the triggering action is not a single uloop command (several inputs in sequence, an external event): run `enable-pause-point` without `--await` in the foreground (its response returning is the arm confirmation), then start `uloop await-pause-point --id ` in the background, then send the inputs. Do not approximate arm-waiting with a fixed sleep after a backgrounded enable. @@ -29,7 +29,7 @@ The response returns the derived marker `Id` (`Assets/Scripts/Enemy.cs:42`), the A hit pauses Unity at the next frame boundary — the patched method and the rest of that frame still run to completion. Only `CapturedVariables` is evidence of the values at the patched line; state read after the pause (for example via `execute-dynamic-code`) may already have advanced past it. Treat every post-hit live read as a supplement for follow-up digging — the primary evidence for what a value was at the paused line is always `CapturedVariables`. -If the game progresses on its own (timers, gravity, spawners), run `control-play-mode` `Pause` before setting up scenario state and resume with `control-play-mode --action Play` only after `enable-pause-point` succeeds — otherwise the scenario can be consumed before your input arrives. See Fast-Progressing Games below. +If the game progresses on its own (timers, gravity, spawners), freeze with `control-play-mode --action Pause` before setting up scenario state, then arm with `enable-pause-point --await --resume-play --trigger "..."` so the resume and the input happen in one call — see Fast-Progressing Games below. ## Capture Modes and History @@ -63,7 +63,15 @@ Every hit response embeds `CapturedVariables`: the method's in-scope locals, its - 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`. - Pass `--expect 'name=value'` (repeatable; on `await-pause-point` and `enable-pause-point --await`, not `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. - 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): it raises the element cap and scales the preview's character budget proportionally, so each element keeps the same ~100-character share it has at the default — plenty for numeric or boolean cells, but elements that are individually long can still be clipped by the scaled budget (`CapturedVariablesTruncated` tells you when that happened). The enable response echoes the effective `MaxPreviewElements`. -- While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; 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**: the hit's method ran to completion before the pause landed, so anything it changed — or destroyed — afterwards is already applied. A captured object that the method later passed to `Destroy()` reads as destroyed/null through this API even though `CapturedVariables` shows its pre-line field values intact. Never use live reads to reconstruct what a value was at the paused line; that is what the `CapturedVariables` snapshot is for. Use live reads only to dig further into objects that are still alive. +- While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; the holder clears on resume. (file:line marker hits only — id-only markers store no capture) The return type is the tuple `(bool Found, object Value)`, not the value itself — deconstruct it before use: + + ```csharp + (bool found, object value) = UloopPausePoint.TryGetCapturedValue("this"); + if (!found) { return "capture missing"; } + return value; + ``` + + These are **live objects in their frame-completed state, not snapshots**: the hit's method ran to completion before the pause landed, so anything it changed — or destroyed — afterwards is already applied. A captured object that the method later passed to `Destroy()` reads as destroyed/null through this API even though `CapturedVariables` shows its pre-line field values intact. Never use live reads to reconstruct what a value was at the paused line; that is what the `CapturedVariables` snapshot is for. Use live reads only to dig further into objects that are still alive. Before interpreting unexpected, missing, or truncated values, nested previews that render as type names, Unity-object `Value` strings, capture-time vs live evidence trade-offs, the hit response's `Warning`/`MatchingLogs` fields, marker freshness (`Generation`, `EnabledAtUtc`), or the raw capture API in detail, read [references/captured-variables.md](references/captured-variables.md). @@ -130,21 +138,29 @@ The `enable-pause-point --timeout-seconds` countdown freezes while a hit holds t ## Fast-Progressing Games -When the game advances on its own (a ball keeps bouncing, blocks keep falling), the state you are arranging can move past the target line before the input command and the wait are even issued. Pause the Editor and walk frames explicitly instead: +When the game advances on its own (timers, gravity, spawners, a ball that keeps bouncing, pieces that keep falling), any state you arrange with PlayMode live can be consumed by the game before your next command arrives — each CLI round-trip costs real seconds. Freeze first, build while paused, then resume and fire the input in one call: ```bash -# Freeze the whole player loop while arranging the scenario +# 1) Freeze the whole player loop before arranging anything uloop control-play-mode --action Pause -# ... enable pause points, inspect/arrange state with execute-dynamic-code, get-hierarchy, get-logs ... -# Advance exactly one frame and stay paused (the Editor's Next Frame button) -uloop control-play-mode --action Step -# Resume right before sending the input you are verifying (input simulation needs an unpaused player) -uloop control-play-mode --action Play + +# 2) While paused, build the exact scenario (production methods preferred; see below) +uloop execute-dynamic-code --code '...' + +# 3) One call: confirm the marker armed, resume PlayMode, fire the input, await the hit +uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seconds 60 \ + --await --resume-play --trigger "simulate-keyboard --action Press --key Space" ``` +`--resume-play` (requires `--await`; `await-pause-point` accepts it too) 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`). If the resume fails, the trigger is not dispatched and `TriggerResult.Error` says so. When the game reaches the line on its own after resuming (gravity, physics), omit `--trigger` and keep `--resume-play`. + +Size `--timeout-seconds` generously when arming while paused: a manual Pause does **not** freeze the marker lifetime countdown — only a pause owned by a pause-point hit does. + +This sequence closes the gap that used to require resuming and sending input as separate commands: post-resume drift from gravity or CharacterController motion no longer accumulates across round-trips, and input triggers no longer fail on a paused player, because the resume happens first. + Do not use `Time.timeScale = 0` for this: projects that read unscaled time keep advancing regardless, and the value silently persists into the next PlayMode session. Editor pause and `Step` freeze the entire player loop independent of `Time.timeScale`. -Pause and Step leave one residual race: input simulation needs an unpaused player, so the game runs freely between the final `--action Play` and your input landing. When a single command round-trip takes longer than the game's natural tick interval (for example a piece that auto-falls every 0.8 seconds), the tick fires before the input arrives no matter how the steps are ordered. Remove the race instead of trying to outrun it: temporarily overwrite the tick-interval field with `execute-dynamic-code` (for example set the fall interval to a very large value), run the verification, then restore the original value and confirm the restore with a re-read. +One residual race remains: after the resume, the game runs freely for the single in-process round-trip until the trigger input lands. When even that is longer than the game's natural tick interval (for example a piece that auto-falls every 0.8 seconds), remove the race instead of trying to outrun it: temporarily overwrite the tick-interval field with `execute-dynamic-code`, run the verification, then restore the original value and confirm the restore with a re-read. A pause point hit leaves Unity in this same paused state, so `Step` also works right after a hit: inspect the paused frame, then step forward to watch the following frames commit one at a time. diff --git a/.claude/skills/uloop-simulate-keyboard/SKILL.md b/.claude/skills/uloop-simulate-keyboard/SKILL.md index 9e6879ee5..8b9faf96d 100644 --- a/.claude/skills/uloop-simulate-keyboard/SKILL.md +++ b/.claude/skills/uloop-simulate-keyboard/SKILL.md @@ -26,9 +26,9 @@ uloop simulate-keyboard --action --key [options] | Parameter | Type | Default | Description | |-----------|------|---------|-------------| -| `--action` | enum | `Press` | `Press`, `KeyDown`, `KeyUp` | -| `--key` | string | (required) | 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`. | -| `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp. | +| `--action` | enum | `Press` | `Press`, `KeyDown`, `KeyUp`, `ReleaseAll` | +| `--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. | ### Actions @@ -37,6 +37,7 @@ uloop simulate-keyboard --action --key [options] | `Press` | KeyDown → wait → KeyUp | One-shot tap (jump, use item) | | `KeyDown` | KeyDown only (held until KeyUp) | Start continuous movement, hold sprint | | `KeyUp` | KeyUp only (release held key) | Stop movement, release sprint | +| `ReleaseAll` | Force-releases every tracked and device-pressed key (bookkeeping and Input System device state) | Recover a clean keyboard state after a pause-point interruption | There is no separate hold action: to hold a key, use `Press --duration ` (fixed-time hold) or `KeyDown` followed later by `KeyUp` (open-ended hold). @@ -44,9 +45,11 @@ Use `Press` for edge-triggered keyboard code such as `Keyboard.current.spaceKey. `KeyDown` emits one initial press edge, then only keeps the key held. It does not keep `wasPressedThisFrame` true while the key remains held. If a successful `Press` or `KeyDown` leaves `Keyboard.current..isPressed` true but runtime state does not change, do not immediately rewrite the user's runtime code to `isPressed`. First verify that the target component is active during the command, that it polls input in the configured Input System update phase, and that a missed `KeyDown` edge is followed by `KeyUp` before retrying. +`ReleaseAll` is a recovery action, not part of normal gameplay simulation: after a pause-point interruption, bookkeeping and the Input System device can disagree (`KeyUp` says the key is not held while a fresh `Press` reports it already down), or a stale press latch can keep `isPressed` true after resume. Run `uloop simulate-keyboard --action ReleaseAll` (no `--key`) to force both back to a clean slate — it also works while Unity is still paused for pause-point inspection, and it does not clear pause-point captures. `ReleasedKeys` in the response lists what was released. For ordinary releases during gameplay simulation, keep using `KeyUp`. + ### Pause Point Inspection (Standard for E2E) -For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused and input bookkeeping was safely released; `PressEdgeObserved` is still reported on pause-point interruptions. Interruption detection covers the whole press lifetime: a pause landing while `Press` is holding the key (during the duration wait) also returns promptly with `InterruptedByPausePoint: true`, and the pause takes precedence even when the requested duration had already elapsed — treat such a response as the pause reporting in, not as a delivery failure. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation. +For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused and input bookkeeping was safely released; `PressEdgeObserved` is still reported on pause-point interruptions. Interruption detection covers the whole press lifetime: a pause landing while `Press` is holding the key (during the duration wait) also returns promptly with `InterruptedByPausePoint: true`, and the pause takes precedence even when the requested duration had already elapsed — treat such a response as the pause reporting in, not as a delivery failure. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation. If a later key action still reports inconsistent state after an interruption, recover with `--action ReleaseAll` instead of retrying `KeyUp`. ### KeyDown/KeyUp Rules @@ -74,6 +77,9 @@ uloop simulate-keyboard --action KeyDown --key W uloop screenshot --capture-mode rendering uloop simulate-keyboard --action KeyUp --key W uloop simulate-keyboard --action KeyUp --key LeftShift + +# Recover a clean keyboard state (works while paused; e.g. after a pause-point interruption) +uloop simulate-keyboard --action ReleaseAll ``` ## Output @@ -82,14 +88,15 @@ Returns JSON with: - `Success` (boolean): Whether the action succeeded (e.g. `KeyDown` on a not-yet-held key, `KeyUp` on a currently-held key, or `Press` round-trip) - `Message` (string): Description of what happened or why it failed -- `Action` (string): The `--action` value that was applied (`Press`, `KeyDown`, or `KeyUp`) +- `Action` (string): The `--action` value that was applied (`Press`, `KeyDown`, `KeyUp`, or `ReleaseAll`) - `KeyName` (string, nullable): The key that was acted on; may be `null` when the action could not resolve a key +- `ReleasedKeys` (string list, nullable): Set only for `ReleaseAll`; the key names that were force-released (empty when nothing was held) - `InterruptedByPausePoint` / `PausePointId` / `PausePointHitCount` / `PausePointHits`: Pause-point interruption info (all nullable except the boolean). `PausePointHits` lists every marker hit during this input in hit order; `PausePointId` only names the latest one. See the Pause Point Inspection section above - `PressEdgeObserved` (boolean, nullable): For `Press` and `KeyDown`, whether the press edge (`wasPressedThisFrame`) was actually visible inside a gameplay input update. `false` means the CLI succeeded but gameplay polling most likely missed the edge (e.g. the press was consumed by an editor-only input update) — retry the input or verify with a focused log instead of trusting `Success` alone. `null` only for `KeyUp` and for timed-out responses; pause-point interruptions still report the observed value. When a single-shot pause point is armed, do not blindly retry on `PressEdgeObserved=false` — the input may still have registered late. Check `pause-point-status` for a hit first, so a retry does not consume a re-enabled marker or double-fire the scenario - `PressEdgeConsumedByUpdateType` / `PressEdgeAnyDynamicUpdateObserved` / `PressEdgeKeyAlreadyPressedBeforeQueue` (nullable): Diagnostics populated only when `PressEdgeObserved` is `false` (all `null` when the edge was observed). Read them before retrying: `PressEdgeKeyAlreadyPressedBeforeQueue=true` means the key was already held so no press transition could occur — release it with `KeyUp` before pressing again; `PressEdgeConsumedByUpdateType` naming a non-`Dynamic` update type (for example `Editor`) means an editor-side update consumed the edge before gameplay polling could see it; `PressEdgeAnyDynamicUpdateObserved=false` means no gameplay input update ran at all during the press window, so check that PlayMode is running and unpaused rather than retrying blindly. `Message` carries the same diagnosis as text ## Prerequisites -- Unity must be in **PlayMode** +- Unity must be in **PlayMode**. `Press`/`KeyDown`/`KeyUp` additionally require PlayMode to be unpaused; `ReleaseAll` is allowed while paused. - **Input System package** (`com.unity.inputsystem`) must be installed; this tool only works with the New Input System. - Game code must read input via Input System API (e.g. `Keyboard.current[Key.W].isPressed`), not legacy `Input.GetKey()` diff --git a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md index 23bba524d..667493fc6 100644 --- a/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md +++ b/Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md @@ -15,7 +15,7 @@ Use this small loop for one representative frame you care about. No source edit uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seconds 30 --await --trigger "simulate-keyboard --action Press --key Space" ``` -`--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so there is no arm-vs-input race and nothing needs to run in the background. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. +`--trigger` runs a single uloop subcommand in-process only after the marker's arming is confirmed, so there is no arm-vs-input race and nothing needs to run in the background. The hit response additionally carries `TriggerResult` with the triggered command's own response (or, when the trigger was skipped, `Completed: false` and the reason in `Error`). The trigger string cannot name another pause-point wait (`await-pause-point`/`enable-pause-point`) and cannot pass `--project-path` — the enclosing command's project is used. `await-pause-point --id --trigger ...` accepts the same flag for a marker enabled earlier. Both commands also accept `--resume-play` (requires `--await`) to resume a manually paused PlayMode after arm confirmation and before the trigger — see Fast-Progressing Games. When the game reaches the line on its own, omit `--trigger`. Fall back to split steps only when the triggering action is not a single uloop command (several inputs in sequence, an external event): run `enable-pause-point` without `--await` in the foreground (its response returning is the arm confirmation), then start `uloop await-pause-point --id ` in the background, then send the inputs. Do not approximate arm-waiting with a fixed sleep after a backgrounded enable. @@ -29,7 +29,7 @@ The response returns the derived marker `Id` (`Assets/Scripts/Enemy.cs:42`), the A hit pauses Unity at the next frame boundary — the patched method and the rest of that frame still run to completion. Only `CapturedVariables` is evidence of the values at the patched line; state read after the pause (for example via `execute-dynamic-code`) may already have advanced past it. Treat every post-hit live read as a supplement for follow-up digging — the primary evidence for what a value was at the paused line is always `CapturedVariables`. -If the game progresses on its own (timers, gravity, spawners), run `control-play-mode` `Pause` before setting up scenario state and resume with `control-play-mode --action Play` only after `enable-pause-point` succeeds — otherwise the scenario can be consumed before your input arrives. See Fast-Progressing Games below. +If the game progresses on its own (timers, gravity, spawners), freeze with `control-play-mode --action Pause` before setting up scenario state, then arm with `enable-pause-point --await --resume-play --trigger "..."` so the resume and the input happen in one call — see Fast-Progressing Games below. ## Capture Modes and History @@ -63,7 +63,15 @@ Every hit response embeds `CapturedVariables`: the method's in-scope locals, its - 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`. - Pass `--expect 'name=value'` (repeatable; on `await-pause-point` and `enable-pause-point --await`, not `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. - 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): it raises the element cap and scales the preview's character budget proportionally, so each element keeps the same ~100-character share it has at the default — plenty for numeric or boolean cells, but elements that are individually long can still be clipped by the scaled budget (`CapturedVariablesTruncated` tells you when that happened). The enable response echoes the effective `MaxPreviewElements`. -- While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; 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**: the hit's method ran to completion before the pause landed, so anything it changed — or destroyed — afterwards is already applied. A captured object that the method later passed to `Destroy()` reads as destroyed/null through this API even though `CapturedVariables` shows its pre-line field values intact. Never use live reads to reconstruct what a value was at the paused line; that is what the `CapturedVariables` snapshot is for. Use live reads only to dig further into objects that are still alive. +- While Unity is still paused, `UloopPausePoint.TryGetCapturedValue("name")` (and `"this"`) returns live captured references for `execute-dynamic-code`; the holder clears on resume. (file:line marker hits only — id-only markers store no capture) The return type is the tuple `(bool Found, object Value)`, not the value itself — deconstruct it before use: + + ```csharp + (bool found, object value) = UloopPausePoint.TryGetCapturedValue("this"); + if (!found) { return "capture missing"; } + return value; + ``` + + These are **live objects in their frame-completed state, not snapshots**: the hit's method ran to completion before the pause landed, so anything it changed — or destroyed — afterwards is already applied. A captured object that the method later passed to `Destroy()` reads as destroyed/null through this API even though `CapturedVariables` shows its pre-line field values intact. Never use live reads to reconstruct what a value was at the paused line; that is what the `CapturedVariables` snapshot is for. Use live reads only to dig further into objects that are still alive. Before interpreting unexpected, missing, or truncated values, nested previews that render as type names, Unity-object `Value` strings, capture-time vs live evidence trade-offs, the hit response's `Warning`/`MatchingLogs` fields, marker freshness (`Generation`, `EnabledAtUtc`), or the raw capture API in detail, read [references/captured-variables.md](references/captured-variables.md). @@ -130,21 +138,29 @@ The `enable-pause-point --timeout-seconds` countdown freezes while a hit holds t ## Fast-Progressing Games -When the game advances on its own (a ball keeps bouncing, blocks keep falling), the state you are arranging can move past the target line before the input command and the wait are even issued. Pause the Editor and walk frames explicitly instead: +When the game advances on its own (timers, gravity, spawners, a ball that keeps bouncing, pieces that keep falling), any state you arrange with PlayMode live can be consumed by the game before your next command arrives — each CLI round-trip costs real seconds. Freeze first, build while paused, then resume and fire the input in one call: ```bash -# Freeze the whole player loop while arranging the scenario +# 1) Freeze the whole player loop before arranging anything uloop control-play-mode --action Pause -# ... enable pause points, inspect/arrange state with execute-dynamic-code, get-hierarchy, get-logs ... -# Advance exactly one frame and stay paused (the Editor's Next Frame button) -uloop control-play-mode --action Step -# Resume right before sending the input you are verifying (input simulation needs an unpaused player) -uloop control-play-mode --action Play + +# 2) While paused, build the exact scenario (production methods preferred; see below) +uloop execute-dynamic-code --code '...' + +# 3) One call: confirm the marker armed, resume PlayMode, fire the input, await the hit +uloop enable-pause-point --file Assets/Scripts/Enemy.cs --line 42 --timeout-seconds 60 \ + --await --resume-play --trigger "simulate-keyboard --action Press --key Space" ``` +`--resume-play` (requires `--await`; `await-pause-point` accepts it too) 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`). If the resume fails, the trigger is not dispatched and `TriggerResult.Error` says so. When the game reaches the line on its own after resuming (gravity, physics), omit `--trigger` and keep `--resume-play`. + +Size `--timeout-seconds` generously when arming while paused: a manual Pause does **not** freeze the marker lifetime countdown — only a pause owned by a pause-point hit does. + +This sequence closes the gap that used to require resuming and sending input as separate commands: post-resume drift from gravity or CharacterController motion no longer accumulates across round-trips, and input triggers no longer fail on a paused player, because the resume happens first. + Do not use `Time.timeScale = 0` for this: projects that read unscaled time keep advancing regardless, and the value silently persists into the next PlayMode session. Editor pause and `Step` freeze the entire player loop independent of `Time.timeScale`. -Pause and Step leave one residual race: input simulation needs an unpaused player, so the game runs freely between the final `--action Play` and your input landing. When a single command round-trip takes longer than the game's natural tick interval (for example a piece that auto-falls every 0.8 seconds), the tick fires before the input arrives no matter how the steps are ordered. Remove the race instead of trying to outrun it: temporarily overwrite the tick-interval field with `execute-dynamic-code` (for example set the fall interval to a very large value), run the verification, then restore the original value and confirm the restore with a re-read. +One residual race remains: after the resume, the game runs freely for the single in-process round-trip until the trigger input lands. When even that is longer than the game's natural tick interval (for example a piece that auto-falls every 0.8 seconds), remove the race instead of trying to outrun it: temporarily overwrite the tick-interval field with `execute-dynamic-code`, run the verification, then restore the original value and confirm the restore with a re-read. A pause point hit leaves Unity in this same paused state, so `Step` also works right after a hit: inspect the paused frame, then step forward to watch the following frames commit one at a time. diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md index 9e6879ee5..8b9faf96d 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md @@ -26,9 +26,9 @@ uloop simulate-keyboard --action --key [options] | Parameter | Type | Default | Description | |-----------|------|---------|-------------| -| `--action` | enum | `Press` | `Press`, `KeyDown`, `KeyUp` | -| `--key` | string | (required) | 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`. | -| `--duration` | number | `0` | Hold duration in seconds for Press action (0 = one-shot tap). Ignored by KeyDown/KeyUp. | +| `--action` | enum | `Press` | `Press`, `KeyDown`, `KeyUp`, `ReleaseAll` | +| `--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. | ### Actions @@ -37,6 +37,7 @@ uloop simulate-keyboard --action --key [options] | `Press` | KeyDown → wait → KeyUp | One-shot tap (jump, use item) | | `KeyDown` | KeyDown only (held until KeyUp) | Start continuous movement, hold sprint | | `KeyUp` | KeyUp only (release held key) | Stop movement, release sprint | +| `ReleaseAll` | Force-releases every tracked and device-pressed key (bookkeeping and Input System device state) | Recover a clean keyboard state after a pause-point interruption | There is no separate hold action: to hold a key, use `Press --duration ` (fixed-time hold) or `KeyDown` followed later by `KeyUp` (open-ended hold). @@ -44,9 +45,11 @@ Use `Press` for edge-triggered keyboard code such as `Keyboard.current.spaceKey. `KeyDown` emits one initial press edge, then only keeps the key held. It does not keep `wasPressedThisFrame` true while the key remains held. If a successful `Press` or `KeyDown` leaves `Keyboard.current..isPressed` true but runtime state does not change, do not immediately rewrite the user's runtime code to `isPressed`. First verify that the target component is active during the command, that it polls input in the configured Input System update phase, and that a missed `KeyDown` edge is followed by `KeyUp` before retrying. +`ReleaseAll` is a recovery action, not part of normal gameplay simulation: after a pause-point interruption, bookkeeping and the Input System device can disagree (`KeyUp` says the key is not held while a fresh `Press` reports it already down), or a stale press latch can keep `isPressed` true after resume. Run `uloop simulate-keyboard --action ReleaseAll` (no `--key`) to force both back to a clean slate — it also works while Unity is still paused for pause-point inspection, and it does not clear pause-point captures. `ReleasedKeys` in the response lists what was released. For ordinary releases during gameplay simulation, keep using `KeyUp`. + ### Pause Point Inspection (Standard for E2E) -For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused and input bookkeeping was safely released; `PressEdgeObserved` is still reported on pause-point interruptions. Interruption detection covers the whole press lifetime: a pause landing while `Press` is holding the key (during the duration wait) also returns promptly with `InterruptedByPausePoint: true`, and the pause takes precedence even when the requested duration had already elapsed — treat such a response as the pause reporting in, not as a delivery failure. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation. +For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused and input bookkeeping was safely released; `PressEdgeObserved` is still reported on pause-point interruptions. Interruption detection covers the whole press lifetime: a pause landing while `Press` is holding the key (during the duration wait) also returns promptly with `InterruptedByPausePoint: true`, and the pause takes precedence even when the requested duration had already elapsed — treat such a response as the pause reporting in, not as a delivery failure. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation. If a later key action still reports inconsistent state after an interruption, recover with `--action ReleaseAll` instead of retrying `KeyUp`. ### KeyDown/KeyUp Rules @@ -74,6 +77,9 @@ uloop simulate-keyboard --action KeyDown --key W uloop screenshot --capture-mode rendering uloop simulate-keyboard --action KeyUp --key W uloop simulate-keyboard --action KeyUp --key LeftShift + +# Recover a clean keyboard state (works while paused; e.g. after a pause-point interruption) +uloop simulate-keyboard --action ReleaseAll ``` ## Output @@ -82,14 +88,15 @@ Returns JSON with: - `Success` (boolean): Whether the action succeeded (e.g. `KeyDown` on a not-yet-held key, `KeyUp` on a currently-held key, or `Press` round-trip) - `Message` (string): Description of what happened or why it failed -- `Action` (string): The `--action` value that was applied (`Press`, `KeyDown`, or `KeyUp`) +- `Action` (string): The `--action` value that was applied (`Press`, `KeyDown`, `KeyUp`, or `ReleaseAll`) - `KeyName` (string, nullable): The key that was acted on; may be `null` when the action could not resolve a key +- `ReleasedKeys` (string list, nullable): Set only for `ReleaseAll`; the key names that were force-released (empty when nothing was held) - `InterruptedByPausePoint` / `PausePointId` / `PausePointHitCount` / `PausePointHits`: Pause-point interruption info (all nullable except the boolean). `PausePointHits` lists every marker hit during this input in hit order; `PausePointId` only names the latest one. See the Pause Point Inspection section above - `PressEdgeObserved` (boolean, nullable): For `Press` and `KeyDown`, whether the press edge (`wasPressedThisFrame`) was actually visible inside a gameplay input update. `false` means the CLI succeeded but gameplay polling most likely missed the edge (e.g. the press was consumed by an editor-only input update) — retry the input or verify with a focused log instead of trusting `Success` alone. `null` only for `KeyUp` and for timed-out responses; pause-point interruptions still report the observed value. When a single-shot pause point is armed, do not blindly retry on `PressEdgeObserved=false` — the input may still have registered late. Check `pause-point-status` for a hit first, so a retry does not consume a re-enabled marker or double-fire the scenario - `PressEdgeConsumedByUpdateType` / `PressEdgeAnyDynamicUpdateObserved` / `PressEdgeKeyAlreadyPressedBeforeQueue` (nullable): Diagnostics populated only when `PressEdgeObserved` is `false` (all `null` when the edge was observed). Read them before retrying: `PressEdgeKeyAlreadyPressedBeforeQueue=true` means the key was already held so no press transition could occur — release it with `KeyUp` before pressing again; `PressEdgeConsumedByUpdateType` naming a non-`Dynamic` update type (for example `Editor`) means an editor-side update consumed the edge before gameplay polling could see it; `PressEdgeAnyDynamicUpdateObserved=false` means no gameplay input update ran at all during the press window, so check that PlayMode is running and unpaused rather than retrying blindly. `Message` carries the same diagnosis as text ## Prerequisites -- Unity must be in **PlayMode** +- Unity must be in **PlayMode**. `Press`/`KeyDown`/`KeyUp` additionally require PlayMode to be unpaused; `ReleaseAll` is allowed while paused. - **Input System package** (`com.unity.inputsystem`) must be installed; this tool only works with the New Input System. - Game code must read input via Input System API (e.g. `Keyboard.current[Key.W].isPressed`), not legacy `Input.GetKey()` From c118b8de0b337b58a7d003c7b23eedb098bb2600 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 24 Jul 2026 03:40:39 +0900 Subject: [PATCH 4/5] Address CodeRabbit findings on pause-point round 9 Cancel the apply timeout CTS on the timeout path so the pause watcher stops instead of polling a disposed token. Clarify ReleaseAll synopsis and cover the combined not-armed resume+trigger skip path. Co-authored-by: Cursor --- .../skills/uloop-simulate-keyboard/SKILL.md | 3 +- .../skills/uloop-simulate-keyboard/SKILL.md | 3 +- .../InputSystemConfiguredUpdateApplier.cs | 2 + .../SimulateKeyboard/Skill/SKILL.md | 3 +- .../pause_point_resume_play_test.go | 72 +++++++++++++++++++ 5 files changed, 80 insertions(+), 3 deletions(-) diff --git a/.agents/skills/uloop-simulate-keyboard/SKILL.md b/.agents/skills/uloop-simulate-keyboard/SKILL.md index 8b9faf96d..23ec638ad 100644 --- a/.agents/skills/uloop-simulate-keyboard/SKILL.md +++ b/.agents/skills/uloop-simulate-keyboard/SKILL.md @@ -19,7 +19,8 @@ Simulate keyboard input on Unity PlayMode. ## Tool Reference ```bash -uloop simulate-keyboard --action --key [options] +uloop simulate-keyboard --action --key [options] +uloop simulate-keyboard --action ReleaseAll ``` ### Parameters diff --git a/.claude/skills/uloop-simulate-keyboard/SKILL.md b/.claude/skills/uloop-simulate-keyboard/SKILL.md index 8b9faf96d..23ec638ad 100644 --- a/.claude/skills/uloop-simulate-keyboard/SKILL.md +++ b/.claude/skills/uloop-simulate-keyboard/SKILL.md @@ -19,7 +19,8 @@ Simulate keyboard input on Unity PlayMode. ## Tool Reference ```bash -uloop simulate-keyboard --action --key [options] +uloop simulate-keyboard --action --key [options] +uloop simulate-keyboard --action ReleaseAll ``` ### Parameters diff --git a/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs b/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs index 837031757..af8714b52 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs @@ -138,6 +138,8 @@ void TryDiscardForPause() if (completedTask == timeoutTask) { await timeoutTask.ConfigureAwait(false); + // Why: cancel before scope-exit dispose so the pause watcher exits instead of polling a disposed token. + timeoutCts.Cancel(); if (Interlocked.CompareExchange( ref applyWaitState, ApplyWaitStateFinishedWithoutApply, diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md index 8b9faf96d..23ec638ad 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md @@ -19,7 +19,8 @@ Simulate keyboard input on Unity PlayMode. ## Tool Reference ```bash -uloop simulate-keyboard --action --key [options] +uloop simulate-keyboard --action --key [options] +uloop simulate-keyboard --action ReleaseAll ``` ### Parameters 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 26c5faa65..5eb4124ab 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 @@ -351,6 +351,78 @@ func TestWaitForPausePointSkipsResumeWhenNotArmed(t *testing.T) { } } +// Verifies not-armed markers leave both ResumePlayResult.Error and TriggerResult.Error set when +// --resume-play and --trigger are requested together, without dispatching either action. +func TestWaitForPausePointSkipsResumeAndTriggerWhenNotArmed(t *testing.T) { + originalQuery := queryPausePointStatus + originalDispatch := dispatchPausePointTriggerCommand + originalResume := resumePlayModeForPausePoint + defer func() { + queryPausePointStatus = originalQuery + dispatchPausePointTriggerCommand = originalDispatch + resumePlayModeForPausePoint = originalResume + }() + + queryPausePointStatus = func( + ctx context.Context, + connection unityipc.Connection, + id string, + ) (pausePointStatusResponse, error) { + return pausePointStatusResponse{Id: id, Status: pausePointStatusNotEnabled}, nil + } + + resumeCalled := false + resumePlayModeForPausePoint = func( + ctx context.Context, + connection unityipc.Connection, + ) pausePointResumePlayResult { + resumeCalled = true + return pausePointResumePlayResult{WasPaused: true, Resumed: true} + } + + dispatchCalled := false + dispatchPausePointTriggerCommand = func( + ctx context.Context, + connection unityipc.Connection, + command string, + commandArgs []string, + startPath string, + stdout io.Writer, + stderr io.Writer, + ) int { + dispatchCalled = true + _, _ = stdout.Write([]byte(`{"Success":true}`)) + return 0 + } + + _, state, triggerResult, resumeResult, err := waitForPausePoint(context.Background(), unityipc.Connection{}, waitForPausePointOptions{ + id: "does-not-exist", + timeoutSeconds: 1, + timeout: time.Second, + resumePlay: true, + triggerCommand: "simulate-keyboard", + triggerArgs: []string{"--action", "Press"}, + }) + if err != nil { + t.Fatalf("waitForPausePoint failed: %v", err) + } + if state != pausePointWaitStateNotEnabled { + t.Fatalf("expected not_enabled state, got %q", state) + } + if resumeCalled { + t.Fatal("expected resumePlayModeForPausePoint not to be called for a not-armed marker") + } + if dispatchCalled { + t.Fatal("expected dispatchPausePointTriggerCommand not to be called for a not-armed marker") + } + if resumeResult == nil || resumeResult.Error == "" { + t.Fatalf("expected ResumePlayResult.Error explaining the skip, got %#v", resumeResult) + } + if triggerResult == nil || triggerResult.Error == "" { + t.Fatalf("expected TriggerResult.Error explaining the skip, got %#v", triggerResult) + } +} + // Verifies armed + --resume-play alone (no --trigger) still resumes and leaves TriggerResult nil. func TestWaitForPausePointResumesWithoutTriggerWhenArmed(t *testing.T) { originalQuery := queryPausePointStatus From e6c62d7a8620976b16722f0ffdf9f5b1d4af5e71 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 24 Jul 2026 03:50:38 +0900 Subject: [PATCH 5/5] Drop redundant null assert that trips CS8602 on the pause watcher Co-authored-by: Cursor --- .../Common/InputSystem/InputSystemConfiguredUpdateApplier.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs b/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs index af8714b52..f98c33ea3 100644 --- a/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs +++ b/Packages/src/Editor/FirstPartyTools/Common/InputSystem/InputSystemConfiguredUpdateApplier.cs @@ -168,7 +168,6 @@ void TryDiscardForPause() private static async Task WatchForPauseDiscardAsync(Action tryDiscardForPause, CancellationToken ct) { - Debug.Assert(tryDiscardForPause != null, "tryDiscardForPause must not be null"); while (!ct.IsCancellationRequested) { if (InputSystemUpdateHelper.IsPaused())