chore: simplify accessors network object #3831
Conversation
com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs
Outdated
Show resolved
Hide resolved
com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs
Outdated
Show resolved
Hide resolved
| // Only dynamically spawned NetworkObjects that are not already in the newly assigned active scene will migrate | ||
| // and update their scene handles | ||
| if (IsSceneObject.HasValue && !IsSceneObject.Value && gameObject.scene != next && gameObject.transform.parent == null) | ||
| if (IsSceneObject.HasValue && !IsSceneObject.Value && m_SceneOrigin != next && m_CachedParent == null) |
There was a problem hiding this comment.
m_SceneOrigin is the actual one or the starting one (is a ref or a copy)
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
But are they for transform.parent just bellow?
| // Only dynamically spawned NetworkObjects that are not already in the newly assigned active scene will migrate | ||
| // and update their scene handles | ||
| if (IsSceneObject.HasValue && !IsSceneObject.Value && gameObject.scene != next && gameObject.transform.parent == null) | ||
| if (IsSceneObject.HasValue && !IsSceneObject.Value && m_SceneOrigin != next && m_CachedParent == null) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
noellie-velez
force-pushed
the
chore/simplify-accessors-network-object
branch
from
02d061d to
aab3f4d
Compare
| // Early exit if the NetworkManager is shutting down, the NetworkObject | ||
| // is not spawned, or an in-scene placed NetworkObject | ||
| if (NetworkManager == null || NetworkManager.ShutdownInProgress || !IsSpawned || IsSceneObject != false) | ||
| if (!IsSpawned || IsSceneObject != false || NetworkManager.ShutdownInProgress) |
There was a problem hiding this comment.
IsSceneObject is a bool? type which means the value can be null | true | false so this check needs to return true when the value is null | true but false when the value is false.
That is to say, this check here is already a contraction of (IsSceneObject == null || IsSceneObject.HasValue). Unfortunately we can't contract it further.
| } | ||
|
|
||
| // Do the safety loop first to prevent putting the netcode in an invalid state. | ||
| for (int i = 0; i < networkObjects.Count; i++) |
There was a problem hiding this comment.
Fun fact, we can remove this whole loop, as all these checks should be done in the underlying function.
I think the most important next task for you is to pull out these exceptions!
There was a problem hiding this comment.
I should address this comment in another PR, right? I mean, I should not remove the loop until I pull out these exceptions, right?
There was a problem hiding this comment.
Seeing the following comment I guess I should remove both loops and pull out the exceptions on a next PR.Let me know if that's not what you meant to put it back in.
| } | ||
|
|
||
| // Do the safety loop first to prevent putting the netcode in an invalid state. | ||
| for (int i = 0; i < networkObjects.Count; i++) |
There was a problem hiding this comment.
Same thing here, just remove this whole pre-process loop
…hub.com/Unity-Technologies/com.unity.netcode.gameobjects into chore/simplify-accessors-network-object
…ject # Conflicts: # com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs
Purpose of this PR
Jira ticket
MTT-13657
Changelog
Documentation
Testing & QA (How your changes can be verified during release Playtest)
Functional Testing
Manual testing :
Manual testing doneAutomated tests:
Covered by existing automated testsCovered by new automated testsDoes the change require QA team to:
Review automated tests?Execute manual tests?Provide feedback about the PR?If any boxes above are checked the QA team will be automatically added as a PR reviewer.
Backports