Need to ensure asmdefs are correctly set up for 2019 before packing for UPM

microsoft / MixedRealityToolkit-Unity

This repository is for the legacy Mixed Reality Toolkit (MRTK) v2. For the latest version of the MRTK please visit https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity

https://aka.ms/mrtkdocs

MIT License

6.01k stars 2.12k forks source link

Need to ensure asmdefs are correctly set up for 2019 before packing for UPM #8574

Closed keveleigh closed 4 years ago

keveleigh commented 4 years ago

Describe the bug

If the asmdefs aren't correctly set up for Unity 2019 before packing for UPM, the auto-config-checkers attempt to edit immutable assets and fail:

The package cache was invalidated and rebuilt because the following immutable asset(s) were unexpectedly altered:
  Packages/com.microsoft.mixedreality.toolkit.foundation/Providers/XRSDK/Microsoft.MixedReality.Toolkit.Providers.XRSDK.asmdef
  Packages/com.microsoft.mixedreality.toolkit.foundation/Providers/WindowsMixedReality/XRSDK/Microsoft.MixedReality.Toolkit.Providers.XRSDK.WMR.asmdef

This prevents references and asmdef-specific compilation defines from being set, preventing certain code from properly running.

To reproduce

Steps to reproduce the behavior:

Import MRTK via UPM
Watch the console

Tracking

[x] XRSDK (#8591)
[x] WMR XRSDK (#8591)
[x] Oculus (#8655)
[x] Leap (#8697)
[x] UnityAR (#8667)

wiwei commented 4 years ago

@keveleigh if you're heads down this week on other things, can you redirect this to @MaxWang-MS?

david-c-kline commented 4 years ago

Fixed with #8591

keveleigh commented 4 years ago

@davidkline-ms I'm going to reopen this for now, to continue tracking the Leap Motion and Oculus asmdefs, which I believe need additional changes to set them up for when the corresponding SDKs are installed. https://github.com/microsoft/MixedRealityToolkit-Unity/pull/8591#issuecomment-696393464

keveleigh commented 4 years ago

@davidkline-ms Thoughts on adding any configuration checkers to a .npmignore once we get the asmdefs configured correctly in the repo? That way there isn't any confusion or attempt to edit the asmdefs

MaxWang-MS commented 4 years ago

According to @RogPodge the Oculus integration works just fine when he tested using UPM MRTK. Maybe @RogPodge could take a look?

keveleigh commented 4 years ago

After some additional testing, I think Unity 2019 might allow you to edit these asmdefs even though they're in immutable folders, but the message in the original post about an invalidated package cache happens in Unity 2020. We should prepare for that and not edit files from UPM wherever possible.

hybridherbst commented 4 years ago

May I recommend having the Oculus asmdefs referenced directly? If the Oculus integration is not there they're not there, but the asmdefs still work. (this is also the preferred way to work with packages that might potentially be there but are not always).

Also I can confirm that the Oculus Integration does not work directly with the current packages (2.5.0-preview.20200922.2). There's that asmdef issue, plus additional documentation issues on how to set up hands.

I'm happy to do a PR to include them.

RogPodge commented 4 years ago

https://github.com/microsoft/MixedRealityToolkit-Unity/pull/8655

PR up here, still needs manual testing before we push it.

david-c-kline commented 4 years ago

Completed with #8697