fix: Improve compile and pause-point skill guidance for AI agents#1836
Merged
Conversation
AI agents tend to reach for --force-recompile "just in case" after external file edits, but incremental change detection is Unity's job and a plain compile already runs every required recompilation. On large projects the forced full rebuild freezes the Editor for a long time, usually returns COMPILE_RESULT_UNKNOWN across the domain reload, and puts the Editor into the unstable just-after-reload state. Document the few legitimate use cases and make plain `uloop compile` the default guidance.
Contributor
📝 WalkthroughWalkthroughThe PR updates synchronized ChangesForce Recompile Documentation
Dynamic Pause-Point Documentation
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant DynamicCode
participant Unity
CLI->>Unity: Enable id-only pause point
CLI->>DynamicCode: Register update watcher
DynamicCode->>Unity: Pause when runtime condition holds
CLI->>Unity: Await pause point
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
Timing-sensitive PlayMode verification (short motions, one-frame effects, state transitions) cannot be captured reliably by sleeping and taking a screenshot. Document the enable -> register watcher -> await -> inspect -> resume pattern: execute-dynamic-code registers an EditorApplication.update watcher that fires UloopPausePoint.Pause(id) on the first frame a runtime condition holds, while await-pause-point does the waiting on the CLI side. - Add the pattern with a full watcher example to the pause-point skill - Add hit-then-Step guidance for pausing right after simulated input and for frame-offset positioning, and warn against race-prone Time.frameCount arithmetic in watchers - Cross-reference the pattern from the execute-dynamic-code skill - Regenerate .claude/.agents copies via `uloop skills install`
hatayama
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Packages/src/Editor/CliOnlyTools`~/PausePoint/Skill/SKILL.md:
- Around line 131-134: Add an abort/timeout cleanup path to the watcher guidance
so its EditorApplication.update subscription is removed when the condition never
fires, while preserving the existing self-unsubscribe behavior on a hit. Apply
the same documentation update consistently in
Packages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.md lines 131-134,
.agents/skills/uloop-pause-point/SKILL.md lines 131-134, and
.claude/skills/uloop-pause-point/SKILL.md lines 131-134.
🪄 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: 5c6b6138-1bf9-4306-b6c4-68e18e856f19
📒 Files selected for processing (6)
.agents/skills/uloop-execute-dynamic-code/SKILL.md.agents/skills/uloop-pause-point/SKILL.md.claude/skills/uloop-execute-dynamic-code/SKILL.md.claude/skills/uloop-pause-point/SKILL.mdPackages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.md
The watcher example only unsubscribed from EditorApplication.update on the hit path, so a condition that never holds would leak the delegate until the next domain reload. Add a deadline guard matching the marker's --timeout-seconds and update the rules bullet accordingly. Mirrored copies under .agents/ and .claude/ are synced byte-identically. Addresses CodeRabbit review feedback on PR #1836.
hatayama
changed the title
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
--force-recompileis almost never needed and that plainuloop compileis the default.User Impact
--force-recompile"just in case" after editing files externally, freezing Unity with a full rebuild, gettingCOMPILE_RESULT_UNKNOWNacross the domain reload, and leaving the Editor in the unstable just-after-reload state.Changes
Packages/src/.../Compile/Skill/SKILL.md) and reworded the parameter description..claude/and.agents/skill copies throughuloop skills install --claude --agents.Verification
diffconfirms the source and both generated copies are byte-identical.Second commit: condition-triggered pause via dynamic-code watcher
execute-dynamic-coderegisters anEditorApplication.updatewatcher that callsUloopPausePoint.Pause(id)on the first frame a runtime condition holds (animation peak, HP reaching zero, spawn), whileawait-pause-pointblocks on the CLI side. No .cs file is written.control-play-mode --action Step. Explicitly warns against race-proneTime.frameCountarithmetic in watchers..claude/and.agents/copies regenerated throughuloop skills install --claude --agents;diffconfirms byte-identical.