GitMerge-for-Unity icon indicating copy to clipboard operation
GitMerge-for-Unity copied to clipboard

Think about merging GameObject hierarchies

Open FlaShG opened this issue 11 years ago • 14 comments

Also: Merging a GameObject that has been deleted on one of the branches. "A added a Component, B deleted the whole GameObject."

GameObjects probably need to be displayed in a hierarchy. Child objects probably should not be displayed when the parent is deleted in the current merge state.

I guess that reverting the delete GameObject MergeAction will not recreate deleted child objects right now.

FlaShG avatar Nov 07 '14 10:11 FlaShG

"A added a Component, B deleted the whole GameObject."

  • If this happens, we only get the DeleteGameObject MergeAction, offering us to either keep the GameObject with the added component or delete it. The GameObject version without the component is obsolete either way.

FlaShG avatar Nov 15 '14 13:11 FlaShG

Maybe displaying Transform.parent is enough?

FlaShG avatar Dec 01 '14 22:12 FlaShG

Currently are there any technical limitations on preserving parent/child relationships while doing the merge? When I tried GitMerge it completely undid my scene hierarchy, which meant I couldn't use the results of the merge. If possible I'd like to help get this supported so I can start using GitMerge for my project.

randomPoison avatar Feb 03 '15 23:02 randomPoison

Since the hierarchy consist only of the property Transform.parent. Since that property is serialized like every other property, the tool should not break any hierarchies. However, I only tested that on flat hierarchies yet...

FlaShG avatar Feb 04 '15 05:02 FlaShG

So I put together a simple test to find out in what cases the parent/child relationship doesn't get properly merged. In a scene with a root object with one parent A and a child B, I create a branch where a added a child C and then removed child B on the original branch (so the first branch has just the root A and the second branch has A with a child C). When I tried merging them I wound up with both B and C in the scene, but neither of them were childed to A. From what I can tell, children that haven't moved get merged correctly.

I also tried this with trying to merge hidden properties (as in #15). As it turns out, the parent/child relationship is done as a list of children in the parent, rather than having the child's transform having a link back to the parent:

An example from a different merge.

To properly preserve the hierarchy special effort has to be made to merge the two arrays and then re-connect the parent to the child. As it stands, even when the children list is shown in the merge, you can't use their version because unity gives the error "Transform child has another parent". By the looks of it you can't set an object's children directly, you have to go through the child's transform.parent.

Also, this is something that will benefit from #41. It's really easy to tell what the correct merge is when an object has just been added or removed, but without three-way merging it's impossible to tell what that correct merge is.

randomPoison avatar Feb 12 '15 22:02 randomPoison

The hard part of getting Transform.parent exposed in the merge view is that, when serialized, the parent holds the list of of children in its "m_Children" property. That means we need a special case when comparing the components of an object to check if the component is a transform and, if it is, compare the parent between versions. From there it only gets more difficult since changing the Transform.parent value in the child means actually changing the value of two other objects: the old parent and the new parent. I'm working now on putting this all together, the changes in #43 are a step in the right direction.

randomPoison avatar Feb 14 '15 04:02 randomPoison

Wait, setting the parent via SerializableProperty does not implicitly update the children? :astonished: //edit: I tried some stuff, here's the result:

  • The parent property isn't ever exposed, since it is not a serialized property at all, but a wrapper.
  • What's accessible is the "Father" property, along with the children.
  • We should expose neither. I'll instead add some special mechanics for parenting.

FlaShG avatar Feb 14 '15 07:02 FlaShG

4d80f88 added a MergeAction for parenting.

I'd like to keep this issue open though, as there is still room to think about hierarchy presentation. Deep hierarchies might be quite a pain to merge by setting parent by parent. Also, the order of GameObjects will be broken. I cannot think of a better solution right now though.

FlaShG avatar Feb 14 '15 08:02 FlaShG

~~How did you test out the parenting merge action? Because in the test scene that I've been using for testing parenting it's not detecting the change in parents at all.~~ Never mind, there was a mistake on my part causing the problem.

Another problem that still needs to be addressed is when an object has been added in "theirs" as a child of another object, or has been removed in "ours" but is still present in "theirs". In both of these cases the object shows up as being at the root level of the hierarchy, even though in both "ours" and "theirs" they were childed to another object. It seems that in cases where an object has been added or removed, some kind of reference patching will be needed to make sure that the merged version of the object is correctly childed to "our" version of their parent.

randomPoison avatar Feb 14 '15 15:02 randomPoison

Added this with c3e31c0.

FlaShG avatar Feb 14 '15 16:02 FlaShG

Excellent! This was the biggest problem keeping us from using GitMerge on our scenes. I'll start testing this in our game and let you know how it goes.

randomPoison avatar Feb 14 '15 16:02 randomPoison

It seems like in the case where:

  1. "they" add a new game object to the scene and then
  2. move an existing game object to be a child of that new object

GitMerge does not detect that the moved child's parent has changed. I suspect that this happens when the moved child is diffed before the new parent has been added to ObjectDictionaries, so in my case it wasn't detecting that "their" version of the object had a parent (and "our" version of the object also didn't have a parent, hence no difference). If "our" version of the object had a parent I think it would at least detect that there was a difference, but it wouldn't correctly detect "their" version of the parent.

randomPoison avatar Feb 14 '15 17:02 randomPoison

0e2e29e adds some behvaiour for this problem. Whenever a parent is set, GitMerge tries to find the version of the object existing in the scene. If it doesn't exist at the time, the user is asked whether or not they want to have the missing GameObject added. The MergeAction responsible for the (non-)existence of the Object will be told to do whatever makes the GameObject exist. I will use the new structures for changed "regular" values, too (see #44).

The thing is still not 100% functional, since noone keeps the user from deleting an object (like the parent) that another MergeAction depends on. Deleting a GameObject that has children involved in the merge currently crashes the tool and leaves behind corrupted scene data.

FlaShG avatar Feb 14 '15 19:02 FlaShG

I have a bug that may be related.

My original scene :

  • UIRootCanvas
    • Camp
    • A lot of children (my "screens")
  • EventSystem

My new Scene :

  • MyScene
    • UIRootCanvas (with modifications inside)
      • A lot of children (my "screens", modified)
    • Camp
    • EventSystem

The result of the merge is :

  • MyScene
    • UIRootCanvas (without my modifications)
      • Camp (old version, should have been deleted)
      • A lot of children (my "screens", without their modifications)
    • Camp
    • EventSystem

Edit : I don't know why, seems the last part of my post was lost.

I did the merge correctly, unchecking "auto merge" (I also tried with Auto merging on) and merging each element manually, checking everything. There was a lot of parenting changes, which is good. UIRootCanvas parent was changed correctly, same for EventSystem. Adding the new object "MyScene" automatically added its child "Camp". But nothing about the modifications in UIRootCanvas : just the change of parents. I think that this change overrided the others in some way. By doing this, no conflicts/merge were asked for all its children. The 2 objects "Camp" were apparently not considered the same. No idea why, both are almost similar (very few changes here). And thus they are not merged together.

RDeluxe avatar Feb 19 '15 18:02 RDeluxe