YarnSpinnerTool / YarnSpinner-Unity

The official Unity integration for Yarn Spinner, the friendly dialogue tool.
MIT License
524 stars 91 forks source link

Collision with DLL in TypeReference package causes build failure #253

Open Runaway01 opened 1 year ago

Runaway01 commented 1 year ago

What is the current behavior? When having both Yarn Spinner and Type References in the same project the following error prevents building to any platform: Plugin 'System.Runtime.CompilerServices.Unsafe.dll' is used from several locations.

Please provide the steps to reproduce, and if possible a minimal demo of the problem:

  1. Add Yarn Spinner to a blank Unity project (I did this via OpenUPM).
  2. Also add Type References via OpenUPM: openupm install com.solidalloy.type-references.
  3. Save the default scene and add it to the scenes in build list.
  4. Build the project; the error will appear, causing the build to fail

Here's a project with this already set up which just needs to be built to produce the error.

What is the expected behavior? The build completes without error.

Please tell us about your environment:

Other information I think if YarnSpinner had these DLLs as dependencies rather than shipping them this would not be an issue.

Here's the full error:

Plugin 'System.Runtime.CompilerServices.Unsafe.dll' is used from several locations:
Packages/dev.yarnspinner.unity/Editor/Analysis/DLLs/System.Runtime.CompilerServices.Unsafe.dll would be copied to <PluginPath>/x86_64/System.Runtime.CompilerServices.Unsafe.dll
Packages/org.nuget.system.runtime.compilerservices.unsafe/System.Runtime.CompilerServices.Unsafe.dll would be copied to <PluginPath>/x86_64/System.Runtime.CompilerServices.Unsafe.dll
Please fix plugin settings and try again.

UnityEditor.Modules.DefaultPluginImporterExtension:CheckFileCollisions (string,string[])
UnityEditorInternal.PluginsHelper:CheckFileCollisions (UnityEditor.BuildTarget,string[])
UnityEngine.GUIUtility:ProcessEvent (int,intptr,bool&)

Thanks for your time.

TebiChascon commented 1 year ago

I'm having the same issue where Yarn 2.3.1 dll collides with the one in the NUGET package, NUGET adds the dll as a dependency which causes the problem.

I agree with @Runaway01 that if Yarn also added this dll as a dependency it would not have this issue.

Multiple precompiled assemblies with the same name System.Runtime.CompilerServices.Unsafe.dll included on the current platform. Only one assembly with the same name is allowed per platform. (.../Library/PackageCache/org.nuget.system.runtime.compilerservices.unsafe@4.5.3/System.Runtime.CompilerServices.Unsafe.dll)

TebiChascon commented 1 year ago

Sadly this continue to happen with the new version 2.4.0

McJones commented 1 year ago

This is something we've been trying to work out the best way to fix. The issue is because we ship DLLs and we need to do this because we distribute Yarn Spinner outside of OpenUPM. In OpenUPM we could just go "hey we have dependancies on X" and let it handle it, but for everyone else the opposite issue now arises of "hey I don't have a DLL, what gives?"

So some solutions we've been thinking about:

  1. Make OpenUPM the only way to install YS
  2. Use an IL repacker to rename, merge, and hide the DLLs we need
  3. Tell people to just delete the conflicting DLLs
  4. Make two versions of YS:
    • OpenUPM one wouldn't have the DLLs and have the dependency
    • Non-OpenUPM one would have the DLLs
    • both would be the same code and likely be done via a commit hook

None of these is ideal, hence why we are sorta stuck. For now the terrible quick hack of deleting the conflicting DLLs from the YS package on disk should solve the issue but it isn't the long term solution.

TebiChascon commented 1 year ago

Ah I understand the problem now.

What I've done as a workaround is to replace Yarn Spinner Package Manager integration by downloading the package from github and adding it directly. This way I can safely remove any dll that may cause conflicts.

Runaway01 commented 1 year ago

This is something we've been trying to work out the best way to fix. The issue is because we ship DLLs and we need to do this because we distribute Yarn Spinner outside of OpenUPM. In OpenUPM we could just go "hey we have dependancies on X" and let it handle it, but for everyone else the opposite issue now arises of "hey I don't have a DLL, what gives?"

So some solutions we've been thinking about:

  1. Make OpenUPM the only way to install YS
  2. Use an IL repacker to rename, merge, and hide the DLLs we need
  3. Tell people to just delete the conflicting DLLs
  4. Make two versions of YS:

    • OpenUPM one wouldn't have the DLLs and have the dependency
    • Non-OpenUPM one would have the DLLs
    • both would be the same code and likely be done via a commit hook

None of these is ideal, hence why we are sorta stuck. For now the terrible quick hack of deleting the conflicting DLLs from the YS package on disk should solve the issue but it isn't the long term solution.

Thanks for the info. To sort this, we added the package manually and deleted the conflicting DLLs.

Edit: I didn't mean to close the issue, sorry 😅

Could the dependencies be defined via package.json, then resolved and embedded when creating the .unity package as a build step or something like that?

McJones commented 1 year ago

Edit: I didn't mean to close the issue, sorry 😅

Ah the number of times I have closed issues by accident are too high to count...