Upgrade Invaders to use Netcode For Gameobject 1.8.1 [MTT-8503] #172
Conversation
| [ClientRpc] | ||
| void SpawnExplosionVFXClientRpc(Vector3 spawnPosition, Quaternion spawnRotation) | ||
| [Rpc(SendTo.ClientsAndHost)] | ||
| void ClientSpawnExplosionVFXRpc(Vector3 spawnPosition, Quaternion spawnRotation) |
There was a problem hiding this comment.
In order to maintain consistency with NGO's naming (and with the gaming industry's best practice), methods in NetworkBehaviours that are executed on Client or Server should start with "OnClient" or "OnServer" for clarity.
@bastienunity Since you're going through renaming RPCs, could you rename according to that logic?
There was a problem hiding this comment.
Hey Paolo. This suggestion seems to contradict the alignment we had with Noel in this discussion, at least for Rpcs.
As for the other methods, if it's not listed on our coding standard reference doc, we don't necessarily need to follow any naming convention. If we want to introduce that as a convention for our samples moving forward, we should establish that convention first, since that's not present in any sample so far.
There was a problem hiding this comment.
The alignment we had with Noel in that discussion was about keeping a prefix, which is maintained in both cases. How does my previous comment contradict it?
The additional "On" prefix is what is present in NGO's and Small Scale template's codebases. As a user, I'd expect netcode code to stay consistent with the naming convention of the netcode framework I'm using.
There was a problem hiding this comment.
The alignment in that conversation was to name Rpcs invoked on clients or server as just ClientRpc, and ServerRpc. This line is a similar scenario, no?
We haven't established yet to add the prefix On on methods invoked on clients, server, or authority yet. We're starting those conversations offline, so let's align there!
There was a problem hiding this comment.
@RikuTheFuffs Let's proceed with the current Rpc naming and come back to refactors, to try and get this PR through.
| } | ||
|
|
||
| base.OnNetworkSpawn(); | ||
| } | ||
|
|
||
| private void OnClientConnected(ulong clientId) | ||
| private void ServerOnConnectionEvent(NetworkManager networkManager, ConnectionEventData connectionEventData) |
| [ClientRpc] | ||
| public void BroadcastGameOverClientRpc(GameOverReason reason) | ||
| [Rpc(SendTo.ClientsAndHost)] | ||
| public void ClientBroadcastGameOverRpc(GameOverReason reason) |
| private void OnClientConnectedCallback(ulong clientId) | ||
| /// <param name="networkManager"></param> | ||
| /// <param name="connectionEventData">Connection event to check for which player id is connecting.</param> | ||
| private void ServerOnConnectionEvent(NetworkManager networkManager, ConnectionEventData connectionEventData) |
| [ClientRpc] | ||
| private void SendClientReadyStatusUpdatesClientRpc(ulong clientId, bool isReady) | ||
| [Rpc(SendTo.ClientsAndHost)] | ||
| private void ClientSendReadyStatusUpdatesRpc(ulong clientId, bool isReady) |
| } | ||
|
|
||
| /// <summary> | ||
| /// SendClientReadyStatusUpdatesClientRpc | ||
| /// ClientSendReadyStatusUpdatesRpc |
There was a problem hiding this comment.
What value does this line in this comment provide?
| [ServerRpc(RequireOwnership = false)] | ||
| private void OnClientIsReadyServerRpc(ulong clientid) | ||
| [Rpc(SendTo.Server)] | ||
| private void ServerOnClientIsReadyRpc(ulong clientid) |
| [ClientRpc] | ||
| void SpawnExplosionVFXClientRPC(Vector3 spawnPosition, Quaternion spawnRotation) | ||
| [Rpc(SendTo.ClientsAndHost)] | ||
| void ClientSpawnExplosionVFXRpc(Vector3 spawnPosition, Quaternion spawnRotation) |
| [ServerRpc] | ||
| private void ShootServerRPC() | ||
| [Rpc(SendTo.Server)] | ||
| private void ServerShootRpc() |
| [ClientRpc] | ||
| private void NotifyGameOverClientRpc(GameOverReason reason, ClientRpcParams clientParams) | ||
| [Rpc(SendTo.Owner)] | ||
| private void ClientNotifyGameOverRpc(GameOverReason reason) |
Description
Issue Number(s)
MTT-8503 Upgrade Invaders to use Netcode For Gameobject 1.8.1 and changed code to new recommended API.
Contribution checklist