refactor!: SpawnManager is no longer static #696
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -89,6 +89,11 @@ public class NetworkManager : MonoBehaviour, INetworkUpdateSystem, IProfilableTr | |||||
| /// </summary> | ||||||
| public static NetworkManager Singleton { get; private set; } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// Gets the SpawnManager for this NetworkManager | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| /// </summary> | ||||||
| public NetworkSpawnManager SpawnManager { get; private set; } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// Gets the networkId of the server | ||||||
| /// </summary> | ||||||
|
|
@@ -313,10 +318,9 @@ private void Init(bool server) | |||||
| ConnectedClients.Clear(); | ||||||
| ConnectedClientsList.Clear(); | ||||||
|
|
||||||
| // Create spawn manager instance | ||||||
|
There was a problem hiding this comment.
Suggested change
isn't it obvious already? 😛 |
||||||
| SpawnManager = new NetworkSpawnManager(this); | ||||||
|
|
||||||
| NetworkSceneManager.RegisteredSceneNames.Clear(); | ||||||
| NetworkSceneManager.SceneIndexToString.Clear(); | ||||||
| NetworkSceneManager.SceneNameToIndex.Clear(); | ||||||
|
|
@@ -441,7 +445,7 @@ public SocketTasks StartServer() | |||||
| IsClient = false; | ||||||
| IsListening = true; | ||||||
|
|
||||||
| SpawnManager.ServerSpawnSceneObjectsOnStartSweep(); | ||||||
|
|
||||||
| OnServerStarted?.Invoke(); | ||||||
|
|
||||||
|
|
@@ -617,7 +621,7 @@ public SocketTasks StartHost() | |||||
| HandleApproval(ServerClientId, NetworkConfig.CreatePlayerPrefab, null, true, null, null); | ||||||
| } | ||||||
|
|
||||||
| SpawnManager.ServerSpawnSceneObjectsOnStartSweep(); | ||||||
|
|
||||||
| OnServerStarted?.Invoke(); | ||||||
|
|
||||||
|
|
@@ -691,8 +695,14 @@ public void Shutdown() | |||||
| IsServer = false; | ||||||
| IsClient = false; | ||||||
| NetworkConfig.NetworkTransport.OnTransportEvent -= HandleRawTransportPoll; | ||||||
|
|
||||||
| if (SpawnManager != null) | ||||||
| { | ||||||
| SpawnManager.DestroyNonSceneObjects(); | ||||||
| SpawnManager.ServerResetShudownStateForSceneObjects(); | ||||||
|
|
||||||
| SpawnManager = null; | ||||||
| } | ||||||
|
|
||||||
| //The Transport is set during Init time, thus it is possible for the Transport to be null | ||||||
| NetworkConfig?.NetworkTransport?.Shutdown(); | ||||||
|
|
@@ -1213,7 +1223,7 @@ private static void ReceiveCallback(NetworkBuffer messageBuffer, RpcQueueContain | |||||
| /// </summary> | ||||||
| /// <param name="queueItem">frame queue item to invoke</param> | ||||||
| #pragma warning disable 618 | ||||||
| internal void InvokeRpc(RpcFrameQueueItem queueItem) | ||||||
| { | ||||||
| #if DEVELOPMENT_BUILD || UNITY_EDITOR | ||||||
| s_InvokeRpc.Begin(); | ||||||
|
|
@@ -1225,12 +1235,12 @@ internal static void InvokeRpc(RpcFrameQueueItem queueItem) | |||||
|
|
||||||
| if (__ntable.ContainsKey(networkMethodId)) | ||||||
| { | ||||||
| if (!SpawnManager.SpawnedObjects.ContainsKey(networkObjectId)) | ||||||
| { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| var networkObject = SpawnManager.SpawnedObjects[networkObjectId]; | ||||||
|
|
||||||
| var networkBehaviour = networkObject.GetNetworkBehaviourAtOrderIndex(networkBehaviourId); | ||||||
| if (networkBehaviour == null) | ||||||
|
|
@@ -1344,10 +1354,10 @@ internal void OnClientDisconnectFromServer(ulong clientId) | |||||
| { | ||||||
| if (ConnectedClients[clientId].PlayerObject != null) | ||||||
| { | ||||||
| if (SpawnManager.CustomDestroyHandlers.ContainsKey(ConnectedClients[clientId].PlayerObject.PrefabHash)) | ||||||
| { | ||||||
| SpawnManager.CustomDestroyHandlers[ConnectedClients[clientId].PlayerObject.PrefabHash](ConnectedClients[clientId].PlayerObject); | ||||||
| SpawnManager.OnDestroyObject(ConnectedClients[clientId].PlayerObject.NetworkObjectId, false); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
|
|
@@ -1361,10 +1371,10 @@ internal void OnClientDisconnectFromServer(ulong clientId) | |||||
| { | ||||||
| if (!ConnectedClients[clientId].OwnedObjects[i].DontDestroyWithOwner) | ||||||
| { | ||||||
| if (SpawnManager.CustomDestroyHandlers.ContainsKey(ConnectedClients[clientId].OwnedObjects[i].PrefabHash)) | ||||||
| { | ||||||
| SpawnManager.CustomDestroyHandlers[ConnectedClients[clientId].OwnedObjects[i].PrefabHash](ConnectedClients[clientId].OwnedObjects[i]); | ||||||
| SpawnManager.OnDestroyObject(ConnectedClients[clientId].OwnedObjects[i].NetworkObjectId, false); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
|
|
@@ -1380,7 +1390,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) | |||||
|
|
||||||
| // TODO: Could(should?) be replaced with more memory per client, by storing the visiblity | ||||||
|
|
||||||
| foreach (var sobj in SpawnManager.SpawnedObjectsList) | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| { | ||||||
| sobj.Observers.Remove(clientId); | ||||||
| } | ||||||
|
|
@@ -1446,15 +1456,15 @@ internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, ulong | |||||
|
|
||||||
| if (createPlayerObject) | ||||||
| { | ||||||
| var networkObject = SpawnManager.CreateLocalNetworkObject(false, 0, playerPrefabHash ?? NetworkConfig.PlayerPrefabHash.Value, ownerClientId, null, position, rotation); | ||||||
| SpawnManager.SpawnNetworkObjectLocally(networkObject, SpawnManager.GetNetworkObjectId(), false, true, ownerClientId, null, false, 0, false, false); | ||||||
|
|
||||||
| ConnectedClients[ownerClientId].PlayerObject = networkObject; | ||||||
| } | ||||||
|
|
||||||
| m_ObservedObjects.Clear(); | ||||||
|
|
||||||
| foreach (var sobj in SpawnManager.SpawnedObjectsList) | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| { | ||||||
| if (ownerClientId == ServerClientId || sobj.CheckObjectVisibility == null || sobj.CheckObjectVisibility(ownerClientId)) | ||||||
| { | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -225,7 +225,7 @@ public void NetworkShow(ulong clientId, Stream payload = null) | |||||
| // Send spawn call | ||||||
| Observers.Add(clientId); | ||||||
|
|
||||||
| NetworkManager.Singleton.SpawnManager.SendSpawnCallForObject(clientId, this, payload); | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| /// <summary> | ||||||
|
|
@@ -265,7 +265,7 @@ public static void NetworkShow(List<NetworkObject> networkObjects, ulong clientI | |||||
| // Send spawn call | ||||||
| networkObjects[i].Observers.Add(clientId); | ||||||
|
|
||||||
| NetworkManager.Singleton.SpawnManager.WriteSpawnCallForObject(buffer, clientId, networkObjects[i], payload); | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| InternalMessageSender.Send(clientId, NetworkConstants.ADD_OBJECTS, NetworkChannel.Internal, buffer); | ||||||
|
|
@@ -361,9 +361,9 @@ public static void NetworkHide(List<NetworkObject> networkObjects, ulong clientI | |||||
|
|
||||||
| private void OnDestroy() | ||||||
| { | ||||||
| if (NetworkManager.Singleton != null && NetworkManager.Singleton.SpawnManager.SpawnedObjects.ContainsKey(NetworkObjectId)) | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| { | ||||||
| NetworkManager.Singleton.SpawnManager.OnDestroyObject(NetworkObjectId, false); | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -385,13 +385,13 @@ private void SpawnInternal(Stream spawnPayload, bool destroyWithScene, ulong? ow | |||||
| spawnPayload.Position = 0; | ||||||
| } | ||||||
|
|
||||||
| NetworkManager.Singleton.SpawnManager.SpawnNetworkObjectLocally(this, NetworkManager.Singleton.SpawnManager.GetNetworkObjectId(), false, playerObject, ownerClientId, spawnPayload, spawnPayload != null, spawnPayload == null ? 0 : (int)spawnPayload.Length, false, destroyWithScene); | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
|
|
||||||
| for (int i = 0; i < NetworkManager.Singleton.ConnectedClientsList.Count; i++) | ||||||
| { | ||||||
| if (Observers.Contains(NetworkManager.Singleton.ConnectedClientsList[i].ClientId)) | ||||||
| { | ||||||
| NetworkManager.Singleton.SpawnManager.SendSpawnCallForObject(NetworkManager.Singleton.ConnectedClientsList[i].ClientId, this, spawnPayload); | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -433,7 +433,7 @@ public void SpawnAsPlayerObject(ulong clientId, Stream spawnPayload = null, bool | |||||
| /// </summary> | ||||||
| public void Despawn(bool destroy = false) | ||||||
| { | ||||||
| NetworkManager.Singleton.SpawnManager.DespawnObject(this, destroy); | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| } | ||||||
|
|
||||||
|
|
||||||
|
|
@@ -442,7 +442,7 @@ public void Despawn(bool destroy = false) | |||||
| /// </summary> | ||||||
| public void RemoveOwnership() | ||||||
| { | ||||||
| NetworkManager.Singleton.SpawnManager.RemoveOwnership(this); | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| /// <summary> | ||||||
|
|
@@ -451,7 +451,7 @@ public void RemoveOwnership() | |||||
| /// <param name="newOwnerClientId">The new owner clientId</param> | ||||||
| public void ChangeOwnership(ulong newOwnerClientId) | ||||||
| { | ||||||
| NetworkManager.Singleton.SpawnManager.ChangeOwnership(this, newOwnerClientId); | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| internal void InvokeBehaviourOnLostOwnership() | ||||||
|
|
||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
I agree and disagree, but mostly disagree. The name change is maybe valid (though having small insignificant names for insignificant variables and vice versa is a style choice I actually prefer), but there's also the "when in Rome" principal where we want to make the PR as focused on the actual change at hand, not mix in some minor unrelated style edits to keep the signal / noise ratio high.
Uh oh!
There was an error while loading. Please reload this page.
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.
I don't understand why you'd consider this change in a separate PR.
we're changing it from:
to
already.
my only suggestion here is to kill
.Singletonpart as wellbecause there is already
NetworkBehaviour.NetworkManagerproperty available, so there is no need to accessNetworkManageroverNetworkManager.Singletoninstead ofNetworkBehaviour.NetworkManager.I do not think it should go under a separate PR — we are making a change there already, let's make it even better.
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.
This will come in my next PR!
That's where I will convert the whole NetworkObject & Behaviour.