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

Prefab overrides inconsistent - leading to all components on prefabs in scenes being completely dirty #4

Open
TheXRMonk opened this issue Jun 13, 2023 · 21 comments

Comments

@TheXRMonk
Copy link

It might be me who's doing something wrong, since we started using ForceReserializeAssets to automatically update all our assets when we update unity, or when we pulled in this package. But it seems like different values are written to the serialized asset based on whether it's run by ForceReserializeAssets, or inspected in prefab mode, or in scene mode.

Are you able to shine some light on this issue?

image
image

@hybridherbst
Copy link
Contributor

Hmm, I thought we serialize the global object ID of the script file here, which shouldn't change; but looks like it may be the ID of the object instead.

Thanks for the report!

@marwie
Copy link
Member

marwie commented Jun 14, 2023

We're using the global object id yes (as you can see in the string itself too) - I have seen Unity serializing different data depending on context for this before. Not sure what we can do about it - perhaps ignore it when the guid is missing (just 000000)

@HamzaRino
Copy link

HamzaRino commented Jun 14, 2023

hello, sorry if I'm hijacking this issue,

I have more than one question if I may,

I will start with one related to the issue reported here:

I'm interested to know if the "$ {globalObjectId}" suffix is necessary here (at all). for instance it's not being used here

thanks a lot
and keep on rocking the unity tools world!

NOTE: @hybridherbst still waiting for the UPM extensions package to be open sourced as you said in the JetBrains video 😛

@TheXRMonk
Copy link
Author

I'm interested to know if the "$ {globalObjectId}" suffix is necessary here (at all). for instance it's not being used here

Are you using that package instead? Does it have similar issues? Pros/cons?

@hybridherbst
Copy link
Contributor

@marwie I meant we could use the GlobalObjectID of the script file instead of the object (

var identifier = type.AssemblyQualifiedName;
) but on second thought that may be stupid since when the guid changes that would change as well 🙃

@HamzaRino feel free to join our discord https://discord.needle.tools, Package Tools have already been made public on our registry for everyone to use, just not open sourced yet

@HamzaRino
Copy link

HamzaRino commented Jun 19, 2023

I'm interested to know if the "$ {globalObjectId}" suffix is necessary here (at all). for instance it's not being used here

Are you using that package instead? Does it have similar issues? Pros/cons?

