Search Unity

How do I edit prefabs from scripts?

Discussion in 'Prefabs' started by Baste, May 28, 2019.

  1. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    4,207
    This API is a disaster:

    Code (csharp):
    1. var prefabAsset = Selection.activeObject;
    2.  
    3. var root = PrefabUtility.LoadPrefabContents(AssetDatabase.GetAssetPath(prefabAsset));
    4. foreach (var comp in root.GetComponentsInChildren<SomeScript>(true)) {
    5.     Debug.Log("Destroying on " + comp.gameObject.name);
    6.     Object.DestroyImmediate(comp);
    7. }
    8.  
    9. // How do I get the thing to save?
    10. // This doesn't do anything
    11. EditorUtility.SetDirty(root);
    12. EditorSceneManager.MarkSceneDirty(root.scene);
    13.  
    14. // This throws ArgumentException: Can't save a Prefab instance
    15. PrefabUtility.SavePrefabAsset(root);
    16.  
    17. // This nullrefs; the root object doesn't have a prefab stage:
    18. EditorSceneManager.MarkSceneDirty(PrefabStageUtility.GetPrefabStage(root).scene);
    19.  
    20. PrefabUtility.UnloadPrefabContents(root);
    There's a million methods, they all have mile long names, and none of them seems to do what their name implies, and they don't work.


    Again, the API is a disaster:
    - It's very concerned with somewhat opaque internals.
    - Methods are documented for Unity employees, not external users. Here's SavePrefabAsset: "Use this function to write a Prefab from the Library folder back to the Assets folder as a source asset.". I have no clue what you mean by that. I've worked with the engine for years.
    - Very simple tasks - like editing a prefab from script - doesn't have an obvious way to achieve them.
    - Checking if something is a prefab or not is the least intuitive thing in the world, because if you open a prefab in the prefab stage, it's "not a prefab". This is the definition of leaky abstractions.

    The things the new prefab system does with nesting and variants are great. But the API is really bad. I can't get anything done with it before I become too frustrated and give up. It seems easier to write code that parses the yaml file and edits it than to do this the proper way.
     
    PowerhoofDave, NeatWolf and halley like this.
  2. halley

    halley

    Joined:
    Aug 26, 2013
    Posts:
    729
    NeatWolf likes this.
  3. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    4,207
    The solution is pretty straight forwards - mark the correct gameobjects dirty. But there's so much Prefab crud to wade through that I can't see that, because I'm wondering which of a hundred methods I got wrong.
     
    NeatWolf and halley like this.
  4. Mads-Nyholm

    Mads-Nyholm

    Unity Technologies

    Joined:
    Aug 19, 2013
    Posts:
    92
  5. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    4,207
    It helps, but note the general criticism of this thread. There's two method that are named almost exactly the same thing, that does very different things. Even if I read those exact docs, I'm going to remember that there's a method named something with "save", "prefab" and "asset" in it.

    It's also not something I'm going to remember, so every time I'm writing code against this api I'm going to have to remember if I'm "saving a prefab" or if I'm "saving as a prefab".

    There's in general a huge problem where the API is super-concerned about what object we have - is it a prefab instance, is it a prefab, is it a nested prefab, etc - but all of those things are just typed as GameObjects. So our code don't have any type-strong way of handling any of this.


    If you want the new prefabs to have a good API, you'll need to stop referencing the docs and internal abstractions and instead think about what's something that the receiving users can understand.
    A good start would be to introduce a Prefab type that wraps prefab assets and allow us to be very clear about what we're working with.
     
    zornor90, KB73 and halley like this.
  6. zornor90

    zornor90

    Joined:
    Sep 16, 2015
    Posts:
    163
    Yeah this is really bad, if you're forcing users to remember this kind of thing its a sign your API isn't very clear. A good API should be as clear as possible so that users of it don't have to constantly do mental gymnastics each time they want to do something, aka "reduce the cognitive load"
     
    Last edited: May 29, 2019