RFI: Questions about "ExtensionMethods.IsPrefab" logic

[Trisibo]

I don't quite understand the logic behind some parts of the method ExtensionMethods.IsPrefab, I was wondering if it should be changed or if there's some context I'm missing:

• The check for !gameObject.scene.isLoaded seems redundant, as !gameObject.scene.IsValid() already checks if the scene is valid, and an invalid scene shouldn't be marked as loaded, is there any situation in which that doesn't happen?
• The check gameObject.GetInstanceID() >= 0 doesn't seem safe, at least in the editor, as Unity's documentation states that "the sign of the InstanceID value is not a safe indicator for whether or not the object is persistent". I haven't really been able to create a prefab with a negative instance id, but I don't see anything saying it's not possible now or in the future.
• I don't understand the check !gameObject.hideFlags.HasFlag(HideFlags.HideInHierarchy) at all. The next comment seems to suggest that's to discard prefab children, but I don't get why the HideInHierarchy flag would have anything to do with it.

Wouldn't a check like this suffice?:

return !gameObject.scene.IsValid() && gameObject.transform.parent == null;

[SimonDarksideJ]

Apologies for the delay in response, github rarely sends me notifications these days.

As to your questions, this is a patten I used in most Unity projects relating to whether the instance of an object was spawned from a Prefab or not, critically it can change how components are validated as prefabs can have specific overrides (or cause other non-intentional effects).

In reference to the three points:

• A object can be part of a scene and not fully initialized while the scene is being loaded, an aftereffect especially if you use addressables.
• Unity documentation is not always the best. This solution was devised long ago after MUCH testing.
• If an object is deemed to be hidden in the Hierarchy, then it is not valid for traversal, and including such items can lead to null reference exceptions or other "bad things"

Is there some specific issue you were facing using the toolkit, or was this more of a query related to its implementation?

[Trisibo]

Apologies for the delay in response

No worries.

Is there some specific issue you were facing using the toolkit, or was this more of a query related to its implementation?

Just a query.

A object can be part of a scene and not fully initialized while the scene is being loaded, an aftereffect especially if you use addressables.

Yes, but I don't think you can access the GameObject from them until they are properly initialized, right? In any case, if it was possible and we checked a prefab instance, and assuming scene.IsValid returned false while the scene is loading (which isn't the case in a quick test with LoadSceneAsync(activateOnLoad: false)), scene.isLoaded is also false, so it's redundant and only the next checks are relevant, or am I missing something?

Unity documentation is not always the best.

Absolutely true, but if it says something is not safe or guaranteed to have a value I would follow that advice, it's not the same as it saying "this always has this value" and then finding it doesn't. Even with all the testing possible, you can miss many cases where the assumption fails, or Unity could decide at any point to do some change that breaks the assumption, as the documentation explicitly says the assumption is not correct.

I've been in that situation myself in the past, doing things that the documentation said weren't safe, but in all my tests worked perfectly, only to find it breaking in some later update (and believe me, it wasn't fun at all, as things broke in VERY strange ways that seemed completely unrelated to the actual cause).

If an object is deemed to be hidden in the Hierarchy, then it is not valid for traversal, and including such items can lead to null reference exceptions or other "bad things"

I don't think that's correct, as far as I know hidden objects can be fully accessed and used. But in any case, that's not an issue for the method itself (or it would fail before checking the flags), and checking the flag shouldn't be related to it being a children or not as the comment suggests, or to the prefab check, unless there's something weird I'm missing.