New function structure?

[redinkinc]

Hello, I modified the Dynanimator Numeric Parameter as shown in the attached screenshot. In my opinion this has several advantages.

• the DynanimatorFunction ExportAsImmage can be used in all other Dynanimator nodes
• the other DynanimatorFunctions (as in this case the Numeric Parameter) can be used in other projects to iterate through parameters without saving the images.

If you agree, I will apply this modification to all nodes and Push/pull this to your repo. If you have already other thoughts on this I would be happy to know about. Thanks, Fabian

[andydandy74]

This looks great! I actually see another advantage: it makes it a lot easier swapping out the image export function against another function like e.g. a SAT Export or a Save As operation for the Revit model.

I would strongly recommend this change. Any objections, @vasshaug & @jbenoit44?

@redinkinc - If we go ahead with this, please keep the two following points in mind:

• From the screenshot, it looks like you're using Dynamo 0.8 - for the time being, we'd like to keep Dynanimator in 0.7.5, so any changes made to Dynanimator should be done in that version (whcih can be installed side by side with 0.8).
• Your proposed change will not work for all nodes. If you look at the DynanimatorFunction Phases node, you will notice that it doesn't use the iteration number for naming, but rather the phase name (which BTW is a design flaw, it should use both like so: iteration + phase name). We will have the same issue once we have nodes for animating sun position and design options. So it would be best if the iteration input of DynanimatorFunction ExportAsImage were called name and expected a string (basically we would have to use String from Object outside this node...).

[andydandy74]

No objections from @jbenoit44 - waiting for @vasshaug ...

[andydandy74]

Green light from @vasshaug, too. Go ahead, Fabian! :-)

[andydandy74]

@redinkinc - I wonder if it may make sense to keep the Transaction.End nodes in the top level Dynanimate nodes (in front of the Function.Compose node).

[redinkinc]

I now versioned to Dynamo 0.7.5.3566 und uploaded my new version to https://github.com/redinkinc/Dynanimator. I called the functions Helper instead of Nested because in my opinion all functions are nested here. I also added the Number Parameter for Export as image as optional, because the node can be used without handing in a number. then a view can be exported with the same node. The version I first intended (see top image of this issue) appeared to be unstable to iteration-handling. Please let me now if this is also feasible for you. Fabian

[andydandy74]

@redinkinc - Yes, I remember now that that was the reason I used the nested custom nodes in the first place. It's awkward, but otherwise the List.Map operation won't execute correctly.

[andydandy74]

@redinkinc - So, having thought about this a little, I still think it's an improvement compared to what we have now and you should go ahead with it. While we are in fact introducing another level of nesting, the advantages you identified are still there - and I think they outweigh the disadvantage of the added complexity of another level of nesting. There is just one more thing: The Number input of the ExportAsImage node should be labeled Name or, perhaps even better, Name Suffix as it may not always be a number. For example, have a look at this change I am planning to make to the DynanimatorFunction Phases node (which btw also shows that we may not always be able to create cleanly separated helper functions):

[redinkinc]

I have also added a new function ExportAsGBXML in my local structure. Question is now if this should also be included in Dynanimator to generate multiple design solutions to evaluate in GBS. When yes, issue #7 is even more important, because "DynanimateFunction Numeric Parameter" is then no longer distinct. Fabian

[andydandy74]

@redinkinc - Regarding the gbXML export - cool!

So far we have avoided adding nodes to Dynanimator that contain "regular" functionality (i.e. nodes that would work perfectly well without Dynanimator). That's the reason why Dynanimator has a dependency on package Clockwork: it uses some of Clockwork's nodes (e.g. to set element transparency) but only does so in wrapping a helper function around the original node. Once @jbenoit44 has migrated the displacement set node he made for last year's hackathon, we'll have another dependency on package SteamNodes.

I would actually like to keep it that way. This way, everyone can maintain their stuff separately. Any chance you could add it to your DynamoUtilities package instead and we make it a dependency?

[andydandy74]

And yes, I am aware that there's Dynamo does not deal well with dependency versions at the moment (as noted in https://github.com/DynamoDS/Dynamo/issues/3433 reported by me and https://github.com/DynamoDS/Dynamo/issues/4384 reported by you). But I am sure that's going to be resolved once they revamp the package manager. Also, if you want to keep DynamoUitilities strictly DLL-Based, I would have no problem at all hosting the gbXML export in package Clockwork... ;-)

[andydandy74]

Merged the pull request #8 and did some tests - everything's great. Thanks again, @redinkinc !