Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Know if a prefab was reverted in custom inspector.

Discussion in 'Scripting' started by lordofduct, Sep 19, 2014.

  1. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,514
    In my custom inspectors sometimes when I revert a prefab instance the old data that was being tracked up to that point may be now bad and I have to reset it. But I don't know if revert has been clicked or not. This causes for me to have a bunch of weird checks on the data every OnGUI/OnInspectorGUI to see if it's in some state that doesn't make sense and just assume that Revert was clicked.

    Does anyone know of a consistent way to check if the user has clicked 'revert' on a prefab instance?
     
  2. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    Do you have an example on how data can suddenly make no sense?
     
  3. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,514
    Lets say I have a collection/array that I track an index for a "selected" entry of. The user adds new entries to the collection and selects the last one. Then the user reverts the data on the prefab. That index is now out of range.

    In such a scenario I currently test if the index is out of range and I reset accordingly.


    Another scenario is I have a component that I use to give more control over setting up and handling animations. It requires an 'Animation' component as well (as that's what does the real animation work). When editing it I have a 'sync animations' button that allows updating the animations between Animation and this CustomAnimationManager (for a lack of a better term). This sync adds and removes animations from the Animation component. This of course puts the AnimationStates into memory for that component... but when you revert the prefab, Animation doesn't purge said memory. So now if I go to sync again, and it goes to remove an animation that was added but then reverted (and subsequently doesn't exist in the 'data' for it, but does in memory), unity throw up a warning in the console that you're attempting to remove an animation that doesn't exist...

    There isn't a whole lot I can do in this situation since the only reason I know its happening is I see the warning in the console. I have no idea how I could track this scenario in code.



    Should I list other possible scenarios? I have several.
     
  4. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    I just don't understand you how end up with an editor that has a physical "copy" of the data and is not picking "fresh" data every times it's refreshed. For the array/list, you should always check that the index is valid. For object, if you do "if (myObject)", Unity will return false if the unmanaged version of the object have been erased.
     
  5. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,514
    The selected index should remain the same between refreshes. So the user can select an index... and be sure it'll remain that index. This is exactly how the built in UnityEditorInternal.ReorderableList works as well to record the index of which entry is selected. And that's actually a 3rd instance I have a problem. The ReorderableList also buggers up various things if you click revert. That being an 'Internal' namespaced object, I'm accepting of the fact it does. Thing is I know why, as it's a problem I ran into on my own terms as well.

    As for your object answer. I don't think you understand what I was saying. Sorry, I wrote that last night after a long day work... A null test isn't going to do me here as the object isn't null... it's out of sync.

    There is the 'data' representation of the object (the serialized object). And the actual managed object in memory. You can modify the one in memory and call 'SerializedObject.Update' to sync it back to the serializedObject. Or you can modify the serializedObject and update the in memory one by calling SerializedObject.ApplyModifiedProperties.

    The Animation component, if I go to add clips to it in code based off a change in the data for my 'CustomAnimationComponent', this modifies the 'in memory' version of the Animation, and the serializedObject data gets updated as a result. The thing is, if I revert the prefab, and the unity 'Animation' component changes its animation clips on itself as a result (because it was reverted) setting its serializedObject data back to what it should be. BUT for some reason then if I go to loop over all the clips/states on the Animation, it returns the clips that were on it pre-revert... as if the 'in memory' version was not updated by the revert (I have no control over this, it's unity's code). Problem is, it's RemoveClip must effect the serializedObject becuase even though it returned the clip as one of its members, if I call to remove it, it says the clip doesn't exist as a member. But no exceptions are thrown... instead it logs a warning (not sure why) which isn't something I can trap and react to.

    Note, this is not a in memory version I create. It's the in memory version that exists for all objects at design time. It's why there is the 'SerializedObject' (the data representation for serialization), and the 'target' object (the in memory representation).
     
    Last edited: Sep 19, 2014
  6. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    Not a null test, a boolean test.

    Unity returns false when the unmanaged version have been destroyed, but the managed one still exist. Unity should destroy the unmanaged representation of object that are no longer applied to a GameObject. If not, that would be a pretty big bug on Unity's side.
     
  7. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,514
    It's not destroyed. It's the object I'm getting back when I get the Animation component. It's not like I'm storing the Animation and accessing it.

    And yes, it's still technically a null test. Anything that inherits from 'UnityEngine.Object' overrides its comparison so that if you compare it to null when it's destroyed it returns true. You can short hand it as well through the implicit conversion as you showed above.

    Really dude... are you going to be this pedantic on the semantics of my statement? Do you think I don't understand the simplest of concepts when it comes to Unity? I've been working with unity for 3 years now, and I've been in software engineering for nearly a decade. I think I learned that most basic of basic concepts in the Unity API.

    Also, here is the code that removes all clips from the animation. '_owner' is my custom animation component. As you can see, yes, there is a null test.

    Code (csharp):
    1.  
    2.             if(_owner.animation != null)
    3.             {
    4.                 var clips = (from a in _owner.animation.Cast<AnimationState>() select a.clip).Distinct().ToArray(); //returns clips
    5.                 foreach(var c in clips)
    6.                 {
    7.                     _owner.animation.RemoveClip(c); //throws warning on a clip it just gave me
    8.                 }
    9.             }
    10.  
    Note, it's all in the same block. A clip that is handed back to me by Animation then throw this warning when I remove it just 1 operation later.
     
    Last edited: Sep 19, 2014
  8. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    I just tried your code, and it works fine. I'm not getting any "data that make no sense", reverted or not. The only thing I removed was "_owner.". Maybe it's your "_owner" which doesn't make sense? Or are you overriding/hiding the "animation" property from MonoBehaviour, maybe caching the Animation component along the way?

    Look, I may be pedantic, but you don't have to be an asshole. You may think you know Unity, but somewhat you can't make that code work while I can. By experience, when that happens, it's usually the simplest things that break. You may not like going over the basic, but you have to admit you would do the same if someone brought you this issue.
     
  9. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,514
    You don't have the necessary steps to recreate that bug. That offending code works for me 99% of the time, it's when certain steps are taken that it doesn't work.

    But more importantly. That wasn't my question. My question was if there is a way to be signaled if a prefab is reverted.

    Note, that animation thing is one of several scenarios where it could be useful.

    Also, as bug testing goes. I'll do that on my own terms, and ask for assistance when necessary. I'm the one with the code, and the steps recreate... I can look into that and figure out the problem. IF I need help, I'll come here and describe it in complete detail with recreatable steps if I need to. But... as I said, that's not the question I came here with. It was an example scenario where what my question was about could be useful for me.

    The question, if there is a way to be signaled when a prefab is reverted, has a very simple answer. If it doesn't, that answer is "No", or maybe it's "I don't know". I don't know, so I'd say, I don't know.

    You may think I'm being an asshole, but you came to this thread derailing it. Implying I don't even understand what the simple task of 'null testing' is. When I said I have, you corrected me on my semantics. Then you go on to then say "you may think you know unity"... yeah, I don't know everything, but I know how to test if an object has been deleted. You don't need to harp on about it. And I know it's often the simple things.

    But that has NOTHING to do with my question, and serves only to be condescending. If I'm an asshole, it's only because you were as well.
     
    Last edited: Sep 19, 2014
  10. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,514
    Also, just to demonstrate to you what the off topic thing is... considering that the thread is thoroughly derailed. Maybe you can play with it.

    This is the barest of code to reproduce.

    Component:
    Code (csharp):
    1.  
    2. using UnityEngine;
    3. using System.Collections.Generic;
    4. using System.Linq;
    5.  
    6. public class CustomAnimator : MonoBehaviour
    7. {
    8.  
    9.     public CustomAnimState[] states;
    10.  
    11.     public void SyncAnimations()
    12.     {
    13.         if (this.animation != null)
    14.         {
    15.             var clips = (from a in this.animation.Cast<AnimationState>() select a.clip).Distinct().ToArray();
    16.             foreach (var c in clips)
    17.             {
    18.                 this.animation.RemoveClip(c);
    19.             }
    20.         }
    21.  
    22.         foreach(var item in states)
    23.         {
    24.             this.animation.AddClip(item.clip, item.name);
    25.  
    26.             //this here causes it to happen, if you comment this out the warning doesn't happen
    27.             var state = this.animation[item.name];
    28.             state.weight = item.weight;
    29.             state.layer = item.layer;
    30.         }
    31.     }
    32.  
    33.     [System.Serializable()]
    34.     public class CustomAnimState
    35.     {
    36.         public string name;
    37.         public AnimationClip clip;
    38.         public int layer = 0;
    39.         public float weight = 1f;
    40.     }
    41.  
    42. }
    43.  
    Inspector:
    Code (csharp):
    1.  
    2. using UnityEngine;
    3. using UnityEditor;
    4. using System.Collections.Generic;
    5.  
    6. [CustomEditor(typeof(CustomAnimator))]
    7. public class CustomAnimatorInspector : Editor
    8. {
    9.  
    10.     public override void OnInspectorGUI()
    11.     {
    12.  
    13.         this.DrawDefaultInspector();
    14.  
    15.         if(GUILayout.Button("Sync Animations"))
    16.         {
    17.             var targ = this.target as CustomAnimator;
    18.             if (targ.animation == null)
    19.             {
    20.                 targ.gameObject.AddComponent<Animation>();
    21.                 targ.animation.playAutomatically = false;
    22.             }
    23.             targ.SyncAnimations();
    24.         }
    25.  
    26.     }
    27.  
    28. }
    29.  
    Steps to reproduce:

    1) add component to a gameobject, it is not necessary to include the 'Animation' component. Though you can if you'd like. Doesn't change the outcome.

    2) In the custom animator add X number of entries (doesn't matter how many). Attach an animation clip to each appropriately, and name it the SAME name as the clip itself.

    3) hit 'sync animation'. An Aniamtion component shall be attached and filled with the appropriate clips.

    4) create prefab from this gameobject, make sure the prefab is saved, hit revert to make sure that the animations remain in both the custom component AND the Animation component.

    5) rename any of the clips on the custom animation component, I just do "asdf"

    6) click 'sync animations'

    7) click 'revert' - all anims in both components are reverted appropriately

    8) click 'sync animations' - warning appears in console

    NOTE - I know what the offending code is that puts the states into memory so that they persist after revert. That code is commented in the component. If you comment out those three lines of code, the warning does not occur. I'm assuming the AnimationState isn't created in memory unless you access it through the Animation component during design time.

    NOTE2 - this actually isn't of utmost importance to me at this time, since I just don't sync the animations at design time anymore. I don't even have the designer attach an Animation anymore, and instead just do that at runtime. Because currently I'm operating under the idea that the 'CustomAnimator' will be used solely, and the 'Animation' was there to just be the work horse. Thing was I tripped over this because I was designing up something that allowed the Animation to be worked dually, which meant a more complex 'sync' method that only removed those added by the CustomAnimator. I backed off as I don't actually need it to operate as such... but I scaled back the code just to demonstrate that it exists.

    NOTE3 - as I mentioned earlier, I don't actually consider this a bug. I'm doing something that Unity did not expect someone to be doing at design time. And that's fine by me. It's a scenario though where my original question could be useful to implement my own way of dealing... if I so wanted to continue down designing that more complex animation sync routine.
     
    Last edited: Sep 19, 2014
  11. LightStriker

    LightStriker

    Joined:
    Aug 3, 2013
    Posts:
    2,717
    Code (CSharp):
    1.             Debug.LogError(animation.GetClipCount());
    2.             Debug.LogError(animation.Cast<AnimationState>().Count());
    They don't returns the same value after a revert. From what I see, the AnimationState is not properly updated to reflect the new AnimationClip list after a prefab action.

    AnimationState is a TrackedReference, and not a UnityEngine.Object. I think they may not be serialized "as is" and are rebuilt once the Animation component is loaded or when clips are changed explicitly using AddClip and RemoveClip. A quick check in a .NET Reflector shows they hold no methods and no variables. It's only a wrapping class around a collection of properties.

    However, even weirder, the failed AnimationState even persist after a context/code reload. The only way I found to purge the messed up AnimationState was to reload the scene itself, showing they are most likely recreated on load and not saved "as is" but created from the list of clip reference.

    I doubt you could do anything, even if you could trap the prefab actions. There's no way for you to erase existing messed up AnimationState. Unity would need to trap their own prefab event and trigger a rebuilt of the AnimationStates.

    To me, it's a nice edge case for the bug database of Unity.