rorywalsh / CsoundUnity

GNU Lesser General Public License v2.1
70 stars 19 forks source link

Unity editor crashes in fresh project beginning with release version 3.2.0 #33

Closed Despair-Bear closed 1 year ago

Despair-Bear commented 2 years ago

Tested in Unity 2021.2.18f1 (Windows 10). Created a fresh project and went through all release versions from 3.0+. Starting at version 3.2.0, launching play mode on the BasicTest example scenes causes the Unity Editor to crash to desktop

giovannibedetti commented 2 years ago

Thanks for your tests. Will investigate this afternoon!

giovannibedetti commented 2 years ago

I cannot reproduce the crash on Unity 2021.1.19f1, 2021.2.10f1 and 2021.2.18f1 on Windows 10. Can you post here the editor.log? You can open it from the console window, from the 3 dots top right.

Despair-Bear commented 2 years ago

Sure thing. Here's the log, I'll be poking around this a bit throughout the day today, being we're in different time zones so I'll post whatever I find 👍


    Native Crash Reporting
=================================================================
Got a UNKNOWN while executing native code. This usually indicates
a fatal error in the mono runtime or one of the native libraries 
used by your application.
=================================================================

=================================================================
    Managed Stacktrace:
=================================================================
      at <unknown> <0xffffffff>
      at NativeMethods:csoundCreate <0x00089>
      at CsoundUnityBridge:.ctor <0x00052>
      at CsoundUnity:Awake <0x0028a>
      at System.Object:runtime_invoke_void__this__ <0x00087>
