search

BHoM / Rhinoceros_Toolkit

Set of functionalities for communication with Rhinoceros
GNU Lesser General Public License v3.0
7 stars 4 forks source link

Make FromRhino interface methods compliant to avoid stack overflow crash #178

Closed adecler closed 4 years ago

adecler commented 4 years ago

NOTE: Needed for

https://github.com/BHoM/Grasshopper_Toolkit/pull/567

Issues addressed by this PR

The FromRhino interface methods were not compliant (Not starting with I) therefore causing a stack overflow crash when a FromRhino method could not be found for a specific type

Test files

See https://github.com/BHoM/Grasshopper_Toolkit/pull/567#pullrequestreview-533235943 for an example of crash scenario.

Changelog

Additional comments

adecler commented 4 years ago

Note that FromRhino is only used in one place outside this repo and the Grasshopper_Toolkit: https://github.com/BHoM/LadybugTools_Toolkit/blob/ff45fafa051d46255720a4fe5c0d1399f211b3ee/LadybugTools_Engine/Convert/FromHoneybeeSurface.cs#L54

That is already using the as dynamic though therefore bypassing the need for the IFromRhino interface method altogether.

@FraserGreenroyd , still mentioning it here if case you want to leverage the new IFromRhino method at some point.

@alelom , you also had one reference of FromRhino in the Speckle_Toolkit but that code was commented out. So just mentioning it a something to keep in mind if you uncomment that code at some point.

adecler commented 4 years ago

/azp run Rhinoceros_Toolkit.CheckInstaller

azure-pipelines[bot] commented 4 years ago
Azure Pipelines successfully started running 1 pipeline(s).