XRTK / com.xrtk.core

The Official Mixed Reality Framework for Unity
https://xrtk.io
MIT License
307 stars 34 forks source link

Invalid behavior of Component Extension SetActive #903

Closed FejZa closed 2 years ago

FejZa commented 2 years ago

XRTK - Mixed Reality Toolkit Bug Report

Describe the bug

XRTK's component extension SetActive

public static void SetActive(this Component component, bool isActive)
        {
            if (component.gameObject.activeInHierarchy != isActive)
            {
                component.gameObject.SetActive(isActive);
            }
        }

is error prone and it differs from people would expect when setting a GameObject active / inactive. activeInHierarchy is not the same as activeSelf. If an object is not activeInHierarchy but activeSelf calling SetActive(false) using this extension will result in the object not being set to inactive.

StephenHodgson commented 2 years ago

It's a lot easier to set a component active/inactive using the enabled property.

This extension method was meant to be used purely to enable/disable the game object that the component is attached to.

FejZa commented 2 years ago

Yes, but enabled is again a whole different story.

"This extension method was meant to be used purely to enable/disable the game object that the component is attached to."

Exactly, and that is what it is failing to do.

FejZa commented 2 years ago

IF the parent game object is disabled but the game object that it is attached to is not, then SetActive(false) won't disable it.

FejZa commented 2 years ago

The fix would be:

if (component.gameObject.activeSelf != isActive)
            {
                component.gameObject.SetActive(isActive);
            }
StephenHodgson commented 2 years ago

Makes sense to me. Thanks for catching this! 🙏