fix: Adding INetworkSerializeByMemcpy where recommended for network serialization [MTT-5912] (#822)

* adding INetworkSerializeByMemcpy where recommended

* changelog cleanup & addition

* comments inside lobbyplayerstate, index added, changelog revision

Author: fernando-cortez

Assets/Scripts/Gameplay/Action/ActionID.cs

@@ -7,15 +7,10 @@ namespace Unity.BossRoom.Gameplay.Actions
     /// This struct is used by Action system (and GameDataSource) to refer to a specific action in runtime.
     /// It wraps a simple integer.
     /// </summary>
-    public struct ActionID : INetworkSerializable, IEquatable<ActionID>
+    public struct ActionID : INetworkSerializeByMemcpy, IEquatable<ActionID>
     {
         public int ID;
 
-        public void NetworkSerialize<T>(BufferSerializer<T> serializer) where T : IReaderWriter
-        {
-            serializer.SerializeValue(ref ID);
-        }
-
         public bool Equals(ActionID other)
         {
             return ID == other.ID;

Assets/Scripts/Gameplay/GameState/NetworkCharSelection.cs

@@ -20,6 +20,12 @@ public enum SeatState : byte
         /// <summary>
         /// Describes one of the players in the lobby, and their current character-select status.
         /// </summary>
+        /// <remarks>
+        /// Putting FixedString inside an INetworkSerializeByMemcpy struct is not recommended because it will lose the
+        /// bandwidth optimization provided by INetworkSerializable -- an empty FixedString128Bytes serialized normally
+        /// or through INetworkSerializable will use 4 bytes of bandwidth, but inside an INetworkSerializeByMemcpy, that
+        /// same empty value would consume 132 bytes of bandwidth. 
+        /// </remarks>
         public struct LobbyPlayerState : INetworkSerializable, IEquatable<LobbyPlayerState>
         {
             public ulong ClientId;

Assets/Scripts/Infrastructure/NetworkGuid.cs

@@ -3,16 +3,10 @@
 
 namespace Unity.BossRoom.Infrastructure
 {
-    public class NetworkGuid : INetworkSerializable
+    public struct NetworkGuid : INetworkSerializeByMemcpy
     {
         public ulong FirstHalf;
         public ulong SecondHalf;
-
-        public void NetworkSerialize<T>(BufferSerializer<T> serializer) where T : IReaderWriter
-        {
-            serializer.SerializeValue(ref FirstHalf);
-            serializer.SerializeValue(ref SecondHalf);
-        }
     }
 
     public static class NetworkGuidExtensions

CHANGELOG.md

@@ -10,13 +10,12 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Changed
 * Replaced our polling for lobby updates with a subscription to the new Websocket based LobbyEvents (#805). This saves up a significant amount of bandwidth usage to and from the service, since updates are infrequent in this game. Now clients and hosts only use up bandwidth on the Lobby service when it is needed. With polling, we used to send a GET request per client once every 2s. The responses were between ~550 bytes and 900 bytes, so if we suppose an average of 725 bytes and 100 000 concurrent users (CCU), this amounted to around 725B * 30 calls per minute * 100 000 CCU = 2.175 GB per minute. Scaling this to a month would get us 93.96 TB per month. In our case, since the only changes to the lobbies happen when a user connects or disconnects, most of that data was not necessary and can be saved to reduce bandwidth usage. Since the cost of using the Lobby service depends on bandwidth usage, this would also save money on an actual game.
-* 
+* Simplified reconnection flow by offloading responsibility to ConnectionMethod (#804). Now the ClientReconnectingState uses the ConnectionMethod it is configured with to handle setting up reconnection (i.e. reconnecting to the Lobby before trying to reconnect to the Relay server if it is using Relay and Lobby). It can now also fail early and stop retrying if the lobby doesn't exist anymore.
+
 ### Cleanup
 * Clarified a TODO comment inside ClientCharacter, detailing how anticipation should only be executed on owning client players (#786)
-* Removed now unnecessary cached NetworkBehaviour status on some components, since they now do not allocate memory (#799)
-
-### Changed
-* Simplified reconnection flow by offloading responsibility to ConnectionMethod (#804). Now the ClientReconnectingState uses the ConnectionMethod it is configured with to handle setting up reconnection (i.e. reconnecting to the Lobby before trying to reconnect to the Relay server if it is using Relay and Lobby). It can now also fail early and stop retrying if the lobby doesn't exist anymore.
+* Removed now unnecessary cached NetworkBehaviour status on some components, since they now do not allocate memory (#799) 
+* Certain structs converted to implement interface INetworkSerializeByMemcpy instead of INetworkSerializable (#822) INetworkSerializeByMemcpy optimizes for performance at the cost of bandwidth usage and flexibility, however it will only work with structs containing value types. For more details see the official [doc](https://docs-multiplayer.unity3d.com/netcode/current/advanced-topics/serialization/inetworkserializebymemcpy/index.html).
 
 ### Fixed
 * EnemyPortals' VFX get disabled and re-enabled once the breakable crystals are broken (#784)

README.md

@@ -169,6 +169,7 @@ Running the game over internet currently requires setting up a relay.
 * Client driven movements - Boss Room is server driven with anticipation animation. See [Client Driven bitesize](https://github.com/Unity-Technologies/com.unity.multiplayer.samples.bitesize/tree/main/Basic/ClientDriven) for client driven gameplay
 * Player spawn - SpawnPlayer() in [Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs](Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs)
 * Player camera setup (with cinemachine) - OnNetworkSpawn() in [Assets/Scripts/Gameplay/GameplayObjects/Character/ClientCharacter.cs](Assets/Scripts/Gameplay/GameplayObjects/Character/ClientCharacter.cs)
+* INetworkSerializable (bandwidth optimization) vs INetworkSerializeByMemcpy (performance optimization) usage. See LobbyPlayerState vs ActionID structs [Assets/Scripts/Gameplay/GameState/NetworkCharSelection.cs](Assets/Scripts/Gameplay/GameState/NetworkCharSelection.cs) vs [Assets/Scripts/Gameplay/Action/ActionID.cs](Assets/Scripts/Gameplay/Action/ActionID.cs)
 
 ### Game Flow
 * Application Controller - [Assets/Scripts/ApplicationLifecycle/ApplicationController.cs ](Assets/Scripts/ApplicationLifecycle/ApplicationController.cs)