fix: Play mode control waits reliably and input overlays avoid missing script warnings#1219
Merged
hatayama merged 5 commits intoMay 26, 2026
Merged
Conversation
The packaged input overlay should not reference demo-only scripts from Assets, because installed packages do not include those project test assets and Unity reports them as missing scripts when the overlay prefab is instantiated.
Return control-play-mode responses only after Unity reaches the requested PlayMode state, and expose a timeout budget so slow projects can opt into a longer wait instead of receiving a started message while still in Edit Mode.
Keep the Unity RPC from awaiting across PlayMode domain reloads, and poll a hidden status-only request from the native CLI so Play/Stop can survive bridge recovery before returning final state.
Timeouts mean the requested PlayMode state was not reached, so return a failed CLI result instead of letting automation continue as if the Editor is ready.
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 (1)
📝 WalkthroughWalkthroughThis PR adds configurable timeout support to the control-play-mode tool, allowing the CLI to wait for Unity play-mode state transitions within a timeout window. It introduces StatusOnly state-query capability on the editor side, comprehensive polling and retry logic on the CLI side, and adds validation tests for overlay prefab assets. ChangesControl Play Mode Timeout and StatusOnly State Query
Prefab Asset Validation Tests
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Packages/src/Editor/FirstPartyTools/ControlPlayMode/Skill/SKILL.md (1)
59-59: ⚡ Quick winDocument timeout failure behavior.
The note explains that the command waits and suggests increasing the timeout, but it doesn't explain what happens when the timeout expires. The PR objectives state that "timeout failures return actionable errors," which is important user-facing behavior that should be documented.
📝 Suggested addition to clarify timeout behavior
-- The command waits for the requested state before returning. Increase `--timeout-seconds` for projects with slow PlayMode entry. +- The command waits for the requested state before returning. Increase `--timeout-seconds` for projects with slow PlayMode entry. The command will fail with an error if the timeout is reached before the requested state is achieved.🤖 Prompt for 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. In `@Packages/src/Editor/FirstPartyTools/ControlPlayMode/Skill/SKILL.md` at line 59, Update the SKILL.md note about "--timeout-seconds" so it documents what happens when the timeout expires: state that the command returns a non-zero exit code, prints an actionable error message explaining the timeout caused failure (including which state was waited for), and suggests remediation (increase --timeout-seconds or diagnose PlayMode entry delays); reference the "--timeout-seconds" flag and the PlayMode entry/state wait behavior so readers know the exact failure semantics and next steps.Assets/Tests/Editor/InputVisualizationCanvasPrefabTests.cs (1)
22-40: ⚡ Quick winConsolidate duplicated prefab path lists to avoid drift.
OverlayPrefabPathsandOverlayPrefabFilePathsmust be kept in sync manually. A single paired structure reduces maintenance mistakes and keeps both tests aligned.♻️ Refactor sketch
- private static readonly string[] OverlayPrefabPaths = + private static readonly (string packagePath, string sourcePath)[] OverlayPrefabs = { - "Packages/io.github.hatayama.uloopmcp/Runtime/Common/InputVisualizationCanvas.prefab", - "Packages/io.github.hatayama.uloopmcp/Runtime/SimulateMouseInput/SimulateMouseInputOverlay.prefab", - ... + ( + "Packages/io.github.hatayama.uloopmcp/Runtime/Common/InputVisualizationCanvas.prefab", + "Packages/src/Runtime/Common/InputVisualizationCanvas.prefab" + ), + ( + "Packages/io.github.hatayama.uloopmcp/Runtime/SimulateMouseInput/SimulateMouseInputOverlay.prefab", + "Packages/src/Runtime/SimulateMouseInput/SimulateMouseInputOverlay.prefab" + ), + // ... };Then use
.packagePathin the load/instantiate test and.sourcePathin the YAML scan test.🤖 Prompt for 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. In `@Assets/Tests/Editor/InputVisualizationCanvasPrefabTests.cs` around lines 22 - 40, Replace the duplicated arrays OverlayPrefabPaths and OverlayPrefabFilePaths with a single paired structure (e.g., an array of objects with packagePath and sourcePath properties) so the two path variants cannot drift; update the tests to read .packagePath when performing the load/instantiate tests and .sourcePath when performing the YAML scan tests, and replace all references to OverlayPrefabPaths and OverlayPrefabFilePaths accordingly (look for usages in the load/instantiate test and the YAML scan test).
🤖 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 `@Assets/Tests/Editor/InputVisualizationCanvasPrefabTests.cs`:
- Around line 164-169: The test currently lets empty assetPath from
AssetDatabase.GUIDToAssetPath(guid) pass the Does.Not.StartWith("Assets/")
check, so change the assertion to explicitly require a non-empty, project path:
retrieve assetPath (via AssetDatabase.GUIDToAssetPath(guid)), then Assert that
assetPath is not null or empty and that it StartsWith "Assets/" for the
OverlayPrefabFilePaths[pathIndex] reference to guid; use the same variables
(assetPath, guid, OverlayPrefabFilePaths[pathIndex], pathIndex) so the failure
message still identifies the prefab and GUID.
---
Nitpick comments:
In `@Assets/Tests/Editor/InputVisualizationCanvasPrefabTests.cs`:
- Around line 22-40: Replace the duplicated arrays OverlayPrefabPaths and
OverlayPrefabFilePaths with a single paired structure (e.g., an array of objects
with packagePath and sourcePath properties) so the two path variants cannot
drift; update the tests to read .packagePath when performing the
load/instantiate tests and .sourcePath when performing the YAML scan tests, and
replace all references to OverlayPrefabPaths and OverlayPrefabFilePaths
accordingly (look for usages in the load/instantiate test and the YAML scan
test).
In `@Packages/src/Editor/FirstPartyTools/ControlPlayMode/Skill/SKILL.md`:
- Line 59: Update the SKILL.md note about "--timeout-seconds" so it documents
what happens when the timeout expires: state that the command returns a non-zero
exit code, prints an actionable error message explaining the timeout caused
failure (including which state was waited for), and suggests remediation
(increase --timeout-seconds or diagnose PlayMode entry delays); reference the
"--timeout-seconds" flag and the PlayMode entry/state wait behavior so readers
know the exact failure semantics and next steps.
🪄 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: e4323145-cd11-4353-8fb4-0075258e2ff8
⛔ Files ignored due to path filters (5)
Assets/Tests/Editor/ControlPlayModeUseCaseTests.cs.metais excluded by none and included by nonePackages/src/Cli~/dist/darwin-amd64/uloopis excluded by!**/dist/**and included by nonePackages/src/Cli~/dist/darwin-arm64/uloopis excluded by!**/dist/**and included by nonePackages/src/Cli~/dist/windows-amd64/uloop.exeis excluded by!**/dist/**,!**/*.exeand included by nonePackages/src/Runtime/SimulateMouseInput/SimulateMouseInputOverlay.prefabis excluded by!**/*.prefaband included by none
📒 Files selected for processing (10)
Assets/Tests/Editor/ControlPlayModeUseCaseTests.csAssets/Tests/Editor/InputVisualizationCanvasPrefabTests.csPackages/src/Cli~/internal/cli/control_play_mode_wait.goPackages/src/Cli~/internal/cli/control_play_mode_wait_test.goPackages/src/Cli~/internal/cli/error_envelope.goPackages/src/Cli~/internal/cli/run.goPackages/src/Cli~/internal/tools/default-tools.jsonPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeSchema.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.csPackages/src/Editor/FirstPartyTools/ControlPlayMode/Skill/SKILL.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Detect empty GUID resolution results so package overlay prefab tests catch broken m_Script references instead of passing them through the project-script check.
Merged
This was referenced May 26, 2026
This was referenced May 27, 2026
This was referenced Jun 9, 2026
5 tasks
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
uloop control-play-modenow waits until Unity reaches the requested Play, Stop, or Pause state before returning.User Impact
IsPlayingis still false.--timeout-secondsto give Unity more time, and timeout failures now return an actionable CLI error.Changes
Verification
~/.codex/skills/codex-review/scripts/codex-review v3-betago test ./...scripts/check-go-cli.shPackages/src/Cli~/dist/darwin-arm64/uloop compile --project-path "$(git rev-parse --show-toplevel)"Packages/src/Cli~/dist/darwin-arm64/uloop run-tests --project-path "$(git rev-parse --show-toplevel)" --test-mode EditMode --filter-type regex --filter-value "(ControlPlayModeUseCaseTests|InputVisualizationCanvasPrefabTests)"