Search Unity

  1. Unity 6 Preview is now available. To find out what's new, have a look at our Unity 6 Preview blog post.
    Dismiss Notice
  2. Unity is excited to announce that we will be collaborating with TheXPlace for a summer game jam from June 13 - June 19. Learn more.
    Dismiss Notice
  3. Dismiss Notice

Question SerializedReference gets wiped and restoring causes unity to crash

Discussion in 'Editor & General Support' started by sking995, Apr 23, 2024.

  1. sking995

    sking995

    Joined:
    Mar 22, 2021
    Posts:
    26
    I'm using [SerializedReference] to a collection serialize objects of an interface type. I do this because I'm using Generics, and I can't store a collection of my generic type Foo<T>. For some reason, whenever I edit Foo<T> class, Like for example, adding a serialized property, unity decides it doesn't like it and clears the RIDs for half of my assets with this a serialized reference. I try to restore the RIDs with source control, but then when I select the asset, and unity tries to draw the inspector, Unity crashes. Obviously this RID is no longer useful.

    Can someone please help me understand why SerializedReference is so unstable? What am I doing wrong exactly? I don't change the name of the class, or the containing namespace, OR the assembly of the reference object. So WHY does it have to self destruct like this.
     
  2. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    8,401
    How are you modifying the managed references? Can you post some example code?
     
  3. sking995

    sking995

    Joined:
    Mar 22, 2021
    Posts:
    26
    Sure, let me give some context to the files:

    This is for a Modifier system in a game where Modifiers have effects which are applied when a modifier is applied (it's irrelevant what it's applied to). The way we store the modifier effects is via a serialized reference to a collection of IModifierEffects. We use the interface because the concrete modifier effects inherit from ModifierEffect<T> where T is the static game data for the effect (ModifierEffectGameData). Using serialized reference, we can have unique instances of a Modifier Effect that has its own values (i.e, attack speed effect can be +10% for one modifier, and +20% for another). Effects were originally added to the list via ModifierGameDataEditor, where I was literally just using C# Activator to create an instance of the desired ModifierEffect type, and adding it to the collection directly, not via SerializedProperty. I've sinced changed that to be via SerializedProperty because I heard it can make a difference.

    Things more or less work fine until I try to change anything about ModifierEffect<T>. For example, when I added the SerializedField CreatureCategory enum, some fraction of my Modifiers LOST their SerializedReferences. Their RIDS were cleared. When I tried to revert the RIDS in source control, and the other related information like the type, namespace and the assembly, selecting the modifier asset causes the editor to crash. I cannot figure out how restore my references at all because of this.
     

    Attached Files:

  4. sking995

    sking995

    Joined:
    Mar 22, 2021
    Posts:
    26
    Here's what the diff looks like: upload_2024-4-23_12-23-24.png

    When attempting to revert these line changes, selecting the asset causes the editor to crash.
     
  5. sking995

    sking995

    Joined:
    Mar 22, 2021
    Posts:
    26
    upload_2024-4-23_12-24-33.png
    Here's one of the assets, whos reference I tried to restore in source control, that crashes when I click it.

    upload_2024-4-23_12-25-57.png
    Example asset that DIDN'T die when I added the change
     
  6. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    8,401
    Wow there is so much wrong going here.

    Especially this bit of code:
    Code (CSharp):
    1.  private void AddEffect(Type effectType, ModifierGameData data)
    2. {
    3.      var instance = Activator.CreateInstance(effectType) as UnityEngine.Object;
    4.      effectsProperty.arraySize++;
    5.      int newIndex = effectsProperty.arraySize - 1;
    6.      SerializedProperty newElement = effectsProperty.GetArrayElementAtIndex(newIndex);
    7.      newElement.objectReferenceValue = instance;
    8.      serializedObject.ApplyModifiedProperties();
    9.      EditorUtility.SetDirty(data);
    10. }
    This is probably why you're clearing your values. Because if you're not making a Unity object type, and then cast it like this, you get null. If it is a Unity object type, you should never make Unity objects this way.

    If you are making plain C# objects, you need to assign to the managed reference value: https://docs.unity3d.com/ScriptReference/SerializedProperty-managedReferenceValue.html
     
    CodeSmile likes this.
  7. sking995

    sking995

    Joined:
    Mar 22, 2021
    Posts:
    26
    Thanks for looking at that. I actually just recently added that code to attempt to fix my problem. Previously, I was creating an instance of the type the same way as IModifierEffect. Then I was adding this to the modifierEffects collection via the ModifierEffects property directly. But yeah as you said this code is completely wrong. It's not even of type Object like you said. The code has never actually run.

    I'll try fix it anyway with your suggestion.
     
  8. sking995

    sking995

    Joined:
    Mar 22, 2021
    Posts:
    26
    I updated it to this and it seems to add them like before:

    Code (CSharp):
    1.     private void AddEffect(Type effectType, ModifierGameData data)
    2.     {
    3.         var instance = Activator.CreateInstance(effectType) as IModifierEffect;
    4.         effectsProperty.arraySize++;
    5.         int newIndex = effectsProperty.arraySize - 1;
    6.         SerializedProperty referenceProperty = effectsProperty.GetArrayElementAtIndex(newIndex);
    7.         referenceProperty.managedReferenceValue = instance;
    8.         serializedObject.ApplyModifiedProperties();
    9.         EditorUtility.SetDirty(data);
    10.     }
     
  9. sking995

    sking995

    Joined:
    Mar 22, 2021
    Posts:
    26
    I figured out the issue. When I added the new serialized field, I guess unity failed to add that to the YAML of some of the host assets of the Serialized Reference. Perhaps there was some exception along the way. Adding the outlined code allowed unity the reconstruct the object.

    upload_2024-4-23_15-55-33.png

    Thanks for your insights, it helped me solve the problem somewhat
     
  10. CodeSmile

    CodeSmile

    Joined:
    Apr 10, 2014
    Posts:
    6,943
    Effectively, it looks like a decent attempt at shooting yourself in the foot. You better refactor that whole as spiney pointed out - a lot of wrongs going on there. :)


    :eek:

    Whatever you do, don't use Activator. This isn't type-safe, you can pass in any type and you get a nullref if the type isn't implementing your interface.

    An Add method should not instantiate objects to begin with. You should pass the already-instantiated effect to the Add method:
    private void AddEffect(IModifierEffect effect)


    No idea why you are also passing that "data" in, because the Add method only marks it dirty unconditionally. The Add() method certainly isn't making changes to the "data" object so it shouldn't be the one calling it dirty, and perhaps it needn't be dirtied at all.

    I see you already have an enum for all modifiers. It really makes no sense then to use a Type to instantiate a modifier effect.

    What you ought to implement is a factory class that instantiates these classes for you, based on their enum value. And then you'd have a simple List<int> (or the enum type) that's perfectly serializable with no fuss. If you do that, you can likely also remove that comment here:
    Because with a factory, the developers are forced to implement a corresponding switch that returns a new instance (again: don't use types).

    Perfectly exemplified by the GetEffectTypes() method that looks through all assemblies ... wait, no, it only looks at Assembly-CSharp which means you aren't using Assembly Definitions (and when you do, your code will break, but if you don't your code will become spaghetti). This is also not going to perform well, and getting worse over time. For that reason Unity has TypeCache.
     
  11. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    8,401
    It's editor code, it doesn't really matter.

    And I'd only say not to use Activator as it just fails for unknown reasons too, sometimes. I prefer to get the ConstructorInfo from the type and check for a parameterless constructor (or any other constructor really), and can properly inform yourself when you have a type without the correct constructor.

    In either case, how else are you going to instantiate the instance from the type alone?

    In the case of the enum, If they're going for this composition design approach, the enum is useless and can be discarded.