yes I settled on using that one instead because I was not sure about the "$ globalObjectId" suffix also about not sure if we need all the heuristics comparisons (LevenshteinDistance for all types?)...
so it seemed like it's too much for me (maybe it covers more use cases for bigger projects)
while the other solution looked much more simpler.
but at the end I had to also make a custom one (based on SolidAlloy's) for us and simply search for files based on their names either using AssetDatabase.Find or simple plain Directory.GetFiles as there are cases where the script is there, unchanged (same path, same name, same GUID) but Unity fails to pick it up (while switching branches or changing packages)...

besides the MissingReferences folder and tools here should probably belong somewhere else?

@HamzaRino
Copy link

@HamzaRino feel free to join our discord https://discord.needle.tools, Package Tools have already been made public on our registry for everyone to use, just not open sourced yet

thanks @hybridherbst
when I tried the package after watching the video it threw errors in our project (I think it was Unity 2021 LTS back then now we are on 2022), I was just curious why the other packages are open while this one is available to use but not open...maybe you want to sell it at some point but hey I'm not complaining I love your work and what you do!

@TheXRMonk
Copy link
Author

TheXRMonk commented Jun 20, 2023

there are cases where the script is there, unchanged (same path, same name, same GUID) but Unity fails to pick it up (while switching branches or changing packages)...

Funny, that's the exact same issue we have and why we need a package like this. We just need to know which asmdef failed to import, then we can just reimport it from within Unity and everything will work fine again. Tried raising the issue in the unity forums but they just dismissed the issue with "the meta file must have changed" and "prove it".

@marwie
Copy link
Member

marwie commented Jun 20, 2023

Are you able to get this information from the data written to the EditorClassIdentifier or is there something missing for you @TheXRMonk ?

@HamzaRino The distance of types etc is only invoked if the script is missing AND you open the relevant foldout in the UI to try to find a matching name. There should be no performance overhead by default after writing the script information into the EditorClassIdentifier field. Let me know if you see cases where that's not the case

@TheXRMonk
Copy link
Author

Are you able to get this information from the data written to the EditorClassIdentifier or is there something missing for you @TheXRMonk ?

Get which information? Why the id changes?

@marwie
Copy link
Member

marwie commented Jun 20, 2023

Which asmdef failed to import

@TheXRMonk
Copy link
Author

Which asmdef failed to import

I don't see how that's relevant. I was just confirming the need for this package because it helps with solving these issues that come up with unity sometimes. Unrelated to wether or not this package is installed.

My issue as originally posted is that the EditorClassIdentifier is serialized differently based on where you inspect the component. Prefab or scene or through the ForceReserializeAssets.

@marwie
Copy link
Member

marwie commented Jun 20, 2023

Ok great 😉

I got that

@marwie
Copy link
Member

marwie commented Jun 24, 2023

Just published an update (1.5.1) that should prevent serializing data like reported above.
I couldnt reproduce it here unfortunately in 2022.3.3 and with ForceReserialize. Could you perhaps check if this fixes the issue for you @TheXRMonk ?

@TheXRMonk
Copy link
Author

TheXRMonk commented Jun 26, 2023

Just published an update (1.5.1) that should prevent serializing data like reported above. I couldnt reproduce it here unfortunately in 2022.3.3 and with ForceReserialize. Could you perhaps check if this fixes the issue for you @TheXRMonk ?

Something still seems to be up;

If I apply all the overwrites that are in my scene to the prefab, it then looks like this in git;
image

All good - then for some reason, I have changes in my scene asset as well, so I just discard those. However, now there are new overwrites to the prefab. I then try to apply those, and this is what I get;
image
Also from what I can see in the 1.5.1 update if the GUID is invalid you just completely skip writing any data. When I force-reserialize all my prefabs there are no changes to any of them, which I guess is good as they are "invalid". However, this also means any missing scripts on those prefabs won't have any data at all unless I've opened and inspected them from a scene and then overwritten the field to the prefab asset. And I guess having multiple instances of the same prefab in a scene would then lead to them constantly overwriting each other as they each have their own GUID?

I think I will give MissingScriptType a try instead. All I need to know is the name of the script in order to know what I'm missing and need to do to fix it. Or am I missing some essential reason for the GUID?

@marwie
Copy link
Member

marwie commented Jun 26, 2023

I checked again why the ObjectID is being serialized:
It's currently just used to find and display the serialized properties in the unity file if the script is missing.

I guess this can also archieved differently (e.g. using a combination of Namespace + Class + some custom integer that we count up once)

@TheXRMonk
Copy link
Author

Just realized SolidAlloy's implementation requires custom editors. 🙃
I don't think we'll need the ability to see serialized properties - when we fix the missing script we will see them then - or alternatively, we can just look at the .asset or prefab.
I'll try to get it working without writing ObjectIDs 👍

@marwie
Copy link
Member

marwie commented Jun 27, 2023

Removed the usage of GlobalObjectId in 1.6.0

@TheXRMonk
Copy link
Author

Damn you're fast!

I meanwhile fixed it myself. I also extracted this functionality;

public static bool UpdateEditorClassIdentifier(Editor editor)
{
	var serializedObject = editor.serializedObject;
	var prop = serializedObject.FindProperty("m_EditorClassIdentifier");
	if (editor.target)
	{
		if (prop != null)
		{
			var type = editor.target.GetType();
			
			var fullName = type.FullName;
			if (fullName == null || prop.stringValue.StartsWith(fullName))
				return false;
			
			var identifier = type.AssemblyQualifiedName;
			if (identifier == null)
				return false;
			
			identifier = string.Join(",", identifier.Split(',').Take(2));
			// var id = GlobalObjectId.GetGlobalObjectIdSlow(editor.target);
			// Prevent serializing invalid asset guids
			// see https://github.com/needle-tools/missing-component-info/issues/4
			// if (id.assetGUID.ToString().StartsWith("0000000000"))
			// {
			// 	return;
			// }
			prop.stringValue = $"{identifier}";// + id;}}
			serializedObject.ApplyModifiedProperties();
			EditorUtility.SetDirty(editor.target);
		}
		return false;
	}

	return true;
}

and call it like this;

foreach (string _prefabPath in EditorFileUtilities.GetAllPrefabs())
{
    GameObject _prefabAsset = AssetDatabase.LoadAssetAtPath<GameObject>(_prefabPath);
    if(_prefabAsset == null)
    {
        Debug.LogError($"Prefab at {_prefabPath} is null");
        continue;
    }
    
    if (PrefabUtility.IsPartOfImmutablePrefab(_prefabAsset))
        continue;

    foreach (Component component in _prefabAsset.GetComponentsInChildren<Component>())
    {
        if(component == null)
        {
            Debug.LogError($"A Component is null on prefab {_prefabAsset.name} at {_prefabPath}");
            continue;
        }
        
        var editor = UnityEditor.Editor.CreateEditor(component);
        if (editor == null)
        {
            Debug.LogError($"Editor for component {component.GetType()} at {_prefabPath} is null");
            continue;
        }
        MissingComponentHelper.UpdateEditorClassIdentifier(editor);
        Object.DestroyImmediate(editor);
    }
    
    PrefabUtility.SavePrefabAsset(_prefabAsset);
}

I saw you made a ForceReserialize menu, which is not enough - an editor needs to be created.

@marwie
Copy link
Member

marwie commented Jun 27, 2023

Ah ok cool, I was thinking about adding something like this earlier - we can add it as a menu item perhaps (?)

About ForceSerialize: Do you think you could send an example?

@TheXRMonk
Copy link
Author

Ah ok cool, I was thinking about adding something like this earlier - we can add it as a menu item perhaps (?)

About ForceSerialize: Do you think you could send an example?

It's the thing i already posted;

[MenuItem("Tools/Update EditorClassIdentifiers/ .asset")]
private static void onClick_UpdateEditorClassIdentifiers_Assets()
{
    var startTimeSeconds = Time.realtimeSinceStartup;
    foreach (string _objectPath in EditorFileUtilities.GetAllAssetsOfObject<Object>())
    {
        Object _objectAsset = AssetDatabase.LoadAssetAtPath<Object>(_objectPath);
        if(_objectAsset == null)
        {
            Debug.LogError($"Asset at {_objectPath} is null");
            continue;
        }
        
        var editor = UnityEditor.Editor.CreateEditor(_objectAsset);
        
        if (editor == null)
        {
            Debug.LogError($"Editor for component {_objectAsset.GetType()} at {_objectPath} is null");
            continue;
        }
        
        UpdateEditorClassIdentifier(editor);
        Object.DestroyImmediate(editor);
        AssetDatabase.SaveAssetIfDirty(_objectAsset);
    }

    Debug.Log($"EditorClassIdentifiers updated for all assets - took {Time.realtimeSinceStartup - startTimeSeconds} seconds");
}

The code above was just with prefabs instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants