search

FlaShG / GitMerge-for-Unity

Unity plugin for merging sceneand prefabs files when using git.
GNU General Public License v2.0
197 stars 26 forks source link

Hidden SerializedProperties #15

Closed FlaShG closed 9 years ago

FlaShG commented 9 years ago

Maybe we shouldn't use NextVisible, but Next when iterating over SerializedProperties of Components. Some invisible properties might be worth merging.

randomPoison commented 9 years ago

I'm testing this out now in the hopes that it will help with handling the parent/child hierarchy discussed in #7. It seems to work for the most part, but there are still some new errors and changes cropping up. Also it seems like things might be getting merged differently, in my case it didn't detect the addition of a component where it was handling it before.

So far the most breaking change that I've run into has been the GameObject.Component property:

gameobject component

If I try to push either either side to the center it corrupts the prefab and break the merge process, and sometimes leads to unity crashes, but GitMerge won't let me accept the merge until I've done something with it. I'm guessing that value should just be ignored, but I haven't yet figured out detect that property to skip listing it.

randomPoison commented 9 years ago

One thing I'm noticing with the hidden properties is that GameObject.IsActive on theirs always seems to be False, even when the object was enabled in the scene. Is this something that GitMerge is doing to facilitate the merge process, or is it a quirk of Unity serialization?

FlaShG commented 9 years ago

I can see that showing all hidden properties is a pretty bad thing to to. The component array and a transform's children shouldn't be visible here. Some hidden things might still have to be serialized.

randomPoison commented 9 years ago

So what do we do about the hidden properties? It would be nice if we could just iterate over the hidden properties, or check if a given property is visible, and then just handle hidden properties as special cases, but those don't seem to be an option with the SerializedProperty API. It might be possible to collect a list of all the properties and a list of just the visible ones and then take the difference to find out which ones are hidden. We could then determine if there are any that can be shown safely, or if we need to do any special actions when applying them.

randomPoison commented 9 years ago

Ah, I see now that GitMerge disables and hides "their" objects, which would explain why GameObject.IsActive is always set to false.

FlaShG commented 9 years ago

The thing is that I don't yet know of any hidden properties that would be worth exposing.

randomPoison commented 9 years ago

The list of children is a hidden property, and it seems like isActive is (though I'm not completely sure).

Though even with the children I get that it might not be the best option to directly expose the list of children. Rather, having each child that's been moved show the two parents it might have would probably be easier to manage when doing a merge.

Is there a way to see what all the properties are? Does Unity keep that documented anywhere? That would certainly make it easier to tell what should and shouldn't be merged.

FlaShG commented 9 years ago

The list of children should definitely not be exposed. transform.parent is a property that changes the children of the old and new parent as a side effect. So having that exposed is enough.

I know not of any documentation of those properties.

randomPoison commented 9 years ago

Yeah, I agree with that. There's a need to manipulate hidden properties, but for the most part exposing them directly is a bad idea.

FlaShG commented 9 years ago

I don't see the need to manipulate either. I mean, those are properties you can't even access while regularly using the editor. Why sould they become interesting in a merge? At least, I don't know a hidden property yet that somehow isn't rather edited by side effects of other property changes.

randomPoison commented 9 years ago

I was under the impression that we would need to manipulate the list of children, but based on your latest changes it looks like unity takes care of that automatically.

FlaShG commented 9 years ago

I think this applies to all hidden properties. They are what never can be edited directly in the editor. So whenever there is a conflict in one of those properties, we should, instead of exposing it, find the (wrapper) property that actually has been edited (like Transform.parent) and expose that one instead.

Since I don't know of any more hidden properties that need to be addressed, I guess this issue can be closed for now.