ayezioro
commented
8 years ago Hi Chris and thanks for this thorough input. We know that there is a lot of work to do, and we are now working on some features that go beyond getting a geometric outcome. We will update when time comes and we are more "cooked". As for the rhinocommon, of course we will make the try 9and beyond that). I'm lack of experience with that, and that's why most of the code uses rhinoscript. If you have good pointers to make this less painful i'll appreciate. Point 2 (grid size). Good idea we will implement. Point 3 (outputs).Of course. This is for control of the results. We will hide the unnecessary items (but you should see how it looks the original component ... :-) ) Point 4 (lines/vectors). This is only for visualizing. It will probably dissapear later on, but just for the exercise we will try to have the exact size for each line. Point 5 (rectangle). I'm curious about this sensing. I have an idea already how to do it and will try it. But i know you use this term (sensing) a lot in other components. Will be glad to your sensing approaches. Specially if you have examples of sensing input items, that according to some inputs other input items appear or dissapear or change name. We are probably going to need that. Point 6 (descriptions). Yes ... yes ... The less likable part of all of this, but important. We know, we know ... Point 7 (tolerance) Good idea. but i can see also the advantages of letting users change it. Point 8 (aesthetics). Agree. Those are remainings of the start of the road. I think is good to have the separation lines, but the titles are unnecessary. Thanks again, -A.
chriswmackey
AntonelloDN
This issue was created to give @ayezioro and @AntonelloDN feedback on the new sun shades component that they are creating.
I absolutely agree that we should have the capability to generate shades based on ray-shooting rather than geometric methods as I am convinced that it is faster in cases with atypical geometry and it also seems to have less potential to fail in odd cases like parallel vectors and surfaces, etc. It also appears to be much more resilient in cases where you have very low sun angles, which I know tended to make the older shade component unstable.
Here is my feedback on the current component:
1) I see that the current component is using a lot of rhinoscript inside the component instead of rhinocommon and I would strongly recommend using rhinocommon, for which you can find the full documentation here. I know that it may seem unnecessary when you are just starting to code geometry functions (as it did for me) but please trust me that it really is worthwhile in the end to use rhinocommon as your scripts will be MUCH faster and you will gain a lot more control over the geometry objects. Not having the code in rhinocommon will also make it very difficult to integrate into the new ladybugx and honeybeex libraries, which will be all based in rhinocommon on the GH end (it's of course different for Dynamo). The rhinocommon SDK is also very well documented and working with the documentation will give you some experience that is applicable to how we are documenting the Ladybug API.
2) It is customary in all of the components that subdivide surfaces into a grid of points to ask for a gridSize instead of separate udiv and vdiv. A lot of the time, people do not know what the orientation of U vs V is in their input geometry and it's just much easier to only put in 1 value in rhino model units and use that throughout. If you want to have a default for the that is selected based on the size of the input _window, I would recommend using a bounding box as I do for the shade benefit evaluator.
3) I would suggest hiding outputs that are not critical. I imagine that most people really only want to see the shade surface and are not always interested in all of the points and lines (though they are helpful if people want to dig in). Use code like this to hide these outputs.
4) It's a bit misleading to have the line geometry output when the calculation is actually accounting for intersections that can occur beyond the length of this line. Why not just set the length of the line based on where they intersect the context or the shadeSrf? You can use the 2-point method for line creation instead of point and vector.
5) A number of important descriptions are missing. I am not sure what the rectangularWindow input does or why you would not just have some code that auto-sense whether the input is a rectangle.
6) All of the deluany inputs could use a better description. For example, I am told that the offsetFactor is VERY important but not why it is important or what it is.
7) I would not have tolerance as an input since you almost never want to change it and it can be confusing to users who do not know how rhino common intersections work. I would just always use the Rhino model tolerance, which you get from sc.doc.ModelAbsoluteTolerance.
8) This is more of a personal aesthetic issue but the component seems very large and to have a lot more text in the inputs than is needed. Personally, I don't think that you need to label all of the categories of inputs as people are usually smart enough to figure this out after using the component a couple of times and labeling them might also be confusing to new users who think they are inputs.
-Chris