ExtendRealityLtd / VRTK

An example of how to use the Tilia packages to create great content with VRTK v4.
https://www.vrtk.io/
MIT License
3.69k stars 993 forks source link

FindEvenInactiveComponent(s) only searches in active scene #1580

Closed Peaj closed 6 years ago

Peaj commented 7 years ago

Environment

Steps to reproduce

Use VRTK_SharedMethods.FindEvenInactiveComponent, FindEvenInactiveComponents or FindEvenInactiveGameObject to find a GameObject or Component which is not in the active scene (with mutliple scenes loaded)

Expected behavior

The GameObject or Component should be found.

Current behavior

The methods dont find anything because they check for "gameObject.scene == activeScene".

All 3 methods can only find objects in the currently active scene which leads to errors e.g. if the SimulatorRig is not located in the active scene because it will not find itself.

Possible solution

I would recommend using this solution to find (even inactive) Components in all loaded Scenes.

bddckr commented 7 years ago

Cross-scene references are a huge pain in Unity.

Example 1:

  1. Get a reference to the Simulator camera rig in another scene.
  2. Unload that other scene.
  3. Litter your whole code base to handle a destroyed camera rig.
Peaj commented 7 years ago

But the current implementation does not prevent cross references. If it is located in the active scene you would still find and reference it from any other scene. And the active scene can be unloaded like any other scene.

As "SDK_InputSimulator.FindInScene" does only search in the active scene, it just has the (unnecessary?) limitation that the SDKManager cannot be located anywhere else. This makes it really hard to use in a multiscene setup.

Because I never need to unload the VRManager (and even to prevent broken references) I like to put it in a global scene wich is loaded once and only unloaded once the game quits. This would in most cases not be the active scene, because the active scene is also responsible for lighting etc.

In this case all the other simulator components cannot find the InputSimulator. Maybe they should even use GetComponentInParent istead or at least a method that searches in the scene the searching object is located in?

All in all I wouldn't expect any of these methods to only search in the active scene. Because none of the Unity methods like FindObjectsOfType does either.

bddckr commented 7 years ago

Be our guest doing a PR and testing it. There have been a lot of issues with the cross-scene approaches on my end when I implemented it. Especially users that use the DontDestroyOnLoad attribute and functionality will always have nightmarish issues since that scene isn't available at runtime at all.

Why isn't the Simulator camera rig in your "VRManager scene"? We never had someone request this because they just put it in the master scene I think.

We want to revamp the example scenes and when that happens I always thought we'd just create a way to force additive scene usage upon our users. Means we can finally take control over everything in an easier way, without having to support all the different things.

Until then changing that method is probably a good idea, just need to remember performance costs traversing everything etc. VRTK also needs to get more supportive with regard to cross-scene referencing and unloading. Even when you do additive scene loading you probably have your Interactable Objects in another scene which will be unloaded later, often resulting in null ref exceptions in various parts of VRTK.

Peaj commented 7 years ago

Be our guest doing a PR and testing it.

Ok. I'll test it in our projects and make a pull request as soon as im confident it works. I really appreciate your work on this great framework and would love to contribute wherever possible.

Why isn't the Simulator camera rig in your "VRManager scene"?

Maybe I did not explain it very well. The Simulator rig is in the same scene as the VRManger. It just happens to not be the active scene. As those methods only search the active scene and not the current (like stated in the comments) it cannot be found. (I would understand current as the scene I'm searching from)

In our case the VRManager is in the "global" scene, which cannot be the active scene because the active scene is responsible for settings like lighting etc. Thus the active scene is our environment (where the artists work on)

Even when you do additive scene loading you probably have your Interactable Objects in another scene which will be unloaded later, often resulting in null ref exceptions in various parts of VRTK.

Fortunately I did not encounter any problems of that kind yet, but I see where this might happen.

thestonefox commented 6 years ago

Any news on this?

Peaj commented 6 years ago

It seems to work correctly in our project. I will try to create a pull request next week. Btw. is there a specific Unity Version I should use to test it?

Peaj commented 6 years ago

I created a pull request (#1637) and changed the commit message after GitCop complained about the length of the lines. However it still complains even though the subject and body should be less then 72 characters per line now

thestonefox commented 6 years ago

Try closing the pull request, and raising a new one with the cleaned up git message

Peaj commented 6 years ago

Seems like it worked :) #1645

kristianpd commented 6 years ago

This can be closed now