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(navisworks) CNX-7436 navisworks is sending xzyz flip #3107

Merged
merged 12 commits into from
Dec 13, 2023

Conversation

jsdbroughton
Copy link
Contributor

Description & motivation

In the end a simple fix despite chasing edge cases around the houses.

Changes:

  • Refactor renaming of mode flags to be more descriptive of their purpose
  • Definition of a static const s_canonicalUp to replace a using on every conversion
  • Key change: Additional event handler for the Models collection changing. The internal 'up-ness' is re-evaluated.
  • Comparison to canonicalUp is made, no longer makes reference to canonicalRight

Screenshots:

Before:
image

After:
image

Validation of changes:

Previous example models have been retested when the first XY corrections were made. These continue to be correctly translated.

Checklist:

  • My pull request follows the guidelines in the Contributing guide?
  • My pull request does not duplicate any other open Pull Requests for the same update/change?
  • My commits are related to the pull request and do not amend unrelated code or documentation.
  • My code follows a similar style to existing code.
  • I have added appropriate tests.
  • I have updated or added relevant documentation.

This commit updates the code to set or reset the IsUpright flag instead of the ElevationMode flag for model orientation. The changes are made in multiple files, including ConverterNavisworks.cs and ConverterNavisworks.Geometry.cs. The SetModelOrientationMode method is modified to use the IsUpright flag instead of checking vector matches. Additionally, the PrimitiveProcessor class is updated to use the TargetUpVector method instead of SetElevationModeVector method.
- Simplify variable names for better readability
- Use explicit conditions instead of negating the original condition
This commit adds event handlers to the `ConnectorBindingsNavisworks` class. The `RegisterAppEvents` method now registers event handlers for changes in the document filename, selection sets, and model collection.

A new method `IsInvalidOrClearDocument` has also been added to evaluate if a sender object from an event handler is a valid Document.
Finally, `NullifyCommitCache` has been implemented to reset the commit cache when necessary.
@jsdbroughton jsdbroughton added bug Something isn't working navisworks issues related to Navisworks labels Dec 12, 2023
// TODO: do both need to match or would UP be enough?
ElevationMode = upMatch && rightMatch;
}
private static void SetModelOrientationMode() => IsUpright = VectorMatch(Doc.UpVector, s_canonicalUp);
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@jsdbroughton jsdbroughton merged commit 555cb6f into dev Dec 13, 2023
30 checks passed
@jsdbroughton jsdbroughton deleted the CNX-7436-navisworks-is-sending-xzyz-flip branch December 13, 2023 15:48
@JR-Morgan JR-Morgan added this to the 2.18 milestone Dec 13, 2023
JR-Morgan pushed a commit that referenced this pull request Dec 19, 2023
* Simplify methods consistently using lambda expressions

* Remove commented out code for model collection change event

* Set or reset the IsUpright flag for model orientation

This commit updates the code to set or reset the IsUpright flag instead of the ElevationMode flag for model orientation. The changes are made in multiple files, including ConverterNavisworks.cs and ConverterNavisworks.Geometry.cs. The SetModelOrientationMode method is modified to use the IsUpright flag instead of checking vector matches. Additionally, the PrimitiveProcessor class is updated to use the TargetUpVector method instead of SetElevationModeVector method.

* Update ConverterNavisworks.Geometry.cs to use a constant for the target units in BoxToSpeckle method.

* Refactor geometry path comparison logic

- Simplify variable names for better readability
- Use explicit conditions instead of negating the original condition

* Modify global event handlers for Navisworks document events

This commit adds event handlers to the `ConnectorBindingsNavisworks` class. The `RegisterAppEvents` method now registers event handlers for changes in the document filename, selection sets, and model collection.

A new method `IsInvalidOrClearDocument` has also been added to evaluate if a sender object from an event handler is a valid Document.
Finally, `NullifyCommitCache` has been implemented to reset the commit cache when necessary.

* whitespace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working navisworks issues related to Navisworks
Projects
No open projects
Status: 🧪 Merged (Test)
Development

Successfully merging this pull request may close these issues.

None yet

2 participants