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

Issue when trying to assign a GameObject field wrapped inside another class on a component in a prefab #103

Closed
FlaxenFlash opened this issue Jul 1, 2014 · 2 comments
Labels
Core Area: Duality runtime or launcher Usability Related to API and UI usability

Comments

@FlaxenFlash
Copy link

Assigning a GameObject when handling an AssignFieldError does not create a link in the scene if the GameObject is wrapped in a class on a component in a prefab.

Steps to reproduce:

1.Create the following component:

[Serializable]
    public class TestClass : Component
    {
        public GameObject Target { get; set; }
    }

2.Create a GameObject with a TestClass component
3.Create a second GameObject and set it as the Target on the TestClass
4.Create a third GameObject and add the first two as its children
5.Create a prefab out of the third game object

6.Replace the TestClass code with the following:

    [Serializable]
    public class TestClass : Component
    {
        public Wrapper Wrapper { get; set; }
    }

    [Serializable]
    public class Wrapper
    {
        public GameObject Target { get; set; }
    }

    public class TestSerializationErrorHandler : SerializeErrorHandler
    {
        public override void HandleError(SerializeError error)
        {
            var fieldError = error as AssignFieldError;

            if (fieldError == null)
                return;

            if (!fieldError.FieldName.Contains("Target"))
                return;

            var target = (GameObject)fieldError.FieldValue;
            if (target == null)
                return;

            var testClass = (TestClass)fieldError.TargetObject;
            if (testClass == null)
                return;

            testClass.Wrapper = new Wrapper();
            testClass.Wrapper.Target = target;

            fieldError.AssignSuccess = true;
        }
    }

7.Build the code again

Target on TestClass appears to have been updated correctly, but clicking the eye button beside the reference in the editor does not highlight the object in the scene, revealing the link to be to another instance (one that doesn't exist in the scene).
This all works fine if the Component is not part of a prefab or if the field is not wrapped inside another class

@AdamsLair
Copy link
Collaborator

While the behavior you're reporting is in fact counter-intuitive, it turns out to be intended.

  • It's not an issue with serialization. When you look at the deserialized Prefab, its references are set up just fine. The problem occurs during instantiation of the Prefab, which involves cloning of the main GameObject.
  • Cloning in general is quite tricky, when done fully automated - because the algorithm has to carefully determine which objects to deepcopy, and which to handle as shared references.
  • When cloning in the context of GameObjects, Duality uses heuristic behavior for this: It will assume that all ICollection Types (Lists, Arrays, etc.) are to be cloned deeply and everything else is not.
  • Since Wrapper is not an ICollection, it will be simply assigned as shared reference instead of being deepcloned. And that's the problem here.

To avoid this, you will need to tell Duality that you require custom cloning behavior. There are two ways of doing this:

  1. Override the OnCopyTo method in your Component and handle all the cloning your self. Be aware of the fact that once you implement this, Duality's default fallback behavior will be inactive for that specific Component Type, so you'll have to handle all of your other fields as well.
  2. Use a struct Wrapper instead of a class Wrapper. This is not a general recommendation, I'm just noting here that it would work with a struct, so.. in case there is no actual reasons to prefer a class, you might consider that.

Regarding future outlook, I will definitely implement a better algorithm for determining what instances to handle which way - as soon as one crosses my mind. If you have any ideas, feel free to discuss.

An alternative would be to invent a new field or class attribute to tell the cloning system "This is not an independent kind of object", which would suffice as a criterion to deepclone it wherever encountered.

@AdamsLair AdamsLair added Usability and removed Bug labels Jul 1, 2014
@FlaxenFlash
Copy link
Author

Thanks, I think in this case I'm going to go with the struct. We have done the OnCopy override in a couple of other places previously so I'm aware of the potential pitfalls and would rather avoid them if possible. A struct is fine for my use case anyway.
The attribute approach seems like a decent solution to me. Initially I liked class attribute better, but that won't be much help if the thing you want to control the cloning of isn't something you can change, or isn't something you use exclusively in Duality so maybe field attributes would be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Area: Duality runtime or launcher Usability Related to API and UI usability
Development

No branches or pull requests

1 participant