[BUG] Error Caused by Unity's Object Not Supporting C# Syntactic Sugar

[WeiYiHua]

UIS 1.2.18

Some C# syntactic sugar cannot be directly used to evaluate Unity objects. Refer to https://github.com/microsoft/Microsoft.Unity.Analyzers/blob/main/doc/UNT0008.md for details.

However, I noticed one instance where incorrect syntactic sugar is being used:
UltimateInventorySystem\Scripts\UI\Panels\DisplayPanelManager.cs Line: 294

var eventSystem = EventSystemManager.GetEvenSystemFor(gameObject);
var currentSelectable = eventSystem.currentSelectedGameObject?.GetComponent<Selectable>();
if (panel.IsMenuPanel == false) {
    panel.Open(m_SelectedDisplayPanel, currentSelectable);
    return;
}

Should be changed to the following format:

var eventSystem = EventSystemManager.GetEvenSystemFor(gameObject);
Selectable currentSelectable;
if (eventSystem.currentSelectedGameObject != null)
{
    currentSelectable = eventSystem.currentSelectedGameObject.GetComponent<Selectable>();
}
else
{
    currentSelectable = null;
}
if (panel.IsMenuPanel == false) {
    panel.Open(m_SelectedDisplayPanel, currentSelectable);
    return;
}

I'm not sure if there are other places in the entire codebase that violate this rule.

 

[Sangemdoko]

I do think there are some other places I do this.
Usually if I can safely assume that the object will not be destroyed I'll do it.

In this case though you are right, I should make the change since the selected game object can be destroyed at any time.
I'll do it in a single line like this, this will be fixed in the next update:

var currentSelectable = eventSystem.currentSelectedGameObject != null ? eventSystem.currentSelectedGameObject.GetComponent<Selectable>() : null;

 

[Zaddo]

@WeiYiHua That is a scary problem. Thank you for posting this.

Since it affects null propogation, does that mean that anything after the "?" will never execute and always just a assign a null to the result?

Do you know if this applies to MonoBehaviour's?

 

[WeiYiHua]

UnityEngine.Object

public static bool operator ==(Object x, Object y)
{
    return CompareBaseObjects(x, y);
}

public static bool operator !=(Object x, Object y)
{
    return !CompareBaseObjects(x, y);
}

All types derived from UnityEngine.Object have operator overloads.

 

[Zaddo]

Okay.... So, monbehavior, scriptableobject, etc. best to avoid null propogating on anything Unity except structs.

I have this syntax scattered throughout my code. I am surprised I haven't noticed bugs.

Thanks again.

 

[Sangemdoko]

@Zaddo,
just to clarify.
Using null propagation on Unity objects will not break your game as long as the Unity Objects you are checking are never destroyed at runtime.

If the unity object was never set, therfore null -> Fine
If the unity object was set, not null -> Fine
If the unity object was set but destroy, null (but not really) -> Not Fine, that's when you'll get errors from using null propagation

I hope that clarifies it. I assume the reason you haven't gotten any issues so far is because you've always been in the first two scenarios. Whether that is just luck or because you designed it that way, is where you need to be careful

 

[Zaddo]

@Sangemdoko, thanks that helps clarify the risk.
I just finished cleaning up my code. Most of the occurrences were defensive programming and so unlikely to ever trigger. But still I am glad to be aware of the issue.