Changing the default for GUID regen to FALSE #717
Conversation
SimonDarksideJ
added
the
Ready for review
|
Why though? It's tested and resolves the issues we have. |
|
I agree with @FejZa , when the original assets and profiles were in the projects and not in hidden folders, the regen made sense. But this only turns the generation OFF by default and can still be utilized by platforms as required @StephenHodgson Plus this solves all the pre-existing issues for the 0.2 release. We can review going forward as required, |
|
I'm mainly worried about guid and asset collisions when updating assets in the future. If someone makes an edit to a prefab that has been regenerated then it won't get overwritten or collide with a future update. I'm not entirely convinced this solves the issues we're running into in the best way |
|
Well, as that is not a situation that is likely to be caused today, it isn't an issue. Although I'm also not convinced there would be an issue based on your use case, as these are fixes assets inside our packages. The only way it could be affected is if we replace the assets with new assets, then when the user re-imports the assets there would be redundant (not conflicting assets). Also, this is situation wouldn't be possible as the ability to re-import the assets is only possible if you delete the previous one, since the menu option is not enabled if assets exist. I believe this is a workable solution for the 0.2 release, and with current testing, there are no perceived issues or cases where what you describe could happen (as we aren't changing any assets). The code isn't removed and is still available fo ruse, it is simply off by default and platforms can be configured to activate it as required. |
|
You have a point with assets being overwritten @StephenHodgson. But I agree with Simon, we can resolve that if needed in whatever comes after v0.2. I think it's not as crucial because:
As I said, I get your point and I am all for finding an even better solution to it in the near future. |
|
I'm surprised this is still an issue after we cached the guid map in #710 |
The whole point of copying the assets is for people to modify them bc they can't when embedded in packages. |
|
IMHO if we're not going to regenerate the guids, then there's no point to copying the assets in the first place. We could just keep them embedded in the package instead. |
|
There is a benefit to copying the assets, as keeping source assets in the packages created more issues. Plus as they are in hidden folders now, so they can't conflict and are a good refresh source. Plus copying them fixes a MAJOR issue that users of that other toolkit complain about, having to clone EVERYTHING to just make a simple change. This is simply a case of documentation and use. Going to merge this PR for now and we can re-evaluate going forward in 0.3 |
I disagree, we need to finish this discussion instead of just merging without consensus |
XRTK - Mixed Reality Toolkit Pull Request
Overview
Since the move to place the default profiles and prefabs for all the various platforms to HIDDEN folders, there isn't actually a need to regenerate the GUID's for copied assets by default.
This ensures that any references between packages are retained as the asset guids remain unchanged when copied to a projects asset folder. And since the originals are in hidden folders, there is no conflict anymore.
The facility remains should it be needed, it is simply now OFF by default
Changes
Simply updated the default for GUID regeneration to FALSE within the Core Package Installer script. No other changes needed.
Tested in a new project without issue and previous referencing issues are no longer a problem.