Fix NamedControl 'name' String convertion

[patrickdupuis]

Purpose and Motivation

A NamedControl's name is converted from a Symbol to a String in cases where the .init function is called (when res == nil). I stumbled upon this while testing #3814. This issue should be fixed before that PR gets merged.

I'm not sure how to demonstrate the issue using current develop, but it may be simple enough to understand just by looking at the changes. Some unused variables were removed (ctlName, ctl, & selector), and name stays a Symbol throughout the .init function.

Types of changes

• Bug fix

To-do list

• Code is tested
• All tests are passing
• Updated documentation
• This PR is ready for review

[jamshark70]

I see nothing wrong with this. I worried a bit about ctlName but it really is unused! So, yes, simplify.

I'm just baffled how the IdentityDictionary lookup ever worked before...

[patrickdupuis]

Well name gets converted back to a Symbol later on when a ControlName is created. That's probably why it all works.

[telephon]

Not removing the prefix from the ctlName might result in some ambiguity. Just make sure to ckeck whether something like \t_trig.kr and trig.tr don't interfere in a bad way...

[jamshark70]

Not removing the prefix from the ctlName might result in some ambiguity. Just make sure to ckeck whether something like \t_trig.kr and trig.tr don't interfere in a bad way...

I had the same question, but... in the currently released version, ctlName is assigned in this block and then never used.

https://github.com/supercollider/supercollider/blob/develop/SCClassLibrary/Common/Control/GraphBuilder.sc#L123

Here, there are exactly 3 occurrences of ctlName: once to declare it, and twice in the if where it's assigned, and then... nothing.

So ctlName is cruft, and deleting the variable entirely is correct.

[jamshark70]

Well name gets converted back to a Symbol later on when a ControlName is created. That's probably why it all works.

It's moot because you're fixing it, but...

		name = name.asSymbol;

		this.initDict;
		res = currentControls.at(name);

Dictionary lookup by symbol, ok. Then, if a previously created control is not found:

			res = super.newCopyArgs(name, values, lags, rate, fixedLag).init;
			currentControls.put(name, res);

... where .init converts name to a string and then, immediately after, with no intervening statements, IdentityDictionary storage is based on the string.

I might be misreading but it does look like this couldn't have worked in the past. But with the fix, it won't be a problem, so it's fine.