Important bug warning, and how to fix it

Unity Courses Talk
,
#1

Just documenting a bug that was discovered by one of our students against this lecture. This should be patched soon, but I’m putting this message here to highlight the issue.

After implementing the changes in this lecture, if you create a new Dialogue in your assets, Unity will lock up and eventually crash with a Stack Overflow error which logs will show traces to line 61 of Dialogue.cs. DO NOT DO THIS or you will lose any other changes you’ve made to your project! Just take my word for it, the bug exists.

The offending line in CreateNode() is
Undo.RecordObject(this, "Added New Node");

The cause of this bug relates to when we first create the Dialogue. In OnBeforeSerialize, we have added a call to CreateNode() if the list of nodes is empty. The problem is that for that brief moment after we first create a dialogue until we name the file, there is no actual file. It doesn’t exist. A call to Undo.RecordObject(this) will fail because this isn’t on the disk yet.
The solution is simple… surround the Undo call with an if statement to check to make sure that the file actually exists.

        if (AssetDatabase.GetAssetPath(this)!="")
        {
            Undo.RecordObject(this, "Added Dialogue Node"); 
        }
5 Likes
NullReferenceException Error
#2

Should we get in the habit of always doing this when we use Undo.RecordObject() ?

#3

Well, this was a corner case… normally, when we add a node (by clicking the add node button), the Dialogue has already been fully created and exists on the disk. The actual bug in this case is adding the node too soon.
I would say that it’s something to mindful of. If the method containing the Undo.RecordObject will be called in any Awake() or OnBeforeSerialize() method (Both of which can fire before a Serialized Object can fully exist) then yes, there should be that check. If it’s just a method you’ll be calling as a result of something in OnGUI, I wouldn’t worry, OnGUI won’t fire until after the object has been written to disk.

To clarify what “written to disk” means… When a ScriptableObject (or any asset) is created through the Create menu, an icon and a default name in edit mode is placed in the intended directory. Until you click away or finish renaming the file, it does not exist. Once you name it (or click away leaving it with it’s default name), only then does it exist.

1 Like
#4

I had to add one more additional guard to get reliable operation. In OnBeforeSerialize() just before calling AssetDatabase.AddObjectToAsset(node, this), I found it useful to check that node isn’t null, because occasionally it is (try hitting undo lots of times quickly) and the result isn’t pretty. Looks like the path for null is also "". I don’t know why the foreach loop is ending up with a null value from a List though.

And I think I’ve found another related corner case - would be interesting to know if anyone else sees similar behaviour:

Close the Dialogue Editor if open. Create a brand new Dialogue asset, click to accept filename, then double-click to open the Editor. Then without doing anything else, hit ctrl-z six times (to undo six times).

Now click on the Node - the Inspector will show under the Nodes list that Element 0 is Missing, and although you can see a Node, you can’t interact with it properly. Haven’t worked out what’s going on here yet…

EDIT: actually if you don’t fix that null issue I mentioned, then the Node just disappears completely. I’ve also managed to reduce the number of Undo operations to reproduce this to just two with my own changes here and there, but the overall result is the same.

1 Like

Privacy & Terms