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

MainFXGui ignores specs from NamedControl #22

Open
madskjeldgaard opened this issue Jul 17, 2021 · 8 comments
Open

MainFXGui ignores specs from NamedControl #22

madskjeldgaard opened this issue Jul 17, 2021 · 8 comments

Comments

@madskjeldgaard
Copy link

madskjeldgaard commented Jul 17, 2021

Hi ! Stumbled on a small issue when using NamedControl style specs/arguments with the ProxyChain/MainFXGui classes. The specs are ignored unless input in the specs argument explicitly:

// Does not work:
ProxyChain.add3(
    srcName: \jpverb, 
	source: \filter -> { |in| 
		JPverb.ar(
			in: in,  
			t60: \time.kr(10, spec: [0.0001,100.0,\exp]),  
		);
	}
);

// Does work:
ProxyChain.add3(
    srcName: \jpverb, 
	source: \filter -> { |in| 
		JPverb.ar(
			in: in,  
			t60: \time.kr(10),  
		);
	},
    specs: (time: [0.0001,100.0,\exp])
)
@adcxyz
Copy link
Contributor

adcxyz commented Aug 4, 2021

Thanks! Unfortunately, this happens even lower:

Ndef(\x, { SinOsc.ar(\freq1.kr(400, spec: [30, 3000, \exp])) * 0.1 }).play.gui;

will check ASAP,
adc

@LFSaw
Copy link
Contributor

LFSaw commented Aug 4, 2022

another reproducer:

Ndef(\foo, { 
	SinOsc.ar(\freq.kr(spec:ControlSpec(200, 20000))) 
});

// specs are stored in specs variable
Ndef(\foo).specs

// but this (used in all NodeProxy-related stuff) returns default spec, resp. a spec defined in the Halo
Ndef(\foo).getSpec(\freq)

// hence, this does not know about the spec
Ndef(\foo).edit

any ideas?

I'm happy to help

@bgola
Copy link
Contributor

bgola commented Aug 4, 2022

I was taking a look at this and the issue seems to be when getSpec is called, it will try to find an spec using Halo, and if it doesn't find it will fallback to the global specs by doing name.asSpec.

When NamedControls are used the NamedControl class has no access to the proxy so it can't add the spec to the proxies' Halo. It uses UGen.buildSynthDef to add itself to the SynthDef.specs array, which is ignored by getSpec (which is used for building GUIs and so on). Checking the this.specs array before fallback to the global specs would fix that:

        getSpec { |name|
		var spec;
		var specs = Halo.at(this, \spec);
		if (name.isNil) { ^specs };
		if (specs.notNil) { spec = specs.at(name) };
		if (spec.isNil and: { name == '#' }) {
			name = this.key;
			if (specs.notNil) { spec = specs.at(name) };
		};
		^spec ?? { this.specs.at(name) ?? { name.asSpec } }
	}

Hope this makes sense / help :)

@LFSaw
Copy link
Contributor

LFSaw commented Aug 4, 2022

Thanks, @bgola

Do we need to implement this only for Object or also for other Classes?
It is a little difficult to understand where what is used...

also, since not all objects that support Halo also have a spec method, I'd propose

+ Object {
        getSpec { |name|
		var spec;
		var specs = Halo.at(this, \spec);
		if (name.isNil) { ^specs };
		if (specs.notNil) { spec = specs.at(name) };
		if (spec.isNil and: { name == '#' }) {
			name = this.key;
			if (specs.notNil) { spec = specs.at(name) };
		};
		if (this.respondsTo(\specs), {
			^spec ?? { this.specs.at(name) ?? { name.asSpec } }
		}, {
			^spec ?? { name.asSpec }
		})
	}
}

Additionally, the question is really, if we need two concurrent implementations for specs anyhow...

@bgola
Copy link
Contributor

bgola commented Aug 4, 2022

Definitely it is a bit confusing both .specs and .getSpec, they are used in different scenarios apparently. .specs is defined in SynthDef (so mainstream SC code) and .getSpec is JITLibExtensions stuff... I guess the right approach, as it is implemented now, is that JITLib takes care of using "official" SC stuff in the back. And probably we should add some explanation to the docs somehow :)

From what I can see simply implementing it for Object is enough.

@LFSaw
Copy link
Contributor

LFSaw commented Aug 4, 2022

my proposal would be to remove the Halo implementation of spec handling in favour of mainstream SC but keep backwards compatibility by changing the impelemtation of getSpec and addSpec to use specs(_) internally.

@adcxyz what do you think?

@adcxyz
Copy link
Contributor

adcxyz commented May 2, 2023

Hm,
I am not entirely sure that addSpec and getSpec can be moved transparently to use .specs instead of Halo.
would be good to sketch it and see if anything breaks.

@adcxyz
Copy link
Contributor

adcxyz commented May 2, 2023

the other option would be to add inline-defined specs to the tree that getSpec currently uses.

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

4 participants