Repository navigation
fix: simulate-mouse-input reports whether an interrupted press reached the game instead of "may have registered" - #2530
Merged
Conversation
Contributor
|
Warning Review limit reachedNext included review available in 9 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ 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)
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 |
![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/MouseInputSimulationResponseFactory.cs">
<violation number="1" location="Packages/src/Editor/FirstPartyTools/SimulateMouseInput/MouseInputSimulationResponseFactory.cs:60">
P1: The reworded interruption messages break the existing exact-match assertions in Assets/Tests/PlayMode/MouseInputSimulationResponseFactoryTests.cs (InterruptedButtonResult_WithMultiplePausePointHits_MapsPauseEvidenceAndButtonPosition and InterruptedButtonResult_WhenPressWasApplied_ReportsDeliveredBeforePauseMessage), which still assert the old messages via Is.EqualTo. Update those tests to the new wording (and assert PressDeliveredToGame) so the PlayMode suite keeps passing.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| string message = pressWasApplied | ||
| ? $"Mouse input stopped because Unity paused during Pause Point inspection. Button '{buttonName}' press was already delivered to the game before the pause; Unity CLI Loop released it from bookkeeping, so the game may have registered the press." | ||
| : $"Mouse input stopped because Unity paused during Pause Point inspection. Button '{buttonName}' was released from Unity CLI Loop bookkeeping; the queued input edge was discarded."; | ||
| ? $"Mouse input stopped because Unity paused during Pause Point inspection. Button '{buttonName}' press was delivered to the game before the pause: the Input System processed the press edge in a gameplay update, so game code polling that frame observed it and the world state may already have changed. Do not retry the press; re-check the affected state (and pause-point-status) before deciding the next step." |
There was a problem hiding this comment.
P1: The reworded interruption messages break the existing exact-match assertions in Assets/Tests/PlayMode/MouseInputSimulationResponseFactoryTests.cs (InterruptedButtonResult_WithMultiplePausePointHits_MapsPauseEvidenceAndButtonPosition and InterruptedButtonResult_WhenPressWasApplied_ReportsDeliveredBeforePauseMessage), which still assert the old messages via Is.EqualTo. Update those tests to the new wording (and assert PressDeliveredToGame) so the PlayMode suite keeps passing.
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/MouseInputSimulationResponseFactory.cs, line 60:
<comment>The reworded interruption messages break the existing exact-match assertions in Assets/Tests/PlayMode/MouseInputSimulationResponseFactoryTests.cs (InterruptedButtonResult_WithMultiplePausePointHits_MapsPauseEvidenceAndButtonPosition and InterruptedButtonResult_WhenPressWasApplied_ReportsDeliveredBeforePauseMessage), which still assert the old messages via Is.EqualTo. Update those tests to the new wording (and assert PressDeliveredToGame) so the PlayMode suite keeps passing.</comment>
<file context>
@@ -47,15 +47,18 @@ internal static SimulateMouseInputResponse SuccessButtonResult(
string message = pressWasApplied
- ? $"Mouse input stopped because Unity paused during Pause Point inspection. Button '{buttonName}' press was already delivered to the game before the pause; Unity CLI Loop released it from bookkeeping, so the game may have registered the press."
- : $"Mouse input stopped because Unity paused during Pause Point inspection. Button '{buttonName}' was released from Unity CLI Loop bookkeeping; the queued input edge was discarded.";
+ ? $"Mouse input stopped because Unity paused during Pause Point inspection. Button '{buttonName}' press was delivered to the game before the pause: the Input System processed the press edge in a gameplay update, so game code polling that frame observed it and the world state may already have changed. Do not retry the press; re-check the affected state (and pause-point-status) before deciding the next step."
+ : $"Mouse input stopped because Unity paused during Pause Point inspection. Button '{buttonName}' was released from Unity CLI Loop bookkeeping; the queued input edge was discarded before any gameplay update processed it, so the game never observed a press and it is safe to retry after resume.";
SimulateMouseInputResponse result = new()
</file context>
Merged
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.Refs #2382
https://claude.ai/code/session_01R3jkx6NbNYKPdy5q1NSWYo
https://claude.ai/code/session_01R3jkx6NbNYKPdy5q1NSWYo