GameObjects retain a path to the prefab they were originally instantiated from

[Matt9440]

The problem

PrefabInstanceSource returns the value held in PrefabInstanceData.PrefabSource, however, when a prefab is network spawned BreakFromPrefab() is also called. This in turn clears the reference to _prefabInstanceData through ClearPrefabInstance(), resulting in PrefabInstanceSource returning an empty string.

Being able to know which prefab a GameObject originated from is incredibly useful and the lack of this is a problem I and many others have had to face on numerous occasions.

My current workaround is to hold a reference to the prefab within the prefab through a property, this feels like a massive hack. I understand you can also grab the value from PrefabInstanceSource in OnAwake(), but in my experience this is unreliable.

Proposed solution

Obsolete PrefabInstanceSource in favour of a more appropriately named property PrefabSource, use it internally for all the same reasons but retain its value through Clone(), BreakFromPrefab() and NetworkSpawn().

Note

The value held in PrefabSource will only be accessible by the user who instantiated the GameObject, I have not shared this value over the network as I don't believe it's entirely necessary, but I do have a separate branch where this value is shared over the network through ObjectCreateMsg if that is the preferred behaviour.

Related/resolves issues

#4361
#4182
#5633

[garrynewman]

This seems fine to me

[lolleko]

I need to think about this. Definitely needs unit tests.

[Matt9440]

I need to think about this. Definitely needs unit tests.

I'll get some unit tests sorted in the meantime

[lolleko]

Thanks for adding the tests, I can see why you would want this Property and why it's an improvement over the current situation.

However, The problem I have with this change is that it feels like a band-aid fix.
We would set PrefabSource on something which is no longer a prefab, which is confusing and feels wrong to me.

The ideal solution is to stop breaking prefabs on network spawn. Which is definitely possible, the question really is how to do it in a performant non messy way. I have some ideas i want to try.

[Matt9440]

Thanks for adding the tests, I can see why you would want this Property and why it's an improvement over the current situation.

However, The problem I have with this change is that it feels like a band-aid fix. We would set PrefabSource on something which is no longer a prefab, which is confusing and feels wrong to me.

This is partially the reason why I chose to rename the property, to try and avoid confusion. I thought it would only take a user to understand that GameObjects can come from prefabs to understand why PrefabSource would be set on a GameObject, or why it wouldn't for GameObjects that weren't originally prefab instances. The XML summary should clear this up for people who don't, perhaps I named the property poorly?

The ideal solution is to stop breaking prefabs on network spawn. Which is definitely possible, the question really is how to do it in a performant non messy way. I have some ideas i want to try.

I was expecting this reply as it's something that has been mentioned a number of times over the years, unfortunately this is yet to materialize. I would argue that I wouldn't care about prefab instances breaking when network spawned if I still had a way to access the source prefab file, I'm certain others would agree. The solution to the problem here would be a simple call to GameObject.GetPrefab( PrefabSource ).

This is a real pain point when working with prefabs, first highlighted nearly 2 years ago (see #4455). Regardless of the ideal solution, this PR provides an immediate meaningful improvement with very little interference to the solution you're aiming for and I'd still like to see it merged in the meantime.

[lolleko]

I understand why you want an immediate fix, but adding this fix will change our public API.
When we eventually fix it properly we will need to continue to support PrefabSource, because games may started relying on it.
We would also have [Obsolete] PrefabInstanceSource hanging around forever. I would like to avoid this.

Give me a few more weeks more to figure this out properly.

[lolleko]

We merged a different solution, give it a try.