feat: Add press-edge diagnostics to simulate-keyboard - #1912
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughPress and key-down execution now records why gameplay did not observe a press edge, formats scenario-specific diagnostics, and exposes update-type information in responses. New NUnit tests cover the formatter’s diagnostic branches. ChangesPress-edge diagnostics
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant KeyboardInputActionExecutor
participant InputSystem
participant PressEdgeDiagnosticsMessageFormatter
participant SimulateKeyboardResponse
KeyboardInputActionExecutor->>InputSystem: Monitor onAfterUpdate
InputSystem-->>KeyboardInputActionExecutor: Report update type and press-edge state
KeyboardInputActionExecutor->>PressEdgeDiagnosticsMessageFormatter: Build diagnostic suffix
PressEdgeDiagnosticsMessageFormatter-->>KeyboardInputActionExecutor: Return scenario-specific suffix
KeyboardInputActionExecutor->>SimulateKeyboardResponse: Populate diagnostic fields
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Round-6 feedback reported intermittent "press edge was not observed" warnings from simulate-keyboard Press/KeyDown. An extensive reproduction attempt (roughly 200 attempts across this repo's dev project and cli-loop-block-kuzushi, covering concurrent load, main- thread contention, unfocused/runInBackground=false, and pause-point races) could not reproduce the miss even once, and every observed KeyDown event was consumed by a Dynamic update. Changing the observation window or adding a speculative re-press would be a guess, not a fix. Instead, extend the existing onAfterUpdate press-edge monitor to also record which update type (if any) actually saw wasPressedThisFrame, whether any Dynamic update ran during the wait, and whether the key was already down before the event was queued. These are additive, nullable fields set only when the edge is not observed, so a future field occurrence can be diagnosed directly from the response instead of needing another investigation like this one. Press/KeyDown timing, grace, and retry behavior are unchanged.
d8c5750 to
6fe08fb
Compare
Remove the ref out-parameters from RecordPressEdgeMissDiagnostics per project convention (no out/ref) by bundling the three diagnostic locals into a PressEdgeMissDiagnostics class captured by the onAfterUpdate monitor lambda instead. Also group the XML doc comments on SimulateKeyboardResponse's three PressEdge* diagnostic fields so they read as one cohesive set.
b9548f1
into
feature/round6-pause-point-improvements
Summary
simulate-keyboard's Press/KeyDown response now carries extra diagnostic detail whenever it reports "press edge was not observed", so a real occurrence can be diagnosed directly from the response instead of needing a fresh investigation.User Impact
simulate-keyboardoccasionally reported a Press/KeyDown "press edge was not observed" warning, there was no way to tell from the response alone whether the key-down event was consumed at the wrong moment, never consumed at all, whether gameplay updates even ran during the wait, or whether the key was already down before the action started.PressEdgeConsumedByUpdateType,PressEdgeAnyDynamicUpdateObserved,PressEdgeKeyAlreadyPressedBeforeQueue) set only in that case. Timing, grace period, and retry behavior for Press/KeyDown are unchanged.Investigation
runInBackground=false, and pause-point races) never reproduced a missed edge — the injected key-down event was consumed by a Dynamic update in every single case.Changes
onAfterUpdatepress-edge monitor inKeyboardInputActionExecutor(bothExecutePressandExecuteKeyDown) to also record which Input System update type (if any) sawwasPressedThisFrame, whether any Dynamic update ran during the wait, and whether the key was already down right before the event was queued.PressEdgeDiagnosticsMessageFormatterthat turns those three signals into a short message suffix, covered by unit tests.SimulateKeyboardResponse, populated only when the press edge was not observed.Verification
dist/darwin-arm64/uloop compile --project-path <repo>: 0 errors, 0 warnings.BuildSuffixto return"", recompiled, confirmed 5/5 new tests failed, then reverted and recompiled to confirm 0 errors/warnings again.dist/darwin-arm64/uloop run-testsfiltered toPausePoint|SimulateKeyboard|PressEdge|PressHoldUntilEdge: 240/240 passed.dist/darwin-arm64/uloop run-tests(full EditMode suite): 2262/2272 passed; the 3 failures are pre-existing inCliSetupApplicationServiceTests(dispatcher release tag string-length assertions) and unrelated to this change — not modified by this PR.