Repository navigation
Rework Guitar Hero failure recovery and temporal note timing in V10 - #147
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5c5fa8eaa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if history and gap > history[-1] + self.approach_backtrack_tolerance: | ||
| history.clear() | ||
| history.append(gap) |
There was a problem hiding this comment.
Reject downward component switches before confirming approach
When the closest contour changes to a different same-color component with a smaller gap—for example, transient stage lighting appearing below a tracked note—this logic preserves the history because it only treats gap increases as discontinuities. The resulting large downward jump is interpreted as approach velocity and can immediately satisfy both confirmation and the dynamic trigger threshold, causing the temporal filter to generate the false fret presses it is intended to suppress. Track contour identity or reject implausible downward jumps before appending them.
Useful? React with 👍 / 👎.
| if gap is None: | ||
| self._missing_frames[lane] += 1 | ||
| if self._missing_frames[lane] > self.approach_missing_grace_frames: | ||
| history.clear() | ||
| return False, 0.0, self.note_trigger_gap |
There was a problem hiding this comment.
Account for missing frames in the velocity estimate
With the default one-frame missing grace, a sequence such as gaps 0.07, None, 0.045 retains only the two numeric endpoints, so the later delta calculation treats movement spanning two frame intervals as one-frame velocity. That inflates the dynamic trigger lead and can fire notes early specifically after a brief detection dropout or occlusion. Preserve elapsed-frame information in the history or divide the displacement by the number of intervening frames.
Useful? React with 👍 / 👎.
Why this is a larger calibration pass
The latest live run made two things clear: lifecycle recovery still needs to be deterministic, and the gameplay path is not good enough to tune by moving one static threshold around. V10 changes the shape of the problem instead of adding another screenshot-specific patch.
Failure-screen recovery
Gameplay timing
Calibration posture
The profile stays on Easy, with whammy and Star Power disabled, so the next live pass isolates base perception and timing. This still is not being promoted beyond diagnostic maturity until a complete Easy song survives.
Regression coverage
Adds tests for stationary-clutter suppression, approaching-note timing, deterministic failure-row navigation, retained failure ownership when NEW SONG is highlighted, active V10 registry export, and V10 timing telemetry.