Skip to content

fix: Adds a null check for NetworkObjectReference => GameObject conversion [NCCBUG-172] #2158

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

Conversation

ShadauxCat
Copy link
Collaborator

Changelog

  • Fixed: Implicit conversion of NetworkObjectReference to GameObject will now return null instead of throwing an exception if the referenced object could not be found (i.e., was already despawned)

Testing and Documentation

  • Includes integration tests.
  • No documentation changes or additions were necessary.

@ShadauxCat ShadauxCat requested a review from a team as a code owner August 26, 2022 17:51
@@ -139,7 +139,7 @@ public override int GetHashCode()
/// </summary>
/// <param name="networkObjectRef">The <see cref="NetworkObjectReference"/> to convert from.</param>
/// <returns>This returns the <see cref="GameObject"/> that the <see cref="NetworkObject"/> is attached to and is referenced by the <see cref="NetworkObjectReference"/> passed in as a parameter</returns>
public static implicit operator GameObject(NetworkObjectReference networkObjectRef) => Resolve(networkObjectRef).gameObject;
public static implicit operator GameObject(NetworkObjectReference networkObjectRef) => Resolve(networkObjectRef)?.gameObject;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use null-conditional operator on MonoBehaviour's - it's not supported. You have to do the full if (ref != null) and the engine marshalls to native to verify the object hasn't been destroyed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. Thanks for pointing that out.

Copy link
Contributor

@JesseOlmer JesseOlmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null conditional issue definitely needs to be fixed. The test stuff is a suggestion but not required for my sign-off

ShadauxCat and others added 3 commits August 29, 2022 15:34
@ShadauxCat ShadauxCat enabled auto-merge (squash) September 6, 2022 19:29
@ShadauxCat ShadauxCat merged commit 7936ea2 into develop Sep 6, 2022
@ShadauxCat ShadauxCat deleted the fix/null_check_for_NetworkObjectReference_GameObject_conversion branch September 6, 2022 20:09
jakobbbb pushed a commit to GooseGirlGames/com.unity.netcode.gameobjects that referenced this pull request Feb 22, 2023
…rsion [NCCBUG-172] (Unity-Technologies#2158)

* fix: Adds a null check for NetworkObjectReference => GameObject conversion [NCCBUG-172]

* Changelog

* style

* Update com.unity.netcode.gameobjects/Tests/Runtime/Serialization/NetworkObjectReferenceTests.cs

Co-authored-by: Jesse Olmer <jesseo@unity3d.com>

* Review feedback.

Co-authored-by: Jesse Olmer <jesseo@unity3d.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants