Adding loop functionality to Sampler (#1310) #1350
Merged
Conversation
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #1350 +/- ##
==========================================
+ Coverage 98.95% 98.97% +0.01%
==========================================
Files 203 203
Lines 22725 22829 +104
Branches 996 1005 +9
==========================================
+ Hits 22487 22593 +106
+ Misses 237 236 -1
+ Partials 1 0 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tambien
reviewed
Jun 22, 2025
Tone/instrument/Sampler.ts
Outdated
| // get the current sources | ||
| this._activeSources.forEach((sourceList) => { | ||
| sourceList.forEach((source) => { | ||
| source.loopStart = loopEnd; |
There was a problem hiding this comment.
Looks like you accidentally set loopStart instead of loopEnd here.
Tone/instrument/Sampler.ts
Outdated
| @@ -197,6 +226,7 @@ export class Sampler extends Instrument<SamplerOptions> { | |||
| if (!Array.isArray(notes)) { | |||
| notes = [notes]; | |||
| } | |||
| const offset = defaultArg(0, this._loopStart); | |||
There was a problem hiding this comment.
I think you have this reversed, should be
const offset = defaultArg(this._loopStart, 0);
Tone/instrument/Sampler.ts
Outdated
| @@ -210,16 +240,22 @@ export class Sampler extends Instrument<SamplerOptions> { | |||
| const playbackRate = intervalToFrequencyRatio( | |||
| difference + remainder | |||
| ); | |||
| let duration = this._loop | |||
There was a problem hiding this comment.
nit: if duration isn't redefined, could be const instead of let
Tone/instrument/Sampler.ts
Outdated
| * @param loopStart The loop start time | ||
| * @param loopEnd The loop end time | ||
| * @example | ||
| * const sampler = new Tone.Sampler("https://tonejs.github.io/audio/berklee/guitar_chord4.mp3").toDestination(); |
There was a problem hiding this comment.
nit: this @example isn't a valid use of the API. i think it'd need to look like:
const sampler = new Tone.Sampler({
urls: {
A1: "https://tonejs.github.io/audio/berklee/guitar_chord4.mp3",
},
}).toDestination();
// loop between the given points
sampler.setLoopPoints(0.2, 0.3);
sampler.loop = true;
|
I've pushed fixes for the nits and linting errors. Let me know if anything else needs adjusting! |
|
great work! thanks so much! |
error-four-o-four
pushed a commit
to error-four-o-four/forked-tone
that referenced
this pull request
Jul 2, 2025
* Adding loop functionality to Sampler (Tonejs#1310) * Addressed nits and fixed linting errors
tambien
pushed a commit
that referenced
this pull request
Aug 13, 2025
* Remove fs-extra dependency and update file operations to use fs/promises * Remove rimraf dependency and update build script to use fs.rmSync for directory removal * Replace glob with tinyglobby for improved performance and update test scripts * Refactor imports to use node: prefix and update ESLint configuration for improved file handling * fix: test_html.cjs * Revert "Replace glob with tinyglobby for improved performance and update test scripts" This reverts commit e262a38. * Refactor test scripts as ES Modules * fix: update docs script to use typedoc directly * Replace glob with tinyglobby * Replace tmp-promise with fs-fixture * Replace showdown with marked * Bump jsdom from 16.7.0 to 26.1.0 * Bump @rollup/plugin-commonjs from 25.0.7 to 28.0.3 * fix: use correct script in 'Check typedocs' * Revert "Bump @rollup/plugin-commonjs from 25.0.7 to 28.0.3" This reverts commit ce653e2. * Bump @rollup/plugin-commonjs from 25.0.7 to 26.0.3 * Adding loop functionality to Sampler (#1310) (#1350) * Adding loop functionality to Sampler (#1310) * Addressed nits and fixed linting errors * revert: rename npm script `docs:json` * fix: update log messages to indicate successful testing * Revert "fix: use correct script in 'Check typedocs'" This reverts commit 5089f42. --------- Co-authored-by: Caden-Hornyak <108236501+Caden-Hornyak@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR adds looping functionality to the Sampler consistent with Tone.Player. Looping is useful for Sampler instruments that need permanent continuous sound through crossfading such as the Violin.
An earlier PR by maximelebreton's from 2023 introduced basic looping but it lacked Unit testing as well as consistent behavior with Tone.Player such as getter's/setter's.