Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting null as parent using TrySetParent throws a null reference exception #2621

Closed
powersupersport opened this issue Jul 12, 2023 · 0 comments · Fixed by #2625
Closed

Setting null as parent using TrySetParent throws a null reference exception #2621

powersupersport opened this issue Jul 12, 2023 · 0 comments · Fixed by #2625
Assignees
Labels
priority:high stat:imported Issue is tracked internally at Unity type:bug Bug Report

Comments

@powersupersport
Copy link

Here's an example code:

public class Example : NetworkBehaviour
{
    private void Start()
    {
        Transform newParent = null; // -------------> Technically means set the parent of the object to root.
        NetworkObject.Spawn();
        NetworkObject.TrySetParent(newParent); // --------------> Throws a null reference exception.
    }
}

Here's the offending overload:

public bool TrySetParent(Transform parent, bool worldPositionStays = true)

public bool TrySetParent(Transform parent, bool worldPositionStays = true)
{
    var networkObject = parent.GetComponent<NetworkObject>();  // ----------> Why does this not check for null?

    // If the parent doesn't have a NetworkObjet then return false, otherwise continue trying to parent
    return networkObject == null ? false : TrySetParent(networkObject, worldPositionStays);
}

There is a workaround, but that's not the point. The workaround is:

public class Example : NetworkBehaviour
{
    private void Start()
    {
        Transform newParent = null;
        NetworkObject.Spawn();

        if (parent != null)
            NetworkObject.TrySetParent(newParent);
        else
            NetworkObject.TrySetParent((NetworkObject)null); // --------> The overload that accepts a NetworkObject perfectly accepts a null value.
    }
}

The reasons I consider this a bug and not a feature (of overloading methods) are two:

  • UnityEngine's transform.SetParent() perfectly accepts null as value.
  • The naming convention of putting Try in front of methods (TrySomething) means it's not supposed to throw you an exception.
@powersupersport powersupersport added stat:awaiting triage Status - Awaiting triage from the Netcode team. type:bug Bug Report labels Jul 12, 2023
@fluong6 fluong6 added priority:high stat:import stat:imported Issue is tracked internally at Unity and removed stat:awaiting triage Status - Awaiting triage from the Netcode team. stat:import labels Jul 12, 2023
ShadauxCat added a commit that referenced this issue Jul 13, 2023
NoelStephensUnity added a commit that referenced this issue Jul 20, 2023
* fix: Add null check in TrySetParent

fixes #2621

* Added tests

* Changelog

---------

Co-authored-by: Noel Stephens <noel.stephens@unity3d.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:high stat:imported Issue is tracked internally at Unity type:bug Bug Report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants