Create doneAction aliases in Done.sc #2616

mossheim · 2017-01-03T15:06:42Z

From discussion at #2601. Very open to suggestions.

Todo:

~~create DoneAction.sc~~ (migrated to Done.sc)
~~create~~ amend Done~~Action~~.schelp
agree on naming for constants

I have no strong preferences here. Several people have tried their hand at it (including myself), and we also have the existing names from ScalaCollider, although some of those seem unhelpful to me.

add convenience methods to Done (discuss)

Not sure about this. Some obvious possibilities are fromValue(int) (returns the name/description of a doneAction code) and values() (returns all possible names), but these would require having an internal dictionary object. Even though it sounds redundant, I think the constants should be kept in either case so as to aid auto-complete.

link to Done~~Action~~ from existing class documentation
update code examples in existing documentation to use Done~~Action~~

They don't all have to, but this would be a quick way to show new users the feature.

migrate/delete UGen-doneAction.schelp (discuss)
~~replace instances of EnvGen.xr(Env ... with the more convenient Env.xr~~ TBD in another PR

telephon · 2017-01-03T15:47:46Z

Just for reference, and as a warning when testing: some done actions don't work as described: #2189.

(edited: they don't work for EnvGen when called within a block after start).

mossheim · 2017-01-03T16:54:10Z

@telephon Thanks, didn't know that.

nhthn · 2017-01-03T21:42:34Z

Thanks for your work on this, Brian.

I slightly prefer Done to DoneAction. Less typing with negligible loss in readability.

After merge, would it be a good idea to start recommending these as best practice? They're so much better than the magic numbers, and we should popularize them by updating the tutorials and examples. Also, while we're at it, we should also promote Env:ar and Env:kr.

vivid-synth · 2017-01-03T21:56:42Z

@snappizz do you propose we fold this into the Done class of Done.kr fame? A little unorthodox maybe but I'd be okay with it.

nhthn · 2017-01-03T22:04:14Z

Advantages of DoneAction: ideological purity, slightly cleaner IDE autocompletion.

Advantages of Done: less typing at no cost to clarity.

Done wins in my book.

nhthn · 2017-01-03T22:14:31Z

HelpSource/Classes/DoneAction.schelp

+## freeSelfAndPrev || 3 || free both this synth and the preceding node
+## freeSelfAndNext || 4 || free both this synth and the following node
+## freeSelfAndFreeAllPrev || 5 || free this synth; if the preceding node is a group then do g_freeAll on it, else free it
+## freeSelfAndFreeAllNext || 6 || free this synth; if the following node is a group then do g_freeAll on it, else free it


How about freeSelfAndFreeAllInPrev and freeSelfAndFreeAllInNext?

works for me.

mossheim · 2017-01-03T22:24:09Z

Yeah, absolutely. I'll move it to Done.

I might be able to do some quick replacements on patterns like EnvGen.xr(Env ... throughout the docs. I actually didn't know about Env.kr/.ar until now, that is such a great shortcut.

mossheim · 2017-01-04T02:24:16Z

looks like there are about 200 instances of EnvGen.[ak]r(Env in the help files. I'm not able to think of a way to automate the replacements. Might be better to leave for a different PR.

nhthn · 2017-01-04T02:25:11Z

Sure thing.

vivid-synth · 2017-01-04T05:25:15Z

HelpSource/Classes/Buffer.schelp

@@ -748,7 +748,7 @@ code::
 s.boot;
 (
 SynthDef(\help_Buffer_copy, { arg out=0, buf;
-	Line.ar(0, 0, dur: BufDur.kr(buf), doneAction: 2); // frees itself
+	Line.ar(0, 0, dur: BufDur.kr(buf), doneAction: Done.freeSelf); // frees itself


Doesn't need comment

vivid-synth · 2017-01-04T05:26:40Z

HelpSource/Classes/Event.schelp

@@ -220,7 +220,7 @@ code::
 SynthDef(\test, { | out = 0, amp = 0.01, freq = #[300,400,400], pan, gate = 1 |
 	var audio, env;
 	audio = Mix.ar(Pulse.ar(freq, 0.5));	// this is a mixture of three oscillators
-	env = Linen.kr(gate, susLevel: amp , doneAction: 2);	// envelope deletes the synt when done
+	env = Linen.kr(gate, susLevel: amp , doneAction: Done.freeSelf);	// envelope deletes the synt when done


Doesn't need comment

mossheim · 2017-01-06T01:13:42Z

@vivid-synth Thanks for the review, I automated those replacements.

Would still like more input on the incomplete tasks I've marked above:

naming of constants
possible convenience methods
possible deletion of UGen-doneAction.schelp

nhthn · 2017-01-06T01:15:09Z

HelpSource/Classes/Done.schelp

+## freeSelfAndPrev || 3 || free both this synth and the preceding node
+## freeSelfAndNext || 4 || free both this synth and the following node
+## freeSelfAndFreeAllPrev || 5 || free this synth; if the preceding node is a group then do g_freeAll on it, else free it
+## freeSelfAndFreeAllNext || 6 || free this synth; if the following node is a group then do g_freeAll on it, else free it


These two names still haven't been updated in the docs.

Thanks, fixed.

nhthn

Looks good to me.

mossheim · 2017-01-15T15:45:28Z

ping. Does anyone mind if I delete UGen-doneAction.schelp? I've already migrated its contents to the Done helpfile and changed all the links.

nhthn · 2017-01-15T17:29:56Z

Yes, go ahead and delete.

mossheim · 2017-01-17T01:40:37Z

I think this is ready to merge if we can get consensus.

vivid-synth · 2017-01-17T23:52:03Z

HelpSource/Classes/Done.schelp

+
+table::
+## name     || value || description
+## doNothing || 0 || do nothing when the UGen is finished


Should we add idle?

Or link to all the aliases actually?

Should we add idle?

Thanks for catching that, I actually used idle instead of doNothing in the .sc file, maybe I should use both?

Or link to all the aliases actually?

Not sure what you mean.

Whoops, I see that's the main one. Nevermind.

mossheim · 2017-01-18T02:06:02Z

@snappizz I think the documentation is equally as important as the new class, so I don't mind keeping it atomic!

@vivid-synth Hmm.. in all the helpfiles for UGens with doneAction arguments, the description for doneAction always points to the DoneAction class, and that seems to be enough to me. However, considering it further, unfortunately the only way to change the display of the default argument values is by editing the .sc file itself, and when I change the default value of e.g. DetectSilence.ar's doneAction to DoneAction.idle, it just shows up in the helpfile as having no default value. Are you OK with having it be an unaliased integer for the default value, but link to the DoneAction helpfile in the description?

nhthn · 2017-01-18T02:12:39Z

You can edit SCDocHTMLRenderer.makeArgString so that it automatically detects a keyword argument named doneAction. I can take care of that.

mossheim · 2017-01-18T02:19:49Z

@snappizz I think one of the whole reasons we took this route was to avoid decisions being made on the basis of a method having an argument named doneAction. I'm not sure amending .makeArgString to account for this exception is a good solution.

nhthn · 2017-01-18T03:12:12Z

I don't see what's wrong with it. No SuperCollider user writes an argument named doneAction unless it's a done action.

mossheim · 2017-01-18T03:37:44Z

I don't see what's wrong with it. No SuperCollider user writes an argument named doneAction unless it's a done action.

Until someone does, and then we run into something like #2614. To state it in more general terms, I want to avoid hardcoded magic words. Here are some more things to consider:

SCDocHTMLRenderer shouldn't have to make decisions based on the contents of the strings it's handling. If it needs to handle more exotic cases, they should be handled as general qualities rather than specific hardcoded instances. In the most basic instance, this would be a boolean flag that could be optionally set by any class.
This ties SCDocHTMLRenderer to the existence of DoneAction, making the code less portable.
The name doneAction may change in the future.
It creates a non-self-documenting precedence for future enum-like classes.

vivid-synth · 2017-01-18T04:59:39Z

I'm fine with merging this and fixing docs separately. Is the issue we want to create "Allow class methods to render as default arguments"?

nhthn · 2017-01-19T20:44:28Z

Yeah, let's deal with that later. I'm sure users are smart enough to figure out that doneAction: 2 and doneAction: Done.freeSelf are the same.

mossheim · 2017-01-19T21:13:08Z

Ok, I think this is ready to merge. Anyone want to give a final look-over?

nhthn · 2017-01-19T22:03:08Z

Not sure about Done.idle... the Synth isn't really idling, is it?

mossheim · 2017-01-19T23:55:43Z

For the record, the reasons I chose idle initially are:

doNothing was not liked by some because it's more keystrokes
noOp or noop is jargony
none made sense when the class was DoneAction and not Done - in other words Done.none is not disclosive of its function
idle (as a verb) is a synonym for "do nothing" and I like the metaphor of an idling engine for a UGen that has completed its task but has not yet been "shut off". I'll admit that it might not work, though, as a metaphor for the behavior of the enclosing synth.

I'm open to reasonable alternatives, or even multiple aliases for 0 as a compromise. I'm going to poll the dev list for opinions, because once in place this is going to be a fairly difficult naming convention to change.

jamshark70 · 2017-01-20T04:32:51Z

I can't quite get on board with idle. What we're doing is keeping the synth active... Maybe Done.keep?

mossheim · 2017-01-20T05:04:55Z

keep is an interesting idea--I've been thinking of 0 as an inherently negative command. What about related commands like stay or live or hang? Or keepSelf as a parallel to freeSelf? (Brings to mind motivational posters--beYourSelf 😺)

nhthn · 2017-01-20T05:58:09Z

I like none (once the bug gets fixed) mostly because the pronunciation of Done.none is amusing to me. keep is also good.

I would prefer Done.free to Done.freeSelf, but the free method has a standard meaning and I'd rather not mess with that.

vivid-synth · 2017-01-28T07:38:40Z

I'm fine with Done.none or Done.idle

telephon · 2017-01-28T17:38:08Z

+1 Done.none

mossheim · 2017-02-03T01:00:33Z

HelpSource/Classes/MIDIClient.schelp

@@ -73,7 +73,7 @@ The number of output ports that SuperCollider created. This is mainly useful to

 METHOD:: getClientID
 Linux only. This gets the client ID by which the MIDIClient is defined in the ALSA subsystem. It can be used to identify whether a port is belonging to this client or another one.
-
+On non-linux systems, this posts a warning and reutrns nil.


Already reverted :)

mossheim · 2017-02-03T01:01:57Z

Since Done.none has the most support (and because we want to brag about our new capability to actually use that name) I've gone with that.

nhthn · 2017-02-03T01:04:27Z

Can you rebase? (doesn't need to be any deeper than squashing the revert)

No longer needed since all this material has been migrated to DoneAction.schelp

mossheim · 2017-02-03T01:21:26Z

OK! First time rebaseing, that was fun.

Brian Heim added 3 commits January 2, 2017 18:23

Add DoneAction class to EnvGen.sc

8402181

standardize naming to outside of class name

1b30ad7

create DoneAction.schelp

e127b63

nhthn added the comp: class library SC class library label Jan 3, 2017

nhthn reviewed Jan 3, 2017

View reviewed changes

mossheim mentioned this pull request Jan 3, 2017

Allow doneActions to be invoked by name #2601

Closed

Brian Heim added 4 commits January 3, 2017 20:41

migrate DoneAction constants to Done.sc

79020cc

Delete DoneAction.schelp

4cc9198

Update code examples to use Done constants

91249bb

Make UGen-doneAction links point to Done

332b9c4

vivid-synth reviewed Jan 4, 2017

View reviewed changes

Remove junk comments, improve readability

03c33d7

nhthn reviewed Jan 6, 2017

View reviewed changes

nhthn approved these changes Jan 6, 2017

View reviewed changes

mossheim added the Work In Progress (WIP) - don't merge yet label Jan 13, 2017

mossheim removed the Work In Progress (WIP) - don't merge yet label Jan 17, 2017

vivid-synth reviewed Jan 17, 2017

View reviewed changes

mossheim added the Work In Progress (WIP) - don't merge yet label Jan 18, 2017

mossheim changed the title ~~Create doneAction enum class~~ Create doneAction aliases in Done.sc Jan 19, 2017

mossheim removed the Work In Progress (WIP) - don't merge yet label Jan 19, 2017

mossheim added the waiting for consensus label Jan 20, 2017

mossheim commented Feb 3, 2017

View reviewed changes

Brian Heim and others added 3 commits February 2, 2017 20:14

Change Done.idle => Done.none

5d69809

Update Done.schelp

bcf438c

Delete UGen-doneActions.schelp

b7034e4

No longer needed since all this material has been migrated to DoneAction.schelp

mossheim removed the waiting for consensus label Feb 3, 2017

mossheim merged commit 4d96b43 into supercollider:master Feb 3, 2017

mossheim deleted the topic-done-action-class branch February 3, 2017 13:48

nhthn added this to the 3.9 milestone Feb 3, 2017

Create doneAction aliases in Done.sc #2616

Create doneAction aliases in Done.sc #2616

Conversation

mossheim commented Jan 3, 2017 • edited Loading

telephon commented Jan 3, 2017 • edited Loading

mossheim commented Jan 3, 2017

nhthn commented Jan 3, 2017

vivid-synth commented Jan 3, 2017

nhthn commented Jan 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mossheim commented Jan 3, 2017

mossheim commented Jan 4, 2017

nhthn commented Jan 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mossheim commented Jan 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhthn left a comment

Choose a reason for hiding this comment

mossheim commented Jan 15, 2017

nhthn commented Jan 15, 2017

mossheim commented Jan 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mossheim commented Jan 18, 2017

nhthn commented Jan 18, 2017

mossheim commented Jan 18, 2017

nhthn commented Jan 18, 2017

mossheim commented Jan 18, 2017

vivid-synth commented Jan 18, 2017

nhthn commented Jan 19, 2017

mossheim commented Jan 19, 2017

nhthn commented Jan 19, 2017

mossheim commented Jan 19, 2017 • edited Loading

jamshark70 commented Jan 20, 2017

mossheim commented Jan 20, 2017

nhthn commented Jan 20, 2017

vivid-synth commented Jan 28, 2017

telephon commented Jan 28, 2017

Choose a reason for hiding this comment

mossheim commented Feb 3, 2017

nhthn commented Feb 3, 2017

mossheim commented Feb 3, 2017

mossheim commented Jan 3, 2017 •

edited

Loading

telephon commented Jan 3, 2017 •

edited

Loading

mossheim commented Jan 19, 2017 •

edited

Loading