Bug [Worker0] in ScriptableObject.Enable() call is very annoying

We all know ScriptableObject calls are somewhat random, Awake(), OnEnable() - you never really know when they are executed.
Reset() seems to be not executed at all.

And now, since Unity 2020.1.x, I think when the new QuickSearch-Features arrived, the OnEnabled() calls in ScriptableObjects are executed from another Thread.

Unity itself isn't threadsafe, meaning that any calls in the OnEnable() to the UnityAPI or static fields that should be initialized can cause issues.



Is the result of this call:
(when duplicating an ScriptableObject Asset in the Project Window)



As you can see, I wrote a workaround, subscribing to EditorApplication.update, then executing the code, then checking if the class still exists or is NULL ...
(Code below)

This workaround is quite a bit of boilerplate - can we instead have a OnEnable() Method in ScriptableObjects that is only called on the main thread?

And a default OnSceneLoaded() Method would also be quite cool for many different reasons.

(if anyone has a better, cleaner workaround, please tell me)

Code (CSharp):

1.     static List<DbEntrySO<TDBEntrySO>> _entriesToRegister = new List<DbEntrySO<TDBEntrySO>>();
2.  
3.         protected override void OnEnable()
4.         {
5.             base.OnEnable();
6. #if UNITY_EDITOR
7.             Debug.Log("Add ME " + name, this);
8.             _entriesToRegister.Add(this);
9.             UnityEditor.EditorApplication.update -= Register;
10.             UnityEditor.EditorApplication.update += Register;
11. #else
12.             RegisterEntry(this);
13. #endif
14.         }
15.  
16.         static void Register()
17.         {
18.             Debug.Log("_entriesToRegister.Count " + _entriesToRegister.Count);
19.             foreach(DbEntrySO<TDBEntrySO> entry in _entriesToRegister)
20.             {
21.                 if(!entry) continue; // is NULL when executed from [Worker0] Thread
22.                 RegisterEntry(entry);
23.             }
24.             _entriesToRegister.Clear();
25.             UnityEditor.EditorApplication.update -= Register;
26.         }
27.  
28.         static void RegisterEntry(DbEntrySO<TDBEntrySO> entry)
29.         {
30.             string tempID = ScriptableDB<TDBEntrySO>.Register(entry as TDBEntrySO);
31.             if (tempID != entry.ID)
32.             {
33.                 entry._ID = tempID;
34.             }
35.         }

 

It is in general simplier to make something thread safe if it can be called from within a thread.

We usually simply enclose the content of ScriptableDB<TDBEntrySO>.Register() in a lock(){} statement (where the ID assignement should also take place) which is why we wouldnt need more than one line of code.

 

In my case the ScriptableDB is a ScriptableObject - I can't load it from another thread, so the ScriptableDB.Instance will be NULL.
IDK if loading via Assetdatabase.Load would work - but even then, I'd need a "path" variable, which has to be static because I can't read the serialized value of "string path" when I can't load the ScriptableObject that holds this path variable.
And without access to the ScriptableDB-Object, I cannot verify that the ID is unique (duplicating a DBEntrySO) therefor I am unable to return a valid unique ID, no matter if I use lock() or not.

How does this all work in your code? Is your static "path" variable a hardcoded string? (do you even have a "path" variable?
(path = asset path like "Assets/MyScriptables/ItemScriptables/ItemDB.asset"))

 

For things like manifests (or DB as you call them) If they must exist/be loaded at startup, I use the [InitializeOnLoad] or [InitializeOnLoadMethod] attributes to initialize a reference on an instance, or the singleton pattern if they must exist based on first invocation rule, which also sort thing ups in regard to to the manifest being there upon registering entries (even if those entries are scripatable objects). So I never really had problems of things being NULL or being called from a thread. As for AssetDataBase, it is UnityEditor only.

 

Same problem as above -> [InitializeOnLoadMethod] is for static methods only (no path, no instance, I would have to search the whole AssetDatabase to find my asset without a hardcoded path - having 10 different ScriptableDB objects and 10k asses, this would slow down Recompile Time tremendously - the whole point of letting the DBEntries register themselfs at the DB is not having to search through the whole AssetDatabase to find them).

