Fix FramedBeatmapClock offset subscription not updating on source change - #71
Fix FramedBeatmapClock offset subscription not updating on source change#71winnerspiros wants to merge 4 commits into
Conversation
- Refactored offset subscription logic into `updateBeatmapOffsetSubscription`. - Bound `beatmap` bindable to `updateBeatmapOffsetSubscription` in `LoadComplete` to ensure subscription updates when the beatmap changes. - Added regression test `TestSceneFramedBeatmapClock`. Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
- Refactored offset subscription logic into `updateBeatmapOffsetSubscription`. - Bound `beatmap` bindable to `updateBeatmapOffsetSubscription` in `LoadComplete` to ensure subscription updates when the beatmap changes. - Added regression test `TestSceneFramedBeatmapClock` which verifies offset updates, including handling of platform-specific offsets on Windows. Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
- Refactored offset subscription logic into `updateBeatmapOffsetSubscription`. - Bound `beatmap` bindable to `updateBeatmapOffsetSubscription` in `LoadComplete` to ensure subscription updates when the beatmap changes. - Added regression test `TestSceneFramedBeatmapClock` which verifies offset updates, including handling of platform-specific offsets on Windows. Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
- Refactored offset subscription logic into `updateBeatmapOffsetSubscription`. - Bound `beatmap` bindable to `updateBeatmapOffsetSubscription` in `LoadComplete` to ensure subscription updates when the beatmap changes. - Added regression test `TestSceneFramedBeatmapClock` which verifies offset updates, including handling of platform-specific offsets on Windows. Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
1 participant
The
FramedBeatmapClockwas only subscribing to the beatmap offset settings once duringLoadComplete. This meant that if the beatmap source changed (e.g. viaChangeSourceor bindable update), the clock would continue using the offset from the initial beatmap.This change binds to the
beatmapbindable's value changes to update the subscription dynamically. It also refactors the subscription logic into a reusable method.Verified with a new visual test
TestSceneFramedBeatmapClockwhich reproduces the issue and confirms the fix.PR created automatically by Jules for task 3402989738039062734 started by @winnerspiros