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

EMSUSD-1161 - Fix the crash when attempting to save a USD file from USD Layer Editor #3697

Merged
merged 2 commits into from
Apr 11, 2024

Conversation

samuelliu-adsk
Copy link
Collaborator

No description provided.

@samuelliu-adsk samuelliu-adsk self-assigned this Apr 4, 2024
seando-adsk
seando-adsk previously approved these changes Apr 8, 2024
Copy link
Collaborator

@seando-adsk seando-adsk left a comment

Choose a reason for hiding this comment

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

Seems a bit strange that it returns true if the input parent item is null. How is that a safe path to add?

@samuelliu-adsk
Copy link
Collaborator Author

Seems a bit strange that it returns true if the input parent item is null. How is that a safe path to add?

The reason I return true there is that this function checks if "sublayer path that is the same as the item or one of its parent", so if there's no parent layer then there's not gonna be any conflicts...

@seando-adsk seando-adsk added the bug Something isn't working label Apr 8, 2024
@pierrebai-adsk
Copy link
Collaborator

While a corner case, there is no correct answer to the question "is is safe to add something to nothing". One could argue both ways. If it solves a crash, then I think there is no harm.

pierrebai-adsk
pierrebai-adsk previously approved these changes Apr 9, 2024
Copy link
Collaborator

@pierrebai-adsk pierrebai-adsk left a comment

Choose a reason for hiding this comment

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

The only improvement I could think of is documenting the behaviour with null parent layer in the header file.

@samuelliu-adsk samuelliu-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Apr 10, 2024
@seando-adsk seando-adsk merged commit defc281 into dev Apr 11, 2024
11 checks passed
@seando-adsk seando-adsk deleted the samuelliu-adsk/EMSUSD-1161/crash_when_save branch April 11, 2024 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants