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

BoundingBoxXYZ.FromRevit() added and closest element query tweaked #1279

Merged
merged 7 commits into from
Nov 24, 2022

Conversation

pawelbaran
Copy link
Member

Issues addressed by this PR

Closes #1277
Closes #1278

Test files

Changelog

Additional comments

@pawelbaran pawelbaran added the type:feature New capability or enhancement label Nov 18, 2022
@pawelbaran pawelbaran self-assigned this Nov 18, 2022
@pawelbaran
Copy link
Member Author

@BHoMBot check core
@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 18, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check core
  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

There are 1 requests in the queue ahead of you.

@pawelbaran
Copy link
Member Author

@BHoMBot check versioning

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 18, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check versioning

There are 11 requests in the queue ahead of you.

@pawelbaran
Copy link
Member Author

@BHoMBot check installer
@BHoMBot check serialisation
@BHoMBot check null-handling

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 18, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check installer
  • check serialisation
  • check null-handling

There are 23 requests in the queue ahead of you.

@pawelbaran
Copy link
Member Author

@BHoMBot check installer

2 similar comments
@pawelbaran
Copy link
Member Author

@BHoMBot check installer

@pawelbaran
Copy link
Member Author

@BHoMBot check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 18, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check installer

There are 9 requests in the queue ahead of you.

Copy link
Contributor

@michal-pekacki michal-pekacki left a comment

Choose a reason for hiding this comment

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

Converting to BH.oM.Geometry.BoundingBox will definatelly be useful in geometric methods and I'm happy with this part 👍

One comment to the ClosestElement method:

// get original element location
Location location = originalElement.Location;
if (!(location is LocationPoint))
{
BH.Engine.Base.Compute.RecordError(String.Format("Original element to search for closest element is not point XYZ based. Location of type {0} is not implemented in this search.",location.GetType()));
return null;
}
XYZ point = (location as LocationPoint).Point;

Shouldn't we get the location of non-point elements? We can do it in the same way as in line 129:

searchedPoint = (searchedBoundingBox.Min + searchedBoundingBox.Max) / 2;

@pawelbaran
Copy link
Member Author

Fair point @michal-pekacki, I have rewritten the method to actually cover all types of locations - should be ready to re-review now.

@pawelbaran
Copy link
Member Author

@BHoMBot check core
@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 23, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check core
  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

@pawelbaran
Copy link
Member Author

@BHoMBot check versioning
@BHoMBot check serialisation
@BHoMBot check null-handling

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 23, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check versioning
  • check serialisation
  • check null-handling

There are 2 requests in the queue ahead of you.

Copy link
Contributor

@michal-pekacki michal-pekacki left a comment

Choose a reason for hiding this comment

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

My comment is included, happy to approve 👍

@pawelbaran
Copy link
Member Author

@BHoMBot check installer
@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 24, 2022

@pawelbaran to confirm, the following actions are now queued:

  • check installer
  • check ready-to-merge

@pawelbaran pawelbaran merged commit 479ecde into main Nov 24, 2022
@pawelbaran pawelbaran deleted the Revit_Toolkit-#1277-BoundingBoxConvert branch November 24, 2022 10:44
@pawelbaran pawelbaran changed the title BoundingBoxXYZ.FromRevit() added and closest element query tweaked to optionally include nested curtain elements BoundingBoxXYZ.FromRevit() added and closest element query tweaked Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
None yet
2 participants