OSVR / OSVR-Unity

Package for authoring OSVR experiences with Unity.
Apache License 2.0
99 stars 38 forks source link

InterfaceAdapter implementation for Unity #57

Closed JeroMiya closed 9 years ago

JeroMiya commented 9 years ago

Implements https://github.com/OSVR/OSVR-Unity/issues/53

Also obsoletes InterfaceCallbacks and InterfaceBase classes.

DuFF14 commented 9 years ago

Good work, this seems to fix the scene switching bug :) I went through each scene to make sure there weren't any errors. There were some in minigame, devscene, and UntrackedVRDisplayDemo. In minigame.unity, Left1Button and LeftAnalogTrigger throw a null reference exception when the game starts. Do you mind going through each scene and making sure the example button/analog objects are working with this? There are some other errors in these scenes that are unrelated to this work, but should be very easy to fix while you are there. For example, I think the VRDisplayUntracked prefab needs a DisplayInterface and OsvrDistortion component.

You also might want to add prefabs for button and analog objects to OSVRUnity/Prefabs.

JeroMiya commented 9 years ago

I was able to reproduce your null reference exceptions by testing in the unity editor as opposed to just the standalone mode. They had two causes: one - minigame.unity had the wrong interface type attached to the button and analog game objects (InterfaceGameObject instead of ButtonInterface and AnalogInterface, respectively), and two I wasn't ensuring Start was called from the IInterface property getters for each interface type, as was done in InterfaceGameObject. Turns out Unity doesn't ensure Start is called prior to returning an object with GetComponent, which I wasn't expecting.

I made the requested modifications to VRDisplayUntracked prefab and added the button and analog prefabs as suggested.

I wasn't able to reproduce any other errors in these scenes. Could you describe the steps to reproduce them? I can take a second look if need be.

I've tested all the scenes (devscene, minigame, TrackerView, VRDisplayDemo, UntrackedVRDisplayDemo, OSVRDemo, and OSVRDemo2 - let me know if I missed one) in both the editor and in standalone mode with the changes in 39514ff. I'm using OSVR-Core snapshot v0.2-158-gb66f7af-build116.

rpavlik commented 9 years ago

@DuFF14 so should we be merging this? or do you need to add to it first?

DuFF14 commented 9 years ago

minigame and devscene are causing Unity to crash for me, using 4.6.5 and 4.6.1. Works for me with Unity 5.0.3f2 64bit and similar 32 bit.

Here is the editor log Looks like a metadata issue. @JeroMiya, in Project Settings -> Editor do you have Version Control Mode set to Visible Meta Files and Asset Serialization Mode set to force text?

Other than the metadata thing in Unity 4, this looks good. After this is merged, I will update the persistent singleton branch.

DuFF14 commented 9 years ago

Another editor log that looks different. Looked up the "CheckConsistency: Restored Transform child parent pointer from NULL" message and found this thread about it: http://forum.unity3d.com/threads/objects-loose-info-after-import.34344/

Looks like a weird bug related to... modifying prefabs? (sorry, I asked for this) There are a couple of solutions to try here. I tried opening the scene in Unity 5, breaking prefab instances related to the prefabs we just modified, re-saving the scene and trying to open in Unity 4, but still get a crash.

edit: copying the scenes and deleting the old versions did not work. Reverting to previous commit does work. Not sure what could cause this. Perhaps the scene was saved in Unity 5, and the scene file has some properties in it that Unity 4 doesn't know what to do with?

JeroMiya commented 9 years ago

Yep, that is likely it. I was editing the scenes in unity 5. I will redo the scene changes in unity 4 tonight.

JeroMiya commented 9 years ago

I have confirmed that the issue was unity 5 scene/prefab incompatibility. I re-implemented the scene and prefab changes in unity 4 from the prior versions, and re-tested all scenes in that version. That fixed the console errors. I'm using unity 4.6.7.

Edit: I also confirmed that my editor settings are set as @DuFF14 described.

DuFF14 commented 9 years ago

I thought I'd be able to add commits here, but it appears I have to open a new pull request without push access to the forked repo. There were still some errors for me in devscene.unity, but I fixed them along with some other minor changes in the demo scenes: https://github.com/OSVR/OSVR-Unity/pull/58 Tested each scene and everything looks like it is working properly and can be merged.

rpavlik commented 9 years ago

Oh rats, forgot to have you write an entry in the changes file for anyone upgrading: what will they have to change in their existing code? Can someone do this?

JeroMiya commented 9 years ago

I put that change in a separate pull request: https://github.com/OSVR/OSVR-Unity/pull/60