change the call for OnReloadEditor to be called from AssetPostprocessor.OnPostProcessAllAssets method instead of [InitializeOnLoadMethod]

[daniel-moonactive]

change the call for OnReloadEditor in NodeEditorAssetModProcessor class to be called from AssetPostprocessor.OnPostProcessAllAssets method instead of [InitializeOnLoadMethod]

going by the documentation of Unity, its not correct to load assets in methods that were invoked by InitializeOnLoadMethod: https://docs.unity3d.com/ScriptReference/InitializeOnLoadAttribute.html

[shinymerlyn]

When is this code actually supposed to be run?

The docs on InitializeOnLoadMethod (https://docs.unity3d.com/ScriptReference/InitializeOnLoadMethodAttribute.html) seem to imply that it gets called when the project loads. Same with the docs for InitializeOnLoad, as mentioned above. Note that those are two different attributes, though they seem to be for similar purposes.

This InitializeOnLoadMethod tagged method gets called every time the appdomain reloads, which is basically whenever you hit the play button. I am not sure this is the intention? The docs make it look like this attribute is supposed to be for calling code on project launch.

If this is intended to only happen on project load, then AssetPostprocessor.OnPostProcessAllAssets doesn't really seem to quite solve the problem either. That gets called when doing asset builds. If you add the optional bool didDomainReload param, that method also seems to get triggered whenever you hit play.

If this code is supposed to only be called on project launch, then I think there isn't really a correct Unity hook for that, at least not on the code side. I think the only solution for that is using Unity's project settings.

The reason I bring this up is that this code is EXTREMELY slow on full production sized projects that have a good number of SOs referenced by node graphs, at least when Visual Studio's debugger is attached. See the ticket I filed for that: https://github.com/Siccity/xNode/issues/351 - I think that whatever solution there is, it should address this problem as well. Would be good to know what the code is even trying to solve.

[shinymerlyn]

Here's a potential implementation:

namespace XNodeEditor {
    /// <summary> Automatically re-add loose node assets to the Graph node list </summary>
    internal sealed class GraphLooseAssetAddAssetProcessor : AssetPostprocessor {
        private static void OnPostprocessAllAssets(
            string[] importedAssets,
            string[] deletedAssets,
            string[] movedAssets,
            string[] movedFromAssetPaths,
            bool didDomainReload)
        {
            for (int i = 0; i < importedAssets.Length; i++)
            {
                string assetpath = importedAssets[i];
                var assetType = AssetDatabase.GetMainAssetTypeAtPath(assetpath);
                if (typeof(XNode.NodeGraph).IsAssignableFrom(assetType))
                {
                    AttachLooseChildNodes(assetpath);
                }
            }

            for (int i = 0; i < movedAssets.Length; i++)
            {
                string assetpath = movedAssets[i];
                var assetType = AssetDatabase.GetMainAssetTypeAtPath(assetpath);
                if (typeof(XNode.NodeGraph).IsAssignableFrom(assetType))
                {
                    AttachLooseChildNodes(assetpath);
                }
            }
        }

        public static void AttachLooseChildNodes(string assetpath)
        {
            XNode.NodeGraph graph = AssetDatabase.LoadAssetAtPath<XNode.NodeGraph>(assetpath);
            if (graph != null)
            {
                graph.nodes.RemoveAll(x => x == null); //Remove null items
                Object[] objs = AssetDatabase.LoadAllAssetRepresentationsAtPath(assetpath);
                // Ensure that all sub node assets are present in the graph node list
                for (int u = 0; u < objs.Length; u++)
                {
                    // Ignore null sub assets
                    if (objs[u] == null) continue;
                    if (!graph.nodes.Contains(objs[u] as XNode.Node)) graph.nodes.Add(objs[u] as XNode.Node);
                }
            }
        }
    }
}

This implementation will get called all the time when assets get edited, but the extra filtering at the top level should help reduce performance drain.

I can make a PR for this if it makes sense, but want some feedback on this approach before I go ahead.

In my project I've added a menu item to do the full scan and rebuild like the old code did, and a unit test that detects if any of the changes that would be made are required (which only adds about 0.5s to the unit test runtime). I'm not sure if they're necessary with the above code in place, but I'm trying to be extra cautious in our project and not break this library's behavior.