Skip to content

Conversation

alelievr
Copy link
Member

Purpose of this PR

Fix case https://fogbugz.unity3d.com/f/cases/1317156/

Apparently, the AssetDatabase.SaveAssets() function caused the serializedObject of the volume to be invalid and thus all its serialized properties were broken when we tried to access them to create the volume editor. Moving the SaveAssets just after the creation of the volume editor seems to fix the issue.


Testing status

Tested adding and removing components on a Volume profile from a GameObject in the scene, from an asset on the disk and a preset on the disk. Everything looks correct.


Comments to reviewers

@Chman Do you know why the AssetDatabase Save was made between the code that gets the new serialized property for the volume and the editor creation?

Copy link
Contributor

@remi-chapelain remi-chapelain left a comment

Choose a reason for hiding this comment

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

This looks safe and don't have much more testing to do. ✔️
However, discussing it with @iM0ve, we were curious to understand why this regressed ? Do you have more info about it ? :)

@alelievr
Copy link
Member Author

No idea :) could be an AssetDatabase behavior that changed recently too (though I'm not really sure why re-serializing the asset would cause its serialized references to break)

@sebastienlagarde sebastienlagarde marked this pull request as ready for review February 24, 2021 21:29
@sebastienlagarde sebastienlagarde merged commit bf444c8 into hd/bugfix Feb 24, 2021
@sebastienlagarde sebastienlagarde deleted the hd/fix/volume-component-nullref branch February 24, 2021 21:29
@Chman
Copy link
Contributor

Chman commented Feb 25, 2021

Definitely looks like a regression on the ADB side. To be honest I can't remember the rational for this specific part of the code but I know it was highly sensitive to the order in which things are done (at the time anyway, and it was made with ADBv1). Something to do with the order of serialization, Unity writing the file to disk, the editor refresh sequence and whatnot. As long as it works now it looks fine though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants