Probable mistake in GraphOwnerInspector.OnDestroy()

[erwanParticipant]

While making sure breakpoints worked in my improbable setup of NodeCanvas I found that in GraphOwnerInspector.cs line 38 :

1 2 3 4 5 6 7 8

void OnDestroy() {   if (owner == null && owner.graph != null){     if (owner.graphIsBound){       Undo.DestroyObjectImmediate(owner.graph);     } }

Should probably read :

1 2 3 4 5 6 7

void OnDestroy() {   if (owner != null && owner.graph != null){     if (owner.graphIsBound){       Undo.DestroyObjectImmediate(owner.graph);     } }

You probably caught that one already but here it is 😉

[GavalakisKeymaster]

Hey!
owner == null is actually on purpose 😛 and it is a hack to cleanup the graph if the component is removed from the Unity inspector (right click -> remove component for example).
It’s not really important though, since the GC will clean that up anyway on an enter/exit play mode (as well as other cases).

[erwanParticipant]

Not wanting to appear anoying but conditionals are resolved left to right so in this case :
owner == null && owner.graph != null
If the following is true :
owner == null
then owner will be null and when the next conditional will be resolved (because it is a &&) it will crash because you are using owner : owner.graph != null

[psykawParticipant]

You’re both almost right.
owner == null could return true but owner could be not null (will be destroyed at the end of the frame). == operator is overriden by Unity.

To prevent null ref exception you should replace condition with this code:
owner == null && !ReferenceEquals(owner, null) && owner.graph != null

[erwanParticipant]

Haa ok I failed to see the overload.

[Author]

Posts