Repository navigation
fix: simulate-mouse-input reports whether an interrupted press reached the game instead of "may have registered" - #2509
Merged
Conversation
…interrupted by a pause point When a Click or LongPress landed on the same frame as a pause-point hit, the response hedged: "the game may have registered the press". Callers then had to re-derive whether the world changed from unrelated observations. The verdict is actually definite: a press is applied only inside an Input System update of the configured gameplay type, so pressWasApplied means the game's polling in that frame observed the edge, and its absence means the queued edge was discarded before any gameplay update ran. - Add PressDeliveredToGame (nullable) to the mouse input response, set to true/false on interrupted Click/LongPress and left null elsewhere - Reword both interruption messages: delivered means the world state may already have changed, so re-check it instead of retrying; discarded means the game never observed a press and a retry after resume is safe - Document the field in the skill reference and point the SKILL.md interruption note at it (generated copies regenerated) - Add factory tests for the three outcomes Closes #2382 Claude-Session: https://claude.ai/code/session_01R3jkx6NbNYKPdy5q1NSWYo
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe mouse input response now reports whether an interrupted press reached gameplay before a pause point. The factory sets this nullable value, tests cover its states, and the bundled skill documentation explains how to interpret it. ChangesInterrupted mouse input verdict
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to The PR makes a localized response and documentation change for interrupted mouse input, with no actionable merge-blocking risk remaining after normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The implementation satisfies issue Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (6 skipped: 6 unsupported.) ✨ 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}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 10 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/SimulateMouseInput/Skill/SKILL.md">
<violation number="1" location="Packages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.md:62">
P3: For interrupted MoveDelta/SmoothDelta/Scroll, `InterruptedByPausePoint` can be true while `PressDeliveredToGame` is always null, so this unconditional instruction sends the agent to a field with no guidance. Scope the note to Click/LongPress, matching the reference doc's "null for other actions" rule.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ### Pause Point Inspection (Standard for E2E) | ||
|
|
||
| For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused and input bookkeeping was safely released. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation. | ||
| For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused; read `PressDeliveredToGame` before retrying. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation. |
There was a problem hiding this comment.
P3: For interrupted MoveDelta/SmoothDelta/Scroll, InterruptedByPausePoint can be true while PressDeliveredToGame is always null, so this unconditional instruction sends the agent to a field with no guidance. Scope the note to Click/LongPress, matching the reference doc's "null for other actions" rule.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/src/Editor/FirstPartyTools/SimulateMouseInput/Skill/SKILL.md, line 62:
<comment>For interrupted MoveDelta/SmoothDelta/Scroll, `InterruptedByPausePoint` can be true while `PressDeliveredToGame` is always null, so this unconditional instruction sends the agent to a field with no guidance. Scope the note to Click/LongPress, matching the reference doc's "null for other actions" rule.</comment>
<file context>
@@ -59,7 +59,7 @@ uloop simulate-mouse-input --dry-run --x <x> --y <y> [--layer-mask <mask>] [--ma
### Pause Point Inspection (Standard for E2E)
-For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused and input bookkeeping was safely released. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation.
+For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused; read `PressDeliveredToGame` before retrying. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation.
## When to use this vs simulate-mouse-ui
</file context>
Suggested change
| For standard frame proof when this input drives a state transition, follow the `uloop-pause-point` skill — it covers line placement and interruption semantics. Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused; read `PressDeliveredToGame` before retrying. Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation. | |
| Tool-specific note: if `InterruptedByPausePoint: true`, Unity is paused; for `Click`/`LongPress`, read `PressDeliveredToGame` before retrying (motion/scroll actions return null). Clear inspection-only pause points (`uloop clear-pause-point --all`) before final validation. |
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
simulate-mouse-inputClick or LongPress is interrupted by a pause-point hit, the response now gives a definitePressDeliveredToGame: true/falseverdict instead of "the game may have registered the press".User Impact
...so the game may have registered the press.The world-state change was left indeterminate. In the reported session the click had mined a block and the tester only found out three commands later by back-tracking an unexpected coordinate.PressDeliveredToGame: true— the Input System processed the press edge in a gameplay update before the pause, so game code polling that frame observed it and the world state may already have changed. The message says not to retry and to re-check the affected state andpause-point-status.PressDeliveredToGame: false— the queued edge was discarded before any gameplay update ran, the game never observed a press, and a retry after resume is safe.nullfor non-button actions and for uninterrupted responses.Changes
SimulateMouseInputResponse.PressDeliveredToGame(nullable bool), set on interrupted Click/LongPress from the existingpressWasAppliedflag.output-and-coordinates.mddocuments the field; the SKILL.md interruption note points at it (generated.claude/and.agents/copies regenerated; SKILL.md stays under the size cap).MouseInputSimulationResponseFactoryTestscovering delivered, discarded, and non-button outcomes.Verification
uloop run-tests --filter-type regex --filter-value "MouseInputSimulationResponseFactoryTests|SimulateMouseInputDryRunTests": the 3 new factory tests pass. Two pre-existingSimulateMouseInputDryRunTestscases fail in this Editor environment (Hit: false; the test reads a 640x480 Game View size while the actual Game View is 1728x1028). That path does not touch the interruption response, so it is unrelated to this change.check-skill-size: no SKILL.md over the limit.Closes #2382
https://claude.ai/code/session_01R3jkx6NbNYKPdy5q1NSWYo