=================================================================```
Despair-Bear commented 2 years ago

Okay so I found the culprit

https://github.com/rorywalsh/CsoundUnity/blob/master/Runtime/CsoundUnityBridge.cs#L49

If you don't add anything to the global environment list then there will be a crash on launch, where before this was handled automatically. Might be good to throw an error in the case of the list being empty to prevent the crash at the very least or set a default environment variable in the case of an empty list with a warning message at the most. Should probably update the Getting Started docs as well, as this step isn't mentioned. I can update these based on the desired direction, I'm assuming you wanted to move away from default declaration considering the changes, but let me know and I'll put in a hotfix PR for this 👍

giovannibedetti commented 2 years ago

Thanks for finding the issue! It's strange I don't have the crash, but I imagine it's something that depends on how the environment list is serialized. I will push an hotfix to master and then publish a new release in the next days.

giovannibedetti commented 2 years ago

Uhm there is already a check for the list being null or empty: https://github.com/rorywalsh/CsoundUnity/blob/e1de8be18c4bfe6a5445d29dffb0ff6eb2b90071/Runtime/CsoundUnityBridge.cs#L49

giovannibedetti commented 2 years ago

I just pushed an hotfix, can you checkout master and see if the crash is gone?

Despair-Bear commented 2 years ago

Uhm there is already a check for the list being null or empty:

https://github.com/rorywalsh/CsoundUnity/blob/e1de8be18c4bfe6a5445d29dffb0ff6eb2b90071/Runtime/CsoundUnityBridge.cs#L49

This is what's causing the issue. If you replace this with something like this:

if (environmentSettings == null || environmentSettings.Count == 0)
{
    throw new Exception("No environment settings were declared, please add at least one to the list under Settings");
}

then this effectively solves the issue as the exception will prevent it from attempting to initialize Csounds with missing platform/OS information. If it simply silently returns, then none of those parameters get set and it causes a crash when it continues on in the CsoundUnityBridge constructor to initialize Csounds.

Despair-Bear commented 2 years ago

I just pushed an hotfix, can you checkout master and see if the crash is gone?

I was still getting a crash when leaving the list empty. The code added in the hotfix will not execute with an empty list and thus the parameters Csounds requires will still not be set when csoundStart is called (which seems to be the culprit of the crash)

giovannibedetti commented 2 years ago

This is very strange, Csound shoudn't need any environment variable set to be able to start! What env setting are you using?

Il Lun 4 Apr 2022, 00:08 Despair-Bear @.***> ha scritto:

I just pushed an hotfix, can you checkout master and see if the crash is gone?

I was still getting an empty crash when leaving the list empty. That portion of the code will not execute with an empty list and thus the parameters Csounds requires will still not be set

— Reply to this email directly, view it on GitHub https://github.com/rorywalsh/CsoundUnity/issues/33#issuecomment-1086968787, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOTBF6FPPI5CZOLQVBUA7DVDIQIRANCNFSM5SMLDMLQ . You are receiving this because you commented.Message ID: @.***>

Despair-Bear commented 2 years ago

This is very strange, Csound shoudn't need any environment variable set to be able to start! What env setting are you using?

Oh really? That's strange. Doesn't the setting of that environment variable give csounds the path to its dll? Well when I add an environment variable to the list it starts just fine, it's only if I don't define any in the list that Csounds seems to crash here Maybe Csounds has a default global env variable that is incompatible with some systems? That's strange that you're unable to reproduce, I wonder why that is. In my testing I had just replaced the new SetEnvironmentSettings function call with what was present in v 3.1.1 of CsoundUnity:

string dataPath = Path.GetFullPath(Path.Combine("Packages", packageName, "Runtime"));

#if UNITY_EDITOR_WIN || UNITY_STANDALONE_WIN
        dataPath = Path.Combine(dataPath, "Win64"); // Csound plugin libraries in Windows Editor
#elif UNITY_EDITOR_OSX || UNITY_STANDALONE_OSX
        dataPath = Path.Combine(dataPath, "macOS");
#endif

which then sets the global env variable in CsoundUnityBridge:

Csound6.NativeMethods.csoundSetGlobalEnv("OPCODE6DIR64", csoundDir);

I'll test on a Windows and Mac laptop with a fresh install of Unity tomorrow to see if I can reproduce. I don't think it would be anything with my local environment that would be causing weirdness, but doesn't hurt to check.

giovannibedetti commented 2 years ago

Yes those settings were useless since the OPCODE6DIR64 is the one for Csound plugins, and there are no plugins in the repo! Not sure what's going on. Will try again later on clean projects on different computer and OSes. Edit: tested on Mac M1 Unity 2021.2.10f1 (with no Csound framework installed), no issues!

giovannibedetti commented 2 years ago

Btw the crash could be related with the params. What do you have in the Project Audio Settings Sample Rate?

Despair-Bear commented 2 years ago

Hey sorry for the delay, busy week. I'll get back to you tomorrow evening with test results

giovannibedetti commented 2 years ago

HI again @Despair-Bear, any findings on this?

Despair-Bear commented 2 years ago

Hey @giovannibedetti, sorry for the delay, back went out at the end of last week so I was avoiding sitting at the computer for a few days while it recovered. Will test this evening, promise! Haha

Despair-Bear commented 2 years ago

Okay so completely fresh install of Unity on a pretty close to fresh Windows 10 install and hard drive, and I am still getting the crash when I don't set anything in the environment variable field. Injecting the old code that force sets it to some default value resolves the crash

giovannibedetti commented 2 years ago

Tried on Unity 2021.2.18f1 on Win 10, removed Csound from the Windows System Environment Variables, all good, no crash. Can you send me the full log from %LOCALAPPDATA%\Unity\Editor\Editor.log?

Despair-Bear commented 2 years ago

Same line throwing it as on my primary machine:

    Managed Stacktrace:
=================================================================
      at <unknown> <0xffffffff>
      at NativeMethods:csoundCreate <0x00089>
      at CsoundUnityBridge:.ctor <0x00052>
      at CsoundUnity:Awake <0x0028a>
      at System.Object:runtime_invoke_voidthis <0x00087>
=================================================================

The laptop I installed and ran it on has almost nothing installed on it aside from internet browser. It's essentially a fresh Win10 install.

giovannibedetti commented 2 years ago

I could reproduce the crash for Unity <= 2019.4 but only for the Environment Vars samples. The other samples work correctly. Investigating ;) Any other interesting bit in the logs?

giovannibedetti commented 2 years ago

Btw about this:

Doesn't the setting of that environment variable give csounds the path to its dll?

No, Unity should reference the dll inside the package by itself.

Despair-Bear commented 2 years ago

Nothing else particularly interesting in the logs. I was testing on Unity 2021.3.0f1 with the "Basic Test" sample

Despair-Bear commented 2 years ago

I'll do a test on my lunch break tomorrow on a Mac to see if it's Windows specific

giovannibedetti commented 2 years ago

I can confirm the crash with version 2021.3.0 (LTS) on a M1 Mac for some samples, apparently it tries to load plugins for some of them. Will restore the old code that sets the OPCODE6DIR64 path to the package itself, because it served us well until now ;) Will draft a new release soon!

giovannibedetti commented 2 years ago

can you try importing this branch in the package manager and see if this is fixed on your side?

https://github.com/rorywalsh/CsoundUnity.git#fix-crash-on-start

I can still produce the crash for Unity < 2019.4 but only for the EnvironmentVars samples. Interestingly, if I put Debug.Break(); at line 120 in this version of CsoundUnityBridge, the crash doesn't happen!

// the above setting could be overridden by the EnvironmentSettings
SetEnvironmentSettings(environmentSettings);

Debug.Break();

Csound6.NativeMethods.csoundInitialize(1);

But I have no output so there is still something wrong going on.

rorywalsh commented 1 year ago

I cannot reproduce this crash here. I'm using the current LTS release of Unity and Windows 10.

rorywalsh commented 1 year ago

I was able to reproduce this now with an older laptop. The problem seems to stem from the fact that the Csound 6.18 in the latest version of CsoundUnity on Windows is trying to load system Csound plugins in C:/Program Files/Csound_x64.... To avoid this issue either install the latest version of Csound so it matches that of CsoundUnity, or simply rename your csound folder while working with Unity. We'll keep looking into it..

rorywalsh commented 1 year ago

I've just pushed a workaround for this on Windows. From hereon in CsoundUnity on Windows will only attempt to load plugin opcode IF the user explicitly requests it by setting the opcode6dir64 path. Testing and working here on the windows machine I was able to create the crash with.