GameObjectMergeActions.FindComponentDifferences() Doesn't Handle Null Components

[randomPoison]

I was trying out GitMerge and I ran into an error at GameObjectExtensions.cs line 17:

var c = go.AddComponent(original.GetType());

In this case, original was null so the call to GetType() was throwing an exception. I walked up the call stack to find the where original was passed in, and found it was in GameObjectMergeActions.FindComponentDifferences():

    private void FindComponentDifferences()
    {
        var ourComponents = ours.GetComponents<Component>(); // <-- one element of ourComponents is null.
        var theirComponents = theirs.GetComponents<Component>();

        //Map "their" Components to their respective ids
        var theirDict = new Dictionary<int, Component>();
        foreach(var theirComponent in theirComponents)
        {
            theirDict.Add(ObjectIDFinder.GetIdentifierFor(theirComponent), theirComponent);
        }

        foreach(var ourComponent in ourComponents)
        {
            // if the component is null there's nothing we can do about it.
            if (ourComponent == null) continue;

            //Try to find "their" equivalent to our Components
            var id = ObjectIDFinder.GetIdentifierFor(ourComponent);
            Component theirComponent;
            theirDict.TryGetValue(id, out theirComponent);

            if(theirComponent) //Both Components exist
            {
                FindPropertyDifferences(ourComponent, theirComponent);
                //Remove "their" Component from the dict to only keep those new to us
                theirDict.Remove(id);
            }
            else //Component doesn't exist in their version, offer a deletion
            {
                actions.Add(new MergeActionDeleteComponent(ours, ourComponent));
            }
        }

This winds up happening if a game object has a missing script. The best solution would be to better handle these cases of missing scripts, but for the time being it would be good to skip over any elements of ourComponents or theirComponents that are null.

If you think this is a reasonable solution, I'll be happy to make a pull request for it.

[FlaShG]

Hm, missing scripts... I cannot think of any solution that would be smarter than just ignoring those components. So yes, a pull request woul be appreciated :smiley:

[randomPoison]

I made a pull request with the changes. That solves the problem of crashing when attempting to merge a scene with missing scripts, but I don't think it should be the final solution. Missing scripts are often caused by bad merges, such as renaming a script file without updating the associated components. It would be better to at least warn the user that this is happening rather than silently discarding the component.