Search Unity

3 different behaviours when trying to add nested prefab

Discussion in 'Prefabs' started by MatthieuPr, Nov 1, 2018.

  1. MatthieuPr

    MatthieuPr

    Joined:
    May 4, 2017
    Posts:
    56
    So I have been looking into converting our 2018.2 project into 2018.3 beta, part of this process includes investigating the new prefab system and ECS usability within our project.

    What I can say from the start is that nested prefab is as unfriendly as they come...
    As you can see below in the print screen. I try to add a nested prefab using an editor script and depending on where I am when I do that, the result is completely different. As a bonus I dragged a prefab as a child in there as well and behaviour is different as well.

    So what do we actually see on the picture:
    - 2 prefabs created from editor script in editor mode (grey icon with +)
    - 2 prefabs created from editor script in prefab mode (grey icon)
    - 1 prefab dragged in prefab mode (blue icon)



    Does anybody knows how to create nested prefabs properly from editor script? (I watched the Unite 2018 LA talk, but did not get any workable answer from there)
    Why does the same functionality has 2 very different behaviours in the prefab system?
    Our editor script keeps note of it's children, but when leaving prefab mode back to editor mode the link is broken as well. Why?

    In my opinion either all 4 prefabs created from editor script (nested prefab) should have the grey icon with + or even be properly displayed as nested prefabs. Also if you drag a prefab in editor mode as child of the scrollable, it has blue icon with a + (4th behaviour), could not reproduce all 4 in same print screen due to crashing the editor at 100% reproduction rate. (bug reported already in 2018.3.b04 about this)

    For those wandering the use case: think simple inventory, dynamically adding children to prefab.
    Why have that option in the editor as well? so can visualise how it looks like with 2 items or 153 items...
     

    Attached Files:

  2. runevision

    runevision

    Joined:
    Nov 28, 2007
    Posts:
    1,892
    If those icons are gray, then they are not Prefab instance roots. Did you use Object.Instantiate to create Prefabs from script? That doesn't actually create Prefab instances, it only clones objects. It's meant for runtime use where Prefabs don't exist.

    To create a proper editor Prefab instance, you need to use InstantiatePrefab:
    https://docs.unity3d.com/ScriptReference/PrefabUtility.InstantiatePrefab.html
     
  3. MatthieuPr

    MatthieuPr

    Joined:
    May 4, 2017
    Posts:
    56
    @runevision this is the old code

    Code (CSharp):
    1.  
    2.             // Unity 2018.3b8 new nested prefab system
    3.             // go = PrefabUtility.InstantiatePrefab(prefab) as GameObject;
    4.             go = Instantiate(prefab) as GameObject;
    5.             go.transform.SetParent(transform);
    6.             go.transform.localScale = Vector3.one;
    7.             go.transform.localRotation = Quaternion.identity;
    8.             go.transform.localPosition = Vector3.zero;
    When I change the code to use the commented out line
    Code (CSharp):
    1.  
    2.             // Unity 2018.3b4 new nested prefab system
    3.             go = PrefabUtility.InstantiatePrefab(prefab) as GameObject;
    4.             // go = Instantiate(prefab) as GameObject;
    5.             go.transform.SetParent(transform);
    6.             go.transform.localScale = Vector3.one;
    7.             go.transform.localRotation = Quaternion.identity;
    8.             go.transform.localPosition = Vector3.zero;
    I do get the prefab (blue icon) in stead of the grey icon (wasn't working in b04 so seems fixed now).

    But my script still loses the link to his children when added them by editor script in prefab mode. They are saved as part of the prefab as unmutable nested prefabs and that breaks the entire purpose of the script. Those should be mutable prefabs as the amount need to change at runtime.

    We use this same system for our UI (Text and Buttons) where we modify the nested prefab based on an enum on an editor script (this allows us to have default looks for all our button types and a set amount of text formats). My plan is to recreate those using variants, but they become unmutable at that time on the parent (Button or Text) and thus no longer allows to change them.
     
  4. MatthieuPr

    MatthieuPr

    Joined:
    May 4, 2017
    Posts:
    56
    On a separate note, when I created children by editor script on a prefab, go into prefab mode and create additional children on that prefab, return to editor mode and select the parent of the children the entire editor crashes/freezes...
    I have noticed that in b04 already...
     
  5. runevision

    runevision

    Joined:
    Nov 28, 2007
    Posts:
    1,892
    I'm not sure what you mean by unmutable/immutable nested Prefabs. In what way unmutable?

    If you still get a crash in the latest beta, we'd appreciate a bug report with repro project and repro steps.
     
  6. MatthieuPr

    MatthieuPr

    Joined:
    May 4, 2017
    Posts:
    56
    I was able to file few bugs related to new prefab system including the editor crashing. (ID: 1089493)

    Related to the unmutable part (hard to explain in written form but will try):

    We have several wrapper classes to allow easy customisation by the UI artists without breaking the prefabs in the code or requiring relinking. Main wrappers we are using are for Text, Button and a Collection script who keeps track of children.

    I wil use the ButtonWrapper as example.
    We have custom editor script for the ButtonWrapper that allows the UI artist to change visuals based on templates:

    Code (CSharp):
    1. [CustomEditor(typeof(ButtonWrapper))]
    2. public class ButtonWrapperEditor : Editor
    3. {
    4.     ButtonWrapper.VisualTemplate previousTemplate;  
    5.     ButtonWrapper.ButtonType previousType;
    6.     string previousLabel;
    7.     Sprite previousSprite;
    8.     Sprite previousCostIcon;
    9.  
    10.     ButtonWrapper button;
    11.  
    12.     void Awake()
    13.     {
    14.         if (button == null)
    15.         {
    16.             button = (ButtonWrapper)target;
    17.             SetAllPreviousVariables();
    18.  
    19.             //if (!string.IsNullOrEmpty(button.labelKey))
    20.             {
    21.                 LS.Init();
    22.             }
    23.         }
    24.     }
    25.  
    26.     public override void OnInspectorGUI()
    27.     {
    28.         DrawDefaultInspector();
    29.      
    30.         if (Application.isPlaying)
    31.             return;
    32.      
    33.         if (button == null)
    34.             button = (ButtonWrapper)target;
    35.  
    36.         if (GUILayout.Button("Update") || previousTemplate != button.visualTemplate || previousType != button.btype
    37.             || previousLabel != button.labelKey || previousSprite != button.sprite || previousCostIcon != button.costIcon)
    38.         {
    39.             UpdateButton();
    40.         }
    41.     }
    42.  
    43.     void UpdateButton()
    44.     {
    45.         button.Init(true);
    46.         //if (!string.IsNullOrEmpty(button.labelKey))
    47.         {
    48.             LS.Init();
    49.         }
    50.         SetAllPreviousVariables();
    51.     }
    52.  
    53.     void SetAllPreviousVariables()
    54.     {
    55.         previousTemplate = button.visualTemplate;
    56.         previousType = button.btype;
    57.         previousLabel = button.labelKey;
    58.         previousSprite = button.sprite;
    59.         previousCostIcon = button.costIcon;
    60.  
    61.         string label = LS.Get(button.labelKey);
    62.         if (button.ForceUpperCase)
    63.             label = label.ToUpper();
    64.  
    65.         button.SetLabel(label);
    66.     }
    67. }
    When the template is changed through the editor, it will destroy the old template and create new one from a prefab (with new system this would be different variants)

    Code (CSharp):
    1.  
    2.     public void Init(bool forceIt = false)
    3.     {
    4.         if (!initialized || forceIt)
    5.         {
    6.             initialized = true;
    7.             InstantiateVisuals();
    8.         }
    9.     }
    10.  
    11.     void InstantiateVisuals()
    12.     {
    13.         if (visuals != null)
    14.         {
    15.             if (!Application.isPlaying && Application.isEditor)
    16.                 DestroyImmediate(visuals.gameObject, true);
    17.             else
    18.                 Destroy(visuals.gameObject);
    19.         }
    20.  
    21.         Object template;
    22.         if (cachedTemplates.ContainsKey(visualTemplate))
    23.         {
    24.             template = cachedTemplates[visualTemplate];
    25.         }
    26.         else
    27.         {
    28.             template = Resources.Load("button-templates/"+visualTemplate.ToString(), typeof(ButtonWrapperTemplate));
    29.             cachedTemplates[visualTemplate] = template;
    30.         }
    31.  
    32.         if (template != null)
    33.         {
    34.            visuals = PrefabUtility.InstantiatePrefab(template) as PLRButtonTemplate;
    35.            visuals.transform.SetParent(transform);
    36.         }
    37.         else
    38.         {
    39.             Debug.LogError("Template called " + visualTemplate.ToString() + " not found for " + UnityUtils.FullName(gameObject));
    40.             return;
    41.         }
    42.  
    43.         visuals.transform.localScale = Vector3.one;
    44.         visuals.transform.localPosition = Vector3.zero;
    45.  
    46.         RectTransform rt = visuals.GetComponent<RectTransform>();
    47.         rt.anchorMin = Vector2.zero;
    48.         rt.anchorMax = Vector2.one;
    49.         rt.anchoredPosition = Vector2.zero;
    50.         rt.sizeDelta = Vector2.zero;
    51.         visuals.name = "DONT TOUCH!!!";
    52.  
    53.         SetState(State.normal);
    54.         // DO MORE INIT STUFF HERE
    55.     }
    56.  
    This functionality no longer works with the new prefab system. If the child prefab is generated in Prefab mode, the ButtonWrapper script loses all reference to the ButtonWrapperTemplate. All current ButtonWrapperTemplate objects in are game are not maintained as nested prefabs, but seen as part of the original prefab, this causes error messages to appear when trying to edit them

    ERROR: InvalidOperationException: Destroying a GameObject inside a Prefab instance is not allowed

    So atm the only way to move to nested prefabs is to recreate entire UI, not ideal at all when having 100+ prefabs and screens. But even in that case the difference in behaviour between the 2 edit modes (editor mode and prefab mode) still breaks the ability for the UI artist to quickly change and visualise the work. The scripts auto generate the templates for ease of use, but once leaving the prefab mode it no longer contains the internal link between parent and child.

    Sorry for the long reply, but I hope this clarifies it better...
     
  7. runevision

    runevision

    Joined:
    Nov 28, 2007
    Posts:
    1,892
    I didn't catch how everything works, but I noted a few things:

    You are setting variables without using SerializedProperty and also without using RecordPrefabInstancePropertyModifications. If this is on a Prefab instance, then those modifications will likely not get saved. You need to use one of those two for the changes to be persisted.

    Yes, you can't destroy children inside a Prefab instance. This is not specific to nested Prefabs, you can't destroy or reparent children in Prefab instances in general regardless of whether the children are Prefabs too or not. In previous versions of Unity, doing this would break the Prefab connection and make the instance disconnected. After this, the instance would no longer receive updates from the Asset. This concept was already complex but got too complex when combined with nested Prefabs and Variants, so we had to discontinue support for disconnected Prefab instances. This is why it now generates errors when trying to do these things. If your tooling relies on doing operations that broke the Prefab connection, then you'll need to find other ways to make the tooling work now which does not rely on that. I understand it can be very inconvenient, but it was a tradeoff we had to make to make everything work.
     
  8. MatthieuPr

    MatthieuPr

    Joined:
    May 4, 2017
    Posts:
    56
    It is a fairly simple system but almost inposible to explain with just text, I had same reaction when I started to modify the system :D

    The template should get serialised, it is defined in following way in the ButtonWrapper class:
    [SerializeField][HideInInspector] public ButtonWrapperTemplate visuals;

    part of our coding standards is that all fields should have [SerializeField] and private private access level (except some exceptions like in this case). I will look into the RecordPrefabInstancePropertyModifications, but in current system my best guess is that I should disallow script to run in prefab mode completely, cause the behaviour in prefab mode does not allow for versatility afterwards...
     
  9. runevision

    runevision

    Joined:
    Nov 28, 2007
    Posts:
    1,892
    All right. If there are any remaining issues you believe are bugs, I think we'll need to rely on bug reports, since your setup is a bit complex to understand via forum without being able to dig in to the project and reproduce. Our QA is looking at your existing bug reports already.
     
  10. MatthieuPr

    MatthieuPr

    Joined:
    May 4, 2017
    Posts:
    56
    Thanks for the help, I have 1 more related question:

    Are there any callbacks in editor script to grab entering and leaving prefab mode?
    OnPrefabModeEnter() OnPrefabModeExit()
    This would allow code to be executed when leaving or entering prefab mode (cleaning up the prefab from temporary stuff, adding different InspectorGUI functionality)
    similar in a way as OnInspectorGUI()
     
  11. SteenLund

    SteenLund

    Unity Technologies

    Joined:
    Jan 20, 2011
    Posts:
    639
  12. MatthieuPr

    MatthieuPr

    Joined:
    May 4, 2017
    Posts:
    56
  13. SteenLund

    SteenLund

    Unity Technologies

    Joined:
    Jan 20, 2011
    Posts:
    639