Skip to content

fix: Make Unity readiness and PlayMode stop results clearer#1300

Merged
hatayama merged 3 commits into
v3-betafrom
feature/improve-launch-r
Jun 9, 2026
Merged

fix: Make Unity readiness and PlayMode stop results clearer#1300
hatayama merged 3 commits into
v3-betafrom
feature/improve-launch-r

Conversation

@hatayama

@hatayama hatayama commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • Launch now shows when it is waiting for Unity CLI Loop server readiness and prints a final ready JSON payload after the project IPC path is usable.
  • Stopping Play Mode now distinguishes a real state change from a no-op when Play Mode was already stopped.

User Impact

  • Agents and scripts no longer have to infer whether uloop launch -r finished at Unity startup or at CLI readiness.
  • control-play-mode --action Stop now reports Changed and WasAlreadyStopped, so cleanup commands can tell whether they actually stopped Play Mode.

Changes

  • Added a launch readiness response and tests around the launch wait flow.
  • Added Control Play Mode response fields and preserved them through CLI state polling.
  • Updated source and generated skill docs for the new output contract.

Verification

  • go test ./internal/cli -run "TestRunLaunchWritesReadyResponseAfterToolReadiness|TestRunControlPlayModeWithStateWaitPreservesStopChangeFields"
  • scripts/check-go-cli.sh
  • cli/dist/darwin-arm64/uloop launch -r --project-path "$(git rev-parse --show-toplevel)"
  • cli/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 "io.github.hatayama.UnityCliLoop.Tests.Editor.ControlPlayModeUseCaseTests"
  • cli/dist/darwin-arm64/uloop control-play-mode --project-path "$(git rev-parse --show-toplevel)" --action Stop
  • git diff --check

Report explicit readiness JSON after launch waits for Unity CLI Loop server access, and distinguish no-op PlayMode Stop responses with structured change fields. Update the synced skill docs so agents can rely on the new output contract.
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@hatayama, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 5 minutes and 4 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 08092d74-66de-4d09-99b1-edebd819b779

📥 Commits

Reviewing files that changed from the base of the PR and between b1cf8f6 and 4ddfaad.

📒 Files selected for processing (3)
  • Assets/Tests/Editor/CliSetupApplicationServiceTests.cs
  • cli/internal/cli/launch.go
  • cli/internal/cli/launch_ready.go
📝 Walkthrough

Walkthrough

The PR extends the play mode control response with state-transition metadata (Changed, WasAlreadyStopped) and adds structured readiness output to the launch flow. Changes span C# contracts/implementation, Go integration, tests, and documentation across multiple SKILL.md file variants.

Changes

Control Play Mode Response Enhancement

Layer / File(s) Summary
Play Mode Response Type & C# Implementation
Packages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeResponse.cs, Packages/src/Editor/FirstPartyTools/ControlPlayMode/ControlPlayModeUseCase.cs
ControlPlayModeResponse gains Changed and WasAlreadyStopped boolean properties. The use case computes these for each action based on prior play/pause state: StatusOnly returns both false, Play detects resume vs. start, Stop detects whether editor was already stopped, and Pause detects if play was active. A new CreateResponse overload populates these fields with IsPlaying/IsPaused.
C# Play Mode Test
Assets/Tests/Editor/ControlPlayModeUseCaseTests.cs
Adds UnityEditor namespace and a new test ExecuteAsync_WhenStopAlreadyStopped_ReturnsNoOpState verifying that Stop when play mode is inactive returns Changed=false, WasAlreadyStopped=true, and the appropriate message.
Go Response Integration & Propagation
cli/internal/cli/control_play_mode_wait.go
Updates the Go controlPlayModeResponse struct with Changed and WasAlreadyStopped JSON fields. runControlPlayModeWithStateWait copies these fields from the initial response into the final output, preserving transition metadata across the state-polling flow.
Go Play Mode Test
cli/internal/cli/control_play_mode_wait_test.go
Introduces TestRunControlPlayModeWithStateWaitPreservesStopChangeFields that verifies runControlPlayModeWithStateWait preserves Changed and WasAlreadyStopped values from a Stop response while continuing to poll for subsequent status.
Play Mode Response Documentation
.agents/skills/uloop-control-play-mode/SKILL.md, .claude/skills/uloop-control-play-mode/SKILL.md, Packages/src/Editor/FirstPartyTools/ControlPlayMode/Skill/SKILL.md
All three SKILL.md variants document the new Changed and WasAlreadyStopped JSON fields and update Notes to specify field values when play mode is already stopped.

Launch Readiness Output & Flow

Layer / File(s) Summary
Launch Readiness Output Module
cli/internal/cli/launch_ready.go
New file implements readiness output with message constants, a launchReadyResponse struct (Success, Ready, ServerReady, ProjectIpcReady, Message JSON fields), and helpers: writeLaunchReadinessWait updates spinner with waiting message; writeLaunchReadyResponse builds and writes success JSON to stdout.
Launch Flow Integration & Indirection
cli/internal/cli/launch.go
Refactors to route helper functions through indirection variables for testability. Updates runLaunch to use resolveUnityExecutablePathForLaunch, and revises the post-launch readiness sequence: waits for lockfile, emits readiness-wait output, waits for tool readiness, stops spinner, and emits readiness-success response.
Launch Readiness Test
cli/internal/cli/launch_test.go
Introduces TestRunLaunchWritesReadyResponseAfterToolReadiness that stubs helper function variables to simulate successful Unity-CLI interaction and verifies that runLaunch stdout contains the full readiness payload with all flags true and matching message text.
Launch Readiness Documentation
.agents/skills/uloop-launch/SKILL.md, .claude/skills/uloop-launch/SKILL.md, Packages/src/Editor/CliOnlyTools~/Launch/Skill/SKILL.md
All three SKILL.md variants document the new readiness-wait behavior: waiting for Unity CLI Loop server readiness, waiting for startup completion, and returning structured JSON with success/ready/server/project flags and readiness summary message.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • hatayama/unity-cli-loop#1219: Introduces the foundational uloop control-play-mode wait/response path that this PR extends by adding and propagating the new Changed/WasAlreadyStopped metadata fields.
  • hatayama/unity-cli-loop#1027: Related to the launch readiness flow improvements, particularly the indirection-based helper setup and spinner/readiness output sequencing refactored in this PR's launch.go.
  • hatayama/unity-cli-loop#997: Overlaps on uloop-launch documentation about waiting for Unity startup; may contain related wording or readiness behavior edits.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: improving clarity of Unity readiness reporting and PlayMode stop behavior through new response fields and messaging.
Description check ✅ Passed The description is directly related to the changeset, detailing the launch readiness output and PlayMode stop distinctions with clear user impact and verification steps.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/improve-launch-r

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 14 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cli/internal/cli/launch_ready.go">

<violation number="1" location="cli/internal/cli/launch_ready.go:36">
P2: `panic(err)` is used for `json.Marshal` failure, which is inconsistent with the codebase convention. Every other `json.Marshal` call in the `cli` package handles errors gracefully. Replace `panic(err)` with proper error handling that writes a classified error to stderr or returns early.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread cli/internal/cli/launch_ready.go
The PR changes Go CLI response behavior that the Unity package now documents and consumes. Update the package minimum CLI version and release contract files so the CI minimum-version guard recognizes the new CLI contract.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread Packages/src/Editor/Domain/CliConstants.cs
Replace the launch ready response panic path with the CLI error envelope flow used by neighboring command code. Also keep CLI setup tests tied to the version constants so minimum CLI bumps do not leave stale hardcoded expectations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant