Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added LPF to delay module #58

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Added LPF to delay module #58

wants to merge 3 commits into from

Conversation

CuttyBang
Copy link

Added a LPF to the Delay module and chained it in. Creates dubby tape delay sound. Can be BPF if preferred. Freqs just need to be shaved on each pass. Thanks guys.

@zxqx
Copy link
Owner

zxqx commented Sep 2, 2016

Thanks for the PR! Will check this out later today.

@CuttyBang
Copy link
Author

Might want to check my routing. Attempted to follow the patterns, but it looks like something is failing in testing....

@trivorak
Copy link
Collaborator

trivorak commented Sep 22, 2016

at the top of the delay module you need to import the filter class. Also where you're setting your defaults it shouldn't have ".node"

filter.setFilterType('lowpass'); filter.setFrequency(1000);

not
filter.node.setFilterType or filter.node.setFrequency

also where you're connecting your filter to your delay module, that should end with a semi-colon

@CuttyBang
Copy link
Author

Aha! I knew there was something I wasn't doing right. I hadn't gotten the hang of this pattern yet. Sorry about that. I hope it still has some value!

Nathan McElwain

On Sep 22, 2016, at 4:41 PM, Drake Champagne notifications@github.com wrote:

at the top of the delay module you need to import the filter class. Also where you're setting your defaults it shouldn't have ".node"

filter.setFilterType('lowpass');
filter.setFrequency(1000);

not
'filter.node.setFilterType or filter.node.setFrequency'

also where you're connecting your filter to your delay module, that should end with a semi-colon


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@zxqx
Copy link
Owner

zxqx commented Sep 23, 2016

@CuttyBang Hey no problem man. Do you want to update the PR and then I can merge it in?

@CuttyBang
Copy link
Author

Absolutely. I'm gonna hop on it this afternoon, as soon as I can get back to my desk! Thank you for keeping the PR alive!

Nathan McElwain

On Sep 23, 2016, at 8:46 AM, Zak Angelle notifications@github.com wrote:

@CuttyBang Hey no problem man. Do you want to update the PR and then I can merge it in?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@zxqx
Copy link
Owner

zxqx commented Sep 23, 2016

No problem! I've been busy but I'm back and working on getting this to a 1.0 state over the next few months. If you have any other ideas feel free to submit them.

Also, I made an update last night so that you can spin up an example synth locally in your browser if you wanna test any new features (see the Example section in the README file for setting it up).

Copy link
Author

@CuttyBang CuttyBang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repaired the paths and syntax error. Added a comment to explain the routing a bit.

@CuttyBang
Copy link
Author

Wow, what a flake! Sorry man. I guess I just got completely sidetracked and spaced on this in the worst way possible. At any rate, I finally fixed it, and added a comment about it.

@zxqx
Copy link
Owner

zxqx commented Apr 21, 2018

Thanks man! Will check this out when I get home tomorrow. Appreciate the contribution.

@zxqx
Copy link
Owner

zxqx commented May 13, 2018

@CuttyBang Hey, just wanted to let you know we are reworking some of the library internals to simplify things. The Delay module shouldn't be changing too much (if at all), and I'll be spending some time porting it over later this week and looking at your code changes.

@zxqx
Copy link
Owner

zxqx commented May 24, 2018

@CuttyBang So wavedef has been restructured a bit - the Delay module is now located at src/modules/Delay.js. Otherwise, not much else has changed, save for stricter linting rules and a few additional methods here and there.

How do you feel about porting over this change to the new code base and adding unit tests in Delay.test.js?

@CuttyBang
Copy link
Author

Yea man, sure. I'm game. I see all the restructuring going on. Looks great. I'll hop on this some time over the next few days.

@zxqx zxqx force-pushed the master branch 5 times, most recently from 6548b87 to 04b6b49 Compare June 10, 2018 01:25
@zxqx zxqx force-pushed the master branch 2 times, most recently from 0db0b3a to 82753d2 Compare February 11, 2022 06:43
@zxqx zxqx force-pushed the master branch 3 times, most recently from 62aa691 to 9803931 Compare August 12, 2024 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants