Unity-Technologies / UIElementsExamples

Unity project containing examples to use UIElements in the Editor
435 stars 66 forks source link

Misleading comment in Unity Style Sheet Documentation #11

Closed ericdrobinson closed 7 years ago

ericdrobinson commented 7 years ago

The Unity Style Sheets Reference is referred to by the linked UI Elements Documentation in this repo. That document has the following comment:

In the editor, path contain the extension and are relative to any “Resources” folder, similar to what EditorGUIUtility.Load accepts.

It would be far, far better to specifically callout "Assets/Editor Default Resources" directly, rather than insinuate "any 'Resources' folder...". The standard "Resources" folders are not searched by this API but are also far more well-known. The documentation would be much more clear if written as something like:

In the editor, paths contain the extension and are relative to the "Assets/Editor Default Resources" folder (the same as accepted by EditorGUIUtility.Load).

For the record, the documentation is marked "View Only". I would have preferred to leave this as an inline comment, but comment functionality appears to be turned off...

Scorr commented 7 years ago

The docs don't seem to mention this, but a Resources folder placed in an Editor folder (Editor/Resources) is also searched by EditorGUIUtility.Load, without including it in your build. This lets you put your resources in a more relevant location instead of the project-wide "Assets/Editor Default Resources".

ericdrobinson commented 7 years ago

@Scorr Has that always been the case? When did the API begin to work that way? Also, is such a Resources folder required to be directly below the Editor folder or does an Editor folder simply have to exist in the folder's parent hierarchy?

Antoine-Lassauzay commented 7 years ago

Hey, thank you for this feedback. We will work on clarifying the current state of things in the documentation, and we're also working on general improvements to the way we deal with resources from USS.

ericdrobinson commented 7 years ago

@Antoine-Lassauzay Do you happen to know if the EditorGUIUtility.Load function will work with Resources folders found beneath an Editor folder hierarchy?

If so, when did this happen? Has it always been this way?

Scorr commented 7 years ago

@ericdrobinson No idea since when or if it was always the case. Regarding if it needs to be directly below, I haven't tested but considering editor scripts work in sub-folders of an Editor folder I imagine it's the same for Resources.

Seems that the documentation here https://docs.unity3d.com/Manual/SpecialFolders.html briefly mentions it:

Use the EditorGUIUtility.Load function in Editor scripts to load Assets from a Resources folder within an Editor folder. These Assets can only be loaded via Editor scripts, and are stripped from builds.

ericdrobinson commented 7 years ago

@Scorr Thanks for pointing that out. That is an oddity. I've asked some Unity folks for clarification and will report back if I hear anything...

ericdrobinson commented 7 years ago

@Scorr @Antoine-Lassauzay I heard back from an Editor-dev contact I know at Unity about this. Apparently, the EditorGUIUtility.Load function looks like this:

private static Object Load(string filename, Type type)
{
    Object asset = AssetDatabase.LoadAssetAtPath("Assets/Editor Default Resources/" + filename, type);
    if (asset != null)
        return asset;

    AssetBundle bundle = GetEditorAssetBundle();
    if (bundle == null)
    {
        // If in batch mode, loading any Editor UI items shouldn't be needed
        if (Application.isBatchmode)
            return null;
        throw new NullReferenceException("Failure to load editor resource asset bundle.");
    }

    asset = bundle.LoadAsset(filename, type);
    if (asset != null)
        return asset;

    // For user project defined asset paths. Used e.g when specifying icon path for a EditorWindow, with this
    // their icon no longer needs to be located in "Assets/Editor Default Resources" folder in their project.
    return AssetDatabase.LoadAssetAtPath(filename, type);
}

You can see that we're actually both wrong. Here's how it works:

1) Check Assets/Editor Default Resources for the specified asset. 1) Check the built-in Editor Asset Bundles for the specified asset. 1) Attempt to load the asset from anywhere in the project.

The fastest solution is to have your custom assets within Assets/Editor Default Resources. The slowest solution would be to have the custom assets anywhere else. What's more, there is no restriction on assets being within a Resources folder, whether that be under an Editor folder or not.

The documentation, therefore, is wrong in many locations - this includes:

It would be very helpful to clarify this in those locations, or at least unify the suggestions. This function seems to have been expanded over time to support every possible asset location none of the documentation clearly states that fact.

Antoine-Lassauzay commented 7 years ago

Hi, we've update the USS reference document to detail how resource paths are interpreted. Regarding the Scripting/Manual pages, please open bugs in the "Documentation" category. *

Thank you for your feedback.

*edit: via Unity's bug reporting system : https://unity3d.com/unity/qa/bug-reporting

ericdrobinson commented 7 years ago

@Antoine-Lassauzay Thanks for taking a look into this. A quick followup:

The section in question now looks like this:

You attach style sheets to subtree by using the VisualContainer.AddStyleSheetPath() method. The path is a resource path, similarly to what you would give to Resources.Load() (ref).

Does this mean that the VisualContainer.AddStyleSheetPath() function requires that assets exist within a Resources folder hierarchy?

If not, then this should probably be fixed.

If so, then I cannot stress enough how much you should drop the reference to Resources.Load() and describe to users that such resources should exist in a Resources folder that exists within an Editor hierarchy.

The problem is this: by comparing this to Resources.Load() you are telling users that "Resources folders are fine!" But that is very, very dangerous because all assets found within Resources folders are included within builds. This is not the case for Resources folders underneath Editor folders, so this distinction should be clearly spelled out.

Therefore, I would suggest the following:

You attach style sheets to a subtree by using the VisualContainer.AddStyleSheetPath() method. Style sheets passed to this method should be placed in a folder labeled "Resources" located underneath an "Editor" folder. This will keep such assets from needlessly appearing in builds.

Clear, unambiguous, and does not suggest poor asset management habits.

Cameron-Owen commented 2 years ago

5 years on and this is still confusing and examples in the documentation are often wrong.

ZayshaaCodes commented 1 year ago

Another year on, they did nothing to fix the documentation, I've probably reported these issues a couple times now. Please fix the documentation so it's clear how you should be loading assets for editor ui. specifically in cases of custom inspectors and property drawers. if someone at unity knows about the issue, it should not be on us to file the bug report in the right place. you're the ones getting paid, forward a link to this thread to a person paid to deal with bug reports.

To give some semblance of actually useful info here because the first half is missinfo: it seems the best way in cases of an EditorWindow and Editor (both derive from ScriptableObject) is to assign the uxml(VisualTreeAsset) and uss(StyleSheet) asset to the script in the inspector. you will need the editor scripts filename to match the class, and the field must be public or tagged [SerializeField]

In the case of a CustomProperyDrawer... not actually sure, according to this: https://github.com/Unity-Technologies/UIElementsExamples/blob/master/Assets/Examples/Editor/Bindings/CustomPropertyDrawerExamples.cs Unity doesnt know either cause they use AssetDatabase.LoadAssetAtPath<VisualTreeAsset>("Assets/Examples/Editor/Bindings/custom-drawer.uxml"); i'm sure there's some hacky solution for that too so you have a relative path from your editor folder... i havent looked for it yet.