amplitude / unity-plugin

Official Amplitude Unity Plugin
https://developers.amplitude.com/docs/unity
MIT License
44 stars 34 forks source link

UPM Install: Editor.asmdef conflicts with com.unity.purchasing Editor.dll #66

Closed fahall closed 3 years ago

fahall commented 3 years ago

Expected Behavior

Installing Amplitude via UPM should not cause compilation errors.

Current Behavior

After installing Amplitude via https://github.com/amplitude/unity-plugin.git?path=/Assets (on 3/23/2021), I see the following compilation error:

Plugin 'Assets/Plugins/UnityPurchasing/Bin/Editor.dll' has the same filename as Assembly Definition File 'Packages/com.amplitude.unityplugin/Editor/Editor.asmdef'. Rename the assemblies to avoid hard to diagnose issues and crashes.

Possible Solution

Perhaps the Editor code could be optional?

Steps to Reproduce

  1. Create Unity Project
  2. Install com.unity.purchasing
  3. Install Amplitude via UPM at url: https://github.com/amplitude/unity-plugin.git?path=/Assets
  4. Check console for compilation errors (seen on iOS and Android build platforms)

Environment

dantetam commented 3 years ago

Hello @fahall ,

I'll take a look into this. Thank you for the detailed context and steps to reproduce this bug.

It looks like this step Install com.unity.purchasing causes an issue where our Amplitude SDK Editor asmdef (assembly definition file bundled with the plugin) conflicts with another plugin.

Dante

fahall commented 3 years ago

Yeah, it does seem to be a conflict between the two.

I just installed the unitypackage from the most recent release in Github. Seems to be working so far 🤞 . If so, that would at least help narrow down when the conflict was introduced.

Also, since Editor is a magic folder in Unity, it does strike me as add to put an asmdef in there. I don't write a ton of Editor code though 🤷

dantetam commented 3 years ago

Our download through UPM feature is actually very recent, only built a week ago. We only have one UPM config so we know where the issue resides.

I thought files in any folder named Editor requires an editor specific .asmdef file. Of course, these .asmdef files can collide often. Unity architecture is weirdly not so stable in this way.

dantetam commented 3 years ago

Fixing this today. Thank you again @fahall for the excellent debugging information.

And yes, Editor code is important for the Unity SDK and must be included in an Editor-specific .asmdef because it contains scripts necessary for iOS export, such as one that changes the flag GCC_ENABLE_OBJC_EXCEPTIONS

dantetam commented 3 years ago

Tested the Amplitude SDK with the package com.unity.purchasing on both Android and iOS.

fahall commented 3 years ago

Great, thanks!

I like the encapsulation provided by asmdefs, but Unity’s decision to combine them with magic folders makes things tricky.

I’m glad we got this resolved so quickly!