KybernetikGames / animancer

Documentation for the Animancer Unity Plugin.
66 stars 8 forks source link

List element reference mixed #204

Closed SolarianZ closed 2 years ago

SolarianZ commented 2 years ago

Environment

Description

In List, modify a SerializeReference ITransition filed will disturb other list elements.

Reproduction

Steps to reproduce the bug:

  1. Use following codes, create an asset.
  2. Add an element to list, and set the element to ClipTransition, and assign an animation clip.
  3. Add an element to list by click "+" button(Unity will copy last element automatically).
  4. Assign element 1 another animation clip.
  5. You can see that element 0 changed when modify element 1.

Code:

[UnityEngine.CreateAssetMenu(fileName = "AnimancerRepro", menuName = "Test/AnimancerRepro")]
public class AnimancerRepro : UnityEngine.ScriptableObject
{
    public System.Collections.Generic.List<Wrapper> transitions = new System.Collections.Generic.List<Wrapper>();
}

[System.Serializable]
public class Wrapper
{
    [UnityEngine.SerializeReference]
    public Animancer.ITransition transition;
}

demo

SolarianZ commented 2 years ago

In this situation, I also found another issue: When clear list elements(click Reset or set list size to 0), will got an error Could not reach source property name 'transitions.Array.data[0]' while extracting diffs, the reference does not exist in the source serialized data. clear_list_error

KybernetikGames commented 2 years ago

Both of those seem to be Unity bugs. The first can be replicated with the following script that doesn't use Animancer and the second is fixed in Unity 2021.2 (possibly before that, but that's the one I tested with after getting the error in an older version).

using System;
using UnityEngine;

public class NewBehaviourScript : MonoBehaviour
{
    public MyClass[] refs;

    private void Reset()
    {
        refs = new MyClass[] { new MyClass() { transition = new Transition() } };
    }
}

[Serializable]
public class MyClass
{
    [SerializeReference] public Transition transition;
}

[Serializable]
public class Transition
{
    public int value;
}

With a regular serialized list, assigning an item to two elements of the list would deserialize two copies of it, but with a serialized reference list it would only deserialize one and give the list two references to the same object. And when you increase the size of a list in the Inspector it just clones the last item so evidently it's not even bothering to clone it and just adding another reference to the same object.

With Animancer's Transitions, you can work around it by just using the dropdown to select the type of the transition (even if you set it to the already selected one) because that causes it to create a new object so it won't reference the same thing any more.

I'll see if I can come up with a way to detect when two reference fields are actually referencing the same transition so I can show something to point that out.