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

Database not updated when an asset is renamed #15

Closed
rhys-vdw opened this issue Jun 21, 2023 · 7 comments · Fixed by #17
Closed

Database not updated when an asset is renamed #15

rhys-vdw opened this issue Jun 21, 2023 · 7 comments · Fixed by #17

Comments

@rhys-vdw
Copy link
Contributor

Firstly, thank you for this amazing project, it has become an important part of our workflow.

Our team has reported that references are not updated when an asset is renamed.

To reproduce:

  1. Open curator window
  2. Select asset A that references asset B
  3. Rename asset B to "C"
  4. Observe that neither B nor C appear in A' references
  5. Select asset C and observe that A no longer appears in its "referencers"

Rebuilding the project fixes this problem.

@rhys-vdw
Copy link
Contributor Author

rhys-vdw commented Jul 3, 2023

This also applies to assets that are moved.

@rhys-vdw
Copy link
Contributor Author

rhys-vdw commented Jul 3, 2023

This is coming up a fair bit in my team, so I'm happy to schedule a fix. Are you open to a PR @ogxd?

@ogxd
Copy link
Owner

ogxd commented Jul 3, 2023

Hi @rhys-vdw !
It is supposed to update when an asset is moved (and renamed I guess) but I'll check the reproduction steps when I have some time :)

static AssetMoveResult OnWillMoveAsset(string sourcePath, string destinationPath)

Yes feel free to make a PR if you think you know where the issue lies!

@rhys-vdw
Copy link
Contributor Author

rhys-vdw commented Jul 3, 2023

Good timing. I just started looking into it. I'll let you know how I go.

@rhys-vdw
Copy link
Contributor Author

rhys-vdw commented Jul 3, 2023

It seems a fundamental issue here is that your AssetInfo class stores a list of paths instead of asset GUIDs. Is there a reason why this is the case?

If AssetInfo stored GUIDs then there would be no need to handle the move case at all, because there is no change to references resulting in a file move.

@ogxd
Copy link
Owner

ogxd commented Jul 3, 2023

Mhhh it has been a while since I wrote this, I don't recall the reasons for using a path instead of the Guid. I'll have a look at it! There are probably many improvements that could be done.

@rhys-vdw
Copy link
Contributor Author

rhys-vdw commented Jul 3, 2023

I'm very close to having finished refactoring it to use GUIDs, so you can check out the diff when it's done.

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 a pull request may close this issue.

2 participants