labstreaminglayer / LSL4Unity

A integration approach of the LabStreamingLayer Framework for Unity3D
Other
87 stars 39 forks source link

DllNotFoundException #52

Closed gregbci closed 1 year ago

gregbci commented 1 year ago

Hello,

I'm getting a DLLNotFoundException with LSL4Unity on a mac (m1 / arm64). I can reproduce it in a new Unity project with LSL4Unity installed. Bringing up LSL editor menu triggers the error:

Screenshot 2023-07-25 at 4 23 31 PM

Looking at the installed LSL4Unity package in my Unity project, I can see that it's missing the DLL symlinks for Plugins/LSL/macOS/arm64. The links are present in the LSL4Unity repo, so Unity probably doesn't support them / create them.

If I recreate the links and reimport, then the problem is fixed. I noticed the linux plugin has copies instead of symlinks, which might have been a workaround to this issue? I'd be happy to make a PR to fix this, just wanted to check that I'm not missing something.

cboulay commented 1 year ago

It certainly worked at one point. That's a shame that the symlinks didn't survive.

So yeah, please go ahead with a PR. I don't think we need 3 copies of the dylib. I don't remember the thought process behind that in the Linux variant. I think we can just drop the symlinks and have the Unity plugin directly link the true dylib we keep.

Which filename do we give the kept dylib? The one with the fully qualified version, then we have to go into the Unity code to update the links everytime we change the dylib version? Or to the unqualified liblsl.dylib, and then we have the possibility of forgetting which version of the dylib we're bundling?

I think the former (fully qualified name) is OK because I think we have to go into the Unity code anyway whenever there's a dylib update, because we have to set some flags on the file or something. My memory on that is hazy.

gregbci commented 1 year ago

The unqualified name is used in LSL.cs. If we stuck with that, I don't think a code change would be needed for a liblsl update, just drop in the new libs. Libsls for Windows ships as just "lsl.lib" already.

That said, I also like seeing the fully qualified version in the repo. So, for the want of a few megabytes, I say just copy all three as was done on the Linux side?

I'll put the PR together today.

cboulay commented 1 year ago

The unqualified name is used in LSL.cs

Of course. Sorry I forgot about that. Your solution sounds good. I look forward to the PR. Thanks!

gregbci commented 1 year ago

K, will do. @cboulay I need repo access to create a branch / PR

cboulay commented 1 year ago

I think a typical gitflow for this circumstance is to fork and create a PR from the fork. I could probably setup access to this repo but I'd need to add in some protections first. The fork pathway is faster for now.

gregbci commented 1 year ago

K will do, thanks!

gregbci commented 1 year ago

@cboulay The PR is up, let me know if I can help with testing. I have some team members that can pitch in if need be.