search

specklesystems / speckle-unity

AEC Interoperability for Unity through Speckle
https://speckle.systems/tag/unity/
Apache License 2.0
55 stars 20 forks source link

Unity Error : "Destroy may not be called from edit mode!" during editor mode stream recieving. #24

Closed JR-Morgan closed 3 years ago

JR-Morgan commented 3 years ago

When receiving streams during editor mode. Many GameObjects are not cleared up and are left parent-less in the root of the scene hierarchy . (see Image)

The following Unity error is printed to the console many times:

Destroy may not be called from edit mode! Use DestroyImmediate instead. 

Destroy may not be called from edit mode! Use DestroyImmediate instead.
Destroying an object in edit mode destroys it permanently.
UnityEngine.Object:Destroy (UnityEngine.Object)
Speckle.ConnectorUnity.RecursiveConverter:TryConvertItemToNative (object) (at Assets/Speckle Connector/RecursiveConverter.cs:137)
Speckle.ConnectorUnity.RecursiveConverter:RecurseTreeToNative (object) (at Assets/Speckle Connector/RecursiveConverter.cs:95)
Speckle.ConnectorUnity.RecursiveConverter:b__3_0 (object) (at Assets/Speckle Connector/RecursiveConverter.cs:84)
System.Linq.Enumerable:ToList (System.Collections.Generic.IEnumerable`1)
Speckle.ConnectorUnity.RecursiveConverter:RecurseTreeToNative (object) (at Assets/Speckle Connector/RecursiveConverter.cs:84)
Speckle.ConnectorUnity.RecursiveConverter:ConvertRecursivelyToNative (Speckle.Core.Models.Base,string) (at Assets/Speckle Connector/RecursiveConverter.cs:61)
Speckle.ConnectorUnity.StreamManagerEditor/d__43:MoveNext () (at Assets/Speckle Connector/Editor/StreamManagerEditor.cs:173)
System.Runtime.CompilerServices.AsyncTaskMethodBuilder:Startd__43> (Speckle.ConnectorUnity.StreamManagerEditor/d__43&)
Speckle.ConnectorUnity.StreamManagerEditor:Receive ()
Speckle.ConnectorUnity.StreamManagerEditor/d__44:MoveNext () (at Assets/Speckle Connector/Editor/StreamManagerEditor.cs:309)
System.Runtime.CompilerServices.AsyncVoidMethodBuilder:Startd__44> (Speckle.ConnectorUnity.StreamManagerEditor/d__44&)
Speckle.ConnectorUnity.StreamManagerEditor:OnInspectorGUI ()
UnityEngine.GUIUtility:ProcessEvent (int,intptr,bool&)

This is because, if the stream contains a Base where no children can be converted to a native object, Their corresponding GameObject should be destroyed here: https://github.com/specklesystems/speckle-unity/blob/8a1e781e9e849132e960fa846f477359465e4e1e/Assets/Speckle%20Connector/RecursiveConverter.cs#L135-L139

However, Due to running during Editor mode, The Object.Destroy(Object) function does not work as intended (a restriction imposed by Unity)

I have drafted a potential solution to this here https://github.com/JR-Morgan/speckle-unity/commit/5a789f99568b1eeb52c0c2553b1287e48b344315#. Streams now properly import without leaving a bunch of empty and parent-less GameObjects. However, Unity is still complaining about another editor mode related issue (also present on main).

Instantiating mesh due to calling MeshFilter.mesh during edit mode. This will leak meshes. Please use MeshFilter.sharedMesh instead. 

Instantiating mesh due to calling MeshFilter.mesh during edit mode. This will leak meshes. Please use MeshFilter.sharedMesh instead.
UnityEngine.StackTraceUtility:ExtractStackTrace ()
Objects.Converter.Unity.ConverterUnity:MeshToNative (Objects.Geometry.Mesh,Objects.Other.RenderMaterial,System.Collections.Generic.Dictionary`2) (at Assets/Speckle Connector/ConverterUnity.Geometry.cs:275)
Objects.Converter.Unity.ConverterUnity:MeshToNative (Speckle.Core.Models.Base) (at Assets/Speckle Connector/ConverterUnity.Geometry.cs:220)
Objects.Converter.Unity.ConverterUnity:ConvertToNative (Speckle.Core.Models.Base) (at Assets/Speckle Connector/ConverterUnity.cs:68)
Speckle.ConnectorUnity.RecursiveConverter:TryConvertItemToNative (object) (at Assets/Speckle Connector/RecursiveConverter.cs:160)
Speckle.ConnectorUnity.RecursiveConverter:RecurseTreeToNative (object) (at Assets/Speckle Connector/RecursiveConverter.cs:98)
Speckle.ConnectorUnity.RecursiveConverter:b__4_0 (object) (at Assets/Speckle Connector/RecursiveConverter.cs:87)
System.Linq.Enumerable:ToList (System.Collections.Generic.IEnumerable`1)
Speckle.ConnectorUnity.RecursiveConverter:RecurseTreeToNative (object) (at Assets/Speckle Connector/RecursiveConverter.cs:87)
Speckle.ConnectorUnity.RecursiveConverter:ConvertRecursivelyToNative (Speckle.Core.Models.Base,string) (at Assets/Speckle Connector/RecursiveConverter.cs:64)
Speckle.ConnectorUnity.StreamManagerEditor/d__43:MoveNext () (at Assets/Speckle Connector/Editor/StreamManagerEditor.cs:173)
System.Runtime.CompilerServices.AsyncTaskMethodBuilder:Startd__43> (Speckle.ConnectorUnity.StreamManagerEditor/d__43&)
Speckle.ConnectorUnity.StreamManagerEditor:Receive ()
Speckle.ConnectorUnity.StreamManagerEditor/d__44:MoveNext () (at Assets/Speckle Connector/Editor/StreamManagerEditor.cs:309)
System.Runtime.CompilerServices.AsyncVoidMethodBuilder:Startd__44> (Speckle.ConnectorUnity.StreamManagerEditor/d__44&)
Speckle.ConnectorUnity.StreamManagerEditor:OnInspectorGUI ()
UnityEngine.GUIUtility:ProcessEvent (int,intptr,bool&)

Steps to reproduce

Clone speckle-unity main branch and open Unity Project. Add a StreamManager to any GameObject in the scene and import a stream that has nested non-native convertible objects. (eg. this stream https://speckle.xyz/streams/59185c5b8a Click Receive and wait for the editor dialogue to complete. Observe scene Hierarchy and console errors.

haitheredavid commented 3 years ago

@JR-Morgan Great catch! In edit mode you have to call DestroyImmediate(), I just submitted a PR that fixes that issue. But in the mean time if you need to implement a fix for that here is a handy function I use for destroying objects.

I also fixed the issue of the leaky meshes in that same PR. This was a new one for me since it usually asks for you to use a MeshFilter.sharedmesh but that didn't seem to fix the issue when i was testing it prior. The fix seems to be that the mesh object needs to be built in an isolated fashion then assigned to the filter.