[InitializeOnLoad] only applies to the constructor which can't be used on ScriptableObjects.


I need those Database ScriptableObjects in order to create EditorWindows, to get an ScriptableObject by ID (especially setting this ID and ensuring it is unique), as well as keeping track of all existing ScriptableObjects of a specific type.

 

[InitializeOnLoad] or [InitializeOnLoadMethod] can be assigned to any utility classes other than scriptable objects, should you decide to use such utility classes to initialize scriptable objects instances. That being said, even with 10K assets (this is what big AAA studios get for a whole project), the whole point of organizing assets folders, assets bundles etc is to both keep things clean and limit the searchs. Then, whether you like it or not, you will always have to process cold starts, in which cases, - if - your manifest dont have references on assets in the traditional sens (in some form or another), you will have to do a search. Finally, if something cant be created aynschronously (from an obscure callback for exemple), I just queue a request in a thread safe queue that is processed in the main thread, in which case I also end up not having to do a lot of tinkering. People who come here saying "my problem is unique, there's nothing that can be done about it" we see that a lot. They never wonder why we dont have the problems they have.

 

If this was targeted at me:
- "there's nothing that can be done" -> I have a functioning workaround
- "They never wonder why" -> I asked how it works in your project

Other than that .. I don't want my game to break, just because someone decided to create ScriptableObjects of a specific type outside of their designated folder. ("ItemID couldn't be found, the asset seems to be missing or in the wrong folder" is a Console-Line I'd like to avoid)

... That's the thing with theam projects, huh? Never being sure where someone might move some assets.

To avoid this, I could write an editor window that checks if everything is in the correct folder, updating the ruleset for every collection of SOs we need .. sure, but that's way more work than the workaround I've written. Also .. the workaround is basically only there, to avoid console errors, the whole process of registering at the database worked without the workaround.

And all this because for some reason OnEnable is called from another thread ... why? It wasn't the case pre V2020 (maybe even pre V2021, idk). Why should anyone want OnEnable be called from another thread? What's the benefit?

In my opinion, this is a bug, this shouldn't happen.
And if it isn't a bug, then there should be a new method called like OnEnable() but only on the main thread, something like "OnEnableFromMainThread()".

I mean, I agree with almost everything you said, folder structure and so on, I disagree with the "process cold starts" (code should be written in a way, so the projekt stays fast - slowdown will be there, based on the editor, but custom code shouldn't add to this slowdown in any way). But still, I don't want to worry about these things, I will reuse these databases in various projects which may even require different folder structures (maybe because of git submodules).

 

I get it, it must work even if they put their assets in the ass of a pig. Seriously... Advice: It's best you dont tell your coworkers you think of them as monkeys. In commercial productions checks are an inherent part of tooling, regardless of what you think of your coworkers.

Concerning obscure asynchronous reasons (which can result from inputs callbacks when creating assets and whatnot) you already know by now that it is easily mitigated without tinkering.

As for the thing staying fast, we dont have such problems because we know that separate small assets load slower than grouped ones (hence why Atlases exist, it's not just for rendering perf reasons). You make decisions, you assume their pros and cons. But if 10 scriptable objects among 10K assets (as you put it) make your editor unresponsive, you should consider revising your approach.

 

I appreciate your advice.
"10 scriptable objects among 10K assets [...] unresponsive" - had that with the last project, every recompile, assets would be searched among the entire asset database and that was extremely slow. Also I did iterate over all scene objects and check every component for specific attributes over methods - even when it wasn't needed. Editor Profiling helped solving these issues.

And I don't think of my coworkers as monkeys but human error happes, if it's my fault, I can usually recap what happend and fix it, because I write the systems, I know why things break and how to fix them. If it breaks for the leveldesigner, he can't play the game, he can't make playtests and he can't fix it. Also we haven't etablished rulesets for where assets are placed yet, everyone organizes his stuff in his desired way. The "Assets/0System/Code" is my territory, but said ScriptableObjects are saved in "Assets/Prefabs/Scriptables/..." (../Resources/Items ?).

Yea, we have certain check-tools, checking if any rigidbody has the static flag, checking if any mesh has more than 100k polygons, but I am solo programmer on this 3-person-crew, I have to invest my time with care.