tt-acm / EnergyAnalysisForDynamo

Dynamo <> Autodesk Green Building Studio interoperability
Other
29 stars 18 forks source link

IsGlazingShaded should set to false if shadingDepth is 0. #67

Closed mostaphaRoudsari closed 10 years ago

mostaphaRoudsari commented 10 years ago

@bhowes-tt and @eertugrul can one of you add this one and update the package? We need an else condition for SetWallSurfaceParameters to remove the shadings if existed when shadingDepth is set to 0. I don't have VS on my laptop! :|

Here is the current code: https://github.com/tt-acm/EnergyAnalysisForDynamo/blob/master/src/EnergyAnalysisForDynamo/PrepareEnergyModel.cs#L565-L570

and here is how it should be:

                //set shading if positive
                if (shadingDepth > 0)
                {
                    surf.IsGlazingShaded = true;
                    surf.ShadeDepth = shadingDepth * UnitConverter.DynamoToHostFactor;
                }
               else
               {
               surf.IsGlazingShaded = false; //remove shading if depth is 0
               surf.ShadeDepth = 0; // I'm not sure if we need this line, previous line should do it for us
               }
eertugrul commented 10 years ago

@mostaphaRoudsari fixed it ! @bhowes-tt has pushed the new (0.2.1) release yesterday, can this wait to next ? I am looking on some other issues came up at ACADIA workshop. We can warp them altogether on next release.

mostaphaRoudsari commented 10 years ago

Thanks Elcin. It will help me for the demo this afternoon but it's not critical. I will just generate the shading once.

eertugrul commented 10 years ago

a quick look on manage packages tab, publishing a new version is only allowed the for current maintainer of the package. Ben is the maintainer of the Energy Analysis for Dynamo package.

:four_leaf_clover: good luck

bhowes-tt commented 10 years ago

I'll ask peter about multiple maintainers ... we should all have push access.

mostaphaRoudsari commented 10 years ago

@eertugrul already addressed this issue. I close this and create a new one for multiple maintainers.