feat: Debug.Break pauses can now be inspected with uloop - #1279
Closed
hatayama wants to merge 9 commits into
Closed
Conversation
Add a native wait command that polls Unity pause state so agents can stop at Debug.Break without relying on noisy logs. Expose a lightweight play mode state bridge, add one-frame stepping for paused PlayMode inspection, and require the CLI version that contains the new command.
Add the wait-for-debug-break skill and update control-play-mode guidance so agents know how to step paused PlayMode frames and resume after Debug.Break inspection.
Explain how execute-dynamic-code can schedule a Debug.Break from runtime inspection code and pair it with wait-for-debug-break for frozen-state investigation.
Update generated agent skill copies after documenting scheduled Debug.Break inspection in the source execute-dynamic-code skill.
Explain how keyboard, gameplay mouse, and UI mouse simulations can pair with scheduled Debug.Break waits when screenshots cannot prove that input changed runtime state.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds PlayModeAction.NextFrame with one-frame stepping when paused; introduces an internal get-play-mode-state bridge and GetPlayModeStateResponse; implements a native wait-for-debug-break command that polls until Unity pauses; updates skill docs with Debug.Break verification workflows and bumps CLI/contract versions; adjusts local CLI install scripts to copy binaries. ChangesDebug.Break inspection and frame-stepping features
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/internal/cli/wait_for_debug_break.go`:
- Around line 65-72: The guard only checks initialState.IsPaused but should also
reject when Unity is not in PlayMode (initialState.IsPlaying == false &&
initialState.IsPaused == false) to avoid hanging in waitForDebugBreak; update
the logic around initialState (before spinner.Update/Waiting) to stop the
spinner, surface a clear error via writeErrorEnvelope (or add a new error helper
like debugBreakNotPlayingError) and return a non-zero exit code when neither
IsPlaying nor IsPaused is true, otherwise proceed to call waitForDebugBreak as
before.
- Around line 114-135: The polling loop in waitForDebugBreak currently drops
errors from queryPlayModeState and only returns ctx.Err() on context expiry;
preserve the most recent query error (e.g., add a variable lastErr updated
whenever queryPlayModeState returns a non-nil err) and when the context is done
return lastErr (or lastState, lastErr) instead of always returning ctx.Err();
update the select branch to return lastState and lastErr if lastErr != nil,
otherwise return ctx.Err() to keep existing behavior when no query errors
occurred.
- Around line 37-43: The overall wait currently can block indefinitely because
runWaitForDebugBreak passes a background context through to waitForDebugBreak;
wrap the incoming ctx in runWaitForDebugBreak with a context.WithTimeout using a
new total timeout constant (e.g., waitForDebugBreakOverallTimeout) and defer
cancel(), then pass that derived ctx into waitForDebugBreak so the whole
operation has a deadline while preserving per-probe timeouts
(waitForDebugBreakStatusProbeTimeout) used inside waitForDebugBreak; ensure you
handle the ctx.Err()/deadline-exceeded case and return an appropriate non-zero
exit code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d61434ca-d7d3-49a3-a518-f1104d59069e
⛔ Files ignored due to path filters (3)
Assets/Tests/Editor/PlayModeStateBridgeCommandTests.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/Api/GetPlayModeStateResponse.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/Api/PlayModeStateBridgeCommand.cs.metais excluded by none and included by none
📒 Files selected for processing (43)
.agents/skills/uloop-control-play-mode/SKILL.md.agents/skills/uloop-execute-dynamic-code/SKILL.md.agents/skills/uloop-execute-dynamic-code/references/playmode-inspection.md.agents/skills/uloop-simulate-keyboard/SKILL.md.agents/skills/uloop-simulate-mouse-input/SKILL.md.agents/skills/uloop-simulate-mouse-ui/SKILL.md.agents/skills/uloop-wait-for-debug-break/SKILL.md.claude/skills/uloop-control-play-mode/SKILL.md.claude/skills/uloop-execute-dynamic-code/SKILL.md.claude/skills/uloop-execute-dynamic-code/references/playmode-inspection.md.claude/skills/uloop-simulate-keyboard/SKILL.md.claude/skills/uloop-simulate-mouse-input/SKILL.md.claude/skills/uloop-simulate-mouse-ui/SKILL.md.claude/skills/uloop-wait-for-debug-break/SKILL.mdAssets/Tests/Editor/CliSetupApplicationServiceTests.csAssets/Tests/Editor/ControlPlayModeUseCaseTests.csAssets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.csAssets/Tests/Editor/PlayModeStateBridgeCommandTests.csPackages/src/Editor/CliOnlyTools~/WaitForDebugBreak/Skill/SKILL.mdPackages/src/Editor/Domain/CliConstants.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeSchema.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/references/playmode-inspection.mdPackages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.mdPackages/src/Editor/Infrastructure/Api/GetPlayModeStateResponse.csPackages/src/Editor/Infrastructure/Api/InternalBridgeCommandRouter.csPackages/src/Editor/Infrastructure/Api/PlayModeStateBridgeCommand.csPackages/src/Editor/ToolContracts/UnityCliLoopConstants.cscli/contract.jsoncli/internal/cli/command_help.gocli/internal/cli/command_registry.gocli/internal/cli/completion_test.gocli/internal/cli/control_play_mode_wait_test.gocli/internal/cli/error_envelope.gocli/internal/cli/error_envelope_test.gocli/internal/cli/run.gocli/internal/cli/wait_for_debug_break.gocli/internal/cli/wait_for_debug_break_test.gocli/internal/tools/default-tools.json
Make local CLI activation overwrite the global uloop executable with the current checkout binary so stale backups or symlink targets do not block development workflows.
There was a problem hiding this comment.
2 issues found across 46 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Fast-fail wait-for-debug-break when Unity is not in PlayMode and preserve the last polling error so users see the underlying probe failure instead of only a generic cancellation.
Avoid directing input simulation skills to execute dynamic code directly while preserving the Debug.Break verification workflow through temporary project markers.
There was a problem hiding this comment.
2 issues found across 15 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Copy local CLI binaries to temporary files before replacing the global uloop executable so a failed copy does not remove the working command.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
User Impact
Changes
uloop wait-for-debug-breakcommand and a lightweight internal PlayMode state bridge.control-play-mode --action NextFramefor paused PlayMode frame inspection.3.0.0-beta.25.Verification
scripts/check-go-cli.shcli/dist/darwin-arm64/uloop compile --project-path "$(git rev-parse --show-toplevel)"cli/dist/darwin-arm64/uloop run-tests --project-path "$(git rev-parse --show-toplevel)" --test-mode EditMode --filter-type regex --filter-value "ControlPlayModeUseCaseTests|JsonRpcProcessorCliVersionGateTests|PlayModeStateBridgeCommandTests|CliSetupApplicationServiceTests"git diff --checkcodex-review v3-beta