Updated the soundLoop.interval#458
Updated the soundLoop.interval#458parulpriyedarshani wants to merge 1 commit intoprocessing:mainfrom
Conversation
parulpriyedarshani
force-pushed
the
loop
branch
from
1150f5f to
ec95c08
Compare
|
@therewasaguy Kindly, please review this PR. |
therewasaguy
left a comment
therewasaguy
left a comment
There was a problem hiding this comment.
looks good, thank you! Just requesting some small changes.
|
|
||
| let desiredInterval = 0; | ||
|
|
||
| SoundLoop._interval = max(desiredInterval, 0.01); |
There was a problem hiding this comment.
hrm, SoundLoop doesn't exist, instead we have a function called p5.SoundLoop that exists on the p5 object.
We don't want to set _interval on the class, but not on the instance (which is represented by this).
That is happening on line 66. So we should do this check on that line.
| @@ -65,6 +65,12 @@ define(function (require) { | |||
|
|
|||
| this._interval = interval || 1; | |||
There was a problem hiding this comment.
can we do the check on this line? this._interval = Math.max(interval, 0.01) ? And then, the question is how can we set the default? We could set the default in the constructor function on line 57, by saying interval = 1 https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Default_parameters
There was a problem hiding this comment.
@parulpriyedarshani the code is iterated a lot in the past year, I am not sure if the soundLoop.interval problem still exists or not, can you please look into this once? due to branch change it has some merge conflicts too that need to be addressed
3 participants
Solves issue #313
Initialized the desiredInterval to zero and set the soundLoop interval of 0.