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

Messes up SynthDef.func in SynthDef.build #4

Open
avdrd opened this issue Dec 22, 2021 · 2 comments
Open

Messes up SynthDef.func in SynthDef.build #4

avdrd opened this issue Dec 22, 2021 · 2 comments

Comments

@avdrd
Copy link

avdrd commented Dec 22, 2021

This is related to one of the warnings reported in #1.

This quark does:

// Andrea Valle jan 2010 
// just adding a storage utility to build 
+ SynthDef {

	build { arg ugenGraphFunc, rates, prependArgs;
		protect {
			this.initBuild;
			this.buildUgenGraph(ugenGraphFunc, rates, prependArgs);
			this.finishBuild;
		} {
			UGen.buildSynthDef = nil;
		} ;
		// Just adding this
		SynthDefStorage.synthDefDict.add(name->[this, ugenGraphFunc.asCompileString]) ;
	}
}

But the build in SynthDef nowadays (SC 3.12.1) does

	build { arg ugenGraphFunc, rates, prependArgs;
		protect {
			this.initBuild;
			this.buildUgenGraph(ugenGraphFunc, rates, prependArgs);
			this.finishBuild;
			func = ugenGraphFunc;
		} {
			UGen.buildSynthDef = nil;
		}
	}

Meaning it has that extra func = ugenGraphFunc line. This isn't just theoretical. With this quark installed

d = SynthDef(\yawn, { arg freq = 499, amp = 0.1; Out.ar(0, amp*SinOsc.ar(freq) ) }).add
d.func //-> nil

Generally speaking it's probably not ok to override such a core SynthDef method in a quark, especially in a gui quark, which people would not expect to change synthdef compilation. I'm not sure why this quark doesn't just look up SynthDefs in SynthDescLib. Maybe the latter class didn't exist in 2010, but I kinda doubt that.

I'll submit a quick fix for this one to bring that build method up to date with SC 3.12, but this quark should really be redesigned so it doesn't override SynthDef.build.

@avdrd
Copy link
Author

avdrd commented Dec 22, 2021

I'm guessing that it actually did

SynthDefStorage.synthDefDict.add(name->[this, ugenGraphFunc.asCompileString]) 

precisely because the line

			func = ugenGraphFunc;

didn't exist in some 2010 version of SC, so there was no way to get the func.asCompileString at some point later just from the resulting SynthDef, so this quark added its little lookaside table just for those. Let me see if I can come up with a more proper fix that directly uses that .func field in SynthDef, which exists nowadays. I'm gonna have to hunt for the use/call sites for that SynthDefStorage.synthDefDict. What should probably happen is a two stage lookup, first get the SynthDef from SynthDescLib and then get the func.asString form the latter. Won't work for SynthDefs that were not yet added though, so it will be a minor breaking change, but the quark has been broken for a couple years anyway... for reason of #3.

@avdrd
Copy link
Author

avdrd commented Dec 22, 2021

The strange irony is that there is only use/call site for SynthDefStorage.synthDefDict namely on the line I fixed in #2.

SynthDefStorage.synthDefDict[name.asSymbol][0]

So the 2nd element (@1) of the array that is added to point to the compile string

SynthDefStorage.synthDefDict.add(name->[this, ugenGraphFunc.asCompileString]) 

doesn't actually appear used anywhere! At least not in this quark. Do you know if any other quarks depend on this one? I'm not sure how to determine that (quickly).

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

No branches or pull requests

1 participant