Skip to content

Fix Sequential Note Playback in Simple Tuplet Block#4699

Closed
Ganasekhar-gif wants to merge 1 commit intosugarlabs:masterfrom
Ganasekhar-gif:fix/tuplet-block-timing
Closed

Fix Sequential Note Playback in Simple Tuplet Block#4699
Ganasekhar-gif wants to merge 1 commit intosugarlabs:masterfrom
Ganasekhar-gif:fix/tuplet-block-timing

Conversation

@Ganasekhar-gif
Copy link

@Ganasekhar-gif Ganasekhar-gif commented May 28, 2025

This PR addresses the issue #4633 with the timing and behavior of tuplet blocks, building upon my earlier PRs. After reviewing and testing further, I have restructured and cleaned up the implementation to ensure accurate note scheduling in tuplet patterns.

Debug Logs for Verification(for an 7 1/4 tuplet block):
bpmFactor: 2.6666666666666665
RhythmBlockPaletteBlocks.js:1009 noteBeatValue: 4
RhythmBlockPaletteBlocks.js:1010 beatValue: 0.09523809523809523
RhythmBlockPaletteBlocks.js:1012 Scheduling note 1/7 with delay: 0ms, duration: 4
RhythmBlockPaletteBlocks.js:1012 Scheduling note 2/7 with delay: 95.23809523809523ms, duration: 4
RhythmBlockPaletteBlocks.js:1012 Scheduling note 3/7 with delay: 190.47619047619045ms, duration: 4
RhythmBlockPaletteBlocks.js:1012 Scheduling note 4/7 with delay: 285.7142857142857ms, duration: 4
RhythmBlockPaletteBlocks.js:1012 Scheduling note 5/7 with delay: 380.9523809523809ms, duration: 4
RhythmBlockPaletteBlocks.js:1012 Scheduling note 6/7 with delay: 476.19047619047615ms, duration: 4
RhythmBlockPaletteBlocks.js:1012 Scheduling note 7/7 with delay: 571.4285714285714ms, duration: 4

These logs confirm that the delays are now calculated and scheduled at accurate intervals for each note in the tuplet block.

I would really appreciate it if you could take a look and let me know:

If these changes fully resolve the timing issue
If there are any additional improvements or refinements you'd suggest

Thank you again for your time and guidance throughout this process!

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@Ganasekhar-gif
Copy link
Author

Hi @therealharshit,

Whenever you get a chance, could you please take a look at this PR and share your feedback?

@therealharshit
Copy link
Member

Why you keep on closing PRs and open new one for same issue, this is your 3rd PR for same issue.
This makes it difficult for me to track changes and review.

@Ganasekhar-gif
Copy link
Author

Apologies for the inconvenience caused. In my previous PR, I encountered unexpected changes to the package-lock.json file, and my attempts to resolve it didn’t work as expected. To keep things clean and include detailed logs, I decided to open a new PR.

I understand the importance of keeping changes in one PR and will make sure to avoid this in the future.
No rush—please take your time to review whenever convenient. Thank you!

@Ganasekhar-gif
Copy link
Author

Hi @therealharshit, friendly reminder to review this PR when you get a chance.
Let me know if there’s anything you’d like me to adjust. Thanks!

@therealharshit
Copy link
Member

Hi @therealharshit, friendly reminder to review this PR when you get a chance.
Let me know if there’s anything you’d like me to adjust. Thanks!

Give me some time, I'll review it soon.

@github-actions
Copy link
Contributor

This pull request has been open for more than 60 days without any activity. It will be closed in 3 days unless the stale label is removed or commented on.

@github-actions github-actions bot added the Stale label Aug 13, 2025
@Ganasekhar-gif
Copy link
Author

commenting to keep the P.R active

@github-actions github-actions bot removed the Stale label Aug 17, 2025
@github-actions
Copy link
Contributor

This pull request has been open for more than 60 days without any activity. It will be closed in 3 days unless the stale label is removed or commented on.

@github-actions github-actions bot added the Stale label Oct 17, 2025
@github-actions
Copy link
Contributor

Closed pull request due to inactivity for more than 63 days.

@github-actions github-actions bot closed this Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants