Allow the addReplace action to replace existing nodes while keeping the ... #1130
Conversation
…he same node id. This allows transparent swapping of a synth (e.g. updating synthdefs with a new version) while ensuring all references to the node remain valid. Allow this on Groups for consistancy, though it's functionally identical to freeing all of the Group's children.
| @@ -833,6 +833,8 @@ SCErr meth_s_new(World *inWorld, int inSize, char *inData, ReplyAddress* /*inRep | |||
| case 4 : { | |||
| Node *replaceThisNode = Msg_GetNode(inWorld, msg); | |||
| if (!replaceThisNode) return kSCErr_NodeNotFound; | |||
| Node_RemoveID(replaceThisNode); | |||
|
|
|||
There was a problem hiding this comment.
please follow the coding style of the code of the file you are in. don't mix tabs and spaces. might sound picky, but helps the readability of the code!
There was a problem hiding this comment.
Re white space: I think it might be time to standardise on one style. Whitespace inconsistencies are a very common issue with submitted patches, but derive to a large extent from the fact that massive numbers of files were added that did not follow the original conventions.
I know even mentioning tabs vs. spaces risks starting a holy war, but honestly we should just clean this up.
There was a problem hiding this comment.
tim - fixing, thx.
muellmusik - you're right. It creates problems like this.
There was a problem hiding this comment.
tim - fixing, thx.
muellmusik - you're right. It creates problems like this. And it's annoying.
There was a problem hiding this comment.
Even worse, changing it at this late date would mess up things like git blame, as many lines would be attributed to whomever made the trivial whitespace changes. (This is already true of any whitespace fix commits.) Maybe some git genius knows a way around this…
But it's probably worth it IAC though just to help avoid further inconsistency.
There was a problem hiding this comment.
Git blame can be told to ignore whitespace. Though... I think neither
github nor most gui clients support this, which is where it's most likely
to be used.
- S
On Fri, Jun 13, 2014 at 1:59 PM, muellmusik notifications@github.com
wrote:
In server/scsynth/SC_MiscCmds.cpp:
@@ -833,6 +833,8 @@ SCErr meth_s_new(World inWorld, int inSize, char *inData, ReplyAddress /*inRep
case 4 : {
Node *replaceThisNode = Msg_GetNode(inWorld, msg);
if (!replaceThisNode) return kSCErr_NodeNotFound;
Node_RemoveID(replaceThisNode);
Even worse, changing it at this late date would mess up things like git
blame, as many lines would be attributed to whomever made the trivial
whitespace changes. (This is already true of any whitespace fix commits.)
Maybe some git genius knows a way around this…But it's probably worth it IAC though just to help avoid further
inconsistency.—
Reply to this email directly or view it on GitHub
https://github.com/supercollider/supercollider/pull/1130/files#r13771399
.
There was a problem hiding this comment.
That's better though. People could cl if needed.
There was a problem hiding this comment.
|
apart from the coding style this patch looks good to me. supernova has the same behaviour by the way. |
server: addReplace: replace existing nodes while keeping the same node id.
...same node id.
This allows transparent swapping of a synth (e.g. updating synthdefs with a new version) while ensuring all references to the node remain valid. Allow this on Groups for consistancy, though it's functionally identical to freeing all of the Group's children.