chore: Improve uloop skill guidance for frame debugging#1302
Conversation
Expand the skill trigger so agents use paused-frame proof not only for transient bugs, but also when validating uloop simulated input through major gameplay transitions. Keep ordinary persistent-state checks optional while preserving the warning that command success, logs, sleeps, counters, or time-scale changes are not paused-frame evidence.
Ensure refresh-neighbor-game-skills starts from a clean Git state in each sibling game project before quitting Unity, regenerating generated skill files, committing them, and relaunching the editors. The reset phase runs before any skill-dir cleanliness checks so stale local changes cannot block or contaminate the refresh workflow.
Describe wait-for-debug-break as a quick breakpoint substitute and add a compact usage template so agents can try it without treating the workflow as heavyweight.
Replace game-specific checkpoint examples with runtime-state wording so the skill applies across Unity PlayMode validation scenarios, not only particular game mechanics.
Show the lightweight usage loop before detailed guidance so agents see the practical wait-for-debug-break workflow immediately after the skill triggers.
Fold the essential commands into the quick check template and keep only timeout-specific guidance separately so the skill does not repeat the same workflow twice.
Add a lightweight representative-transition prompt to simulated input skills so agents naturally pair state-changing E2E input with UnityCliLoopDebug.Break and wait-for-debug-break instead of relying only on final logs, screenshots, or durable state.
Make the wait-for-debug-break and simulate input skill descriptions explicit that state-changing PlayMode/E2E checks should pause at least one representative frame. This addresses agents treating durable state, screenshots, Time.timeScale, sleeps, or after-the-fact inspection as substitutes for breakpoint-like frame proof.
Clarify that execute-dynamic-code cannot inspect method locals from an already-running frame, and point agents to add focused Debug.Log entries immediately before debug-break markers. Update generated skill copies so input simulation, log inspection, dynamic-code inspection, and wait-for-debug-break guidance all reinforce the paused-frame workflow.
Marker-only debug-break checks do not prove method locals, intermediate calculations, or branch reasons. Require focused Debug.Log output next to the marker and a matching get-logs read before resuming PlayMode.
Keep the trigger description centered on IDE-style frame inspection so agents choose the skill for specific-frame variable and state checks. Leave the Debug.Log workflow details in the skill body instead of the metadata.
Make the keyboard and mouse input workflows clear that debug-break checks and screenshots are optional evidence choices. Spell out the concrete uloop wait-for-debug-break and get-logs commands for method-local inspection guidance.
|
Warning Review limit reached
More reviews will be available in 9 minutes and 24 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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR updates documentation across six SKILL.md files and a helper script to establish a unified debug-break paused-frame inspection workflow for Unity CLI testing. The changes introduce the "uloop-wait-for-debug-break" pattern, update simulate-* and execute-dynamic-code guidance, and enhance a script with Git reset and multi-phase automation. ChangesDebug-break paused-frame inspection pattern and skill documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Packages/src/Editor/FirstPartyTools/GetLogs/Skill/SKILL.md (1)
11-12: ⚡ Quick winConsider clarifying "checked as one breakpoint-style proof".
The phrase "checked as one breakpoint-style proof" may be unclear to readers. Consider rewording to be more concrete about what this achieves, such as:
- "so both the log entry and marker hit are retrieved together"
- "to retrieve the log and marker hit as a combined evidence set"
This would make the purpose more explicit.
🤖 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/GetLogs/Skill/SKILL.md` around lines 11 - 12, The sentence containing "checked as one breakpoint-style proof" is vague; update the text in SKILL.md where the `--search-text <marker-or-id>` suggestion appears to use a clearer phrase such as "so both the log entry and marker hit are retrieved together" or "to retrieve the log and marker hit as a combined evidence set" (replace the ambiguous phrase with one of these options) so readers understand that the marker and its associated Debug.Log are returned as a single paired result when using `--search-text <marker-or-id>`.Packages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.md (1)
128-128: 💤 Low valueConsider moving alternative-tool guidance to a separate section.
This line provides helpful guidance about when to use
simulate-mouse-inputorexecute-dynamic-codeinstead of this tool. However, it's in the "Prerequisites" section, which typically lists requirements for using the current tool rather than comparisons with alternatives.Consider moving this to a "Related Tools" or "When to Use This Tool" section for clearer organization.
🤖 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/SimulateMouseUi/Skill/SKILL.md` at line 128, The guidance comparing alternatives currently placed in the Prerequisites should be moved into a new section (e.g., "Related Tools" or "When to Use This Tool"); update SKILL.md to remove the sentence from the Prerequisites and add a short new section that states when to prefer `simulate-mouse-input` (for runtime mouse input with the New Input System) versus `execute-dynamic-code` (when not using New Input System), referencing those exact tool names so readers can find them.
🤖 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.
Nitpick comments:
In `@Packages/src/Editor/FirstPartyTools/GetLogs/Skill/SKILL.md`:
- Around line 11-12: The sentence containing "checked as one breakpoint-style
proof" is vague; update the text in SKILL.md where the `--search-text
<marker-or-id>` suggestion appears to use a clearer phrase such as "so both the
log entry and marker hit are retrieved together" or "to retrieve the log and
marker hit as a combined evidence set" (replace the ambiguous phrase with one of
these options) so readers understand that the marker and its associated
Debug.Log are returned as a single paired result when using `--search-text
<marker-or-id>`.
In `@Packages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.md`:
- Line 128: The guidance comparing alternatives currently placed in the
Prerequisites should be moved into a new section (e.g., "Related Tools" or "When
to Use This Tool"); update SKILL.md to remove the sentence from the
Prerequisites and add a short new section that states when to prefer
`simulate-mouse-input` (for runtime mouse input with the New Input System)
versus `execute-dynamic-code` (when not using New Input System), referencing
those exact tool names so readers can find them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08c63ee2-0c1a-4ef5-9ea5-dfdea8940b74
📒 Files selected for processing (19)
.agents/skills/uloop-execute-dynamic-code/SKILL.md.agents/skills/uloop-get-logs/SKILL.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-execute-dynamic-code/SKILL.md.claude/skills/uloop-get-logs/SKILL.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.mdPackages/src/Editor/CliOnlyTools~/PausePoint/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/GetLogs/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.mdPackages/src/Editor/FirstPartyTools/SimulateMouseUi/Skill/SKILL.mdscripts/refresh-neighbor-game-skills.sh
There was a problem hiding this comment.
5 issues found across 19 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="Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md">
<violation number="1" location="Packages/src/Editor/FirstPartyTools/SimulateKeyboard/Skill/SKILL.md:16">
P2: Workflow steps 2→3→4 are ordered incorrectly for the debug-break path: step 3 says to run input and inspect while paused, but step 4 then requires PlayMode (non-paused) to execute simulate-keyboard commands. No resume step bridges the gap.</violation>
</file>
<file name="scripts/refresh-neighbor-game-skills.sh">
<violation number="1" location="scripts/refresh-neighbor-game-skills.sh:245">
P1: New reset step hard-resets and cleans entire sibling repos, which can silently delete unrelated local work. Scope cleanup to generated skill directories (or gate full reset behind explicit opt-in).</violation>
</file>
<file name=".agents/skills/uloop-wait-for-debug-break/SKILL.md">
<violation number="1" location=".agents/skills/uloop-wait-for-debug-break/SKILL.md:34">
P3: The doc recommends `uloop execute-dynamic-code` as a paused-frame evidence tool but omits its failure-semantics diagnostic pathway. Document that when Success=false, inspect CompilationErrors first; if empty, read ErrorMessage and Logs to distinguish runtime exceptions from security/cancellation cases.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| reset_git_state() { | ||
| project=$1 | ||
| log "Resetting Git state: $project" | ||
| run git -C "$project" reset --hard HEAD |
There was a problem hiding this comment.
P1: New reset step hard-resets and cleans entire sibling repos, which can silently delete unrelated local work. Scope cleanup to generated skill directories (or gate full reset behind explicit opt-in).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/refresh-neighbor-game-skills.sh, line 245:
<comment>New reset step hard-resets and cleans entire sibling repos, which can silently delete unrelated local work. Scope cleanup to generated skill directories (or gate full reset behind explicit opt-in).</comment>
<file context>
@@ -238,6 +239,24 @@ assert_project_count() {
+reset_git_state() {
+ project=$1
+ log "Resetting Git state: $project"
+ run git -C "$project" reset --hard HEAD
+ run git -C "$project" clean -fd
+
</file context>
There was a problem hiding this comment.
Thanks for the review. I kept this behavior unchanged because this helper is intentionally designed to reset the three neighboring game repositories to a completely clean state before regenerating skills, which matches the requested workflow for this script.
Commit 75c5323 addresses the other actionable review threads in this round.
There was a problem hiding this comment.
Thanks for the clarification.
|
|
||
| 5. Trigger the behavior with `simulate-keyboard`, `simulate-mouse-input`, UI interaction, dynamic code, or similar commands. | ||
| 6. Wait for the marker: | ||
| 6. While Unity is still paused, capture any additional evidence with `uloop execute-dynamic-code`, `uloop get-hierarchy`, `uloop find-game-objects`, and one screenshot. |
There was a problem hiding this comment.
P3: The doc recommends uloop execute-dynamic-code as a paused-frame evidence tool but omits its failure-semantics diagnostic pathway. Document that when Success=false, inspect CompilationErrors first; if empty, read ErrorMessage and Logs to distinguish runtime exceptions from security/cancellation cases.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .agents/skills/uloop-wait-for-debug-break/SKILL.md, line 34:
<comment>The doc recommends `uloop execute-dynamic-code` as a paused-frame evidence tool but omits its failure-semantics diagnostic pathway. Document that when Success=false, inspect CompilationErrors first; if empty, read ErrorMessage and Logs to distinguish runtime exceptions from security/cancellation cases.</comment>
<file context>
@@ -1,54 +1,63 @@
-5. Trigger the behavior with `simulate-keyboard`, `simulate-mouse-input`, UI interaction, dynamic code, or similar commands.
-6. Wait for the marker:
+6. While Unity is still paused, capture any additional evidence with `uloop execute-dynamic-code`, `uloop get-hierarchy`, `uloop find-game-objects`, and one screenshot.
+7. Clear the marker with `uloop clear-debug-break --id state-transition-applied` or stop PlayMode before moving on.
</file context>
Launches each neighboring Unity project, waits for uloop to respond, and opens Assets/Scenes/SampleScene.unity so the refreshed projects land in the expected scene after Library cleanup.
Keep the focused Debug.Log guidance in the skill body while removing duplicate frame-local inspection wording from the frontmatter description.
Restore installed skill copies to the base branch so this PR only carries source skill and helper-script changes. Generated skill directories can be refreshed separately later.
Keeps the PR diff check clean after the source skill description was shortened.
Updates the source get-logs skill description to refer to the wait-for-debug-break skill name while leaving generated skill copies out of the PR.
Softens the source simulate-mouse-input skill so wait-for-debug-break is presented as optional help when paired with UnityCliLoopDebug.Break, and removes marker-style wording from the workflow, inspection guidance, and output notes.
Removes Debug.Break wording from the simulate-mouse-input description and keeps Debug Break inspection as an optional follow-up in the workflow instead of the default path.
Remove redundant optional phrasing and keep the simulate-mouse-input description focused on the command's mouse input use case.
There was a problem hiding this comment.
2 issues found across 16 files (changes from recent commits).
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="scripts/refresh-neighbor-game-skills.sh">
<violation number="1" location="scripts/refresh-neighbor-game-skills.sh:245">
P1: New reset step hard-resets and cleans entire sibling repos, which can silently delete unrelated local work. Scope cleanup to generated skill directories (or gate full reset behind explicit opt-in).</violation>
</file>
<file name=".agents/skills/uloop-wait-for-debug-break/SKILL.md">
<violation number="1" location=".agents/skills/uloop-wait-for-debug-break/SKILL.md:34">
P3: The doc recommends `uloop execute-dynamic-code` as a paused-frame evidence tool but omits its failure-semantics diagnostic pathway. Document that when Success=false, inspect CompilationErrors first; if empty, read ErrorMessage and Logs to distinguish runtime exceptions from security/cancellation cases.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Remove debug-break triggering language from the simulate-mouse-ui description, make paused-frame inspection an optional follow-up in the workflow, and remove the unresolved task argument placeholder.
Make the simulate-keyboard debug-break guidance an optional follow-up after normal input execution, and validate the execute-dynamic-code response when opening the sample scene.
Summary
User Impact
uloop wait-for-debug-breakwhen a specific PlayMode/E2E frame needs variable, GameObject, or runtime-state inspection.Changes
scripts/refresh-neighbor-game-skills.sh.Verification
git diff --check origin/v3-beta...HEADsh -n scripts/refresh-neighbor-game-skills.shscripts/refresh-neighbor-game-skills.sh --helpscripts/refresh-neighbor-game-skills.sh --dry-run