-
Notifications
You must be signed in to change notification settings - Fork 450
fix: MLAPI spawned objects now sets NetworkManager override #767
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
fix: MLAPI spawned objects now sets NetworkManager override #767
Conversation
@@ -225,6 +225,8 @@ internal NetworkObject CreateLocalNetworkObject(bool softCreate, uint prefabHash | |||
// Otherwise, instantiate an instance of the NetworkPrefab linked to the prefabHash | |||
var networkObject = ((position == null && rotation == null) ? UnityEngine.Object.Instantiate(networkPrefabReference) : UnityEngine.Object.Instantiate(networkPrefabReference, position.GetValueOrDefault(Vector3.zero), rotation.GetValueOrDefault(Quaternion.identity))).GetComponent<NetworkObject>(); | |||
|
|||
networkObject.NetworkManagerTestOverride = NetworkManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth a comment saying something like "by default all network objects are spawned with this singleton, central NM which can be overridden for testing"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as per my comments in #762, I'm still not 100% onboard with this approach yet.
…-network-transform
…oval-spawn-manager-ownership
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as I was commenting in #762, let's align about our direction internally first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the existing comments, the rest looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since now we're aligned, I'm onboard with our short-term solution.
* fix: MLAPI spawned objects now sets NetworkManager override (#767)
No description provided.