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

[draft] [USDU-370] First pass at adding relative path to UsdAsset. #376

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

vickycl
Copy link
Collaborator

@vickycl vickycl commented Apr 21, 2023

Purpose of this PR

Ticket/Jira #: [USDU-370]

When Unity project directories are moved or shared, the full path that is serialized for an imported asset causes lots of errors due to the path being different. This change attempts to, where possible, save only the relative path inside the Unity project folder, but still fallback to the full path when it's not in the project folder. We then also catch cases more cleanly when they don't exist at the stated path. This awkward code is necessary to not break existing projects when upgrading, and still support importing USD files from outside the project folder.

We could alternatively have a one time fix up style approach that happens on domain reload or something, but this seemed the simplest approach that should catch if the user fixes a missing file.

This version will be non backwards compatible, because the UsdAsset is 'fixed' to contain a m_relativeUsdPath, and this new field doesn't exist on older versions of the package. As many users have had too many issues with 3.0.0-exp.2 that 3.0.0-exp.3 should solve, this shouldn't be too common. However I think we should fully discuss this before landing.

Testing

Functional Testing status:

TBC

Performance Testing status:

TBC

We are adding potential overhead to a getter which is certainly undesirable, however in the majority of cases it should fallback to the same cost as before- a single call to Path.GetFullPath().

Overall Product Risks

Complexity: Low

Halo Effect: Medium

Additional information

Note to reviewers:

Reminder:

  • Add entry in CHANGELOG.md (if applicable)

When Unity projects are moved or shared, the full path that is serialized for an imported asset causes lots of errors. This change attempts to, where possible, save only the relative path inside the Unity project folder, but still fallback to the full path when it's not in the project folder. We then also catch cases more cleanly when they don't exist at the stated path.
@vickycl vickycl marked this pull request as draft April 21, 2023 15:06
@vickycl vickycl requested a review from a team April 27, 2023 15:16
/// The path to determine a path relative to the project folder from. We assume the inputPath is a full path, else this will fail.
/// </param>
private string DetermineRelativePathFromFullPath(string inputPath)
{
Copy link

@CraigHutchinson CraigHutchinson May 2, 2023

Choose a reason for hiding this comment

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

May be worth looking at System.IO.Path.GetRelativePath?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried that first, but it's not available on 2019.4 due to the .NET version it uses 😞

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.

2 participants