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

Nested prefab upgrade fixes #82

Merged
merged 5 commits into from Dec 1, 2022
Merged

Conversation

MerlinVR
Copy link
Collaborator

Adds handling for upgrading nested prefabs from U# 0.x.

Does this by first building a directed acyclic graph out of any prefabs with UdonSharpBehaviours. The DAG is organized where 'parent' nodes are prefabs that do not contain any nested prefabs, but may be contained in other prefabs. The children nodes from those parents are then prefabs that nest the parent prefabs along with any prefab variants of the parents. This allows a topological ordering that visits prefabs in an order that will always visit prefabs that other prefabs nest before the nesting prefabs are visited.

After we have a topological ordering of prefabs, we visit it in reverse order and apply prefab modifications to the serialized string and object reference array fields on all UdonBehaviours that are U# behaviours. This ensures that the following upgrade passes do not get partial/leaked data from their 'parent' prefabs already being upgraded.

We next pass over the topological order and do phase 1 of upgrading prefabs which adds the U# behaviour proxies. This is currently different from the old process in that we now open the prefabs in a temp scene to edit them via PrefabUtility.LoadPrefabContents() because I haven't verified that it's safe to modify nested prefabs using the old method where we just load them directly.

Then we do the same phase 2 upgrade where we pass over all the prefabs again, but now it's in topological order, and we copy the data from the UdonBehaviours to their UdonSharpBehaviour proxies. Finally, calling ApplyProperyModifications on the proxy gets Unity to find the deltas from the 'child' prefabs so you get the proper deltas on the U# proxy.

Not actually being used yet since it needs a chunk of work to integrate cleanly with how I assume prefabs work. The algorithm is theoretically there though.
This was way more annoying to figure out because the API docs for PrefabUtility.GetPropertyModifications() tell you literally nothing of value
- Handles prefabs being linked that don't have U# behaviours now
- Added exception handling that falls back to unsorted prefab set and logs an error in the case of a failure
- Turned assertions into exceptions because assertions don't give enough info
Random thing I saw and wanted to make more consistent
- Fixed marking deltas on prefabs for pre-pass because Unity's API for this is not documented at all and the API itself is the most confusing to use API you could imagine.
@momo-the-monster momo-the-monster changed the base branch from master to releases/1.1.6 November 30, 2022 01:03
Comment on lines +404 to +406
// Deltas are stored per-prefab-instance-root in a given prefab, don't question it. Thanks.
// We take care to not accidentally hit any non-U#-behaviour deltas here
// These APIs are not documented properly at all and the only mentions of them on forum posts are how they don't work with no solutions posted :))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let me know if you'd like to submit a support request to get the official info from Unity on how to do any of this.

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 think it's figured out at this point. It would be good to ask Unity to document it properly though because there's a lot of things you can do with https://docs.unity3d.com/ScriptReference/PrefabUtility.SetPropertyModifications.html that 'work' most of the time but only by happenstance or don't properly cover all cases you'd see prefab modifications happen in. I can also submit an edit for the doc page on Unity but I'm not entirely sure that I'm doing it 100% as it should be done.

Copy link
Collaborator

@momo-the-monster momo-the-monster left a comment

Choose a reason for hiding this comment

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

Excited to get this fix in. I edited this PR to point to a new release branch, would you also create release notes for this branch and add info about this fix?

@MerlinVR MerlinVR changed the base branch from releases/1.1.6 to master December 1, 2022 21:37
@MerlinVR MerlinVR merged commit 3fa998d into master Dec 1, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Dec 1, 2022
@momo-the-monster momo-the-monster deleted the nested-prefab-upgrade-fixes branch May 4, 2023 23:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants