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

Structure_Engine: flipping of bars fixed to take release into account #3447

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

pawelbaran
Copy link
Member

@pawelbaran pawelbaran commented Dec 15, 2024

Issues addressed by this PR

Closes #3446

Test files

On SharePoint

Changelog

Additional comments

@pawelbaran pawelbaran added the type:bug Error or unexpected behaviour label Dec 15, 2024
@pawelbaran pawelbaran self-assigned this Dec 15, 2024
@pawelbaran pawelbaran requested a review from rwemay as a code owner December 15, 2024 13:47
@pawelbaran
Copy link
Member Author

@peterjamesnugent @IsakNaslundBh when working on this one, I started thinking whether the orientation angle should not be negated too? Not entirely sure about it, but a part of me argued for that - happy for you to take over this and make an informed decision 👍

@pawelbaran
Copy link
Member Author

@BHoMBot check required

Copy link

bhombot-ci bot commented Dec 15, 2024

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

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

@pawelbaran
Copy link
Member Author

@BHoMBot check unit-tests
@BHoMBot check dataset-compliance
@BHoMBot check copyright-compliance

Copy link

bhombot-ci bot commented Dec 15, 2024

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

  • check unit-tests
  • check dataset-compliance
  • check copyright-compliance

Copy link
Member

@peterjamesnugent peterjamesnugent left a comment

Choose a reason for hiding this comment

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

Happy that the script works as intended, minor change for the code if you think it's worth changing.

I have added a check for a horizontal bar, happy this works for the BarRelease.

As discussed offline, the orientational angle should update too, this needs to be set dependent on whether Bar is vertical if not:

  • If Bar is vertical, newOrientationAngle = -orientationalAngle + pi()
  • If Bar is not vertical ,newOrientationAngle = -orientationAngle

I have added a bit in the script to show this.

@pawelbaran pawelbaran force-pushed the BHoM_Engine-#3446-FlipBarBug branch from 9157e9c to 7893070 Compare December 27, 2024 15:16
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.

Structure_Engine: Bar.Flip() does not flip the release
2 participants