Search Unity

Feedback [Feature Request] Attribute for marking fields variable

Discussion in 'Prefabs' started by MrMatthias, Feb 27, 2019.

  1. MrMatthias

    MrMatthias

    Joined:
    Sep 18, 2012
    Posts:
    191
    It would be gret to be able to mark fields as variable or optionally variable, meaning that instances can change these fields without showing up as overrides. This would be great for things like transforms and labels. A button won't always be positioned and labeled the same. You could show these fields with a different color in the inspector.
     
    NotaNaN, JoNax97, Anisoropos and 3 others like this.
  2. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    I like the idea - these kind of fields would be very useful for handling prefabs. I'm not sure if I like the name "variable" - preferably the names would be very explicit about what's going on.

    Code (csharp):
    1. /*
    2. * foo won't be gotten from the prefab when you create an instance. Instead, it will just use the default value - same as if you add a script instance.
    3. * modifying the field won't show up as an override, and you can't apply any modifications.
    4. * The value of the field is saved in the scene.
    5. */
    6. [PrefabIgnored]
    7. public int foo;
    Code (csharp):
    1. /*
    2. * This field can't be edited in the inspector - it will be grayed out, and always have the field assigned in the prefab.
    3. */
    4. [PrefabLocked]
    5. public int foo;
    These fields would of course not have an effect if the script isn't on a prefab instance.
    The normal rules for nesting (probably) applies - when editing a nested prefab, you can't edit the [PrefabLocked] values from the base, and [PrefabIgnore] values isn't gotten from or applied to the base.
     
  3. IsaiahKelly

    IsaiahKelly

    Joined:
    Nov 11, 2012
    Posts:
    418
    @Baste I also think the name is not ideal but I believe he used the term "variable" because he's taking about overrides (variations) that are simply ignored (never saved to the base prefab). You seem to be talking about the opposite; a static value that can never be overridden. However, I think both concepts could be very useful.

    Code (CSharp):
    1.  
    2. // Value is always independent of base prefab.
    3. [PrefabVariantValue] // or
    4. [PrefabInstanceValue]
    5.  
    6. // Value is always defined by the base prefab.
    7. [PrefabBaseValue] // or
    8. [PrefabStaticValue]
    9.  
     
  4. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    "static" and "instance" means something very specific already, so they're not good names. "variable" is also a thing; yes it means that something varies, but still.

    Maybe [LockedToPrefabValue] and [IndependentFromPrefab]? A bit long, but very clear on what the meaning is.
     
  5. IsaiahKelly

    IsaiahKelly

    Joined:
    Nov 11, 2012
    Posts:
    418
    Yes, the use of "static" and "instance" is probably not ideal but I feel like [LockedToPrefabValue] and [IndependentFromPrefab] are too verbose and don't make the the type of values affected clear. E.g. base, variant, instance, etc. So I think including something like "base" and "variant" in the name is important.

    I also wasn't trying to defend the use of "variable" in the attribute name. I don't think MrMatthias was even suggesting that be the name anyway. Just explaining that he wants the ability to mark something as a variable or optional type value.
     
  6. MrMatthias

    MrMatthias

    Joined:
    Sep 18, 2012
    Posts:
    191
    yeah "variable" is not clear enough since the value can be changed by making an override.
    [ExcludeFromPrefab] and [ExcludeFromPrefab(optional = true)] might be a good choice, it would match the naming of the ExcludeFromPresetAttribute. With optional i mean that the user can decide whether the field should get excluded from the prefab logic. This would require to save that choice for each instance per field, but since this is an editor only feature this wouldn't be that bad.

    Excluding from the prefab logic means:
    • changes don't show up as overrides
    • reverting has no effect on this field
    • applying won't change this field on the prefab
     
  7. MrMatthias

    MrMatthias

    Joined:
    Sep 18, 2012
    Posts:
    191
    Should probably add the Feedback prefix :oops:
     
  8. Roni92pl

    Roni92pl

    Joined:
    Jun 2, 2015
    Posts:
    396
    Id like to upvote that idea seems like obvious must-have feature, I believe Unity already to this for some fields internally.... field "Controller" in Animator component seems to be ignored by override system but I didn't explore that further.
     
  9. Anisoropos

    Anisoropos

    Joined:
    Jul 30, 2012
    Posts:
    102
    After struggling with a PropertyDrawer workaround until we get a built-in attribute as proposed above, my main takeaways are:
    1. Protecting [PrefabOverride] properties from changing the main prefab directly :: This can be done by simply not drawing them on the prefab, or drawing them as non-editable fields, which is straightforward.
    2. Protecting [PrefabOverride] properties from indirect changes to the main prefab (ie. when another instance applies its own changes) :: This can be done by always having the property marked as Dirty on the inspector.
    3. Protecting [PrefabOverride] properties from "revert" operations :: I don't think there is a way to do this via the Drawer.
    // Example use case (Can be attached to non-prefabs as well as prefabs)
    Code (CSharp):
    1. using System;
    2. using UnityEngine;
    3.  
    4. public class MyBehaviour : MonoBehaviour
    5. {
    6.     public int myInt_Shared;
    7.     [PrefabOverride] public int myInt_Override;
    8.  
    9.     public MyStruct myStruct_Shared;
    10.     [PrefabOverride] public MyStruct myStruct_Override;
    11.  
    12.     public MyClass myClass_Shared;
    13.     [PrefabOverride] public MyClass myClass_Override;
    14.  
    15.     [Serializable]
    16.     public struct MyStruct
    17.     {
    18.         public int myInt_Shared;
    19.         [PrefabOverride] public int myInt_Override;
    20.     }
    21.  
    22.     [Serializable]
    23.     public class MyClass
    24.     {
    25.         public int myInt_Shared;
    26.         [PrefabOverride] public int myInt_Override;
    27.     }
    28. }
    // Attribute
    Code (CSharp):
    1. using UnityEngine;
    2.  
    3. public class PrefabOverrideAttribute : PropertyAttribute { }
    // Drawer (Place under /Editor)
    Code (CSharp):
    1. using System.Collections.Generic;
    2. using System.Reflection;
    3. using UnityEditor;
    4. using UnityEngine;
    5.  
    6. [CustomPropertyDrawer(typeof(PrefabOverrideAttribute), true)]
    7. public class PrefabOverrideDrawer : PropertyDrawer
    8. {
    9.     // If we're part of a prefab and that prefab is the source, don't draw
    10.     private bool ShouldDraw(SerializedProperty property) => ShouldDraw(property.serializedObject.targetObject);
    11.     private bool ShouldDraw(Object baseObject) => !PrefabUtility.IsPartOfAnyPrefab(baseObject) || PrefabUtility.GetCorrespondingObjectFromOriginalSource(baseObject) != baseObject;
    12.  
    13.     public override float GetPropertyHeight(SerializedProperty property, GUIContent label)
    14.        => ShouldDraw(property) ? EditorGUI.GetPropertyHeight(property, label) : 0;
    15.     protected virtual void Draw(Rect position, SerializedProperty property, GUIContent label)
    16.         => EditorGUI.PropertyField(position, property, label, true);
    17.  
    18.     public override void OnGUI(Rect position, SerializedProperty property, GUIContent label)
    19.     {
    20.         Object baseObject = property.serializedObject.targetObject;
    21.  
    22.         if (PrefabUtility.IsPartOfAnyPrefab(baseObject))
    23.             HandlePrefab();
    24.  
    25.         if (ShouldDraw(baseObject))
    26.             Draw(position, property, label);
    27.  
    28.         void HandlePrefab()
    29.         {
    30.             // We may be dealing with a field which is part of an object on the base object, and not part of the base object itself
    31.             // In the case of MyScript>myInt, holding object and base object are both this instance of MyScript
    32.             // In the case of MyScript>myStruct>myInt, holding object is the instance of myStruct on MyScript, while base object is the instance of MyScript
    33.             // This is further complicated due to arrays, so all of this is wrapped in an extension method
    34.             KeyValuePair<FieldInfo, object> holdingObjectInfo = property.GetHoldingObject();
    35.  
    36.             FieldInfo holdingObjectAsFieldOfBaseObject = holdingObjectInfo.Key;
    37.             object holdingObject = holdingObjectInfo.Value;
    38.  
    39.             // Force the holding object into a default value
    40.             object currentValue = fieldInfo.GetValue(holdingObject);
    41.  
    42.             fieldInfo.SetValue(holdingObject, default);
    43.  
    44.             // If the holding object is a field of the base object, dirty that as well
    45.             holdingObjectAsFieldOfBaseObject?.SetValue(baseObject, holdingObject);
    46.  
    47.             // Mark it as dirty, so that when we revert to it it's still dirty
    48.             PrefabUtility.RecordPrefabInstancePropertyModifications(baseObject);
    49.  
    50.             // And now go back to the desired value
    51.             fieldInfo.SetValue(holdingObject, currentValue);
    52.  
    53.             // If the holding object is a field of the base object, dirty that as well
    54.             holdingObjectAsFieldOfBaseObject?.SetValue(baseObject, holdingObject);
    55.  
    56.             // Mark it as dirty, so that when we revert to it it's still dirty
    57.             PrefabUtility.RecordPrefabInstancePropertyModifications(baseObject);
    58.         }
    59.     }
    60. }
    // Extensions (Place under /Editor)
    // These extensions are mostly needed for handling sub-fields and arrays
    Code (CSharp):
    1. using System;
    2. using System.Collections;
    3. using System.Collections.Generic;
    4. using System.Reflection;
    5. using UnityEditor;
    6.  
    7. public static class EditorExtensions
    8. {
    9.     public static KeyValuePair<FieldInfo, object> GetHoldingObject(this SerializedProperty property, BindingFlags bindingFlags = BindingFlags.Public | BindingFlags.Static | BindingFlags.NonPublic | BindingFlags.Instance)
    10.     {
    11.         string[] propertyPathParts = property.propertyPath.Split('.');
    12.         UnityEngine.Object baseObject = property.serializedObject.targetObject;
    13.         Type baseType = baseObject.GetType();
    14.         if (propertyPathParts.Length < 2)
    15.         {
    16.             return new KeyValuePair<FieldInfo, object>(null, baseObject);
    17.         }
    18.         else
    19.         {
    20.             string fieldOfBaseType = propertyPathParts[propertyPathParts.Length - 2];
    21.             if (fieldOfBaseType.Contains("["))
    22.             {
    23.                 FieldInfo arrayFieldInfo = baseType.GetFieldRecursively(propertyPathParts[0], bindingFlags);
    24.                 object arrayObj = arrayFieldInfo.GetValue(baseObject);
    25.                 IList iList = (IList)arrayObj;
    26.                 int index = 0;
    27.                 object value = iList[index];
    28.                 return new KeyValuePair<FieldInfo, object>(arrayFieldInfo, value);
    29.             }
    30.             else
    31.             {
    32.                 FieldInfo targetObject_AsFieldOf_baseObject = baseType.GetFieldRecursively(fieldOfBaseType, bindingFlags);
    33.                 object value = targetObject_AsFieldOf_baseObject.GetValue(baseObject);
    34.                 return new KeyValuePair<FieldInfo, object>(targetObject_AsFieldOf_baseObject, value);
    35.             }
    36.         }
    37.     }
    38.  
    39.     public static FieldInfo GetFieldRecursively(this Type type, string fieldName, BindingFlags flags = BINDING_FLAGS_ALL)
    40.     {
    41.         if (type == null) return null;
    42.         FieldInfo fieldInfo = type.GetField(fieldName, flags);
    43.         if (fieldInfo == null) return GetFieldRecursively(type.BaseType, fieldName, flags);
    44.         return fieldInfo;
    45.     }
    46.  
    47.     private const BindingFlags BINDING_FLAGS_ALL = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static;
    48. }
     
    Last edited: Mar 22, 2021
    NotaNaN likes this.