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

DYN-6664: Fix crash when no asm is found. #14958

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

sm6srw
Copy link
Contributor

@sm6srw sm6srw commented Feb 20, 2024

Purpose

DYN-6664: Fix crash when no asm is found.

A crash still happens when a Dyn file is opened when no ASM is found. This can also be fixed in LibG but fixing it here simplifies the removal of ASM229 in 3.0.4 (where we have no panelling support). This fix works for all version of LibG for ASM230.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

(FILL ME IN) Brief description of the fix / enhancement. Mandatory section

Reviewers

(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)

(FILL ME IN, optional) Any additional notes to reviewers or testers.

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

@sm6srw sm6srw added this to the 3.0.4 milestone Feb 20, 2024
Copy link

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

{
handler(new WorkspacesSettingsChangedEventArgs(enableLegacyPolyCurveBehavior));
}
catch (NullReferenceException)
Copy link
Member

Choose a reason for hiding this comment

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

what is null in this case?

Copy link
Member

Choose a reason for hiding this comment

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

is it hard to add a test for this? I would not have expected this to need a try catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something inside LibG because ASM is missing. I think this fix is the best one for 3.0.4 as I don't think we want to add paneling in 3.0.4. That being said, for master, I could go ahead and fix this in LibG and then remove this try/catch when the new LibG version with that fix is integrated.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think mGeometryFactory is null in this line when ASM is not found.

@sm6srw sm6srw merged commit 4e97151 into DynamoDS:master Feb 22, 2024
21 of 22 checks passed
@sm6srw sm6srw deleted the fix-crash-no-asm branch February 22, 2024 13:12
sm6srw added a commit to sm6srw/Dynamo that referenced this pull request Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants