Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unwanted conversion from Rhino when expected input type is Rhino #610

Merged
merged 1 commit into from
Feb 24, 2021

Conversation

adecler
Copy link
Member

@adecler adecler commented Feb 20, 2021

Issues addressed by this PR

Closes #502

A few rare components are actually expecting a Rhino type. The FromGoo method was a bit too eager to convert from Rhino regardless of the expected type. The simple fix in this PR corrects that.

Test files

https://burohappold.sharepoint.com/:u:/r/sites/BHoM/02_Current/12_Scripts/02_Pull%20Request/BHoM/Grasshopper_Toolkit/%23502-FromRhinoBug.gh?csf=1&web=1&e=ZMEIif

The top FromRhino is supposed to fail because it is expecting a Rhino sphere but receive a Rhino Brep instead. I appreciate this was the original focus of the issue but this is something to do with the Grasshopper Sphere component, not with the BHoM. The FromRhino(object) component works perfectly fine with this spherical Brep (and is showing in the Rhino viewport) though so all is good in the world 😉 .

While checking this, I realised that other FromRhino components were not working either (e.g. FromRhino(Line)). So this ended up being the focus on this PR.

@adecler adecler added the type:bug Error or unexpected behaviour label Feb 20, 2021
@adecler adecler added this to the BHoM 4.1 β RC milestone Feb 20, 2021
@adecler adecler self-assigned this Feb 20, 2021
@adecler
Copy link
Member Author

adecler commented Feb 20, 2021

@BHoMBot check copyright

@bhombot-ci
Copy link

bhombot-ci bot commented Feb 20, 2021

@adecler to confirm, check-copyright-compliance task is now queued.

@adecler
Copy link
Member Author

adecler commented Feb 20, 2021

@BHoMBot check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Feb 20, 2021

@adecler to confirm, check-installer task is now queued.

Copy link
Member

@alelom alelom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

@adecler adecler merged commit ba89809 into master Feb 24, 2021
@adecler adecler deleted the Grasshopper_Toolkit-#502-BugOnFromRhinoGoo branch February 24, 2021 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grasshopper Goo and FromRhino() method: FromRhino() for Sphere fails
2 participants