Use assembly definition files; Get play-mode working - Githubissues

Unity-Technologies / EditorXR

Author XR in XR

Other

928 stars 166 forks source link

Use assembly definition files; Get play-mode working #486

Closed AndrewTHEManeri closed 5 years ago

AndrewTHEManeri commented 6 years ago

Purpose of this PR

Enables EditorVR support in play-mode, applies assembly definition files for all submodules.

Testing status

That's what we're doing here. Let's be thorough.

Technical risk

High - the 'editor only' assumption of EXR is going away and we need to be very vigilant looking for unintended side-effects.

Comments to reviewers

Let's see what kind of velocity we have with all-eyes on everything. If we're not making much progress we might do a divide-and-conquer approach instead.

amirebrahimi commented 6 years ago

FYI - there's a change in the mix from Harald Nielsen to assembly definition files that may allow optional dependencies. Until then, we'll need to figure out a solution for our partner SDKs (i.e. Vive and Oculus).

AndrewTHEManeri commented 6 years ago

@amirebrahimi I'm getting errors about missing the SteamVR assembly and OVR Assembly. Did you make those manually or are they in updated libraries somewhere?

amirebrahimi commented 6 years ago

Yeah, those assembly definition files were created manually. I can send them to you if you like. Still need to think about how to patch those SDKs.

AndrewTHEManeri commented 6 years ago

Yeah, send the files over. Maybe we could do something like a combination of the serialized object references and CCU? Determine if an assembly is present, and if so, copy the assembly file into that folder

amirebrahimi commented 6 years ago

Use 7z.exe x -oAssets partner-sdk-asmdefs.zip to overlay files over existing directory structure. This assumes that your zip is in the root of your unity project folder and that the OVR and SteamVR SDKs have not been moved from the original installation.

partner-sdk-asmdefs.zip

AndrewTHEManeri commented 6 years ago

I am getting another error: Assets/EditorVR/Scripts/Core/VRView.cs(466,24): error CS0122: `OVRPlugin' is inaccessible due to its protection level

Is your version of OVRPlugin not set to internal? Or should the asmdef allow us to access internal entries?

amirebrahimi commented 6 years ago

@AndrewTHEManeri - You're right. I forgot that I also added AssemblyInfo.cs to OVR:

[assembly: InternalsVisibleTo("EXR")]

dunity commented 6 years ago

@amirebrahimi I'm wondering what version of OVR you used during your work on play-mode? AFAIK, only older versions of OVR still have a folder hierarchy similar to that which is contained in "the partner-sdk-asmdefs.zip"?

amirebrahimi commented 6 years ago

@amirebrahimi I'm wondering what version of OVR you used during your work on play-mode? AFAIK, only older versions of OVR still have a folder hierarchy similar to that which is contained in "the partner-sdk-asmdefs.zip"?

@dunity - When I go to Tools->Oculus->Update plugin it says 1.22.0. The last zip file I have on disk for OVR Utilities is 1.15.0 though. Did they switch their folders around?

amirebrahimi commented 5 years ago

A few updates:

Got in touch with Oculus and will provide a patch to them for assembly definition files.
Optional assembly definitions will likely go into 2019.1, so possibly we will need to use @mtschoen-unity's solution until then.

amirebrahimi commented 5 years ago

@mtschoen-unity - Lemme know how those revert changes go and then I'll approve.

mtschoen-unity commented 5 years ago

I'd say this is good to go at this point. I've been able to use all workspaces and tools in Edit Mode, a subset of workspaces in Play Mode, and the MiniWorld Workspace and tools in a built player. I haven't tested all actions in the player but we can refine individual functionality in smaller PRs. I have made tasks for a few remaining cleanup items.

The asmdef fix works out pretty nicely. EXR won't compile on first import, but all you have to do is immediately import the asmdef package and you're good to go. We can even include the asmdefs in the release package to avoid this issue for users of the package. The issue will also go away in 2019.1, where missing references are simply ignored and compilation can continue. I've updated the readme to explain this.

There is one minor issue where the EXR object (the one w/ the EditingContextManager) in the open scene gets destroyed right before you enter Play Mode. I think there is some code in the EXR lifecycle that destroys the ECM, but I don't want to get rid of it now in case we end up leaking them or something. I have created a task to investigate and fix this issue. This does not affect Edit Mode.

Some things I encountered and fixed during testing:

The DefaultScriptReference script had a slight issue where it would check/create the Resources directory after first trying to instantiate the prefab. Also, I should note that I had to manually add the SpatialMenu to the list of types to collect. We might want to add a catch-all interface for miscellaneous scripts that need their references serialized.
Oculus Utilities on Asset Store now has assembly definitions, so I've updated our references and the contents of the unity package to contain these assembly definitions in their imported paths
Something about this branch, or the new Oculus Utilites cause the Touch input axes to not zero out completely, which means nothing ever unlocks controls after being consumed. I added a dead zone check to OVRInputToEvents. Oddly, I was not able to reproduce this issue with latest development and the new Oculus Utilities, but couldn't track down a particular change here or in the input prototype that would have lead to this issue.
Our TextMeshPro warning was coming up on first import, even though the package is included by default, because of the way the CCU does multiple passes. I was able to suppress the warning with a reflection check on the first pass. I also updated the warning to indicate the package manager instead of Asset Store. I checked that the TMP package works back in 2017.2, and it does!
The Getting Started guide needed updating, and I also added a bit about play mode in the readme.
I had to change some GUIDs on our copies of the Touch controller assets so that they don't get overwritten by the Oculus Utilities package import. What was happening is that the package importer found the assets in EXR, updated them, and broke texture references. It also modifies files in the repo, which is less than ideal. A number of the files that were being modified weren't even used, so I deleted them. That's why 1b360f2 deletes a bunch of .fbx files.
My plan of compiling out the Project, etc. workspaces didn't work out for Play Mode. I added an EditorOnlyWorkspaceAttribute to mark workspaces that we don't want available in Play Mode. Eventually we can probably let contexts handle this.
SerializedPreferences weren't used in player builds, but they seem to work anyway. I was hoping to resolve an issue in the AnnotationTool where it would hit null refs if you had never serialized its preferences. I had to just check against that case because of the order in which preferences are loaded.